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 1a6576b9fe..17c018df54 100644 --- a/kubernetes/gateway-operator/api/v1/zz_generated.deepcopy.go +++ b/kubernetes/gateway-operator/api/v1/zz_generated.deepcopy.go @@ -2618,6 +2618,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..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" @@ -39,18 +40,22 @@ 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" ) 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 +155,24 @@ 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 { + resolveErr = classifySecretSourceResolveError(err) + } + var resolvedPlanID *string + if resolveErr == nil { + resolvedPlanID, resolveErr = resolveSubscriptionPlanID(ctx, k8sClient, cr) + } + 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 + if resolveErr != nil { + return DeployResult{}, resolveErr } payload := gatewayclient.SubscriptionCreatePayload{ + ExternalRefId: cr.Spec.ExternalRefId, ApiId: cr.Spec.ApiId, SubscriptionToken: token, ApplicationId: cr.Spec.ApplicationId, @@ -172,13 +185,26 @@ 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) + // 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 := applySubscriptionUpdate(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 +215,212 @@ 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) + // 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) } } - // Secret-backed token: management PUT only supports status, not token rotation. - fpNow, err := subscriptionTokenSecretBindingFingerprint(ctx, k8sClient, cr) - if err != nil { + 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.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 := applySubscriptionUpdate(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 +} + +// 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, 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, 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 { + 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 && + 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, " + + "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"), + } +} + +// 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 +} - 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 +// 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 { + payload := gatewayclient.SubscriptionUpdatePayload{ + Status: cr.Spec.Status, + SubscriptionPlanId: planID, + SubscriptionToken: &token, + } + 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 DeployResult{Id: cr.Status.Id}, nil + 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 +// 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. 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) { + if cr.Spec.ExternalRefId != nil && *cr.Spec.ExternalRefId != "" { + subscriptions, err := gatewayclient.ListSubscriptions(ctx, gatewayEndpoint, cr.Spec.ApiId, nil, authFn) + 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 + } + } + if len(subscriptions) == 0 || gatewaySupportsExternalRefID { + return "", subscriptionConflictError() + } + } + 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 +441,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 +466,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..b324b1b2e3 --- /dev/null +++ b/kubernetes/gateway-operator/internal/controller/subscription_controller_test.go @@ -0,0 +1,512 @@ +/* +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" + "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. +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", "externalRefId": "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", "externalRefId": "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) +} + +// 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, 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 + 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 + } + 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) + } + })) + 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"}}}, + }, + "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 }, + }, + "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) +} + +// 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 c6e830d985..be8dcd54b0 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,18 +40,28 @@ 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"` - 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 { @@ -77,6 +89,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") } @@ -103,12 +127,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 +143,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 +163,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 +181,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 +} - return "", 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) +} + +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: |-