feat(common): abstract service-discovery topology in serviceDiscovery.envFlat - #1746
Closed
guimoreirar wants to merge 1 commit into
Closed
feat(common): abstract service-discovery topology in serviceDiscovery.envFlat#1746guimoreirar wants to merge 1 commit into
guimoreirar wants to merge 1 commit into
Conversation
….envFlat
Evolve the helper to derive the SD external/internal endpoints from
chart topology and bake the platform-standard defaults, so every
productized chart gets a consistent lib-service-discovery contract:
- new optional inputs serviceName/namespace/servicePort/ingressHost:
SD_INTERNAL_ADDRESS = <serviceName>.<namespace>.svc.cluster.local
SD_INTERNAL_PORT = servicePort
SD_EXTERNAL_ADDRESS = ingressHost
- platform defaults: SD_PREFER_VIEW=internal, SD_INTERNAL_SCHEME=http,
SD_EXTERNAL_PORT=443, SD_TLS_SKIP_VERIFY=true, SD_TLS=false.
- precedence unchanged: configmap.SD_* (legacy) > caller/grouped param >
derived/opinionated default > "".
Backward-safe: callers without topology inputs still render (addresses
default ""). No merged consumer yet (helper landed in #1742).
📝 WalkthroughWalkthroughChangesService discovery environment generation
Sequence Diagram(s)sequenceDiagram
participant TopologyInputs
participant serviceDiscoveryEnvFlat
participant envFlatBlock
TopologyInputs->>serviceDiscoveryEnvFlat: Provide optional topology inputs
serviceDiscoveryEnvFlat->>serviceDiscoveryEnvFlat: Derive SD_* values and apply defaults
serviceDiscoveryEnvFlat->>envFlatBlock: Pass resolved values and ordered keys
envFlatBlock-->>serviceDiscoveryEnvFlat: Render KEY: value entries
Possibly related PRs
✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@charts/lerian-common/README.md`:
- Around line 22-30: The generic flat-passthrough description should exclude
serviceDiscovery.envFlat because that helper derives topology values and applies
platform defaults. Update the description to apply only to the truly flat
helpers, while preserving the documented precedence and derivation behavior for
serviceDiscovery.envFlat.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8c36a9c4-2216-4548-ba0c-3c65565718d1
📒 Files selected for processing (2)
charts/lerian-common/README.mdcharts/lerian-common/templates/_service_discovery.tpl
Comment on lines
+22
to
+30
| `serviceDiscovery.envFlat` additionally accepts OPTIONAL topology inputs | ||
| (`serviceName`, `namespace`, `servicePort`, `ingressHost`) that DERIVE | ||
| `SD_INTERNAL_ADDRESS` (`<serviceName>.<namespace>.svc.cluster.local`, only when both | ||
| given), `SD_INTERNAL_PORT` (`servicePort | toString`) and `SD_EXTERNAL_ADDRESS` | ||
| (`ingressHost`); each stays `""` when its inputs are absent. It also bakes the | ||
| opinionated platform defaults `SD_PREFER_VIEW="internal"`, `SD_TLS_SKIP_VERIFY="true"`, | ||
| `SD_EXTERNAL_PORT="443"`, `SD_INTERNAL_SCHEME="http"` unconditionally. All of these are | ||
| lowest precedence — per key: `configmap.SD_<KEY>` (legacy) > `defaults.SD_<KEY>` | ||
| (grouped param) > derived/opinionated default > `""`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Narrow the no-derivation claim to exclude serviceDiscovery.envFlat.
The preceding flat-passthrough description still says that serviceDiscovery.envFlat reproduces native blocks byte-for-byte with no derivation, while this section documents topology-derived values and baked defaults. Update the generic description to apply only to the truly flat helpers.
🤖 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 `@charts/lerian-common/README.md` around lines 22 - 30, The generic
flat-passthrough description should exclude serviceDiscovery.envFlat because
that helper derives topology values and applies platform defaults. Update the
description to apply only to the truly flat helpers, while preserving the
documented precedence and derivation behavior for serviceDiscovery.envFlat.
Member
Author
|
Redundante — o |
This was referenced Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Evolves
lerian-common.serviceDiscovery.envFlatto abstract the lib-service-discovery topology, so every productized chart gets a consistent SD contract instead of hand-wiring 17 rawSD_*keys.What changed
New OPTIONAL topology inputs → derived defaults:
serviceName+namespaceSD_INTERNAL_ADDRESS = <serviceName>.<namespace>.svc.cluster.localservicePortSD_INTERNAL_PORTingressHostSD_EXTERNAL_ADDRESSPlatform-standard baked defaults:
SD_PREFER_VIEW=internal,SD_INTERNAL_SCHEME=http,SD_EXTERNAL_PORT=443,SD_TLS_SKIP_VERIFY=true,SD_TLS=false(ConsulSD_ADDRESSstayslocalhost:8500,SD_ENABLED=false).Precedence unchanged:
configmap.SD_<KEY>(legacy, WINS) > caller/grouped param (defaults) > derived/opinionated default >"".Verified (scratch render)
SD_INTERNAL_ADDRESS: "ledger.midaz-ns.svc.cluster.local",SD_INTERNAL_PORT: "3000",SD_EXTERNAL_ADDRESS: "ledger.stg.lerian.net",SD_EXTERNAL_PORT: "443",SD_PREFER_VIEW: "internal",SD_TLS_SKIP_VERIFY: "true".configmap.SD_INTERNAL_ADDRESSoverride → wins over derived.defaults→ wins over derived, loses to configmap.""(graceful), scalar defaults applied.0 violations.Backward-safe: the helper landed in #1742 and has no merged consumer yet, so evolving its defaults is safe. Consumers (midaz, then fees/access-manager/...) pass
serviceName/namespace/servicePort/ingressHostto opt in.Scope: only
_service_discovery.tpl+README.md.