Skip to content

DEVOPS-3406 - helm: namespace resouces - #1

Merged
raul-gherman-modaoperandi merged 4 commits into
devfrom
DEVOPS-3406
Sep 4, 2026
Merged

DEVOPS-3406 - helm: namespace resouces#1
raul-gherman-modaoperandi merged 4 commits into
devfrom
DEVOPS-3406

Conversation

@raul-gherman-modaoperandi

Copy link
Copy Markdown

Summary

  • Adds a top-level namespace value (default openwork) to the openwork-ee Helm chart and sets metadata.namespace from it on all 11 namespaced resources: ConfigMap, Secret, Ingress, migration Job, env-probe test Job, and the den-api / den-web / inference Services and Deployments.
  • Introduces an openwork-ee.namespace helper in templates/_helpers.tpl that falls back to .Release.Namespace when the value is cleared.
  • Documents the value in README.md (new "Namespace" section), surfaces the rendered namespace in templates/NOTES.txt, and adds a tests/namespace.sh chart check.

Why

  • Rendered manifests carried no metadata.namespace, so helm template ... | kubectl apply -f - pipelines fell back to the kubectl context namespace — the migration Job landed in kube-system.
  • Explicit namespacing pins every resource to the intended namespace regardless of how the chart is rendered/applied.

Issue

  • Closes #

Scope

  • packaging/helm/openwork-ee/values.yaml: new documented namespace: openwork value.
  • packaging/helm/openwork-ee/templates/_helpers.tpl: new openwork-ee.namespace helper.
  • metadata.namespace added in configmap.yaml, secret.yaml, migration-job.yaml, ingress.yaml, den-api.yaml (Service + Deployment), den-web.yaml (Service + Deployment), inference.yaml (Service + Deployment), tests/env-probe-job.yaml.
  • templates/NOTES.txt: prints the resolved namespace after install.
  • README.md: new "Namespace" section documenting the value, --namespace alignment for helm install, and the need to pre-create the namespace in helm template | kubectl apply flows.
  • tests/namespace.sh: new render-assertion chart check.

