fix(subscription): do not panic when validating a subscription to an API without context (#1781)

(cherry picked from commit 43db68d)

# Conflicts:
#	internal/admission/subscription/validate.go

Author: paulatulis

internal/admission/subscription/validate.go

@@ -137,7 +137,7 @@ func validateCreate(ctx context.Context, obj runtime.Object) *errors.AdmissionEr
 		return errs
 	}
 
-	errs.Add(validateContextRefs(api, app))
+	errs.Add(ValidateContextRefs(api, app))
 	if errs.IsSevere() {
 		return errs
 	}
@@ -258,12 +258,22 @@ func validateApiKind(sub core.SubscriptionObject) *errors.AdmissionError {
 	return nil
 }
 
-func validateContextRefs(api core.ApiDefinitionObject, app core.ApplicationObject) *errors.AdmissionError {
-	apiCtx, appCtx := api.ContextRef(), app.ContextRef()
+func ValidateContextRefs(api core.ApiDefinitionObject, app core.ApplicationObject) *errors.AdmissionError {
+	if !api.HasContext() {
+		return errors.NewSeveref(
+			"unable to subscribe to API [%s] because it does not reference a management context",
+			api.GetRef(),
+		)
+	}
+
+	if !app.HasContext() {
+		return errors.NewSeveref(
+			"unable to subscribe from application [%s] because it does not reference a management context",
+			app.GetRef(),
+		)
+	}
 
-	mismatch := appCtx.GetName() != apiCtx.GetName()
-	mismatch = mismatch || appCtx.GetNamespace() != apiCtx.GetNamespace()
-	if mismatch {
+	if !sameContextRef(api.ContextRef(), app.ContextRef()) {
 		return errors.NewSeveref(
 			"management contexts must match between application [%s] and API [%s], got [%v] and [%v]",
 			app.GetRef(),
@@ -272,9 +282,16 @@ func validateContextRefs(api core.ApiDefinitionObject, app core.ApplicationObjec
 			api.ContextRef(),
 		)
 	}
+
 	return nil
 }
 
+// sameContextRef compares two management context references. Both are expected to be
+// non nil, which callers guarantee by checking HasContext beforehand.
+func sameContextRef(left core.ObjectRef, right core.ObjectRef) bool {
+	return left.GetName() == right.GetName() && left.GetNamespace() == right.GetNamespace()
+}
+
 func validateEndingAt(endingAt *string) *errors.AdmissionError {
 	if endingAt != nil {
 		t, err := time.Parse(time.RFC3339, *endingAt)

test/integration/admission/subscription/create_withContext_and_apiWithoutContext_test.go

@@ -0,0 +1,73 @@
+// Copyright (C) 2015 The Gravitee team (http://gravitee.io)
+//
+// 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 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 subscription
+
+import (
+	"context"
+
+	adm "github.com/gravitee-io/gravitee-kubernetes-operator/internal/admission/subscription"
+	"github.com/gravitee-io/gravitee-kubernetes-operator/internal/errors"
+	"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/assert"
+	"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/constants"
+	"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/fixture"
+	"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/labels"
+	"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/random"
+
+	. "github.com/onsi/ginkgo/v2"
+
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Validate create", labels.WithContext, func() {
+	ctx := context.Background()
+	admissionCtrl := adm.AdmissionCtrl{}
+
+	fixtures := fixture.
+		Builder().
+		WithAPIv4(constants.ApiV4WithJWTPlanFile).
+		WithApplication(constants.ApplicationWithClientIDFile).
+		WithSubscription(constants.SubscriptionFile).
+		WithContext(constants.ContextWithCredentialsFile).
+		Build()
+
+	clientID := random.GetName()
+	fixtures.Application.Spec.Settings.App.ClientID = &clientID
+
+	// An API is not required to reference a management context, while an application is.
+	fixtures.APIv4.Spec.Context = nil
+
+	// The subscription is submitted to the admission controller directly and never applied:
+	// the reconciler cannot build an APIM client for an API without a contextRef, so the
+	// resource would never reach a terminal state.
+	subscription := fixtures.Subscription
+	fixtures.Subscription = nil
+
+	fixtures.Apply()
+
+	It("should reject the subscription if the API has no context", func() {
+		Eventually(func() error {
+			Expect(admissionCtrl.Default(ctx, subscription)).ToNot(HaveOccurred())
+			_, err := admissionCtrl.ValidateCreate(ctx, subscription)
+			return assert.Equals(
+				"error",
+				errors.NewSeveref(
+					"unable to subscribe to API [%s] because it does not reference a management context",
+					fixtures.APIv4.GetRef(),
+				),
+				err,
+			)
+		}, constants.EventualTimeout, constants.Interval).Should(Succeed())
+	})
+})

test/unit/admission/subscription/suite_test.go

@@ -0,0 +1,28 @@
+// Copyright (C) 2015 The Gravitee team (http://gravitee.io)
+//
+// 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 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 subscription
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	//+kubebuilder:scaffold:imports
+)
+
+func TestSubscriptionAdmission(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Subscription admission hook tests suite")
+}

test/unit/admission/subscription/validate_context_refs.go

@@ -0,0 +1,58 @@
+// Copyright (C) 2015 The Gravitee team (http://gravitee.io)
+//
+// 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 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 subscription
+
+import (
+	"github.com/gravitee-io/gravitee-kubernetes-operator/api/model/refs"
+	"github.com/gravitee-io/gravitee-kubernetes-operator/api/v1alpha1"
+	"github.com/gravitee-io/gravitee-kubernetes-operator/internal/admission/subscription"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func contextRef(namespace, name string) *refs.NamespacedName {
+	return &refs.NamespacedName{Namespace: namespace, Name: name}
+}
+
+// A contextRef is optional on API definitions, so it reaches validation as an interface
+// wrapping a nil pointer. Every combination must yield a verdict rather than a panic.
+var _ = Describe("Validating context refs", func() {
+	DescribeTable("subscribing an application to an API",
+		func(apiContext, appContext *refs.NamespacedName, expectRejection bool) {
+			api := &v1alpha1.ApiV4Definition{Spec: v1alpha1.ApiV4DefinitionSpec{Context: apiContext}}
+			app := &v1alpha1.Application{Spec: v1alpha1.ApplicationSpec{Context: appContext}}
+
+			err := subscription.ValidateContextRefs(api, app)
+
+			if expectRejection {
+				Expect(err).ToNot(BeNil())
+				return
+			}
+			Expect(err).To(BeNil())
+		},
+		Entry("is accepted when both reference the same context",
+			contextRef("default", "dev-ctx"), contextRef("default", "dev-ctx"), false),
+		Entry("is rejected when the context names differ",
+			contextRef("default", "prod-ctx"), contextRef("default", "dev-ctx"), true),
+		Entry("is rejected when the context namespaces differ",
+			contextRef("other", "dev-ctx"), contextRef("default", "dev-ctx"), true),
+		Entry("is rejected when the API has no context",
+			nil, contextRef("default", "dev-ctx"), true),
+		Entry("is rejected when the application has no context",
+			contextRef("default", "dev-ctx"), nil, true),
+		Entry("is rejected when neither has a context",
+			nil, nil, true),
+	)
+})