From 430d3ff9e78f8b10cd115d9a23a0ff955dbc1eec Mon Sep 17 00:00:00 2001 From: Gaurav Jaswal Date: Thu, 21 Nov 2024 18:15:26 -0500 Subject: [PATCH 1/4] Starting aws registration by spoke by assuming IAM role on startup and adding annotations to ManagedCluster CR Signed-off-by: Erica Jin <132393634+EricaJ6@users.noreply.github.com> --- ...lusterlet-registration-serviceaccount.yaml | 4 + .../klusterlet-work-serviceaccount.yaml | 4 + .../klusterlet-agent-deployment.yaml | 16 +++- .../klusterlet-registration-deployment.yaml | 16 +++- .../klusterlet-work-deployment.yaml | 8 ++ pkg/common/helpers/aws.go | 16 ++++ .../klusterlet_controller.go | 54 +++++++++++++- .../klusterlet_controller_test.go | 73 +++++++++++++++++-- pkg/registration/helpers/helpers.go | 12 --- pkg/registration/spoke/options.go | 12 ++- pkg/registration/spoke/spokeagent.go | 10 +-- 11 files changed, 194 insertions(+), 31 deletions(-) create mode 100644 pkg/common/helpers/aws.go diff --git a/manifests/klusterlet/managed/klusterlet-registration-serviceaccount.yaml b/manifests/klusterlet/managed/klusterlet-registration-serviceaccount.yaml index bbbe83dc7..431e8ef07 100644 --- a/manifests/klusterlet/managed/klusterlet-registration-serviceaccount.yaml +++ b/manifests/klusterlet/managed/klusterlet-registration-serviceaccount.yaml @@ -9,5 +9,9 @@ metadata: "{{ $key }}": "{{ $value }}" {{ end }} {{ end }} + {{ if and .ManagedClusterRoleArn (eq .RegistrationDriver.AuthType "awsirsa") }} + annotations: + eks.amazonaws.com/role-arn: {{ .ManagedClusterRoleArn }} + {{ end }} imagePullSecrets: - name: open-cluster-management-image-pull-credentials diff --git a/manifests/klusterlet/managed/klusterlet-work-serviceaccount.yaml b/manifests/klusterlet/managed/klusterlet-work-serviceaccount.yaml index bf2f326f0..b16ddcf20 100644 --- a/manifests/klusterlet/managed/klusterlet-work-serviceaccount.yaml +++ b/manifests/klusterlet/managed/klusterlet-work-serviceaccount.yaml @@ -9,5 +9,9 @@ metadata: "{{ $key }}": "{{ $value }}" {{ end }} {{ end }} + {{ if and .ManagedClusterRoleArn (eq .RegistrationDriver.AuthType "awsirsa") }} + annotations: + eks.amazonaws.com/role-arn: {{ .ManagedClusterRoleArn }} + {{ end }} imagePullSecrets: - name: open-cluster-management-image-pull-credentials diff --git a/manifests/klusterlet/management/klusterlet-agent-deployment.yaml b/manifests/klusterlet/management/klusterlet-agent-deployment.yaml index 6a4f69a60..2c93df472 100644 --- a/manifests/klusterlet/management/klusterlet-agent-deployment.yaml +++ b/manifests/klusterlet/management/klusterlet-agent-deployment.yaml @@ -109,11 +109,15 @@ spec: {{if .AppliedManifestWorkEvictionGracePeriod}} - "--appliedmanifestwork-eviction-grace-period={{ .AppliedManifestWorkEvictionGracePeriod }}" {{end}} - {{if .RegistrationDriver.AuthType}} + {{if and .RegistrationDriver .RegistrationDriver.AuthType}} - "--registration-auth={{ .RegistrationDriver.AuthType }}" - {{end}} {{if eq .RegistrationDriver.AuthType "awsirsa"}} - "--hub-cluster-arn={{ .RegistrationDriver.AwsIrsa.HubClusterArn }}" + - "--managed-cluster-arn={{ .RegistrationDriver.AwsIrsa.ManagedClusterArn }}" + {{if .ManagedClusterRoleSuffix}} + - "--managed-cluster-role-suffix={{ .ManagedClusterRoleSuffix }}" + {{end}} + {{end}} {{end}} env: - name: POD_NAME @@ -144,6 +148,10 @@ spec: mountPath: "/spoke/hub-kubeconfig" - name: tmpdir mountPath: /tmp + {{if and .RegistrationDriver .RegistrationDriver.AuthType (eq .RegistrationDriver.AuthType "awsirsa")}} + - name: dot-aws + mountPath: /.aws + {{end}} {{if eq .InstallMode "SingletonHosted"}} - name: spoke-kubeconfig-secret mountPath: "/spoke/config" @@ -195,6 +203,10 @@ spec: medium: Memory - name: tmpdir emptyDir: { } + {{if and .RegistrationDriver .RegistrationDriver.AuthType (eq .RegistrationDriver.AuthType "awsirsa")}} + - name: dot-aws + emptyDir: { } + {{end}} {{if eq .InstallMode "SingletonHosted"}} - name: spoke-kubeconfig-secret secret: diff --git a/manifests/klusterlet/management/klusterlet-registration-deployment.yaml b/manifests/klusterlet/management/klusterlet-registration-deployment.yaml index 1a6b4893d..2e0623907 100644 --- a/manifests/klusterlet/management/klusterlet-registration-deployment.yaml +++ b/manifests/klusterlet/management/klusterlet-registration-deployment.yaml @@ -97,11 +97,15 @@ spec: {{if gt .RegistrationKubeAPIBurst 0}} - "--kube-api-burst={{ .RegistrationKubeAPIBurst }}" {{end}} - {{if .RegistrationDriver.AuthType}} + {{if and .RegistrationDriver .RegistrationDriver.AuthType}} - "--registration-auth={{ .RegistrationDriver.AuthType }}" - {{end}} {{if eq .RegistrationDriver.AuthType "awsirsa"}} - "--hub-cluster-arn={{ .RegistrationDriver.AwsIrsa.HubClusterArn }}" + - "--managed-cluster-arn={{ .RegistrationDriver.AwsIrsa.ManagedClusterArn }}" + {{if .ManagedClusterRoleSuffix}} + - "--managed-cluster-role-suffix={{ .ManagedClusterRoleSuffix }}" + {{end}} + {{end}} {{end}} env: - name: POD_NAME @@ -132,6 +136,10 @@ spec: mountPath: "/spoke/hub-kubeconfig" - name: tmpdir mountPath: /tmp + {{if and .RegistrationDriver .RegistrationDriver.AuthType (eq .RegistrationDriver.AuthType "awsirsa")}} + - name: dot-aws + mountPath: /.aws + {{end}} {{if eq .InstallMode "Hosted"}} - name: spoke-kubeconfig-secret mountPath: "/spoke/config" @@ -183,6 +191,10 @@ spec: medium: Memory - name: tmpdir emptyDir: { } + {{if and .RegistrationDriver .RegistrationDriver.AuthType (eq .RegistrationDriver.AuthType "awsirsa")}} + - name: dot-aws + emptyDir: { } + {{end}} {{if eq .InstallMode "Hosted"}} - name: spoke-kubeconfig-secret secret: diff --git a/manifests/klusterlet/management/klusterlet-work-deployment.yaml b/manifests/klusterlet/management/klusterlet-work-deployment.yaml index 0a71fc4f4..cb9913840 100644 --- a/manifests/klusterlet/management/klusterlet-work-deployment.yaml +++ b/manifests/klusterlet/management/klusterlet-work-deployment.yaml @@ -107,6 +107,10 @@ spec: readOnly: true - name: tmpdir mountPath: /tmp + {{if and .RegistrationDriver .RegistrationDriver.AuthType (eq .RegistrationDriver.AuthType "awsirsa")}} + - name: dot-aws + mountPath: /.aws + {{end}} {{if eq .InstallMode "Hosted"}} - name: spoke-kubeconfig-secret mountPath: "/spoke/config" @@ -147,6 +151,10 @@ spec: secretName: {{ .HubKubeConfigSecret }} - name: tmpdir emptyDir: { } + {{if and .RegistrationDriver .RegistrationDriver.AuthType (eq .RegistrationDriver.AuthType "awsirsa")}} + - name: dot-aws + emptyDir: { } + {{end}} {{if eq .InstallMode "Hosted"}} - name: spoke-kubeconfig-secret secret: diff --git a/pkg/common/helpers/aws.go b/pkg/common/helpers/aws.go new file mode 100644 index 000000000..c9e789de8 --- /dev/null +++ b/pkg/common/helpers/aws.go @@ -0,0 +1,16 @@ +package helpers + +import ( + "regexp" +) + +// IsEksArnWellFormed checks if the EKS cluster ARN is well-formed +// Example of a well-formed ARN: arn:aws:eks:us-west-2:123456789012:cluster/my-cluster +func IsEksArnWellFormed(eksArn string) bool { + pattern := "^arn:aws:eks:([a-zA-Z0-9-]+):(\\d{12}):cluster/([a-zA-Z0-9-]+)$" + matched, err := regexp.MatchString(pattern, eksArn) + if err != nil { + return false + } + return matched +} diff --git a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go index 30e1c5ee8..3ea388a95 100644 --- a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go +++ b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go @@ -2,6 +2,9 @@ package klusterletcontroller import ( "context" + "crypto/md5" // #nosec G501 + "encoding/hex" + er "errors" "fmt" "strings" "time" @@ -114,7 +117,8 @@ func NewKlusterletController( } type AwsIrsa struct { - HubClusterArn string + HubClusterArn string + ManagedClusterArn string } type RegistrationDriver struct { @@ -187,6 +191,10 @@ type klusterletConfig struct { // Labels of the agents are synced from klusterlet CR. Labels map[string]string RegistrationDriver RegistrationDriver + + ManagedClusterArn string + ManagedClusterRoleArn string + ManagedClusterRoleSuffix string } // If multiplehubs feature gate is enabled, using the bootstrapkubeconfigs from klusterlet CR. @@ -329,12 +337,32 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto //Configuring Registration driver depending on registration auth if &klusterlet.Spec.RegistrationConfiguration.RegistrationDriver != nil && klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType == AwsIrsaAuthType { + + hubClusterArn := klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.HubClusterArn + managedClusterArn := klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.ManagedClusterArn + + if !commonhelpers.IsEksArnWellFormed(hubClusterArn) { + errorMsg := fmt.Sprintf("HubClusterArn %s is not well formed", hubClusterArn) + klog.Errorf(errorMsg) + return er.New(errorMsg) + } + + if !commonhelpers.IsEksArnWellFormed(managedClusterArn) { + errorMsg := fmt.Sprintf("ManagedClusterArn %s is not well formed", managedClusterArn) + klog.Errorf(errorMsg) + return er.New(errorMsg) + } + config.RegistrationDriver = RegistrationDriver{ AuthType: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType, AwsIrsa: &AwsIrsa{ - HubClusterArn: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.HubClusterArn, + HubClusterArn: hubClusterArn, + ManagedClusterArn: managedClusterArn, }, } + managedClusterRoleArn, managedClusterRoleSuffix := n.generateManagedRoleArnAndSuffix(klusterlet) + config.ManagedClusterRoleArn = managedClusterRoleArn + config.ManagedClusterRoleSuffix = managedClusterRoleSuffix } else { config.RegistrationDriver = RegistrationDriver{ AuthType: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType, @@ -433,6 +461,23 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto return utilerrors.NewAggregate(errs) } +func (n *klusterletController) generateManagedRoleArnAndSuffix(klusterlet *operatorapiv1.Klusterlet) (string, string) { + hubClusterArn := klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.HubClusterArn + managedClusterArn := klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.ManagedClusterArn + + hubClusterStringParts := strings.Split(hubClusterArn, ":") + + managedClusterStringParts := strings.Split(managedClusterArn, ":") + hubClusterName := strings.Split(hubClusterStringParts[5], "/")[1] + hubClusterAccountId := hubClusterStringParts[4] + managedClusterName := strings.Split(managedClusterStringParts[5], "/")[1] + managedClusterAccountId := managedClusterStringParts[4] + md5HashUniqueIdentifier := generateMd5HashUniqueIdentifier(hubClusterAccountId, hubClusterName, managedClusterAccountId, managedClusterName) + //arn:aws:iam:::role/ocm-managed-cluster- + managedClusterRoleArn := "arn:aws:iam::" + managedClusterAccountId + ":role/ocm-managed-cluster-" + md5HashUniqueIdentifier + return managedClusterRoleArn, md5HashUniqueIdentifier +} + // TODO also read CABundle from ExternalServerURLs and set into registration deployment func getServersFromKlusterlet(klusterlet *operatorapiv1.Klusterlet) string { if klusterlet.Spec.ExternalServerURLs == nil { @@ -536,3 +581,8 @@ func serviceAccountName(suffix string, klusterlet *operatorapiv1.Klusterlet) str } return fmt.Sprintf("%s-%s", klusterlet.Name, suffix) } + +func generateMd5HashUniqueIdentifier(hubClusterAccountId string, hubClusterName string, managedClusterAccountId string, managedClusterName string) string { + hash := md5.Sum([]byte(hubClusterAccountId + "#" + hubClusterName + "#" + managedClusterAccountId + "#" + managedClusterName)) // #nosec G401 + return hex.EncodeToString(hash[:]) +} diff --git a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go index 983bafe11..9998ce263 100644 --- a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go +++ b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go @@ -388,6 +388,9 @@ func assertKlusterletDeployment(t *testing.T, actions []clienttesting.Action, ve } args := deployment.Spec.Template.Spec.Containers[0].Args + volumeMounts := deployment.Spec.Template.Spec.Containers[0].VolumeMounts + volumes := deployment.Spec.Template.Spec.Volumes + expectedArgs := []string{ "/registration-operator", "agent", @@ -406,13 +409,37 @@ func assertKlusterletDeployment(t *testing.T, actions []clienttesting.Action, ve } expectedArgs = append(expectedArgs, "--status-sync-interval=60s", "--kube-api-qps=20", "--kube-api-burst=60", - "--registration-auth=awsirsa", "--hub-cluster-arn=arneks:us-west-2:123456789012:cluster/hub-cluster1") + "--registration-auth=awsirsa", + "--hub-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1", + "--managed-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1", + "--managed-cluster-role-suffix=7f8141296c75f2871e3d030f85c35692") if !equality.Semantic.DeepEqual(args, expectedArgs) { t.Errorf("Expect args %v, but got %v", expectedArgs, args) return } + assert.True(t, isDotAwsMounted(volumeMounts)) + assert.True(t, isDotAwsVolumePresent(volumes)) + +} + +func isDotAwsVolumePresent(volumes []corev1.Volume) bool { + for _, volume := range volumes { + if volume.Name == "dot-aws" { + return true + } + } + return false +} + +func isDotAwsMounted(mounts []corev1.VolumeMount) bool { + for _, mount := range mounts { + if mount.Name == "dot-aws" && mount.MountPath == "/.aws" { + return true + } + } + return false } func assertRegistrationDeployment(t *testing.T, actions []clienttesting.Action, verb, serverURL, clusterName string, replica int32, awsAuth bool) { @@ -444,7 +471,10 @@ func assertRegistrationDeployment(t *testing.T, actions []clienttesting.Action, expectedArgs = append(expectedArgs, "--kube-api-qps=10", "--kube-api-burst=60") if awsAuth { - expectedArgs = append(expectedArgs, "--registration-auth=awsirsa", "--hub-cluster-arn=arneks:us-west-2:123456789012:cluster/hub-cluster1") + expectedArgs = append(expectedArgs, "--registration-auth=awsirsa", + "--hub-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1", + "--managed-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1", + "--managed-cluster-role-suffix=7f8141296c75f2871e3d030f85c35692") } if !equality.Semantic.DeepEqual(args, expectedArgs) { t.Errorf("Expect args %v, but got %v", expectedArgs, args) @@ -988,18 +1018,50 @@ func TestGetServersFromKlusterlet(t *testing.T) { } } +func TestAWSIrsaAuthInSingletonModeWithInvalidClusterArns(t *testing.T) { + klusterlet := newKlusterlet("klusterlet", "testns", "cluster1") + awsIrsaRegistrationDriver := operatorapiv1.RegistrationDriver{ + AuthType: AwsIrsaAuthType, + AwsIrsa: &operatorapiv1.AwsIrsa{ + HubClusterArn: "arn:aws:bks:us-west-2:123456789012:cluster/hub-cluster1", + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1", + }, + } + klusterlet.Spec.RegistrationConfiguration.RegistrationDriver = awsIrsaRegistrationDriver + klusterlet.Spec.DeployOption.Mode = operatorapiv1.InstallModeSingleton + hubSecret := newSecret(helpers.HubKubeConfig, "testns") + hubSecret.Data["kubeconfig"] = []byte("dummykubeconfig") + hubSecret.Data["cluster-name"] = []byte("cluster1") + objects := []runtime.Object{ + newNamespace("testns"), + newSecret(helpers.BootstrapHubKubeConfig, "testns"), + hubSecret, + } + + syncContext := testingcommon.NewFakeSyncContext(t, "klusterlet") + controller := newTestController(t, klusterlet, syncContext.Recorder(), nil, false, + objects...) + + err := controller.controller.sync(context.TODO(), syncContext) + if err != nil { + assert.Equal(t, err.Error(), "HubClusterArn arn:aws:bks:us-west-2:123456789012:cluster/hub-cluster1 is not well formed") + } + +} + func TestAWSIrsaAuthInSingletonMode(t *testing.T) { klusterlet := newKlusterlet("klusterlet", "testns", "cluster1") awsIrsaRegistrationDriver := operatorapiv1.RegistrationDriver{ AuthType: AwsIrsaAuthType, AwsIrsa: &operatorapiv1.AwsIrsa{ - HubClusterArn: "arneks:us-west-2:123456789012:cluster/hub-cluster1", + HubClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1", + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1", }, } klusterlet.Spec.RegistrationConfiguration.RegistrationDriver = awsIrsaRegistrationDriver klusterlet.Spec.DeployOption.Mode = operatorapiv1.InstallModeSingleton hubSecret := newSecret(helpers.HubKubeConfig, "testns") - hubSecret.Data["kubeconfig"] = []byte("dummuykubeconnfig") + hubSecret.Data["kubeconfig"] = []byte("dummykubeconfig") hubSecret.Data["cluster-name"] = []byte("cluster1") objects := []runtime.Object{ newNamespace("testns"), @@ -1024,7 +1086,8 @@ func TestAWSIrsaAuthInNonSingletonMode(t *testing.T) { awsIrsaRegistrationDriver := operatorapiv1.RegistrationDriver{ AuthType: AwsIrsaAuthType, AwsIrsa: &operatorapiv1.AwsIrsa{ - HubClusterArn: "arneks:us-west-2:123456789012:cluster/hub-cluster1", + HubClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1", + ManagedClusterArn: "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1", }, } klusterlet.Spec.RegistrationConfiguration.RegistrationDriver = awsIrsaRegistrationDriver diff --git a/pkg/registration/helpers/helpers.go b/pkg/registration/helpers/helpers.go index 2fc8b5508..6156c46f0 100644 --- a/pkg/registration/helpers/helpers.go +++ b/pkg/registration/helpers/helpers.go @@ -3,7 +3,6 @@ package helpers import ( "embed" "net/url" - "regexp" "github.com/openshift/library-go/pkg/assets" "github.com/openshift/library-go/pkg/operator/resource/resourceapply" @@ -177,14 +176,3 @@ func IsCSRSupported(nativeClient kubernetes.Interface) (bool, bool, error) { } return v1CSRSupported, v1beta1CSRSupported, nil } - -// IsEksArnWellFormed checks if the EKS cluster ARN is well-formed -// Example of a well-formed ARN: arn:aws:eks:us-west-2:123456789012:cluster/my-cluster -func IsEksArnWellFormed(eksArn string) bool { - pattern := "^arn:aws:eks:([a-zA-Z0-9-]+):(\\d{12}):cluster/([a-zA-Z0-9-]+)$" - matched, err := regexp.MatchString(pattern, eksArn) - if err != nil { - return false - } - return matched -} diff --git a/pkg/registration/spoke/options.go b/pkg/registration/spoke/options.go index f9c0649de..6c75e24e4 100644 --- a/pkg/registration/spoke/options.go +++ b/pkg/registration/spoke/options.go @@ -38,7 +38,9 @@ type SpokeAgentOptions struct { ClientCertExpirationSeconds int32 ClusterAnnotations map[string]string RegistrationAuth string - EksHubClusterArn string + HubClusterArn string + ManagedClusterArn string + ManagedClusterRoleSuffix string } func NewSpokeAgentOptions() *SpokeAgentOptions { @@ -79,8 +81,12 @@ func (o *SpokeAgentOptions) AddFlags(fs *pflag.FlagSet) { //Consider grouping these flags for driverOption in a new Option struct and add the flags using function driverOptions.AddFlags(fs). fs.StringVar(&o.RegistrationAuth, "registration-auth", o.RegistrationAuth, "The type of authentication to use to authenticate with hub.") - fs.StringVar(&o.EksHubClusterArn, "hub-cluster-arn", o.EksHubClusterArn, + fs.StringVar(&o.HubClusterArn, "hub-cluster-arn", o.HubClusterArn, "The ARN of the EKS based hub cluster.") + fs.StringVar(&o.ManagedClusterArn, "managed-cluster-arn", o.ManagedClusterArn, + "The ARN of the EKS based managed cluster.") + fs.StringVar(&o.ManagedClusterRoleSuffix, "managed-cluster-role-suffix", o.ManagedClusterRoleSuffix, + "The suffix of the managed cluster IAM role.") } // Validate verifies the inputs. @@ -113,7 +119,7 @@ func (o *SpokeAgentOptions) Validate() error { return errors.New("client certificate expiration seconds must greater or qual to 3600") } - if (o.RegistrationAuth == AwsIrsaAuthType) && (o.EksHubClusterArn == "") { + if (o.RegistrationAuth == AwsIrsaAuthType) && (o.HubClusterArn == "") { return errors.New("EksHubClusterArn cannot be empty if RegistrationAuth is awsirsa") } diff --git a/pkg/registration/spoke/spokeagent.go b/pkg/registration/spoke/spokeagent.go index 5b9cfeeb9..3483956a0 100644 --- a/pkg/registration/spoke/spokeagent.go +++ b/pkg/registration/spoke/spokeagent.go @@ -31,7 +31,6 @@ import ( "open-cluster-management.io/ocm/pkg/common/helpers" commonoptions "open-cluster-management.io/ocm/pkg/common/options" "open-cluster-management.io/ocm/pkg/features" - registrationHelpers "open-cluster-management.io/ocm/pkg/registration/helpers" "open-cluster-management.io/ocm/pkg/registration/register" awsIrsa "open-cluster-management.io/ocm/pkg/registration/register/aws_irsa" "open-cluster-management.io/ocm/pkg/registration/register/csr" @@ -194,13 +193,14 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, var registerDriver register.RegisterDriver if o.registrationOption.RegistrationAuth == AwsIrsaAuthType { // TODO: may consider add additional validations - if o.registrationOption.EksHubClusterArn != "" && registrationHelpers.IsEksArnWellFormed(o.registrationOption.EksHubClusterArn) { + 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"] = "" //TODO: find arn from current context - o.registrationOption.ClusterAnnotations[operatorv1.ClusterAnnotationsKeyPrefix+"/managed-cluster-iam-role-suffix"] = "" //TODO: Add role suffix after RE-7249 + 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") @@ -324,7 +324,7 @@ func (o *SpokeAgentConfig) RunSpokeAgentWithSpokeInformers(ctx context.Context, var registrationAuthOption any if o.registrationOption.RegistrationAuth == AwsIrsaAuthType { - if o.registrationOption.EksHubClusterArn != "" && registrationHelpers.IsEksArnWellFormed(o.registrationOption.EksHubClusterArn) { + if o.registrationOption.HubClusterArn != "" { registrationAuthOption, err = registration.NewAWSOption( secretOption, bootstrapClusterInformerFactory.Cluster(), From efb1c79e8e4d664496027624abe5b2c8c74d6b48 Mon Sep 17 00:00:00 2001 From: Gaurav Jaswal Date: Fri, 22 Nov 2024 15:44:09 -0500 Subject: [PATCH 2/4] Adding integration tests for aws registration Signed-off-by: Erica Jin <132393634+EricaJ6@users.noreply.github.com> --- .../operator/klusterlet_singleton_aws_test.go | 144 ++++++++++++++++++ .../operator/klusterlet_singleton_test.go | 36 ++++- 2 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 test/integration/operator/klusterlet_singleton_aws_test.go diff --git a/test/integration/operator/klusterlet_singleton_aws_test.go b/test/integration/operator/klusterlet_singleton_aws_test.go new file mode 100644 index 000000000..f279459cf --- /dev/null +++ b/test/integration/operator/klusterlet_singleton_aws_test.go @@ -0,0 +1,144 @@ +package operator + +import ( + "context" + "fmt" + "strings" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/rand" + + operatorapiv1 "open-cluster-management.io/api/operator/v1" + + "open-cluster-management.io/ocm/pkg/operator/helpers" + "open-cluster-management.io/ocm/test/integration/util" +) + +var hubClusterArn string = "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" +var managedClusterArn string = "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1" +var managedClusterRoleSuffix string = "7f8141296c75f2871e3d030f85c35692" +var prerequisiteSpokeRoleArn string = "arn:aws:iam::123456789012:role/ocm-managed-cluster-" + managedClusterRoleSuffix +var irsaAnnotationKey string = "eks.amazonaws.com/role-arn" + +var _ = ginkgo.Describe("Klusterlet Singleton mode with aws auth", func() { + var cancel context.CancelFunc + var klusterlet *operatorapiv1.Klusterlet + var agentNamespace string + var deploymentName string + var saName string + + ginkgo.BeforeEach(func() { + var ctx context.Context + klusterlet = &operatorapiv1.Klusterlet{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("klusterlet-%s", rand.String(6)), + }, + Spec: operatorapiv1.KlusterletSpec{ + Namespace: fmt.Sprintf("%s-aws", helpers.KlusterletDefaultNamespace), + ImagePullSpec: "quay.io/open-cluster-management/registration-operator", + ExternalServerURLs: []operatorapiv1.ServerURL{ + { + URL: "https://localhost", + }, + }, + ClusterName: "testcluster", + DeployOption: operatorapiv1.KlusterletDeployOption{ + Mode: operatorapiv1.InstallModeSingleton, + }, + RegistrationConfiguration: &operatorapiv1.RegistrationConfiguration{ + RegistrationDriver: operatorapiv1.RegistrationDriver{ + AuthType: "awsirsa", + AwsIrsa: &operatorapiv1.AwsIrsa{ + HubClusterArn: hubClusterArn, + ManagedClusterArn: managedClusterArn, + }, + }, + }, + }, + } + agentNamespace = helpers.AgentNamespace(klusterlet) + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: agentNamespace, + }, + } + _, err := kubeClient.CoreV1().Namespaces().Create(context.Background(), ns, metav1.CreateOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + ctx, cancel = context.WithCancel(context.Background()) + go startKlusterletOperator(ctx) + }) + + ginkgo.AfterEach(func() { + err := kubeClient.CoreV1().Namespaces().Delete(context.Background(), agentNamespace, metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + if cancel != nil { + cancel() + } + }) + + ginkgo.Context("Deploy and clean klusterlet component with aws auth", func() { + ginkgo.BeforeEach(func() { + deploymentName = fmt.Sprintf("%s-agent", klusterlet.Name) + saName = fmt.Sprintf("%s-work-sa", klusterlet.Name) + }) + + ginkgo.AfterEach(func() { + gomega.Expect(operatorClient.OperatorV1().Klusterlets().Delete(context.Background(), klusterlet.Name, metav1.DeleteOptions{})).To(gomega.BeNil()) + }) + + ginkgo.It("should have expected resource created successfully when registered with aws auth", func() { + _, err := operatorClient.OperatorV1().Klusterlets().Create(context.Background(), klusterlet, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // Check service account + gomega.Eventually(func() bool { + sa, err := kubeClient.CoreV1().ServiceAccounts(agentNamespace).Get(context.Background(), saName, metav1.GetOptions{}) + if err != nil { + return false + } + return sa.ObjectMeta.Annotations[irsaAnnotationKey] == prerequisiteSpokeRoleArn + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + // Check deployment + gomega.Eventually(func() bool { + deployment, err := kubeClient.AppsV1().Deployments(agentNamespace).Get(context.Background(), deploymentName, metav1.GetOptions{}) + if err != nil { + return false + } + + isRegistrationAuthPresent := false + isManagedClusterArnPresent := false + isManagedClusterRoleSuffixPresent := false + for _, arg := range deployment.Spec.Template.Spec.Containers[0].Args { + if strings.Contains(arg, "--registration-auth=awsirsa") { + isRegistrationAuthPresent = true + } + if strings.Contains(arg, "--managed-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1") { + isManagedClusterArnPresent = true + } + if strings.Contains(arg, "--managed-cluster-role-suffix="+managedClusterRoleSuffix) { + isManagedClusterRoleSuffixPresent = true + } + } + allCommandLineOptionsPresent := isRegistrationAuthPresent && isManagedClusterArnPresent && isManagedClusterRoleSuffixPresent + + isDotAwsMounted := false + for _, volumeMount := range deployment.Spec.Template.Spec.Containers[0].VolumeMounts { + if volumeMount.Name == "dot-aws" && volumeMount.MountPath == "/.aws" { + isDotAwsMounted = true + } + } + + awsCliSpecificVolumesMounted := isDotAwsMounted + return allCommandLineOptionsPresent && awsCliSpecificVolumesMounted + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "Applied", "KlusterletApplied", metav1.ConditionTrue) + }) + }) +}) diff --git a/test/integration/operator/klusterlet_singleton_test.go b/test/integration/operator/klusterlet_singleton_test.go index cdcc61b47..539a3c001 100644 --- a/test/integration/operator/klusterlet_singleton_test.go +++ b/test/integration/operator/klusterlet_singleton_test.go @@ -3,6 +3,7 @@ package operator import ( "context" "fmt" + "strings" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" @@ -185,18 +186,45 @@ var _ = ginkgo.Describe("Klusterlet Singleton mode", func() { // Check service account gomega.Eventually(func() bool { - if _, err := kubeClient.CoreV1().ServiceAccounts(agentNamespace).Get(context.Background(), saName, metav1.GetOptions{}); err != nil { + sa, err := kubeClient.CoreV1().ServiceAccounts(agentNamespace).Get(context.Background(), saName, metav1.GetOptions{}) + if err != nil { return false } - return true + return sa.ObjectMeta.Annotations[irsaAnnotationKey] != prerequisiteSpokeRoleArn }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) // Check deployment gomega.Eventually(func() bool { - if _, err := kubeClient.AppsV1().Deployments(agentNamespace).Get(context.Background(), deploymentName, metav1.GetOptions{}); err != nil { + deployment, err := kubeClient.AppsV1().Deployments(agentNamespace).Get(context.Background(), deploymentName, metav1.GetOptions{}) + if err != nil { return false } - return true + + isRegistrationAuthPresent := false + isManagedClusterArnPresent := false + isManagedClusterRoleSuffixPresent := false + for _, arg := range deployment.Spec.Template.Spec.Containers[0].Args { + if strings.Contains(arg, "--registration-auth=awsirsa") { + isRegistrationAuthPresent = true + } + if strings.Contains(arg, "--managed-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1") { + isManagedClusterArnPresent = true + } + if strings.Contains(arg, "--managed-cluster-role-suffix="+managedClusterRoleSuffix) { + isManagedClusterRoleSuffixPresent = true + } + } + anyCommandLineOptionsPresent := isRegistrationAuthPresent || isManagedClusterArnPresent || isManagedClusterRoleSuffixPresent + + isDotAwsMounted := false + for _, volumeMount := range deployment.Spec.Template.Spec.Containers[0].VolumeMounts { + if volumeMount.Name == "dot-aws" && volumeMount.MountPath == "/.aws" { + isDotAwsMounted = true + } + } + + awsCliSpecificVolumesMounted := isDotAwsMounted + return !(anyCommandLineOptionsPresent || awsCliSpecificVolumesMounted) }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) // Check addon namespace From 2ab40699b031a47812bff33b4ea4a08f8690f48c Mon Sep 17 00:00:00 2001 From: Gaurav Jaswal Date: Fri, 22 Nov 2024 17:50:46 -0500 Subject: [PATCH 3/4] Adding more integration tests Signed-off-by: Erica Jin <132393634+EricaJ6@users.noreply.github.com> --- ...lusterlet-registration-serviceaccount.yaml | 4 +- .../klusterlet-work-serviceaccount.yaml | 4 +- .../klusterlet_controller.go | 5 +- .../operator/klusterlet_aws_test.go | 174 ++++++++++++++++++ .../operator/klusterlet_singleton_aws_test.go | 45 +---- .../operator/klusterlet_singleton_test.go | 29 +-- test/integration/operator/klusterlet_test.go | 13 ++ test/integration/util/aws.go | 43 +++++ 8 files changed, 245 insertions(+), 72 deletions(-) create mode 100644 test/integration/operator/klusterlet_aws_test.go create mode 100644 test/integration/util/aws.go diff --git a/manifests/klusterlet/managed/klusterlet-registration-serviceaccount.yaml b/manifests/klusterlet/managed/klusterlet-registration-serviceaccount.yaml index 431e8ef07..1d2ac4c56 100644 --- a/manifests/klusterlet/managed/klusterlet-registration-serviceaccount.yaml +++ b/manifests/klusterlet/managed/klusterlet-registration-serviceaccount.yaml @@ -9,9 +9,9 @@ metadata: "{{ $key }}": "{{ $value }}" {{ end }} {{ end }} - {{ if and .ManagedClusterRoleArn (eq .RegistrationDriver.AuthType "awsirsa") }} + {{ if and .ManagedClusterRoleArn (eq .RegistrationDriver.AuthType "awsirsa") }} annotations: eks.amazonaws.com/role-arn: {{ .ManagedClusterRoleArn }} - {{ end }} + {{ end }} imagePullSecrets: - name: open-cluster-management-image-pull-credentials diff --git a/manifests/klusterlet/managed/klusterlet-work-serviceaccount.yaml b/manifests/klusterlet/managed/klusterlet-work-serviceaccount.yaml index b16ddcf20..951f88642 100644 --- a/manifests/klusterlet/managed/klusterlet-work-serviceaccount.yaml +++ b/manifests/klusterlet/managed/klusterlet-work-serviceaccount.yaml @@ -9,9 +9,9 @@ metadata: "{{ $key }}": "{{ $value }}" {{ end }} {{ end }} - {{ if and .ManagedClusterRoleArn (eq .RegistrationDriver.AuthType "awsirsa") }} + {{ if and .ManagedClusterRoleArn (eq .RegistrationDriver.AuthType "awsirsa") }} annotations: eks.amazonaws.com/role-arn: {{ .ManagedClusterRoleArn }} - {{ end }} + {{ end }} imagePullSecrets: - name: open-cluster-management-image-pull-credentials diff --git a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go index 3ea388a95..f2ac0c94c 100644 --- a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go +++ b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go @@ -4,7 +4,6 @@ import ( "context" "crypto/md5" // #nosec G501 "encoding/hex" - er "errors" "fmt" "strings" "time" @@ -344,13 +343,13 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto if !commonhelpers.IsEksArnWellFormed(hubClusterArn) { errorMsg := fmt.Sprintf("HubClusterArn %s is not well formed", hubClusterArn) klog.Errorf(errorMsg) - return er.New(errorMsg) + return fmt.Errorf(errorMsg) } if !commonhelpers.IsEksArnWellFormed(managedClusterArn) { errorMsg := fmt.Sprintf("ManagedClusterArn %s is not well formed", managedClusterArn) klog.Errorf(errorMsg) - return er.New(errorMsg) + return fmt.Errorf(errorMsg) } config.RegistrationDriver = RegistrationDriver{ diff --git a/test/integration/operator/klusterlet_aws_test.go b/test/integration/operator/klusterlet_aws_test.go new file mode 100644 index 000000000..41b41b65d --- /dev/null +++ b/test/integration/operator/klusterlet_aws_test.go @@ -0,0 +1,174 @@ +package operator + +import ( + "context" + "fmt" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/rand" + + operatorapiv1 "open-cluster-management.io/api/operator/v1" + + "open-cluster-management.io/ocm/pkg/operator/helpers" + "open-cluster-management.io/ocm/pkg/registration/spoke" + "open-cluster-management.io/ocm/test/integration/util" +) + +var _ = ginkgo.Describe("Klusterlet using aws auth", func() { + var cancel context.CancelFunc + var klusterlet *operatorapiv1.Klusterlet + var hubKubeConfigSecret *corev1.Secret + var klusterletNamespace string + var registrationDeploymentName string + var registrationSAName string + var workDeploymentName string + var workSAName string + var agentLabelSelector string + + ginkgo.BeforeEach(func() { + var ctx context.Context + + klusterletNamespace = fmt.Sprintf("open-cluster-management-aws-%s", rand.String(6)) + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: klusterletNamespace, + }, + } + _, err := kubeClient.CoreV1().Namespaces().Create(context.Background(), ns, metav1.CreateOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + klusterlet = &operatorapiv1.Klusterlet{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("klusterlet-%s", rand.String(6)), + Labels: map[string]string{"test": "123", "component": "klusterlet", "123": "312"}, + }, + Spec: operatorapiv1.KlusterletSpec{ + RegistrationImagePullSpec: "quay.io/open-cluster-management/registration", + WorkImagePullSpec: "quay.io/open-cluster-management/work", + ExternalServerURLs: []operatorapiv1.ServerURL{ + { + URL: "https://localhost", + }, + }, + ClusterName: "testcluster", + Namespace: klusterletNamespace, + RegistrationConfiguration: &operatorapiv1.RegistrationConfiguration{ + RegistrationDriver: operatorapiv1.RegistrationDriver{ + AuthType: spoke.AwsIrsaAuthType, + AwsIrsa: &operatorapiv1.AwsIrsa{ + HubClusterArn: util.HubClusterArn, + ManagedClusterArn: util.ManagedClusterArn, + }, + }, + }, + }, + } + + agentLabelSelector = metav1.FormatLabelSelector(&metav1.LabelSelector{ + MatchLabels: helpers.GetKlusterletAgentLabels(klusterlet), + }) + + hubKubeConfigSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: helpers.HubKubeConfig, + Namespace: klusterletNamespace, + }, + Data: map[string][]byte{ + "placeholder": []byte("placeholder"), + }, + } + _, err = kubeClient.CoreV1().Secrets(klusterletNamespace).Create(context.Background(), hubKubeConfigSecret, metav1.CreateOptions{}) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + + ctx, cancel = context.WithCancel(context.Background()) + go startKlusterletOperator(ctx) + }) + + ginkgo.AfterEach(func() { + err := kubeClient.CoreV1().Namespaces().Delete(context.Background(), klusterletNamespace, metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + if cancel != nil { + cancel() + } + }) + + ginkgo.Context("Deploy and clean klusterlet component using aws auth", func() { + ginkgo.BeforeEach(func() { + registrationDeploymentName = fmt.Sprintf("%s-registration-agent", klusterlet.Name) + workDeploymentName = fmt.Sprintf("%s-work-agent", klusterlet.Name) + + registrationSAName = fmt.Sprintf("%s-registration-sa", klusterlet.Name) + workSAName = fmt.Sprintf("%s-work-sa", klusterlet.Name) + }) + + ginkgo.AfterEach(func() { + gomega.Expect(operatorClient.OperatorV1().Klusterlets().Delete(context.Background(), klusterlet.Name, metav1.DeleteOptions{})).To(gomega.BeNil()) + }) + + ginkgo.It("should have expected resource created successfully using aws auth", func() { + _, err := operatorClient.OperatorV1().Klusterlets().Create(context.Background(), klusterlet, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // Check service account + gomega.Eventually(func() bool { + serviceaccouts, err := kubeClient.CoreV1().ServiceAccounts(klusterletNamespace).List(context.Background(), + metav1.ListOptions{LabelSelector: agentLabelSelector}) + if err != nil { + return false + } + if len(serviceaccouts.Items) != 2 { + return false + } + for _, serviceAccount := range serviceaccouts.Items { + if serviceAccount.GetName() != registrationSAName && + serviceAccount.GetName() != workSAName { + return false + } + if serviceAccount.ObjectMeta.Annotations[util.IrsaAnnotationKey] != util.PrerequisiteSpokeRoleArn { + return false + } + } + return true + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + // Check deployment + gomega.Eventually(func() bool { + deployments, err := kubeClient.AppsV1().Deployments(klusterletNamespace).List(context.Background(), + metav1.ListOptions{LabelSelector: agentLabelSelector}) + if err != nil { + return false + } + if len(deployments.Items) != 2 { + return false + } + + for _, deployment := range deployments.Items { + if deployment.GetName() != registrationDeploymentName && + deployment.GetName() != workDeploymentName { + return false + } + if deployment.GetName() == registrationDeploymentName { + if !util.AllCommandLineOptionsPresent(deployment) || !util.AwsCliSpecificVolumesMounted(deployment) { + return false + } + } + if deployment.GetName() == workDeploymentName { + if !util.AwsCliSpecificVolumesMounted(deployment) { + return false + } + } + } + + return true + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) + + util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "Applied", "KlusterletApplied", metav1.ConditionTrue) + }) + + }) + +}) diff --git a/test/integration/operator/klusterlet_singleton_aws_test.go b/test/integration/operator/klusterlet_singleton_aws_test.go index f279459cf..a94d5ee0d 100644 --- a/test/integration/operator/klusterlet_singleton_aws_test.go +++ b/test/integration/operator/klusterlet_singleton_aws_test.go @@ -3,7 +3,6 @@ package operator import ( "context" "fmt" - "strings" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" @@ -14,15 +13,10 @@ import ( operatorapiv1 "open-cluster-management.io/api/operator/v1" "open-cluster-management.io/ocm/pkg/operator/helpers" + "open-cluster-management.io/ocm/pkg/registration/spoke" "open-cluster-management.io/ocm/test/integration/util" ) -var hubClusterArn string = "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" -var managedClusterArn string = "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1" -var managedClusterRoleSuffix string = "7f8141296c75f2871e3d030f85c35692" -var prerequisiteSpokeRoleArn string = "arn:aws:iam::123456789012:role/ocm-managed-cluster-" + managedClusterRoleSuffix -var irsaAnnotationKey string = "eks.amazonaws.com/role-arn" - var _ = ginkgo.Describe("Klusterlet Singleton mode with aws auth", func() { var cancel context.CancelFunc var klusterlet *operatorapiv1.Klusterlet @@ -37,7 +31,7 @@ var _ = ginkgo.Describe("Klusterlet Singleton mode with aws auth", func() { Name: fmt.Sprintf("klusterlet-%s", rand.String(6)), }, Spec: operatorapiv1.KlusterletSpec{ - Namespace: fmt.Sprintf("%s-aws", helpers.KlusterletDefaultNamespace), + Namespace: fmt.Sprintf("%s-singleton-aws", helpers.KlusterletDefaultNamespace), ImagePullSpec: "quay.io/open-cluster-management/registration-operator", ExternalServerURLs: []operatorapiv1.ServerURL{ { @@ -50,10 +44,10 @@ var _ = ginkgo.Describe("Klusterlet Singleton mode with aws auth", func() { }, RegistrationConfiguration: &operatorapiv1.RegistrationConfiguration{ RegistrationDriver: operatorapiv1.RegistrationDriver{ - AuthType: "awsirsa", + AuthType: spoke.AwsIrsaAuthType, AwsIrsa: &operatorapiv1.AwsIrsa{ - HubClusterArn: hubClusterArn, - ManagedClusterArn: managedClusterArn, + HubClusterArn: util.HubClusterArn, + ManagedClusterArn: util.ManagedClusterArn, }, }, }, @@ -101,7 +95,7 @@ var _ = ginkgo.Describe("Klusterlet Singleton mode with aws auth", func() { if err != nil { return false } - return sa.ObjectMeta.Annotations[irsaAnnotationKey] == prerequisiteSpokeRoleArn + return sa.ObjectMeta.Annotations[util.IrsaAnnotationKey] == util.PrerequisiteSpokeRoleArn }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) // Check deployment @@ -110,32 +104,7 @@ var _ = ginkgo.Describe("Klusterlet Singleton mode with aws auth", func() { if err != nil { return false } - - isRegistrationAuthPresent := false - isManagedClusterArnPresent := false - isManagedClusterRoleSuffixPresent := false - for _, arg := range deployment.Spec.Template.Spec.Containers[0].Args { - if strings.Contains(arg, "--registration-auth=awsirsa") { - isRegistrationAuthPresent = true - } - if strings.Contains(arg, "--managed-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1") { - isManagedClusterArnPresent = true - } - if strings.Contains(arg, "--managed-cluster-role-suffix="+managedClusterRoleSuffix) { - isManagedClusterRoleSuffixPresent = true - } - } - allCommandLineOptionsPresent := isRegistrationAuthPresent && isManagedClusterArnPresent && isManagedClusterRoleSuffixPresent - - isDotAwsMounted := false - for _, volumeMount := range deployment.Spec.Template.Spec.Containers[0].VolumeMounts { - if volumeMount.Name == "dot-aws" && volumeMount.MountPath == "/.aws" { - isDotAwsMounted = true - } - } - - awsCliSpecificVolumesMounted := isDotAwsMounted - return allCommandLineOptionsPresent && awsCliSpecificVolumesMounted + return util.AllCommandLineOptionsPresent(*deployment) && util.AwsCliSpecificVolumesMounted(*deployment) }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) util.AssertKlusterletCondition(klusterlet.Name, operatorClient, "Applied", "KlusterletApplied", metav1.ConditionTrue) diff --git a/test/integration/operator/klusterlet_singleton_test.go b/test/integration/operator/klusterlet_singleton_test.go index 539a3c001..4baf46027 100644 --- a/test/integration/operator/klusterlet_singleton_test.go +++ b/test/integration/operator/klusterlet_singleton_test.go @@ -3,7 +3,6 @@ package operator import ( "context" "fmt" - "strings" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" @@ -190,7 +189,7 @@ var _ = ginkgo.Describe("Klusterlet Singleton mode", func() { if err != nil { return false } - return sa.ObjectMeta.Annotations[irsaAnnotationKey] != prerequisiteSpokeRoleArn + return sa.ObjectMeta.Annotations[util.IrsaAnnotationKey] != util.PrerequisiteSpokeRoleArn }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) // Check deployment @@ -200,31 +199,7 @@ var _ = ginkgo.Describe("Klusterlet Singleton mode", func() { return false } - isRegistrationAuthPresent := false - isManagedClusterArnPresent := false - isManagedClusterRoleSuffixPresent := false - for _, arg := range deployment.Spec.Template.Spec.Containers[0].Args { - if strings.Contains(arg, "--registration-auth=awsirsa") { - isRegistrationAuthPresent = true - } - if strings.Contains(arg, "--managed-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1") { - isManagedClusterArnPresent = true - } - if strings.Contains(arg, "--managed-cluster-role-suffix="+managedClusterRoleSuffix) { - isManagedClusterRoleSuffixPresent = true - } - } - anyCommandLineOptionsPresent := isRegistrationAuthPresent || isManagedClusterArnPresent || isManagedClusterRoleSuffixPresent - - isDotAwsMounted := false - for _, volumeMount := range deployment.Spec.Template.Spec.Containers[0].VolumeMounts { - if volumeMount.Name == "dot-aws" && volumeMount.MountPath == "/.aws" { - isDotAwsMounted = true - } - } - - awsCliSpecificVolumesMounted := isDotAwsMounted - return !(anyCommandLineOptionsPresent || awsCliSpecificVolumesMounted) + return !util.AllCommandLineOptionsPresent(*deployment) && !util.AwsCliSpecificVolumesMounted(*deployment) }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) // Check addon namespace diff --git a/test/integration/operator/klusterlet_test.go b/test/integration/operator/klusterlet_test.go index c467a01f0..914a51dfe 100644 --- a/test/integration/operator/klusterlet_test.go +++ b/test/integration/operator/klusterlet_test.go @@ -270,6 +270,9 @@ var _ = ginkgo.Describe("Klusterlet", func() { serviceAccount.GetName() != workSAName { return false } + if serviceAccount.ObjectMeta.Annotations[util.IrsaAnnotationKey] == util.PrerequisiteSpokeRoleArn { + return false + } } return true }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) @@ -289,6 +292,16 @@ var _ = ginkgo.Describe("Klusterlet", func() { deployment.GetName() != workDeploymentName { return false } + if deployment.GetName() == registrationDeploymentName { + if util.AllCommandLineOptionsPresent(deployment) || util.AwsCliSpecificVolumesMounted(deployment) { + return false + } + } + if deployment.GetName() == workDeploymentName { + if util.AwsCliSpecificVolumesMounted(deployment) { + return false + } + } } return true }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) diff --git a/test/integration/util/aws.go b/test/integration/util/aws.go new file mode 100644 index 000000000..498030fc6 --- /dev/null +++ b/test/integration/util/aws.go @@ -0,0 +1,43 @@ +package util + +import ( + "strings" + + v1 "k8s.io/api/apps/v1" +) + +const ( + HubClusterArn = "arn:aws:eks:us-west-2:123456789012:cluster/hub-cluster1" + ManagedClusterArn = "arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1" + ManagedClusterRoleSuffix = "7f8141296c75f2871e3d030f85c35692" + PrerequisiteSpokeRoleArn = "arn:aws:iam::123456789012:role/ocm-managed-cluster-" + ManagedClusterRoleSuffix + IrsaAnnotationKey = "eks.amazonaws.com/role-arn" +) + +func AwsCliSpecificVolumesMounted(deployment v1.Deployment) bool { + isDotAwsMounted := false + for _, volumeMount := range deployment.Spec.Template.Spec.Containers[0].VolumeMounts { + if volumeMount.Name == "dot-aws" && volumeMount.MountPath == "/.aws" { + isDotAwsMounted = true + } + } + return isDotAwsMounted +} + +func AllCommandLineOptionsPresent(deployment v1.Deployment) bool { + isRegistrationAuthPresent := false + isManagedClusterArnPresent := false + isManagedClusterRoleSuffixPresent := false + for _, arg := range deployment.Spec.Template.Spec.Containers[0].Args { + if strings.Contains(arg, "--registration-auth=awsirsa") { + isRegistrationAuthPresent = true + } + if strings.Contains(arg, "--managed-cluster-arn=arn:aws:eks:us-west-2:123456789012:cluster/managed-cluster1") { + isManagedClusterArnPresent = true + } + if strings.Contains(arg, "--managed-cluster-role-suffix="+ManagedClusterRoleSuffix) { + isManagedClusterRoleSuffixPresent = true + } + } + return isRegistrationAuthPresent && isManagedClusterArnPresent && isManagedClusterRoleSuffixPresent +} From d4de9a57798d0093e88018d5028903ebcb3dea40 Mon Sep 17 00:00:00 2001 From: Gaurav Jaswal Date: Mon, 25 Nov 2024 15:50:33 -0500 Subject: [PATCH 4/4] Addressing review comments Signed-off-by: Erica Jin <132393634+EricaJ6@users.noreply.github.com> --- pkg/common/helpers/aws.go | 16 ----- .../klusterlet_controller.go | 65 +++++++++---------- .../operator/klusterlet_singleton_test.go | 3 +- 3 files changed, 32 insertions(+), 52 deletions(-) delete mode 100644 pkg/common/helpers/aws.go diff --git a/pkg/common/helpers/aws.go b/pkg/common/helpers/aws.go deleted file mode 100644 index c9e789de8..000000000 --- a/pkg/common/helpers/aws.go +++ /dev/null @@ -1,16 +0,0 @@ -package helpers - -import ( - "regexp" -) - -// IsEksArnWellFormed checks if the EKS cluster ARN is well-formed -// Example of a well-formed ARN: arn:aws:eks:us-west-2:123456789012:cluster/my-cluster -func IsEksArnWellFormed(eksArn string) bool { - pattern := "^arn:aws:eks:([a-zA-Z0-9-]+):(\\d{12}):cluster/([a-zA-Z0-9-]+)$" - matched, err := regexp.MatchString(pattern, eksArn) - if err != nil { - return false - } - return matched -} diff --git a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go index f2ac0c94c..850c249e8 100644 --- a/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go +++ b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go @@ -125,6 +125,26 @@ type RegistrationDriver struct { AwsIrsa *AwsIrsa } +type ManagedClusterIamRole struct { + AwsIrsa *AwsIrsa +} + +func (managedClusterIamRole *ManagedClusterIamRole) arn() string { + managedClusterAccountId, _ := getAwsAccountIdAndClusterName(managedClusterIamRole.AwsIrsa.ManagedClusterArn) + md5HashUniqueIdentifier := managedClusterIamRole.md5HashSuffix() + + //arn:aws:iam:::role/ocm-managed-cluster- + return "arn:aws:iam::" + managedClusterAccountId + ":role/ocm-managed-cluster-" + md5HashUniqueIdentifier +} + +func (managedClusterIamRole *ManagedClusterIamRole) md5HashSuffix() string { + hubClusterAccountId, hubClusterName := getAwsAccountIdAndClusterName(managedClusterIamRole.AwsIrsa.HubClusterArn) + managedClusterAccountId, managedClusterName := getAwsAccountIdAndClusterName(managedClusterIamRole.AwsIrsa.ManagedClusterArn) + + hash := md5.Sum([]byte(strings.Join([]string{hubClusterAccountId, hubClusterName, managedClusterAccountId, managedClusterName}, "#"))) // #nosec G401 + return hex.EncodeToString(hash[:]) +} + // klusterletConfig is used to render the template of hub manifests type klusterletConfig struct { KlusterletName string @@ -340,18 +360,6 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto hubClusterArn := klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.HubClusterArn managedClusterArn := klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.ManagedClusterArn - if !commonhelpers.IsEksArnWellFormed(hubClusterArn) { - errorMsg := fmt.Sprintf("HubClusterArn %s is not well formed", hubClusterArn) - klog.Errorf(errorMsg) - return fmt.Errorf(errorMsg) - } - - if !commonhelpers.IsEksArnWellFormed(managedClusterArn) { - errorMsg := fmt.Sprintf("ManagedClusterArn %s is not well formed", managedClusterArn) - klog.Errorf(errorMsg) - return fmt.Errorf(errorMsg) - } - config.RegistrationDriver = RegistrationDriver{ AuthType: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType, AwsIrsa: &AwsIrsa{ @@ -359,9 +367,11 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto ManagedClusterArn: managedClusterArn, }, } - managedClusterRoleArn, managedClusterRoleSuffix := n.generateManagedRoleArnAndSuffix(klusterlet) - config.ManagedClusterRoleArn = managedClusterRoleArn - config.ManagedClusterRoleSuffix = managedClusterRoleSuffix + managedClusterIamRole := ManagedClusterIamRole{ + AwsIrsa: config.RegistrationDriver.AwsIrsa, + } + config.ManagedClusterRoleArn = managedClusterIamRole.arn() + config.ManagedClusterRoleSuffix = managedClusterIamRole.md5HashSuffix() } else { config.RegistrationDriver = RegistrationDriver{ AuthType: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType, @@ -460,23 +470,6 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto return utilerrors.NewAggregate(errs) } -func (n *klusterletController) generateManagedRoleArnAndSuffix(klusterlet *operatorapiv1.Klusterlet) (string, string) { - hubClusterArn := klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.HubClusterArn - managedClusterArn := klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.ManagedClusterArn - - hubClusterStringParts := strings.Split(hubClusterArn, ":") - - managedClusterStringParts := strings.Split(managedClusterArn, ":") - hubClusterName := strings.Split(hubClusterStringParts[5], "/")[1] - hubClusterAccountId := hubClusterStringParts[4] - managedClusterName := strings.Split(managedClusterStringParts[5], "/")[1] - managedClusterAccountId := managedClusterStringParts[4] - md5HashUniqueIdentifier := generateMd5HashUniqueIdentifier(hubClusterAccountId, hubClusterName, managedClusterAccountId, managedClusterName) - //arn:aws:iam:::role/ocm-managed-cluster- - managedClusterRoleArn := "arn:aws:iam::" + managedClusterAccountId + ":role/ocm-managed-cluster-" + md5HashUniqueIdentifier - return managedClusterRoleArn, md5HashUniqueIdentifier -} - // TODO also read CABundle from ExternalServerURLs and set into registration deployment func getServersFromKlusterlet(klusterlet *operatorapiv1.Klusterlet) string { if klusterlet.Spec.ExternalServerURLs == nil { @@ -581,7 +574,9 @@ func serviceAccountName(suffix string, klusterlet *operatorapiv1.Klusterlet) str return fmt.Sprintf("%s-%s", klusterlet.Name, suffix) } -func generateMd5HashUniqueIdentifier(hubClusterAccountId string, hubClusterName string, managedClusterAccountId string, managedClusterName string) string { - hash := md5.Sum([]byte(hubClusterAccountId + "#" + hubClusterName + "#" + managedClusterAccountId + "#" + managedClusterName)) // #nosec G401 - return hex.EncodeToString(hash[:]) +func getAwsAccountIdAndClusterName(clusterArn string) (string, string) { + clusterStringParts := strings.Split(clusterArn, ":") + clusterName := strings.Split(clusterStringParts[5], "/")[1] + awsAccountId := clusterStringParts[4] + return awsAccountId, clusterName } diff --git a/test/integration/operator/klusterlet_singleton_test.go b/test/integration/operator/klusterlet_singleton_test.go index 4baf46027..63c15efc2 100644 --- a/test/integration/operator/klusterlet_singleton_test.go +++ b/test/integration/operator/klusterlet_singleton_test.go @@ -189,7 +189,8 @@ var _ = ginkgo.Describe("Klusterlet Singleton mode", func() { if err != nil { return false } - return sa.ObjectMeta.Annotations[util.IrsaAnnotationKey] != util.PrerequisiteSpokeRoleArn + _, present := sa.ObjectMeta.Annotations[util.IrsaAnnotationKey] + return !present }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) // Check deployment