Skip to content

Commit

Permalink
Merge pull request #16351 from rifelpet/iam-policy-refactor
Browse files Browse the repository at this point in the history 
Refactor IAM Policy Builder
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Feb 13, 2024
2 parents 5a3b1e2 + 4643c66 commit 9f43b03
Show file tree
Hide file tree
Showing 96 changed files with 654 additions and 652 deletions.
4 changes: 2 additions & 2 deletions pkg/model/awsmodel/iam.go
View file
Expand Up @@ -28,7 +28,7 @@ import (
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/model"
"k8s.io/kops/pkg/model/iam"
"k8s.io/kops/pkg/util/stringorslice"
"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"
Expand Down Expand Up @@ -432,7 +432,7 @@ func formatAWSIAMStatement(accountId, partition, oidcProvider, namespace, name s
Principal: iam.Principal{
Federated: "arn:" + partition + ":iam::" + accountId + ":oidc-provider/" + oidcProvider,
},
Action: stringorslice.String("sts:AssumeRoleWithWebIdentity"),
Action: stringorset.String("sts:AssumeRoleWithWebIdentity"),
Condition: map[string]interface{}{
condition: map[string]interface{}{
oidcProvider + ":sub": "system:serviceaccount:" + namespace + ":" + name,
Expand Down
6 changes: 3 additions & 3 deletions pkg/model/awsmodel/iam_test.go
View file
Expand Up @@ -21,7 +21,7 @@ import (
"testing"

"k8s.io/kops/pkg/model/iam"
"k8s.io/kops/pkg/util/stringorslice"
"k8s.io/kops/pkg/util/stringorset"
)

func TestIAMServiceEC2(t *testing.T) {
Expand Down Expand Up @@ -70,7 +70,7 @@ func Test_formatAWSIAMStatement(t *testing.T) {
Principal: iam.Principal{
Federated: "arn:aws-test:iam::0123456789:oidc-provider/oidc-test",
},
Action: stringorslice.String("sts:AssumeRoleWithWebIdentity"),
Action: stringorset.String("sts:AssumeRoleWithWebIdentity"),
Condition: map[string]interface{}{
"StringEquals": map[string]interface{}{
"oidc-test:sub": "system:serviceaccount:test:test",
Expand Down Expand Up @@ -104,7 +104,7 @@ func Test_formatAWSIAMStatement(t *testing.T) {
Principal: iam.Principal{
Federated: "arn:aws-test:iam::0123456789:oidc-provider/oidc-test",
},
Action: stringorslice.String("sts:AssumeRoleWithWebIdentity"),
Action: stringorset.String("sts:AssumeRoleWithWebIdentity"),
Condition: map[string]interface{}{
"StringLike": map[string]interface{}{
"oidc-test:sub": "system:serviceaccount:test-*:test",
Expand Down
14 changes: 7 additions & 7 deletions pkg/model/components/addonmanifests/certmanager/iam.go
View file
Expand Up @@ -22,7 +22,7 @@ import (
"k8s.io/apimachinery/pkg/types"

"k8s.io/kops/pkg/model/iam"
"k8s.io/kops/pkg/util/stringorslice"
"k8s.io/kops/pkg/util/stringorset"
)

// ServiceAccount represents the service-account used by cert-manager.
Expand Down Expand Up @@ -57,22 +57,22 @@ func addCertManagerPermissions(b *iam.PolicyBuilder, p *iam.Policy) {

p.Statement = append(p.Statement, &iam.Statement{
Effect: iam.StatementEffectAllow,
Action: stringorslice.Of("route53:ChangeResourceRecordSets",
Action: stringorset.Of("route53:ChangeResourceRecordSets",
"route53:ListResourceRecordSets",
),
Resource: stringorslice.Slice(zoneResources),
Resource: stringorset.Set(zoneResources),
})

p.Statement = append(p.Statement, &iam.Statement{
Effect: iam.StatementEffectAllow,
Action: stringorslice.Slice([]string{"route53:GetChange"}),
Resource: stringorslice.Slice([]string{fmt.Sprintf("arn:%v:route53:::change/*", b.Partition)}),
Action: stringorset.Set([]string{"route53:GetChange"}),
Resource: stringorset.Set([]string{fmt.Sprintf("arn:%v:route53:::change/*", b.Partition)}),
})

wildcard := stringorslice.Slice([]string{"*"})
wildcard := stringorset.Set([]string{"*"})
p.Statement = append(p.Statement, &iam.Statement{
Effect: iam.StatementEffectAllow,
Action: stringorslice.Slice([]string{"route53:ListHostedZonesByName"}),
Action: stringorset.Set([]string{"route53:ListHostedZonesByName"}),
Resource: wildcard,
})
}
74 changes: 37 additions & 37 deletions pkg/model/iam/iam_builder.go
View file
Expand Up @@ -38,7 +38,7 @@ import (

"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/kops/model"
"k8s.io/kops/pkg/util/stringorslice"
"k8s.io/kops/pkg/util/stringorset"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
"k8s.io/kops/util/pkg/vfs"
Expand All @@ -50,9 +50,9 @@ const PolicyDefaultVersion = "2012-10-17"
// Policy Struct is a collection of fields that form a valid AWS policy document
type Policy struct {
clusterName string
unconditionalAction sets.String
clusterTaggedAction sets.String
clusterTaggedCreateAction sets.String
unconditionalAction sets.Set[string]
clusterTaggedAction sets.Set[string]
clusterTaggedCreateAction sets.Set[string]
Statement []*Statement
partition string
Version string
Expand All @@ -77,8 +77,8 @@ func (p *Policy) AddEC2CreateAction(actions, resources []string) {
p.Statement = append(p.Statement,
&Statement{
Effect: StatementEffectAllow,
Action: stringorslice.String("ec2:CreateTags"),
Resource: stringorslice.Slice(actualResources),
Action: stringorset.String("ec2:CreateTags"),
Resource: stringorset.Set(actualResources),
Condition: Condition{
"StringEquals": map[string]interface{}{
"aws:RequestTag/KubernetesCluster": p.clusterName,
Expand All @@ -89,11 +89,11 @@ func (p *Policy) AddEC2CreateAction(actions, resources []string) {

&Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Slice([]string{
Action: stringorset.Set([]string{
"ec2:CreateTags",
"ec2:DeleteTags", // aws.go, tag.go
}),
Resource: stringorslice.Slice(actualResources),
Resource: stringorset.Set(actualResources),
Condition: Condition{
"Null": map[string]string{
"aws:RequestTag/KubernetesCluster": "true",
Expand All @@ -111,15 +111,15 @@ func (p *Policy) AsJSON() (string, error) {
if len(p.unconditionalAction) > 0 {
p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Of(p.unconditionalAction.List()...),
Resource: stringorslice.String("*"),
Action: stringorset.Of(sets.List(p.unconditionalAction)...),
Resource: stringorset.String("*"),
})
}
if len(p.clusterTaggedAction) > 0 {
p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Of(p.clusterTaggedAction.List()...),
Resource: stringorslice.String("*"),
Action: stringorset.Of(sets.List(p.clusterTaggedAction)...),
Resource: stringorset.String("*"),
Condition: Condition{
"StringEquals": map[string]string{
"aws:ResourceTag/KubernetesCluster": p.clusterName,
Expand All @@ -130,8 +130,8 @@ func (p *Policy) AsJSON() (string, error) {
if len(p.clusterTaggedCreateAction) > 0 {
p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Of(p.clusterTaggedCreateAction.List()...),
Resource: stringorslice.String("*"),
Action: stringorset.Of(sets.List(p.clusterTaggedCreateAction)...),
Resource: stringorset.String("*"),
Condition: Condition{
"StringEquals": map[string]string{
"aws:RequestTag/KubernetesCluster": p.clusterName,
Expand All @@ -143,8 +143,8 @@ func (p *Policy) AsJSON() (string, error) {
if p.clusterTaggedCreateAction.Has("ec2:CreateSecurityGroup") {
p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Of("ec2:CreateSecurityGroup"),
Resource: stringorslice.String(fmt.Sprintf("arn:%s:ec2:*:*:vpc/*", p.partition)),
Action: stringorset.Of("ec2:CreateSecurityGroup"),
Resource: stringorset.String(fmt.Sprintf("arn:%s:ec2:*:*:vpc/*", p.partition)),
})
}
}
Expand Down Expand Up @@ -176,8 +176,8 @@ type Condition map[string]interface{}
type Statement struct {
Effect StatementEffect
Principal Principal
Action stringorslice.StringOrSlice
Resource stringorslice.StringOrSlice
Action stringorset.StringOrSet
Resource stringorset.StringOrSet
Condition Condition
}

Expand Down Expand Up @@ -338,9 +338,9 @@ func NewPolicy(clusterName, partition string) *Policy {
p := &Policy{
Version: PolicyDefaultVersion,
clusterName: clusterName,
unconditionalAction: sets.NewString(),
clusterTaggedAction: sets.NewString(),
clusterTaggedCreateAction: sets.NewString(),
unconditionalAction: sets.New[string](),
clusterTaggedAction: sets.New[string](),
clusterTaggedCreateAction: sets.New[string](),
partition: partition,
}
return p
Expand Down Expand Up @@ -594,13 +594,13 @@ func (b *PolicyBuilder) AddS3Permissions(p *Policy) (*Policy, error) {
for _, s3Bucket := range s3Buckets.List() {
p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Of(
Action: stringorset.Of(
"s3:GetBucketLocation",
"s3:GetEncryptionConfiguration",
"s3:ListBucket",
"s3:ListBucketVersions",
),
Resource: stringorslice.Slice([]string{
Resource: stringorset.Set([]string{
fmt.Sprintf("arn:%v:s3:::%v", p.partition, s3Bucket),
}),
})
Expand All @@ -612,13 +612,13 @@ func (b *PolicyBuilder) AddS3Permissions(p *Policy) (*Policy, error) {
func (b *PolicyBuilder) buildS3WriteStatements(p *Policy, iamS3Path string) {
p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Slice([]string{
Action: stringorset.Set([]string{
"s3:GetObject",
"s3:DeleteObject",
"s3:DeleteObjectVersion",
"s3:PutObject",
}),
Resource: stringorslice.Of(
Resource: stringorset.Of(
fmt.Sprintf("arn:%v:s3:::%v/*", p.partition, iamS3Path),
),
})
Expand All @@ -640,8 +640,8 @@ func (b *PolicyBuilder) buildS3GetStatements(p *Policy, iamS3Path string) error

p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Slice([]string{"s3:Get*"}),
Resource: stringorslice.Of(resources...),
Action: stringorset.Set([]string{"s3:Get*"}),
Resource: stringorset.Of(resources...),
})
}
return nil
Expand Down Expand Up @@ -800,10 +800,10 @@ func addEtcdManagerPermissions(p *Policy) {
p.Statement = append(p.Statement,
&Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Of(
Action: stringorset.Of(
"ec2:AttachVolume",
),
Resource: stringorslice.Slice([]string{"*"}),
Resource: stringorset.Set([]string{"*"}),
Condition: Condition{
"StringEquals": map[string]string{
"aws:ResourceTag/k8s.io/role/master": "1",
Expand Down Expand Up @@ -1061,22 +1061,22 @@ func AddDNSControllerPermissions(b *PolicyBuilder, p *Policy) {

p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Of("route53:ChangeResourceRecordSets",
Action: stringorset.Of("route53:ChangeResourceRecordSets",
"route53:ListResourceRecordSets",
"route53:GetHostedZone"),
Resource: stringorslice.Slice([]string{fmt.Sprintf("arn:%v:route53:::hostedzone/%v", b.Partition, hostedZoneID)}),
Resource: stringorset.Set([]string{fmt.Sprintf("arn:%v:route53:::hostedzone/%v", b.Partition, hostedZoneID)}),
})

p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Slice([]string{"route53:GetChange"}),
Resource: stringorslice.Slice([]string{fmt.Sprintf("arn:%v:route53:::change/*", b.Partition)}),
Action: stringorset.Set([]string{"route53:GetChange"}),
Resource: stringorset.Set([]string{fmt.Sprintf("arn:%v:route53:::change/*", b.Partition)}),
})

wildcard := stringorslice.Slice([]string{"*"})
wildcard := stringorset.Set([]string{"*"})
p.Statement = append(p.Statement, &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Slice([]string{"route53:ListHostedZones", "route53:ListTagsForResource"}),
Action: stringorset.Set([]string{"route53:ListHostedZones", "route53:ListTagsForResource"}),
Resource: wildcard,
})
}
Expand Down Expand Up @@ -1169,10 +1169,10 @@ func addAmazonVPCCNIPermissions(p *Policy) {
p.Statement = append(p.Statement,
&Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Slice([]string{
Action: stringorset.Set([]string{
"ec2:CreateTags",
}),
Resource: stringorslice.Slice([]string{
Resource: stringorset.Set([]string{
strings.Join([]string{"arn:", p.partition, ":ec2:*:*:network-interface/*"}, ""),
}),
},
Expand Down
12 changes: 6 additions & 6 deletions pkg/model/iam/iam_builder_test.go
View file
Expand Up @@ -26,7 +26,7 @@ import (
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/testutils"
"k8s.io/kops/pkg/testutils/golden"
"k8s.io/kops/pkg/util/stringorslice"
"k8s.io/kops/pkg/util/stringorset"
"k8s.io/kops/upup/pkg/fi"
)

Expand All @@ -38,18 +38,18 @@ func TestRoundTrip(t *testing.T) {
{
IAM: &Statement{
Effect: StatementEffectAllow,
Action: stringorslice.Of("ec2:DescribeRegions"),
Resource: stringorslice.Of("*"),
Action: stringorset.Of("ec2:DescribeRegions"),
Resource: stringorset.Of("*"),
},
JSON: "{\"Action\":\"ec2:DescribeRegions\",\"Effect\":\"Allow\",\"Resource\":\"*\"}",
},
{
IAM: &Statement{
Effect: StatementEffectDeny,
Action: stringorslice.Of("ec2:DescribeRegions", "ec2:DescribeInstances"),
Resource: stringorslice.Of("a", "b"),
Action: stringorset.Of("ec2:DescribeRegions", "ec2:DescribeInstances"),
Resource: stringorset.Of("a", "b"),
},
JSON: "{\"Action\":[\"ec2:DescribeRegions\",\"ec2:DescribeInstances\"],\"Effect\":\"Deny\",\"Resource\":[\"a\",\"b\"]}",
JSON: "{\"Action\":[\"ec2:DescribeInstances\",\"ec2:DescribeRegions\"],\"Effect\":\"Deny\",\"Resource\":[\"a\",\"b\"]}",
},
{
IAM: &Statement{
Expand Down
8 changes: 4 additions & 4 deletions pkg/model/iam/tests/iam_builder_master_gossip.json
View file
Expand Up @@ -45,8 +45,8 @@
},
"Effect": "Allow",
"Resource": [
"arn:aws-test:ec2:*:*:volume/*",
"arn:aws-test:ec2:*:*:snapshot/*"
"arn:aws-test:ec2:*:*:snapshot/*",
"arn:aws-test:ec2:*:*:volume/*"
]
},
{
Expand All @@ -64,8 +64,8 @@
},
"Effect": "Allow",
"Resource": [
"arn:aws-test:ec2:*:*:volume/*",
"arn:aws-test:ec2:*:*:snapshot/*"
"arn:aws-test:ec2:*:*:snapshot/*",
"arn:aws-test:ec2:*:*:volume/*"
]
},
{
Expand Down
8 changes: 4 additions & 4 deletions pkg/model/iam/tests/iam_builder_master_gossip_ecr.json
View file
Expand Up @@ -45,8 +45,8 @@
},
"Effect": "Allow",
"Resource": [
"arn:aws-test:ec2:*:*:volume/*",
"arn:aws-test:ec2:*:*:snapshot/*"
"arn:aws-test:ec2:*:*:snapshot/*",
"arn:aws-test:ec2:*:*:volume/*"
]
},
{
Expand All @@ -64,8 +64,8 @@
},
"Effect": "Allow",
"Resource": [
"arn:aws-test:ec2:*:*:volume/*",
"arn:aws-test:ec2:*:*:snapshot/*"
"arn:aws-test:ec2:*:*:snapshot/*",
"arn:aws-test:ec2:*:*:volume/*"
]
},
{
Expand Down
8 changes: 4 additions & 4 deletions pkg/model/iam/tests/iam_builder_master_strict.json
View file
Expand Up @@ -45,8 +45,8 @@
},
"Effect": "Allow",
"Resource": [
"arn:aws-test:ec2:*:*:volume/*",
"arn:aws-test:ec2:*:*:snapshot/*"
"arn:aws-test:ec2:*:*:snapshot/*",
"arn:aws-test:ec2:*:*:volume/*"
]
},
{
Expand All @@ -64,8 +64,8 @@
},
"Effect": "Allow",
"Resource": [
"arn:aws-test:ec2:*:*:volume/*",
"arn:aws-test:ec2:*:*:snapshot/*"
"arn:aws-test:ec2:*:*:snapshot/*",
"arn:aws-test:ec2:*:*:volume/*"
]
},
{
Expand Down

0 comments on commit 9f43b03

Please sign in to comment.