Merge pull request #16353 from rifelpet/iam-policy-refactor

Use IAM Policy Builder for SQS Queue Policy
kubernetes · Feb 15, 2024 · 51fb2cf · 51fb2cf
2 parents 8f71e68 + 4edcc95
commit 51fb2cf
Show file tree

Hide file tree

Showing 68 changed files with 1,005 additions and 671 deletions.
diff --git a/pkg/model/awsmodel/nodeterminationhandler.go b/pkg/model/awsmodel/nodeterminationhandler.go
@@ -18,29 +18,20 @@ package awsmodel
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/arn"
 	"github.com/aws/aws-sdk-go/service/eventbridge"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/model"
+	"k8s.io/kops/pkg/model/iam"
+	"k8s.io/kops/pkg/util/stringorset"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
 	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
 )
 
 const (
-	NTHTemplate = `{
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:{{ AWS_PARTITION }}:sqs:{{ AWS_REGION }}:{{ ACCOUNT_ID }}:{{ SQS_QUEUE_NAME }}"
-		}]
-	}`
 	DefaultMessageRetentionPeriod = 300
 )
 
@@ -123,15 +114,33 @@ func (b *NodeTerminationHandlerBuilder) configureASG(c *fi.CloudupModelBuilderCo
 
 func (b *NodeTerminationHandlerBuilder) build(c *fi.CloudupModelBuilderContext) error {
 	queueName := model.QueueNamePrefix(b.ClusterName()) + "-nth"
-	policy := strings.ReplaceAll(NTHTemplate, "{{ AWS_REGION }}", b.Region)
-	policy = strings.ReplaceAll(policy, "{{ AWS_PARTITION }}", b.AWSPartition)
-	policy = strings.ReplaceAll(policy, "{{ ACCOUNT_ID }}", b.AWSAccountID)
-	policy = strings.ReplaceAll(policy, "{{ SQS_QUEUE_NAME }}", queueName)
+
+	policy := iam.NewPolicy(b.ClusterName(), b.AWSPartition)
+	arn := arn.ARN{
+		Partition: b.AWSPartition,
+		Service:   "sqs",
+		Region:    b.Region,
+		AccountID: b.AWSAccountID,
+		Resource:  queueName,
+	}
+
+	policy.Statement = append(policy.Statement, &iam.Statement{
+		Effect: iam.StatementEffectAllow,
+		Principal: iam.Principal{
+			Service: fi.PtrTo(stringorset.Of("events.amazonaws.com", "sqs.amazonaws.com")),
+		},
+		Action:   stringorset.Of("sqs:SendMessage"),
+		Resource: stringorset.String(arn.String()),
+	})
+	policyJSON, err := policy.AsJSON()
+	if err != nil {
+		return fmt.Errorf("rendering policy as json: %w", err)
+	}
 
 	queue := &awstasks.SQS{
 		Name:                   aws.String(queueName),
 		Lifecycle:              b.Lifecycle,
-		Policy:                 fi.NewStringResource(policy),
+		Policy:                 fi.NewStringResource(policyJSON),
 		MessageRetentionPeriod: DefaultMessageRetentionPeriod,
 		Tags:                   b.CloudTags(queueName, false),
 	}

diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go
@@ -278,12 +278,12 @@ func (s *Statement) MarshalJSON() ([]byte, error) {
 }
 
 type Principal struct {
-	Federated string `json:",omitempty"`
-	Service   string `json:",omitempty"`
+	Federated string                   `json:",omitempty"`
+	Service   *stringorset.StringOrSet `json:",omitempty"`
 }
 
 func (p *Principal) IsEmpty() bool {
-	return *p == Principal{}
+	return p.Federated == "" && (p.Service == nil || p.Service.IsEmpty())
 }
 
 // Equal compares two IAM Statements and returns a bool

diff --git a/pkg/model/iam/iam_builder_test.go b/pkg/model/iam/iam_builder_test.go
@@ -64,7 +64,7 @@ func TestRoundTrip(t *testing.T) {
 		{
 			IAM: &Statement{
 				Effect:    StatementEffectDeny,
-				Principal: Principal{Service: "service"},
+				Principal: Principal{Service: fi.PtrTo(stringorset.Of("service"))},
 				Condition: map[string]interface{}{
 					"bar": "baz",
 				},

diff --git a/...ate_cluster/additionalobjects/data/aws_sqs_queue_additionalobjects-example-com-nth_policy b/...ate_cluster/additionalobjects/data/aws_sqs_queue_additionalobjects-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:additionalobjects-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:additionalobjects-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/...tegration/update_cluster/apiservernodes/data/aws_sqs_queue_minimal-example-com-nth_policy b/...tegration/update_cluster/apiservernodes/data/aws_sqs_queue_minimal-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:minimal-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:minimal-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/...ration/update_cluster/aws-lb-controller/data/aws_sqs_queue_minimal-example-com-nth_policy b/...ration/update_cluster/aws-lb-controller/data/aws_sqs_queue_minimal-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:minimal-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:minimal-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/...ter/bastionadditional_user-data/data/aws_sqs_queue_bastionuserdata-example-com-nth_policy b/...ter/bastionadditional_user-data/data/aws_sqs_queue_bastionuserdata-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:bastionuserdata-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:bastionuserdata-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/...ty-expander-custom/data/aws_sqs_queue_cas-priority-expander-custom-example-com-nth_policy b/...ty-expander-custom/data/aws_sqs_queue_cas-priority-expander-custom-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:cas-priority-expander-custom-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:cas-priority-expander-custom-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/...oscaler-priority-expander/data/aws_sqs_queue_cas-priority-expander-example-com-nth_policy b/...oscaler-priority-expander/data/aws_sqs_queue_cas-priority-expander-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:cas-priority-expander-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:cas-priority-expander-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/tests/integration/update_cluster/complex/data/aws_sqs_queue_complex-example-com-nth_policy b/tests/integration/update_cluster/complex/data/aws_sqs_queue_complex-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:complex-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:complex-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/tests/integration/update_cluster/compress/data/aws_sqs_queue_compress-example-com-nth_policy b/tests/integration/update_cluster/compress/data/aws_sqs_queue_compress-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:compress-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:compress-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/...ion/update_cluster/containerd-custom/data/aws_sqs_queue_containerd-example-com-nth_policy b/...ion/update_cluster/containerd-custom/data/aws_sqs_queue_containerd-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:containerd-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:containerd-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/...ntegration/update_cluster/containerd/data/aws_sqs_queue_containerd-example-com-nth_policy b/...ntegration/update_cluster/containerd/data/aws_sqs_queue_containerd-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:containerd-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:containerd-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}
diff --git a/tests/integration/update_cluster/digit/data/aws_sqs_queue_123-example-com-nth_policy b/tests/integration/update_cluster/digit/data/aws_sqs_queue_123-example-com-nth_policy
@@ -1,11 +1,16 @@
 {
-		"Version": "2012-10-17",
-		"Statement": [{                     
-			"Effect": "Allow",
-			"Principal": {
-				"Service": ["events.amazonaws.com", "sqs.amazonaws.com"]
-			},
-			"Action": "sqs:SendMessage",
-			"Resource": "arn:aws-test:sqs:us-test-1:123456789012:123-example-com-nth"
-		}]
-	}
+  "Statement": [
+    {
+      "Action": "sqs:SendMessage",
+      "Effect": "Allow",
+      "Principal": {
+        "Service": [
+          "events.amazonaws.com",
+          "sqs.amazonaws.com"
+        ]
+      },
+      "Resource": "arn:aws-test:sqs:us-test-1:123456789012:123-example-com-nth"
+    }
+  ],
+  "Version": "2012-10-17"
+}