[mosip/mosip-infra#1890] Added domainConfig support in helm charts#2128
[mosip/mosip-infra#1890] Added domainConfig support in helm charts#2128bhumi46 wants to merge 1 commit into
Conversation
…osip#1988) * [mosip/mosip-infra#1890] Removed esignet-global, added domainConfig support in helm charts and deploy scripts Signed-off-by: bhumi46 <thisisbn46@gmail.com> * [mosip/mosip-infra#1890] Set chart versions to 0.0.1-develop Signed-off-by: bhumi46 <thisisbn46@gmail.com> * migrate to domainConfig helm values mosip#1890 Signed-off-by: bhumi46 <thisisbn46@gmail.com> --------- Signed-off-by: bhumi46 <thisisbn46@gmail.com> Co-authored-by: bhumi46 <bhumi11111a@gmail.com> (cherry picked from commit 429c6cb) Signed-off-by: bhumi46 <thisisbn46@gmail.com>
WalkthroughThis PR adds a new ChangesHelm domainConfig env injection
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@helm/esignet/templates/deployment.yaml`:
- Around line 125-128: Document that domainConfig entries are expected to be
scalar/string values because the deployment template’s range over domainConfig
uses quote for each $val and will stringify non-scalar maps/lists incorrectly;
update the chart’s values documentation around domainConfig to state the
expected type clearly and, if needed, reference the deployment template’s
domainConfig loop so consumers know to avoid nested objects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8729538c-01a6-432a-80f9-8225ba613b5a
📒 Files selected for processing (2)
helm/esignet/templates/deployment.yamlhelm/esignet/values.yaml
| {{- range $key, $val := .Values.domainConfig }} | ||
| - name: {{ $key }} | ||
| value: {{ $val | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Document expected value type for domainConfig.
{{ $val | quote }} assumes scalar values. If a consumer sets a domainConfig entry to a map/list, quote will stringify Go's default map/list representation into the env value, which is unlikely to be the intended output. Consider documenting (in values.yaml) that entries must be string/scalar values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@helm/esignet/templates/deployment.yaml` around lines 125 - 128, Document that
domainConfig entries are expected to be scalar/string values because the
deployment template’s range over domainConfig uses quote for each $val and will
stringify non-scalar maps/lists incorrectly; update the chart’s values
documentation around domainConfig to state the expected type clearly and, if
needed, reference the deployment template’s domainConfig loop so consumers know
to avoid nested objects.
Summary
Cherry-pick of #1988 from
developontorelease-1.8.x: addsdomainConfigsupport in the esignet helm charts (deployment.yaml and values.yaml), replacingesignet-globalwith a generic domainConfig values mechanism.Original commit: 429c6cb
Test plan
Summary by CodeRabbit