✨ Starting aws registration by spoke by assuming IAM role on startup …

…and adding annotations to ManagedCluster CR (#714) * Starting aws registration by spoke by assuming IAM role on startup and adding annotations to ManagedCluster CR Signed-off-by: Erica Jin <[email protected]> * Adding integration tests for aws registration Signed-off-by: Erica Jin <[email protected]> * Adding more integration tests Signed-off-by: Erica Jin <[email protected]> * Addressing review comments Signed-off-by: Erica Jin <[email protected]> --------- Signed-off-by: Erica Jin <[email protected]>
open-cluster-management-io · Nov 26, 2024 · 93db6de · 93db6de
1 parent bb5f6ef
commit 93db6de
Show file tree

Hide file tree

Showing 15 changed files with 523 additions and 35 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go b/pkg/operator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller.go
@@ -2,6 +2,8 @@ package klusterletcontroller
 
 import (
 	"context"
+	"crypto/md5" // #nosec G501
+	"encoding/hex"
 	"fmt"
 	"strings"
 	"time"
@@ -114,14 +116,35 @@ func NewKlusterletController(
 }
 
 type AwsIrsa struct {
-	HubClusterArn string
+	HubClusterArn     string
+	ManagedClusterArn string
 }
 
 type RegistrationDriver struct {
 	AuthType string
 	AwsIrsa  *AwsIrsa
 }
 
+type ManagedClusterIamRole struct {
+	AwsIrsa *AwsIrsa
+}
+
+func (managedClusterIamRole *ManagedClusterIamRole) arn() string {
+	managedClusterAccountId, _ := getAwsAccountIdAndClusterName(managedClusterIamRole.AwsIrsa.ManagedClusterArn)
+	md5HashUniqueIdentifier := managedClusterIamRole.md5HashSuffix()
+
+	//arn:aws:iam::<managed-cluster-account-id>:role/ocm-managed-cluster-<md5-hash-unique-identifier>
+	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
@@ -187,6 +210,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 +356,22 @@ 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
+
 			config.RegistrationDriver = RegistrationDriver{
 				AuthType: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType,
 				AwsIrsa: &AwsIrsa{
-					HubClusterArn: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.HubClusterArn,
+					HubClusterArn:     hubClusterArn,
+					ManagedClusterArn: managedClusterArn,
 				},
 			}
+			managedClusterIamRole := ManagedClusterIamRole{
+				AwsIrsa: config.RegistrationDriver.AwsIrsa,
+			}
+			config.ManagedClusterRoleArn = managedClusterIamRole.arn()
+			config.ManagedClusterRoleSuffix = managedClusterIamRole.md5HashSuffix()
 		} else {
 			config.RegistrationDriver = RegistrationDriver{
 				AuthType: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType,
@@ -536,3 +573,10 @@ func serviceAccountName(suffix string, klusterlet *operatorapiv1.Klusterlet) str
 	}
 	return fmt.Sprintf("%s-%s", klusterlet.Name, suffix)
 }
+
+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/...rator/operators/klusterlet/controllers/klusterletcontroller/klusterlet_controller_test.go b/...rator/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
@@ -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
@@ -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")
 	}