Support new, plural aws.giantswarm.io/irsa-trust-domains annotation…

… on the AWSCluster object (#334)
giantswarm · Sep 20, 2024 · ad9c60a · ad9c60a
1 parent 2627c4f
commit ad9c60a
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 70 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Support new, plural `aws.giantswarm.io/irsa-trust-domains` annotation on the AWSCluster object that centrally defines which service account issuer domains to use. The previous annotation is supported for backward compatibility.
+
 ## [0.27.1] - 2024-08-21
 
 ### Fixed

diff --git a/controllers/awsmachinetemplate_controller.go b/controllers/awsmachinetemplate_controller.go
@@ -265,9 +265,9 @@ func (r *AWSMachineTemplateReconciler) reconcileNormal(ctx context.Context, iamS
 
 			irsaDomain := key.IRSADomain(baseDomain, awsCluster.Spec.Region, accountID, clusterName)
 
-			oldIrsaDomain := key.GetAdditionalIrsaDomain(awsMachineTemplate)
+			irsaTrustDomains := key.GetIRSATrustDomains(awsMachineTemplate, awsCluster, irsaDomain)
 
-			err = iamService.ReconcileRolesForIRSA(accountID, irsaDomain, oldIrsaDomain)
+			err = iamService.ReconcileRolesForIRSA(accountID, irsaTrustDomains)
 			if err != nil {
 				return ctrl.Result{}, errors.WithStack(err)
 			}

diff --git a/controllers/awsmanagedcontrolplane_controller.go b/controllers/awsmanagedcontrolplane_controller.go
@@ -146,7 +146,7 @@ func (r *AWSManagedControlPlaneReconciler) Reconcile(ctx context.Context, req ct
 		}
 
 		iamService.SetPrincipalRoleARN(eksRoleARN)
-		err = iamService.ReconcileRolesForIRSA(accountID, eksOpenIdDomain, "")
+		err = iamService.ReconcileRolesForIRSA(accountID, []string{eksOpenIdDomain})
 		if err != nil {
 			return ctrl.Result{}, microerror.Mask(err)
 		}

diff --git a/pkg/iam/control_plane_template.go b/pkg/iam/control_plane_template.go
@@ -6,7 +6,7 @@ const ec2TrustIdentityPolicyTemplate = `{
     {
       "Effect": "Allow",
       "Principal": {
-        "Service": "{{.EC2ServiceDomain}}"
+        "Service": "{{ .EC2ServiceDomain }}"
       },
       "Action": "sts:AssumeRole"
     }
@@ -36,7 +36,7 @@ const controlPlanePolicyTemplate = `{
     {
       "Condition": {
         "StringEquals": {
-          "autoscaling:ResourceTag/sigs.k8s.io/cluster-api-provider-aws/cluster/{{.ClusterName}}": "owned"
+          "autoscaling:ResourceTag/sigs.k8s.io/cluster-api-provider-aws/cluster/{{ .ClusterName }}": "owned"
         }
       },
       "Action": [

diff --git a/pkg/iam/iam.go b/pkg/iam/iam.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/url"
 	"reflect"
+	"slices"
 	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -61,14 +62,13 @@ type IAMService struct {
 }
 
 type Route53RoleParams struct {
-	AWSDomain                  string
-	EC2ServiceDomain           string
-	AccountID                  string
-	CloudFrontDomain           string
-	AdditionalCloudFrontDomain string
-	Namespace                  string
-	ServiceAccount             string
-	PrincipalRoleARN           string
+	AWSDomain        string
+	EC2ServiceDomain string
+	AccountID        string
+	IRSATrustDomains []string
+	Namespace        string
+	ServiceAccount   string
+	PrincipalRoleARN string
 }
 
 func New(config IAMServiceConfig) (*IAMService, error) {
@@ -163,12 +163,12 @@ func (s *IAMService) ReconcileKiamRole() error {
 	return nil
 }
 
-func (s *IAMService) ReconcileRolesForIRSA(awsAccountID string, cloudFrontDomain string, oldCloudFrontDomain string) error {
+func (s *IAMService) ReconcileRolesForIRSA(awsAccountID string, irsaTrustDomains []string) error {
 	s.log.Info("reconciling IAM roles for IRSA")
 
 	for _, roleTypeToReconcile := range getIRSARoles() {
 		var params Route53RoleParams
-		params, err := s.generateRoute53RoleParams(roleTypeToReconcile, awsAccountID, cloudFrontDomain, oldCloudFrontDomain)
+		params, err := s.generateRoute53RoleParams(roleTypeToReconcile, awsAccountID, irsaTrustDomains)
 		if err != nil {
 			s.log.Error(err, "failed to generate Route53 role parameters")
 			return err
@@ -184,7 +184,11 @@ func (s *IAMService) ReconcileRolesForIRSA(awsAccountID string, cloudFrontDomain
 	return nil
 }
 
-func (s *IAMService) generateRoute53RoleParams(roleTypeToReconcile string, awsAccountID string, cloudFrontDomain string, additionalCloudFrontDomain string) (Route53RoleParams, error) {
+func (s *IAMService) generateRoute53RoleParams(roleTypeToReconcile string, awsAccountID string, irsaTrustDomains []string) (Route53RoleParams, error) {
+	if len(irsaTrustDomains) == 0 || slices.ContainsFunc(irsaTrustDomains, func(irsaTrustDomain string) bool { return irsaTrustDomain == "" }) {
+		return Route53RoleParams{}, fmt.Errorf("irsaTrustDomains cannot be empty or have empty values: %v", irsaTrustDomains)
+	}
+
 	namespace := "kube-system"
 	serviceAccount, err := getServiceAccount(roleTypeToReconcile)
 	if err != nil {
@@ -196,15 +200,11 @@ func (s *IAMService) generateRoute53RoleParams(roleTypeToReconcile string, awsAc
 		AWSDomain:        awsDomain(s.region),
 		EC2ServiceDomain: ec2ServiceDomain(s.region),
 		AccountID:        awsAccountID,
-		CloudFrontDomain: cloudFrontDomain,
+		IRSATrustDomains: irsaTrustDomains,
 		Namespace:        namespace,
 		ServiceAccount:   serviceAccount,
 	}
 
-	if additionalCloudFrontDomain != "" {
-		params.AdditionalCloudFrontDomain = additionalCloudFrontDomain
-	}
-
 	return params, nil
 }
 

diff --git a/pkg/iam/kiam_template.go b/pkg/iam/kiam_template.go
@@ -6,7 +6,7 @@ const kiamTrustIdentityPolicy = `{
     {
       "Effect": "Allow",
       "Principal": {
-        "AWS": "{{.ControlPlaneRoleARN}}"
+        "AWS": "{{ .ControlPlaneRoleARN }}"
       },
       "Action": "sts:AssumeRole"
     }

diff --git a/pkg/iam/nodes_template.go b/pkg/iam/nodes_template.go
@@ -27,7 +27,7 @@ const nodesTemplate = `{
     {
       "Condition": {
         "StringEquals": {
-          "autoscaling:ResourceTag/sigs.k8s.io/cluster-api-provider-aws/cluster/{{.ClusterName}}": "owned"
+          "autoscaling:ResourceTag/sigs.k8s.io/cluster-api-provider-aws/cluster/{{ .ClusterName }}": "owned"
         }
       },
       "Action": [

diff --git a/pkg/iam/route53_template.go b/pkg/iam/route53_template.go
@@ -3,95 +3,65 @@ package iam
 const trustIdentityPolicyIRSA = `{
   "Version": "2012-10-17",
   "Statement": [
+    {{- range $index, $domain := .IRSATrustDomains }}
+    {{ if gt $index 0 }},{{ end -}}
     {
       "Effect": "Allow",
       "Principal": {
-        "Federated": "arn:{{.AWSDomain}}:iam::{{.AccountID}}:oidc-provider/{{.CloudFrontDomain}}"
+        "Federated": "arn:{{ $.AWSDomain }}:iam::{{ $.AccountID }}:oidc-provider/{{ $domain }}"
       },
       "Action": "sts:AssumeRoleWithWebIdentity",
       "Condition": {
         "StringEquals": {
-          "{{.CloudFrontDomain}}:sub": "system:serviceaccount:{{.Namespace}}:{{.ServiceAccount}}"
-        }
-      }
-    }{{if .AdditionalCloudFrontDomain}},
-    {
-      "Effect": "Allow",
-      "Principal": {
-        "Federated": "arn:{{.AWSDomain}}:iam::{{.AccountID}}:oidc-provider/{{.AdditionalCloudFrontDomain}}"
-      },
-      "Action": "sts:AssumeRoleWithWebIdentity",
-      "Condition": {
-        "StringEquals": {
-          "{{.AdditionalCloudFrontDomain}}:sub": "system:serviceaccount:{{.Namespace}}:{{.ServiceAccount}}"
+          "{{ $domain }}:sub": "system:serviceaccount:{{ $.Namespace }}:{{ $.ServiceAccount }}"
         }
       }
     }
-    {{end}}
+    {{- end }}
   ]
 }
 `
 
 const albControllerTrustIdentityPolicyIRSA = `{
   "Version": "2012-10-17",
   "Statement": [
+    {{- range $index, $domain := .IRSATrustDomains }}
+    {{ if gt $index 0 }},{{ end -}}
     {
       "Effect": "Allow",
       "Principal": {
-        "Federated": "arn:{{.AWSDomain}}:iam::{{.AccountID}}:oidc-provider/{{.CloudFrontDomain}}"
+        "Federated": "arn:{{ $.AWSDomain }}:iam::{{ $.AccountID }}:oidc-provider/{{ $domain }}"
       },
       "Action": "sts:AssumeRoleWithWebIdentity",
       "Condition": {
         "StringLike": {
-          "{{.CloudFrontDomain}}:sub": "system:serviceaccount:*:{{.ServiceAccount}}"
-        }
-      }
-    }{{if .AdditionalCloudFrontDomain}},
-    {
-      "Effect": "Allow",
-      "Principal": {
-        "Federated": "arn:{{.AWSDomain}}:iam::{{.AccountID}}:oidc-provider/{{.AdditionalCloudFrontDomain}}"
-      },
-      "Action": "sts:AssumeRoleWithWebIdentity",
-      "Condition": {
-        "StringLike": {
-          "{{.AdditionalCloudFrontDomain}}:sub": "system:serviceaccount:*:{{.ServiceAccount}}"
+          "{{ $domain }}:sub": "system:serviceaccount:*:{{ $.ServiceAccount }}"
         }
       }
     }
-    {{end}}
+    {{- end }}
   ]
 }
 `
 
 const externalDnsTrustIdentityPolicyIRSA = `{
   "Version": "2012-10-17",
   "Statement": [
+    {{- range $index, $domain := .IRSATrustDomains }}
+    {{ if gt $index 0 }},{{ end -}}
     {
       "Effect": "Allow",
       "Principal": {
-        "Federated": "arn:{{.AWSDomain}}:iam::{{.AccountID}}:oidc-provider/{{.CloudFrontDomain}}"
-      },
-      "Action": "sts:AssumeRoleWithWebIdentity",
-      "Condition": {
-        "StringLike": {
-          "{{.CloudFrontDomain}}:sub": "system:serviceaccount:*:*{{.ServiceAccount}}*"
-        }
-      }
-    }{{if .AdditionalCloudFrontDomain}},
-    {
-      "Effect": "Allow",
-      "Principal": {
-        "Federated": "arn:{{.AWSDomain}}:iam::{{.AccountID}}:oidc-provider/{{.AdditionalCloudFrontDomain}}"
+        "Federated": "arn:{{ $.AWSDomain }}:iam::{{ $.AccountID }}:oidc-provider/{{ $domain }}"
       },
       "Action": "sts:AssumeRoleWithWebIdentity",
       "Condition": {
         "StringLike": {
-          "{{.AdditionalCloudFrontDomain}}:sub": "system:serviceaccount:*:*{{.ServiceAccount}}*"
+          "{{ $domain }}:sub": "system:serviceaccount:*:*{{ $.ServiceAccount }}*"
         }
       }
     }
-    {{end}}
+    {{- end }}
   ]
 }
 `

diff --git a/pkg/key/key.go b/pkg/key/key.go
@@ -3,6 +3,7 @@ package key
 import (
 	"context"
 	"fmt"
+	"slices"
 	"strings"
 
 	awsarn "github.com/aws/aws-sdk-go/aws/arn"
@@ -163,8 +164,23 @@ func GetAWSAccountID(awsClusterRoleIdentity *capa.AWSClusterRoleIdentity) (strin
 	return a.AccountID, nil
 }
 
-func GetAdditionalIrsaDomain(o v1.Object) string {
-	return GetAnnotation(o, "aws.giantswarm.io/irsa-additional-domain")
+func GetIRSATrustDomains(awsMachineTemplate *capa.AWSMachineTemplate, awsCluster *capa.AWSCluster, ensurePrimaryIRSATrustDomain string) []string {
+	var values []string
+	if s := GetAnnotation(awsCluster, "aws.giantswarm.io/irsa-trust-domains"); s != "" {
+		values = strings.Split(s, ",")
+	} else if s = GetAnnotation(awsMachineTemplate, "aws.giantswarm.io/irsa-additional-domain"); s != "" {
+		// Fall back to previously-used, singular annotation for backward compatibility
+		values = append(values, s)
+	}
+
+	irsaTrustDomains := []string{ensurePrimaryIRSATrustDomain}
+	for _, value := range values {
+		value = strings.TrimSpace(value)
+		if value != "" && !slices.Contains(irsaTrustDomains, value) {
+			irsaTrustDomains = append(irsaTrustDomains, value)
+		}
+	}
+	return irsaTrustDomains
 }
 
 // GetAnnotation returns the value of the specified annotation.