From 7e835fc50e0920d8c7883c251cb2b50438eab2aa Mon Sep 17 00:00:00 2001 From: Aleksei Sizov Date: Mon, 14 Oct 2024 14:50:56 -0500 Subject: [PATCH] Add credential admission checks for managedcluster --- .../controller/managedcluster_controller.go | 2 - .../managedcluster_controller_test.go | 47 ++++++- internal/webhook/managedcluster_webhook.go | 70 ++++++++++ .../webhook/managedcluster_webhook_test.go | 120 ++++++++++++++++-- test/objects/credential/credential.go | 66 ++++++++++ test/objects/managedcluster/managedcluster.go | 6 + 6 files changed, 299 insertions(+), 12 deletions(-) create mode 100644 test/objects/credential/credential.go diff --git a/internal/controller/managedcluster_controller.go b/internal/controller/managedcluster_controller.go index e29981f24..324b186b7 100644 --- a/internal/controller/managedcluster_controller.go +++ b/internal/controller/managedcluster_controller.go @@ -308,8 +308,6 @@ func (r *ManagedClusterReconciler) Update(ctx context.Context, managedCluster *h Reason: hmc.FailedReason, Message: "Credential is not in Ready state", }) - return ctrl.Result{}, - fmt.Errorf("credential is not in Ready state") } apimeta.SetStatusCondition(managedCluster.GetConditions(), metav1.Condition{ diff --git a/internal/controller/managedcluster_controller_test.go b/internal/controller/managedcluster_controller_test.go index 96f77e23c..a7070fa7e 100644 --- a/internal/controller/managedcluster_controller_test.go +++ b/internal/controller/managedcluster_controller_test.go @@ -38,7 +38,8 @@ var _ = Describe("ManagedCluster Controller", func() { managedClusterName = "test-managed-cluster" managedClusterNamespace = "test" - templateName = "test-template" + templateName = "test-template" + credentialName = "test-credential" ) ctx := context.Background() @@ -50,6 +51,7 @@ var _ = Describe("ManagedCluster Controller", func() { managedCluster := &hmc.ManagedCluster{} template := &hmc.ClusterTemplate{} management := &hmc.Management{} + credential := &hmc.Credential{} namespace := &corev1.Namespace{} BeforeEach(func() { @@ -92,6 +94,13 @@ var _ = Describe("ManagedCluster Controller", func() { Raw: []byte(`{"foo":"bar"}`), }, }, + Providers: hmc.ProvidersTupled{ + InfrastructureProviders: []hmc.ProviderTuple{ + { + Name: "aws", + }, + }, + }, } Expect(k8sClient.Status().Update(ctx, template)).To(Succeed()) } @@ -106,7 +115,40 @@ var _ = Describe("ManagedCluster Controller", func() { Spec: hmc.ManagementSpec{}, } Expect(k8sClient.Create(ctx, management)).To(Succeed()) + management.Status = hmc.ManagementStatus{ + AvailableProviders: hmc.ProvidersTupled{ + InfrastructureProviders: []hmc.ProviderTuple{ + { + Name: "aws", + }, + }, + }, + } + Expect(k8sClient.Status().Update(ctx, management)).To(Succeed()) } + By("creating the custom resource for the Kind Credential") + err = k8sClient.Get(ctx, typeNamespacedName, credential) + if err != nil && errors.IsNotFound(err) { + credential = &hmc.Credential{ + ObjectMeta: metav1.ObjectMeta{ + Name: credentialName, + Namespace: managedClusterNamespace, + }, + Spec: hmc.CredentialSpec{ + IdentityRef: &corev1.ObjectReference{ + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta2", + Kind: "AWSClusterStaticIdentity", + Name: "foo", + }, + }, + } + Expect(k8sClient.Create(ctx, credential)).To(Succeed()) + credential.Status = hmc.CredentialStatus{ + State: hmc.CredentialReady, + } + Expect(k8sClient.Status().Update(ctx, credential)).To(Succeed()) + } + By("creating the custom resource for the Kind ManagedCluster") err = k8sClient.Get(ctx, typeNamespacedName, managedCluster) if err != nil && errors.IsNotFound(err) { @@ -116,7 +158,8 @@ var _ = Describe("ManagedCluster Controller", func() { Namespace: managedClusterNamespace, }, Spec: hmc.ManagedClusterSpec{ - Template: templateName, + Template: templateName, + Credential: credentialName, }, } Expect(k8sClient.Create(ctx, managedCluster)).To(Succeed()) diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index 21d57237b..f095d8769 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -70,6 +70,10 @@ func (v *ManagedClusterValidator) ValidateCreate(ctx context.Context, obj runtim return admission.Warnings{"Failed to validate k8s version compatibility with ServiceTemplates"}, fmt.Errorf("failed to validate k8s compatibility: %v", err) } + if err := v.validateCredential(ctx, managedCluster, template); err != nil { + return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) + } + return nil, nil } @@ -93,6 +97,10 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, _ runtime. return admission.Warnings{"Failed to validate k8s version compatibility with ServiceTemplates"}, fmt.Errorf("failed to validate k8s compatibility: %v", err) } + if err := v.validateCredential(ctx, newManagedCluster, template); err != nil { + return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err) + } + return nil, nil } @@ -178,6 +186,18 @@ func (v *ManagedClusterValidator) getManagedClusterTemplate(ctx context.Context, return tpl, v.Get(ctx, client.ObjectKey{Namespace: templateNamespace, Name: templateName}, tpl) } +func (v *ManagedClusterValidator) getManagedClusterCredential(ctx context.Context, credNamespace, credName string) (*hmcv1alpha1.Credential, error) { + cred := &hmcv1alpha1.Credential{} + credRef := client.ObjectKey{ + Name: credName, + Namespace: credNamespace, + } + if err := v.Get(ctx, credRef, cred); err != nil { + return nil, err + } + return cred, nil +} + func isTemplateValid(template *hmcv1alpha1.ClusterTemplate) error { if !template.Status.Valid { return fmt.Errorf("the template is not valid: %s", template.Status.ValidationError) @@ -185,3 +205,53 @@ func isTemplateValid(template *hmcv1alpha1.ClusterTemplate) error { return nil } + +func (v *ManagedClusterValidator) validateCredential(ctx context.Context, managedCluster *hmcv1alpha1.ManagedCluster, template *hmcv1alpha1.ClusterTemplate) error { + infraProviders := template.Status.Providers.InfrastructureProviders + + if len(infraProviders) == 0 { + return fmt.Errorf("template %q has no infrastructure providers defined", template.Name) + } + + cred, err := v.getManagedClusterCredential(ctx, managedCluster.Namespace, managedCluster.Spec.Credential) + if err != nil { + return err + } + + if cred.Status.State != hmcv1alpha1.CredentialReady { + return fmt.Errorf("credential is not Ready") + } + + return isCredMatchTemplate(cred, template) +} + +func isCredMatchTemplate(cred *hmcv1alpha1.Credential, template *hmcv1alpha1.ClusterTemplate) error { + idtyKind := cred.Spec.IdentityRef.Kind + infraProviders := template.Status.Providers.InfrastructureProviders + + errMsg := func(idtyKind string, provider string) error { + return fmt.Errorf("wrong kind of the ClusterIdentity %q for provider %q", idtyKind, provider) + } + + for _, provider := range infraProviders { + switch provider.Name { + case "aws": + if idtyKind != "AWSClusterStaticIdentity" && + idtyKind != "AWSClusterRoleIdentity" && + idtyKind != "AWSClusterControllerIdentity" { + return errMsg(idtyKind, provider.Name) + } + case "azure": + if idtyKind != "AzureClusterIdentity" { + return errMsg(idtyKind, provider.Name) + } + case "vsphere": + if idtyKind != "VSphereClusterIdentity" { + return errMsg(idtyKind, provider.Name) + } + default: + return fmt.Errorf("unsupported infrastructure provider %s", provider.Name) + } + } + return nil +} diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index b1157f1c3..1dc4589b8 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -21,11 +21,13 @@ import ( . "github.com/onsi/gomega" admissionv1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/test/objects/credential" "github.com/Mirantis/hmc/test/objects/managedcluster" "github.com/Mirantis/hmc/test/objects/management" "github.com/Mirantis/hmc/test/objects/template" @@ -33,8 +35,9 @@ import ( ) var ( - testTemplateName = "template-test" - testNamespace = "test" + testTemplateName = "template-test" + testCredentialName = "cred-test" + testNamespace = "test" mgmt = management.NewManagement( management.WithAvailableProviders(v1alpha1.ProvidersTupled{ @@ -44,6 +47,16 @@ var ( }), ) + cred = credential.NewCredential( + credential.WithName(testCredentialName), + credential.WithState(v1alpha1.CredentialReady), + credential.WithIdentityRef( + &corev1.ObjectReference{ + Kind: "AWSClusterStaticIdentity", + Name: "awsclid", + }), + ) + createAndUpdateTests = []struct { name string managedCluster *v1alpha1.ManagedCluster @@ -57,10 +70,14 @@ var ( err: "the ManagedCluster is invalid: clustertemplates.hmc.mirantis.com \"\" not found", }, { - name: "should fail if the ClusterTemplate is not found in the ManagedCluster's namespace", - managedCluster: managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(testTemplateName)), + name: "should fail if the ClusterTemplate is not found in the ManagedCluster's namespace", + managedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + ), existingObjects: []runtime.Object{ mgmt, + cred, template.NewClusterTemplate( template.WithName(testTemplateName), template.WithNamespace(testNamespace), @@ -69,10 +86,14 @@ var ( err: fmt.Sprintf("the ManagedCluster is invalid: clustertemplates.hmc.mirantis.com \"%s\" not found", testTemplateName), }, { - name: "should fail if the cluster template was found but is invalid (some validation error)", - managedCluster: managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(testTemplateName)), + name: "should fail if the cluster template was found but is invalid (some validation error)", + managedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + ), existingObjects: []runtime.Object{ mgmt, + cred, template.NewClusterTemplate( template.WithName(testTemplateName), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ @@ -84,12 +105,21 @@ var ( err: "the ManagedCluster is invalid: the template is not valid: validation error example", }, { - name: "should succeed", - managedCluster: managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(testTemplateName)), + name: "should succeed", + managedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + ), existingObjects: []runtime.Object{ mgmt, + cred, template.NewClusterTemplate( template.WithName(testTemplateName), + template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws"}}, + BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), }, @@ -101,6 +131,7 @@ var ( managedcluster.WithServiceTemplate(testTemplateName), ), existingObjects: []runtime.Object{ + cred, management.NewManagement(management.WithAvailableProviders(v1alpha1.ProvidersTupled{ InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws", VersionOrConstraint: "v1.0.0"}}, BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s", VersionOrConstraint: "v1.0.0"}}, @@ -120,6 +151,79 @@ var ( err: fmt.Sprintf(`failed to validate k8s compatibility: k8s version v1.30.0 of the ManagedCluster default/%s does not satisfy constrained version <1.30 from the ServiceTemplate default/%s`, managedcluster.DefaultName, testTemplateName), warnings: admission.Warnings{"Failed to validate k8s version compatibility with ServiceTemplates"}, }, + { + name: "should fail if the credential is unset", + managedCluster: managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(testTemplateName)), + existingObjects: []runtime.Object{ + mgmt, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws"}}, + BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + err: "the ManagedCluster is invalid: credentials.hmc.mirantis.com \"\" not found", + }, + { + name: "should fail if credential is not Ready", + managedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + ), + existingObjects: []runtime.Object{ + mgmt, + credential.NewCredential( + credential.WithName(testCredentialName), + credential.WithState(v1alpha1.CredentialNotFound), + credential.WithIdentityRef( + &corev1.ObjectReference{ + Kind: "AWSClusterStaticIdentity", + Name: "awsclid", + }), + ), + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "aws"}}, + BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + err: "the ManagedCluster is invalid: credential is not Ready", + }, + { + name: "should fail if credential and template providers doesn't match", + managedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + ), + existingObjects: []runtime.Object{ + cred, + management.NewManagement( + management.WithAvailableProviders(v1alpha1.ProvidersTupled{ + InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "azure"}}, + BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + }), + ), + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithProvidersStatus(v1alpha1.ProvidersTupled{ + InfrastructureProviders: []v1alpha1.ProviderTuple{{Name: "azure"}}, + BootstrapProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + ControlPlaneProviders: []v1alpha1.ProviderTuple{{Name: "k0s"}}, + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + err: "the ManagedCluster is invalid: wrong kind of the ClusterIdentity \"AWSClusterStaticIdentity\" for provider \"azure\"", + }, } ) diff --git a/test/objects/credential/credential.go b/test/objects/credential/credential.go new file mode 100644 index 000000000..31739931a --- /dev/null +++ b/test/objects/credential/credential.go @@ -0,0 +1,66 @@ +// Copyright 2024 +// +// 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 credential + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/Mirantis/hmc/api/v1alpha1" +) + +const ( + DefaultName = "credential" +) + +type Opt func(credential *v1alpha1.Credential) + +func NewCredential(opts ...Opt) *v1alpha1.Credential { + p := &v1alpha1.Credential{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultName, + Namespace: metav1.NamespaceDefault, + }, + } + + for _, opt := range opts { + opt(p) + } + return p +} + +func WithName(name string) Opt { + return func(p *v1alpha1.Credential) { + p.Name = name + } +} + +func WithNamespace(namespace string) Opt { + return func(p *v1alpha1.Credential) { + p.Namespace = namespace + } +} + +func WithIdentityRef(idtyRef *corev1.ObjectReference) Opt { + return func(p *v1alpha1.Credential) { + p.Spec.IdentityRef = idtyRef + } +} + +func WithState(state v1alpha1.CredentialState) Opt { + return func(p *v1alpha1.Credential) { + p.Status.State = state + } +} diff --git a/test/objects/managedcluster/managedcluster.go b/test/objects/managedcluster/managedcluster.go index 4204576a6..fc6c00770 100644 --- a/test/objects/managedcluster/managedcluster.go +++ b/test/objects/managedcluster/managedcluster.go @@ -81,3 +81,9 @@ func WithServiceTemplate(templateName string) Opt { }) } } + +func WithCredential(credName string) Opt { + return func(p *v1alpha1.ManagedCluster) { + p.Spec.Credential = credName + } +}