Skip to content

Commit

Permalink
Make the IAM ECR Permissions optional, can be specified within the Cl…
Browse files Browse the repository at this point in the history 
…uster Spec.
  • Loading branch information
@KashifSaadat
KashifSaadat committed Oct 24, 2017
1 parent 28c4b7a commit 5bfb22a
Show file tree
Hide file tree
Showing 14 changed files with 360 additions and 123 deletions.
3 changes: 2 additions & 1 deletion pkg/apis/kops/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ type Assets struct {

// IAMSpec adds control over the IAM security policies applied to resources
type IAMSpec struct {
Legacy bool `json:"legacy"`
Legacy bool `json:"legacy"`
AllowContainerRegistry bool `json:"allowContainerRegistry,omitempty"`
}

// HookSpec is a definition hook
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/kops/v1alpha1/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ type Assets struct {

// IAMSpec adds control over the IAM security policies applied to resources
type IAMSpec struct {
Legacy bool `json:"legacy"`
Legacy bool `json:"legacy"`
AllowContainerRegistry bool `json:"allowContainerRegistry,omitempty"`
}

// HookSpec is a definition hook
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/kops/v1alpha1/zz_generated.conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1543,6 +1543,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.Legacy = in.Legacy
out.AllowContainerRegistry = in.AllowContainerRegistry
return nil
}

Expand All @@ -1553,6 +1554,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.Legacy = in.Legacy
out.AllowContainerRegistry = in.AllowContainerRegistry
return nil
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/kops/v1alpha2/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ type Assets struct {

// IAMSpec adds control over the IAM security policies applied to resources
type IAMSpec struct {
Legacy bool `json:"legacy"`
Legacy bool `json:"legacy"`
AllowContainerRegistry bool `json:"allowContainerRegistry,omitempty"`
}

// HookSpec is a definition hook
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/kops/v1alpha2/zz_generated.conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1652,6 +1652,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.Legacy = in.Legacy
out.AllowContainerRegistry = in.AllowContainerRegistry
return nil
}

Expand All @@ -1662,6 +1663,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.Legacy = in.Legacy
out.AllowContainerRegistry = in.AllowContainerRegistry
return nil
}

