From 1104450b90f66874c97fa75a99741749ac8b6ff4 Mon Sep 17 00:00:00 2001 From: Kevin Downey Date: Wed, 18 Oct 2023 10:14:32 -0700 Subject: [PATCH] feat: Always use default trust policy and append patches (#148) Signed-off-by: kevdowney Signed-off-by: Kevin Downey --- .gitignore | 1 + api/v1alpha1/iamrole_types.go | 11 +++ api/v1alpha1/iamrole_types_test.go | 66 ++++++++++++++++++ go.mod | 1 + go.sum | 13 +++- internal/utils/utils.go | 50 +++++++++----- internal/utils/utils_test.go | 104 +++++++++++++++++++++++++++-- 7 files changed, 225 insertions(+), 21 deletions(-) create mode 100644 api/v1alpha1/iamrole_types_test.go diff --git a/.gitignore b/.gitignore index 43704e1..101a37a 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ kubebuilder* #IDE files .idea/ bin/ +.tool-versions diff --git a/api/v1alpha1/iamrole_types.go b/api/v1alpha1/iamrole_types.go index 1c2ab7d..3f377a1 100644 --- a/api/v1alpha1/iamrole_types.go +++ b/api/v1alpha1/iamrole_types.go @@ -16,6 +16,10 @@ limitations under the License. package v1alpha1 import ( + "fmt" + "hash/adler32" + "strings" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -95,6 +99,13 @@ type TrustPolicyStatement struct { Condition *Condition `json:"Condition,omitempty"` } +// Id returns the sid of the trust policy statement, ignoring conditions. +func (tps *TrustPolicyStatement) Id() string { + return strings.Title(fmt.Sprintf("%s%s%x", tps.Effect, + strings.ReplaceAll(strings.Title(tps.Action), ":", ""), + adler32.Checksum([]byte(fmt.Sprintf("%+v", tps.Principal))))) +} + // Principal struct holds AWS principal // +optional type Principal struct { diff --git a/api/v1alpha1/iamrole_types_test.go b/api/v1alpha1/iamrole_types_test.go new file mode 100644 index 0000000..0dd060c --- /dev/null +++ b/api/v1alpha1/iamrole_types_test.go @@ -0,0 +1,66 @@ +package v1alpha1 + +import "testing" + +func TestTrustPolicyStatement_Id(t *testing.T) { + type fields struct { + Effect Effect + Action string + Principal Principal + Condition *Condition + } + tests := []struct { + name string + fields fields + want string + }{ + { + name: "test1", + fields: fields{ + Effect: "Allow", + Action: "sts:AssumeRoleWithWebIdentity", + Principal: Principal{ + Federated: "arn:aws:iam::123456789012:oidc-provider/oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041", + }, + }, + want: "AllowStsAssumeRoleWithWebIdentityef522ae8", + }, + { + name: "test2: with conditions", + fields: fields{ + Effect: "Allow", + Action: "sts:AssumeRoleWithWebIdentity", + Principal: Principal{ + Federated: "arn:aws:iam::123456789012:oidc-provider/oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041", + }, + }, + want: "AllowStsAssumeRoleWithWebIdentityef522ae8", // same as test1 + }, + { + name: "test3", + fields: fields{ + Effect: "Allow", + Action: "sts:AssumeRole", + Principal: Principal{ + AWS: []string{ + "arn:aws:iam::123456789012:root", + }, + }, + }, + want: "AllowStsAssumeRole21d512f8", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tps := &TrustPolicyStatement{ + Effect: tt.fields.Effect, + Action: tt.fields.Action, + Principal: tt.fields.Principal, + Condition: tt.fields.Condition, + } + if got := tps.Id(); got != tt.want { + t.Errorf("Id() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/go.mod b/go.mod index 7bab6c2..2ee7694 100644 --- a/go.mod +++ b/go.mod @@ -57,6 +57,7 @@ require ( golang.org/x/term v0.13.0 // indirect golang.org/x/text v0.13.0 // indirect golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect + golang.org/x/tools v0.14.0 // indirect golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect google.golang.org/appengine v1.6.7 // indirect diff --git a/go.sum b/go.sum index b3a1822..f5e46b5 100644 --- a/go.sum +++ b/go.sum @@ -429,6 +429,7 @@ golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20211202192323-5770296d904e/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= +golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -465,6 +466,8 @@ golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449/go.mod h1:s0Qsj1ACt9ePp/hM golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.13.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -504,6 +507,8 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= +golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= +golang.org/x/net v0.16.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM= golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -523,6 +528,8 @@ golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= +golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -576,6 +583,7 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= @@ -585,6 +593,7 @@ golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuX golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= +golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek= golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -650,8 +659,10 @@ golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= +golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= +golang.org/x/tools v0.14.0 h1:jvNa2pY0M4r62jkRQ6RwEZZyPcymeL9XZMLBbV7U2nc= +golang.org/x/tools v0.14.0/go.mod h1:uYBEerGOWcJyEORxN+Ek8+TT266gXkNlHdJBwexUsBg= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/utils/utils.go b/internal/utils/utils.go index 7e73744..03481fe 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -23,6 +23,18 @@ func GetTrustPolicy(ctx context.Context, role *iammanagerv1alpha1.Iamrole) (stri var statements []iammanagerv1alpha1.TrustPolicyStatement + // Always apply default trust policy when it is not provided in the spec + if tPolicy == nil || len(tPolicy.Statement) == 0 { + trustPolicy, err := DefaultTrustPolicy(ctx, config.Props.DefaultTrustPolicy(), role.Namespace) + if err != nil { + msg := "unable to get the trust policy. It must follow v1alpha1.AssumeRolePolicyDocument syntax" + log.Error(err, msg) + return "", err + } + + statements = AppendOrReplaceTrustPolicyStatement(statements, trustPolicy.Statement...) + } + // Is it IRSA use case // Construct AssumeRoleWithWebIdentity if flag, saNames := ParseIRSAAnnotation(ctx, role); flag { @@ -41,27 +53,13 @@ func GetTrustPolicy(ctx context.Context, role *iammanagerv1alpha1.Iamrole) (stri }, }, } - statements = append(statements, statement) - } - - } else { - // NON - IRSA which should cover AssumeRole usecase - //For default use cases - if tPolicy == nil || len(tPolicy.Statement) == 0 { - trustPolicy, err := DefaultTrustPolicy(ctx, config.Props.DefaultTrustPolicy(), role.Namespace) - if err != nil { - msg := "unable to get the trust policy. It must follow v1alpha1.AssumeRolePolicyDocument syntax" - log.Error(err, msg) - return "", err - } - - statements = append(statements, trustPolicy.Statement...) + statements = AppendOrReplaceTrustPolicyStatement(statements, statement) } } // If anything included in the request if tPolicy != nil && len(tPolicy.Statement) > 0 { - statements = append(statements, role.Spec.AssumeRolePolicyDocument.Statement...) + statements = AppendOrReplaceTrustPolicyStatement(statements, role.Spec.AssumeRolePolicyDocument.Statement...) } tDoc := iammanagerv1alpha1.AssumeRolePolicyDocument{ Version: "2012-10-17", @@ -80,6 +78,26 @@ func GetTrustPolicy(ctx context.Context, role *iammanagerv1alpha1.Iamrole) (stri return string(output), nil } +func AppendOrReplaceTrustPolicyStatement(statements []iammanagerv1alpha1.TrustPolicyStatement, newStatements ...iammanagerv1alpha1.TrustPolicyStatement) []iammanagerv1alpha1.TrustPolicyStatement { + statementMap := make(map[string]int) + for i, st := range statements { + statementMap[st.Id()] = i + } + + for _, st := range newStatements { + if i, ok := statementMap[st.Id()]; ok { + // replace the statement with new one if matches the id + statements[i] = st + continue + } + + // otherwise append the statement + statementMap[st.Id()] = len(statements) + statements = append(statements, st) + } + return statements +} + // Fields Template fields type Fields struct { AccountID string diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index 74d7f0d..0b90a1f 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -100,8 +100,8 @@ func (s *UtilsTestSuite) TestDefaultTrustPolicyWithGoTemplate(c *check.C) { c.Assert(err, check.IsNil) c.Assert(resp, check.NotNil) c.Assert(*resp, check.DeepEquals, expect) - } + func (s *UtilsTestSuite) TestDefaultTrustPolicyMultipleNoGoTemplate(c *check.C) { tD := `{"Version": "2012-10-17", "Statement": [{"Effect": "Allow","Principal": {"AWS": ["arn:aws:iam::123456789012:role/trust_role"]},"Action": "sts:AssumeRole"}, {"Effect": "Allow","Principal": {"Federated": "arn:aws:iam::AWS_ACCOUNT_ID:oidc-provider/OIDC_PROVIDER"},"Action": "sts:AssumeRoleWithWebIdentity","Condition": {"StringEquals": {"OIDC_PROVIDER:sub": "system:serviceaccount:SERVICE_ACCOUNT_NAMESPACE:SERVICE_ACCOUNT_NAME"}}}]}` expect := v1alpha1.AssumeRolePolicyDocument{ @@ -166,7 +166,6 @@ func (s *UtilsTestSuite) TestDefaultTrustPolicyMultipleWithGoTemplate(c *check.C c.Assert(err, check.IsNil) c.Assert(resp, check.NotNil) c.Assert(*resp, check.DeepEquals, expect) - } func (s *UtilsTestSuite) TestGetTrustPolicyDefaultRoleWithMultiple(c *check.C) { @@ -436,6 +435,25 @@ func (s *UtilsTestSuite) TestGetTrustPolicyWithIRSAAnnotation(c *check.C) { expect := v1alpha1.AssumeRolePolicyDocument{ Version: "2012-10-17", Statement: []v1alpha1.TrustPolicyStatement{ + { + Effect: "Allow", + Action: "sts:AssumeRoleWithWebIdentity", + Principal: v1alpha1.Principal{ + Federated: "arn:aws:iam::AWS_ACCOUNT_ID:oidc-provider/OIDC_PROVIDER", + }, + Condition: &v1alpha1.Condition{ + StringEquals: map[string]string{ + "OIDC_PROVIDER:sub": "system:serviceaccount:k8s-namespace-dev:SERVICE_ACCOUNT_NAME", + }, + }, + }, + { + Effect: "Allow", + Action: "sts:AssumeRole", + Principal: v1alpha1.Principal{ + AWS: []string{"arn:aws:iam::123456789012:role/trust_role"}, + }, + }, { Effect: "Allow", Action: "sts:AssumeRoleWithWebIdentity", @@ -466,7 +484,6 @@ func (s *UtilsTestSuite) TestGetTrustPolicyWithIRSAAnnotation(c *check.C) { roleString, err := utils.GetTrustPolicy(s.ctx, input) c.Assert(err, check.IsNil) c.Assert(roleString, check.Equals, string(expected)) - } func (s *UtilsTestSuite) TestGetTrustPolicyWithIRSAAnnotationAndServiceRoleInRequest(c *check.C) { @@ -524,7 +541,6 @@ func (s *UtilsTestSuite) TestGetTrustPolicyWithIRSAAnnotationAndServiceRoleInReq roleString, err := utils.GetTrustPolicy(s.ctx, input) c.Assert(err, check.IsNil) c.Assert(roleString, check.Equals, string(expected)) - } func (s *UtilsTestSuite) TestGenerateNameFunction(c *check.C) { @@ -755,3 +771,83 @@ func (s *UtilsTestSuite) TestParsePrivilegedAnnotationSuccess(c *check.C) { resp := utils.ParsePrivilegedAnnotation(s.ctx, &ns) c.Assert(resp, check.Equals, true) } + +func (s *UtilsTestSuite) TestAppendOrReplaceTrustPolicyStatement(c *check.C) { + input := []v1alpha1.TrustPolicyStatement{ + { + Effect: "Allow", + Action: "sts:AssumeRoleWithWebIdentity", + Principal: v1alpha1.Principal{ + Federated: "arn:aws:iam::123456789012:oidc-provider/google.com/OIDC", + }, + Condition: &v1alpha1.Condition{ + StringEquals: map[string]string{ + "google.com/OIDC:sub": "system:serviceaccount:k8s-namespace-dev:default", + }, + }, + }, + { + // no sid + Effect: "Allow", + Action: "sts:AssumeRole", + Principal: v1alpha1.Principal{ + Service: "ec2.amazonaws.com", + }, + }, + } + + newStatement1 := v1alpha1.TrustPolicyStatement{ + // no sid + Effect: "Allow", + Action: "sts:AssumeRole", + Principal: v1alpha1.Principal{ + AWS: []string{"arn:aws:iam::123456789012:role/trust_role"}, + }, + } + + newStatement2 := v1alpha1.TrustPolicyStatement{ + Effect: "Allow", + Action: "sts:AssumeRole", + Principal: v1alpha1.Principal{ + AWS: []string{"arn:aws:iam::123456789012:role/custom_role"}, + }, + } + + actual := utils.AppendOrReplaceTrustPolicyStatement(input, newStatement1, newStatement2) + + c.Assert(actual, check.DeepEquals, []v1alpha1.TrustPolicyStatement{ + { + Effect: "Allow", + Action: "sts:AssumeRoleWithWebIdentity", + Principal: v1alpha1.Principal{ + Federated: "arn:aws:iam::123456789012:oidc-provider/google.com/OIDC", + }, + Condition: &v1alpha1.Condition{ + StringEquals: map[string]string{ + "google.com/OIDC:sub": "system:serviceaccount:k8s-namespace-dev:default", + }, + }, + }, + { + Effect: "Allow", + Action: "sts:AssumeRole", + Principal: v1alpha1.Principal{ + Service: "ec2.amazonaws.com", + }, + }, + { + Effect: "Allow", + Action: "sts:AssumeRole", + Principal: v1alpha1.Principal{ + AWS: []string{"arn:aws:iam::123456789012:role/trust_role"}, + }, + }, + { + Effect: "Allow", + Action: "sts:AssumeRole", + Principal: v1alpha1.Principal{ + AWS: []string{"arn:aws:iam::123456789012:role/custom_role"}, + }, + }, + }) +}