From 6fa8be2716be72fada063230afbb01fef8970bc6 Mon Sep 17 00:00:00 2001 From: Justin SB Date: Tue, 8 Sep 2020 14:56:36 -0400 Subject: [PATCH] JSON formatting of IAM: Workaround for optional fields AWS IAM is very strict and doesn't support `Resource: []` for example. We implement a custom MarshalJSON method to work around that. --- pkg/model/iam.go | 2 +- pkg/model/iam/iam_builder.go | 103 +++++++++++++++++- pkg/model/iam/iam_builder_test.go | 20 ++++ pkg/util/stringorslice/stringorslice.go | 4 + ....kube-system.sa.minimal.example.com_policy | 1 - 5 files changed, 124 insertions(+), 6 deletions(-) diff --git a/pkg/model/iam.go b/pkg/model/iam.go index 47801dd0f6..b228fa3098 100644 --- a/pkg/model/iam.go +++ b/pkg/model/iam.go @@ -338,7 +338,7 @@ func (b *IAMModelBuilder) buildAWSIAMRolePolicy(role iam.Subject) (fi.Resource, Statement: []*iam.Statement{ { Effect: "Allow", - Principal: &iam.Principal{ + Principal: iam.Principal{ Federated: "arn:aws:iam::" + b.AWSAccountID + ":oidc-provider/" + oidcProvider, }, Action: stringorslice.String("sts:AssumeRoleWithWebIdentity"), diff --git a/pkg/model/iam/iam_builder.go b/pkg/model/iam/iam_builder.go index c5dea19c28..50905b297f 100644 --- a/pkg/model/iam/iam_builder.go +++ b/pkg/model/iam/iam_builder.go @@ -77,10 +77,101 @@ type Condition map[string]interface{} // http://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements.html#Statement type Statement struct { Effect StatementEffect - Principal *Principal `json:",omitempty"` - Action stringorslice.StringOrSlice `json:",omitempty"` - Resource stringorslice.StringOrSlice `json:",omitempty"` - Condition Condition `json:",omitempty"` + Principal Principal + Action stringorslice.StringOrSlice + Resource stringorslice.StringOrSlice + Condition Condition +} + +type jsonWriter struct { + w io.Writer + err error +} + +func (j *jsonWriter) Error() error { + return j.err +} + +func (j *jsonWriter) WriteLiteral(b []byte) { + if j.err != nil { + return + } + _, err := j.w.Write(b) + if err != nil { + j.err = err + } +} + +func (j *jsonWriter) StartObject() { + j.WriteLiteral([]byte("{")) +} + +func (j *jsonWriter) EndObject() { + j.WriteLiteral([]byte("}")) +} + +func (j *jsonWriter) Comma() { + j.WriteLiteral([]byte(",")) +} + +func (j *jsonWriter) Field(s string) { + if j.err != nil { + return + } + b, err := json.Marshal(s) + if err != nil { + j.err = err + return + } + j.WriteLiteral(b) + j.WriteLiteral([]byte(": ")) +} + +func (j *jsonWriter) Marshal(v interface{}) { + if j.err != nil { + return + } + b, err := json.Marshal(v) + if err != nil { + j.err = err + return + } + j.WriteLiteral(b) +} + +// MarshalJSON formats the IAM statement for the AWS IAM restrictions. +// For example, `Resource: []` is not allowed, but golang would force us to use pointers. +func (s *Statement) MarshalJSON() ([]byte, error) { + var b bytes.Buffer + + jw := &jsonWriter{w: &b} + jw.StartObject() + jw.Field("Effect") + jw.Marshal(s.Effect) + + if !s.Principal.IsEmpty() { + jw.Comma() + jw.Field("Principal") + jw.Marshal(s.Principal) + } + if !s.Action.IsEmpty() { + jw.Comma() + jw.Field("Action") + jw.Marshal(s.Action) + } + if !s.Resource.IsEmpty() { + jw.Comma() + jw.Field("Resource") + jw.Marshal(s.Resource) + } + if len(s.Condition) != 0 { + jw.Comma() + jw.Field("Condition") + jw.Marshal(s.Condition) + } + jw.EndObject() + + return b.Bytes(), jw.Error() } type Principal struct { @@ -88,6 +179,10 @@ type Principal struct { Service string `json:",omitempty"` } +func (p *Principal) IsEmpty() bool { + return *p == Principal{} +} + // Equal compares two IAM Statements and returns a bool // TODO: Extend to support Condition Keys func (l *Statement) Equal(r *Statement) bool { diff --git a/pkg/model/iam/iam_builder_test.go b/pkg/model/iam/iam_builder_test.go index 79f8818412..e07bd3e7e6 100644 --- a/pkg/model/iam/iam_builder_test.go +++ b/pkg/model/iam/iam_builder_test.go @@ -48,6 +48,26 @@ func TestRoundTrip(t *testing.T) { }, JSON: "{\"Effect\":\"Deny\",\"Action\":[\"ec2:DescribeRegions\",\"ec2:DescribeInstances\"],\"Resource\":[\"a\",\"b\"]}", }, + { + IAM: &Statement{ + Effect: StatementEffectDeny, + Principal: Principal{Federated: "federated"}, + Condition: map[string]interface{}{ + "foo": 1, + }, + }, + JSON: "{\"Effect\":\"Deny\",\"Principal\":{\"Federated\":\"federated\"},\"Condition\":{\"foo\":1}}", + }, + { + IAM: &Statement{ + Effect: StatementEffectDeny, + Principal: Principal{Service: "service"}, + Condition: map[string]interface{}{ + "bar": "baz", + }, + }, + JSON: "{\"Effect\":\"Deny\",\"Principal\":{\"Service\":\"service\"},\"Condition\":{\"bar\":\"baz\"}}", + }, } for _, g := range grid { actualJSON, err := json.Marshal(g.IAM) diff --git a/pkg/util/stringorslice/stringorslice.go b/pkg/util/stringorslice/stringorslice.go index 0d04fc9588..7b88f40e24 100644 --- a/pkg/util/stringorslice/stringorslice.go +++ b/pkg/util/stringorslice/stringorslice.go @@ -27,6 +27,10 @@ type StringOrSlice struct { forceEncodeAsArray bool } +func (s *StringOrSlice) IsEmpty() bool { + return len(s.values) == 0 +} + // Slice will build a value that marshals to a JSON array func Slice(v []string) StringOrSlice { return StringOrSlice{values: v, forceEncodeAsArray: true} diff --git a/tests/integration/update_cluster/public-jwks/data/aws_iam_role_dns-controller.kube-system.sa.minimal.example.com_policy b/tests/integration/update_cluster/public-jwks/data/aws_iam_role_dns-controller.kube-system.sa.minimal.example.com_policy index 67e05c8317..256ce24ad2 100644 --- a/tests/integration/update_cluster/public-jwks/data/aws_iam_role_dns-controller.kube-system.sa.minimal.example.com_policy +++ b/tests/integration/update_cluster/public-jwks/data/aws_iam_role_dns-controller.kube-system.sa.minimal.example.com_policy @@ -7,7 +7,6 @@ "Federated": "arn:aws:iam::123456789012:oidc-provider/api.minimal.example.com" }, "Action": "sts:AssumeRoleWithWebIdentity", - "Resource": [], "Condition": { "StringEquals": { "api.minimal.example.com:sub": "system:serviceaccount:kube-system:dns-controller"