Merge pull request #12342 from eddycharly/irsa-wildcard

feat: add support for wildcard in roles generated for IRSA
This commit is contained in:
Kubernetes Prow Robot 2021-09-22 16:09:10 -07:00 committed by GitHub
commit 74f9a8e2fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 198 additions and 38 deletions

View File

@ -652,6 +652,7 @@ func TestCustomIRSA(t *testing.T) {
newIntegrationTest("minimal.example.com", "irsa").
withOIDCDiscovery().
withServiceAccountRole("myserviceaccount.default", false).
withServiceAccountRole("myserviceaccount.test-wildcard", false).
withServiceAccountRole("myotherserviceaccount.myapp", true).
withAddons(dnsControllerAddon).
runTestTerraformAWS(t)

View File

@ -57,6 +57,7 @@ go_test(
"//pkg/model:go_default_library",
"//pkg/model/iam:go_default_library",
"//pkg/testutils:go_default_library",
"//pkg/util/stringorslice:go_default_library",
"//upup/pkg/fi:go_default_library",
"//upup/pkg/fi/cloudup/awstasks:go_default_library",
"//upup/pkg/fi/fitasks:go_default_library",

View File

@ -256,7 +256,7 @@ func (b *IAMModelBuilder) buildIAMRolePolicy(role iam.Subject, iamName string, i
func (b *IAMModelBuilder) roleKey(role iam.Subject) (string, bool) {
serviceAccount, ok := role.ServiceAccount()
if ok {
return strings.ToLower(serviceAccount.Namespace + "-" + serviceAccount.Name), true
return strings.ToLower(strings.ReplaceAll(serviceAccount.Namespace+"-"+serviceAccount.Name, "*", "wildcard")), true
}
// This isn't great, but we have to be backwards compatible with the old names.
@ -405,6 +405,33 @@ func IAMServiceEC2(region string) string {
return "ec2.amazonaws.com"
}
func formatAWSIAMStatement(accountId, oidcProvider, namespace, name string) (*iam.Statement, error) {
// disallow wildcard in the service account name
if strings.Contains(name, "*") {
return nil, fmt.Errorf("service account name cannot contain a wildcard %s", name)
}
// if the namespace contains a wildcard, use StringLike condition instead of StringEquals
condition := "StringEquals"
if strings.Contains(namespace, "*") {
condition = "StringLike"
}
return &iam.Statement{
Effect: "Allow",
Principal: iam.Principal{
Federated: "arn:aws:iam::" + accountId + ":oidc-provider/" + oidcProvider,
},
Action: stringorslice.String("sts:AssumeRoleWithWebIdentity"),
Condition: map[string]interface{}{
condition: map[string]interface{}{
oidcProvider + ":sub": "system:serviceaccount:" + namespace + ":" + name,
},
},
},
nil
}
// buildAWSIAMRolePolicy produces the AWS IAM role policy for the given role.
func (b *IAMModelBuilder) buildAWSIAMRolePolicy(role iam.Subject) (fi.Resource, error) {
var policy string
@ -412,22 +439,14 @@ func (b *IAMModelBuilder) buildAWSIAMRolePolicy(role iam.Subject) (fi.Resource,
if ok {
oidcProvider := strings.TrimPrefix(*b.Cluster.Spec.KubeAPIServer.ServiceAccountIssuer, "https://")
statement, err := formatAWSIAMStatement(b.AWSAccountID, oidcProvider, serviceAccount.Namespace, serviceAccount.Name)
if err != nil {
return nil, err
}
iamPolicy := &iam.Policy{
Version: iam.PolicyDefaultVersion,
Statement: []*iam.Statement{
{
Effect: "Allow",
Principal: iam.Principal{
Federated: "arn:aws:iam::" + b.AWSAccountID + ":oidc-provider/" + oidcProvider,
},
Action: stringorslice.String("sts:AssumeRoleWithWebIdentity"),
Condition: map[string]interface{}{
"StringEquals": map[string]interface{}{
oidcProvider + ":sub": "system:serviceaccount:" + serviceAccount.Namespace + ":" + serviceAccount.Name,
},
},
},
},
Version: iam.PolicyDefaultVersion,
Statement: []*iam.Statement{statement},
}
s, err := iamPolicy.AsJSON()
if err != nil {

View File

@ -17,7 +17,11 @@ limitations under the License.
package awsmodel
import (
"reflect"
"testing"
"k8s.io/kops/pkg/model/iam"
"k8s.io/kops/pkg/util/stringorslice"
)
func TestIAMServiceEC2(t *testing.T) {
@ -36,3 +40,85 @@ func TestIAMServiceEC2(t *testing.T) {
}
}
}
func Test_formatAWSIAMStatement(t *testing.T) {
type args struct {
acountId string
oidcProvider string
namespace string
name string
}
tests := []struct {
name string
args args
want *iam.Statement
wantErr bool
}{
{
name: "namespace and name without wildcard",
args: args{
acountId: "0123456789",
oidcProvider: "oidc-test",
namespace: "test",
name: "test",
},
wantErr: false,
want: &iam.Statement{
Effect: "Allow",
Principal: iam.Principal{
Federated: "arn:aws:iam::0123456789:oidc-provider/oidc-test",
},
Action: stringorslice.String("sts:AssumeRoleWithWebIdentity"),
Condition: map[string]interface{}{
"StringEquals": map[string]interface{}{
"oidc-test:sub": "system:serviceaccount:test:test",
},
},
},
},
{
name: "name contains wildcard",
args: args{
acountId: "0123456789",
oidcProvider: "oidc-test",
namespace: "test",
name: "test-*",
},
wantErr: true,
},
{
name: "namespace contains wildcard",
args: args{
acountId: "0123456789",
oidcProvider: "oidc-test",
namespace: "test-*",
name: "test",
},
wantErr: false,
want: &iam.Statement{
Effect: "Allow",
Principal: iam.Principal{
Federated: "arn:aws:iam::0123456789:oidc-provider/oidc-test",
},
Action: stringorslice.String("sts:AssumeRoleWithWebIdentity"),
Condition: map[string]interface{}{
"StringLike": map[string]interface{}{
"oidc-test:sub": "system:serviceaccount:test-*:test",
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := formatAWSIAMStatement(tt.args.acountId, tt.args.oidcProvider, tt.args.namespace, tt.args.name)
if (err != nil) != tt.wantErr {
t.Errorf("formatAWSIAMStatement() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("formatAWSIAMStatement() = %v, want %v", got, tt.want)
}
})
}
}

View File

@ -19,6 +19,7 @@ package iam
import (
"encoding/json"
"fmt"
"strings"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
@ -52,7 +53,7 @@ func (b *IAMModelContext) IAMNameForServiceAccountRole(role Subject) (string, er
return "", fmt.Errorf("role %v does not have ServiceAccount", role)
}
name := serviceAccount.Name + "." + serviceAccount.Namespace + ".sa." + b.ClusterName()
name := serviceAccount.Name + "." + strings.ReplaceAll(serviceAccount.Namespace, "*", "wildcard") + ".sa." + b.ClusterName()
name = awsup.TruncateString(name, awsup.TruncateStringOptions{MaxLength: MaxLengthIAMRoleName, AlwaysAddHash: false})
return name, nil

View File

@ -0,0 +1,17 @@
{
"Statement": [
{
"Action": "sts:AssumeRoleWithWebIdentity",
"Condition": {
"StringLike": {
"discovery.example.com/minimal.example.com:sub": "system:serviceaccount:test-*:myserviceaccount"
}
},
"Effect": "Allow",
"Principal": {
"Federated": "arn:aws:iam::123456789012:oidc-provider/discovery.example.com/minimal.example.com"
}
}
],
"Version": "2012-10-17"
}

View File

@ -55,6 +55,11 @@ spec:
- arn:aws:iam::123456789012:policy/UsersManageOwnCredentials
name: myserviceaccount
namespace: default
- aws:
policyARNs:
- arn:aws:iam::123456789012:policy/UsersManageOwnCredentials
name: myserviceaccount
namespace: test-*
- aws:
inlinePolicy: |
[

View File

@ -25,6 +25,11 @@ spec:
aws:
policyARNs:
- arn:aws:iam::123456789012:policy/UsersManageOwnCredentials
- name: myserviceaccount
namespace: test-*
aws:
policyARNs:
- arn:aws:iam::123456789012:policy/UsersManageOwnCredentials
- name: myotherserviceaccount
namespace: myapp
aws:

View File

@ -1,25 +1,27 @@
locals {
cluster_name = "minimal.example.com"
default-myserviceaccount_role_arn = aws_iam_role.myserviceaccount-default-sa-minimal-example-com.arn
default-myserviceaccount_role_name = aws_iam_role.myserviceaccount-default-sa-minimal-example-com.name
iam_openid_connect_provider_arn = aws_iam_openid_connect_provider.minimal-example-com.arn
iam_openid_connect_provider_issuer = "discovery.example.com/minimal.example.com"
master_autoscaling_group_ids = [aws_autoscaling_group.master-us-test-1a-masters-minimal-example-com.id]
master_security_group_ids = [aws_security_group.masters-minimal-example-com.id]
masters_role_arn = aws_iam_role.masters-minimal-example-com.arn
masters_role_name = aws_iam_role.masters-minimal-example-com.name
myapp-myotherserviceaccount_role_arn = aws_iam_role.myotherserviceaccount-myapp-sa-minimal-example-com.arn
myapp-myotherserviceaccount_role_name = aws_iam_role.myotherserviceaccount-myapp-sa-minimal-example-com.name
node_autoscaling_group_ids = [aws_autoscaling_group.nodes-minimal-example-com.id]
node_security_group_ids = [aws_security_group.nodes-minimal-example-com.id]
node_subnet_ids = [aws_subnet.us-test-1a-minimal-example-com.id]
nodes_role_arn = aws_iam_role.nodes-minimal-example-com.arn
nodes_role_name = aws_iam_role.nodes-minimal-example-com.name
region = "us-test-1"
route_table_public_id = aws_route_table.minimal-example-com.id
subnet_us-test-1a_id = aws_subnet.us-test-1a-minimal-example-com.id
vpc_cidr_block = aws_vpc.minimal-example-com.cidr_block
vpc_id = aws_vpc.minimal-example-com.id
cluster_name = "minimal.example.com"
default-myserviceaccount_role_arn = aws_iam_role.myserviceaccount-default-sa-minimal-example-com.arn
default-myserviceaccount_role_name = aws_iam_role.myserviceaccount-default-sa-minimal-example-com.name
iam_openid_connect_provider_arn = aws_iam_openid_connect_provider.minimal-example-com.arn
iam_openid_connect_provider_issuer = "discovery.example.com/minimal.example.com"
master_autoscaling_group_ids = [aws_autoscaling_group.master-us-test-1a-masters-minimal-example-com.id]
master_security_group_ids = [aws_security_group.masters-minimal-example-com.id]
masters_role_arn = aws_iam_role.masters-minimal-example-com.arn
masters_role_name = aws_iam_role.masters-minimal-example-com.name
myapp-myotherserviceaccount_role_arn = aws_iam_role.myotherserviceaccount-myapp-sa-minimal-example-com.arn
myapp-myotherserviceaccount_role_name = aws_iam_role.myotherserviceaccount-myapp-sa-minimal-example-com.name
node_autoscaling_group_ids = [aws_autoscaling_group.nodes-minimal-example-com.id]
node_security_group_ids = [aws_security_group.nodes-minimal-example-com.id]
node_subnet_ids = [aws_subnet.us-test-1a-minimal-example-com.id]
nodes_role_arn = aws_iam_role.nodes-minimal-example-com.arn
nodes_role_name = aws_iam_role.nodes-minimal-example-com.name
region = "us-test-1"
route_table_public_id = aws_route_table.minimal-example-com.id
subnet_us-test-1a_id = aws_subnet.us-test-1a-minimal-example-com.id
test-wildcard-myserviceaccount_role_arn = aws_iam_role.myserviceaccount-test-wildcard-sa-minimal-example-com.arn
test-wildcard-myserviceaccount_role_name = aws_iam_role.myserviceaccount-test-wildcard-sa-minimal-example-com.name
vpc_cidr_block = aws_vpc.minimal-example-com.cidr_block
vpc_id = aws_vpc.minimal-example-com.id
}
output "cluster_name" {
@ -98,6 +100,14 @@ output "subnet_us-test-1a_id" {
value = aws_subnet.us-test-1a-minimal-example-com.id
}
output "test-wildcard-myserviceaccount_role_arn" {
value = aws_iam_role.myserviceaccount-test-wildcard-sa-minimal-example-com.arn
}
output "test-wildcard-myserviceaccount_role_name" {
value = aws_iam_role.myserviceaccount-test-wildcard-sa-minimal-example-com.name
}
output "vpc_cidr_block" {
value = aws_vpc.minimal-example-com.cidr_block
}
@ -321,6 +331,16 @@ resource "aws_iam_role" "myserviceaccount-default-sa-minimal-example-com" {
}
}
resource "aws_iam_role" "myserviceaccount-test-wildcard-sa-minimal-example-com" {
assume_role_policy = file("${path.module}/data/aws_iam_role_myserviceaccount.test-wildcard.sa.minimal.example.com_policy")
name = "myserviceaccount.test-wildcard.sa.minimal.example.com"
tags = {
"KubernetesCluster" = "minimal.example.com"
"Name" = "myserviceaccount.test-wildcard.sa.minimal.example.com"
"kubernetes.io/cluster/minimal.example.com" = "owned"
}
}
resource "aws_iam_role" "nodes-minimal-example-com" {
assume_role_policy = file("${path.module}/data/aws_iam_role_nodes.minimal.example.com_policy")
name = "nodes.minimal.example.com"
@ -354,6 +374,11 @@ resource "aws_iam_role_policy_attachment" "external-myserviceaccount-default-sa-
role = aws_iam_role.myserviceaccount-default-sa-minimal-example-com.name
}
resource "aws_iam_role_policy_attachment" "external-myserviceaccount-test-wildcard-sa-minimal-example-com-3186075376" {
policy_arn = "arn:aws:iam::123456789012:policy/UsersManageOwnCredentials"
role = aws_iam_role.myserviceaccount-test-wildcard-sa-minimal-example-com.name
}
resource "aws_internet_gateway" "minimal-example-com" {
tags = {
"KubernetesCluster" = "minimal.example.com"