diff --git a/controllers/authorization/authorization_controller_test.go b/controllers/authorization/authorization_controller_test.go index d59f30c..39759dc 100644 --- a/controllers/authorization/authorization_controller_test.go +++ b/controllers/authorization/authorization_controller_test.go @@ -10,25 +10,38 @@ import ( . "github.com/onsi/gomega" "github.com/opendatahub-io/odh-platform/pkg/metadata" "github.com/opendatahub-io/odh-platform/test" + "github.com/opendatahub-io/odh-platform/test/matchers" "istio.io/api/security/v1beta1" istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" utilrand "k8s.io/apimachinery/pkg/util/rand" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) +const watchedCR = ` +apiVersion: opendatahub.io/v1 +kind: Component +metadata: + name: %[1]s + namespace: %[2]s +spec: + name: %[1]s + host: example.com +` + var _ = Describe("Checking Authorization Resource Creation", test.EnvTest(), func() { var ( resourceName string testNamespaceName string testNamespace *corev1.Namespace - createdCfgMap *corev1.ConfigMap + createdComponent *unstructured.Unstructured ) BeforeEach(func(ctx context.Context) { - resourceName = "test-configmap" + resourceName = "test-component" base := "test-namespace" testNamespaceName = fmt.Sprintf("%s%s", base, utilrand.String(7)) @@ -46,34 +59,15 @@ var _ = Describe("Checking Authorization Resource Creation", test.EnvTest(), fun }) Expect(err).ToNot(HaveOccurred()) - testConfigMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: resourceName, - Namespace: testNamespaceName, - }, - Data: map[string]string{ - "host": "example.com", - }, - } - _, err = controllerutil.CreateOrUpdate(ctx, envTest.Client, testConfigMap, func() error { - return nil - }) - Expect(err).ToNot(HaveOccurred()) - - createdCfgMap = &corev1.ConfigMap{} - err = envTest.Client.Get(ctx, types.NamespacedName{ - Name: resourceName, - Namespace: testNamespaceName, - }, createdCfgMap) - Expect(err).NotTo(HaveOccurred()) + var errCreate error + createdComponent, errCreate = test.CreateOrUpdateResource(ctx, envTest.Client, componentResource(resourceName, testNamespaceName)) + Expect(errCreate).ToNot(HaveOccurred()) }) AfterEach(func() { - envTest.DeleteAll(createdCfgMap, testNamespace) + envTest.DeleteAll(createdComponent, testNamespace) }) - // TODO: rename other references to cfgmap to target CR (similar) - // TODO: test reconcile/update on anonymous vs rules defined (verify both cases) It("should create an anonymous AuthConfig resource by default when a target CR is created", func(ctx context.Context) { Eventually(func() error { createdAuthConfig := &authorinov1beta2.AuthConfig{} @@ -86,29 +80,26 @@ var _ = Describe("Checking Authorization Resource Creation", test.EnvTest(), fun return err } - Expect(createdAuthConfig.ObjectMeta.OwnerReferences).To(HaveLen(1)) - ownerRef := createdAuthConfig.ObjectMeta.OwnerReferences[0] - checkOwnerRef(ownerRef, *createdCfgMap) - - Expect(createdAuthConfig.Spec.Hosts).To(ContainElement("example.com")) + Expect(createdAuthConfig).To(matchers.HaveHosts("example.com")) Expect(createdAuthConfig.Labels).To(HaveKeyWithValue("security.opendatahub.io/authorization-group", "default")) - // Check for non-anonymous authentication - authMethod := createdAuthConfig.Spec.Authentication - Expect(authMethod).To(HaveKey("anonymous-access")) - Expect(authMethod).NotTo(HaveKey("kubernetes-user")) + Expect(createdAuthConfig).To(matchers.HaveAuthenticationMethod("anonymous-access")) + Expect(createdAuthConfig).NotTo(matchers.HaveAuthenticationMethod("kubernetes-user")) + Expect(createdAuthConfig).NotTo(matchers.HaveKubernetesTokenReview()) return nil }, 10*time.Second, 2*time.Second).Should(Succeed()) }) It("should create a non-anonymous AuthConfig resource when annotation is specified", func(ctx context.Context) { - if createdCfgMap.Annotations == nil { - createdCfgMap.Annotations = map[string]string{} + if createdComponent.GetAnnotations() == nil { + createdComponent.SetAnnotations(map[string]string{}) } - createdCfgMap.Annotations[metadata.Annotations.AuthEnabled] = "true" - Expect(envTest.Client.Update(ctx, createdCfgMap)).To(Succeed()) + annotations := createdComponent.GetAnnotations() + annotations[metadata.Annotations.AuthEnabled] = "true" + createdComponent.SetAnnotations(annotations) + Expect(envTest.Client.Update(ctx, createdComponent)).To(Succeed()) Eventually(func() error { createdAuthConfig := &authorinov1beta2.AuthConfig{} @@ -121,24 +112,18 @@ var _ = Describe("Checking Authorization Resource Creation", test.EnvTest(), fun return err } - Expect(createdAuthConfig.Spec.Hosts).To(ContainElement("example.com")) + Expect(createdAuthConfig).To(matchers.HaveHosts("example.com")) Expect(createdAuthConfig.Labels).To(HaveKeyWithValue("security.opendatahub.io/authorization-group", "default")) - // Check for non-anonymous authentication - authMethod := createdAuthConfig.Spec.Authentication - Expect(authMethod).To(HaveKey("kubernetes-user")) - Expect(authMethod).NotTo(HaveKey("anonymous-access")) - - // Verify Kubernetes Token Review is configured - kubernetesTokenReview := authMethod["kubernetes-user"].KubernetesTokenReview - Expect(kubernetesTokenReview).NotTo(BeNil()) + Expect(createdAuthConfig).To(matchers.HaveAuthenticationMethod("kubernetes-user")) + Expect(createdAuthConfig).NotTo(matchers.HaveAuthenticationMethod("anonymous-access")) + Expect(createdAuthConfig).To(matchers.HaveKubernetesTokenReview()) return nil }, 10*time.Second, 2*time.Second).Should(Succeed()) }) - // Custom matchers (gomega) - It("should create an AuthorizationPolicy when a ConfigMap is created", func(ctx context.Context) { + It("should create an AuthorizationPolicy when a Component is created", func(ctx context.Context) { Eventually(func() error { createdAuthPolicy := &istiosecurityv1beta1.AuthorizationPolicy{} err := envTest.Client.Get(ctx, types.NamespacedName{ @@ -150,17 +135,13 @@ var _ = Describe("Checking Authorization Resource Creation", test.EnvTest(), fun return err } - Expect(createdAuthPolicy.ObjectMeta.OwnerReferences).To(HaveLen(1)) - ownerRef := createdAuthPolicy.ObjectMeta.OwnerReferences[0] - checkOwnerRef(ownerRef, *createdCfgMap) - Expect(createdAuthPolicy.Spec.GetAction()).To(Equal(v1beta1.AuthorizationPolicy_CUSTOM)) return nil }, 10*time.Second, 2*time.Second).Should(Succeed()) }) - It("should create a PeerAuthentication when a ConfigMap is created", func(ctx context.Context) { + It("should create a PeerAuthentication when a Component is created", func(ctx context.Context) { Eventually(func() error { createdPeerAuth := &istiosecurityv1beta1.PeerAuthentication{} err := envTest.Client.Get(ctx, types.NamespacedName{ @@ -172,25 +153,20 @@ var _ = Describe("Checking Authorization Resource Creation", test.EnvTest(), fun return err } - Expect(createdPeerAuth.ObjectMeta.OwnerReferences).To(HaveLen(1)) - ownerRef := createdPeerAuth.ObjectMeta.OwnerReferences[0] - checkOwnerRef(ownerRef, *createdCfgMap) - Expect(createdPeerAuth.Spec.GetMtls().GetMode()).To(Equal(v1beta1.PeerAuthentication_MutualTLS_PERMISSIVE)) return nil }, 10*time.Second, 2*time.Second).Should(Succeed()) }) - // TODO: + // TODO: fill out stubs once owner-name labels are propagated to auth resources PIt("should have ownerReference on all created auth resources", func(ctx context.Context) { - // get by owner name label + // get all three resources by owner name label + // use matchers.owner.go to ensure that correct ownerReference is set on all of them }) }) -func checkOwnerRef(owner metav1.OwnerReference, cfgMap corev1.ConfigMap) { - Expect(owner.Name).To(Equal(cfgMap.Name)) - Expect(owner.UID).To(Equal(cfgMap.UID)) - Expect(owner.BlockOwnerDeletion).To(BeNil()) +func componentResource(name, namespace string) []byte { + return []byte(fmt.Sprintf(watchedCR, name, namespace)) } diff --git a/controllers/authorization/suite_test.go b/controllers/authorization/suite_test.go index b900d33..c93fd70 100644 --- a/controllers/authorization/suite_test.go +++ b/controllers/authorization/suite_test.go @@ -34,12 +34,16 @@ var _ = SynchronizedBeforeSuite(func(ctx context.Context) { ctrl.Log.WithName("controllers").WithName("platform"), spi.AuthorizationComponent{ CustomResourceType: spi.ResourceSchema{ - GroupVersionKind: schema.GroupVersionKind{Version: "v1", Kind: "configmap"}, - Resources: "configmaps", + GroupVersionKind: schema.GroupVersionKind{ + Version: "v1", + Group: "opendatahub.io", + Kind: "Component", + }, + Resources: "components", }, WorkloadSelector: map[string]string{}, Ports: []string{}, - HostPaths: []string{"data.host"}, + HostPaths: []string{"spec.host"}, }, authorization.PlatformAuthorizationConfig{ Label: config.GetAuthorinoLabel(), diff --git a/test/data/crds/test.component.crd.yaml b/test/data/crds/test.component.crd.yaml index 01e6d82..a2ca929 100644 --- a/test/data/crds/test.component.crd.yaml +++ b/test/data/crds/test.component.crd.yaml @@ -17,6 +17,8 @@ spec: properties: name: type: string + host: + type: string scope: Namespaced names: plural: components diff --git a/test/fixtures.go b/test/fixtures.go index 58257d3..b03dba5 100644 --- a/test/fixtures.go +++ b/test/fixtures.go @@ -1,12 +1,17 @@ package test import ( + "context" _ "embed" "fmt" "os" "path/filepath" "github.com/onsi/ginkgo/v2" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/yaml" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) //go:embed data/expected_authconfig.yaml @@ -41,3 +46,16 @@ func ProjectRoot() string { return rootDir } + +func CreateOrUpdateResource(ctx context.Context, cli client.Client, data []byte) (*unstructured.Unstructured, error) { + unstrObj := &unstructured.Unstructured{} + if err := yaml.Unmarshal(data, &unstrObj.Object); err != nil { + return nil, fmt.Errorf("error unmarshalling YAML to unstructured: %w", err) + } + + _, err := controllerutil.CreateOrUpdate(ctx, cli, unstrObj, func() error { + return nil + }) + + return unstrObj, err +} diff --git a/test/matchers/authconfig.go b/test/matchers/authconfig.go new file mode 100644 index 0000000..b108aac --- /dev/null +++ b/test/matchers/authconfig.go @@ -0,0 +1,103 @@ +package matchers + +import ( + "fmt" + + authorinov1beta2 "github.com/kuadrant/authorino/api/v1beta2" + "github.com/onsi/gomega" + "github.com/onsi/gomega/format" + "github.com/onsi/gomega/types" +) + +// HaveHosts is a custom matcher to verify hosts in AuthConfigs +func HaveHosts(expectedHosts ...string) types.GomegaMatcher { + return &authConfigHostsMatcher{expectedHosts: expectedHosts} +} + +type authConfigHostsMatcher struct { + expectedHosts []string +} + +func (matcher *authConfigHostsMatcher) Match(actual any) (bool, error) { + if actual == nil { + return false, nil + } + + authConfig, ok := actual.(*authorinov1beta2.AuthConfig) + if !ok { + return false, fmt.Errorf("expected AuthConfig. Got:\n%s", format.Object(actual, 1)) + } + + return gomega.Equal(matcher.expectedHosts).Match(authConfig.Spec.Hosts) +} + +func (matcher *authConfigHostsMatcher) FailureMessage(actual any) string { + return format.Message(actual, "to have hosts", matcher.expectedHosts) +} + +func (matcher *authConfigHostsMatcher) NegatedFailureMessage(actual any) string { + return format.Message(actual, "not to have hosts", matcher.expectedHosts) +} + +// HaveKubernetesTokenReview is a custom matcher to verify Kubernetes Token Review configuration in AuthConfigs +func HaveKubernetesTokenReview() types.GomegaMatcher { + return &kubernetesTokenReviewMatcher{} +} + +type kubernetesTokenReviewMatcher struct{} + +func (matcher *kubernetesTokenReviewMatcher) Match(actual any) (bool, error) { + if actual == nil { + return false, nil + } + + authConfig, ok := actual.(*authorinov1beta2.AuthConfig) + if !ok { + return false, fmt.Errorf("expected AuthConfig. Got:\n%s", format.Object(actual, 1)) + } + + authMethod, found := authConfig.Spec.Authentication["kubernetes-user"] + if !found { + return false, nil + } + + return authMethod.KubernetesTokenReview != nil, nil +} + +func (matcher *kubernetesTokenReviewMatcher) FailureMessage(actual any) string { + return format.Message(actual, "to have Kubernetes Token Review configured for kubernetes-user authentication") +} + +func (matcher *kubernetesTokenReviewMatcher) NegatedFailureMessage(actual any) string { + return format.Message(actual, "not to have Kubernetes Token Review configured for kubernetes-user authentication") +} + +// HaveAuthenticationMethod is a custom matcher to verify the presence of an authentication method in AuthConfigs +func HaveAuthenticationMethod(method string) types.GomegaMatcher { + return &authConfigMethodMatcher{expectedMethod: method} +} + +type authConfigMethodMatcher struct { + expectedMethod string +} + +func (matcher *authConfigMethodMatcher) Match(actual any) (bool, error) { + if actual == nil { + return false, nil + } + + authConfig, ok := actual.(*authorinov1beta2.AuthConfig) + if !ok { + return false, fmt.Errorf("expected AuthConfig. Got:\n%s", format.Object(actual, 1)) + } + + return gomega.HaveKey(matcher.expectedMethod).Match(authConfig.Spec.Authentication) +} + +func (matcher *authConfigMethodMatcher) FailureMessage(actual any) string { + return format.Message(actual, "to have authentication method", matcher.expectedMethod) +} + +func (matcher *authConfigMethodMatcher) NegatedFailureMessage(actual any) string { + return format.Message(actual, "not to have authentication method", matcher.expectedMethod) +} diff --git a/test/matchers/owner.go b/test/matchers/owner.go new file mode 100644 index 0000000..ebb51c3 --- /dev/null +++ b/test/matchers/owner.go @@ -0,0 +1,53 @@ +package matchers + +import ( + "fmt" + + "github.com/onsi/gomega/format" + "github.com/onsi/gomega/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func HaveOwnerReference(expectedOwner metav1.OwnerReference) types.GomegaMatcher { + return &ownerReferenceMatcher{expectedOwner: expectedOwner} +} + +type ownerReferenceMatcher struct { + expectedOwner metav1.OwnerReference +} + +func (matcher *ownerReferenceMatcher) Match(actual any) (bool, error) { + if actual == nil { + return false, nil + } + + metaObject, ok := actual.(metav1.Object) + if !ok { + return false, fmt.Errorf("expected metav1.Object. Got:\n%s", format.Object(actual, 1)) + } + + ownerReferences := metaObject.GetOwnerReferences() + if len(ownerReferences) == 0 { + return false, nil + } + + for _, owner := range ownerReferences { + if owner.Name == matcher.expectedOwner.Name && + owner.UID == matcher.expectedOwner.UID && + owner.BlockOwnerDeletion == nil { + return true, nil + } + } + + return false, nil +} + +func (matcher *ownerReferenceMatcher) FailureMessage(actual any) string { + return format.Message(actual, "to have owner reference with Name", matcher.expectedOwner.Name, + "UID", matcher.expectedOwner.UID, "and BlockOwnerDeletion as nil") +} + +func (matcher *ownerReferenceMatcher) NegatedFailureMessage(actual any) string { + return format.Message(actual, "not to have owner reference with Name", matcher.expectedOwner.Name, + "UID", matcher.expectedOwner.UID, "and BlockOwnerDeletion as nil") +}