fix(ledger): resolve configured gRPC port in V3GRPCBackendRef - #506
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Dav-14
force-pushed
the
fix/ledger-grpc-port
branch
from
August 3, 2026 08:46
be58cb7 to
07bd41d
Compare
Contributor
✅ Approve — automated reviewThe fix correctly resolves the configured gRPC port via LedgerConfiguration in V3GRPCBackendRef, with proper context and error propagation. The added tests cover default, scoped, wildcard, and precedence cases. No blocking or major issues were identified by any reviewer, and there are no new findings beyond what was already addressed in prior discussion. No findings. |
Dav-14
force-pushed
the
fix/ledger-grpc-port
branch
2 times, most recently
from
August 3, 2026 15:24
0686df3 to
e5e169c
Compare
Dav-14
added a commit
that referenced
this pull request
Aug 3, 2026
…tworkPolicy The allow-ledger-v3-from-connectivity policy hardcoded port 8888, so a stack overriding spec.cluster.service.grpcPort had its connectivity gRPC traffic silently denied by the default-ingress policy — even though #506 makes the client dial the configured port. Resolve the effective port from ledgers.V3GRPCBackendRef (the same source of truth the client uses, honoring the LedgerConfiguration override and falling back to the default) instead of the constant. Add a test covering the override alongside the default (raised in review of #507).
Dav-14
force-pushed
the
fix/ledger-grpc-port
branch
from
August 3, 2026 15:46
e5e169c to
2c3b378
Compare
V3GRPCBackendRef, the single source of truth consumed by the connectivity module to build its ledgerAddress, always passed port 0 to ledgerV3GRPCBackendRef and therefore assumed the default gRPC port. The gateway backend (v3.go / v3_preview.go) instead resolves the port from the stack LedgerConfiguration (spec.cluster.service.grpcPort), so a stack overriding the ledger Cluster gRPC service port got a Connectivity pointed at the wrong port while the gateway stayed correct. Resolve the configured port from the LedgerConfiguration inside V3GRPCBackendRef (via ledgerV3BaseSpec, the same base the gateway derives its clusterSpec from) so both consumers honour the override and fall back to the default port when unset. Thread the reconciler Context through the export and its connectivity caller. Add a table-driven unit test covering default, stack-scoped, wildcard, and precedence cases.
Dav-14
added a commit
that referenced
this pull request
Aug 3, 2026
…tworkPolicy The allow-ledger-v3-from-connectivity policy hardcoded port 8888, so a stack overriding spec.cluster.service.grpcPort had its connectivity gRPC traffic silently denied by the default-ingress policy — even though #506 makes the client dial the configured port. Resolve the effective port from ledgers.V3GRPCBackendRef (the same source of truth the client uses, honoring the LedgerConfiguration override and falling back to the default) instead of the constant. Add a test covering the override alongside the default (raised in review of #507).
Dav-14
force-pushed
the
fix/ledger-grpc-port
branch
from
August 3, 2026 16:03
2c3b378 to
226efd6
Compare
Dav-14
added a commit
that referenced
this pull request
Aug 3, 2026
…tworkPolicy The allow-ledger-v3-from-connectivity policy hardcoded port 8888, so a stack overriding spec.cluster.service.grpcPort had its connectivity gRPC traffic silently denied by the default-ingress policy — even though #506 makes the client dial the configured port. Resolve the effective port from ledgers.V3GRPCBackendRef (the same source of truth the client uses, honoring the LedgerConfiguration override and falling back to the default) instead of the constant. Add a test covering the override alongside the default (raised in review of #507).
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
3 participants
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.
V3GRPCBackendRef, the single source of truth consumed by the
connectivity module to build its ledgerAddress, always passed port 0 to
ledgerV3GRPCBackendRef and therefore assumed the default gRPC port. The
gateway backend (v3.go / v3_preview.go) instead resolves the port from
the stack LedgerConfiguration (spec.cluster.service.grpcPort), so a stack
overriding the ledger Cluster gRPC service port got a Connectivity
pointed at the wrong port while the gateway stayed correct.
Resolve the configured port from the LedgerConfiguration inside
V3GRPCBackendRef (via ledgerV3BaseSpec, the same base the gateway derives
its clusterSpec from) so both consumers honour the override and fall back
to the default port when unset. Thread the reconciler Context through the
export and its connectivity caller.
Add a table-driven unit test covering default, stack-scoped, wildcard,
and precedence cases.
Stack created with GitHub Stacks CLI • Give Feedback 💬