From 2686893b2abf99891d6bd90337c3d3bfffde79a2 Mon Sep 17 00:00:00 2001 From: thivindu Date: Sun, 25 Oct 2026 23:01:28 +0530 Subject: [PATCH 1/3] Support externalRefId and in-place plan/token updates in the gateway operator Add an optional spec.externalRefId to the Subscription CR, used as the gateway subscription ID. Update subscriptionPlanId, subscriptionToken (including a rotated valueFrom Secret) and status in place through the management API instead of rejecting them as immutable. Removing the plan from the CR clears it on the gateway. Only externalRefId, apiId, applicationId and the billing IDs stay immutable. The immutable-fields fingerprint replaces the old create-fields fingerprint, and the legacy annotation is removed on the next reconcile. After a create conflict, an existing subscription is adopted only when it belongs to the CR's API and application: - With externalRefId, the subscription with that ID must be listed under the CR's API and application; otherwise a generic, non-retryable 409 is reported that doesn't say whether the ID or the token collided. - Without it, an ID is returned only when exactly one subscription on the API matches the CR's application (unset, empty and blank application IDs mean no application); otherwise the conflict is retried. Co-Authored-By: Claude Opus 5.5 --- gateway/examples/subscription.yaml | 7 +- .../api/v1/subscription_types.go | 24 ++- .../api/v1/zz_generated.deepcopy.go | 5 + .../api/v1alpha1/subscription_types.go | 24 ++- .../api/v1alpha1/zz_generated.deepcopy.go | 5 + ...y.api-platform.wso2.com_subscriptions.yaml | 48 ++++-- .../controller/subscription_controller.go | 146 +++++++++++------- .../subscription_controller_test.go | 133 ++++++++++++++++ .../gatewayclient/subscription_client.go | 85 +++++----- .../gatewayclient/subscription_client_test.go | 123 +++++++++++++++ ...y.api-platform.wso2.com_subscriptions.yaml | 48 ++++-- 11 files changed, 520 insertions(+), 128 deletions(-) create mode 100644 kubernetes/gateway-operator/internal/controller/subscription_controller_test.go diff --git a/gateway/examples/subscription.yaml b/gateway/examples/subscription.yaml index ec978f549d..be2f0eca09 100644 --- a/gateway/examples/subscription.yaml +++ b/gateway/examples/subscription.yaml @@ -11,13 +11,16 @@ # A Subscription binds an application to an API and (optionally) a plan. # The token is sourced from a Kubernetes Secret so the plaintext is never # stored in the CR; the operator forwards it to the gateway which only -# persists its hash. The gateway-issued UUID is recorded in .status.id and -# used for subsequent update/delete calls. +# persists its hash. The gateway-issued UUID (or spec.externalRefId, when set) +# is recorded in .status.id and used for subsequent update/delete calls. +# subscriptionPlanId, status and the token (including a change to the Secret +# data) are updated in place; the other spec fields are immutable. apiVersion: gateway.api-platform.wso2.com/v1 kind: Subscription metadata: name: petstore-acme-bronze spec: + # externalRefId: acme-petstore-sub-01 # optional; used as the subscription ID apiId: petstore-api-v1.0 subscriptionPlanId: bronze-1k-per-min status: ACTIVE diff --git a/kubernetes/gateway-operator/api/v1/subscription_types.go b/kubernetes/gateway-operator/api/v1/subscription_types.go index 5a92cbb49b..66e0bd7000 100644 --- a/kubernetes/gateway-operator/api/v1/subscription_types.go +++ b/kubernetes/gateway-operator/api/v1/subscription_types.go @@ -21,16 +21,28 @@ import ( ) // SubscriptionSpec mirrors the management-API SubscriptionCreateRequest -// payload. The gateway-controller issues a UUID id which the controller -// persists to .status.id for subsequent PUT/DELETE. +// payload. The gateway-controller issues a UUID id (or uses externalRefId when +// set) which the controller persists to .status.id for subsequent PUT/DELETE. +// subscriptionToken, subscriptionPlanId and status can be changed after the +// subscription is deployed; the other fields are immutable. type SubscriptionSpec struct { + // ExternalRefId is an optional caller-supplied reference ID. When set, the + // gateway uses it as the subscription id instead of generating one. + // Must be unique on the gateway. Immutable once deployed. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=64 + // +kubebuilder:validation:Pattern=`^[A-Za-z0-9][A-Za-z0-9._-]*$` + // +optional + ExternalRefId *string `json:"externalRefId,omitempty"` + // ApiId is the API identifier (deployment id or handle). // +kubebuilder:validation:Required ApiId string `json:"apiId"` // SubscriptionToken is the opaque token used to invoke the API. The // token is sent as plaintext to the gateway which stores only its hash; - // inline in the CR or supplied via valueFrom. + // inline in the CR or supplied via valueFrom. Changing it (or the + // referenced Secret data) rotates the token on the gateway in place. // +kubebuilder:validation:Required SubscriptionToken SecretValueSource `json:"subscriptionToken"` @@ -38,9 +50,9 @@ type SubscriptionSpec struct { // +optional ApplicationId *string `json:"applicationId,omitempty"` - // SubscriptionPlanId is an optional plan UUID. When the plan is also - // managed via SubscriptionPlan CR, set this to the SubscriptionPlan's - // .status.id once it has been deployed. + // SubscriptionPlanId is an optional plan UUID, or the name of a + // SubscriptionPlan CR in the same namespace. Changing it moves the + // subscription to the new plan in place; removing it removes the plan. // +optional SubscriptionPlanId *string `json:"subscriptionPlanId,omitempty"` diff --git a/kubernetes/gateway-operator/api/v1/zz_generated.deepcopy.go b/kubernetes/gateway-operator/api/v1/zz_generated.deepcopy.go index ecd1902a12..63b03c4b9a 100644 --- a/kubernetes/gateway-operator/api/v1/zz_generated.deepcopy.go +++ b/kubernetes/gateway-operator/api/v1/zz_generated.deepcopy.go @@ -2239,6 +2239,11 @@ func (in *SubscriptionPlanSpec) DeepCopy() *SubscriptionPlanSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SubscriptionSpec) DeepCopyInto(out *SubscriptionSpec) { *out = *in + if in.ExternalRefId != nil { + in, out := &in.ExternalRefId, &out.ExternalRefId + *out = new(string) + **out = **in + } in.SubscriptionToken.DeepCopyInto(&out.SubscriptionToken) if in.ApplicationId != nil { in, out := &in.ApplicationId, &out.ApplicationId diff --git a/kubernetes/gateway-operator/api/v1alpha1/subscription_types.go b/kubernetes/gateway-operator/api/v1alpha1/subscription_types.go index 9ed3062057..af0eea292c 100644 --- a/kubernetes/gateway-operator/api/v1alpha1/subscription_types.go +++ b/kubernetes/gateway-operator/api/v1alpha1/subscription_types.go @@ -21,16 +21,28 @@ import ( ) // SubscriptionSpec mirrors the management-API SubscriptionCreateRequest -// payload. The gateway-controller issues a UUID id which the controller -// persists to .status.id for subsequent PUT/DELETE. +// payload. The gateway-controller issues a UUID id (or uses externalRefId when +// set) which the controller persists to .status.id for subsequent PUT/DELETE. +// subscriptionToken, subscriptionPlanId and status can be changed after the +// subscription is deployed; the other fields are immutable. type SubscriptionSpec struct { + // ExternalRefId is an optional caller-supplied reference ID. When set, the + // gateway uses it as the subscription id instead of generating one. + // Must be unique on the gateway. Immutable once deployed. + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=64 + // +kubebuilder:validation:Pattern=`^[A-Za-z0-9][A-Za-z0-9._-]*$` + // +optional + ExternalRefId *string `json:"externalRefId,omitempty"` + // ApiId is the API identifier (deployment id or handle). // +kubebuilder:validation:Required ApiId string `json:"apiId"` // SubscriptionToken is the opaque token used to invoke the API. The // token is sent as plaintext to the gateway which stores only its hash; - // inline in the CR or supplied via valueFrom. + // inline in the CR or supplied via valueFrom. Changing it (or the + // referenced Secret data) rotates the token on the gateway in place. // +kubebuilder:validation:Required SubscriptionToken SecretValueSource `json:"subscriptionToken"` @@ -38,9 +50,9 @@ type SubscriptionSpec struct { // +optional ApplicationId *string `json:"applicationId,omitempty"` - // SubscriptionPlanId is an optional plan UUID. When the plan is also - // managed via SubscriptionPlan CR, set this to the SubscriptionPlan's - // .status.id once it has been deployed. + // SubscriptionPlanId is an optional plan UUID, or the name of a + // SubscriptionPlan CR in the same namespace. Changing it moves the + // subscription to the new plan in place; removing it removes the plan. // +optional SubscriptionPlanId *string `json:"subscriptionPlanId,omitempty"` diff --git a/kubernetes/gateway-operator/api/v1alpha1/zz_generated.deepcopy.go b/kubernetes/gateway-operator/api/v1alpha1/zz_generated.deepcopy.go index aa0a944792..1404a3aae6 100644 --- a/kubernetes/gateway-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/kubernetes/gateway-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -2239,6 +2239,11 @@ func (in *SubscriptionPlanSpec) DeepCopy() *SubscriptionPlanSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SubscriptionSpec) DeepCopyInto(out *SubscriptionSpec) { *out = *in + if in.ExternalRefId != nil { + in, out := &in.ExternalRefId, &out.ExternalRefId + *out = new(string) + **out = **in + } in.SubscriptionToken.DeepCopyInto(&out.SubscriptionToken) if in.ApplicationId != nil { in, out := &in.ApplicationId, &out.ApplicationId diff --git a/kubernetes/gateway-operator/config/crd/bases/gateway.api-platform.wso2.com_subscriptions.yaml b/kubernetes/gateway-operator/config/crd/bases/gateway.api-platform.wso2.com_subscriptions.yaml index 3746967daf..543336ced9 100644 --- a/kubernetes/gateway-operator/config/crd/bases/gateway.api-platform.wso2.com_subscriptions.yaml +++ b/kubernetes/gateway-operator/config/crd/bases/gateway.api-platform.wso2.com_subscriptions.yaml @@ -48,8 +48,10 @@ spec: spec: description: |- SubscriptionSpec mirrors the management-API SubscriptionCreateRequest - payload. The gateway-controller issues a UUID id which the controller - persists to .status.id for subsequent PUT/DELETE. + payload. The gateway-controller issues a UUID id (or uses externalRefId when + set) which the controller persists to .status.id for subsequent PUT/DELETE. + subscriptionToken, subscriptionPlanId and status can be changed after the + subscription is deployed; the other fields are immutable. properties: apiId: description: ApiId is the API identifier (deployment id or handle). @@ -64,6 +66,15 @@ spec: description: BillingSubscriptionId is an optional billing subscription identifier. type: string + externalRefId: + description: |- + ExternalRefId is an optional caller-supplied reference ID. When set, the + gateway uses it as the subscription id instead of generating one. + Must be unique on the gateway. Immutable once deployed. + maxLength: 64 + minLength: 1 + pattern: ^[A-Za-z0-9][A-Za-z0-9._-]*$ + type: string status: description: |- Status is the lifecycle state for this subscription. Mirrors the @@ -71,15 +82,16 @@ spec: type: string subscriptionPlanId: description: |- - SubscriptionPlanId is an optional plan UUID. When the plan is also - managed via SubscriptionPlan CR, set this to the SubscriptionPlan's - .status.id once it has been deployed. + SubscriptionPlanId is an optional plan UUID, or the name of a + SubscriptionPlan CR in the same namespace. Changing it moves the + subscription to the new plan in place; removing it removes the plan. type: string subscriptionToken: description: |- SubscriptionToken is the opaque token used to invoke the API. The token is sent as plaintext to the gateway which stores only its hash; - inline in the CR or supplied via valueFrom. + inline in the CR or supplied via valueFrom. Changing it (or the + referenced Secret data) rotates the token on the gateway in place. properties: value: description: |- @@ -240,8 +252,10 @@ spec: spec: description: |- SubscriptionSpec mirrors the management-API SubscriptionCreateRequest - payload. The gateway-controller issues a UUID id which the controller - persists to .status.id for subsequent PUT/DELETE. + payload. The gateway-controller issues a UUID id (or uses externalRefId when + set) which the controller persists to .status.id for subsequent PUT/DELETE. + subscriptionToken, subscriptionPlanId and status can be changed after the + subscription is deployed; the other fields are immutable. properties: apiId: description: ApiId is the API identifier (deployment id or handle). @@ -256,6 +270,15 @@ spec: description: BillingSubscriptionId is an optional billing subscription identifier. type: string + externalRefId: + description: |- + ExternalRefId is an optional caller-supplied reference ID. When set, the + gateway uses it as the subscription id instead of generating one. + Must be unique on the gateway. Immutable once deployed. + maxLength: 64 + minLength: 1 + pattern: ^[A-Za-z0-9][A-Za-z0-9._-]*$ + type: string status: description: |- Status is the lifecycle state for this subscription. Mirrors the @@ -263,15 +286,16 @@ spec: type: string subscriptionPlanId: description: |- - SubscriptionPlanId is an optional plan UUID. When the plan is also - managed via SubscriptionPlan CR, set this to the SubscriptionPlan's - .status.id once it has been deployed. + SubscriptionPlanId is an optional plan UUID, or the name of a + SubscriptionPlan CR in the same namespace. Changing it moves the + subscription to the new plan in place; removing it removes the plan. type: string subscriptionToken: description: |- SubscriptionToken is the opaque token used to invoke the API. The token is sent as plaintext to the gateway which stores only its hash; - inline in the CR or supplied via valueFrom. + inline in the CR or supplied via valueFrom. Changing it (or the + referenced Secret data) rotates the token on the gateway in place. properties: value: description: |- diff --git a/kubernetes/gateway-operator/internal/controller/subscription_controller.go b/kubernetes/gateway-operator/internal/controller/subscription_controller.go index ef990c71b4..d22f968c7b 100644 --- a/kubernetes/gateway-operator/internal/controller/subscription_controller.go +++ b/kubernetes/gateway-operator/internal/controller/subscription_controller.go @@ -44,13 +44,16 @@ import ( const ( subscriptionFinalizer = "gateway.api-platform.wso2.com/subscription-finalizer" - annSubscriptionLastAppliedCreateFingerprint = "gateway.api-platform.wso2.com/subscription-last-applied-create-fingerprint" + annSubscriptionLastAppliedImmutableFingerprint = "gateway.api-platform.wso2.com/subscription-last-applied-immutable-fingerprint" + // annSubscriptionLegacyCreateFingerprint was written by earlier operator versions, when plan and token were + // also immutable. It is removed when the immutable fingerprint is first stored. + annSubscriptionLegacyCreateFingerprint = "gateway.api-platform.wso2.com/subscription-last-applied-create-fingerprint" annSubscriptionLastAppliedTokenSecretBindingRV = "gateway.api-platform.wso2.com/last-applied-subscription-token-secret-rv" ) // SubscriptionReconciler reconciles Subscription CRs against /subscriptions. -// The gateway issues a UUID id on first POST that the controller persists -// to .status.id. +// The gateway issues a UUID id on first POST (or uses spec.externalRefId) that +// the controller persists to .status.id. type SubscriptionReconciler struct { GenericReconciler } @@ -150,16 +153,18 @@ func (a *subscriptionAdapter) GatewaySelectionKey(obj client.Object) (string, ma func (a *subscriptionAdapter) Deploy(ctx context.Context, k8sClient client.Client, gatewayEndpoint string, obj client.Object, authFn gatewayclient.AuthHeaderFunc) (DeployResult, error) { cr := obj.(*apiv1.Subscription) + token, err := secretsource.Resolve(ctx, k8sClient, "spec.subscriptionToken", cr.Spec.SubscriptionToken, cr.Namespace) + if err != nil { + return DeployResult{}, classifySecretSourceResolveError(err) + } + resolvedPlanID, err := resolveSubscriptionPlanID(ctx, k8sClient, cr) + if err != nil { + return DeployResult{}, err + } + if cr.Status.Id == "" { - token, err := secretsource.Resolve(ctx, k8sClient, "spec.subscriptionToken", cr.Spec.SubscriptionToken, cr.Namespace) - if err != nil { - return DeployResult{}, classifySecretSourceResolveError(err) - } - resolvedPlanID, err := resolveSubscriptionPlanIDForCreate(ctx, k8sClient, cr) - if err != nil { - return DeployResult{}, err - } payload := gatewayclient.SubscriptionCreatePayload{ + ExternalRefId: cr.Spec.ExternalRefId, ApiId: cr.Spec.ApiId, SubscriptionToken: token, ApplicationId: cr.Spec.ApplicationId, @@ -172,13 +177,17 @@ func (a *subscriptionAdapter) Deploy(ctx context.Context, k8sClient client.Clien if err != nil { var nr *gatewayclient.NonRetryableError if errors.As(err, &nr) && nr.StatusCode == http.StatusConflict { - recoveredID, recErr := gatewayclient.FindSubscriptionIDByAPIAndApplication(ctx, gatewayEndpoint, cr.Spec.ApiId, cr.Spec.ApplicationId, authFn) + recoveredID, recErr := recoverSubscriptionIDAfterConflict(ctx, gatewayEndpoint, cr, authFn) if recErr != nil { return DeployResult{}, recErr } if recoveredID != "" { - if fp, fpErr := subscriptionCreateFingerprint(cr); fpErr == nil { - _ = setSubscriptionLastAppliedCreateFingerprint(ctx, k8sClient, cr, fp) + // The existing subscription may predate spec changes; bring its mutable fields in line. + if err := updateSubscriptionMutableFields(ctx, gatewayEndpoint, recoveredID, cr, token, resolvedPlanID, authFn); err != nil { + return DeployResult{}, err + } + if fp, fpErr := subscriptionImmutableFingerprint(cr); fpErr == nil { + _ = setSubscriptionLastAppliedImmutableFingerprint(ctx, k8sClient, cr, fp) } return DeployResult{Id: recoveredID}, nil } @@ -189,57 +198,82 @@ func (a *subscriptionAdapter) Deploy(ctx context.Context, k8sClient client.Clien } return DeployResult{}, err } - // Persist a fingerprint of create-only fields so later reconciles can detect forbidden updates. - if fp, fpErr := subscriptionCreateFingerprint(cr); fpErr == nil { + // Persist a fingerprint of immutable fields so later reconciles can detect forbidden updates. + if fp, fpErr := subscriptionImmutableFingerprint(cr); fpErr == nil { // Best-effort; failure to persist the fingerprint should not cause gateway duplicate creates. - _ = setSubscriptionLastAppliedCreateFingerprint(ctx, k8sClient, cr, fp) + _ = setSubscriptionLastAppliedImmutableFingerprint(ctx, k8sClient, cr, fp) } return DeployResult{Id: resp.Id}, nil } - // If this Subscription is already deployed (Status.Id set), then create-only fields must remain unchanged. - if lastFp := subscriptionLastAppliedCreateFingerprint(cr); lastFp != "" { - if fp, fpErr := subscriptionCreateFingerprint(cr); fpErr == nil && lastFp != fp { + // If this Subscription is already deployed (Status.Id set), then immutable fields must remain unchanged. + if lastFp := subscriptionLastAppliedImmutableFingerprint(cr); lastFp != "" { + if fp, fpErr := subscriptionImmutableFingerprint(cr); fpErr == nil && lastFp != fp { return DeployResult{}, &gatewayclient.NonRetryableError{ - Err: fmt.Errorf("create-only fields for Subscription (spec.apiId, spec.subscriptionToken, spec.applicationId, spec.subscriptionPlanId, spec.billingCustomerId, spec.billingSubscriptionId) are immutable once status.id is set; delete and recreate the Subscription to apply changes"), + Err: fmt.Errorf("spec.externalRefId, spec.apiId, spec.applicationId, spec.billingCustomerId and spec.billingSubscriptionId are immutable once status.id is set; delete and recreate the Subscription to apply changes"), } } } else { // No prior fingerprint stored (e.g. pre-upgrade CR): establish a baseline. - if fp, fpErr := subscriptionCreateFingerprint(cr); fpErr == nil { - _ = setSubscriptionLastAppliedCreateFingerprint(ctx, k8sClient, cr, fp) + if fp, fpErr := subscriptionImmutableFingerprint(cr); fpErr == nil { + _ = setSubscriptionLastAppliedImmutableFingerprint(ctx, k8sClient, cr, fp) } } - // Secret-backed token: management PUT only supports status, not token rotation. - fpNow, err := subscriptionTokenSecretBindingFingerprint(ctx, k8sClient, cr) - if err != nil { + // status, subscriptionPlanId and subscriptionToken are updated in place. Sending the token on every + // update also applies a rotated valueFrom Secret, since Secret drift triggers this redeploy. + if err := updateSubscriptionMutableFields(ctx, gatewayEndpoint, cr.Status.Id, cr, token, resolvedPlanID, authFn); err != nil { return DeployResult{}, err } - if fpNow != "" { - prev := subscriptionLastAppliedTokenSecretBindingFingerprint(cr) - if prev != "" && prev != fpNow { - return DeployResult{}, &gatewayclient.NonRetryableError{ - Err: fmt.Errorf("spec.subscriptionToken valueFrom Secret data changed but the gateway management API cannot update an existing subscription token (only status). Delete this Subscription CR and recreate it to register the new token"), - } - } + return DeployResult{Id: cr.Status.Id}, nil +} + +// updateSubscriptionMutableFields PUTs the CR's status, plan and token to the gateway subscription. +// The plan is always sent, so a plan removed from the CR is cleared on the gateway. +func updateSubscriptionMutableFields(ctx context.Context, gatewayEndpoint, subscriptionID string, cr *apiv1.Subscription, token string, planID *string, authFn gatewayclient.AuthHeaderFunc) error { + payload := gatewayclient.SubscriptionUpdatePayload{ + Status: cr.Spec.Status, + SubscriptionPlanId: planID, + SubscriptionToken: &token, } + _, err := gatewayclient.UpdateSubscription(ctx, gatewayEndpoint, subscriptionID, payload, authFn) + return err +} - if cr.Spec.Status != nil { - updatePayload := gatewayclient.SubscriptionUpdatePayload{Status: cr.Spec.Status} - if _, err := gatewayclient.UpdateSubscription(ctx, gatewayEndpoint, cr.Status.Id, updatePayload, authFn); err != nil { - return DeployResult{}, err +// recoverSubscriptionIDAfterConflict finds the gateway subscription that a conflicting create collided +// with. With spec.externalRefId the subscription id is known, so it is adopted only when that id is one of +// this API's subscriptions and belongs to the CR's application; otherwise the id or the token belongs to a +// different subscription, and the conflict is reported without saying which, so the error never confirms +// that a token is in use. Without +// spec.externalRefId the id is looked up by apiId and applicationId, and is returned only when exactly one +// subscription matches; otherwise no id is returned and the create conflict is retried. +func recoverSubscriptionIDAfterConflict(ctx context.Context, gatewayEndpoint string, cr *apiv1.Subscription, authFn gatewayclient.AuthHeaderFunc) (string, error) { + if cr.Spec.ExternalRefId != nil && *cr.Spec.ExternalRefId != "" { + subscriptions, err := gatewayclient.ListSubscriptions(ctx, gatewayEndpoint, cr.Spec.ApiId, nil, authFn) + if err != nil { + return "", err + } + for i := range subscriptions { + if subscriptions[i].Id == *cr.Spec.ExternalRefId && + gatewayclient.SameApplication(subscriptions[i].ApplicationId, cr.Spec.ApplicationId) { + return subscriptions[i].Id, nil + } + } + return "", &gatewayclient.NonRetryableError{ + StatusCode: http.StatusConflict, + Err: fmt.Errorf("the subscription conflicts with an existing subscription on the gateway; " + + "check that spec.externalRefId and spec.subscriptionToken are not used by another subscription"), } } - return DeployResult{Id: cr.Status.Id}, nil + return gatewayclient.FindSubscriptionIDByAPIAndApplication(ctx, gatewayEndpoint, cr.Spec.ApiId, cr.Spec.ApplicationId, authFn) } -// resolveSubscriptionPlanIDForCreate allows Subscription.spec.subscriptionPlanId +// resolveSubscriptionPlanID allows Subscription.spec.subscriptionPlanId // to be either a literal gateway UUID or a same-namespace SubscriptionPlan name. // If a SubscriptionPlan CR with that name exists but has no status.id yet, we // return a retryable error so reconciliation naturally retries after the plan is // created in the gateway. -func resolveSubscriptionPlanIDForCreate(ctx context.Context, k8sClient client.Client, cr *apiv1.Subscription) (*string, error) { +func resolveSubscriptionPlanID(ctx context.Context, k8sClient client.Client, cr *apiv1.Subscription) (*string, error) { if cr.Spec.SubscriptionPlanId == nil || *cr.Spec.SubscriptionPlanId == "" { return nil, nil } @@ -260,21 +294,20 @@ func resolveSubscriptionPlanIDForCreate(ctx context.Context, k8sClient client.Cl return &id, nil } -type subscriptionCreateFingerprintData struct { - ApiId string `json:"apiId"` - SubscriptionToken apiv1.SecretValueSource `json:"subscriptionToken"` - ApplicationId *string `json:"applicationId,omitempty"` - SubscriptionPlanId *string `json:"subscriptionPlanId,omitempty"` - BillingCustomerId *string `json:"billingCustomerId,omitempty"` - BillingSubscriptionId *string `json:"billingSubscriptionId,omitempty"` +// subscriptionImmutableFingerprintData holds the fields the management API can't change after create. +type subscriptionImmutableFingerprintData struct { + ExternalRefId *string `json:"externalRefId,omitempty"` + ApiId string `json:"apiId"` + ApplicationId *string `json:"applicationId,omitempty"` + BillingCustomerId *string `json:"billingCustomerId,omitempty"` + BillingSubscriptionId *string `json:"billingSubscriptionId,omitempty"` } -func subscriptionCreateFingerprint(cr *apiv1.Subscription) (string, error) { - data := subscriptionCreateFingerprintData{ +func subscriptionImmutableFingerprint(cr *apiv1.Subscription) (string, error) { + data := subscriptionImmutableFingerprintData{ + ExternalRefId: cr.Spec.ExternalRefId, ApiId: cr.Spec.ApiId, - SubscriptionToken: cr.Spec.SubscriptionToken, ApplicationId: cr.Spec.ApplicationId, - SubscriptionPlanId: cr.Spec.SubscriptionPlanId, BillingCustomerId: cr.Spec.BillingCustomerId, BillingSubscriptionId: cr.Spec.BillingSubscriptionId, } @@ -286,19 +319,22 @@ func subscriptionCreateFingerprint(cr *apiv1.Subscription) (string, error) { return base64.RawStdEncoding.EncodeToString(sum[:]), nil } -func subscriptionLastAppliedCreateFingerprint(cr *apiv1.Subscription) string { +func subscriptionLastAppliedImmutableFingerprint(cr *apiv1.Subscription) string { if cr.Annotations == nil { return "" } - return cr.Annotations[annSubscriptionLastAppliedCreateFingerprint] + return cr.Annotations[annSubscriptionLastAppliedImmutableFingerprint] } -func setSubscriptionLastAppliedCreateFingerprint(ctx context.Context, k8sClient client.Client, cr *apiv1.Subscription, fingerprint string) error { +// setSubscriptionLastAppliedImmutableFingerprint stores the fingerprint and drops the legacy +// create-fingerprint annotation, which also covered the now-mutable plan and token. +func setSubscriptionLastAppliedImmutableFingerprint(ctx context.Context, k8sClient client.Client, cr *apiv1.Subscription, fingerprint string) error { orig := cr.DeepCopy() if cr.Annotations == nil { cr.Annotations = map[string]string{} } - cr.Annotations[annSubscriptionLastAppliedCreateFingerprint] = fingerprint + cr.Annotations[annSubscriptionLastAppliedImmutableFingerprint] = fingerprint + delete(cr.Annotations, annSubscriptionLegacyCreateFingerprint) return k8sClient.Patch(ctx, cr, client.MergeFrom(orig)) } diff --git a/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go b/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go new file mode 100644 index 0000000000..b9365d83fd --- /dev/null +++ b/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go @@ -0,0 +1,133 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the Apache License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + + apiv1 "github.com/wso2/api-platform/kubernetes/gateway-operator/api/v1" + "github.com/wso2/api-platform/kubernetes/gateway-operator/internal/gatewayclient" +) + +// subscriptionListServer serves GET /subscriptions?apiId=... with the subscriptions each API holds. +func subscriptionListServer(t *testing.T, byAPI map[string][]map[string]any) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodGet, r.Method) + require.Equal(t, gatewayclient.ManagementAPIBasePath+"/subscriptions", r.URL.Path) + items := byAPI[r.URL.Query().Get("apiId")] + if items == nil { + items = []map[string]any{} + } + _ = json.NewEncoder(w).Encode(map[string]any{"subscriptions": items, "count": len(items)}) + })) + t.Cleanup(srv.Close) + return srv +} + +func subscriptionCR(apiID string, externalRefID *string) *apiv1.Subscription { + cr := &apiv1.Subscription{} + cr.Spec.ApiId = apiID + cr.Spec.ExternalRefId = externalRefID + return cr +} + +func TestRecoverSubscriptionIDAfterConflict_AdoptsExternalRefIdOnTheSameAPI(t *testing.T) { + srv := subscriptionListServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "other-sub"}, {"id": "acme-sub-01", "externalRefId": "acme-sub-01"}}, + }) + ref := "acme-sub-01" + + id, err := recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, subscriptionCR("orders-api", &ref), nil) + require.NoError(t, err) + require.Equal(t, "acme-sub-01", id) +} + +// A subscription with the same ID on another API must never be adopted: the follow-up update would +// overwrite that subscription's plan and token. The error must not say whether the ID or the token +// collided, so it never confirms that a token is in use. +func TestRecoverSubscriptionIDAfterConflict_RejectsExternalRefIdNotOnTheAPI(t *testing.T) { + srv := subscriptionListServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "other-sub"}}, + "payments-api": {{"id": "acme-sub-01", "externalRefId": "acme-sub-01"}}, + }) + ref := "acme-sub-01" + + id, err := recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, subscriptionCR("orders-api", &ref), nil) + require.Empty(t, id) + var nonRetryable *gatewayclient.NonRetryableError + require.ErrorAs(t, err, &nonRetryable) + require.Equal(t, http.StatusConflict, nonRetryable.StatusCode) + require.Contains(t, err.Error(), "conflicts with an existing subscription") + require.NotContains(t, err.Error(), "already used") + require.NotContains(t, err.Error(), "payments-api") +} + +func TestRecoverSubscriptionIDAfterConflict_WithoutExternalRefIdUsesTheAPIAndApplication(t *testing.T) { + srv := subscriptionListServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "generated-uuid"}}, + }) + + id, err := recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, subscriptionCR("orders-api", nil), nil) + require.NoError(t, err) + require.Equal(t, "generated-uuid", id) + + empty := "" + id, err = recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, subscriptionCR("orders-api", &empty), nil) + require.NoError(t, err) + require.Equal(t, "generated-uuid", id, "an empty externalRefId behaves as if it were not set") +} + +// An externalRefId subscription on the same API but for another application belongs to another CR, so it +// must not be adopted and overwritten. +func TestRecoverSubscriptionIDAfterConflict_RequiresTheCRApplication(t *testing.T) { + srv := subscriptionListServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "acme-sub-01", "externalRefId": "acme-sub-01", "applicationId": "app-x"}}, + }) + ref, app := "acme-sub-01", "app-1" + + cr := subscriptionCR("orders-api", &ref) + cr.Spec.ApplicationId = &app + id, err := recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, cr, nil) + require.Empty(t, id) + var nonRetryable *gatewayclient.NonRetryableError + require.ErrorAs(t, err, &nonRetryable) + require.Equal(t, http.StatusConflict, nonRetryable.StatusCode) + + app = "app-x" + id, err = recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, cr, nil) + require.NoError(t, err) + require.Equal(t, "acme-sub-01", id) +} + +// Without externalRefId, the only subscription on the API belonging to another application yields no id, +// so Deploy takes its retryable conflict path instead of updating that subscription. +func TestRecoverSubscriptionIDAfterConflict_DoesNotAdoptAnotherApplicationsSubscription(t *testing.T) { + srv := subscriptionListServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "sub-x", "applicationId": "app-x"}}, + }) + + id, err := recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, subscriptionCR("orders-api", nil), nil) + require.NoError(t, err) + require.Empty(t, id) +} diff --git a/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go b/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go index c6e830d985..c332daa7f2 100644 --- a/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go +++ b/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go @@ -24,11 +24,13 @@ import ( "io" "net/http" "net/url" + "strings" ) // SubscriptionCreatePayload mirrors the management-API // SubscriptionCreateRequest body. type SubscriptionCreatePayload struct { + ExternalRefId *string `json:"externalRefId,omitempty"` ApiId string `json:"apiId"` SubscriptionToken string `json:"subscriptionToken"` ApplicationId *string `json:"applicationId,omitempty"` @@ -38,15 +40,22 @@ type SubscriptionCreatePayload struct { Status *string `json:"status,omitempty"` } -// SubscriptionUpdatePayload mirrors SubscriptionUpdateRequest. +// SubscriptionUpdatePayload mirrors SubscriptionUpdateRequest, a partial update +// where omitted fields are left unchanged on the gateway. type SubscriptionUpdatePayload struct { Status *string `json:"status,omitempty"` + // SubscriptionPlanId is always sent so the CR stays the source of truth: + // nil marshals as an explicit null, which removes the plan on the gateway. + SubscriptionPlanId *string `json:"subscriptionPlanId"` + // SubscriptionToken rotates the token in place when set. + SubscriptionToken *string `json:"subscriptionToken,omitempty"` } // SubscriptionResponse captures the gateway-issued fields returned from // POST/PUT /subscriptions. type SubscriptionResponse struct { Id string `json:"id"` + ExternalRefId *string `json:"externalRefId,omitempty"` ApiId string `json:"apiId"` ApplicationId *string `json:"applicationId,omitempty"` GatewayId string `json:"gatewayId"` @@ -103,12 +112,11 @@ func DeleteSubscription(ctx context.Context, gatewayEndpoint, subscriptionID str return DeleteResource(ctx, gatewayEndpoint, subscriptionsPath, subscriptionID, auth) } -// FindSubscriptionIDByAPIAndApplication lists subscriptions for the given apiID -// (and optional applicationID) and returns a single matching subscription ID -// when it can be unambiguously identified. -func FindSubscriptionIDByAPIAndApplication(ctx context.Context, gatewayEndpoint, apiID string, applicationID *string, auth AuthHeaderFunc) (string, error) { +// ListSubscriptions GETs /subscriptions?apiId=... (and applicationId when set). The gateway resolves +// apiId from either the API handle or its internal id and returns only that API's subscriptions. +func ListSubscriptions(ctx context.Context, gatewayEndpoint, apiID string, applicationID *string, auth AuthHeaderFunc) ([]SubscriptionResponse, error) { if apiID == "" { - return "", fmt.Errorf("api id is required") + return nil, fmt.Errorf("api id is required") } q := url.Values{} @@ -120,18 +128,18 @@ func FindSubscriptionIDByAPIAndApplication(ctx context.Context, gatewayEndpoint, endpoint := gatewayEndpoint + subscriptionsPath + "?" + q.Encode() req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) if err != nil { - return "", &RetryableError{Err: fmt.Errorf("create HTTP request: %w", err)} + return nil, &RetryableError{Err: fmt.Errorf("create HTTP request: %w", err)} } if auth != nil { if err := auth(ctx, req); err != nil { - return "", err + return nil, err } } client := &http.Client{Timeout: defaultProbeTimeout} resp, err := client.Do(req) if err != nil { - return "", &RetryableError{Err: fmt.Errorf("list subscriptions: %w", err)} + return nil, &RetryableError{Err: fmt.Errorf("list subscriptions: %w", err)} } defer resp.Body.Close() @@ -140,12 +148,12 @@ func FindSubscriptionIDByAPIAndApplication(ctx context.Context, gatewayEndpoint, case resp.StatusCode == http.StatusOK: // continue case IsRetryableStatusCode(resp.StatusCode): - return "", &RetryableError{ + return nil, &RetryableError{ Err: fmt.Errorf("list subscriptions returned status %d: %s", resp.StatusCode, string(body)), StatusCode: resp.StatusCode, } default: - return "", &NonRetryableError{ + return nil, &NonRetryableError{ Err: fmt.Errorf("list subscriptions returned status %d: %s", resp.StatusCode, string(body)), StatusCode: resp.StatusCode, } @@ -158,40 +166,47 @@ func FindSubscriptionIDByAPIAndApplication(ctx context.Context, gatewayEndpoint, } else { var plain []SubscriptionResponse if err := json.Unmarshal(body, &plain); err != nil { - return "", fmt.Errorf("decode subscription list response: %w", err) + return nil, fmt.Errorf("decode subscription list response: %w", err) } items = plain } - if len(items) == 0 { - return "", nil - } - if len(items) == 1 { - return items[0].Id, nil - } + return items, nil +} - if applicationID != nil && *applicationID != "" { - matches := make([]SubscriptionResponse, 0, len(items)) - for i := range items { - if items[i].ApplicationId != nil && *items[i].ApplicationId == *applicationID { - matches = append(matches, items[i]) - } - } - if len(matches) == 1 { - return matches[0].Id, nil - } - return "", nil +// FindSubscriptionIDByAPIAndApplication lists subscriptions for the given apiID +// (and optional applicationID) and returns the id of the one subscription whose +// application matches applicationID. It returns "" when no subscription or more +// than one matches, so a caller never adopts another application's subscription. +func FindSubscriptionIDByAPIAndApplication(ctx context.Context, gatewayEndpoint, apiID string, applicationID *string, auth AuthHeaderFunc) (string, error) { + items, err := ListSubscriptions(ctx, gatewayEndpoint, apiID, applicationID, auth) + if err != nil { + return "", err } - nonAppMatches := make([]SubscriptionResponse, 0, len(items)) + matchID := "" + matches := 0 for i := range items { - if items[i].ApplicationId == nil || *items[i].ApplicationId == "" { - nonAppMatches = append(nonAppMatches, items[i]) + if SameApplication(items[i].ApplicationId, applicationID) { + matches++ + matchID = items[i].Id } } - if len(nonAppMatches) == 1 { - return nonAppMatches[0].Id, nil + if matches != 1 { + return "", nil } + return matchID, nil +} + +// SameApplication reports whether two subscription application ids refer to the +// same application. An unset, empty or whitespace-only id means no application. +func SameApplication(a, b *string) bool { + return normalizedApplicationID(a) == normalizedApplicationID(b) +} - return "", nil +func normalizedApplicationID(id *string) string { + if id == nil { + return "" + } + return strings.TrimSpace(*id) } diff --git a/kubernetes/gateway-operator/internal/gatewayclient/subscription_client_test.go b/kubernetes/gateway-operator/internal/gatewayclient/subscription_client_test.go index 1b93dc5bde..c854c2c77b 100644 --- a/kubernetes/gateway-operator/internal/gatewayclient/subscription_client_test.go +++ b/kubernetes/gateway-operator/internal/gatewayclient/subscription_client_test.go @@ -86,3 +86,126 @@ func TestFindSubscriptionIDByAPIAndApplication_ConflictStatusClassification(t *t require.ErrorAs(t, err, &nr) require.Equal(t, http.StatusConflict, nr.StatusCode) } + +func TestListSubscriptions(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodGet, r.Method) + require.Equal(t, ManagementAPIBasePath+"/subscriptions", r.URL.Path) + switch r.URL.Query().Get("apiId") { + case "hello-sub-api": + _ = json.NewEncoder(w).Encode(map[string]any{ + "subscriptions": []map[string]any{ + {"id": "ext-ref-1", "externalRefId": "ext-ref-1", "apiId": "hello-sub-api"}, + {"id": "sub-2", "apiId": "hello-sub-api"}, + }, + "count": 2, + }) + case "flaky-api": + http.Error(w, "unavailable", http.StatusServiceUnavailable) + default: + _ = json.NewEncoder(w).Encode(map[string]any{"subscriptions": []map[string]any{}, "count": 0}) + } + })) + defer srv.Close() + + auth := AuthHeaderFunc(func(_ context.Context, _ *http.Request) error { return nil }) + got, err := ListSubscriptions(context.Background(), srv.URL, "hello-sub-api", nil, auth) + require.NoError(t, err) + require.Len(t, got, 2) + require.Equal(t, "ext-ref-1", got[0].Id) + require.NotNil(t, got[0].ExternalRefId) + require.Equal(t, "ext-ref-1", *got[0].ExternalRefId) + + empty, err := ListSubscriptions(context.Background(), srv.URL, "other-api", nil, auth) + require.NoError(t, err) + require.Empty(t, empty) + + _, err = ListSubscriptions(context.Background(), srv.URL, "flaky-api", nil, auth) + var retryable *RetryableError + require.ErrorAs(t, err, &retryable) + + _, err = ListSubscriptions(context.Background(), srv.URL, "", nil, auth) + require.Error(t, err) +} + +func TestUpdateSubscription_PayloadShape(t *testing.T) { + var bodies []map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPut, r.Method) + require.Equal(t, ManagementAPIBasePath+"/subscriptions/sub-1", r.URL.Path) + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + bodies = append(bodies, body) + _ = json.NewEncoder(w).Encode(map[string]any{"id": "sub-1"}) + })) + defer srv.Close() + + auth := AuthHeaderFunc(func(_ context.Context, _ *http.Request) error { return nil }) + plan, token, status := "gold", "new-token", "ACTIVE" + _, err := UpdateSubscription(context.Background(), srv.URL, "sub-1", SubscriptionUpdatePayload{ + Status: &status, SubscriptionPlanId: &plan, SubscriptionToken: &token, + }, auth) + require.NoError(t, err) + // A nil plan must be sent as an explicit null so the gateway removes it. + _, err = UpdateSubscription(context.Background(), srv.URL, "sub-1", SubscriptionUpdatePayload{}, auth) + require.NoError(t, err) + + require.Len(t, bodies, 2) + require.Equal(t, map[string]any{"status": "ACTIVE", "subscriptionPlanId": "gold", "subscriptionToken": "new-token"}, bodies[0]) + planValue, present := bodies[1]["subscriptionPlanId"] + require.True(t, present) + require.Nil(t, planValue) + require.NotContains(t, bodies[1], "status") + require.NotContains(t, bodies[1], "subscriptionToken") +} + +// A lookup must never return another application's subscription, even when it is the only one on the API. +func TestFindSubscriptionIDByAPIAndApplication_MatchesNormalizedApplicationOnly(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var items []map[string]any + switch r.URL.Query().Get("apiId") { + case "single-other-app": + items = []map[string]any{{"id": "sub-x", "applicationId": "app-x"}} + case "padded-app": + items = []map[string]any{{"id": "sub-1", "applicationId": " app-1 "}} + case "empty-app": + items = []map[string]any{{"id": "sub-1", "applicationId": ""}} + case "two-same-app": + items = []map[string]any{{"id": "sub-1", "applicationId": "app-1"}, {"id": "sub-2", "applicationId": "app-1"}} + } + _ = json.NewEncoder(w).Encode(map[string]any{"subscriptions": items}) + })) + defer srv.Close() + auth := AuthHeaderFunc(func(_ context.Context, _ *http.Request) error { return nil }) + app1, blank := "app-1", " " + + id, err := FindSubscriptionIDByAPIAndApplication(context.Background(), srv.URL, "single-other-app", nil, auth) + require.NoError(t, err) + require.Empty(t, id, "the only subscription belongs to another application") + + id, err = FindSubscriptionIDByAPIAndApplication(context.Background(), srv.URL, "single-other-app", &app1, auth) + require.NoError(t, err) + require.Empty(t, id, "an application filter the gateway did not apply must not match") + + id, err = FindSubscriptionIDByAPIAndApplication(context.Background(), srv.URL, "padded-app", &app1, auth) + require.NoError(t, err) + require.Equal(t, "sub-1", id) + + id, err = FindSubscriptionIDByAPIAndApplication(context.Background(), srv.URL, "empty-app", &blank, auth) + require.NoError(t, err) + require.Equal(t, "sub-1", id, "an empty and a blank application both mean no application") + + id, err = FindSubscriptionIDByAPIAndApplication(context.Background(), srv.URL, "two-same-app", &app1, auth) + require.NoError(t, err) + require.Empty(t, id, "two matches are ambiguous") +} + +func TestSameApplication(t *testing.T) { + s := func(v string) *string { return &v } + require.True(t, SameApplication(nil, nil)) + require.True(t, SameApplication(nil, s(""))) + require.True(t, SameApplication(s(" "), nil)) + require.True(t, SameApplication(s(" app-1"), s("app-1 "))) + require.False(t, SameApplication(s("app-1"), nil)) + require.False(t, SameApplication(s("app-1"), s("app-2"))) +} diff --git a/kubernetes/helm/operator-helm-chart/crds/gateway.api-platform.wso2.com_subscriptions.yaml b/kubernetes/helm/operator-helm-chart/crds/gateway.api-platform.wso2.com_subscriptions.yaml index 3746967daf..543336ced9 100644 --- a/kubernetes/helm/operator-helm-chart/crds/gateway.api-platform.wso2.com_subscriptions.yaml +++ b/kubernetes/helm/operator-helm-chart/crds/gateway.api-platform.wso2.com_subscriptions.yaml @@ -48,8 +48,10 @@ spec: spec: description: |- SubscriptionSpec mirrors the management-API SubscriptionCreateRequest - payload. The gateway-controller issues a UUID id which the controller - persists to .status.id for subsequent PUT/DELETE. + payload. The gateway-controller issues a UUID id (or uses externalRefId when + set) which the controller persists to .status.id for subsequent PUT/DELETE. + subscriptionToken, subscriptionPlanId and status can be changed after the + subscription is deployed; the other fields are immutable. properties: apiId: description: ApiId is the API identifier (deployment id or handle). @@ -64,6 +66,15 @@ spec: description: BillingSubscriptionId is an optional billing subscription identifier. type: string + externalRefId: + description: |- + ExternalRefId is an optional caller-supplied reference ID. When set, the + gateway uses it as the subscription id instead of generating one. + Must be unique on the gateway. Immutable once deployed. + maxLength: 64 + minLength: 1 + pattern: ^[A-Za-z0-9][A-Za-z0-9._-]*$ + type: string status: description: |- Status is the lifecycle state for this subscription. Mirrors the @@ -71,15 +82,16 @@ spec: type: string subscriptionPlanId: description: |- - SubscriptionPlanId is an optional plan UUID. When the plan is also - managed via SubscriptionPlan CR, set this to the SubscriptionPlan's - .status.id once it has been deployed. + SubscriptionPlanId is an optional plan UUID, or the name of a + SubscriptionPlan CR in the same namespace. Changing it moves the + subscription to the new plan in place; removing it removes the plan. type: string subscriptionToken: description: |- SubscriptionToken is the opaque token used to invoke the API. The token is sent as plaintext to the gateway which stores only its hash; - inline in the CR or supplied via valueFrom. + inline in the CR or supplied via valueFrom. Changing it (or the + referenced Secret data) rotates the token on the gateway in place. properties: value: description: |- @@ -240,8 +252,10 @@ spec: spec: description: |- SubscriptionSpec mirrors the management-API SubscriptionCreateRequest - payload. The gateway-controller issues a UUID id which the controller - persists to .status.id for subsequent PUT/DELETE. + payload. The gateway-controller issues a UUID id (or uses externalRefId when + set) which the controller persists to .status.id for subsequent PUT/DELETE. + subscriptionToken, subscriptionPlanId and status can be changed after the + subscription is deployed; the other fields are immutable. properties: apiId: description: ApiId is the API identifier (deployment id or handle). @@ -256,6 +270,15 @@ spec: description: BillingSubscriptionId is an optional billing subscription identifier. type: string + externalRefId: + description: |- + ExternalRefId is an optional caller-supplied reference ID. When set, the + gateway uses it as the subscription id instead of generating one. + Must be unique on the gateway. Immutable once deployed. + maxLength: 64 + minLength: 1 + pattern: ^[A-Za-z0-9][A-Za-z0-9._-]*$ + type: string status: description: |- Status is the lifecycle state for this subscription. Mirrors the @@ -263,15 +286,16 @@ spec: type: string subscriptionPlanId: description: |- - SubscriptionPlanId is an optional plan UUID. When the plan is also - managed via SubscriptionPlan CR, set this to the SubscriptionPlan's - .status.id once it has been deployed. + SubscriptionPlanId is an optional plan UUID, or the name of a + SubscriptionPlan CR in the same namespace. Changing it moves the + subscription to the new plan in place; removing it removes the plan. type: string subscriptionToken: description: |- SubscriptionToken is the opaque token used to invoke the API. The token is sent as plaintext to the gateway which stores only its hash; - inline in the CR or supplied via valueFrom. + inline in the CR or supplied via valueFrom. Changing it (or the + referenced Secret data) rotates the token on the gateway in place. properties: value: description: |- From 55f142db753f65ac237d99a5fb9f49fedf435529 Mon Sep 17 00:00:00 2001 From: thivindu Date: Fri, 30 Oct 2026 12:09:14 +0530 Subject: [PATCH 2/3] Fix review comments --- .../controller/subscription_controller.go | 127 +++++++- .../subscription_controller_test.go | 285 +++++++++++++++++- .../gatewayclient/subscription_client.go | 12 + 3 files changed, 412 insertions(+), 12 deletions(-) diff --git a/kubernetes/gateway-operator/internal/controller/subscription_controller.go b/kubernetes/gateway-operator/internal/controller/subscription_controller.go index d22f968c7b..74aa558b12 100644 --- a/kubernetes/gateway-operator/internal/controller/subscription_controller.go +++ b/kubernetes/gateway-operator/internal/controller/subscription_controller.go @@ -39,6 +39,7 @@ import ( apiv1 "github.com/wso2/api-platform/kubernetes/gateway-operator/api/v1" "github.com/wso2/api-platform/kubernetes/gateway-operator/internal/config" "github.com/wso2/api-platform/kubernetes/gateway-operator/internal/gatewayclient" + "github.com/wso2/api-platform/kubernetes/gateway-operator/internal/registry" "github.com/wso2/api-platform/kubernetes/gateway-operator/internal/secretsource" ) @@ -153,16 +154,22 @@ func (a *subscriptionAdapter) GatewaySelectionKey(obj client.Object) (string, ma func (a *subscriptionAdapter) Deploy(ctx context.Context, k8sClient client.Client, gatewayEndpoint string, obj client.Object, authFn gatewayclient.AuthHeaderFunc) (DeployResult, error) { cr := obj.(*apiv1.Subscription) + // A deployed subscription still gets its status when the token or plan can't be resolved, so the + // resolution error is held until the status has been applied. + var resolveErr error token, err := secretsource.Resolve(ctx, k8sClient, "spec.subscriptionToken", cr.Spec.SubscriptionToken, cr.Namespace) if err != nil { - return DeployResult{}, classifySecretSourceResolveError(err) + resolveErr = classifySecretSourceResolveError(err) } - resolvedPlanID, err := resolveSubscriptionPlanID(ctx, k8sClient, cr) - if err != nil { - return DeployResult{}, err + var resolvedPlanID *string + if resolveErr == nil { + resolvedPlanID, resolveErr = resolveSubscriptionPlanID(ctx, k8sClient, cr) } if cr.Status.Id == "" { + if resolveErr != nil { + return DeployResult{}, resolveErr + } payload := gatewayclient.SubscriptionCreatePayload{ ExternalRefId: cr.Spec.ExternalRefId, ApiId: cr.Spec.ApiId, @@ -182,6 +189,15 @@ func (a *subscriptionAdapter) Deploy(ctx context.Context, k8sClient client.Clien return DeployResult{}, recErr } if recoveredID != "" { + // Adopting a subscription another CR already manages would let this CR overwrite its + // token and plan, and deleting either CR would delete it for both. + owned, ownErr := subscriptionIDOwnedByAnotherCR(ctx, k8sClient, gatewayEndpoint, cr, recoveredID) + if ownErr != nil { + return DeployResult{}, ownErr + } + if owned { + return DeployResult{}, subscriptionConflictError() + } // The existing subscription may predate spec changes; bring its mutable fields in line. if err := updateSubscriptionMutableFields(ctx, gatewayEndpoint, recoveredID, cr, token, resolvedPlanID, authFn); err != nil { return DeployResult{}, err @@ -214,20 +230,108 @@ func (a *subscriptionAdapter) Deploy(ctx context.Context, k8sClient client.Clien } } } else { - // No prior fingerprint stored (e.g. pre-upgrade CR): establish a baseline. + // No prior fingerprint stored (a CR from an earlier operator version, or a lost annotation write). + // The spec may hold an edit an earlier version rejected, so it becomes the baseline only once the + // gateway subscription is confirmed to still match it. + if err := verifyDeployedSubscriptionMatchesSpec(ctx, gatewayEndpoint, cr, authFn); err != nil { + return DeployResult{}, err + } if fp, fpErr := subscriptionImmutableFingerprint(cr); fpErr == nil { _ = setSubscriptionLastAppliedImmutableFingerprint(ctx, k8sClient, cr, fp) } } + if resolveErr != nil { + // Only a lasting failure, such as a missing Secret, applies the status alone. A transient one, such + // as a SubscriptionPlan not deployed yet, is retried and then applies everything together. + var nr *gatewayclient.NonRetryableError + if errors.As(resolveErr, &nr) { + return DeployResult{}, updateSubscriptionStatusOnFailure(ctx, gatewayEndpoint, cr, resolveErr, authFn) + } + return DeployResult{}, resolveErr + } // status, subscriptionPlanId and subscriptionToken are updated in place. Sending the token on every // update also applies a rotated valueFrom Secret, since Secret drift triggers this redeploy. if err := updateSubscriptionMutableFields(ctx, gatewayEndpoint, cr.Status.Id, cr, token, resolvedPlanID, authFn); err != nil { + var nr *gatewayclient.NonRetryableError + if errors.As(err, &nr) && (nr.StatusCode == http.StatusBadRequest || nr.StatusCode == http.StatusConflict) { + // The gateway rejected the plan or the token; the status alone can still be applied. + return DeployResult{}, updateSubscriptionStatusOnFailure(ctx, gatewayEndpoint, cr, err, authFn) + } return DeployResult{}, err } return DeployResult{Id: cr.Status.Id}, nil } +// updateSubscriptionStatusOnFailure applies spec.status alone after the plan or token could not be applied, +// so a broken plan or token reference never blocks revoking or deactivating the subscription. It returns +// cause either way, so the CR still reports why the full update failed. +func updateSubscriptionStatusOnFailure(ctx context.Context, gatewayEndpoint string, cr *apiv1.Subscription, cause error, authFn gatewayclient.AuthHeaderFunc) error { + if cr.Spec.Status != nil { + _, _ = gatewayclient.UpdateSubscriptionStatus(ctx, gatewayEndpoint, cr.Status.Id, cr.Spec.Status, authFn) + } + return cause +} + +// verifyDeployedSubscriptionMatchesSpec checks that the gateway subscription in status.id belongs to +// spec.apiId and spec.applicationId, and, on gateways that report externalRefId, that it matches +// spec.externalRefId. +func verifyDeployedSubscriptionMatchesSpec(ctx context.Context, gatewayEndpoint string, cr *apiv1.Subscription, authFn gatewayclient.AuthHeaderFunc) error { + subscriptions, err := gatewayclient.ListSubscriptions(ctx, gatewayEndpoint, cr.Spec.ApiId, nil, authFn) + if err != nil { + return err + } + for i := range subscriptions { + sub := &subscriptions[i] + if sub.Id != cr.Status.Id { + continue + } + externalRefIDMatches := sub.ExternalRefId == nil || cr.Spec.ExternalRefId == nil || + *cr.Spec.ExternalRefId == "" || *cr.Spec.ExternalRefId == sub.Id + if gatewayclient.SameApplication(sub.ApplicationId, cr.Spec.ApplicationId) && externalRefIDMatches { + return nil + } + break + } + return &gatewayclient.NonRetryableError{ + Err: fmt.Errorf("the gateway subscription in status.id does not match spec.externalRefId, spec.apiId and " + + "spec.applicationId; these fields are immutable once status.id is set, so delete and recreate the " + + "Subscription to apply changes"), + } +} + +// subscriptionIDOwnedByAnotherCR reports whether another Subscription CR that targets the same gateway +// already holds id in status.id. +func subscriptionIDOwnedByAnotherCR(ctx context.Context, k8sClient client.Client, gatewayEndpoint string, cr *apiv1.Subscription, id string) (bool, error) { + var list apiv1.SubscriptionList + if err := k8sClient.List(ctx, &list); err != nil { + return false, &gatewayclient.RetryableError{Err: fmt.Errorf("list Subscriptions: %w", err)} + } + for i := range list.Items { + other := &list.Items[i] + if other.Status.Id != id || (other.Namespace == cr.Namespace && other.Name == cr.Name) { + continue + } + // Deleting a CR deletes its subscription from every gateway it matches, so any shared gateway counts. + for _, gw := range registry.GetGatewayRegistry().FindMatchingGateways(other.Namespace, other.Labels) { + if gw.GetGatewayServiceEndpoint() == gatewayEndpoint { + return true, nil + } + } + } + return false, nil +} + +// subscriptionConflictError reports a create conflict without saying whether the id or the token collided, +// so the error never confirms that a token is in use. +func subscriptionConflictError() error { + return &gatewayclient.NonRetryableError{ + StatusCode: http.StatusConflict, + Err: fmt.Errorf("the subscription conflicts with an existing subscription on the gateway; " + + "check that spec.externalRefId and spec.subscriptionToken are not used by another subscription"), + } +} + // updateSubscriptionMutableFields PUTs the CR's status, plan and token to the gateway subscription. // The plan is always sent, so a plan removed from the CR is cleared on the gateway. func updateSubscriptionMutableFields(ctx context.Context, gatewayEndpoint, subscriptionID string, cr *apiv1.Subscription, token string, planID *string, authFn gatewayclient.AuthHeaderFunc) error { @@ -244,7 +348,8 @@ func updateSubscriptionMutableFields(ctx context.Context, gatewayEndpoint, subsc // with. With spec.externalRefId the subscription id is known, so it is adopted only when that id is one of // this API's subscriptions and belongs to the CR's application; otherwise the id or the token belongs to a // different subscription, and the conflict is reported without saying which, so the error never confirms -// that a token is in use. Without +// that a token is in use. Gateways that predate externalRefId ignore it and assign their own ids, which they +// show by leaving externalRefId out of their responses; those are recovered as if it were not set. Without // spec.externalRefId the id is looked up by apiId and applicationId, and is returned only when exactly one // subscription matches; otherwise no id is returned and the create conflict is retried. func recoverSubscriptionIDAfterConflict(ctx context.Context, gatewayEndpoint string, cr *apiv1.Subscription, authFn gatewayclient.AuthHeaderFunc) (string, error) { @@ -253,16 +358,18 @@ func recoverSubscriptionIDAfterConflict(ctx context.Context, gatewayEndpoint str if err != nil { return "", err } + gatewaySupportsExternalRefID := false for i := range subscriptions { + if subscriptions[i].ExternalRefId != nil { + gatewaySupportsExternalRefID = true + } if subscriptions[i].Id == *cr.Spec.ExternalRefId && gatewayclient.SameApplication(subscriptions[i].ApplicationId, cr.Spec.ApplicationId) { return subscriptions[i].Id, nil } } - return "", &gatewayclient.NonRetryableError{ - StatusCode: http.StatusConflict, - Err: fmt.Errorf("the subscription conflicts with an existing subscription on the gateway; " + - "check that spec.externalRefId and spec.subscriptionToken are not used by another subscription"), + if len(subscriptions) == 0 || gatewaySupportsExternalRefID { + return "", subscriptionConflictError() } } return gatewayclient.FindSubscriptionIDByAPIAndApplication(ctx, gatewayEndpoint, cr.Spec.ApiId, cr.Spec.ApplicationId, authFn) diff --git a/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go b/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go index b9365d83fd..21cadafc12 100644 --- a/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go +++ b/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go @@ -21,12 +21,20 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "path" "testing" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" apiv1 "github.com/wso2/api-platform/kubernetes/gateway-operator/api/v1" "github.com/wso2/api-platform/kubernetes/gateway-operator/internal/gatewayclient" + "github.com/wso2/api-platform/kubernetes/gateway-operator/internal/registry" ) // subscriptionListServer serves GET /subscriptions?apiId=... with the subscriptions each API holds. @@ -54,7 +62,7 @@ func subscriptionCR(apiID string, externalRefID *string) *apiv1.Subscription { func TestRecoverSubscriptionIDAfterConflict_AdoptsExternalRefIdOnTheSameAPI(t *testing.T) { srv := subscriptionListServer(t, map[string][]map[string]any{ - "orders-api": {{"id": "other-sub"}, {"id": "acme-sub-01", "externalRefId": "acme-sub-01"}}, + "orders-api": {{"id": "other-sub", "externalRefId": "other-sub"}, {"id": "acme-sub-01", "externalRefId": "acme-sub-01"}}, }) ref := "acme-sub-01" @@ -68,7 +76,7 @@ func TestRecoverSubscriptionIDAfterConflict_AdoptsExternalRefIdOnTheSameAPI(t *t // collided, so it never confirms that a token is in use. func TestRecoverSubscriptionIDAfterConflict_RejectsExternalRefIdNotOnTheAPI(t *testing.T) { srv := subscriptionListServer(t, map[string][]map[string]any{ - "orders-api": {{"id": "other-sub"}}, + "orders-api": {{"id": "other-sub", "externalRefId": "other-sub"}}, "payments-api": {{"id": "acme-sub-01", "externalRefId": "acme-sub-01"}}, }) ref := "acme-sub-01" @@ -131,3 +139,276 @@ func TestRecoverSubscriptionIDAfterConflict_DoesNotAdoptAnotherApplicationsSubsc require.NoError(t, err) require.Empty(t, id) } + +// Gateways that predate externalRefId ignore it, assign their own ids and leave externalRefId out of their +// responses, so the subscription is recovered by API and application, as it was before externalRefId. +func TestRecoverSubscriptionIDAfterConflict_FallsBackOnGatewaysWithoutExternalRefId(t *testing.T) { + srv := subscriptionListServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "gateway-assigned-uuid"}}, + }) + ref := "acme-sub-01" + + id, err := recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, subscriptionCR("orders-api", &ref), nil) + require.NoError(t, err) + require.Equal(t, "gateway-assigned-uuid", id) +} + +// With no subscription on the API there is nothing to recover, whichever gateway version answered. +func TestRecoverSubscriptionIDAfterConflict_RejectsExternalRefIdWhenTheAPIHasNoSubscriptions(t *testing.T) { + srv := subscriptionListServer(t, map[string][]map[string]any{}) + ref := "acme-sub-01" + + id, err := recoverSubscriptionIDAfterConflict(context.Background(), srv.URL, subscriptionCR("orders-api", &ref), nil) + require.Empty(t, id) + var nonRetryable *gatewayclient.NonRetryableError + require.ErrorAs(t, err, &nonRetryable) + require.Equal(t, http.StatusConflict, nonRetryable.StatusCode) +} + +func TestSubscriptionIDOwnedByAnotherCR(t *testing.T) { + ctx := context.Background() + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, apiv1.AddToScheme(scheme)) + + reg := registry.GetGatewayRegistry() + shared := ®istry.GatewayInfo{ + Name: "owner-check-shared", Namespace: "gw", ServiceName: "shared", ServicePort: 9090, + APISelector: &apiv1.APISelector{Scope: apiv1.NamespacedScope, Namespaces: []string{"owner-team-a", "owner-team-b"}}, + } + other := ®istry.GatewayInfo{ + Name: "owner-check-other", Namespace: "gw", ServiceName: "other", ServicePort: 9090, + APISelector: &apiv1.APISelector{Scope: apiv1.NamespacedScope, Namespaces: []string{"owner-team-c"}}, + } + reg.Register(shared) + reg.Register(other) + t.Cleanup(func() { + reg.Unregister(shared.Namespace, shared.Name) + reg.Unregister(other.Namespace, other.Name) + }) + + deployed := func(ns, name, id string) *apiv1.Subscription { + cr := &apiv1.Subscription{ObjectMeta: metav1.ObjectMeta{Namespace: ns, Name: name}} + cr.Status.Id = id + return cr + } + c := fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(&apiv1.Subscription{}).WithObjects( + deployed("owner-team-a", "orders", "acme-sub-01"), + deployed("owner-team-c", "orders", "acme-sub-02"), + ).Build() + endpoint := shared.GetGatewayServiceEndpoint() + + owned, err := subscriptionIDOwnedByAnotherCR(ctx, c, endpoint, deployed("owner-team-b", "orders", ""), "acme-sub-01") + require.NoError(t, err) + require.True(t, owned, "a CR in another namespace on the same gateway already manages the subscription") + + owned, err = subscriptionIDOwnedByAnotherCR(ctx, c, endpoint, deployed("owner-team-a", "orders", ""), "acme-sub-01") + require.NoError(t, err) + require.False(t, owned, "the CR's own status.id is not another CR's") + + owned, err = subscriptionIDOwnedByAnotherCR(ctx, c, endpoint, deployed("owner-team-b", "orders", ""), "acme-sub-02") + require.NoError(t, err) + require.False(t, owned, "the same id on a different gateway is a different subscription") +} + +// subscriptionGatewayServer serves the subscription list and records every PUT body. putStatuses gives the +// status of successive PUTs; once exhausted, PUTs succeed. +func subscriptionGatewayServer(t *testing.T, byAPI map[string][]map[string]any, putStatuses ...int) (*httptest.Server, *[]map[string]any) { + t.Helper() + var puts []map[string]any + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet: + items := byAPI[r.URL.Query().Get("apiId")] + if items == nil { + items = []map[string]any{} + } + _ = json.NewEncoder(w).Encode(map[string]any{"subscriptions": items, "count": len(items)}) + case http.MethodPut: + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + puts = append(puts, body) + if len(puts) <= len(putStatuses) && putStatuses[len(puts)-1] != http.StatusOK { + w.WriteHeader(putStatuses[len(puts)-1]) + _, _ = w.Write([]byte(`{"status":"error","message":"rejected"}`)) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"id": path.Base(r.URL.Path)}) + default: + t.Fatalf("unexpected %s %s", r.Method, r.URL.Path) + } + })) + t.Cleanup(srv.Close) + return srv, &puts +} + +// deployedSubscription is a CR already on the gateway as acme-sub-01. With baseline set, it carries the +// immutable-field fingerprint an earlier reconcile stored. +func deployedSubscription(t *testing.T, baseline bool) *apiv1.Subscription { + t.Helper() + token, status, plan := "token-1", "REVOKED", "gold-plan-uuid" + cr := &apiv1.Subscription{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "orders"}} + cr.Spec.ApiId = "orders-api" + cr.Spec.SubscriptionToken.Value = &token + cr.Spec.Status = &status + cr.Spec.SubscriptionPlanId = &plan + cr.Status.Id = "acme-sub-01" + if baseline { + fp, err := subscriptionImmutableFingerprint(cr) + require.NoError(t, err) + cr.Annotations = map[string]string{annSubscriptionLastAppliedImmutableFingerprint: fp} + } + return cr +} + +func subscriptionTestClient(t *testing.T, objs ...client.Object) client.Client { + t.Helper() + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, apiv1.AddToScheme(scheme)) + return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() +} + +func TestSubscriptionDeploy_UpdatesStatusPlanAndTokenInOneRequest(t *testing.T) { + cr := deployedSubscription(t, true) + srv, puts := subscriptionGatewayServer(t, nil) + + res, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + require.NoError(t, err) + require.Equal(t, "acme-sub-01", res.Id) + require.Equal(t, []map[string]any{{ + "status": "REVOKED", "subscriptionPlanId": "gold-plan-uuid", "subscriptionToken": "token-1", + }}, *puts) +} + +// A plan or token the gateway rejects must not stop the status, so a subscription can still be revoked. +func TestSubscriptionDeploy_AppliesTheStatusWhenThePlanOrTokenIsRejected(t *testing.T) { + for _, code := range []int{http.StatusBadRequest, http.StatusConflict} { + cr := deployedSubscription(t, true) + srv, puts := subscriptionGatewayServer(t, nil, code) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + var nonRetryable *gatewayclient.NonRetryableError + require.ErrorAs(t, err, &nonRetryable) + require.Equal(t, code, nonRetryable.StatusCode, "the CR still reports why the full update failed") + require.Len(t, *puts, 2) + require.Equal(t, map[string]any{"status": "REVOKED"}, (*puts)[1], "only the status is sent again") + } +} + +// A missing subscription is not a plan or token problem, so no status-only update follows. +func TestSubscriptionDeploy_DoesNotRetryTheStatusWhenTheSubscriptionIsMissing(t *testing.T) { + cr := deployedSubscription(t, true) + srv, puts := subscriptionGatewayServer(t, nil, http.StatusNotFound) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + require.Error(t, err) + require.Len(t, *puts, 1) +} + +func TestSubscriptionDeploy_AppliesTheStatusWhenTheTokenSecretIsMissing(t *testing.T) { + cr := deployedSubscription(t, true) + cr.Spec.SubscriptionToken = apiv1.SecretValueSource{ValueFrom: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "missing-secret"}, Key: "token", + }} + fp, err := subscriptionImmutableFingerprint(cr) + require.NoError(t, err) + cr.Annotations[annSubscriptionLastAppliedImmutableFingerprint] = fp + srv, puts := subscriptionGatewayServer(t, nil) + + _, err = (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + require.Error(t, err) + require.Equal(t, []map[string]any{{"status": "REVOKED"}}, *puts) +} + +// A CR without a stored fingerprint, such as one from an earlier operator version, takes its spec as the +// baseline once the gateway subscription matches it. +func TestSubscriptionDeploy_SetsTheBaselineWhenTheGatewaySubscriptionMatches(t *testing.T) { + cr := deployedSubscription(t, false) + cr.Annotations = map[string]string{annSubscriptionLegacyCreateFingerprint: "legacy"} + srv, puts := subscriptionGatewayServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "acme-sub-01"}}, + }) + c := subscriptionTestClient(t, cr) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), c, srv.URL, cr, nil) + require.NoError(t, err) + require.Len(t, *puts, 1) + + var stored apiv1.Subscription + require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(cr), &stored)) + require.NotEmpty(t, stored.Annotations[annSubscriptionLastAppliedImmutableFingerprint]) + require.NotContains(t, stored.Annotations, annSubscriptionLegacyCreateFingerprint) +} + +// An edit an earlier operator version rejected, here a changed apiId, must still be rejected: the +// subscription is not on the API the spec now names, so nothing is updated and no baseline is stored. +func TestSubscriptionDeploy_RejectsASpecThatNoLongerMatchesTheGatewaySubscription(t *testing.T) { + app := "app-1" + cases := map[string]struct { + byAPI map[string][]map[string]any + mutate func(*apiv1.Subscription) + }{ + "apiId changed": { + byAPI: map[string][]map[string]any{"payments-api": {{"id": "acme-sub-01"}}}, + }, + "applicationId changed": { + byAPI: map[string][]map[string]any{"orders-api": {{"id": "acme-sub-01"}}}, + mutate: func(cr *apiv1.Subscription) { cr.Spec.ApplicationId = &app }, + }, + "externalRefId added": { + byAPI: map[string][]map[string]any{"orders-api": {{"id": "acme-sub-01", "externalRefId": "acme-sub-01"}}}, + mutate: func(cr *apiv1.Subscription) { + ref := "acme-sub-02" + cr.Spec.ExternalRefId = &ref + }, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + cr := deployedSubscription(t, false) + if tc.mutate != nil { + tc.mutate(cr) + } + srv, puts := subscriptionGatewayServer(t, tc.byAPI) + c := subscriptionTestClient(t, cr) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), c, srv.URL, cr, nil) + var nonRetryable *gatewayclient.NonRetryableError + require.ErrorAs(t, err, &nonRetryable) + require.Contains(t, err.Error(), "immutable") + require.Empty(t, *puts) + + var stored apiv1.Subscription + require.NoError(t, c.Get(context.Background(), client.ObjectKeyFromObject(cr), &stored)) + require.NotContains(t, stored.Annotations, annSubscriptionLastAppliedImmutableFingerprint) + }) + } +} + +// Gateways that predate externalRefId never report it, so a CR they deployed under a gateway-assigned id +// still matches its spec.externalRefId. +func TestSubscriptionDeploy_AcceptsAGatewayAssignedIdOnGatewaysWithoutExternalRefId(t *testing.T) { + cr := deployedSubscription(t, false) + ref := "acme-sub-01-ref" + cr.Spec.ExternalRefId = &ref + srv, puts := subscriptionGatewayServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "acme-sub-01"}}, + }) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + require.NoError(t, err) + require.Len(t, *puts, 1) +} + +// A plan that is still being deployed is waited for, as before, and the status is then sent with it. +func TestSubscriptionDeploy_WaitsForAPlanBeingDeployedBeforeSendingTheStatus(t *testing.T) { + cr := deployedSubscription(t, true) + plan := &apiv1.SubscriptionPlan{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "gold-plan-uuid"}} + srv, puts := subscriptionGatewayServer(t, nil) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr, plan), srv.URL, cr, nil) + var retryable *gatewayclient.RetryableError + require.ErrorAs(t, err, &retryable) + require.Empty(t, *puts) +} diff --git a/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go b/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go index c332daa7f2..ff6bb66e4e 100644 --- a/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go +++ b/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go @@ -86,6 +86,18 @@ func CreateSubscription(ctx context.Context, gatewayEndpoint string, payload Sub // UpdateSubscription PUTs a SubscriptionUpdateRequest payload to // /subscriptions/{subscriptionId}. func UpdateSubscription(ctx context.Context, gatewayEndpoint, subscriptionID string, payload SubscriptionUpdatePayload, auth AuthHeaderFunc) (*SubscriptionResponse, error) { + return putSubscription(ctx, gatewayEndpoint, subscriptionID, payload, auth) +} + +// UpdateSubscriptionStatus PUTs only the status, leaving the plan and token unchanged on the gateway. +func UpdateSubscriptionStatus(ctx context.Context, gatewayEndpoint, subscriptionID string, status *string, auth AuthHeaderFunc) (*SubscriptionResponse, error) { + payload := struct { + Status *string `json:"status,omitempty"` + }{Status: status} + return putSubscription(ctx, gatewayEndpoint, subscriptionID, payload, auth) +} + +func putSubscription(ctx context.Context, gatewayEndpoint, subscriptionID string, payload any, auth AuthHeaderFunc) (*SubscriptionResponse, error) { if subscriptionID == "" { return nil, fmt.Errorf("subscription id is required") } From b5d958639f84a81e10aa2c5e0145dba208de3c77 Mon Sep 17 00:00:00 2001 From: thivindu Date: Fri, 30 Oct 2026 12:33:40 +0530 Subject: [PATCH 3/3] Verify subscription updates and keep the status fallback retryable - Retry the reconcile when the status-only fallback fails transiently, keeping the original error in the message; a successful or permanent failure still reports the original error. - Apply the status-only fallback to a subscription adopted after a create conflict too, through one applySubscriptionUpdate helper for both paths. - Fail the update when the gateway reports a different plan than the one sent, so a gateway that ignores in-place plan updates doesn't leave the CR Programmed. - Compare billingCustomerId and billingSubscriptionId before recording the baseline for a CR without a stored fingerprint. - Add tests for each, and for an immutable-field edit after the baseline. Co-Authored-By: Claude Opus 5.5 --- .../controller/subscription_controller.go | 80 ++++++++++---- .../subscription_controller_test.go | 104 +++++++++++++++++- .../gatewayclient/subscription_client.go | 13 ++- 3 files changed, 169 insertions(+), 28 deletions(-) diff --git a/kubernetes/gateway-operator/internal/controller/subscription_controller.go b/kubernetes/gateway-operator/internal/controller/subscription_controller.go index 74aa558b12..2617170f3d 100644 --- a/kubernetes/gateway-operator/internal/controller/subscription_controller.go +++ b/kubernetes/gateway-operator/internal/controller/subscription_controller.go @@ -24,6 +24,7 @@ import ( "errors" "fmt" "net/http" + "strings" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -199,7 +200,7 @@ func (a *subscriptionAdapter) Deploy(ctx context.Context, k8sClient client.Clien return DeployResult{}, subscriptionConflictError() } // The existing subscription may predate spec changes; bring its mutable fields in line. - if err := updateSubscriptionMutableFields(ctx, gatewayEndpoint, recoveredID, cr, token, resolvedPlanID, authFn); err != nil { + if err := applySubscriptionUpdate(ctx, gatewayEndpoint, recoveredID, cr, token, resolvedPlanID, authFn); err != nil { return DeployResult{}, err } if fp, fpErr := subscriptionImmutableFingerprint(cr); fpErr == nil { @@ -246,36 +247,51 @@ func (a *subscriptionAdapter) Deploy(ctx context.Context, k8sClient client.Clien // as a SubscriptionPlan not deployed yet, is retried and then applies everything together. var nr *gatewayclient.NonRetryableError if errors.As(resolveErr, &nr) { - return DeployResult{}, updateSubscriptionStatusOnFailure(ctx, gatewayEndpoint, cr, resolveErr, authFn) + return DeployResult{}, updateSubscriptionStatusOnFailure(ctx, gatewayEndpoint, cr.Status.Id, cr, resolveErr, authFn) } return DeployResult{}, resolveErr } // status, subscriptionPlanId and subscriptionToken are updated in place. Sending the token on every // update also applies a rotated valueFrom Secret, since Secret drift triggers this redeploy. - if err := updateSubscriptionMutableFields(ctx, gatewayEndpoint, cr.Status.Id, cr, token, resolvedPlanID, authFn); err != nil { - var nr *gatewayclient.NonRetryableError - if errors.As(err, &nr) && (nr.StatusCode == http.StatusBadRequest || nr.StatusCode == http.StatusConflict) { - // The gateway rejected the plan or the token; the status alone can still be applied. - return DeployResult{}, updateSubscriptionStatusOnFailure(ctx, gatewayEndpoint, cr, err, authFn) - } + if err := applySubscriptionUpdate(ctx, gatewayEndpoint, cr.Status.Id, cr, token, resolvedPlanID, authFn); err != nil { return DeployResult{}, err } return DeployResult{Id: cr.Status.Id}, nil } +// applySubscriptionUpdate PUTs the CR's status, plan and token to the subscription. When the gateway rejects +// the plan or the token, the status alone is still applied. +func applySubscriptionUpdate(ctx context.Context, gatewayEndpoint, subscriptionID string, cr *apiv1.Subscription, token string, planID *string, authFn gatewayclient.AuthHeaderFunc) error { + err := updateSubscriptionMutableFields(ctx, gatewayEndpoint, subscriptionID, cr, token, planID, authFn) + var nr *gatewayclient.NonRetryableError + if errors.As(err, &nr) && (nr.StatusCode == http.StatusBadRequest || nr.StatusCode == http.StatusConflict) { + return updateSubscriptionStatusOnFailure(ctx, gatewayEndpoint, subscriptionID, cr, err, authFn) + } + return err +} + // updateSubscriptionStatusOnFailure applies spec.status alone after the plan or token could not be applied, // so a broken plan or token reference never blocks revoking or deactivating the subscription. It returns -// cause either way, so the CR still reports why the full update failed. -func updateSubscriptionStatusOnFailure(ctx context.Context, gatewayEndpoint string, cr *apiv1.Subscription, cause error, authFn gatewayclient.AuthHeaderFunc) error { - if cr.Spec.Status != nil { - _, _ = gatewayclient.UpdateSubscriptionStatus(ctx, gatewayEndpoint, cr.Status.Id, cr.Spec.Status, authFn) +// cause, so the CR still reports why the full update failed, unless the status update fails transiently: +// then a retryable error is returned so the status is applied on a later attempt. +func updateSubscriptionStatusOnFailure(ctx context.Context, gatewayEndpoint, subscriptionID string, cr *apiv1.Subscription, cause error, authFn gatewayclient.AuthHeaderFunc) error { + if cr.Spec.Status == nil { + return cause + } + _, err := gatewayclient.UpdateSubscriptionStatus(ctx, gatewayEndpoint, subscriptionID, cr.Spec.Status, authFn) + var retryable *gatewayclient.RetryableError + if errors.As(err, &retryable) { + return &gatewayclient.RetryableError{ + StatusCode: retryable.StatusCode, + Err: fmt.Errorf("applying spec.status after the update failed (%v): %w", cause, err), + } } return cause } // verifyDeployedSubscriptionMatchesSpec checks that the gateway subscription in status.id belongs to -// spec.apiId and spec.applicationId, and, on gateways that report externalRefId, that it matches -// spec.externalRefId. +// spec.apiId and spec.applicationId, has spec.billingCustomerId and spec.billingSubscriptionId, and, on +// gateways that report externalRefId, matches spec.externalRefId. func verifyDeployedSubscriptionMatchesSpec(ctx context.Context, gatewayEndpoint string, cr *apiv1.Subscription, authFn gatewayclient.AuthHeaderFunc) error { subscriptions, err := gatewayclient.ListSubscriptions(ctx, gatewayEndpoint, cr.Spec.ApiId, nil, authFn) if err != nil { @@ -288,15 +304,17 @@ func verifyDeployedSubscriptionMatchesSpec(ctx context.Context, gatewayEndpoint } externalRefIDMatches := sub.ExternalRefId == nil || cr.Spec.ExternalRefId == nil || *cr.Spec.ExternalRefId == "" || *cr.Spec.ExternalRefId == sub.Id - if gatewayclient.SameApplication(sub.ApplicationId, cr.Spec.ApplicationId) && externalRefIDMatches { + if gatewayclient.SameApplication(sub.ApplicationId, cr.Spec.ApplicationId) && externalRefIDMatches && + sameOptionalValue(sub.BillingCustomerId, cr.Spec.BillingCustomerId) && + sameOptionalValue(sub.BillingSubscriptionId, cr.Spec.BillingSubscriptionId) { return nil } break } return &gatewayclient.NonRetryableError{ - Err: fmt.Errorf("the gateway subscription in status.id does not match spec.externalRefId, spec.apiId and " + - "spec.applicationId; these fields are immutable once status.id is set, so delete and recreate the " + - "Subscription to apply changes"), + Err: fmt.Errorf("the gateway subscription in status.id does not match spec.externalRefId, spec.apiId, " + + "spec.applicationId, spec.billingCustomerId and spec.billingSubscriptionId; these fields are immutable " + + "once status.id is set, so delete and recreate the Subscription to apply changes"), } } @@ -340,8 +358,30 @@ func updateSubscriptionMutableFields(ctx context.Context, gatewayEndpoint, subsc SubscriptionPlanId: planID, SubscriptionToken: &token, } - _, err := gatewayclient.UpdateSubscription(ctx, gatewayEndpoint, subscriptionID, payload, authFn) - return err + resp, err := gatewayclient.UpdateSubscription(ctx, gatewayEndpoint, subscriptionID, payload, authFn) + if err != nil { + return err + } + // Gateways without in-place plan updates accept the request but keep the old plan, so the plan they + // report back is checked before the update counts as applied. + if !sameOptionalValue(resp.SubscriptionPlanId, planID) { + return &gatewayclient.NonRetryableError{ + Err: fmt.Errorf("the gateway did not apply spec.subscriptionPlanId; this gateway can't change a " + + "subscription's plan in place, so delete and recreate the Subscription to change it"), + } + } + return nil +} + +// sameOptionalValue compares two optional values, treating unset, empty and blank as the same. +func sameOptionalValue(a, b *string) bool { + trimmed := func(v *string) string { + if v == nil { + return "" + } + return strings.TrimSpace(*v) + } + return trimmed(a) == trimmed(b) } // recoverSubscriptionIDAfterConflict finds the gateway subscription that a conflicting create collided diff --git a/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go b/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go index 21cadafc12..b324b1b2e3 100644 --- a/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go +++ b/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go @@ -211,8 +211,9 @@ func TestSubscriptionIDOwnedByAnotherCR(t *testing.T) { require.False(t, owned, "the same id on a different gateway is a different subscription") } -// subscriptionGatewayServer serves the subscription list and records every PUT body. putStatuses gives the -// status of successive PUTs; once exhausted, PUTs succeed. +// subscriptionGatewayServer serves the subscription list, answers every create with 409 and records every +// PUT body. putStatuses gives the status of successive PUTs; once exhausted, PUTs succeed and report the +// plan they were sent, as a gateway with in-place plan updates does. func subscriptionGatewayServer(t *testing.T, byAPI map[string][]map[string]any, putStatuses ...int) (*httptest.Server, *[]map[string]any) { t.Helper() var puts []map[string]any @@ -233,7 +234,15 @@ func subscriptionGatewayServer(t *testing.T, byAPI map[string][]map[string]any, _, _ = w.Write([]byte(`{"status":"error","message":"rejected"}`)) return } - _ = json.NewEncoder(w).Encode(map[string]any{"id": path.Base(r.URL.Path)}) + id := path.Base(r.URL.Path) + resp := map[string]any{"id": id, "externalRefId": id} + if plan, ok := body["subscriptionPlanId"]; ok && plan != nil { + resp["subscriptionPlanId"] = plan + } + _ = json.NewEncoder(w).Encode(resp) + case http.MethodPost: + w.WriteHeader(http.StatusConflict) + _, _ = w.Write([]byte(`{"status":"error","message":"conflict"}`)) default: t.Fatalf("unexpected %s %s", r.Method, r.URL.Path) } @@ -352,6 +361,20 @@ func TestSubscriptionDeploy_RejectsASpecThatNoLongerMatchesTheGatewaySubscriptio "apiId changed": { byAPI: map[string][]map[string]any{"payments-api": {{"id": "acme-sub-01"}}}, }, + "billingCustomerId changed": { + byAPI: map[string][]map[string]any{"orders-api": {{"id": "acme-sub-01", "billingCustomerId": "cus_1"}}}, + mutate: func(cr *apiv1.Subscription) { + customer := "cus_2" + cr.Spec.BillingCustomerId = &customer + }, + }, + "billingSubscriptionId added": { + byAPI: map[string][]map[string]any{"orders-api": {{"id": "acme-sub-01"}}}, + mutate: func(cr *apiv1.Subscription) { + billing := "sub_1" + cr.Spec.BillingSubscriptionId = &billing + }, + }, "applicationId changed": { byAPI: map[string][]map[string]any{"orders-api": {{"id": "acme-sub-01"}}}, mutate: func(cr *apiv1.Subscription) { cr.Spec.ApplicationId = &app }, @@ -412,3 +435,78 @@ func TestSubscriptionDeploy_WaitsForAPlanBeingDeployedBeforeSendingTheStatus(t * require.ErrorAs(t, err, &retryable) require.Empty(t, *puts) } + +// Once a fingerprint is stored, an immutable-field edit is rejected from it, without asking the gateway. +func TestSubscriptionDeploy_RejectsAnImmutableFieldEditAfterTheBaseline(t *testing.T) { + cr := deployedSubscription(t, true) + cr.Spec.ApiId = "payments-api" + srv, puts := subscriptionGatewayServer(t, nil) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + var nonRetryable *gatewayclient.NonRetryableError + require.ErrorAs(t, err, &nonRetryable) + require.Contains(t, err.Error(), "immutable") + require.Empty(t, *puts) +} + +// A CR with matching billing ids on the gateway gets its baseline as usual. +func TestSubscriptionDeploy_SetsTheBaselineWhenTheBillingIdsMatch(t *testing.T) { + cr := deployedSubscription(t, false) + customer, billing := "cus_1", "sub_1" + cr.Spec.BillingCustomerId = &customer + cr.Spec.BillingSubscriptionId = &billing + srv, puts := subscriptionGatewayServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "acme-sub-01", "billingCustomerId": "cus_1", "billingSubscriptionId": "sub_1"}}, + }) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + require.NoError(t, err) + require.Len(t, *puts, 1) +} + +// When the status-only fallback fails transiently, the error is retryable so the status is applied later, +// and it still says why the full update failed. +func TestSubscriptionDeploy_RetriesWhenTheStatusFallbackFailsTransiently(t *testing.T) { + cr := deployedSubscription(t, true) + srv, puts := subscriptionGatewayServer(t, nil, http.StatusBadRequest, http.StatusServiceUnavailable) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + var retryable *gatewayclient.RetryableError + require.ErrorAs(t, err, &retryable) + require.Contains(t, err.Error(), "400") + require.Len(t, *puts, 2) +} + +// A subscription adopted after a create conflict also gets its status when its plan or token is rejected. +func TestSubscriptionDeploy_AppliesTheStatusToARecoveredSubscriptionWhenThePlanOrTokenIsRejected(t *testing.T) { + cr := deployedSubscription(t, false) + cr.Status.Id = "" + ref := "acme-sub-01" + cr.Spec.ExternalRefId = &ref + srv, puts := subscriptionGatewayServer(t, map[string][]map[string]any{ + "orders-api": {{"id": "acme-sub-01", "externalRefId": "acme-sub-01"}}, + }, http.StatusConflict) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + var nonRetryable *gatewayclient.NonRetryableError + require.ErrorAs(t, err, &nonRetryable) + require.Equal(t, http.StatusConflict, nonRetryable.StatusCode) + require.Len(t, *puts, 2) + require.Equal(t, map[string]any{"status": "REVOKED"}, (*puts)[1]) +} + +// A gateway that accepts the update but keeps the old plan has not applied it, so the update fails instead +// of being reported as programmed. +func TestSubscriptionDeploy_FailsWhenTheGatewayKeepsTheOldPlan(t *testing.T) { + cr := deployedSubscription(t, true) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPut, r.Method) + _ = json.NewEncoder(w).Encode(map[string]any{"id": "acme-sub-01", "subscriptionPlanId": "silver-plan-uuid"}) + })) + t.Cleanup(srv.Close) + + _, err := (&subscriptionAdapter{}).Deploy(context.Background(), subscriptionTestClient(t, cr), srv.URL, cr, nil) + var nonRetryable *gatewayclient.NonRetryableError + require.ErrorAs(t, err, &nonRetryable) + require.Contains(t, err.Error(), "did not apply spec.subscriptionPlanId") +} diff --git a/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go b/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go index ff6bb66e4e..be8dcd54b0 100644 --- a/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go +++ b/kubernetes/gateway-operator/internal/gatewayclient/subscription_client.go @@ -54,11 +54,14 @@ type SubscriptionUpdatePayload struct { // SubscriptionResponse captures the gateway-issued fields returned from // POST/PUT /subscriptions. type SubscriptionResponse struct { - Id string `json:"id"` - ExternalRefId *string `json:"externalRefId,omitempty"` - ApiId string `json:"apiId"` - ApplicationId *string `json:"applicationId,omitempty"` - GatewayId string `json:"gatewayId"` + Id string `json:"id"` + ExternalRefId *string `json:"externalRefId,omitempty"` + ApiId string `json:"apiId"` + ApplicationId *string `json:"applicationId,omitempty"` + SubscriptionPlanId *string `json:"subscriptionPlanId,omitempty"` + BillingCustomerId *string `json:"billingCustomerId,omitempty"` + BillingSubscriptionId *string `json:"billingSubscriptionId,omitempty"` + GatewayId string `json:"gatewayId"` } type subscriptionListResponse struct {