From 0aac9b7f8dffdfba6fbe54126224c86cbe17c5d4 Mon Sep 17 00:00:00 2001 From: Kashif Saadat Date: Wed, 16 Aug 2017 10:57:16 +0100 Subject: [PATCH 1/2] Allow the strict IAM policies to be optional, default to original behaviour (not-strict) --- pkg/apis/kops/cluster.go | 8 +++ pkg/apis/kops/v1alpha1/cluster.go | 8 +++ .../kops/v1alpha1/zz_generated.conversion.go | 40 +++++++++++ pkg/apis/kops/v1alpha2/cluster.go | 8 +++ .../kops/v1alpha2/zz_generated.conversion.go | 40 +++++++++++ pkg/model/iam/iam_builder.go | 72 ++++++++++++------- pkg/model/iam/iam_builder_test.go | 49 +++++++++++-- 7 files changed, 194 insertions(+), 31 deletions(-) diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go index 1f065eb048..27d41fa35f 100644 --- a/pkg/apis/kops/cluster.go +++ b/pkg/apis/kops/cluster.go @@ -143,6 +143,9 @@ type ClusterSpec struct { // This API component is under construction, will remove this comment // once this API is fully functional. Assets *Assets `json:"assets,omitempty"` + + // IAM field adds control over the IAM security policies applied to resources + IAM *IAMSpec `json:"iam,omitempty"` } type Assets struct { @@ -150,6 +153,11 @@ type Assets struct { FileRepository *string `json:"fileRepository,omitempty"` } +// IAMSpec adds control over the IAM security policies applied to resources +type IAMSpec struct { + Strict bool `json:"strict"` +} + // HookSpec is a definition hook type HookSpec struct { // Name is an optional name for the hook, otherwise the name is kops-hook- diff --git a/pkg/apis/kops/v1alpha1/cluster.go b/pkg/apis/kops/v1alpha1/cluster.go index bbcbb7d727..350e7ee884 100644 --- a/pkg/apis/kops/v1alpha1/cluster.go +++ b/pkg/apis/kops/v1alpha1/cluster.go @@ -249,6 +249,9 @@ type ClusterSpec struct { // Alternative locations for files and containers Assets *Assets `json:"assets,omitempty"` + + // IAM field adds control over the IAM security policies applied to resources + IAM *IAMSpec `json:"iam,omitempty"` } type Assets struct { @@ -256,6 +259,11 @@ type Assets struct { FileRepository *string `json:"fileRepository,omitempty"` } +// IAMSpec adds control over the IAM security policies applied to resources +type IAMSpec struct { + Strict bool `json:"strict"` +} + // HookSpec is a definition hook type HookSpec struct { // Name is an optional name for the hook, otherwise the name is kops-hook- diff --git a/pkg/apis/kops/v1alpha1/zz_generated.conversion.go b/pkg/apis/kops/v1alpha1/zz_generated.conversion.go index 68f24372f5..11acf8ef0d 100644 --- a/pkg/apis/kops/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/kops/v1alpha1/zz_generated.conversion.go @@ -89,6 +89,8 @@ func RegisterConversions(scheme *runtime.Scheme) error { Convert_kops_HTTPProxy_To_v1alpha1_HTTPProxy, Convert_v1alpha1_HookSpec_To_kops_HookSpec, Convert_kops_HookSpec_To_v1alpha1_HookSpec, + Convert_v1alpha1_IAMSpec_To_kops_IAMSpec, + Convert_kops_IAMSpec_To_v1alpha1_IAMSpec, Convert_v1alpha1_InstanceGroup_To_kops_InstanceGroup, Convert_kops_InstanceGroup_To_v1alpha1_InstanceGroup, Convert_v1alpha1_InstanceGroupList_To_kops_InstanceGroupList, @@ -687,6 +689,15 @@ func autoConvert_v1alpha1_ClusterSpec_To_kops_ClusterSpec(in *ClusterSpec, out * } else { out.Assets = nil } + if in.IAM != nil { + in, out := &in.IAM, &out.IAM + *out = new(kops.IAMSpec) + if err := Convert_v1alpha1_IAMSpec_To_kops_IAMSpec(*in, *out, s); err != nil { + return err + } + } else { + out.IAM = nil + } return nil } @@ -881,6 +892,15 @@ func autoConvert_kops_ClusterSpec_To_v1alpha1_ClusterSpec(in *kops.ClusterSpec, } else { out.Assets = nil } + if in.IAM != nil { + in, out := &in.IAM, &out.IAM + *out = new(IAMSpec) + if err := Convert_kops_IAMSpec_To_v1alpha1_IAMSpec(*in, *out, s); err != nil { + return err + } + } else { + out.IAM = nil + } return nil } @@ -1300,6 +1320,26 @@ func Convert_kops_HookSpec_To_v1alpha1_HookSpec(in *kops.HookSpec, out *HookSpec return autoConvert_kops_HookSpec_To_v1alpha1_HookSpec(in, out, s) } +func autoConvert_v1alpha1_IAMSpec_To_kops_IAMSpec(in *IAMSpec, out *kops.IAMSpec, s conversion.Scope) error { + out.Strict = in.Strict + return nil +} + +// Convert_v1alpha1_IAMSpec_To_kops_IAMSpec is an autogenerated conversion function. +func Convert_v1alpha1_IAMSpec_To_kops_IAMSpec(in *IAMSpec, out *kops.IAMSpec, s conversion.Scope) error { + return autoConvert_v1alpha1_IAMSpec_To_kops_IAMSpec(in, out, s) +} + +func autoConvert_kops_IAMSpec_To_v1alpha1_IAMSpec(in *kops.IAMSpec, out *IAMSpec, s conversion.Scope) error { + out.Strict = in.Strict + return nil +} + +// Convert_kops_IAMSpec_To_v1alpha1_IAMSpec is an autogenerated conversion function. +func Convert_kops_IAMSpec_To_v1alpha1_IAMSpec(in *kops.IAMSpec, out *IAMSpec, s conversion.Scope) error { + return autoConvert_kops_IAMSpec_To_v1alpha1_IAMSpec(in, out, s) +} + func autoConvert_v1alpha1_InstanceGroup_To_kops_InstanceGroup(in *InstanceGroup, out *kops.InstanceGroup, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_v1alpha1_InstanceGroupSpec_To_kops_InstanceGroupSpec(&in.Spec, &out.Spec, s); err != nil { diff --git a/pkg/apis/kops/v1alpha2/cluster.go b/pkg/apis/kops/v1alpha2/cluster.go index 72273711fa..2a2f76796f 100644 --- a/pkg/apis/kops/v1alpha2/cluster.go +++ b/pkg/apis/kops/v1alpha2/cluster.go @@ -175,6 +175,9 @@ type ClusterSpec struct { // Alternative locations for files and containers Assets *Assets `json:"assets,omitempty"` + + // IAM field adds control over the IAM security policies applied to resources + IAM *IAMSpec `json:"iam,omitempty"` } type Assets struct { @@ -182,6 +185,11 @@ type Assets struct { FileRepository *string `json:"fileRepository,omitempty"` } +// IAMSpec adds control over the IAM security policies applied to resources +type IAMSpec struct { + Strict bool `json:"strict"` +} + // HookSpec is a definition hook type HookSpec struct { // Name is an optional name for the hook, otherwise the name is kops-hook- diff --git a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go index 234e91e252..f51f21bcfd 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go +++ b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go @@ -93,6 +93,8 @@ func RegisterConversions(scheme *runtime.Scheme) error { Convert_kops_HTTPProxy_To_v1alpha2_HTTPProxy, Convert_v1alpha2_HookSpec_To_kops_HookSpec, Convert_kops_HookSpec_To_v1alpha2_HookSpec, + Convert_v1alpha2_IAMSpec_To_kops_IAMSpec, + Convert_kops_IAMSpec_To_v1alpha2_IAMSpec, Convert_v1alpha2_InstanceGroup_To_kops_InstanceGroup, Convert_kops_InstanceGroup_To_v1alpha2_InstanceGroup, Convert_v1alpha2_InstanceGroupList_To_kops_InstanceGroupList, @@ -725,6 +727,15 @@ func autoConvert_v1alpha2_ClusterSpec_To_kops_ClusterSpec(in *ClusterSpec, out * } else { out.Assets = nil } + if in.IAM != nil { + in, out := &in.IAM, &out.IAM + *out = new(kops.IAMSpec) + if err := Convert_v1alpha2_IAMSpec_To_kops_IAMSpec(*in, *out, s); err != nil { + return err + } + } else { + out.IAM = nil + } return nil } @@ -934,6 +945,15 @@ func autoConvert_kops_ClusterSpec_To_v1alpha2_ClusterSpec(in *kops.ClusterSpec, } else { out.Assets = nil } + if in.IAM != nil { + in, out := &in.IAM, &out.IAM + *out = new(IAMSpec) + if err := Convert_kops_IAMSpec_To_v1alpha2_IAMSpec(*in, *out, s); err != nil { + return err + } + } else { + out.IAM = nil + } return nil } @@ -1398,6 +1418,26 @@ func Convert_kops_HookSpec_To_v1alpha2_HookSpec(in *kops.HookSpec, out *HookSpec return autoConvert_kops_HookSpec_To_v1alpha2_HookSpec(in, out, s) } +func autoConvert_v1alpha2_IAMSpec_To_kops_IAMSpec(in *IAMSpec, out *kops.IAMSpec, s conversion.Scope) error { + out.Strict = in.Strict + return nil +} + +// Convert_v1alpha2_IAMSpec_To_kops_IAMSpec is an autogenerated conversion function. +func Convert_v1alpha2_IAMSpec_To_kops_IAMSpec(in *IAMSpec, out *kops.IAMSpec, s conversion.Scope) error { + return autoConvert_v1alpha2_IAMSpec_To_kops_IAMSpec(in, out, s) +} + +func autoConvert_kops_IAMSpec_To_v1alpha2_IAMSpec(in *kops.IAMSpec, out *IAMSpec, s conversion.Scope) error { + out.Strict = in.Strict + return nil +} + +// Convert_kops_IAMSpec_To_v1alpha2_IAMSpec is an autogenerated conversion function. +func Convert_kops_IAMSpec_To_v1alpha2_IAMSpec(in *kops.IAMSpec, out *IAMSpec, s conversion.Scope) error { + return autoConvert_kops_IAMSpec_To_v1alpha2_IAMSpec(in, out, s) +} + func autoConvert_v1alpha2_InstanceGroup_To_kops_InstanceGroup(in *InstanceGroup, out *kops.InstanceGroup, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_v1alpha2_InstanceGroupSpec_To_kops_InstanceGroupSpec(&in.Spec, &out.Spec, s); err != nil { diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go index b125816eef..446de1e04f 100644 --- a/pkg/model/iam/iam_builder.go +++ b/pkg/model/iam/iam_builder.go @@ -78,11 +78,17 @@ type IAMPolicyBuilder struct { HostedZoneID string } +// BuildAWSIAMPolicy builds a set of IAM policy statements based on the +// instance group type and IAM Strict setting within the Cluster Spec func (b *IAMPolicyBuilder) BuildAWSIAMPolicy() (*IAMPolicy, error) { wildcard := stringorslice.Slice([]string{"*"}) - iamPrefix := b.IAMPrefix() + strictIAM := false + if b.Cluster.Spec.IAM != nil { + strictIAM = b.Cluster.Spec.IAM.Strict + } + p := &IAMPolicy{ Version: IAMPolicyDefaultVersion, } @@ -231,7 +237,7 @@ func (b *IAMPolicyBuilder) BuildAWSIAMPolicy() (*IAMPolicy, error) { } if s3Path, ok := vfsPath.(*vfs.S3Path); ok { - addS3Permissions(p, iamPrefix, s3Path, b.Role) + addS3Permissions(p, iamPrefix, s3Path, b.Role, strictIAM) } else if _, ok := vfsPath.(*vfs.MemFSPath); ok { // Tests -ignore - nothing we can do in terms of IAM policy glog.Warningf("ignoring memfs path %q for IAM policy builder", vfsPath) @@ -275,7 +281,7 @@ func addRoute53ListHostedZonesPermission(p *IAMPolicy) { // addS3Permissions updates the IAM Policy with statements granting tailored // access to S3 assets, depending on the instance role -func addS3Permissions(p *IAMPolicy, iamPrefix string, s3Path *vfs.S3Path, role api.InstanceGroupRole) { +func addS3Permissions(p *IAMPolicy, iamPrefix string, s3Path *vfs.S3Path, role api.InstanceGroupRole, strictIAM bool) { // Note that the config store may itself be a subdirectory of a bucket iamS3Path := s3Path.Bucket() + "/" + s3Path.Key() iamS3Path = strings.TrimSuffix(iamS3Path, "/") @@ -297,30 +303,42 @@ func addS3Permissions(p *IAMPolicy, iamPrefix string, s3Path *vfs.S3Path, role a ), }) - if role == api.InstanceGroupRoleMaster { - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Slice([]string{"s3:Get*"}), - Resource: stringorslice.Of( - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/*"}, ""), - ), - }) - } else if role == api.InstanceGroupRoleNode { - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Slice([]string{"s3:Get*"}), - Resource: stringorslice.Of( - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/addons/*"}, ""), - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/instancegroup/*"}, ""), - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/pki/issued/*"}, ""), - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/pki/ssh/*"}, ""), - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/pki/private/kube-proxy/*"}, ""), - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/pki/private/kubelet/*"}, ""), - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/secrets/*"}, ""), - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/cluster.spec"}, ""), - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/config"}, ""), - ), - }) + if strictIAM { + if role == api.InstanceGroupRoleMaster { + p.Statement = append(p.Statement, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Slice([]string{"s3:Get*"}), + Resource: stringorslice.Of( + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/*"}, ""), + ), + }) + } else if role == api.InstanceGroupRoleNode { + p.Statement = append(p.Statement, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Slice([]string{"s3:Get*"}), + Resource: stringorslice.Of( + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/addons/*"}, ""), + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/instancegroup/*"}, ""), + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/pki/issued/*"}, ""), + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/pki/ssh/*"}, ""), + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/pki/private/kube-proxy/*"}, ""), + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/pki/private/kubelet/*"}, ""), + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/secrets/*"}, ""), + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/cluster.spec"}, ""), + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/config"}, ""), + ), + }) + } + } else { + if role == api.InstanceGroupRoleMaster || role == api.InstanceGroupRoleNode { + p.Statement = append(p.Statement, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Slice([]string{"s3:*"}), + Resource: stringorslice.Of( + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/*"}, ""), + ), + }) + } } } diff --git a/pkg/model/iam/iam_builder_test.go b/pkg/model/iam/iam_builder_test.go index ef8293a4b5..b0ed18b7a6 100644 --- a/pkg/model/iam/iam_builder_test.go +++ b/pkg/model/iam/iam_builder_test.go @@ -97,10 +97,12 @@ func TestS3PolicyGeneration(t *testing.T) { grid := []struct { Role kops.InstanceGroupRole + StrictIAM bool IAMPolicy IAMPolicy }{ { - Role: "Master", + Role: "Master", + StrictIAM: true, IAMPolicy: IAMPolicy{ Statement: append(defaultS3Statements, &IAMStatement{ Effect: IAMStatementEffectAllow, @@ -114,7 +116,23 @@ func TestS3PolicyGeneration(t *testing.T) { }, }, { - Role: "Node", + Role: "Master", + StrictIAM: false, + IAMPolicy: IAMPolicy{ + Statement: append(defaultS3Statements, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Slice([]string{ + "s3:*", + }), + Resource: stringorslice.Of( + "arn:aws:s3:::bucket-name/cluster-name.k8s.local/*", + ), + }), + }, + }, + { + Role: "Node", + StrictIAM: true, IAMPolicy: IAMPolicy{ Statement: append(defaultS3Statements, &IAMStatement{ Effect: IAMStatementEffectAllow, @@ -136,7 +154,30 @@ func TestS3PolicyGeneration(t *testing.T) { }, }, { - Role: "Bastion", + Role: "Node", + StrictIAM: false, + IAMPolicy: IAMPolicy{ + Statement: append(defaultS3Statements, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Slice([]string{ + "s3:*", + }), + Resource: stringorslice.Of( + "arn:aws:s3:::bucket-name/cluster-name.k8s.local/*", + ), + }), + }, + }, + { + Role: "Bastion", + StrictIAM: true, + IAMPolicy: IAMPolicy{ + Statement: defaultS3Statements, + }, + }, + { + Role: "Bastion", + StrictIAM: false, IAMPolicy: IAMPolicy{ Statement: defaultS3Statements, }, @@ -157,7 +198,7 @@ func TestS3PolicyGeneration(t *testing.T) { continue } - addS3Permissions(ip, "arn:aws", s3Path, x.Role) + addS3Permissions(ip, "arn:aws", s3Path, x.Role, x.StrictIAM) expectedPolicy, err := x.IAMPolicy.AsJSON() if err != nil { From 0e5c393f10be8d4e47fa04d959d5191ac4f1c23c Mon Sep 17 00:00:00 2001 From: Kashif Saadat Date: Tue, 22 Aug 2017 12:09:34 +0100 Subject: [PATCH 2/2] Rename IAM switch to legacy, default to false for new cluster creations. --- cmd/kops/create_cluster.go | 4 +++ cmd/kops/create_cluster_integration_test.go | 14 +++++---- pkg/apis/kops/cluster.go | 4 +-- pkg/apis/kops/v1alpha1/cluster.go | 2 +- pkg/apis/kops/v1alpha1/defaults.go | 6 ++++ .../kops/v1alpha1/zz_generated.conversion.go | 4 +-- pkg/apis/kops/v1alpha2/cluster.go | 2 +- pkg/apis/kops/v1alpha2/defaults.go | 6 ++++ .../kops/v1alpha2/zz_generated.conversion.go | 4 +-- pkg/model/iam/iam_builder.go | 31 ++++++++++--------- pkg/model/iam/iam_builder_test.go | 16 +++++----- .../complex/expected-v1alpha2.yaml | 2 ++ .../create_cluster/ha/expected-v1alpha1.yaml | 2 ++ .../create_cluster/ha/expected-v1alpha2.yaml | 2 ++ .../ha_encrypt/expected-v1alpha1.yaml | 2 ++ .../ha_encrypt/expected-v1alpha2.yaml | 2 ++ .../ha_shared_zones/expected-v1alpha2.yaml | 2 ++ .../minimal/expected-v1alpha1.yaml | 2 ++ .../minimal/expected-v1alpha2.yaml | 2 ++ .../ngwspecified/expected-v1alpha1.yaml | 2 ++ .../ngwspecified/expected-v1alpha2.yaml | 2 ++ .../private/expected-v1alpha1.yaml | 2 ++ .../private/expected-v1alpha2.yaml | 2 ++ 23 files changed, 80 insertions(+), 37 deletions(-) diff --git a/cmd/kops/create_cluster.go b/cmd/kops/create_cluster.go index ea5a48555a..5b26de2fe2 100644 --- a/cmd/kops/create_cluster.go +++ b/cmd/kops/create_cluster.go @@ -833,6 +833,10 @@ func RunCreateCluster(f *util.Factory, out io.Writer, c *CreateClusterOptions) e } } + cluster.Spec.IAM = &api.IAMSpec{ + Legacy: false, + } + sshPublicKeys := make(map[string][]byte) if c.SSHPublicKey != "" { c.SSHPublicKey = utils.ExpandPath(c.SSHPublicKey) diff --git a/cmd/kops/create_cluster_integration_test.go b/cmd/kops/create_cluster_integration_test.go index 093ff87c6e..d60e043095 100644 --- a/cmd/kops/create_cluster_integration_test.go +++ b/cmd/kops/create_cluster_integration_test.go @@ -18,17 +18,19 @@ package main import ( "bytes" - "github.com/golang/glog" "io/ioutil" + "path" + "strings" + "testing" + "time" + + "github.com/golang/glog" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/kops/cmd/kops/util" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/diff" "k8s.io/kops/pkg/testutils" - "path" - "strings" - "testing" - "time" ) var MagicTimestamp = metav1.Time{Time: time.Date(2017, 1, 1, 0, 0, 0, 0, time.UTC)} @@ -182,7 +184,7 @@ func runCreateClusterIntegrationTest(t *testing.T, srcDir string, version string diffString := diff.FormatDiff(expectedYAML, actualYAML) t.Logf("diff:\n%s\n", diffString) - t.Fatalf("YAML differed from expected") + t.Fatalf("YAML differed from expected (%s)", path.Join(srcDir, expectedClusterPath)) } } diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go index 27d41fa35f..81427ce0ed 100644 --- a/pkg/apis/kops/cluster.go +++ b/pkg/apis/kops/cluster.go @@ -155,7 +155,7 @@ type Assets struct { // IAMSpec adds control over the IAM security policies applied to resources type IAMSpec struct { - Strict bool `json:"strict"` + Legacy bool `json:"legacy"` } // HookSpec is a definition hook @@ -179,7 +179,7 @@ type HookSpec struct { // ExecContainerAction defines an hood action type ExecContainerAction struct { // Image is the docker image - Image string `json:"image,omitempty" ` + Image string `json:"image,omitempty"` // Command is the command supplied to the above image Command []string `json:"command,omitempty"` // Environment is a map of environment variables added to the hook diff --git a/pkg/apis/kops/v1alpha1/cluster.go b/pkg/apis/kops/v1alpha1/cluster.go index 350e7ee884..d4ee5c999f 100644 --- a/pkg/apis/kops/v1alpha1/cluster.go +++ b/pkg/apis/kops/v1alpha1/cluster.go @@ -261,7 +261,7 @@ type Assets struct { // IAMSpec adds control over the IAM security policies applied to resources type IAMSpec struct { - Strict bool `json:"strict"` + Legacy bool `json:"legacy"` } // HookSpec is a definition hook diff --git a/pkg/apis/kops/v1alpha1/defaults.go b/pkg/apis/kops/v1alpha1/defaults.go index 2071b06051..2c0f12abf6 100644 --- a/pkg/apis/kops/v1alpha1/defaults.go +++ b/pkg/apis/kops/v1alpha1/defaults.go @@ -75,4 +75,10 @@ func SetDefaults_ClusterSpec(obj *ClusterSpec) { obj.Authorization.AlwaysAllow = &AlwaysAllowAuthorizationSpec{} } + if obj.IAM == nil { + obj.IAM = &IAMSpec{ + Legacy: true, + } + } + } diff --git a/pkg/apis/kops/v1alpha1/zz_generated.conversion.go b/pkg/apis/kops/v1alpha1/zz_generated.conversion.go index 11acf8ef0d..c8ba1bfa4d 100644 --- a/pkg/apis/kops/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/kops/v1alpha1/zz_generated.conversion.go @@ -1321,7 +1321,7 @@ func Convert_kops_HookSpec_To_v1alpha1_HookSpec(in *kops.HookSpec, out *HookSpec } func autoConvert_v1alpha1_IAMSpec_To_kops_IAMSpec(in *IAMSpec, out *kops.IAMSpec, s conversion.Scope) error { - out.Strict = in.Strict + out.Legacy = in.Legacy return nil } @@ -1331,7 +1331,7 @@ func Convert_v1alpha1_IAMSpec_To_kops_IAMSpec(in *IAMSpec, out *kops.IAMSpec, s } func autoConvert_kops_IAMSpec_To_v1alpha1_IAMSpec(in *kops.IAMSpec, out *IAMSpec, s conversion.Scope) error { - out.Strict = in.Strict + out.Legacy = in.Legacy return nil } diff --git a/pkg/apis/kops/v1alpha2/cluster.go b/pkg/apis/kops/v1alpha2/cluster.go index 2a2f76796f..6bcc070078 100644 --- a/pkg/apis/kops/v1alpha2/cluster.go +++ b/pkg/apis/kops/v1alpha2/cluster.go @@ -187,7 +187,7 @@ type Assets struct { // IAMSpec adds control over the IAM security policies applied to resources type IAMSpec struct { - Strict bool `json:"strict"` + Legacy bool `json:"legacy"` } // HookSpec is a definition hook diff --git a/pkg/apis/kops/v1alpha2/defaults.go b/pkg/apis/kops/v1alpha2/defaults.go index 6bc933233f..9e393f6dd1 100644 --- a/pkg/apis/kops/v1alpha2/defaults.go +++ b/pkg/apis/kops/v1alpha2/defaults.go @@ -74,4 +74,10 @@ func SetDefaults_ClusterSpec(obj *ClusterSpec) { // Before the Authorization field was introduced, the behaviour was alwaysAllow obj.Authorization.AlwaysAllow = &AlwaysAllowAuthorizationSpec{} } + + if obj.IAM == nil { + obj.IAM = &IAMSpec{ + Legacy: true, + } + } } diff --git a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go index f51f21bcfd..837ef1edff 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go +++ b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go @@ -1419,7 +1419,7 @@ func Convert_kops_HookSpec_To_v1alpha2_HookSpec(in *kops.HookSpec, out *HookSpec } func autoConvert_v1alpha2_IAMSpec_To_kops_IAMSpec(in *IAMSpec, out *kops.IAMSpec, s conversion.Scope) error { - out.Strict = in.Strict + out.Legacy = in.Legacy return nil } @@ -1429,7 +1429,7 @@ func Convert_v1alpha2_IAMSpec_To_kops_IAMSpec(in *IAMSpec, out *kops.IAMSpec, s } func autoConvert_kops_IAMSpec_To_v1alpha2_IAMSpec(in *kops.IAMSpec, out *IAMSpec, s conversion.Scope) error { - out.Strict = in.Strict + out.Legacy = in.Legacy return nil } diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go index 446de1e04f..06afa9f73a 100644 --- a/pkg/model/iam/iam_builder.go +++ b/pkg/model/iam/iam_builder.go @@ -84,9 +84,10 @@ func (b *IAMPolicyBuilder) BuildAWSIAMPolicy() (*IAMPolicy, error) { wildcard := stringorslice.Slice([]string{"*"}) iamPrefix := b.IAMPrefix() - strictIAM := false + // The Legacy IAM setting deploys an open policy (prior to the hardening PRs) + legacyIAM := false if b.Cluster.Spec.IAM != nil { - strictIAM = b.Cluster.Spec.IAM.Strict + legacyIAM = b.Cluster.Spec.IAM.Legacy } p := &IAMPolicy{ @@ -237,7 +238,7 @@ func (b *IAMPolicyBuilder) BuildAWSIAMPolicy() (*IAMPolicy, error) { } if s3Path, ok := vfsPath.(*vfs.S3Path); ok { - addS3Permissions(p, iamPrefix, s3Path, b.Role, strictIAM) + addS3Permissions(p, iamPrefix, s3Path, b.Role, legacyIAM) } else if _, ok := vfsPath.(*vfs.MemFSPath); ok { // Tests -ignore - nothing we can do in terms of IAM policy glog.Warningf("ignoring memfs path %q for IAM policy builder", vfsPath) @@ -281,7 +282,7 @@ func addRoute53ListHostedZonesPermission(p *IAMPolicy) { // addS3Permissions updates the IAM Policy with statements granting tailored // access to S3 assets, depending on the instance role -func addS3Permissions(p *IAMPolicy, iamPrefix string, s3Path *vfs.S3Path, role api.InstanceGroupRole, strictIAM bool) { +func addS3Permissions(p *IAMPolicy, iamPrefix string, s3Path *vfs.S3Path, role api.InstanceGroupRole, legacyIAM bool) { // Note that the config store may itself be a subdirectory of a bucket iamS3Path := s3Path.Bucket() + "/" + s3Path.Key() iamS3Path = strings.TrimSuffix(iamS3Path, "/") @@ -303,7 +304,17 @@ func addS3Permissions(p *IAMPolicy, iamPrefix string, s3Path *vfs.S3Path, role a ), }) - if strictIAM { + if legacyIAM { + if role == api.InstanceGroupRoleMaster || role == api.InstanceGroupRoleNode { + p.Statement = append(p.Statement, &IAMStatement{ + Effect: IAMStatementEffectAllow, + Action: stringorslice.Slice([]string{"s3:*"}), + Resource: stringorslice.Of( + strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/*"}, ""), + ), + }) + } + } else { if role == api.InstanceGroupRoleMaster { p.Statement = append(p.Statement, &IAMStatement{ Effect: IAMStatementEffectAllow, @@ -329,16 +340,6 @@ func addS3Permissions(p *IAMPolicy, iamPrefix string, s3Path *vfs.S3Path, role a ), }) } - } else { - if role == api.InstanceGroupRoleMaster || role == api.InstanceGroupRoleNode { - p.Statement = append(p.Statement, &IAMStatement{ - Effect: IAMStatementEffectAllow, - Action: stringorslice.Slice([]string{"s3:*"}), - Resource: stringorslice.Of( - strings.Join([]string{iamPrefix, ":s3:::", iamS3Path, "/*"}, ""), - ), - }) - } } } diff --git a/pkg/model/iam/iam_builder_test.go b/pkg/model/iam/iam_builder_test.go index b0ed18b7a6..7439e0b00b 100644 --- a/pkg/model/iam/iam_builder_test.go +++ b/pkg/model/iam/iam_builder_test.go @@ -97,12 +97,12 @@ func TestS3PolicyGeneration(t *testing.T) { grid := []struct { Role kops.InstanceGroupRole - StrictIAM bool + LegacyIAM bool IAMPolicy IAMPolicy }{ { Role: "Master", - StrictIAM: true, + LegacyIAM: false, IAMPolicy: IAMPolicy{ Statement: append(defaultS3Statements, &IAMStatement{ Effect: IAMStatementEffectAllow, @@ -117,7 +117,7 @@ func TestS3PolicyGeneration(t *testing.T) { }, { Role: "Master", - StrictIAM: false, + LegacyIAM: true, IAMPolicy: IAMPolicy{ Statement: append(defaultS3Statements, &IAMStatement{ Effect: IAMStatementEffectAllow, @@ -132,7 +132,7 @@ func TestS3PolicyGeneration(t *testing.T) { }, { Role: "Node", - StrictIAM: true, + LegacyIAM: false, IAMPolicy: IAMPolicy{ Statement: append(defaultS3Statements, &IAMStatement{ Effect: IAMStatementEffectAllow, @@ -155,7 +155,7 @@ func TestS3PolicyGeneration(t *testing.T) { }, { Role: "Node", - StrictIAM: false, + LegacyIAM: true, IAMPolicy: IAMPolicy{ Statement: append(defaultS3Statements, &IAMStatement{ Effect: IAMStatementEffectAllow, @@ -170,14 +170,14 @@ func TestS3PolicyGeneration(t *testing.T) { }, { Role: "Bastion", - StrictIAM: true, + LegacyIAM: false, IAMPolicy: IAMPolicy{ Statement: defaultS3Statements, }, }, { Role: "Bastion", - StrictIAM: false, + LegacyIAM: true, IAMPolicy: IAMPolicy{ Statement: defaultS3Statements, }, @@ -198,7 +198,7 @@ func TestS3PolicyGeneration(t *testing.T) { continue } - addS3Permissions(ip, "arn:aws", s3Path, x.Role, x.StrictIAM) + addS3Permissions(ip, "arn:aws", s3Path, x.Role, x.LegacyIAM) expectedPolicy, err := x.IAMPolicy.AsJSON() if err != nil { diff --git a/tests/integration/create_cluster/complex/expected-v1alpha2.yaml b/tests/integration/create_cluster/complex/expected-v1alpha2.yaml index 0b39bdd0f0..42eec055a4 100644 --- a/tests/integration/create_cluster/complex/expected-v1alpha2.yaml +++ b/tests/integration/create_cluster/complex/expected-v1alpha2.yaml @@ -20,6 +20,8 @@ spec: - instanceGroup: master-us-test-1a name: a name: events + iam: + legacy: false kubernetesApiAccess: - 0.0.0.0/0 kubernetesVersion: v1.7.1 diff --git a/tests/integration/create_cluster/ha/expected-v1alpha1.yaml b/tests/integration/create_cluster/ha/expected-v1alpha1.yaml index b19e2d939f..f30c6c8690 100644 --- a/tests/integration/create_cluster/ha/expected-v1alpha1.yaml +++ b/tests/integration/create_cluster/ha/expected-v1alpha1.yaml @@ -30,6 +30,8 @@ spec: - name: c zone: us-test-1c name: events + iam: + legacy: false kubernetesVersion: v1.6.0-alpha.3 masterPublicName: api.ha.example.com networkCIDR: 172.20.0.0/16 diff --git a/tests/integration/create_cluster/ha/expected-v1alpha2.yaml b/tests/integration/create_cluster/ha/expected-v1alpha2.yaml index ad934ba187..2790180527 100644 --- a/tests/integration/create_cluster/ha/expected-v1alpha2.yaml +++ b/tests/integration/create_cluster/ha/expected-v1alpha2.yaml @@ -28,6 +28,8 @@ spec: - instanceGroup: master-us-test-1c name: c name: events + iam: + legacy: false kubernetesApiAccess: - 0.0.0.0/0 kubernetesVersion: v1.6.0-alpha.3 diff --git a/tests/integration/create_cluster/ha_encrypt/expected-v1alpha1.yaml b/tests/integration/create_cluster/ha_encrypt/expected-v1alpha1.yaml index 1b698240d6..97f1d85f18 100644 --- a/tests/integration/create_cluster/ha_encrypt/expected-v1alpha1.yaml +++ b/tests/integration/create_cluster/ha_encrypt/expected-v1alpha1.yaml @@ -36,6 +36,8 @@ spec: name: c zone: us-test-1c name: events + iam: + legacy: false kubernetesVersion: v1.6.0-alpha.3 masterPublicName: api.ha.example.com networkCIDR: 172.20.0.0/16 diff --git a/tests/integration/create_cluster/ha_encrypt/expected-v1alpha2.yaml b/tests/integration/create_cluster/ha_encrypt/expected-v1alpha2.yaml index cc6cfbe56f..bd6e17ea8d 100644 --- a/tests/integration/create_cluster/ha_encrypt/expected-v1alpha2.yaml +++ b/tests/integration/create_cluster/ha_encrypt/expected-v1alpha2.yaml @@ -34,6 +34,8 @@ spec: instanceGroup: master-us-test-1c name: c name: events + iam: + legacy: false kubernetesApiAccess: - 0.0.0.0/0 kubernetesVersion: v1.6.0-alpha.3 diff --git a/tests/integration/create_cluster/ha_shared_zones/expected-v1alpha2.yaml b/tests/integration/create_cluster/ha_shared_zones/expected-v1alpha2.yaml index 038394bb00..4a614b35fa 100644 --- a/tests/integration/create_cluster/ha_shared_zones/expected-v1alpha2.yaml +++ b/tests/integration/create_cluster/ha_shared_zones/expected-v1alpha2.yaml @@ -36,6 +36,8 @@ spec: - instanceGroup: master-us-test-1a-3 name: a-3 name: events + iam: + legacy: false kubernetesApiAccess: - 0.0.0.0/0 kubernetesVersion: v1.4.8 diff --git a/tests/integration/create_cluster/minimal/expected-v1alpha1.yaml b/tests/integration/create_cluster/minimal/expected-v1alpha1.yaml index 475345007b..d0efe7d3ac 100644 --- a/tests/integration/create_cluster/minimal/expected-v1alpha1.yaml +++ b/tests/integration/create_cluster/minimal/expected-v1alpha1.yaml @@ -22,6 +22,8 @@ spec: - name: a zone: us-test-1a name: events + iam: + legacy: false kubernetesVersion: v1.4.8 masterPublicName: api.minimal.example.com networkCIDR: 172.20.0.0/16 diff --git a/tests/integration/create_cluster/minimal/expected-v1alpha2.yaml b/tests/integration/create_cluster/minimal/expected-v1alpha2.yaml index 1ae576be71..be82e866a0 100644 --- a/tests/integration/create_cluster/minimal/expected-v1alpha2.yaml +++ b/tests/integration/create_cluster/minimal/expected-v1alpha2.yaml @@ -20,6 +20,8 @@ spec: - instanceGroup: master-us-test-1a name: a name: events + iam: + legacy: false kubernetesApiAccess: - 0.0.0.0/0 kubernetesVersion: v1.4.8 diff --git a/tests/integration/create_cluster/ngwspecified/expected-v1alpha1.yaml b/tests/integration/create_cluster/ngwspecified/expected-v1alpha1.yaml index 2145e85140..529ffa009e 100644 --- a/tests/integration/create_cluster/ngwspecified/expected-v1alpha1.yaml +++ b/tests/integration/create_cluster/ngwspecified/expected-v1alpha1.yaml @@ -23,6 +23,8 @@ spec: - name: a zone: us-test-1a name: events + iam: + legacy: false kubernetesVersion: v1.4.8 masterPublicName: api.private.example.com networkCIDR: 172.20.0.0/16 diff --git a/tests/integration/create_cluster/ngwspecified/expected-v1alpha2.yaml b/tests/integration/create_cluster/ngwspecified/expected-v1alpha2.yaml index b0fb601546..9e6baeb666 100644 --- a/tests/integration/create_cluster/ngwspecified/expected-v1alpha2.yaml +++ b/tests/integration/create_cluster/ngwspecified/expected-v1alpha2.yaml @@ -21,6 +21,8 @@ spec: - instanceGroup: master-us-test-1a name: a name: events + iam: + legacy: false kubernetesApiAccess: - 0.0.0.0/0 kubernetesVersion: v1.4.8 diff --git a/tests/integration/create_cluster/private/expected-v1alpha1.yaml b/tests/integration/create_cluster/private/expected-v1alpha1.yaml index bd75637319..d4b4326823 100644 --- a/tests/integration/create_cluster/private/expected-v1alpha1.yaml +++ b/tests/integration/create_cluster/private/expected-v1alpha1.yaml @@ -27,6 +27,8 @@ spec: - name: a zone: us-test-1a name: events + iam: + legacy: false kubernetesVersion: v1.4.8 masterPublicName: api.private.example.com networkCIDR: 172.20.0.0/16 diff --git a/tests/integration/create_cluster/private/expected-v1alpha2.yaml b/tests/integration/create_cluster/private/expected-v1alpha2.yaml index e5ee0a81f7..38536d80d5 100644 --- a/tests/integration/create_cluster/private/expected-v1alpha2.yaml +++ b/tests/integration/create_cluster/private/expected-v1alpha2.yaml @@ -25,6 +25,8 @@ spec: - instanceGroup: master-us-test-1a name: a name: events + iam: + legacy: false kubernetesApiAccess: - 0.0.0.0/0 kubernetesVersion: v1.4.8