Skip to content

fix(arc): sync-wave for kyverno:update-nodes is an annotation, not a label - #12

Open
emmanuelbruno wants to merge 2 commits into
mainfrom
preservation/arc-kyverno-arm64-taint
Open

emmanuelbruno wants to merge 2 commits into
mainfrom
preservation/arc-kyverno-arm64-taint

Conversation

@emmanuelbruno

Copy link
Copy Markdown
Contributor

Follow-up from Copilot review on #8 (Moderate, 2 votes): ArgoCD only honors argocd.argoproj.io/sync-wave from metadata.annotations; it was under metadata.labels, so the intended -1 wave was a no-op.

Copilot's other #8 point (background scan not restoring the taint) assessed as a false positive: Kyverno v1.15 with background: true re-applies mutation to existing resources (mutateExisting defaults true since v1.12); the live resourceFilters Node exclusion is addressed by the values hook + sanctioned one-off CM patch.

Copilot AI lite review requested due to automatic review settings September 14, 2026 09:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The critical Kyverno ordering issue and moderate unmanaged resource-filter activation remain unresolved, with documentation inconsistencies also outstanding.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request updates Kyverno-based ARM node taint enforcement and configures ARC ARM runners to tolerate the taint.

Changes:

  • Corrects Argo CD sync-wave metadata.
  • Adds Kyverno policy, RBAC, and Kustomize wiring.
  • Updates ARC toleration, Helm cache-busters, and cluster documentation.
File summaries
File Reviewed change / final note
kubernetes/arc/README.md Documents taint enforcement and runner scheduling. Nit: activation should be repository-managed rather than an out-of-band patch.
kubernetes/arc/node-arm64-arch-taint.yaml Adds the ARM node mutation policy. Critical: cross-Application sync ordering may submit it before Kyverno is available.
kubernetes/arc/kyverno-node-mutation-rbac.yaml Adds aggregated permissions for background Node mutation.
kubernetes/arc/kustomization.yaml Includes the new policy and RBAC resources.
helm/releases/kyverno/values.yaml Updates Kyverno resource-filter configuration. Moderate: the required live patch remains outside Git reconciliation.
helm/releases/arc/arm-scale-set-values.yaml Adds the ARM runner toleration.
docs/cluster.md Updates node inventory and finding status. Nit: stale untainted-node statements remain.
bootstrap/appset-helm.yaml Bumps the Kyverno values cache-buster.
bootstrap/app-arc-arm-runners.yaml Bumps the ARC values cache-buster.
Review details

Suppressed comments (3)

docs/cluster.md:627

  • This update leaves contradictory statements elsewhere in docs/cluster.md: the capacity section still calls the arm64 worker “untainted” (around line 117), and finding #5 still says the live worker is untainted (around line 622). After this change those claims are stale; update or explicitly qualify them so the cluster documentation has one consistent taint state.
| 10 | Minor | Untainted arm64 test node — **resolved 2026-09-12** | `lima-k3s-agent` (joined 2026-09-07) carried no taints; unowned `arm-test` pod present | Tainted `kubernetes.io/arch=arm64:NoSchedule`, now GitOps-enforced via Kyverno `node-arm64-arch-taint` (admission+background) so it persists; ARC ARM runners tolerate it (`kubernetes/arc/`) |

helm/releases/kyverno/values.yaml:32

  • Documenting kubectl patch as the required way to activate this change leaves the live resourceFilters outside Git and contradicts the repository's reconciliation rule (AGENTS.md:87, which says to fix the source and refresh rather than patch live resources). A normal ArgoCD sync therefore cannot guarantee that Node admission is enabled on the existing cluster; make the ConfigMap update part of a managed render/reconciliation instead of relying on this untracked step.
#   kubectl patch configmap kyverno -n kyverno --type merge -p '{"data":{"resourceFilters":"<INTENDED STRING BELOW>"}}'

kubernetes/arc/README.md:106

  • This new documentation repeats an out-of-band live kubectl patch as part of the GitOps enforcement path. That conflicts with the repository rule in AGENTS.md:87 and means a normal sync can leave the existing cluster filtering Node admission even though the README says the policy is enforced; document a reconciled, repository-managed activation path instead.
  (no separate binding). Kyverno's `resourceFilters` must not exclude
  `[Node,*,*]` for the admission path to see Nodes — handled in
  `helm/releases/kyverno/values.yaml` (`config.resourceFiltersExclude`) plus a
  documented one-off patch to the live `keep`-annotated ConfigMap.
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +15 to +16
annotations:
argocd.argoproj.io/sync-wave: "0"
@emmanuelbruno
emmanuelbruno force-pushed the preservation/arc-kyverno-arm64-taint branch from e471ed8 to ce60d7d Compare September 14, 2026 11:05
@emmanuelbruno

Copy link
Copy Markdown
Contributor Author

/copilot review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants