feat(licence): offline licence validation in operator#423
Open
feat(licence): offline licence validation in operator#423
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:
WalkthroughAdds JWT RS256 licence validation and startup licence resolution, stores licence namespace/state/message on Platform, gates EE reconciliation and image/resource creation on licence validity, and bumps many go-libs imports from v2 → v4. New unit tests cover licence logic and stack licence conditions. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as cmd/main
participant K8s as Kubernetes API
participant Core as internal/core/licence
participant Platform as Platform
participant Controller as Module Controller
CLI->>K8s: Read Secret (name) in namespace
alt Secret with token found
K8s-->>CLI: Secret (token)
CLI->>Core: ResolveLicenceState(reader, secretName, namespace)
Core->>Core: parseFormancePublicKey()\nValidateLicenceToken(token) (RS256 + exp)
Core-->>CLI: LicenceState, LicenceMessage
else Secret missing or empty
CLI-->>CLI: LicenceStateAbsent
end
CLI->>Platform: set LicenceNamespace, LicenceState & LicenceMessage
Controller->>Platform: check LicenceState for EE module
alt LicenceState == LicenceStateValid
Controller->>Controller: proceed with reconciliation / create licence ref / select EE image
else
Controller->>Controller: set LicenceValid=false condition\nskip EE reconciliation / delete licence ref
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Add centralized licence JWT validation directly in the operator using the embedded Formance RSA public key (same as go-libs/v4). This enables offline licence validation without any outbound network call. Behaviour by licence state: - Absent: operator deploys only non-EE modules (unchanged) - Valid: operator deploys all modules including EE (unchanged) - Expired/Invalid: operator continues reconciling non-EE modules, EE module reconciliation is skipped without deleting anything, a LicenceValid=False condition is set on the Stack and EE CRDs Key changes: - New LicenceValidator with embedded RSA public key (internal/core/licence.go) - EE gating in ForModule() — single enforcement point (controllers.go) - LicenceValid condition on Stack CRDs (stacks/init.go) - payments-ee image only used when licence is actually valid - Licence env vars only injected when licence is valid - Operator remains healthy regardless of licence state - 14 new unit tests covering JWT validation and condition behaviour
flemzord
force-pushed
the
feat/offline-licence-validation
branch
from
c1894f5 to
bbebbab
Compare
![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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/main.go`:
- Around line 53-72: The code currently falls back to a cluster-wide Secret list
on any reader.Get error (involving reader.Get, types.NamespacedName, secretName
and reader.List), which can mask RBAC/transient failures and mis-validate
duplicates; instead remove the cluster-wide scan and only treat a NotFound as
"missing": after reader.Get(..., secret) if err != nil { if
apierrors.IsNotFound(err) return core.LicenceStateInvalid, fmt.Sprintf("licence
secret %q not found", secretName) else return core.LicenceStateInvalid,
fmt.Sprintf("failed to get secret %q: %v", secretName, err) } — add the import
for k8s.io/apimachinery/pkg/api/errors as apierrors and drop the
corev1.SecretList/list logic.
- Around line 158-163: The current code computes licenceState/message once at
startup (using validateLicenceFromSecret with licenceSecret and
mgr.GetAPIReader()) and stores them on
platform.LicenceState/Platform.LicenceMessage, causing stale state; change the
logic so the licence is re-evaluated either when the Secret changes or during
each EE reconciliation: call validateLicenceFromSecret(...) from the
EnterpriseEdition reconciler (e.g., inside Reconcile in
internal/core/controllers.go or the EE reconciler method) using the
controller-runtime client or mgr.GetAPIReader(), and update
platform.LicenceState and platform.LicenceMessage with the fresh values (or
avoid global caching and use the returned values directly) so renewed/expired
tokens are reflected without restarting.
- Around line 75-80: The code currently only checks presence of the "token" key
but not its value, causing empty tokens to be treated as absent; update the
block that reads secret.Data["token"] (the token variable) to also validate that
the token is non-empty and, if empty, return core.LicenceStateInvalid with a
clear message rather than calling core.ValidateLicenceToken; ensure you use the
same token identifier and call signature (string(token)) when checking
length/emptiness before invoking core.ValidateLicenceToken.
In `@internal/core/controllers.go`:
- Around line 136-154: Replace the current partial licence check in the EE
branch with a full switch on ctx.GetPlatform().LicenceState (use t.IsEE(),
ctx.GetPlatform(), and t.GetConditions().AppendOrReplace as anchors): for
LicenceStateValid set a LicenceValid condition with Status True (clear any prior
false), for LicenceStateAbsent set LicenceValid False with Reason
LicenceStateAbsent and appropriate message, and for other non-valid states set
LicenceValid False with the corresponding state Reason/message; ensure you
respect LicenceSecret semantics (absence -> Absent) and always AppendOrReplace
the "LicenceValid" condition so transitions (Invalid→Valid and Absent)
update/clear previous values. Add regression tests exercising an EE module when
platform LicenceState is Absent and the Invalid→Valid transition to verify
ForObjectController no longer leaves a stale false condition.
In `@tools/utils/cmd/root.go`:
- Around line 8-9: Remove the full environment dump emitted at startup (the code
that iterates os.Environ or calls a helper to print all env vars) to avoid
leaking secrets; locate the startup/init code that logs environment variables
(e.g., in main() or init() where logger or logging is used) and delete that
dump, and if minimal diagnostics are needed replace it with an explicit
allowlist that uses os.LookupEnv for specific keys (example keys: "LOG_LEVEL",
"POD_NAME", "POD_NAMESPACE") and logs only those via logger.Debugf so secrets
are not exposed.
🪄 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
Run ID: 2b143f66-84db-4665-a3d4-bc5490b3034b
⛔ Files ignored due to path filters (6)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumtools/kubectl-stacks/go.modis excluded by!**/*.modtools/kubectl-stacks/go.sumis excluded by!**/*.sum,!**/*.sumtools/utils/go.modis excluded by!**/*.modtools/utils/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (41)
api/formance.com/v1beta1/shared.gocmd/main.gointernal/core/controllers.gointernal/core/env.gointernal/core/licence.gointernal/core/licence_test.gointernal/core/platform.gointernal/core/reconciler.gointernal/core/setup.gointernal/core/utils.gointernal/resources/applications/application.gointernal/resources/auths/configuration.gointernal/resources/auths/deployment.gointernal/resources/auths/init.gointernal/resources/benthos/controller.gointernal/resources/brokerconsumers/controller.gointernal/resources/brokerconsumers/create.gointernal/resources/brokers/reconcile.gointernal/resources/brokers/utils.gointernal/resources/caddy/caddy.gointernal/resources/databases/init.gointernal/resources/gateways/caddyfile.gointernal/resources/gateways/init.gointernal/resources/jobs/job.gointernal/resources/ledgers/reindex.gointernal/resources/licence/licence.gointernal/resources/payments/init.gointernal/resources/resourcereferences/init.gointernal/resources/services/services_test.gointernal/resources/settings/helpers.gointernal/resources/settings/helpers_test.gointernal/resources/settings/resourcerequirements.gointernal/resources/stacks/init.gointernal/resources/stacks/licence_test.gointernal/tests/internal/matcher_be_owned_by.gotools/kubectl-stacks/list.gotools/utils/cmd/database-create.gotools/utils/cmd/database-drop.gotools/utils/cmd/database.gotools/utils/cmd/root.gotools/utils/main.go
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
internal/core/controllers.go (1)
136-152:⚠️ Potential issue | 🔴 CriticalHandle
Absentand valid transitions explicitly here.This branch still lets EE modules reconcile when
LicenceSecretis empty, which contradicts the PR’s “Absent: EE not deployed” behavior. It also only ever writesLicenceValid=False, so anInvalid/Expired -> Validtransition can leave a stale current-generation false condition behind andForObjectControllerwill keep the module pending. Please make this a full licence-state flow: skip EE reconciliation forAbsent, and clear or replace any previousLicenceValid=Falsewhen the licence becomes valid.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/core/controllers.go` around lines 136 - 152, The current branch for EE licence handling (involving t.IsEE(), ctx.GetPlatform(), LicenceSecret, LicenceState and LicenceStateValid) incorrectly allows reconciliation when LicenceSecret is empty and only ever writes LicenceValid=False; modify the logic to explicitly treat LicenceState==Absent as "EE not deployed" and skip reconciliation in that case (no conditions written), and when LicenceState==LicenceStateValid replace/clear any existing LicenceValid=False by writing a LicenceValid condition with Status metav1.ConditionTrue (ObservedGeneration from t.GetGeneration(), LastTransitionTime metav1.Now(), Reason set appropriately and Message from platform.LicenceMessage if present) using t.GetConditions().AppendOrReplace (matching v1beta1.ConditionTypeMatch("LicenceValid")) so invalid->valid transitions remove the stale false condition and allow ForObjectController to proceed.cmd/main.go (3)
75-80:⚠️ Potential issue | 🟡 MinorTreat an empty
tokenvalue as invalid.
ValidateLicenceToken("")returnsLicenceStateAbsent, so a secret withtoken: ""is currently classified as “no licence” instead of “bad licence secret”. That misstates the condition and can bypass the intended invalid-secret path.Suggested fix
token, ok := secret.Data["token"] -if !ok { - return core.LicenceStateInvalid, "licence secret missing 'token' key" +if !ok || len(token) == 0 { + return core.LicenceStateInvalid, "licence secret missing non-empty 'token' key" } return core.ValidateLicenceToken(string(token))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/main.go` around lines 75 - 80, The code currently only checks for presence of secret.Data["token"] but not for an empty value, causing empty tokens to be treated as LicenceStateAbsent; update the logic that handles secret.Data["token"] (the token retrieval before calling core.ValidateLicenceToken) to also treat an empty byte slice/string as invalid by returning core.LicenceStateInvalid with a clear message (e.g., "licence secret 'token' is empty") instead of calling core.ValidateLicenceToken when token is empty; keep the presence check (ok) and only call core.ValidateLicenceToken(string(token)) when token is non-empty.
51-72:⚠️ Potential issue | 🟠 MajorReject ambiguous cross-namespace secret matches.
Falling back to a cluster-wide
SecretListand picking the first name match makes validation nondeterministic when the same secret name exists in multiple namespaces, and it also masks non-NotFoundread failures. This should either require an explicitnamespace/nameinput or fail when the lookup is ambiguous instead of silently validating an arbitrary secret.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/main.go` around lines 51 - 72, In validateLicenceFromSecret, stop silently falling back to the first name match across the cluster: if reader.Get returns a non-NotFound error, return that error instead of listing; only list when the original Get is a NotFound, then collect all secrets with s.Name == secretName; if zero matches return not found, if multiple matches return an ambiguous error (include the namespaces) instead of picking one, and only proceed when exactly one match is found; update error messages to clearly state ambiguity and prefer accepting explicit "namespace/name" input upstream if ambiguity is possible.
160-163:⚠️ Potential issue | 🟠 MajorRefresh licence state after startup.
platform.LicenceStateis computed once here and then reused by reconciliation ininternal/core/controllers.goand image selection ininternal/resources/payments/init.go. A token that expires or is renewed while the operator is running will stay in the old state until restart, so EE gating can drift from the actual secret contents.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/main.go` around lines 160 - 163, The code sets platform.LicenceState and LicenceMessage once using validateLicenceFromSecret at startup, causing stale EE gating; change callers to re-evaluate the secret or add a refresh method: either (A) replace direct reads of platform.LicenceState/Message in internal/core/controllers.go and internal/resources/payments/init.go with calls to validateLicenceFromSecret(mgr.GetAPIReader(), licenceSecret) at use-time, or (B) add a Platform method like RefreshLicence(apiReader, licenceSecret) that calls validateLicenceFromSecret and updates Platform.LicenceState/LicenseMessage, and invoke that refresh from reconcile paths and image-selection logic whenever the secret is read/used so state is kept current.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@cmd/main.go`:
- Around line 75-80: The code currently only checks for presence of
secret.Data["token"] but not for an empty value, causing empty tokens to be
treated as LicenceStateAbsent; update the logic that handles
secret.Data["token"] (the token retrieval before calling
core.ValidateLicenceToken) to also treat an empty byte slice/string as invalid
by returning core.LicenceStateInvalid with a clear message (e.g., "licence
secret 'token' is empty") instead of calling core.ValidateLicenceToken when
token is empty; keep the presence check (ok) and only call
core.ValidateLicenceToken(string(token)) when token is non-empty.
- Around line 51-72: In validateLicenceFromSecret, stop silently falling back to
the first name match across the cluster: if reader.Get returns a non-NotFound
error, return that error instead of listing; only list when the original Get is
a NotFound, then collect all secrets with s.Name == secretName; if zero matches
return not found, if multiple matches return an ambiguous error (include the
namespaces) instead of picking one, and only proceed when exactly one match is
found; update error messages to clearly state ambiguity and prefer accepting
explicit "namespace/name" input upstream if ambiguity is possible.
- Around line 160-163: The code sets platform.LicenceState and LicenceMessage
once using validateLicenceFromSecret at startup, causing stale EE gating; change
callers to re-evaluate the secret or add a refresh method: either (A) replace
direct reads of platform.LicenceState/Message in internal/core/controllers.go
and internal/resources/payments/init.go with calls to
validateLicenceFromSecret(mgr.GetAPIReader(), licenceSecret) at use-time, or (B)
add a Platform method like RefreshLicence(apiReader, licenceSecret) that calls
validateLicenceFromSecret and updates Platform.LicenceState/LicenseMessage, and
invoke that refresh from reconcile paths and image-selection logic whenever the
secret is read/used so state is kept current.
In `@internal/core/controllers.go`:
- Around line 136-152: The current branch for EE licence handling (involving
t.IsEE(), ctx.GetPlatform(), LicenceSecret, LicenceState and LicenceStateValid)
incorrectly allows reconciliation when LicenceSecret is empty and only ever
writes LicenceValid=False; modify the logic to explicitly treat
LicenceState==Absent as "EE not deployed" and skip reconciliation in that case
(no conditions written), and when LicenceState==LicenceStateValid replace/clear
any existing LicenceValid=False by writing a LicenceValid condition with Status
metav1.ConditionTrue (ObservedGeneration from t.GetGeneration(),
LastTransitionTime metav1.Now(), Reason set appropriately and Message from
platform.LicenceMessage if present) using t.GetConditions().AppendOrReplace
(matching v1beta1.ConditionTypeMatch("LicenceValid")) so invalid->valid
transitions remove the stale false condition and allow ForObjectController to
proceed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25306f4a-753a-4fd2-95ac-fd86b82a7d3d
⛔ Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (9)
cmd/main.gointernal/core/controllers.gointernal/core/licence.gointernal/core/licence_test.gointernal/core/platform.gointernal/resources/licence/licence.gointernal/resources/payments/init.gointernal/resources/stacks/init.gointernal/resources/stacks/licence_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/core/platform.go
- internal/resources/licence/licence.go
- Re-resolve licence state from Secret on each EE reconciliation, not just at startup — prevents stale state when token expires or Secret is updated while operator is running - Handle all licence state transitions in ForModule(): Valid → write LicenceValid=True (clears stale False condition) Absent → skip EE with LicenceValid=False/Absent Expired/Invalid → skip EE with LicenceValid=False - Remove dangerous cluster-wide Secret fallback scan; use explicit namespace via --licence-namespace flag (defaults to POD_NAMESPACE) - Treat empty token value as Invalid, not Absent - Remove env vars dump from utils startup logs (security) - Add SetFormancePublicKeyForTest for cross-package test overrides - Add tests: EmptyToken, SecretNotFound, real Secret-based conditions
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
internal/resources/stacks/init.go (1)
189-217:LicenceStateAbsentfalls through to "Unknown" instead of being handled explicitly.When
ResolveLicenceStatereturnsLicenceStateAbsent(e.g., if secretName is somehow empty after the initial check), this falls to the default case and sets reason "Unknown". For consistency withcontrollers.go(which handlesLicenceStateAbsentexplicitly), consider adding an explicit case.However, given the early return at lines 191-194 when
LicenceSecret == "", the only way to reachLicenceStateAbsentwould be ifResolveLicenceStatereturned it for a non-empty secret name, which doesn't happen in the current implementation. This is a minor consistency concern.♻️ Optional: Add explicit case for Absent
switch licenceState { case LicenceStateValid: condition.SetStatus(metav1.ConditionTrue).SetReason("Valid").SetMessage("Licence is valid") case LicenceStateExpired: condition.SetStatus(metav1.ConditionFalse).SetReason("Expired").SetMessage("Licence token is expired") case LicenceStateInvalid: msg := "Licence token is invalid" if licenceMessage != "" { msg = licenceMessage } condition.SetStatus(metav1.ConditionFalse).SetReason("Invalid").SetMessage(msg) + case LicenceStateAbsent: + // Should not reach here since we return early if LicenceSecret is empty + condition.SetStatus(metav1.ConditionFalse).SetReason("Absent").SetMessage("No licence configured") default: condition.SetStatus(metav1.ConditionFalse).SetReason("Unknown").SetMessage("Licence state unknown") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/resources/stacks/init.go` around lines 189 - 217, In setLicenceCondition, add an explicit case for LicenceStateAbsent (the value returned by ResolveLicenceState) instead of letting it fall through to default; update the condition to SetStatus(metav1.ConditionFalse) and SetReason("Absent") with a clear message (matching the behavior in controllers.go) so LicenceStateAbsent is handled consistently with other states returned by ResolveLicenceState; locate this change in the setLicenceCondition function where the switch on licenceState is implemented and add the new case alongside LicenceStateValid/Expired/Invalid.internal/core/licence.go (1)
87-117: Consider caching the parsed public key.
parseFormancePublicKey()is called on every token validation. While the overhead is minimal for PEM parsing, caching the result in async.Oncewould be more efficient, especially given this is called on every EE reconciliation.♻️ Optional: Cache the parsed public key
+var ( + parsedPublicKey *rsa.PublicKey + parsedPublicKeyOnce sync.Once + parsedPublicKeyErr error +) + +func getFormancePublicKey() (*rsa.PublicKey, error) { + parsedPublicKeyOnce.Do(func() { + parsedPublicKey, parsedPublicKeyErr = parseFormancePublicKey() + }) + return parsedPublicKey, parsedPublicKeyErr +} + func ValidateLicenceToken(token string) (LicenceState, string) { if token == "" { return LicenceStateAbsent, "" } - rsaPub, err := parseFormancePublicKey() + rsaPub, err := getFormancePublicKey() if err != nil {Note: This optimization would require updating
SetFormancePublicKeyForTestto also reset thesync.Once, which adds complexity. Given the current reconciliation frequency, the existing approach is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/core/licence.go` around lines 87 - 117, ValidateLicenceToken currently calls parseFormancePublicKey on every invocation; change this by caching the parsed RSA public key using a package-level variable and sync.Once (e.g., add a once variable and cached rsaPub used by ValidateLicenceToken) so parseFormancePublicKey is only run once, and update SetFormancePublicKeyForTest to reset the cache/once (or provide a test-only setter) so tests can replace the key; ensure error handling from parseFormancePublicKey is preserved and returned when the cached init fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/core/licence.go`:
- Around line 137-146: ResolveLicenceState currently uses context.Background()
which discards caller cancellation/deadline; change its signature to accept a
context.Context parameter (e.g., func ResolveLicenceState(ctx context.Context,
reader client.Reader, secretName string, operatorNamespace string)
(LicenceState, string)) and replace reader.Get(context.Background(), ...) with
reader.Get(ctx, ...). Update all call sites (notably in controllers.go and
cmd/main.go) to pass their existing reconciliation or main context through, and
add/adjust the context import if needed.
---
Nitpick comments:
In `@internal/core/licence.go`:
- Around line 87-117: ValidateLicenceToken currently calls
parseFormancePublicKey on every invocation; change this by caching the parsed
RSA public key using a package-level variable and sync.Once (e.g., add a once
variable and cached rsaPub used by ValidateLicenceToken) so
parseFormancePublicKey is only run once, and update SetFormancePublicKeyForTest
to reset the cache/once (or provide a test-only setter) so tests can replace the
key; ensure error handling from parseFormancePublicKey is preserved and returned
when the cached init fails.
In `@internal/resources/stacks/init.go`:
- Around line 189-217: In setLicenceCondition, add an explicit case for
LicenceStateAbsent (the value returned by ResolveLicenceState) instead of
letting it fall through to default; update the condition to
SetStatus(metav1.ConditionFalse) and SetReason("Absent") with a clear message
(matching the behavior in controllers.go) so LicenceStateAbsent is handled
consistently with other states returned by ResolveLicenceState; locate this
change in the setLicenceCondition function where the switch on licenceState is
implemented and add the new case alongside LicenceStateValid/Expired/Invalid.
🪄 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
Run ID: b8ac89c3-2554-417a-aeb5-06eda43561f5
📒 Files selected for processing (8)
cmd/main.gointernal/core/controllers.gointernal/core/licence.gointernal/core/licence_test.gointernal/core/platform.gointernal/resources/stacks/init.gointernal/resources/stacks/licence_test.gotools/utils/cmd/root.go
Comment on lines
+137
to
+146
| func ResolveLicenceState(reader client.Reader, secretName string, operatorNamespace string) (LicenceState, string) { | ||
| if secretName == "" { | ||
| return LicenceStateAbsent, "" | ||
| } | ||
|
|
||
| secret := &corev1.Secret{} | ||
| err := reader.Get(context.Background(), types.NamespacedName{ | ||
| Name: secretName, | ||
| Namespace: operatorNamespace, | ||
| }, secret) |
Contributor
There was a problem hiding this comment.
Pass caller's context instead of context.Background().
The function uses context.Background() for the Kubernetes API call, which loses the caller's cancellation signals and deadlines. Since this is called during reconciliation (which has its own context), the reconciliation's context should be passed through.
🔧 Proposed fix
-func ResolveLicenceState(reader client.Reader, secretName string, operatorNamespace string) (LicenceState, string) {
+func ResolveLicenceState(ctx context.Context, reader client.Reader, secretName string, operatorNamespace string) (LicenceState, string) {
if secretName == "" {
return LicenceStateAbsent, ""
}
secret := &corev1.Secret{}
- err := reader.Get(context.Background(), types.NamespacedName{
+ err := reader.Get(ctx, types.NamespacedName{
Name: secretName,
Namespace: operatorNamespace,
}, secret)This requires updating callers in controllers.go and cmd/main.go to pass their context.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func ResolveLicenceState(reader client.Reader, secretName string, operatorNamespace string) (LicenceState, string) { | |
| if secretName == "" { | |
| return LicenceStateAbsent, "" | |
| } | |
| secret := &corev1.Secret{} | |
| err := reader.Get(context.Background(), types.NamespacedName{ | |
| Name: secretName, | |
| Namespace: operatorNamespace, | |
| }, secret) | |
| func ResolveLicenceState(ctx context.Context, reader client.Reader, secretName string, operatorNamespace string) (LicenceState, string) { | |
| if secretName == "" { | |
| return LicenceStateAbsent, "" | |
| } | |
| secret := &corev1.Secret{} | |
| err := reader.Get(ctx, types.NamespacedName{ | |
| Name: secretName, | |
| Namespace: operatorNamespace, | |
| }, secret) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/core/licence.go` around lines 137 - 146, ResolveLicenceState
currently uses context.Background() which discards caller cancellation/deadline;
change its signature to accept a context.Context parameter (e.g., func
ResolveLicenceState(ctx context.Context, reader client.Reader, secretName
string, operatorNamespace string) (LicenceState, string)) and replace
reader.Get(context.Background(), ...) with reader.Get(ctx, ...). Update all call
sites (notably in controllers.go and cmd/main.go) to pass their existing
reconciliation or main context through, and add/adjust the context import if
needed.
The offline licence validation feature gates EE module reconciliation behind a valid licence. Integration tests were not configuring any licence state, causing all EE modules (Auth, Wallets, Orchestration) to be skipped and their deployments never created, resulting in timeouts.
The previous fix only set LicenceState without a LicenceSecret, causing GetLicenceEnvVars to create ResourceReferences to an empty secret name. This broke EE module reconciliation and caused cascading failures in non-EE tests. Now the test bootstrap: - Generates a test RSA key pair and overrides the embedded public key - Signs a valid JWT token - Creates a licence Secret in the test cluster - Configures Platform with LicenceSecret, LicenceNamespace, and LicenceState so all licence checks pass properly
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
internal/tests/internal/bootstrap.go (2)
129-135: Misleading variable name:licenceSecretis actually a Namespace.The variable
licenceSecretis declared as*corev1.Namespacebut the name suggests it's a Secret. This creates confusion when reading the code.♻️ Suggested fix
- licenceSecret := &corev1.Namespace{ + licenceNamespace := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: testLicenceSecretNamespace, }, } // default namespace should already exist, ignore error - _ = k8sClient.Create(context.Background(), licenceSecret) + _ = k8sClient.Create(context.Background(), licenceNamespace)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tests/internal/bootstrap.go` around lines 129 - 135, The variable named `licenceSecret` is misleading because it's created as a `*corev1.Namespace`; rename it to something like `licenceNamespace` (or `testLicenceNamespace`) and update all references (the declaration and the `k8sClient.Create(context.Background(), licenceSecret)` call) so the variable name matches its type (`*corev1.Namespace`) and avoids confusion with Secrets.
142-145: Theissuerfield is written but never used.Based on the
ResolveLicenceStateimplementation (which only reads the"token"key) andValidateLicenceToken(which doesn't validate issuer claims), theissuerfield is dead data. Consider removing it to avoid confusion about what the licence validation actually checks.♻️ Suggested fix
Data: map[string][]byte{ "token": []byte(licenceToken), - "issuer": []byte("https://license.formance.cloud/keys"), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/tests/internal/bootstrap.go` around lines 142 - 145, The test writes an unused "issuer" map entry which is dead data because ResolveLicenceState only reads "token" and ValidateLicenceToken doesn't check issuer claims; remove the "issuer": []byte("https://license.formance.cloud/keys") entry from the Data map in the bootstrap test (where licenceToken is used) to avoid confusion, or if issuer validation is intended, update ValidateLicenceToken and ResolveLicenceState to parse and validate the issuer claim accordingly—choose one approach and make the corresponding change so the test and validation code are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/tests/internal/bootstrap.go`:
- Around line 129-135: The variable named `licenceSecret` is misleading because
it's created as a `*corev1.Namespace`; rename it to something like
`licenceNamespace` (or `testLicenceNamespace`) and update all references (the
declaration and the `k8sClient.Create(context.Background(), licenceSecret)`
call) so the variable name matches its type (`*corev1.Namespace`) and avoids
confusion with Secrets.
- Around line 142-145: The test writes an unused "issuer" map entry which is
dead data because ResolveLicenceState only reads "token" and
ValidateLicenceToken doesn't check issuer claims; remove the "issuer":
[]byte("https://license.formance.cloud/keys") entry from the Data map in the
bootstrap test (where licenceToken is used) to avoid confusion, or if issuer
validation is intended, update ValidateLicenceToken and ResolveLicenceState to
parse and validate the issuer claim accordingly—choose one approach and make the
corresponding change so the test and validation code are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9fef6768-4c8f-49f1-9370-a6dcf9d80e79
📒 Files selected for processing (1)
internal/tests/internal/bootstrap.go
…okup The ResourceReference controller finds secrets by filtering on the formance.com/stack label. Without this label, the licence secret was invisible to the resource reference reconciler, causing "item not found: formance-licence" errors that blocked module reconciliation.
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.
Summary
This PR upgrades go-libs from v2.2.3 to v4.1.0 and adds centralized offline licence validation directly in the operator. The operator now validates licence JWT tokens using an embedded Formance RSA public key — no outbound network call required.
What changes for users
Before (go-libs v2)
LICENCE_TOKEN,LICENCE_ISSUER,LICENCE_VALIDATE_TICK) into EE service deploymentsjwk.Fetch)kubectl get stackshowed "Ready" even with an expired licenceAfter (go-libs v4 + operator validation)
--licence-secret)LicenceValid=TrueLicenceValid=False, Reason=ExpiredLicenceValid=False, Reason=InvalidKey behaviours
LicenceValidcondition on Stack and EE module CRDs visible viakubectl describepayments-eeimage is now only selected when licence is actually valid (not just present)Changes
New files
internal/core/licence.go—LicenceStatetype +ValidateLicenceToken()using embedded RSA public keyinternal/core/licence_test.go— 8 unit tests (valid, expired, malformed, wrong key, no exp, production key parse)internal/resources/stacks/licence_test.go— 6 unit tests (condition lifecycle: set, transition, removal)Modified files
go.mod/go.sum— go-libs v2→v4, addedgolang-jwt/jwt/v5internal/core/platform.go— AddedLicenceStateandLicenceMessagefieldsinternal/core/controllers.go— EE gating inForModule()(single enforcement point)internal/resources/stacks/init.go—LicenceValidcondition on Stackinternal/resources/payments/init.go—payments-eeimage only whenLicenceStateValidinternal/resources/licence/licence.go— env var injection only whenLicenceStateValidcmd/main.go— Read licence Secret + validate JWT at startupgo-libs/v2→go-libs/v4Test plan
go build ./...compilesgo vet ./...clean