Fix the full-ecr-access flag to give power user instead of read only

[polothy]

Description

When using a command like eksctl create cluster --full-ecr-access --name MyCluster --region us-east-1 --profile my-profile then the IAM role for accessing ECR would have the policy AmazonEC2ContainerRegistryReadOnly instead of AmazonEC2ContainerRegistryPowerUser.

Workarounds

• Find the user in the AWS Console and remove the AmazonEC2ContainerRegistryReadOnly policy and add the AmazonEC2ContainerRegistryPowerUser policy.
• Don't use the --full-ecr-access flag in eksctl create cluster command.

Checklist

• Code compiles correctly (i.e make build)
• Added tests that cover your change (if possible)
• All tests passing (i.e. make test)
• Added/modified documentation as required (such as the README)
• Added yourself to the humans.txt file

[polothy]

Here is my lame'ish test. I couldn't figure out how to lookup the IAM policies on the generated template (EG: doing a t := rs.Template() and then finding it in t):

diff --git a/pkg/cfn/builder/api_test.go b/pkg/cfn/builder/api_test.go
index e9dc454..b79ddd5 100644
--- a/pkg/cfn/builder/api_test.go
+++ b/pkg/cfn/builder/api_test.go
@@ -300,4 +300,38 @@ var _ = Describe("CloudFormation template builder API", func() {
 			checkScript("/var/lib/cloud/scripts/per-instance/bootstrap.al2.sh", true)
 		})
 	})
+
+	Describe("ECRReadOnlyAccess", func() {
+		spec := &api.ClusterConfig{
+			ClusterName:       clusterName,
+			AvailabilityZones: testAZs,
+			NodeType:          "t2.medium",
+			Region:            "us-west-2",
+		}
+		rs := NewNodeGroupResourceSet(spec, "eksctl-test-123-cluster", 0)
+
+		err := rs.AddAllResources()
+		It("should have read only policy", func() {
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(spec.NodePolicyARNs).To(ContainElement("arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly"))
+		})
+	})
+
+	Describe("ECRFullAccess", func() {
+		spec := &api.ClusterConfig{
+			ClusterName:       clusterName,
+			AvailabilityZones: testAZs,
+			NodeType:          "t2.medium",
+			Region:            "us-west-2",
+		}
+		spec.Addons.WithIAM.PolicyAmazonEC2ContainerRegistryPowerUser = true
+
+		rs := NewNodeGroupResourceSet(spec, "eksctl-test-123-cluster", 0)
+
+		err := rs.AddAllResources()
+		It("should have power user policy", func() {
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(spec.NodePolicyARNs).To(ContainElement("arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryPowerUser"))
+		})
+	})
 })

[richardcase]

Good spot and thanks for the change @polothy. This looks good to me.

One small thing, do you want to add yourself to humans.txt?

[richardcase]

@polothy - would you be able to rebase on master?

[polothy]

Rebase done

[richardcase]

LGTM, thanks for doing the rebase. Please merge when you get time.

[polothy]

Thanks for the approves!

Please merge when you get time.

Sorry, I assume you are talking to @defiantsoftware ? I don't have permission to merge.

[dholbach]

Great work, thanks @polothy! :-)