Skip to content

Fix: Duplicate Env Between Config Secrets - #2445

Open
asad-miah wants to merge 4 commits into
mainfrom
fix/duplicate-env-between-config-secrets
Open

Fix: Duplicate Env Between Config Secrets#2445
asad-miah wants to merge 4 commits into
mainfrom
fix/duplicate-env-between-config-secrets

Conversation

@asad-miah

Copy link
Copy Markdown
Collaborator

This PR introduces changes from the fix/duplicate-env-between-config-secrets branch.

📝 Summary

📁 Files Changed ( 5 files)

.github/workflows/lint.yml
kubernetes/clusters/dev/external-secrets/rhesis-app-secrets.yaml
kubernetes/clusters/prd/external-secrets/rhesis-app-secrets.yaml
kubernetes/clusters/stg/external-secrets/rhesis-app-secrets.yaml
scripts/check-k8s-secret-overlap.sh

📋 Commit Details

46affb65b - chore: remove unused RHESIS_ADMIN/USER/ANALYTICS_USER passwords (Md Asaduzzaman Miah, 2026-08-11 16:07)
1a471f43d - fix(dev): remove keys duplicated between ConfigMap and ExternalSecret (Md Asaduzzaman Miah, 2026-08-11 16:04)

✅ Checklist

  • Code follows the project's style guidelines
  • Self-review of code has been performed
  • Code is commented, particularly in hard-to-understand areas
  • Corresponding changes to documentation have been made
  • Tests have been added/updated for new functionality
  • All tests pass locally

🧪 Testing

📸 Screenshots (if applicable)

🔗 Related Issues

secretRef is listed after configMapRef in every deployment's envFrom, so a
key declared in both silently overrides the chart value and makes
values-*.yaml edits look like no-ops. Verified each duplicate against
Secret Manager before removing: all matched the chart value exactly, so
this is not a behavior change. Adds a CI check so a new overlap fails lint
instead of shipping silently.

Closes #2420

Signed-off-by: Md Asaduzzaman Miah <asad.miah@rhesis.ai>
RHESIS_ADMIN_PASSWORD, RHESIS_USER_PASSWORD, and ANALYTICS_USER_PASSWORD
are projected into the rhesis-app-secrets Secret but are not read by the
Helm chart, backend, frontend, SDK, or Terraform anywhere. They match the
Cloud SQL root/user passwords byte-for-byte, so they are leftovers from
before the move to in-cluster Postgres/CNPG, superseded by
ADMIN_DB_PASS/APP_DB_PASS/ANALYTICS_DB_PASS. The Secret Manager entries
are left in place; only the ExternalSecret wiring is removed.

Signed-off-by: Md Asaduzzaman Miah <asad.miah@rhesis.ai>
@asad-miah asad-miah self-assigned this Aug 11, 2026
@peqy

peqy Bot commented Aug 11, 2026

Copy link
Copy Markdown

Overall this looks like a solid fix: removing non-secret keys from ExternalSecrets (so values-*.yaml changes aren’t silently overridden) + adding a CI guardrail is a good approach.

One concern:

Improvement: scripts/check-k8s-secret-overlap.sh extracts ConfigMap keys via awk '/kind: ConfigMap/,/^---/', which may match a dependency chart’s ConfigMap (postgresql/valkey/otel) depending on render order, and could also run to EOF if there’s no trailing ---.

Fix: target the intended manifest explicitly (e.g. helm template ... --show-only templates/configmap.yaml or filter by metadata.name: {{ include "rhesis.fullname" . }}-config) before parsing data keys.

Found 1 issue (0 critical, 1 improvement).

@peqy peqy Bot 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.

Main suggestion: make check-k8s-secret-overlap.sh select the rhesis ConfigMap deterministically (avoid matching a dependency ConfigMap / missing ---). Everything else in this PR looks consistent with preventing ConfigMap/ExternalSecret env key overrides.

Comment thread scripts/check-k8s-secret-overlap.sh Outdated
secrets_file="kubernetes/clusters/$env/external-secrets/rhesis-app-secrets.yaml"
cm_keys=$(
helm template rhesis charts/rhesis -f "charts/rhesis/values-$env.yaml" \
| awk '/kind: ConfigMap/,/^---/' \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Improvement: awk '/kind: ConfigMap/,/^---/' will grab the first ConfigMap in the helm template output, which may come from a dependency chart (postgresql/valkey/otel) depending on render order, and could also run to EOF if there’s no trailing ---.

Fix: target the rhesis ConfigMap explicitly, e.g. helm template ... --show-only templates/configmap.yaml | grep ..., or select by metadata.name: rhesis-config before extracting data keys.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good fix — helm template ... --show-only templates/configmap.yaml cleanly targets the chart’s ConfigMap and avoids subchart render-order issues. This addresses my earlier concern.

@asad-miah asad-miah linked an issue Aug 11, 2026 that may be closed by this pull request
4 tasks
awk '/kind: ConfigMap/,/^---/' re-triggers for every ConfigMap in the
combined helm template output, including subchart ConfigMaps
(postgresql/valkey/otel) depending on render order, and could run past
EOF if a block has no trailing ---. Use --show-only templates/configmap.yaml
to isolate the chart's own ConfigMap instead.

Signed-off-by: Md Asaduzzaman Miah <asad.miah@rhesis.ai>
@peqy

peqy Bot commented Aug 11, 2026

Copy link
Copy Markdown

Looks good. Removing non-secret keys from ExternalSecrets + adding the Helm --show-only templates/configmap.yaml overlap check should prevent silent overrides going forward. Ship it.

@asad-miah
asad-miah requested a review from harry-rhesis August 18, 2026 09:27
…ween-config-secrets

Signed-off-by: Md Asaduzzaman Miah <asad.miah@rhesis.ai>

# Conflicts:
#	kubernetes/clusters/dev/external-secrets/rhesis-app-secrets.yaml
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.

Dedupe env var keys between Helm ConfigMap and ExternalSecret

1 participant