From cd795d0c8cf0e7f7d95963f9f9219db5196a8db5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Pawe=C5=82=20G=C5=82azik?= Date: Wed, 22 Feb 2017 15:24:05 +0100 Subject: [PATCH 1/2] Resolve DNS Hosted Zone ID while building IAM policy Fixes #1949 --- pkg/model/context.go | 1 + pkg/model/iam.go | 7 +-- pkg/model/iam/iam_builder.go | 65 ++++++++++------------------ upup/pkg/fi/cloudup/apply_cluster.go | 5 +++ 4 files changed, 34 insertions(+), 44 deletions(-) diff --git a/pkg/model/context.go b/pkg/model/context.go index 05ee9dbbcf..f6232cc646 100644 --- a/pkg/model/context.go +++ b/pkg/model/context.go @@ -33,6 +33,7 @@ type KopsModelContext struct { Cluster *kops.Cluster Region string + HostedZoneID string // used to set up route53 IAM policy InstanceGroups []*kops.InstanceGroup SSHPublicKeys [][]byte diff --git a/pkg/model/iam.go b/pkg/model/iam.go index 7d05759070..3ff9128cd4 100644 --- a/pkg/model/iam.go +++ b/pkg/model/iam.go @@ -157,9 +157,10 @@ func (b *IAMModelBuilder) Build(c *fi.ModelBuilderContext) error { // buildAWSIAMPolicy produces the AWS IAM policy for the given role func (b *IAMModelBuilder) buildAWSIAMPolicy(role kops.InstanceGroupRole) (string, error) { pb := &iam.IAMPolicyBuilder{ - Cluster: b.Cluster, - Role: role, - Region: b.Region, + Cluster: b.Cluster, + Role: role, + Region: b.Region, + HostedZoneID: b.HostedZoneID, } policy, err := pb.BuildAWSIAMPolicy() diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go index 407452f034..72dcfbb1ed 100644 --- a/pkg/model/iam/iam_builder.go +++ b/pkg/model/iam/iam_builder.go @@ -68,9 +68,10 @@ func (l *IAMStatement) Equal(r *IAMStatement) bool { } type IAMPolicyBuilder struct { - Cluster *api.Cluster - Role api.InstanceGroupRole - Region string + Cluster *api.Cluster + Role api.InstanceGroupRole + Region string + HostedZoneID string } func (b *IAMPolicyBuilder) BuildAWSIAMPolicy() (*IAMPolicy, error) { @@ -101,25 +102,6 @@ func (b *IAMPolicyBuilder) BuildAWSIAMPolicy() (*IAMPolicy, error) { Resource: wildcard, }) - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Of("route53:ChangeResourceRecordSets", - "route53:ListResourceRecordSets", - "route53:GetHostedZone"), - Resource: stringorslice.Slice([]string{"arn:aws:route53:::hostedzone/" + b.Cluster.Spec.DNSZone}), - }) - - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Slice([]string{"route53:GetChange"}), - Resource: stringorslice.Slice([]string{"arn:aws:route53:::change/*"}), - }) - - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Slice([]string{"route53:ListHostedZones"}), - Resource: wildcard, - }) } { @@ -148,25 +130,6 @@ func (b *IAMPolicyBuilder) BuildAWSIAMPolicy() (*IAMPolicy, error) { Resource: wildcard, }) - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Of("route53:ChangeResourceRecordSets", - "route53:ListResourceRecordSets", - "route53:GetHostedZone"), - Resource: stringorslice.Slice([]string{"arn:aws:route53:::hostedzone/" + b.Cluster.Spec.DNSZone}), - }) - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Slice([]string{"route53:GetChange"}), - Resource: stringorslice.Slice([]string{"arn:aws:route53:::change/*"}), - }) - - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Slice([]string{"route53:ListHostedZones"}), - Resource: wildcard, - }) - p.Statement = append(p.Statement, &IAMStatement{ Effect: IAMStatementEffectAllow, Action: stringorslice.Slice([]string{"elasticloadbalancing:*"}), @@ -212,6 +175,26 @@ func (b *IAMPolicyBuilder) BuildAWSIAMPolicy() (*IAMPolicy, error) { } } + p.Statement = append(p.Statement, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Of("route53:ChangeResourceRecordSets", + "route53:ListResourceRecordSets", + "route53:GetHostedZone"), + Resource: stringorslice.Slice([]string{"arn:aws:route53:::hostedzone/" + b.HostedZoneID}), + }) + + p.Statement = append(p.Statement, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Slice([]string{"route53:GetChange"}), + Resource: stringorslice.Slice([]string{"arn:aws:route53:::change/*"}), + }) + + p.Statement = append(p.Statement, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Slice([]string{"route53:ListHostedZones"}), + Resource: wildcard, + }) + // For S3 IAM permissions, we grant permissions to subtrees. So find the parents; // we don't need to grant mypath and mypath/child. var roots []string diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index d6e3fee374..62ca7bea43 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -346,6 +346,11 @@ func (c *ApplyClusterCmd) Run() error { if err != nil { return err } + dnszone, err := findZone(cluster, cloud) + if err != nil { + return err + } + modelContext.HostedZoneID = dnszone.ID() clusterTags, err := buildCloudupTags(cluster) if err != nil { From 26fac5e17da992a2e9404dbcaac2f3ff7efbf8e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Pawe=C5=82=20G=C5=82azik?= Date: Thu, 23 Feb 2017 11:40:50 +0100 Subject: [PATCH 2/2] Update cloudformation test (IAM policy) --- tests/integration/minimal/cloudformation.json | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/tests/integration/minimal/cloudformation.json b/tests/integration/minimal/cloudformation.json index 2349165df7..4b8d6e532b 100644 --- a/tests/integration/minimal/cloudformation.json +++ b/tests/integration/minimal/cloudformation.json @@ -550,6 +550,27 @@ "*" ] }, + { + "Action": [ + "elasticloadbalancing:*" + ], + "Effect": "Allow", + "Resource": [ + "*" + ] + }, + { + "Action": [ + "autoscaling:DescribeAutoScalingGroups", + "autoscaling:DescribeAutoScalingInstances", + "autoscaling:SetDesiredCapacity", + "autoscaling:TerminateInstanceInAutoScalingGroup" + ], + "Effect": "Allow", + "Resource": [ + "*" + ] + }, { "Action": [ "route53:ChangeResourceRecordSets", @@ -578,27 +599,6 @@ "Resource": [ "*" ] - }, - { - "Action": [ - "elasticloadbalancing:*" - ], - "Effect": "Allow", - "Resource": [ - "*" - ] - }, - { - "Action": [ - "autoscaling:DescribeAutoScalingGroups", - "autoscaling:DescribeAutoScalingInstances", - "autoscaling:SetDesiredCapacity", - "autoscaling:TerminateInstanceInAutoScalingGroup" - ], - "Effect": "Allow", - "Resource": [ - "*" - ] } ], "Version": "2012-10-17" @@ -625,6 +625,21 @@ "*" ] }, + { + "Action": [ + "ecr:GetAuthorizationToken", + "ecr:BatchCheckLayerAvailability", + "ecr:GetDownloadUrlForLayer", + "ecr:GetRepositoryPolicy", + "ecr:DescribeRepositories", + "ecr:ListImages", + "ecr:BatchGetImage" + ], + "Effect": "Allow", + "Resource": [ + "*" + ] + }, { "Action": [ "route53:ChangeResourceRecordSets", @@ -653,21 +668,6 @@ "Resource": [ "*" ] - }, - { - "Action": [ - "ecr:GetAuthorizationToken", - "ecr:BatchCheckLayerAvailability", - "ecr:GetDownloadUrlForLayer", - "ecr:GetRepositoryPolicy", - "ecr:DescribeRepositories", - "ecr:ListImages", - "ecr:BatchGetImage" - ], - "Effect": "Allow", - "Resource": [ - "*" - ] } ], "Version": "2012-10-17"