Skip to content

feat(chart): align chart/ and charts/apl-operator - #3570

Open
CasLubbers wants to merge 16 commits into
mainfrom
APL-2095
Open

feat(chart): align chart/ and charts/apl-operator#3570
CasLubbers wants to merge 16 commits into
mainfrom
APL-2095

Conversation

@CasLubbers

Copy link
Copy Markdown
Contributor

📌 Summary

🔍 Reviewer Notes

🧹 Checklist

  • Code is readable, maintainable, and robust.
  • Unit tests added/updated

@CasLubbers
CasLubbers marked this pull request as ready for review August 25, 2026 08:52
Copilot AI lite review requested due to automatic review settings August 25, 2026 08:52
@svcAPLBot

Copy link
Copy Markdown
Contributor

Comparison of Helm chart templating output:

# apl-operator/templates/deployment.yaml

@@ metadata @@
! - one map entry removed:
- annotations:
-   argocd.argoproj.io/sync-options: "Force=true,Replace=true"
! + one map entry added:
+ namespace: apl-operator

@@ metadata.labels @@
! - two map entries removed:
- helm.sh/chart: apl-operator-0.2.0
- app.kubernetes.io/version: "1.16.0"

@@ spec.template.spec @@
! + one map entry added:
+ # Appended to the cluster resolvers, as a fallback while CoreDNS starts.
+ dnsConfig:
+   nameservers:
+   - "8.8.8.8"
+   - "8.8.4.4"

@@ spec.template.spec.securityContext @@
! + one map entry added:
+ seccompProfile:
+   type: RuntimeDefault

@@ spec.template.spec.securityContext.fsGroup @@
! ± value change
- 2000
+ 3000

@@ spec.template.spec.securityContext.runAsGroup @@
! ± value change
- 1001
+ 999

@@ spec.template.spec.securityContext.runAsUser @@
! ± value change
- 1001
+ 999

@@ spec.template.spec.containers.apl-operator @@
! - one map entry removed:
- envFrom:
- - secretRef:
-     name: apl-sops-secrets
-     optional: true

@@ spec.template.spec.containers.apl-operator.securityContext @@
! + one map entry added:
+ allowPrivilegeEscalation: false

@@ spec.template.spec.containers.apl-operator.env @@
! - one list entry removed:
- - name: CI
-   value: "true"
! + three list entries added:
+ - name: VERBOSITY
+   value: "1"
+ - name: ENV_DIR
+   value: /home/app/stack/env
+ - name: VALUES_INPUT
+   value: /secret/values.yaml

@@ spec.template.spec.containers.apl-operator.env.POLL_INTERVAL_MS.value @@
! ± value change in multiline text (one insert, one deletion)
- 15000
+ 30000

@@ spec.template.spec.containers.apl-operator.volumeMounts @@
! + one list entry added:
+ - name: values-secret
+   mountPath: /secret

@@ spec.template.spec.volumes @@
! + one list entry added:
+ - name: values-secret
+   secret:
+     secretName: apl-values
+     optional: true

# apl-operator/templates/rbac.yaml

@@ metadata.labels @@
# v1/ServiceAccount/apl-operator/apl-operator
! - two map entries removed:
- helm.sh/chart: apl-operator-0.2.0
- app.kubernetes.io/version: "1.16.0"

# Old file deleted: apl-operator/templates/secrets.yaml
# rabbitmq-cluster-operator/templates/messaging-topology-operator/validating-webhook-configuration.yaml

# values-repo.yaml

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns the standalone operator chart (charts/apl-operator) with the installer chart (chart/apl) so both render the same operator Deployment during the handoff from install-time deployment to ArgoCD-managed deployment, and improves operator liveness during long installs.

Changes:

  • Standardize operator Deployment rendering across chart/apl and charts/apl-operator (shared template logic, shared values keys, fixed secret naming, label alignment).
  • Add an install-phase heartbeat so the liveness probe doesn’t kill the pod during long-running installation steps.
  • Add a lint guard (bin/diff-operator-charts.sh) to detect drift between the two charts’ rendered operator Deployments.

Reviewed changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
values/apl-operator/apl-operator.gotmpl Aligns Helmfile-rendered operator values with the updated chart interface (release-aware ORCS mirroring, pull secret names).
src/operator/main.ts Starts/stops a heartbeat timer around long install steps to satisfy liveness constraints.
src/operator/k8s.ts Adds configurable heartbeat file support and a startHeartbeat() helper.
src/operator/k8s.test.ts Adds unit tests validating startHeartbeat() behavior under fake timers.
package.json Adds lint:operator-charts to enforce chart alignment during linting.
charts/apl-operator/values.yaml Adds/aligns shared values keys (namespace/otomi/cluster/valuesSecretName), security contexts, resources, and readiness settings.
charts/apl-operator/templates/secrets.yaml Removes the chart-managed SOPS secret manifest.
charts/apl-operator/templates/deployment.yaml Reworks image/repository selection, pull secret handling, env/volume wiring for mounted values, and install-skip gating.
charts/apl-operator/templates/_helpers.tpl Aligns labels to avoid chart/version label mismatches when rendering the same objects from different charts.
chart/apl/values.yaml Introduces operator chart-compatible values block (mirroring charts/apl-operator) and removes now-redundant operator image repository config.
chart/apl/templates/values-secrets.yaml Switches to a fixed secret name shared with charts/apl-operator for mounting values.
chart/apl/templates/rbac.yaml Fixes labels indentation to render valid metadata.
chart/apl/templates/post-job.yaml Aligns image repository/tag selection logic with the shared deployment behavior.
chart/apl/templates/_helpers.tpl Mirrors label alignment change made in charts/apl-operator.
bin/diff-operator-charts.sh Adds a Helm-render diff check to prevent operator Deployment drift between installer and ArgoCD charts.

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

Comment on lines +68 to +72
value: '1'
- name: ENV_DIR
value: /home/app/stack/env
- name: VALUES_INPUT
value: /secret/values.yaml

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not relevant, because we do not support a SOPS recovery, which would be a legacy version.

Comment on lines +37 to +41
# Appended to the cluster resolvers, as a fallback while CoreDNS starts.
dnsConfig:
nameservers:
- 8.8.8.8
- 8.8.4.4

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good suggestion, but since it is a new feature and should be consistently in the platform I would consider it out of scope here.

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.

4 participants