Out of scope

  • No changes to resource names, labels, selectors, or runtime env/config.
  • No new evals journey spec — chart render assertions in packaging/helm/openwork-ee/tests/*.sh are the established proof pattern for this chart (no journey spec covers chart rendering).
  • No changes to the example values files under packaging/helm/openwork-ee/examples/.

Testing

Ran

  • for test in packaging/helm/openwork-ee/tests/*.sh; do bash "$test"; done (with helm v3.16.3 on PATH)
  • helm template openwork-ee packaging/helm/openwork-ee (+ variants: --set ingress.enabled=true --set inference.enabled=true, --set namespace=platform, --set namespace=, --namespace rel-ns) to inspect rendered namespace: fields

Result

  • pass: all 7 chart check scripts, including namespace chart checks passed (default render puts all 8 default resources / 11 with ingress+inference in openwork; override wins everywhere; cleared value falls back to the release namespace).

CI status

  • pass: pending (run publish-ee-images workflow chart checks on push)
  • code-related failures: none observed locally
  • external/env/auth blockers: none

Manual verification

  1. Run helm template openwork-ee ./packaging/helm/openwork-ee and confirm every manifest has metadata.namespace: openwork (including the migration Job).
  2. Run helm template openwork-ee ./packaging/helm/openwork-ee --set namespace=platform and confirm all manifests use platform.
  3. Run helm template openwork-ee ./packaging/helm/openwork-ee --set namespace= --namespace rel-ns and confirm all manifests fall back to rel-ns.

Evidence

  • N/A (docs-only)

Risk

  • Low: metadata.namespace is declarative and matches the documented --namespace openwork install flow. Values-compatibility risk is minimal — existing releases installed with --namespace openwork (per README) render identical effective namespaces; only pipelines that relied on an implicit context namespace different from openwork must now set namespace explicitly.

Rollback

  • Revert the commit; rendered manifests return to relying on the release/kubectl context namespace.

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 namespace helper currently renders unquoted, which can produce invalid Kubernetes manifests for numeric namespace values (YAML scalar type mismatch).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the openwork-ee Helm chart to explicitly set metadata.namespace on all namespaced resources using a new top-level namespace value (defaulting to openwork), preventing helm template | kubectl apply flows from accidentally targeting the current kubectl context namespace.

Changes:

  • Adds namespace: openwork to chart values and introduces an openwork-ee.namespace helper (with fallback to .Release.Namespace when cleared).
  • Sets metadata.namespace across all namespaced manifests (including test/migration Jobs and optional Ingress/Inference resources).
  • Adds a chart render assertion script (tests/namespace.sh) and surfaces the resolved namespace in NOTES.txt and docs.
File summaries
File Description
packaging/helm/openwork-ee/values.yaml Introduces documented top-level namespace value (default openwork).
packaging/helm/openwork-ee/templates/_helpers.tpl Adds openwork-ee.namespace helper for consistent namespace selection.
packaging/helm/openwork-ee/templates/configmap.yaml Pins ConfigMap to resolved namespace.
packaging/helm/openwork-ee/templates/secret.yaml Pins Secret to resolved namespace.
packaging/helm/openwork-ee/templates/migration-job.yaml Pins migration Job to resolved namespace (prevents kube-system mishaps).
packaging/helm/openwork-ee/templates/tests/env-probe-job.yaml Pins Helm test Job to resolved namespace.
packaging/helm/openwork-ee/templates/den-api.yaml Pins den-api Service/Deployment to resolved namespace.
packaging/helm/openwork-ee/templates/den-web.yaml Pins den-web Service/Deployment to resolved namespace.
packaging/helm/openwork-ee/templates/inference.yaml Pins inference Service/Deployment to resolved namespace when enabled.
packaging/helm/openwork-ee/templates/ingress.yaml Pins Ingress to resolved namespace when enabled.
packaging/helm/openwork-ee/templates/NOTES.txt Prints resolved namespace and clarifies namespaced targeting.
packaging/helm/openwork-ee/tests/namespace.sh Adds render-time assertions for default/override/fallback namespace behavior.
packaging/helm/openwork-ee/README.md Documents the new namespace behavior and recommended install/apply flows.
Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 2
  • 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 thread packaging/helm/openwork-ee/templates/_helpers.tpl
Comment thread packaging/helm/openwork-ee/README.md
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 4, 2026 12:22
Co-authored-by: raul-gherman-modaoperandi <68240333+raul-gherman-modaoperandi@users.noreply.github.com>

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.

🔵 Needs a closer look

The namespace helper renders unquoted YAML scalars, which can cause invalid manifests if users set a numeric/bool-like namespace via --set, and the new test script should be updated to assert the corrected quoted rendering.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

packaging/helm/openwork-ee/tests/namespace.sh:44

  • These assertions look for unquoted namespace: ... lines. If the namespace helper is fixed to quote the value (to prevent numeric/bool YAML parsing), this script should assert quoted output and ideally add a regression check for a numeric --set namespace=123 override.

packaging/helm/openwork-ee/templates/_helpers.tpl:20

  • openwork-ee.namespace currently renders unquoted. If a user sets the namespace via --set namespace=123, Helm will type it as a number and the rendered YAML will emit namespace: 123 (non-string), which the Kubernetes API rejects for metadata.namespace. Quoting here also avoids YAML plain-scalar surprises (e.g. on, yes).
{{- define "openwork-ee.namespace" -}}
{{- .Values.namespace | default .Release.Namespace | quote -}}
{{- end -}}
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 4, 2026 12:25

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.

🟢 Approval recommended

The namespace helper is consistently applied across all chart resources and is backed by automated render-assertion tests covering default, override, and fallback behavior.

Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 4, 2026 12:40

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.

🟢 Approval recommended

The namespace helper is consistently applied across all chart templates and is backed by a dedicated render-assertion test covering defaults, overrides, fallbacks, and YAML-typing edge cases.

Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@raul-gherman-modaoperandi
raul-gherman-modaoperandi merged commit b3febe9 into dev Sep 4, 2026
1 check passed
@raul-gherman-modaoperandi
raul-gherman-modaoperandi deleted the DEVOPS-3406 branch September 4, 2026 13:05
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