From d4de9a57798d0093e88018d5028903ebcb3dea40 Mon Sep 17 00:00:00 2001 From: Gaurav Jaswal Date: Mon, 25 Nov 2024 15:50:33 -0500 Subject: [PATCH] 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