Expand Down
10 changes: 8 additions & 2 deletions pkg/model/iam/iam_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ func (b *PolicyBuilder) BuildAWSPolicyMaster() (*Policy, error) {
addMasterASPolicies(p, resource, b.Cluster.Spec.IAM.Legacy, b.Cluster.GetName())
addMasterELBPolicies(p, resource, b.Cluster.Spec.IAM.Legacy)
addCertIAMPolicies(p, resource)
addECRPermissions(p)

var err error
if p, err = b.AddS3Permissions(p); err != nil {
Expand All @@ -177,6 +176,10 @@ func (b *PolicyBuilder) BuildAWSPolicyMaster() (*Policy, error) {
addRoute53ListHostedZonesPermission(p)
}

if b.Cluster.Spec.IAM.Legacy || b.Cluster.Spec.IAM.AllowContainerRegistry {
addECRPermissions(p)
}

return p, nil
}

Expand All @@ -189,7 +192,6 @@ func (b *PolicyBuilder) BuildAWSPolicyNode() (*Policy, error) {
}

addNodeEC2Policies(p, resource)
addECRPermissions(p)

var err error
if p, err = b.AddS3Permissions(p); err != nil {
Expand All @@ -203,6 +205,10 @@ func (b *PolicyBuilder) BuildAWSPolicyNode() (*Policy, error) {
addRoute53ListHostedZonesPermission(p)
}

if b.Cluster.Spec.IAM.Legacy || b.Cluster.Spec.IAM.AllowContainerRegistry {
addECRPermissions(p)
}

return p, nil
}

Expand Down
70 changes: 48 additions & 22 deletions pkg/model/iam/iam_builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,39 +78,64 @@ func TestRoundTrip(t *testing.T) {

func TestPolicyGeneration(t *testing.T) {
grid := []struct {
Role kops.InstanceGroupRole
LegacyIAM bool
Policy string
Role kops.InstanceGroupRole
LegacyIAM bool
AllowContainerRegistry bool
Policy string
}{
{
Role: "Master",
LegacyIAM: true,
Policy: "tests/iam_builder_master_legacy.json",
Role: "Master",
LegacyIAM: true,
AllowContainerRegistry: false,
Policy: "tests/iam_builder_master_legacy.json",
},
{
Role: "Master",
LegacyIAM: false,
Policy: "tests/iam_builder_master_strict.json",
Role: "Master",
LegacyIAM: false,
AllowContainerRegistry: false,
Policy: "tests/iam_builder_master_strict.json",
},
{
Role: "Node",
LegacyIAM: true,
Policy: "tests/iam_builder_node_legacy.json",
Role: "Master",
LegacyIAM: false,
AllowContainerRegistry: true,
Policy: "tests/iam_builder_master_strict_ecr.json",
},
{
Role: "Node",
LegacyIAM: false,
Policy: "tests/iam_builder_node_strict.json",
Role: "Node",
LegacyIAM: true,
AllowContainerRegistry: false,
Policy: "tests/iam_builder_node_legacy.json",
},
{
Role: "Bastion",
LegacyIAM: true,
Policy: "tests/iam_builder_bastion.json",
Role: "Node",
LegacyIAM: false,
AllowContainerRegistry: false,
Policy: "tests/iam_builder_node_strict.json",
},
{
Role: "Bastion",
LegacyIAM: false,
Policy: "tests/iam_builder_bastion.json",
Role: "Node",
LegacyIAM: false,
AllowContainerRegistry: true,
Policy: "tests/iam_builder_node_strict_ecr.json",
},
{
Role: "Bastion",
LegacyIAM: true,
AllowContainerRegistry: false,
Policy: "tests/iam_builder_bastion.json",
},
{
Role: "Bastion",
LegacyIAM: false,
AllowContainerRegistry: false,
Policy: "tests/iam_builder_bastion.json",
},
{
Role: "Bastion",
LegacyIAM: false,
AllowContainerRegistry: true,
Policy: "tests/iam_builder_bastion.json",
},
}

Expand All @@ -120,7 +145,8 @@ func TestPolicyGeneration(t *testing.T) {
Spec: kops.ClusterSpec{
ConfigStore: "s3://kops-tests/iam-builder-test.k8s.local",
IAM: &kops.IAMSpec{
Legacy: x.LegacyIAM,
Legacy: x.LegacyIAM,
AllowContainerRegistry: x.AllowContainerRegistry,
},
EtcdClusters: []*kops.EtcdClusterSpec{
{
Expand Down
32 changes: 16 additions & 16 deletions pkg/model/iam/tests/iam_builder_master_legacy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,6 @@
"*"
]
},
{
"Sid": "kopsK8sECR",
"Effect": "Allow",
"Action": [
"ecr:GetAuthorizationToken",
"ecr:BatchCheckLayerAvailability",
"ecr:GetDownloadUrlForLayer",
"ecr:GetRepositoryPolicy",
"ecr:DescribeRepositories",
"ecr:ListImages",
"ecr:BatchGetImage"
],
"Resource": [
"*"
]
},
{
"Sid": "kopsK8sS3GetListBucket",
"Effect": "Allow",
Expand Down Expand Up @@ -122,6 +106,22 @@
"Resource": [
"*"
]
},
{
"Sid": "kopsK8sECR",
"Effect": "Allow",
"Action": [
"ecr:GetAuthorizationToken",
"ecr:BatchCheckLayerAvailability",
"ecr:GetDownloadUrlForLayer",
"ecr:GetRepositoryPolicy",
"ecr:DescribeRepositories",
"ecr:ListImages",
"ecr:BatchGetImage"
],
"Resource": [
"*"
]
}
]
}
16 changes: 0 additions & 16 deletions pkg/model/iam/tests/iam_builder_master_strict.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,6 @@
"*"
]
},
{
"Sid": "kopsK8sECR",
"Effect": "Allow",
"Action": [
"ecr:GetAuthorizationToken",
"ecr:BatchCheckLayerAvailability",
"ecr:GetDownloadUrlForLayer",
"ecr:GetRepositoryPolicy",
"ecr:DescribeRepositories",
"ecr:ListImages",
"ecr:BatchGetImage"
],
"Resource": [
"*"
]
},
{
"Sid": "kopsK8sS3GetListBucket",
"Effect": "Allow",
Expand Down
Loading

0 comments on commit 5bfb22a

Please sign in to comment.