[platform] Update lineage labels at upgrade - #1452
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces a Secret-specific label with boolean Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant K as Kubernetes API (Admission)
participant W as Lineage Webhook
participant C as computeLabels/applyLabels
Note over K,W: Admission request for resource (Secret / others)
K->>W: AdmissionReview
W->>C: matchLabelsToExcludeInclude(...)
C->>C: set apps.cozystack.io/tenantresource = "true"/"false"
C->>W: mutated object (labels applied)
W->>K: AdmissionResponse (mutated object)
Note over C: VMCluster special-case removed — unified path
sequenceDiagram
autonumber
participant S as scripts/migrations/20
participant K as kubectl / Kubernetes API
S->>S: ts := RFC3339(nano)
S->>K: check HelmRelease cozy-system/cozystack-controller & cozy-system/cozystack-api
alt HelmReleases exist
S->>K: annotate reconcile.fluxcd.io/forceAt=<ts>, reconcile.fluxcd.io/requestedAt=<ts>
S->>K: kubectl wait --for=condition=Ready --timeout=4m HelmRelease/...
alt Ready
S->>K: for each namespace tenant-* : annotate (pods,svc,pvc,secrets,ingresses,workloadmonitors) migration.cozystack.io=<ts> --overwrite
S->>K: kubectl apply ConfigMap cozy-system/cozystack-version (version=21)
else Timeout/Not Ready
S->>S: exit 1
end
else Not found
S->>K: annotate tenant namespaces and apply ConfigMap regardless
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello Timofei Larkin (@lllamnyp), I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a critical migration script designed to retroactively apply lineage labels to pre-existing Kubernetes resources across the platform. By annotating resources like pods, services, and secrets, the script triggers the lineage webhook to re-evaluate and assign labels that link them to their managing applications, thereby enhancing resource traceability and dependency management during system upgrades. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a migration script to update lineage labels on existing resources and includes a minor change in the lineage webhook. While the idea is sound, the implementation has a few issues. The migration script contains a critical typo that will cause it to fail, a variable scoping issue, and a misleading comment. In the webhook code, a new TODO comment highlights a potential security vulnerability that should be addressed rather than postponed. I've provided four specific comments detailing these issues and suggesting fixes.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/lineagecontrollerwebhook/webhook.go(1 hunks)scripts/migrations/20(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
internal/lineagecontrollerwebhook/webhook.go (1)
138-139: LGTM!The TODO comment provides useful context about future enhancement needs for secret visibility management.
scripts/migrations/20 (2)
16-16: LGTM!The ConfigMap creation and version update logic is correct. The dry-run approach ensures idempotent application.
5-12: Potential undefined variable usage.If the
kubectl get helmreleasecommand fails (helmrelease doesn't exist), thetimestampvariable won't be defined, but it's still used in line 14. Consider initializing the timestamp outside the conditional block.+timestamp=$(date --rfc-3339=ns) if kubectl get helmrelease cozystack-controller -n cozy-system; then - timestamp=$(date --rfc-3339=ns) kubectl annotate helmrelease cozystack-controller -n cozy-system \ reconcile.fluxcd.io/forceAt="$timestamp" \ reconcile.fluxcd.io/requestedAt="$timestamp" \ --overwrite kubectl wait hr/cozystack-controller -n cozy-system --timeout=4m --for=condition=ready || exit 1 fiLikely an incorrect or invalid review comment.
401883d to
3b5af92
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/system/cozystack-controller/templates/mutatingwebhookconfiguration.yaml (1)
24-30: Consider adding timeoutSeconds and matchPolicy for robustness.Prevents long hangs and future-proofs version transitions on CRDs.
You can add (outside the changed lines) under the webhook item:
timeoutSeconds: 10 matchPolicy: Equivalentscripts/migrations/20 (4)
4-4: Make timestamp portable and shell-safe (avoid GNU-only flag and spaces).
date --rfc-3339=nsis GNU-specific and produces a space; prefer POSIX/portable ISO-8601 UTC.Apply this diff:
-timestamp=$(date --rfc-3339=ns) +# Use portable ISO-8601 UTC (avoids spaces and GNU-only flags) +timestamp=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
11-11: Avoid relying on resource shortname aliashr/.Use the full kind to prevent failures where the shortname isn’t registered.
-kubectl wait hr/cozystack-controller -n cozy-system --timeout=4m --for=condition=ready || exit 1 +kubectl wait helmrelease/cozystack-controller -n cozy-system --timeout=4m --for=condition=ready || exit 1
1-1: Enable strict mode for safer script execution.Fail fast on errors/undefined vars and propagate pipeline failures.
#!/bin/sh +set -euo pipefail
15-15: Harden namespace read loop.Use a robust read to avoid edge cases with IFS/trailing backslashes.
- while read namespace ; do + while IFS= read -r namespace; do
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/lineagecontrollerwebhook/webhook.go(1 hunks)packages/system/cozystack-controller/templates/mutatingwebhookconfiguration.yaml(1 hunks)scripts/migrations/20(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/lineagecontrollerwebhook/webhook.go
🔇 Additional comments (1)
packages/system/cozystack-controller/templates/mutatingwebhookconfiguration.yaml (1)
24-30: Good expansion of webhook coverage (ingresses + workloadmonitors).This aligns with the migration and should trigger lineage updates for these resources.
Please confirm:
- The webhook handler supports these GVRs (networking.k8s.io/v1/ingresses and cozystack.io/v1alpha1/workloadmonitors).
- Controller RBAC includes get/list/watch on both resources to resolve ancestry during mutation.
A new dashboard based on https://github.com/PRO-Robotech/openapi-ui project <img width="1720" height="1373" alt="Screenshot 2025-08-01 at 09-01-00 OpenAPI UI" src="proxy.php?url=https%3A%2F%2Fgithub.com%2Fcozystack%2Fcozystack%2Fpull%2F%3Ca+href%3D"https://github.com/user-attachments/assets/7ae04789-24ec-4e4b-830b-6f16e96513eb">https://github.com/user-attachments/assets/7ae04789-24ec-4e4b-830b-6f16e96513eb" /> <img width="1720" height="1373" alt="Screenshot 2025-08-01 at 09-01-14 OpenAPI UI" src="proxy.php?url=https%3A%2F%2Fgithub.com%2Fcozystack%2Fcozystack%2Fpull%2F%3Ca+href%3D"https://github.com/user-attachments/assets/ca5aa85d-43f0-4b5b-b87a-3bc237834f10">https://github.com/user-attachments/assets/ca5aa85d-43f0-4b5b-b87a-3bc237834f10" /> <img width="1720" height="1373" alt="Screenshot 2025-08-01 at 09-02-05 OpenAPI UI" src="proxy.php?url=https%3A%2F%2Fgithub.com%2Fcozystack%2Fcozystack%2Fpull%2F%3Ca+href%3D"https://github.com/user-attachments/assets/ebee7bfa-c3ac-4fe6-b5e1-43e9e7042c6a">https://github.com/user-attachments/assets/ebee7bfa-c3ac-4fe6-b5e1-43e9e7042c6a" /> <!-- Thank you for making a contribution! Here are some tips for you: - Start the PR title with the [label] of Cozystack component: - For system components: [platform], [system], [linstor], [cilium], [kube-ovn], [dashboard], [cluster-api], etc. - For managed apps: [apps], [tenant], [kubernetes], [postgres], [virtual-machine] etc. - For development and maintenance: [tests], [ci], [docs], [maintenance]. - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same [label] as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note [cozystack-api] Implement TenantNamespace, TenantModules, TenantSecret and TenantSecretsTable resources [cozystack-controller] Introduce new dashboard-controller [dashboard] Introduce new dashboard based on openapi-ui ``` Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
3b5af92 to
23e59ea
Compare
Since the VictoriaMetrics operator aggressively manages the metadata on all owned components, the addition of labels by the lineage webhook causes non-stop updates sent to the k8s API server. We mitigate this by modifying the Monitoring Helm chart to set the `managedMetadata` field on all VictoriaMetrics custom resources, where applicable. ```release-note [monitoring] Explicitly set lineage labels on VictoriaMetrics' resources known not to play nice when something modifies their owned resources in flight. ``` Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/lineagecontrollerwebhook/webhook.go (1)
118-132: Sanitize label values and guard against nil map entry.
- application.name can exceed 63 chars; will cause admission failures. Sanitize like you already do for .group.
- Potential nil/absent entry from appCRDMap can panic. Add ok check and default to "false".
Apply this diff to harden the CRD lookup:
- cfg := h.config.Load().(*runtimeConfig) - crd := cfg.appCRDMap[appRef{gv.Group, obj.GetKind()}] + cfg := h.config.Load().(*runtimeConfig) + crd, ok := cfg.appCRDMap[appRef{gv.Group, obj.GetKind()}] + if !ok { + labels["apps.cozystack.io/tenantresource"] = "false" + return labels, err + }Then update label value sanitation (outside the changed hunk); for example:
// helper near this file func sanitizeLabelValue(s string) string { if len(s) <= 63 { // ensure it ends alnum for l := len(s); l > 0; l-- { b := s[l-1] if (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') { return s[:l] } } return s } s = s[:63] for l := len(s); l > 0; l-- { b := s[l-1] if (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') { return s[:l] } } return s }And use it when building labels:
labels := map[string]string{ "apps.cozystack.io/application.group": sanitizeLabelValue(gv.Group), "apps.cozystack.io/application.kind": sanitizeLabelValue(obj.GetKind()), "apps.cozystack.io/application.name": sanitizeLabelValue(obj.GetName()), }Also applies to: 138-146
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
internal/lineagecontrollerwebhook/webhook.go(1 hunks)packages/extra/monitoring/templates/alerta/alerta.yaml(1 hunks)packages/extra/monitoring/templates/vlogs/vlogs.yaml(1 hunks)packages/extra/monitoring/templates/vm/vmalert.yaml(1 hunks)packages/extra/monitoring/templates/vm/vmcluster.yaml(1 hunks)packages/system/cozystack-controller/templates/mutatingwebhookconfiguration.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/system/cozystack-controller/templates/mutatingwebhookconfiguration.yaml
🔇 Additional comments (4)
packages/extra/monitoring/templates/vm/vmalert.yaml (1)
8-12: managedMetadata.labels is supported in VMAlertSpec (operator.victoriametrics.com/v1beta1) — no changes required.packages/extra/monitoring/templates/vlogs/vlogs.yaml (1)
7-11: No changes needed: spec.managedMetadata.labels is valid for VLogs.packages/extra/monitoring/templates/alerta/alerta.yaml (1)
235-239: No changes required: VMAlertmanagerSpec supports spec.managedMetadata.labels.packages/extra/monitoring/templates/vm/vmcluster.yaml (1)
8-12: No changes needed – spec.managedMetadata.labels is supported
VMCluster CRD (operator.victoriametrics.com/v1beta1) has supported spec.managedMetadata.labels since v0.51.0; manifests will be accepted.
| managedMetadata: | ||
| labels: | ||
| apps.cozystack.io/application.group: apps.cozystack.io | ||
| apps.cozystack.io/application.kind: Monitoring | ||
| apps.cozystack.io/application.name: {{ $.Release.Name }} |
There was a problem hiding this comment.
Truncate label value to <=63 chars.
Prevent label overflow on operator-managed resources.
Apply this diff:
- apps.cozystack.io/application.name: {{ $.Release.Name }}
+ apps.cozystack.io/application.name: {{ $.Release.Name | trunc 63 | trimSuffix "-" }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| managedMetadata: | |
| labels: | |
| apps.cozystack.io/application.group: apps.cozystack.io | |
| apps.cozystack.io/application.kind: Monitoring | |
| apps.cozystack.io/application.name: {{ $.Release.Name }} | |
| managedMetadata: | |
| labels: | |
| apps.cozystack.io/application.group: apps.cozystack.io | |
| apps.cozystack.io/application.kind: Monitoring | |
| apps.cozystack.io/application.name: {{ $.Release.Name | trunc 63 | trimSuffix "-" }} |
🤖 Prompt for AI Agents
In packages/extra/monitoring/templates/alerta/alerta.yaml around lines 235-239,
the apps.cozystack.io/application.name label uses the full Release.Name which
can exceed 63 characters; modify the template to emit a truncated value (max 63
chars) for that label, e.g. replace the raw Release.Name usage with a Helm/Sprig
truncation of the release name so the label value is guaranteed <=63 characters.
| managedMetadata: | ||
| labels: | ||
| apps.cozystack.io/application.group: apps.cozystack.io | ||
| apps.cozystack.io/application.kind: Monitoring | ||
| apps.cozystack.io/application.name: {{ $.Release.Name }} |
There was a problem hiding this comment.
Truncate label value to <=63 chars.
Same fix to avoid invalid labels on generated resources.
Apply this diff:
- apps.cozystack.io/application.name: {{ $.Release.Name }}
+ apps.cozystack.io/application.name: {{ $.Release.Name | trunc 63 | trimSuffix "-" }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| managedMetadata: | |
| labels: | |
| apps.cozystack.io/application.group: apps.cozystack.io | |
| apps.cozystack.io/application.kind: Monitoring | |
| apps.cozystack.io/application.name: {{ $.Release.Name }} | |
| managedMetadata: | |
| labels: | |
| apps.cozystack.io/application.group: apps.cozystack.io | |
| apps.cozystack.io/application.kind: Monitoring | |
| apps.cozystack.io/application.name: {{ $.Release.Name | trunc 63 | trimSuffix "-" }} |
🤖 Prompt for AI Agents
In packages/extra/monitoring/templates/vlogs/vlogs.yaml around lines 7 to 11,
the generated label value uses {{ $.Release.Name }} which can exceed the
63-character limit for Kubernetes labels; change the template to truncate the
release name to 63 characters (e.g., use the Helm/Sprig trunc function) so the
label value is at most 63 chars and remains valid, keeping the rest of the label
unchanged.
| managedMetadata: | ||
| labels: | ||
| apps.cozystack.io/application.group: apps.cozystack.io | ||
| apps.cozystack.io/application.kind: Monitoring | ||
| apps.cozystack.io/application.name: {{ $.Release.Name }} |
There was a problem hiding this comment.
Truncate label value to <=63 chars.
Same risk here; ensure Release.Name-based label is safe.
Apply this diff:
- apps.cozystack.io/application.name: {{ $.Release.Name }}
+ apps.cozystack.io/application.name: {{ $.Release.Name | trunc 63 | trimSuffix "-" }}🤖 Prompt for AI Agents
In packages/extra/monitoring/templates/vm/vmalert.yaml around lines 8 to 12, the
label value uses {{ $.Release.Name }} which can exceed the 63-character limit
for Kubernetes label values; change the template to truncate and sanitize the
release name to <=63 characters and ensure DNS-1123 label compliance (lowercase,
replace invalid chars with '-', remove leading/trailing '-' if any). Update the
template expression to pipe $.Release.Name through truncation to 63 chars, then
a regex replace to map invalid characters to '-', trim any leading/trailing '-'
and convert to lowercase so the resulting label value is guaranteed <=63 chars
and valid.
| managedMetadata: | ||
| labels: | ||
| apps.cozystack.io/application.group: apps.cozystack.io | ||
| apps.cozystack.io/application.kind: Monitoring | ||
| apps.cozystack.io/application.name: {{ $.Release.Name }} |
There was a problem hiding this comment.
Truncate label value to <=63 chars (Kubernetes label constraint).
Release.Name can exceed 63 chars, causing downstream object creation to fail when propagated as a label. Truncate and trim the trailing hyphen.
Apply this diff:
- apps.cozystack.io/application.name: {{ $.Release.Name }}
+ apps.cozystack.io/application.name: {{ $.Release.Name | trunc 63 | trimSuffix "-" }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| managedMetadata: | |
| labels: | |
| apps.cozystack.io/application.group: apps.cozystack.io | |
| apps.cozystack.io/application.kind: Monitoring | |
| apps.cozystack.io/application.name: {{ $.Release.Name }} | |
| managedMetadata: | |
| labels: | |
| apps.cozystack.io/application.group: apps.cozystack.io | |
| apps.cozystack.io/application.kind: Monitoring | |
| apps.cozystack.io/application.name: {{ $.Release.Name | trunc 63 | trimSuffix "-" }} |
🤖 Prompt for AI Agents
In packages/extra/monitoring/templates/vm/vmcluster.yaml around lines 8–12, the
apps.cozystack.io/application.name label uses {{ $.Release.Name }} which may
exceed Kubernetes' 63-character label limit; update the template to pipe the
release name through Helm string helpers to truncate to 63 characters and remove
any trailing hyphen (e.g., apply trunc 63 then trimSuffix "-" to the value) so
the label value is guaranteed <=63 chars and has no trailing hyphen.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/installer/images/cozystack/Dockerfile(1 hunks)scripts/migrations/20(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
| FROM alpine:3.22 | ||
|
|
||
| RUN wget -O- https://github.com/cozystack/cozypkg/raw/refs/heads/main/hack/install.sh | sh -s -- -v 1.1.0 | ||
| RUN wget -O- https://github.com/cozystack/cozypkg/raw/refs/heads/main/hack/install.sh | sh -s -- -v 1.2.0 |
There was a problem hiding this comment.
🧩 Analysis chain
Double-check the install.sh 1.2.0 bump.
Please confirm that cozypkg has published install script assets for version 1.2.0; otherwise the image build will fail when this line executes.
Run the following script to verify the asset exists:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect: HTTP 200 if install.sh for v1.2.0 is available.
curl -If https://github.com/cozystack/cozypkg/releases/download/v1.2.0/install.shLength of output: 1063
Ensure install.sh v1.2.0 exists or revert bump
curl against https://github.com/cozystack/cozypkg/releases/download/v1.2.0/install.sh returned HTTP 404, so the asset isn’t published. Either add the install.sh for v1.2.0 to the release or update this line to a published version.
Location: packages/core/installer/images/cozystack/Dockerfile:35
🤖 Prompt for AI Agents
In packages/core/installer/images/cozystack/Dockerfile around line 35, the
Dockerfile downloads install.sh for v1.2.0 which is not published (HTTP 404);
either publish the v1.2.0 install.sh asset to the CozyPkg release or change this
RUN line to reference a published release/tag (or a specific working URL) such
as an existing release version or the stable main/raw URL you have verified;
update the Dockerfile to use that verified URL/version and rebuild to confirm
the asset is reachable.
| kubectl get namespace -o custom-columns=NAME:.metadata.name --no-headers | | ||
| grep '^tenant-' | | ||
| while read namespace ; do | ||
| (set -x; \ | ||
| kubectl annotate \ | ||
| pods,services,pvc,secrets,ingresses.networking.k8s.io,workloadmonitors.cozystack.io \ | ||
| -n "$namespace" --all \ | ||
| migration.cozystack.io="$timestamp" --overwrite) | ||
| done |
There was a problem hiding this comment.
Prevent migration failure when no tenant namespaces exist.
With set -o pipefail enabled, the pipeline aborts if grep '^tenant-' returns exit code 1 (no matches). On clusters that haven’t created tenant namespaces yet, the migration exits early and never applies the annotations. Please make the filter stage tolerant of an empty result set. The awk rewrite below keeps the pipeline exit status at 0 while preserving the intended filtering.
Apply this diff:
-kubectl get namespace -o custom-columns=NAME:.metadata.name --no-headers |
- grep '^tenant-' |
- while read namespace ; do
+kubectl get namespace -o custom-columns=NAME:.metadata.name --no-headers |
+ awk '/^tenant-/ { print $1 }' |
+ while read -r namespace ; do📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| kubectl get namespace -o custom-columns=NAME:.metadata.name --no-headers | | |
| grep '^tenant-' | | |
| while read namespace ; do | |
| (set -x; \ | |
| kubectl annotate \ | |
| pods,services,pvc,secrets,ingresses.networking.k8s.io,workloadmonitors.cozystack.io \ | |
| -n "$namespace" --all \ | |
| migration.cozystack.io="$timestamp" --overwrite) | |
| done | |
| kubectl get namespace -o custom-columns=NAME:.metadata.name --no-headers | | |
| awk '/^tenant-/ { print $1 }' | | |
| while read -r namespace ; do | |
| (set -x; \ | |
| kubectl annotate \ | |
| pods,services,pvc,secrets,ingresses.networking.k8s.io,workloadmonitors.cozystack.io \ | |
| -n "$namespace" --all \ | |
| migration.cozystack.io="$timestamp" --overwrite) | |
| done |
🤖 Prompt for AI Agents
In scripts/migrations/20 around lines 14 to 22, the pipeline using grep
'^tenant-' will return exit code 1 when no tenant namespaces exist (causing
failures with set -o pipefail); replace the grep filter with an awk-based filter
that prints matching lines but exits 0 on no matches (e.g. pipe kubectl output
into awk '/^tenant-/{print}' ) so the pipeline remains tolerant of an empty
result set and the rest of the migration (the while loop and annotations) still
runs correctly; ensure the rest of the command including quoting of "$namespace"
and use of $timestamp is unchanged.
7e405b1 to
efaa145
Compare
This patch makes sure that migration #20 really uses the very latest chart versions by forcing a reconcile with cozypkg, instead of annotating the underlying HelmRelease. ```release-note [installer] Update cozypkg in installer and use it to bulletproof the 20th migration script by reconciling the HelmReleases with the --with-source flag. ``` Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
efaa145 to
0f8a9ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/migrations/20 (1)
1-4: /bin/sh doesn’t supportpipefail— switch to bash or droppipefail.With
#!/bin/shon many systems (e.g., Debian/Ubuntu dash),set -o pipefailis unsupported and will terminate the script. Easiest fix: use bash explicitly.-#!/bin/sh +#!/usr/bin/env bash
🧹 Nitpick comments (2)
scripts/migrations/20 (2)
10-10: Make timestamp generation portable (avoid GNU-specific--rfc-3339=ns).
date --rfc-3339=nsisn’t available on BusyBox/BSD/macOS. Use a widely supported ISO8601 UTC format without spaces.-timestamp=$(date --rfc-3339=ns) +timestamp="$(date -u +"%Y-%m-%dT%H:%M:%SZ")"
13-15: Remove redundant|| exit 1withset -e.
set -ealready aborts on non‑zero. These suffixes are unnecessary noise.-kubectl wait hr/cozystack-controller -n cozy-system --timeout=4m --for=condition=ready || exit 1 +kubectl wait hr/cozystack-controller -n cozy-system --timeout=4m --for=condition=ready ... -kubectl wait hr/cozystack-api -n cozy-system --timeout=4m --for=condition=ready || exit 1 +kubectl wait hr/cozystack-api -n cozy-system --timeout=4m --for=condition=ready
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/installer/images/cozystack/Dockerfile(1 hunks)scripts/migrations/20(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/installer/images/cozystack/Dockerfile
🔇 Additional comments (2)
scripts/migrations/20 (2)
7-7: Confirm scope ofcozystackresourcedefinitions.If it’s cluster-scoped,
--all-namespacesis unnecessary (and can error on some kubectl versions). If it’s namespaced, keep-A. Please verify the resource scope.
16-24: Don’t let grep + pipefail abort when no tenant namespaces exist.With
set -o pipefail,grep '^tenant-'exits 1 on no matches and kills the migration. Use an awk filter and read defensively.-kubectl get namespace -o custom-columns=NAME:.metadata.name --no-headers | - grep '^tenant-' | - while read namespace ; do +kubectl get namespace -o custom-columns=NAME:.metadata.name --no-headers | + awk '/^tenant-/ { print $1 }' | + while read -r namespace ; do
What this PR does
This patch adds a migration script, that adds an annotation to all resources that may be of interest, triggering an update event on the lineage webhook. This will analyze the ancestor tree of these resources and add labels to them, referencing their managing application.
Release note
Summary by CodeRabbit
Chores
Bug Fixes / Behavior Change
New Features