feat(connectivity): add Connectivity module bound to the stack ledger - #492
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds a v1beta1 Connectivity resource, delegated reconciliation through Ledger v3, Gateway routing, credential handling, network policies, retry behavior, tests, and module development documentation. ChangesConnectivity module
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Connectivity
participant Operator
participant Ledger
participant DelegatedConnectivity
participant NetworkPolicy
Connectivity->>Operator: Submit Connectivity configuration
Operator->>Ledger: Check version, readiness, credentials, and gRPC backend
Ledger-->>Operator: Return readiness and backend data
Operator->>DelegatedConnectivity: Create or update delegated Connectivity
DelegatedConnectivity-->>Operator: Report readiness
Operator->>NetworkPolicy: Allow Connectivity access to Ledger v3
Operator-->>Connectivity: Update status conditions
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Contributor
✅ Approve — automated reviewNo actionable correctness issues were found in the current diff. Previously raised active concerns appear addressed at HEAD. No findings. |
NumaryBot
reviewed
Jul 16, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 2 new inline findings.
Summary: #492 (comment)
NumaryBot
reviewed
Jul 16, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot review complete: no remaining inline findings.
Resolved 2 stale NumaryBot review threads (1 fixed, 1 outdated).
Summary: #492 (comment)
Dav-14
force-pushed
the
feat/connectivity-module
branch
from
July 16, 2026 14:40
f5bf061 to
1f6c39e
Compare
NumaryBot
reviewed
Jul 16, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #492 (comment)
NumaryBot
reviewed
Jul 16, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #492 (comment)
NumaryBot
reviewed
Jul 16, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot review complete: no remaining inline findings.
Resolved 1 stale NumaryBot review thread (1 fixed, 0 outdated).
Summary: #492 (comment)
NumaryBot
reviewed
Jul 22, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #492 (comment)
NumaryBot
reviewed
Jul 27, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 2 new inline findings.
Summary: #492 (comment)
NumaryBot
reviewed
Jul 27, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 3 new inline findings.
Summary: #492 (comment)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (5)
internal/tests/ledger_v3_controller_test.go (1)
1084-1093: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRegister an explicit cleanup for the re-created default configuration.
The
DeferCleanupat Line 946 closes over theconfigurationvariable, so reassigning it here is what keeps the second, cluster-scopedDefaultLedgerConfigurationNameobject from leaking. That's implicit and easy to break (e.g. by shadowing with:=), and a leaked defaultLedgerConfigurationwould affect other specs in thisSerialsuite.♻️ Proposed explicit cleanup
Expect(Create(configuration)).To(Succeed()) + DeferCleanup(func() { + Expect(client.IgnoreNotFound(Delete(configuration))).To(Succeed()) + })🤖 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 `@internal/tests/ledger_v3_controller_test.go` around lines 1084 - 1093, After reassigning the cluster-scoped configuration in this test, register an explicit DeferCleanup for that recreated DefaultLedgerConfigurationName object rather than relying on the earlier cleanup closure over configuration. Ensure the cleanup deletes this exact LedgerConfiguration and remains effective even if the variable is later shadowed.internal/tests/networkpolicy_controller_test.go (1)
93-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the actual Raft/gRPC port values, not just the count.
allow-ledger-v2-from-v3below checks the concrete port (8080), but here onlyHaveLen(2)is checked, so a regression swapping 7777/8888 for other ports would pass.♻️ Proposed assertion
- g.Expect(np.Spec.Ingress[0].Ports).To(HaveLen(2)) + g.Expect(np.Spec.Ingress[0].Ports).To(HaveLen(2)) + g.Expect(np.Spec.Ingress[0].Ports[0].Port.IntValue()).To(Equal(7777)) + g.Expect(np.Spec.Ingress[0].Ports[1].Port.IntValue()).To(Equal(8888))🤖 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 `@internal/tests/networkpolicy_controller_test.go` around lines 93 - 98, Update the ingress assertions in the allow-ledger-v2-from-v3 test to verify the concrete Raft and gRPC port values 7777 and 8888, not only that np.Spec.Ingress[0].Ports has length two. Preserve the existing count assertion and assert both expected port entries using the same port-value assertion pattern as the nearby 8080 check.deployment/operator/helpers.go (2)
108-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStore registry credentials directly instead of re-extracting via an unchecked type assertion.
dc.RegistryAuth[0].(dockerbuild.RegistryArgs)panics ifRegistryAuthis ever empty or holds a different concrete type. SincenewDockerConfigalready has the username/password at construction time, storing them directly ondockerConfigavoids this indexing/type-assertion round-trip.♻️ Proposed refactor
type dockerConfig struct { Registry string PullRegistry string BuilderName string ImageTag string Platforms []string RegistryAuth dockerbuild.RegistryArray + Username pulumi.StringPtrInput + Password pulumi.StringPtrInput } ... + username := config.GetSecret(ctx, "registry-username") + password := config.GetSecret(ctx, "registry-password") return &dockerConfig{ ... + Username: username, + Password: password, RegistryAuth: dockerbuild.RegistryArray{ dockerbuild.RegistryArgs{ Address: pulumi.String(registry), - Username: config.GetSecret(ctx, "registry-username"), - Password: config.GetSecret(ctx, "registry-password"), + Username: username, + Password: password, }, }, } ... Registry: dockerbuild.RegistryArgs{ Address: pulumi.String(dc.Registry), - Username: dc.RegistryAuth[0].(dockerbuild.RegistryArgs).Username, - Password: dc.RegistryAuth[0].(dockerbuild.RegistryArgs).Password, + Username: dc.Username, + Password: dc.Password, },Also applies to: 185-197
🤖 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 `@deployment/operator/helpers.go` around lines 108 - 121, Update dockerConfig and newDockerConfig to store the registry username and password directly from config.GetSecret, rather than wrapping them only in RegistryAuth. Replace any dc.RegistryAuth[0].(dockerbuild.RegistryArgs) indexing and type assertions in the affected logic with the direct credential fields, preserving the existing credential values.
94-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueArch-to-platform mapping is fragile.
strings.HasSuffix(p, arch)againstallPlatformshas no case normalization and offers no explicit way to select multiple platforms (e.g., both amd64+arm64) other than relying on a coincidental shared suffix. Consider accepting a comma-separated list and matching exact platform names.🤖 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 `@deployment/operator/helpers.go` around lines 94 - 106, Update the platform-selection logic around cfg.Get("arch") to normalize architecture values and parse comma-separated entries, allowing multiple architectures such as amd64 and arm64. Match each requested architecture against explicit platform names in allPlatforms rather than relying on strings.HasSuffix, while preserving the linux-<arch> fallback when no valid platforms are selected.internal/resources/stacks/networkpolicies.go (1)
222-230: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace
intstr.FromIntwithFromInt32.
intstr.FromIntis deprecated ink8s.io/apimachinery v0.34.2; this helper already converts 64-bitintvalues for a field backed by anint32, so passingint32values explicitly avoids the conversion/deprecation issue.♻️ Proposed refactor
-func networkPolicyTCPPorts(ports ...int) []networkingv1.NetworkPolicyPort { +func networkPolicyTCPPorts(ports ...int32) []networkingv1.NetworkPolicy Port { protocol := corev1.ProtocolTCP ret := make([]networkingv1.NetworkPolicyPort, 0, len(ports)) for _, port := range ports { - value := intstr.FromInt(port) + value := intstr.FromInt32(port) ret = append(ret, networkingv1.NetworkPolicyPort{Protocol: &protocol, Port: &value}) ret = append(ret, networkingv1.NetworkPolicyPort{Protocol: &protocol, Port: &value}) } return ret }🤖 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 `@internal/resources/stacks/networkpolicies.go` around lines 222 - 230, Update networkPolicyTCPPorts to convert each port to int32 and construct the IntOrString value with FromInt32 instead of the deprecated intstr.FromInt, preserving the existing NetworkPolicyPort construction and returned results.
🤖 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 `@deployment/operator/main.go`:
- Around line 96-98: Update the Helm image value in the operator deployment
configuration to use dc.ImageTag instead of the hardcoded "latest" prefix, while
preserving the existing digest suffix from operatorImage.Digest and repository
value.
- Around line 64-81: Update the licence configuration block to retrieve the
token with the Pulumi secret-aware getter config.GetSecret(ctx, "licence-token")
instead of cfg.Get("licence-token"), and pass that secret output through the
existing licenceValues token entry so the Helm release preserves the token as
masked.
In `@Dockerfile`:
- Around line 28-34: Add a non-root USER directive to the final Dockerfile stage
before the ENTRYPOINT, creating or selecting an unprivileged user that can
execute /usr/bin/operator. Ensure the operator runs under that user by default
while preserving the existing binary copy and entrypoint behavior.
In `@docs/04-Modules/03-Ledger.md`:
- Around line 7-8: Update the PostgreSQL and Broker Markdown links in the module
documentation to use descriptive link text naming each configuration guide
instead of “here,” while preserving the existing destinations and optional
Broker labeling.
In `@docs/09-Configuration` reference/02-Custom Resource Definitions.md:
- Around line 2477-2482: Remove the duplicate ready field from the
GatewayGRPCAPIStatus source schema or generator input, preserving the documented
ready description and the info field, then regenerate the Custom Resource
Definitions reference so the table contains ready only once.
- Around line 2650-2653: Update the `cluster` field description in the
configuration reference so `ClusterSpec` no longer links to the unresolved
`#clusterspec` anchor; link it to the authoritative ClusterSpec reference if
available, otherwise document the type locally or render it as plain text.
In `@internal/resources/connectivities/init_test.go`:
- Around line 199-259: Update the Connectivity controller setup to watch
resources with ledgerCredentialsGVK and map credential events to the affected
Connectivity resource using the credential’s stack selector/namespace data,
since the Credential is not owned by Connectivity. Add an event-driven test
alongside TestEnsureLedgerCredentialsCreatesGodCredentialAndReportsPending and
TestEnsureLedgerCredentialsReportsKeyAndSecretWhenReady that starts with a
pending credential, updates status.phase to Ready, and verifies the
corresponding Connectivity reconciliation is requeued.
In `@internal/resources/gateways/Caddyfile.gotpl`:
- Around line 61-63: Update the GRPCServices branch in the Caddyfile template to
preserve encrypted HTTP/2 by configuring the full protocol set, including h1,
h2, and h2c, instead of only h1 and h2c.
In `@internal/resources/gateways/deployment.go`:
- Around line 124-128: Update the Secret lookup in the gateway deployment
reconciliation loop over sortedSecretNames to detect a not-found error and
return NewPendingError() instead of wrapping it as a hard failure. Preserve the
existing wrapped error for other client.Get failures, and keep the successful
secret-processing path unchanged.
---
Nitpick comments:
In `@deployment/operator/helpers.go`:
- Around line 108-121: Update dockerConfig and newDockerConfig to store the
registry username and password directly from config.GetSecret, rather than
wrapping them only in RegistryAuth. Replace any
dc.RegistryAuth[0].(dockerbuild.RegistryArgs) indexing and type assertions in
the affected logic with the direct credential fields, preserving the existing
credential values.
- Around line 94-106: Update the platform-selection logic around cfg.Get("arch")
to normalize architecture values and parse comma-separated entries, allowing
multiple architectures such as amd64 and arm64. Match each requested
architecture against explicit platform names in allPlatforms rather than relying
on strings.HasSuffix, while preserving the linux-<arch> fallback when no valid
platforms are selected.
In `@internal/resources/stacks/networkpolicies.go`:
- Around line 222-230: Update networkPolicyTCPPorts to convert each port to
int32 and construct the IntOrString value with FromInt32 instead of the
deprecated intstr.FromInt, preserving the existing NetworkPolicyPort
construction and returned results.
In `@internal/tests/ledger_v3_controller_test.go`:
- Around line 1084-1093: After reassigning the cluster-scoped configuration in
this test, register an explicit DeferCleanup for that recreated
DefaultLedgerConfigurationName object rather than relying on the earlier cleanup
closure over configuration. Ensure the cleanup deletes this exact
LedgerConfiguration and remains effective even if the variable is later
shadowed.
In `@internal/tests/networkpolicy_controller_test.go`:
- Around line 93-98: Update the ingress assertions in the
allow-ledger-v2-from-v3 test to verify the concrete Raft and gRPC port values
7777 and 8888, not only that np.Spec.Ingress[0].Ports has length two. Preserve
the existing count assertion and assert both expected port entries using the
same port-value assertion pattern as the nearby 8080 check.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 28e6cc05-e158-4ed5-b091-d89ebd421e8e
⛔ Files ignored due to path filters (43)
config/crd/bases/formance.com_connectivities.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_gatewaygrpcapis.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_gatewayhttpapis.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_gateways.yamlis excluded by!**/*.yamlconfig/crd/bases/formance.com_ledgerconfigurations.yamlis excluded by!**/*.yamlconfig/crd/kustomization.yamlis excluded by!**/*.yamlconfig/rbac/ledgerconfiguration_editor_role.yamlis excluded by!**/*.yamlconfig/rbac/ledgerconfiguration_viewer_role.yamlis excluded by!**/*.yamlconfig/rbac/role.yamlis excluded by!**/*.yamlconfig/samples/formance.com_v1beta1_ledgerconfiguration.yamlis excluded by!**/*.yamlconfig/samples/kustomization.yamlis excluded by!**/*.yamldeployment/operator/Pulumi.yamlis excluded by!**/*.yamldeployment/operator/go.modis excluded by!**/*.moddeployment/operator/go.sumis excluded by!**/*.sum,!**/*.sumdocs/09-Configuration reference/settings.catalog.jsonis excluded by!**/*.jsongo.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_connectivities.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gatewaygrpcapis.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gatewayhttpapis.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_gateways.formance.com.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_ledgerconfigurations.formance.com.yamlis excluded by!**/*.yamlhelm/operator/templates/gen/rbac.authorization.k8s.io_v1_clusterrole_formance-manager-role.yamlis excluded by!**/gen/**,!**/*.yaml,!**/gen/**internal/tests/crds/cert-manager.io_certificates.yamlis excluded by!**/*.yamlinternal/tests/crds/cert-manager.io_issuers.yamlis excluded by!**/*.yamlinternal/tests/crds/ledger.formance.com_clusters.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-audit.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-ledger-and-another-service.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-ledger-and-grpc.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-ledger-only.yamlis excluded by!**/*.yamlinternal/tests/testdata/resources/gateway-controller/configmap-with-opentelemetry.yamlis excluded by!**/*.yamltests/e2e/chainsaw/02-stack-lifecycle/asserts/networkpolicies.yamlis excluded by!**/*.yamltests/e2e/chainsaw/14-ledger-module/chainsaw-test.yamlis excluded by!**/*.yamltests/e2e/chainsaw/14-ledger-module/resources/database.yamlis excluded by!**/*.yamltests/e2e/chainsaw/14-ledger-module/resources/stack.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/chainsaw-test.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/gateway.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/grpcapi-updated.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/grpcapi.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/httpapi-ledger.yamlis excluded by!**/*.yamltests/e2e/chainsaw/26-gatewaygrpcapi-sync/resources/stack.yamlis excluded by!**/*.yamltools/kubectl-stacks/go.modis excluded by!**/*.modtools/kubectl-stacks/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (55)
.gitignoreDockerfileEarthfilePROJECTapi/formance.com/v1beta1/connectivity_types.goapi/formance.com/v1beta1/gateway_types.goapi/formance.com/v1beta1/gatewaybackend_types.goapi/formance.com/v1beta1/gatewaygrpcapi_types.goapi/formance.com/v1beta1/gatewayhttpapi_types.goapi/formance.com/v1beta1/ledger_types.goapi/formance.com/v1beta1/ledgerconfiguration_types.goapi/formance.com/v1beta1/zz_generated.deepcopy.godeployment/operator/.gitignoredeployment/operator/helpers.godeployment/operator/main.godocs/04-Modules/03-Ledger.mddocs/09-Configuration reference/01-Settings.mddocs/09-Configuration reference/02-Custom Resource Definitions.mddocs/10-Development/01-Adding a module.mdinternal/core/setup.gointernal/resources/all.gointernal/resources/auths/env.gointernal/resources/connectivities/init.gointernal/resources/connectivities/init_test.gointernal/resources/gatewaygrpcapis/create.gointernal/resources/gatewaygrpcapis/init.gointernal/resources/gatewayhttpapis/create.gointernal/resources/gateways/Caddyfile.gotplinternal/resources/gateways/caddyfile.gointernal/resources/gateways/caddyfile_test.gointernal/resources/gateways/configuration.gointernal/resources/gateways/deployment.gointernal/resources/gateways/init.gointernal/resources/ledgers/exports.gointernal/resources/ledgers/init.gointernal/resources/ledgers/v3.gointernal/resources/ledgers/v3_preview.gointernal/resources/ledgers/v3_spec.gointernal/resources/ledgers/v3_spec_test.gointernal/resources/ledgers/v3_test.gointernal/resources/ledgers/v3_tls.gointernal/resources/settings/opentelemetry.gointernal/resources/stacks/networkpolicies.gointernal/tests/application_test.gointernal/tests/auth_scopes_settings_test.gointernal/tests/gateway_controller_test.gointernal/tests/gatewaygrpcapi_controller_test.gointernal/tests/jobs_controller_test.gointernal/tests/ledger_controller_test.gointernal/tests/ledger_v3_controller_test.gointernal/tests/networkpolicy_controller_test.gointernal/tests/orchestration_controller_test.gointernal/tests/registries_test.gointernal/tests/transactionplane_controller_test.gointernal/tests/wallets_controller_test.go
Introduce a new `formance.com/v1beta1 Connectivity` stack module, mirroring the Ledger v3 delegation pattern. The module does not run the workload itself. It: - detects, at controller start-up, whether the connectivity operator (`connectivity.formance.com` Connectivity CRD) is installed and reachable with the required RBAC — the same capability + API-group probe the Ledger v3 module uses (CRD served-version check + SelfSubjectAccessReview per verb). When absent, the module reports the capability as unavailable and stays pending instead of failing the controller. - gates on the stack's ledger being v3 and ready (connectivity ingests into the Ledger v3 gRPC endpoint). - provisions a `connectivity.formance.com/v1alpha1 Connectivity` resource bound to that ledger: `ledgerAddress` = the ledger v3 gRPC service and `ledgerTLS` = the ledger backend TLS secret. The connection details are taken from `ledgers.V3GRPCBackendRef`, the single source of truth already used to reach the ledger over gRPC, so connectivity and the gateway stay in sync. - reflects the delegated resource's readiness back onto the module status. Includes the module type, reconciler + capability detection, unit tests for the capability-gating paths, and the generated CRD/RBAC/deepcopy + helm CRD.
Address review feedback and the Dirty check: - Resolve the ledger version with core.ResolveModuleVersion so the v3 gate also works for stacks using spec.versionsFromFile (previously the version fell back to empty and Connectivity stayed stuck on LedgerNotV3). - Register bases/formance.com_connectivities.yaml in config/crd/kustomization so non-Helm (kustomize) installs create the Connectivity CRD. - Regenerate CRD reference docs + helm CRD (just pre-commit).
Document the current end-to-end process for adding a stack module: the module CR type (incl. the mandatory formance.com/kind=module label), the reconciler + init registration, all.go + config/crd/kustomization registration, capability detection for delegating modules, version resolution, codegen, and the deployment gotchas (reconcileStrategy: Revision, the startup-only capability probe, and versionsFromFile requirements).
The delegated connectivity.formance.com Connectivity was created without spec.image, so the connectivity operator fell back to its built-in ghcr.io/formancehq/connectivity-core:latest default — bypassing the stack's registry rewrite (e.g. ghcr.io -> registry.v2.formance.dev) and pull secrets, which makes it unpullable on rewritten registries. Resolve the connectivity-core image via registries.GetFormanceImage (using the Connectivity module version) so it honours the stack registry settings, and set spec.image + spec.imagePullSecrets on the delegated resource.
- Always enable the connectivity-api companion on the delegated Connectivity (spec.api.enabled=true), resolving the connectivity-api image through the registry translation so it honours the stack's registry rewrite + pull secrets (not the connectivity operator's ghcr.io/...:latest default). - Register a GatewayHTTPAPI for the module routing /api/connectivity to the connectivity-api Service (<stack>-api:8080) the connectivity operator provisions, and own it so changes reconcile.
…e auth The connectivity module deployed the delegated Connectivity CR but never wired connectivity-core's authentication to the stack's Ledger v3 gRPC endpoint, so connectivity-core sent no token and the ledger rejected every call with 'requires scope ledger:LedgerWrite'. Provision a god-mode ledger.formance.com/Credentials (cluster-scoped, owned by the Stack) selecting the stack's ledger Cluster: the ledger operator generates the Ed25519 keypair, registers the public key on the ledger, and distributes the private seed as a Secret in the stack namespace. Once Ready, wire the Connectivity CR's spec.auth (keyId + secretKeyRef->seed.hex) so the connectivity operator passes --auth-key-id/--auth-key-file and connectivity-core signs its gRPC tokens with the registered key. Regenerates RBAC for the new resource.
The connectivity repo's CI publishes formancehq/connectivity; the former connectivity-core repository no longer exists, so the module kept writing an unpullable image on the delegated resource.
The kubebuilder markers for ledger.formance.com credentials (added with the connectivity module) were present in config/rbac but the helm chart's generated ClusterRole was never refreshed, so the deployed operator was forbidden from listing Credentials -- its informer never synced and every Connectivity reconcile hung before reaching the delegated resource.
Main's ledgerV3GRPCBackendRef now takes the ledger Cluster's configured gRPC port; keep the connectivity-facing export on the default port.
Dav-14
force-pushed
the
feat/connectivity-module
branch
from
July 27, 2026 14:24
3795ef8 to
a6746a0
Compare
NumaryBot
reviewed
Jul 27, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #492 (comment)
…ed resource (#500) * feat(connectivity): provision OpenTelemetry monitoring on the delegated resource Resolve the stack's OpenTelemetry configuration with settings.GetOpenTelemetryConfiguration (collector-aware: points at otel-collector.<stack>:4318 when the per-stack collector exists, else honours the opentelemetry.* Settings) and embed it inline in the delegated Connectivity's spec.monitoring, mirroring the Ledger v3 Cluster pattern. The connectivity operator turns spec.monitoring into OTEL_* env vars on the workload; there is no separate Monitoring object to reference. The whole spec.monitoring block is rebuilt on every reconcile and pruned when telemetry is disabled, so the reconcile stays idempotent. Not-tested: end-to-end env-var emission by the connectivity operator (covered by the connectivity repo); unit tests assert the inline spec mapping and idempotency. * fix(connectivity): drop unresolvable pod-name attribute from delegated monitoring GetOpenTelemetryConfiguration injects pod-name=$(POD_NAME), which only resolves when a downward-API POD_NAME env var is defined ahead of OTEL_RESOURCE_ATTRIBUTES. The connectivity operator emits OTEL_RESOURCE_ATTRIBUTES verbatim from spec.monitoring.attributes and defines no such env var, so the placeholder surfaced literally in the delegated workload's telemetry. Strip attributes whose value carries an unresolvable $(...) placeholder before forwarding, keeping literal resource attributes (stack, custom). Omit the attributes field entirely when nothing resolvable remains.
NumaryBot
reviewed
Jul 31, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 4 new inline findings.
Summary: #492 (comment)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
docs/10-Development/01-Adding a module.md (1)
43-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDocument watches for every readiness dependency.
Line 48 lists only
Ledger. The Connectivity reconciler also depends on the ledgerCredentialsresource. State that delegated modules must watch every readiness-gating resource. Otherwise, the module can remain pending afterCredentialsbecomes ready because no event triggers another reconcile.Proposed documentation update
WithWatchDependency[*v1beta1.<Module>](&v1beta1.Ledger{}), // re-reconcile on dependency change ), ) } + +For delegated modules, add a watch for every readiness-gating resource, +including the ledger `Credentials` resource when credential readiness controls +provisioning.🤖 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 `@docs/10-Development/01-Adding` a module.md around lines 43 - 49, Update the module setup documentation around WithWatchDependency to state that delegated modules must watch every readiness-gating resource, including both the Ledger and its Credentials resource. Clarify that each dependency required by the reconciler’s readiness checks needs a watch so readiness changes trigger reconciliation.internal/resources/connectivities/init_test.go (1)
407-428: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd a Reconcile happy-path test.
The only
Reconciletest covers the capability-unavailable pending path. No test exercises the full success path: a ready Ledger v3, ready credentials, successful image resolution, and the resulting spec fields (ledgerAddress,ledgerTLS,auth.keyId,auth.secretKeyRef) on the created delegated Connectivity resource. Given this function is the central reconciliation path for the module, add a test with a fake client seeded with a readyLedger, a ready ledger Credentials object, and the required scheme registrations, then assert on the resulting unstructured object's spec.🤖 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 `@internal/resources/connectivities/init_test.go` around lines 407 - 428, Add a happy-path test alongside TestConnectivityReconcilePendingWhenCapabilityUnavailable that seeds a fake client with a ready Ledger v3 and ready ledger Credentials, registers the required schemes, and configures successful image resolution before calling Reconcile. Assert that the created delegated Connectivity resource contains the expected spec.ledgerAddress, spec.ledgerTLS, spec.auth.keyId, and spec.auth.secretKeyRef values.
🤖 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 `@internal/resources/connectivities/init.go`:
- Around line 500-509: In internal/resources/connectivities/init.go at lines
500-509, register a watch handler that maps status updates of the ledger
Credentials object (ledgerCredentialsGVK, named connectivity-<stack>) back to
the corresponding Connectivity reconcile request. This watch should be added to
the Init call, either directly via a Watches handler or integrated into the
withConnectivityClusterWatch options pattern, so that when the Credentials
object status changes, the Connectivity resource is requeued. In
internal/resources/connectivities/init_test.go at lines 201-261, add a test that
verifies this requeue path by starting with a pending Credentials object,
updating its status.phase to Ready, and asserting that the corresponding
Connectivity reconcile is requeued as a result of the watch firing.
- Around line 163-167: Update the Connectivity initialization around
V3GRPCBackendRef so it uses the stack’s configured Ledger v3 gRPC port instead
of the default-port helper behavior. Pass or propagate the relevant configured
port into the backend reference construction, ensuring ledgerAddress is built
from the same non-default port used by Gateway.
- Around line 345-395: Add a finalizer-based cleanup mechanism to the
Connectivity resource that ensures the associated Credentials object is deleted
during Connectivity deletion. In the reconciliation logic, add a finalizer
constant for Connectivity deletion and check if the Connectivity resource is
being deleted; when deletion is detected, delete the cluster-scoped Credentials
object named connectivity-<stack.Name> (using the same naming pattern as in
ensureLedgerCredentials) before removing the finalizer from the Connectivity
resource. This ties the privileged Credentials and its distributed Secret to the
Connectivity module lifecycle so they are revoked when the module is removed,
regardless of the Stack's lifecycle.
---
Nitpick comments:
In `@docs/10-Development/01-Adding` a module.md:
- Around line 43-49: Update the module setup documentation around
WithWatchDependency to state that delegated modules must watch every
readiness-gating resource, including both the Ledger and its Credentials
resource. Clarify that each dependency required by the reconciler’s readiness
checks needs a watch so readiness changes trigger reconciliation.
In `@internal/resources/connectivities/init_test.go`:
- Around line 407-428: Add a happy-path test alongside
TestConnectivityReconcilePendingWhenCapabilityUnavailable that seeds a fake
client with a ready Ledger v3 and ready ledger Credentials, registers the
required schemes, and configures successful image resolution before calling
Reconcile. Assert that the created delegated Connectivity resource contains the
expected spec.ledgerAddress, spec.ledgerTLS, spec.auth.keyId, and
spec.auth.secretKeyRef values.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 62aaed12-7a50-4b92-bcf3-128097420cf8
⛔ Files ignored due to path filters (5)
config/crd/bases/formance.com_connectivities.yamlis excluded by!**/*.yamlconfig/crd/kustomization.yamlis excluded by!**/*.yamlconfig/rbac/role.yamlis excluded by!**/*.yamlhelm/crds/templates/crds/apiextensions.k8s.io_v1_customresourcedefinition_connectivities.formance.com.yamlis excluded by!**/*.yamlhelm/operator/templates/gen/rbac.authorization.k8s.io_v1_clusterrole_formance-manager-role.yamlis excluded by!**/gen/**,!**/*.yaml,!**/gen/**
📒 Files selected for processing (8)
api/formance.com/v1beta1/connectivity_types.goapi/formance.com/v1beta1/zz_generated.deepcopy.godocs/09-Configuration reference/02-Custom Resource Definitions.mddocs/10-Development/01-Adding a module.mdinternal/resources/all.gointernal/resources/connectivities/init.gointernal/resources/connectivities/init_test.gointernal/resources/ledgers/exports.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/resources/all.go
- internal/resources/ledgers/exports.go
- api/formance.com/v1beta1/connectivity_types.go
flemzord
reviewed
Aug 4, 2026
flemzord
left a comment
flemzord
left a comment
Member
There was a problem hiding this comment.
Reviewed as the root of the linked stack. The current head still contains the credential watch/cleanup, closed-gate teardown, configurable gRPC port, gateway TLS pending-state, and NetworkPolicy issues tracked in the existing threads; those fixes live in descendant PRs #501–#507 rather than in this head. I am not duplicating the inline findings or approving this root in isolation. It is safe to reconsider once the stack is merged/restacked so those fixes are part of the effective change.
When a backend module has not yet provisioned its TLS Secret, the Gateway deployment reconciler fetched the Secret and returned any error as a hard failure. A NotFound during this race surfaced the Gateway as errored rather than pending. Return core.NewPendingError() on apierrors.IsNotFound so the framework treats it as pending (matching how other 'not ready yet' conditions are handled) and retries. All other Get errors remain hard errors.
…etriggered (#504) The Connectivity reconciler returns a PendingError while the cluster-scoped ledger Credentials it provisions (connectivity-<stack>) is not yet Ready (LedgerCredentialsPending). The reconcile loop treats a PendingError as a terminal ctrl.Result{} with no RequeueAfter, and the Credentials is owned by the Stack rather than the namespaced Connectivity, so nothing re-triggered the module when the ledger operator flipped the Credentials status.phase to Ready: the reconcile could stall indefinitely. Register a raw builder watch on the ledger.formance.com/v1alpha1 Credentials GVK (unstructured, mirroring withConnectivityClusterWatch) that maps a Credentials event back to the Connectivity in the matching stack, derived from the connectivity-<stack> name and listed via the stack field index. The watch is gated on the Credentials CRD being installed so controller setup never fails when the ledger operator is absent. RBAC already grants watch on credentials. Add unit tests for the mapping (enqueues the matching Connectivity, returns nothing for a stack without one, ignores foreign Credentials) and for the capability gate (disabled when the CRD is absent or discovery fails).
… closes (#505) * fix(connectivity): tear down delegated resources when the ledger gate closes When a stack had already provisioned the delegated Connectivity and its GatewayHTTPAPI, closing a hard Ledger gate on a later reconcile only set a pending condition and returned; the delegated workload kept running and stayed exposed through the gateway even though its prerequisite no longer held. Introduce teardownDelegated(ctx, stack, connectivity), which idempotently deletes both the delegated Connectivity and the GatewayHTTPAPI (client.IgnoreNotFound). Call it from the hard/persistent gates only: - LedgerNotFound (module removed) -> teardown - LedgerNotV3 (real downgrade) -> teardown Leave the transient gates untouched so a momentary blip does not flap the workload: - LedgerVersionUnresolved (resolution error, not a downgrade) -> keep - LedgerNotReady (v3 but momentarily not ready) -> keep Add unit tests covering both the teardown-on-not-v3 and keep-on-v3-not-ready paths. * fix(connectivity): tear down credentials and route independently on hard gate close teardownDelegated returned on the first delete error, so a failed delegated-Connectivity deletion left the public GatewayHTTPAPI route exposed; it also never deleted the cluster-scoped god-mode Credentials, whose distributed private-key Secret stack-namespace GC never reclaims (and whose public key stays registered on the v3 Cluster). Attempt all three deletions independently via errors.Join and delete the Credentials (which cascades the ledger operator's key deregistration and Secret cleanup). Tests: assert Credentials cleanup on hard teardown, its retention on a transient gate, and that every deletion is attempted when one fails (raised in review of #505). * fix(connectivity): tear down on a closed ledger gate even without the operator The !connectivityAvailable guard returned before the LedgerNotFound/ LedgerNotV3 teardown, so if the connectivity operator became unavailable after resources were provisioned and the ledger was then removed or downgraded, the gateway route and god-mode Credentials stayed behind. Evaluate the ledger hard gate (ledgerGateClosed) in the capability- unavailable branch and tear down when it is closed, guarded so a transient operator outage with a healthy v3 ledger does not flap the resources. teardownDelegated now tolerates the connectivity CRD being absent (ignoreAbsent) so the delete is a no-op when the API is gone. Tests cover teardown-on-closed-gate and retention-on-open-gate (raised in review of #505).
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.
…a NetworkPolicy (#507) * fix(connectivity): allow connectivity pods to reach Ledger v3 gRPC via NetworkPolicy When networkpolicies.enabled, the default-deny-ingress policy drops all ingress to Ledger v3 pods except the explicitly-allowed gateway and intra-cluster ledger peers. The delegated connectivity workload dials the Ledger v3 gRPC endpoint (ledgers.V3GRPCBackendRef, port 8888) directly, but connectivity pods are neither gateway nor ledger pods, so their gRPC connections were silently dropped on network-policy stacks. Add a dedicated allow-ledger-v3-from-connectivity NetworkPolicy granting the connectivity workload ingress to the Ledger v3 pods on the gRPC port (8888), following the existing dedicated-policy pattern (allow-from-gateway). The connectivity pod labels are owned by the connectivity operator (separate repo) and cannot be confirmed here; connectivitySelector matches the operator-wide convention app.kubernetes.io/name=connectivity (documented as an assumption in code). Add unit tests rendering the policy and extend the network-policy controller test. * fix(stacks): leave connectivity->ledger-v3 gRPC port unrestricted 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. Rather than resolving the port (which would go stale unless the Stack controller also watched LedgerConfiguration), leave the port unrestricted for this tightly scoped same-namespace connectivity->ledger-v3 pair, mirroring allow-ledger-v3-cluster which is already port-agnostic for exactly this reason. The rule can no longer be broken or left stale by a grpcPort override, and needs no config watch.
) The delegated connectivity.formance.com/Connectivity is namespaced (one per stack namespace), so it no longer needs a stack-scoped name. Give it the fixed name "connectivity"; the connectivity operator derives the API Service from it, so it becomes "connectivity-api" instead of "<stack>-api". Credentials (connectivity-<stack>) and the GatewayHTTPAPI (<stack>-connectivity) stay stack-scoped — they are cluster-scoped and need the stack for uniqueness. No migration: the module is not yet released.
NumaryBot
reviewed
Aug 6, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 7 new inline findings.
Summary: #492 (comment)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@internal/resources/connectivities/init.go`:
- Around line 316-321: Update the doc comment for teardownDelegated to state
that the delegated Connectivity uses connectivityDelegatedName ("connectivity")
as its name and the stack name as its namespace; retain the existing
GatewayHTTPAPI naming and scoping description.
- Around line 85-91: Use ledgerCredentialsWatchAvailable in Reconcile when
handling LedgerCredentialsPending: if the watch is unavailable, return a delayed
requeue result so pending Connectivity resources retry without CRD events;
preserve the existing NewPendingError behavior and watch-driven path when the
flag is true.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: c0075e17-fb3c-46d9-b7b5-9e9bf747f064
📒 Files selected for processing (9)
internal/resources/connectivities/init.gointernal/resources/connectivities/init_test.gointernal/resources/gateways/deployment.gointernal/resources/gateways/deployment_test.gointernal/resources/ledgers/exports.gointernal/resources/ledgers/exports_test.gointernal/resources/stacks/networkpolicies.gointernal/resources/stacks/networkpolicies_test.gointernal/tests/networkpolicy_controller_test.go
NumaryBot
reviewed
Aug 7, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot review complete: no remaining inline findings.
Resolved 7 stale NumaryBot review threads (6 fixed, 1 outdated).
Summary: #492 (comment)
This was referenced Aug 7, 2026
flemzord
reviewed
Aug 7, 2026
flemzord
left a comment
flemzord
left a comment
Member
There was a problem hiding this comment.
I found two blocking lifecycle/capability issues and three controller-runtime correctness or policy-scope issues on the current head. CI is green, but the existing E2E suite does not exercise the Connectivity module.
NumaryBot
reviewed
Aug 7, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot posted 2 new inline findings.
Summary: #492 (comment)
NumaryBot
reviewed
Aug 7, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
NumaryBot review complete: no remaining inline findings.
Resolved 2 stale NumaryBot review threads (0 fixed, 2 outdated).
Summary: #492 (comment)
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.
What
Adds a new
formance.com/v1beta1 Connectivitystack module that binds connectivity to the stack's ledger, mirroring the Ledger v3 delegation pattern.The module doesn't run the workload itself — it delegates to the connectivity operator (
connectivity.formance.com) and reflects its readiness, exactly like theLedgermodule delegates to the ledger operator (ledger.formance.com Cluster).Behaviour
Capability + API-group detection (same mechanism as Ledger v3): at controller start-up it lists CRDs, checks the
connectivity.formance.com/ConnectivityCRD is present with a served version, and runs aSelfSubjectAccessReviewfor each required verb. If the group/CRD/RBAC is missing it reports the capability as unavailable and stays pending — it never fails controller setup.Gates on Ledger v3: connectivity ingests into the Ledger v3 gRPC endpoint, so it only provisions once the stack's
Ledgermodule is v3 and ready.Binds to the stack ledger: provisions a
connectivity.formance.com/v1alpha1 ConnectivitywithledgerAddress= the ledger v3 gRPC service, andledgerTLS= the ledger backend TLS secret (CA + SNI).The connection details come from
ledgers.V3GRPCBackendRef— the single source of truth already used to reach the ledger over gRPC — so connectivity and the gateway stay in sync (no duplicated address/secret naming).Reflects the delegated resource's
status.phaseback onto the module's Ready condition.Files
api/formance.com/v1beta1/connectivity_types.go— new module CR (Connectivity, labelledformance.com/kind=module).internal/resources/connectivities/init.go— reconciler, capability detection, ledger-v3 gate, delegated-resource bind.internal/resources/connectivities/init_test.go— unit tests for the capability-gating paths (discovery failure, inaccessible resource, missing RBAC, capability-unavailable reconcile).internal/resources/ledgers/exports.go— exportsIsV3+V3GRPCBackendRefso the ledger v3 gRPC connection stays the single source of truth.internal/resources/all.go— registers the module.Test
go build ./...✅go vet ./internal/resources/connectivities/...✅go test ./internal/resources/connectivities/...✅helm template ./helm/crdsrenders ✅Follow-ups (out of scope)