diff --git a/api/v1alpha1/iamrole_types.go b/api/v1alpha1/iamrole_types.go index 3f377a1..04a468c 100644 --- a/api/v1alpha1/iamrole_types.go +++ b/api/v1alpha1/iamrole_types.go @@ -99,11 +99,48 @@ type TrustPolicyStatement struct { Condition *Condition `json:"Condition,omitempty"` } -// Id returns the sid of the trust policy statement, ignoring conditions. +// Id returns the sid of the trust policy statement func (tps *TrustPolicyStatement) Id() string { - return strings.Title(fmt.Sprintf("%s%s%x", tps.Effect, + sid := strings.Title(fmt.Sprintf("%s%s%x", tps.Effect, strings.ReplaceAll(strings.Title(tps.Action), ":", ""), adler32.Checksum([]byte(fmt.Sprintf("%+v", tps.Principal))))) + + if tps.HasCondition() { + if tps.IsConditionAnyServiceAccount() { + sid = fmt.Sprintf("%s%s", sid, "Any") + } else { + sid = fmt.Sprintf("%s%s", sid, tps.ConditionChecksum()) + } + } + return sid +} + +func (tps *TrustPolicyStatement) HasCondition() bool { + return tps.Condition != nil +} + +func (tps *TrustPolicyStatement) ConditionChecksum() string { + if !tps.HasCondition() { + return "" + } + return fmt.Sprintf("%x", adler32.Checksum([]byte(fmt.Sprintf("%+v", *tps.Condition)))) +} + +func (tps *TrustPolicyStatement) IsConditionAnyServiceAccount() bool { + if !tps.HasCondition() || len(tps.Condition.StringLike) == 0 { + return false + } + + for k, v := range tps.Condition.StringLike { + if strings.HasSuffix(k, ":sub") { + parts := strings.Split(v, ":") + if parts[len(parts)-1] == "*" { + return true + } + } + } + + return false } // Principal struct holds AWS principal diff --git a/api/v1alpha1/iamrole_types_test.go b/api/v1alpha1/iamrole_types_test.go index 0dd060c..8de168f 100644 --- a/api/v1alpha1/iamrole_types_test.go +++ b/api/v1alpha1/iamrole_types_test.go @@ -33,8 +33,13 @@ func TestTrustPolicyStatement_Id(t *testing.T) { Principal: Principal{ Federated: "arn:aws:iam::123456789012:oidc-provider/oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041", }, + Condition: &Condition{ + StringEquals: map[string]string{ + "oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041:sub": "system:serviceaccount:my-namespace:my-serviceaccount", + }, + }, }, - want: "AllowStsAssumeRoleWithWebIdentityef522ae8", // same as test1 + want: "AllowStsAssumeRoleWithWebIdentityef522ae8d16a3945", }, { name: "test3", @@ -49,6 +54,38 @@ func TestTrustPolicyStatement_Id(t *testing.T) { }, want: "AllowStsAssumeRole21d512f8", }, + { + name: "test4 - any service account", + 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", + }, + Condition: &Condition{ + StringLike: map[string]string{ + "oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041:sub": "system:serviceaccount:*:*", + }, + }, + }, + want: "AllowStsAssumeRoleWithWebIdentityef522ae8Any", + }, + { + name: "test5 - Not any service account", + 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", + }, + Condition: &Condition{ + StringLike: map[string]string{ + "oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041:sub": "system:serviceaccount:*:default", + }, + }, + }, + want: "AllowStsAssumeRoleWithWebIdentityef522ae8a57730a3", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -64,3 +101,200 @@ func TestTrustPolicyStatement_Id(t *testing.T) { }) } } + +func TestTrustPolicyStatement_HasCondition(t *testing.T) { + type fields struct { + Effect Effect + Action string + Principal Principal + Condition *Condition + } + tests := []struct { + name string + fields fields + want bool + }{ + { + 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: false, + }, + { + 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", + }, + Condition: &Condition{ + StringEquals: map[string]string{ + "oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041:sub": "system:serviceaccount:my-namespace:my-serviceaccount", + }, + }, + }, + want: true, + }, + } + 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.HasCondition(); got != tt.want { + t.Errorf("HasCondition() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestTrustPolicyStatement_ConditionChecksum(t *testing.T) { + type fields struct { + Effect Effect + Action string + Principal Principal + Condition *Condition + } + tests := []struct { + name string + fields fields + want string + }{ + { + name: "test1 - no condition empty string", + 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: "", + }, + { + 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", + }, + Condition: &Condition{ + StringEquals: map[string]string{ + "oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041:sub": "system:serviceaccount:my-namespace:my-serviceaccount", + }, + }, + }, + want: "d16a3945", + }, + } + 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.ConditionChecksum(); got != tt.want { + t.Errorf("ConditionChecksum() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestTrustPolicyStatement_IsConditionAnyServiceAccount(t *testing.T) { + type fields struct { + Effect Effect + Action string + Principal Principal + Condition *Condition + } + tests := []struct { + name string + fields fields + want bool + }{ + { + name: "test1 - no condition", + 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: false, + }, + { + name: "test2: with conditions - any service account", + 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", + }, + Condition: &Condition{ + StringLike: map[string]string{ + "oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041:sub": "system:serviceaccount:*:*", + }, + }, + }, + want: true, + }, + { + name: "test3: with conditions - any namespace but not any service account", + 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", + }, + Condition: &Condition{ + StringLike: map[string]string{ + "oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041:sub": "system:serviceaccount:*:default", + }, + }, + }, + want: false, + }, + { + name: "test3: with conditions - not like condition", + 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", + }, + Condition: &Condition{ + StringEquals: map[string]string{ + "oidc.eks.us-east-2.amazonaws.com/id/EXAMPLED539D4633E53DE1B716D3041:sub": "system:serviceaccount:my-ns:default", + }, + }, + }, + want: false, + }, + } + 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.IsConditionAnyServiceAccount(); got != tt.want { + t.Errorf("IsConditionAnyServiceAccount() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/controllers/iamrole_reconcile_manager.go b/controllers/iamrole_reconcile_manager.go index edd652c..2ecb186 100644 --- a/controllers/iamrole_reconcile_manager.go +++ b/controllers/iamrole_reconcile_manager.go @@ -7,6 +7,7 @@ import ( api "github.com/keikoproj/iam-manager/api/v1alpha1" "github.com/keikoproj/iam-manager/internal/config" "github.com/keikoproj/iam-manager/pkg/logging" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" ) @@ -71,7 +72,8 @@ func (r *IamroleReconciler) ReconcileAllReadyStateIamRoles(ctx context.Context) continue } - res, err = r.HandleReconcile(ctx, ctrl.Request{}, iamrole) + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: iamrole.Namespace, Name: iamrole.Name}} + res, err = r.HandleReconcile(ctx, req, iamrole) log.Info("Reconcile result", "result", res, "error", err) // sleep for 2 seconds for politeness diff --git a/go.mod b/go.mod index 1592c5b..b782b5c 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/keikoproj/iam-manager go 1.19 require ( - github.com/aws/aws-sdk-go v1.45.28 github.com/go-logr/logr v1.2.4 + github.com/aws/aws-sdk-go v1.46.2 github.com/golang/mock v1.6.0 github.com/onsi/ginkgo v1.16.5 github.com/onsi/gomega v1.20.1 diff --git a/go.sum b/go.sum index 45475a1..c311db9 100644 --- a/go.sum +++ b/go.sum @@ -56,9 +56,12 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= -github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= -github.com/aws/aws-sdk-go v1.45.28 h1:p2ATcaK6ffSw4yZ2UAGzgRyRXwKyOJY6ZCiKqj5miJE= -github.com/aws/aws-sdk-go v1.45.28/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI= +github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o= +github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY= +github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= +github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= +github.com/aws/aws-sdk-go v1.46.2 h1:XZbOmjtN1VCfEtQq7QNFsbxIqO+bB+bRhiOBjp6AzWc= +github.com/aws/aws-sdk-go v1.46.2/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI= github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index 0b90a1f..35dfe65 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -486,6 +486,73 @@ func (s *UtilsTestSuite) TestGetTrustPolicyWithIRSAAnnotation(c *check.C) { c.Assert(roleString, check.Equals, string(expected)) } +func (s *UtilsTestSuite) TestGetTrustPolicyWithMultipleIRSAAnnotations(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", + 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: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:my-sa-1", + }, + }, + }, + }, + } + + expected, _ := json.Marshal(expect) + + input := &v1alpha1.Iamrole{ + ObjectMeta: v1.ObjectMeta{ + Name: "iam-role", + Namespace: "k8s-namespace-dev", + Annotations: map[string]string{ + config.IRSAAnnotation: "default,my-sa-1", + }, + }, + } + + 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) { expect := v1alpha1.AssumeRolePolicyDocument{ Version: "2012-10-17", @@ -813,7 +880,20 @@ func (s *UtilsTestSuite) TestAppendOrReplaceTrustPolicyStatement(c *check.C) { }, } - actual := utils.AppendOrReplaceTrustPolicyStatement(input, newStatement1, newStatement2) + newStatement3 := 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:my-sa", + }, + }, + } + + actual := utils.AppendOrReplaceTrustPolicyStatement(input, newStatement1, newStatement2, newStatement3) c.Assert(actual, check.DeepEquals, []v1alpha1.TrustPolicyStatement{ { @@ -849,5 +929,17 @@ func (s *UtilsTestSuite) TestAppendOrReplaceTrustPolicyStatement(c *check.C) { AWS: []string{"arn:aws:iam::123456789012:role/custom_role"}, }, }, + { + 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:my-sa", + }, + }, + }, }) }