From cea6a14859ec68db80f828660c12d7c195c627a9 Mon Sep 17 00:00:00 2001 From: dtclxy64 <70486866+dtclxy64@users.noreply.github.com> Date: Fri, 6 Dec 2024 17:19:15 -0500 Subject: [PATCH] Extract the logic to add cluster annotations to the driver interface and add unit and integration tests Signed-off-by: dtclxy64 <70486866+dtclxy64@users.noreply.github.com> --- .../register/aws_irsa/aws_irsa.go | 14 ++- pkg/registration/register/csr/csr.go | 4 + pkg/registration/register/interface.go | 4 +- pkg/registration/spoke/spokeagent.go | 20 +-- pkg/registration/spoke/spokeagent_test.go | 14 +++ .../clusterannotations_aws_test.go | 69 ++++++++++ .../registration/clusterannotations_test.go | 10 ++ .../spokecluster_aws_joining_test.go | 118 ++++++++++++++++++ 8 files changed, 235 insertions(+), 18 deletions(-) create mode 100644 test/integration/registration/clusterannotations_aws_test.go create mode 100644 test/integration/registration/spokecluster_aws_joining_test.go diff --git a/pkg/registration/register/aws_irsa/aws_irsa.go b/pkg/registration/register/aws_irsa/aws_irsa.go index d129d7985..bc66075ad 100644 --- a/pkg/registration/register/aws_irsa/aws_irsa.go +++ b/pkg/registration/register/aws_irsa/aws_irsa.go @@ -3,6 +3,7 @@ package aws_irsa import ( "context" "fmt" + operatorv1 "open-cluster-management.io/api/operator/v1" "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" @@ -22,7 +23,9 @@ const ( // TLSKeyFile is the name of tls key file in kubeconfigSecret TLSKeyFile = "tls.key" // TLSCertFile is the name of the tls cert file in kubeconfigSecret - TLSCertFile = "tls.crt" + TLSCertFile = "tls.crt" + ManagedClusterArn = "managed-cluster-arn" + ManagedClusterIAMRoleSuffix = "managed-cluster-iam-role-suffix" ) type AWSIRSADriver struct { @@ -95,6 +98,15 @@ func (c *AWSIRSADriver) IsHubKubeConfigValid(ctx context.Context, secretOption r return true, nil } +func (c *AWSIRSADriver) AddClusterAnnotations(clusterAnnotations map[string]string, managedClusterArn string, managedClusterRoleSuffix string) { + if clusterAnnotations == nil { + clusterAnnotations = map[string]string{} + } + + clusterAnnotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterArn] = managedClusterArn + clusterAnnotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+ManagedClusterIAMRoleSuffix] = managedClusterRoleSuffix +} + func NewAWSIRSADriver() register.RegisterDriver { return &AWSIRSADriver{} } diff --git a/pkg/registration/register/csr/csr.go b/pkg/registration/register/csr/csr.go index b7f63fda1..dd095cd1f 100644 --- a/pkg/registration/register/csr/csr.go +++ b/pkg/registration/register/csr/csr.go @@ -266,6 +266,10 @@ func (c *CSRDriver) IsHubKubeConfigValid(ctx context.Context, secretOption regis return isCertificateValid(logger, certData, nil) } +// AddClusterAnnotations noop for CSR driver +func (c *CSRDriver) AddClusterAnnotations(clusterAnnotations map[string]string, managedClusterArn string, managedClusterRoleSuffix string) { +} + func NewCSRDriver() register.RegisterDriver { return &CSRDriver{} } diff --git a/pkg/registration/register/interface.go b/pkg/registration/register/interface.go index 2ec3b09e6..7283e74bb 100644 --- a/pkg/registration/register/interface.go +++ b/pkg/registration/register/interface.go @@ -2,7 +2,6 @@ package register import ( "context" - "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" corev1 "k8s.io/api/core/v1" @@ -71,6 +70,9 @@ type RegisterDriver interface { // InformerHandler returns informer of the related object. If no object needs to be watched, the func could // return nil, nil. InformerHandler(option any) (cache.SharedIndexInformer, factory.EventFilterFunc) + + // AddClusterAnnotations adds cluster annotations for non-CSR drivers + AddClusterAnnotations(clusterAnnotations map[string]string, managedClusterArn string, managedClusterRoleSuffix string) } // Approvers is the inteface that each driver should implement on hub side. The hub controller will use this driver diff --git a/pkg/registration/spoke/spokeagent.go b/pkg/registration/spoke/spokeagent.go index 8b22f968d..de52fd9fb 100644 --- a/pkg/registration/spoke/spokeagent.go +++ b/pkg/registration/spoke/spokeagent.go @@ -26,8 +26,6 @@ import ( clusterv1informers "open-cluster-management.io/api/client/cluster/informers/externalversions" clusterv1 "open-cluster-management.io/api/cluster/v1" ocmfeature "open-cluster-management.io/api/feature" - operatorv1 "open-cluster-management.io/api/operator/v1" - "open-cluster-management.io/ocm/pkg/common/helpers" commonoptions "open-cluster-management.io/ocm/pkg/common/options" "open-cluster-management.io/ocm/pkg/features" @@ -191,24 +189,14 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, // initiate registration driver var registerDriver register.RegisterDriver - if o.registrationOption.RegistrationAuth == AwsIrsaAuthType { - // TODO: may consider add additional validations - if o.registrationOption.HubClusterArn != "" { - registerDriver = awsIrsa.NewAWSIRSADriver() - if o.registrationOption.ClusterAnnotations == nil { - o.registrationOption.ClusterAnnotations = map[string]string{} - } - o.registrationOption.ClusterAnnotations[operatorv1.ClusterAnnotationsKeyPrefix+"/managed-cluster-arn"] = o.registrationOption.ManagedClusterArn - o.registrationOption.ClusterAnnotations[operatorv1.ClusterAnnotationsKeyPrefix+"/managed-cluster-iam-role-suffix"] = - o.registrationOption.ManagedClusterRoleSuffix - - } else { - panic("A valid EKS Hub Cluster ARN is required with awsirsa based authentication") - } + var registrationOption = o.registrationOption + if registrationOption.RegistrationAuth == AwsIrsaAuthType { + registerDriver = awsIrsa.NewAWSIRSADriver() } else { registerDriver = csr.NewCSRDriver() } + registerDriver.AddClusterAnnotations(registrationOption.ClusterAnnotations, registrationOption.ManagedClusterArn, registrationOption.ManagedClusterRoleSuffix) o.driver = registerDriver // get spoke cluster CA bundle diff --git a/pkg/registration/spoke/spokeagent_test.go b/pkg/registration/spoke/spokeagent_test.go index 2d510b1a6..110146fc9 100644 --- a/pkg/registration/spoke/spokeagent_test.go +++ b/pkg/registration/spoke/spokeagent_test.go @@ -45,6 +45,10 @@ func init() { func TestValidate(t *testing.T) { defaultCompletedOptions := NewSpokeAgentOptions() defaultCompletedOptions.BootstrapKubeconfig = "/spoke/bootstrap/kubeconfig" + awsCompletedOptionsHubArnMissing := *defaultCompletedOptions + awsCompletedOptionsHubArnMissing.RegistrationAuth = AwsIrsaAuthType + awsDefaultCompletedOptions := awsCompletedOptionsHubArnMissing + awsDefaultCompletedOptions.HubClusterArn = "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" cases := []struct { name string @@ -78,6 +82,16 @@ func TestValidate(t *testing.T) { options: defaultCompletedOptions, expectedErr: "", }, + { + name: "default completed options for aws flow", + options: &awsDefaultCompletedOptions, + expectedErr: "", + }, + { + name: "default completed options without HubClusterArn for aws flow", + options: &awsCompletedOptionsHubArnMissing, + expectedErr: "EksHubClusterArn cannot be empty if RegistrationAuth is awsirsa", + }, { name: "default completed options", options: &SpokeAgentOptions{ diff --git a/test/integration/registration/clusterannotations_aws_test.go b/test/integration/registration/clusterannotations_aws_test.go new file mode 100644 index 000000000..2ea2a64aa --- /dev/null +++ b/test/integration/registration/clusterannotations_aws_test.go @@ -0,0 +1,69 @@ +package registration_test + +import ( + "fmt" + operatorv1 "open-cluster-management.io/api/operator/v1" + "path" + "time" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + + commonoptions "open-cluster-management.io/ocm/pkg/common/options" + "open-cluster-management.io/ocm/pkg/registration/register/aws_irsa" + "open-cluster-management.io/ocm/pkg/registration/spoke" + "open-cluster-management.io/ocm/test/integration/util" +) + +var _ = ginkgo.Describe("Cluster Annotations for aws", func() { + ginkgo.It("Cluster Annotations for aws flow should be created on the managed cluster", func() { + managedClusterName := "clusterannotations-spokecluster-aws" + //#nosec G101 + hubKubeconfigSecret := "clusterannotations-hub-kubeconfig-secret" + hubKubeconfigDir := path.Join(util.TestDir, "clusterannotations", "hub-kubeconfig") + + managedClusterArn := "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1" + managedClusterRoleSuffix := "7f8141296c75f2871e3d030f85c35692" + hubClusterArn := "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" + agentOptions := &spoke.SpokeAgentOptions{ + RegistrationAuth: spoke.AwsIrsaAuthType, + HubClusterArn: hubClusterArn, + ManagedClusterArn: managedClusterArn, + ManagedClusterRoleSuffix: managedClusterRoleSuffix, + BootstrapKubeconfig: bootstrapKubeConfigFile, + HubKubeconfigSecret: hubKubeconfigSecret, + ClusterHealthCheckPeriod: 1 * time.Minute, + ClusterAnnotations: map[string]string{ + "agent.open-cluster-management.io/foo": "bar", + "foo": "bar", // this annotation should be filtered out + }, + } + + commOptions := commonoptions.NewAgentOptions() + commOptions.HubKubeconfigDir = hubKubeconfigDir + commOptions.SpokeClusterName = managedClusterName + + // run registration agent + cancel := runAgent("rotationtest", agentOptions, commOptions, spokeCfg) + defer cancel() + + // after bootstrap the spokecluster and csr should be created + gomega.Eventually(func() error { + mc, err := util.GetManagedCluster(clusterClient, managedClusterName) + if err != nil { + return err + } + + if mc.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+aws_irsa.ManagedClusterArn] != managedClusterArn { + return fmt.Errorf("expected annotation "+operatorv1.ClusterAnnotationsKeyPrefix+"/"+aws_irsa.ManagedClusterArn+" to be "+managedClusterArn+", got %s", mc.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+aws_irsa.ManagedClusterArn]) + } + + if mc.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+aws_irsa.ManagedClusterIAMRoleSuffix] != managedClusterRoleSuffix { + return fmt.Errorf("expected annotation "+operatorv1.ClusterAnnotationsKeyPrefix+"/"+aws_irsa.ManagedClusterIAMRoleSuffix+" to be "+managedClusterRoleSuffix+", got %s", mc.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+aws_irsa.ManagedClusterIAMRoleSuffix]) + } + + return nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Succeed()) + + }) +}) diff --git a/test/integration/registration/clusterannotations_test.go b/test/integration/registration/clusterannotations_test.go index 2be6e7817..623115619 100644 --- a/test/integration/registration/clusterannotations_test.go +++ b/test/integration/registration/clusterannotations_test.go @@ -2,6 +2,8 @@ package registration_test import ( "fmt" + operatorv1 "open-cluster-management.io/api/operator/v1" + "open-cluster-management.io/ocm/pkg/registration/register/aws_irsa" "path" "time" @@ -49,6 +51,14 @@ var _ = ginkgo.Describe("Cluster Annotations", func() { return fmt.Errorf("unexpected annotations %v", mc.Annotations) } + if _, ok := mc.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+aws_irsa.ManagedClusterArn]; ok { + return fmt.Errorf("unexpected annotations %v", mc.Annotations) + } + + if _, ok := mc.Annotations[operatorv1.ClusterAnnotationsKeyPrefix+"/"+aws_irsa.ManagedClusterIAMRoleSuffix]; ok { + return fmt.Errorf("unexpected annotations %v", mc.Annotations) + } + if mc.Annotations["agent.open-cluster-management.io/foo"] != "bar" { return fmt.Errorf("expected annotation agent.open-cluster-management.io/foo to be bar, got %s", mc.Annotations["agent.open-cluster-management.io/foo"]) } diff --git a/test/integration/registration/spokecluster_aws_joining_test.go b/test/integration/registration/spokecluster_aws_joining_test.go new file mode 100644 index 000000000..0eb54b4e5 --- /dev/null +++ b/test/integration/registration/spokecluster_aws_joining_test.go @@ -0,0 +1,118 @@ +package registration_test + +import ( + "fmt" + "path" + "time" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/util/rand" + commonoptions "open-cluster-management.io/ocm/pkg/common/options" + "open-cluster-management.io/ocm/pkg/registration/spoke" + "open-cluster-management.io/ocm/test/integration/util" +) + +var _ = ginkgo.Describe("Joining Process for aws flow", func() { + var bootstrapKubeconfig string + var managedClusterName string + var hubKubeconfigSecret string + var hubKubeconfigDir string + + ginkgo.BeforeEach(func() { + postfix := rand.String(5) + managedClusterName = fmt.Sprintf("joiningtest-managedcluster-%s", postfix) + hubKubeconfigSecret = fmt.Sprintf("joiningtest-hub-kubeconfig-secret-%s", postfix) + hubKubeconfigDir = path.Join(util.TestDir, fmt.Sprintf("joiningtest-%s", postfix), "hub-kubeconfig") + }) + + assertJoiningSucceed := func() { + ginkgo.It("managedcluster should join successfully for aws flow", func() { + var err error + + managedClusterArn := "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1" + managedClusterRoleSuffix := "7f8141296c75f2871e3d030f85c35692" + hubClusterArn := "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" + + // run registration agent + agentOptions := &spoke.SpokeAgentOptions{ + RegistrationAuth: spoke.AwsIrsaAuthType, + HubClusterArn: hubClusterArn, + ManagedClusterArn: managedClusterArn, + ManagedClusterRoleSuffix: managedClusterRoleSuffix, + BootstrapKubeconfig: bootstrapKubeconfig, + HubKubeconfigSecret: hubKubeconfigSecret, + ClusterHealthCheckPeriod: 1 * time.Minute, + } + commOptions := commonoptions.NewAgentOptions() + commOptions.HubKubeconfigDir = hubKubeconfigDir + commOptions.SpokeClusterName = managedClusterName + + cancel := runAgent("joiningtest", agentOptions, commOptions, spokeCfg) + defer cancel() + + // the ManagedCluster CR should be created after bootstrap + gomega.Eventually(func() error { + if _, err := util.GetManagedCluster(clusterClient, managedClusterName); err != nil { + return err + } + return nil + }, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred()) + + // the csr should not be created for aws flow after bootstrap + gomega.Eventually(func() error { + if _, err := util.FindUnapprovedSpokeCSR(kubeClient, managedClusterName); err != nil { + return err + } + return nil + }, eventuallyTimeout, eventuallyInterval).Should(gomega.HaveOccurred()) + + // simulate hub cluster admin to accept the managedcluster + err = util.AcceptManagedCluster(clusterClient, managedClusterName) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + err = authn.ApproveSpokeClusterCSR(kubeClient, managedClusterName, time.Hour*24) + gomega.Expect(err).To(gomega.HaveOccurred()) + + // the hub kubeconfig secret should be filled after the ManagedCluster is accepted + // TODO: Revisit while implementing slice 3 + //gomega.Eventually(func() error { + // secret, err := util.GetFilledHubKubeConfigSecret(kubeClient, testNamespace, hubKubeconfigSecret) + // if err != nil { + // return err + // } + // + // // check if the proxyURL is set correctly + // proxyURL, err := getProxyURLFromKubeconfigData(secret.Data["kubeconfig"]) + // if err != nil { + // return err + // } + // if proxyURL != expectedProxyURL { + // return fmt.Errorf("expected proxy url %q, but got %q", expectedProxyURL, proxyURL) + // } + // return nil + //}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred()) + + // the spoke cluster should have joined condition finally + // TODO: Revisit while implementing slice 3 + //gomega.Eventually(func() error { + // spokeCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) + // if err != nil { + // return err + // } + // if !meta.IsStatusConditionTrue(spokeCluster.Status.Conditions, clusterv1.ManagedClusterConditionJoined) { + // return fmt.Errorf("cluster should be joined") + // } + // return nil + //}, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred()) + }) + } + + ginkgo.Context("without proxy", func() { + ginkgo.BeforeEach(func() { + bootstrapKubeconfig = bootstrapKubeConfigFile + }) + assertJoiningSucceed() + }) + +})