Skip to content

Fix secret placeholder in zxp installer#372

Merged
sandipanpanda merged 4 commits into
mainfrom
fix-installer-secret
May 1, 2026
Merged

Fix secret placeholder in zxp installer#372
sandipanpanda merged 4 commits into
mainfrom
fix-installer-secret

Conversation

@sandipanpanda
Copy link
Copy Markdown
Contributor

@sandipanpanda sandipanpanda commented Apr 29, 2026

Fix secret placeholder in zxp installer

📚 Description of Changes

Provide an overview of your changes and why they’re needed. Link to any related issues (e.g., "Fixes #123"). If your PR fixes a bug, resolves a feature request, or updates documentation, please explain how.

  • What Changed:
    (Describe the modifications, additions, or removals.)

  • Why This Change:
    (Explain the problem this PR addresses or the improvement it provides.)

  • Affected Components:
    (Which component does this change affect? - put x for all components)

  • Compose

  • K8s

  • Other (please specify)

❓ Motivation and Context

Why is this change required? What problem does it solve?

  • Context:
    (Provide background information or link to related discussions/issues.)

  • Relevant Tasks/Issues:
    (e.g., Fixes: #GitHub Issue)

🔍 Types of Changes

Indicate which type of changes your code introduces (check all that apply):

  • BUGFIX: Non-breaking fix for an issue.
  • NEW FEATURE: Non-breaking addition of functionality.
  • BREAKING CHANGE: Fix or feature that causes existing functionality to not work as expected.
  • ENHANCEMENT: Improvement to existing functionality.
  • CHORE: Changes that do not affect production (e.g., documentation, build tooling, CI).

🔬 QA / Verification Steps

Describe the steps a reviewer should take to verify your changes:

  1. (Step one: e.g., "Run make test to verify all tests pass.")
  2. (Step two: e.g., "Deploy to a Kind cluster with make create-kind && make deploy.")
  3. (Additional steps as needed.)

✅ Global Checklist

Please check all boxes that apply:

  • I have read and followed the CONTRIBUTING guidelines.
  • My code follows the code style of this project.
  • I have updated the documentation as needed.
  • I have added tests that cover my changes.
  • All new and existing tests have passed locally.
  • I have run this code in a local environment to verify functionality.
  • I have considered the security implications of this change.

Summary by Gitar

  • Installer configuration:
    • Added a transformation in Makefile to dynamically inject the zxporter_namespace into the installer bundle using yq.

This will update automatically on new commits.

Comment thread Makefile
@sandipanpanda sandipanpanda enabled auto-merge (squash) April 29, 2026 17:02
Comment thread Makefile
Comment on lines 321 to +324
@$(YQ) -i '(select(.kind == "ConfigMap" and .metadata.name == "devzero-zxporter-env-config") | .data.DAKR_URL) = "{{ .api_url }}/dakr"' $(DIST_BACKEND_INSTALL_BUNDLE)
@$(YQ) -i '(select(.kind == "Deployment") | .spec.template.spec.containers[]? | select(.image == "ttl.sh/zxporter:latest")).image = "docker.io/devzeroinc/zxporter:latest"' $(DIST_BACKEND_INSTALL_BUNDLE)
@$(YQ) -i '(select(.kind == "Secret" and .metadata.name == "devzero-zxporter-token") | .stringData.CLUSTER_TOKEN) = "{{ .cluster_token }}"' $(DIST_BACKEND_INSTALL_BUNDLE)
@$(YQ) -i '(select(.kind == "Namespace" and .metadata.labels."app.kubernetes.io/managed-by" == "kustomize") | .metadata.name) = "{{.zxporter_namespace}}"' $(DIST_BACKEND_INSTALL_BUNDLE)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Bug: Inconsistent Go-template placeholder spacing may break rendering

The new namespace placeholder on line 324 uses {{.zxporter_namespace}} (no spaces inside braces), while every other placeholder in the same target uses {{ .placeholder }} (with spaces). Although Go templates accept both forms, if the consuming template engine or string-replacement logic expects a consistent format (e.g., a regex like {{ \.\w+ }}), the no-space variant will silently fail to be substituted at install time, leaving a literal {{.zxporter_namespace}} in the rendered YAML.

Please verify the template engine consuming these placeholders. If it's standard Go text/template, both forms work. If it's a custom regex-based replacer, this will break.

Suggested fix:

@$(YQ) -i '(select(.kind == "Namespace" and .metadata.labels."app.kubernetes.io/managed-by" == "kustomize") | .metadata.name) = "{{ .zxporter_namespace }}"' $(DIST_BACKEND_INSTALL_BUNDLE)

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Apr 30, 2026

Code Review ⚠️ Changes requested 1 resolved / 2 findings

Removes stale cluster token placeholders from the bundle, but the new namespace placeholder on line 324 requires corrected template spacing to prevent rendering failures.

⚠️ Bug: Inconsistent Go-template placeholder spacing may break rendering

📄 Makefile:321-324

The new namespace placeholder on line 324 uses {{.zxporter_namespace}} (no spaces inside braces), while every other placeholder in the same target uses {{ .placeholder }} (with spaces). Although Go templates accept both forms, if the consuming template engine or string-replacement logic expects a consistent format (e.g., a regex like {{ \.\w+ }}), the no-space variant will silently fail to be substituted at install time, leaving a literal {{.zxporter_namespace}} in the rendered YAML.

Please verify the template engine consuming these placeholders. If it's standard Go text/template, both forms work. If it's a custom regex-based replacer, this will break.

Suggested fix
@$(YQ) -i '(select(.kind == "Namespace" and .metadata.labels."app.kubernetes.io/managed-by" == "kustomize") | .metadata.name) = "{{ .zxporter_namespace }}"' $(DIST_BACKEND_INSTALL_BUNDLE)
✅ 1 resolved
Edge Case: Placeholder '{{ .cluster_token }}' left in bundle when CLUSTER_TOKEN empty

📄 Makefile:388-390
When CLUSTER_TOKEN is empty, the new conditional (lines 388-390) skips the sed replacement entirely, leaving the literal placeholder CLUSTER_TOKEN: '{{ .cluster_token }}' in the generated bundle. Downstream consumers of install.yaml / zxporter.yaml will see this raw Go-template string instead of an empty or absent value.

If the intent is to support a two-phase workflow where the placeholder is filled later (e.g., by Helm or an updater script), this is fine and you can ignore this comment. But if a user runs make build-installer without setting CLUSTER_TOKEN and then applies the bundle directly, the Secret will contain the literal placeholder, which will silently fail at runtime.

🤖 Prompt for agents
Code Review: Removes stale cluster token placeholders from the bundle, but the new namespace placeholder on line 324 requires corrected template spacing to prevent rendering failures.

1. ⚠️ Bug: Inconsistent Go-template placeholder spacing may break rendering
   Files: Makefile:321-324

   The new namespace placeholder on line 324 uses `{{.zxporter_namespace}}` (no spaces inside braces), while every other placeholder in the same target uses `{{ .placeholder }}` (with spaces). Although Go templates accept both forms, if the consuming template engine or string-replacement logic expects a consistent format (e.g., a regex like `{{ \.\w+ }}`), the no-space variant will silently fail to be substituted at install time, leaving a literal `{{.zxporter_namespace}}` in the rendered YAML.
   
   Please verify the template engine consuming these placeholders. If it's standard Go `text/template`, both forms work. If it's a custom regex-based replacer, this will break.

   Suggested fix:
   @$(YQ) -i '(select(.kind == "Namespace" and .metadata.labels."app.kubernetes.io/managed-by" == "kustomize") | .metadata.name) = "{{ .zxporter_namespace }}"' $(DIST_BACKEND_INSTALL_BUNDLE)

Was this helpful? React with 👍 / 👎 | Gitar

@sandipanpanda sandipanpanda merged commit e9aee40 into main May 1, 2026
26 checks passed
@sandipanpanda sandipanpanda deleted the fix-installer-secret branch May 1, 2026 15:28
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