Skip to content

Commit fb7f703

Browse files
shabbskagalwalaskagalwala
shabbskagalwala
authored and
skagalwala
committed
[feat]: allow adding resource policy to dynamodb tables
1 parent baa9115 commit fb7f703

File tree

10 files changed

+452
-21
lines changed

10 files changed

+452
-21
lines changed

‎apis/v1alpha1/ack-generate-metadata.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
ack_generate_info:
2-
build_date: "2025-10-17T18:47:29Z"
2+
build_date: "2025-10-21T04:38:02Z"
33
build_hash: 6b4211163dcc34776b01da9a18217bac0f4103fd
44
go_version: go1.24.6
55
version: v0.52.0

‎go.mod‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ require (
1212
github.com/aws/aws-sdk-go-v2/service/dynamodb v1.39.8
1313
github.com/aws/smithy-go v1.22.2
1414
github.com/go-logr/logr v1.4.2
15+
github.com/micahhausler/aws-iam-policy v0.4.2
1516
github.com/spf13/pflag v1.0.5
1617
github.com/stretchr/testify v1.9.0
1718
k8s.io/api v0.32.1
@@ -20,6 +21,10 @@ require (
2021
sigs.k8s.io/controller-runtime v0.20.4
2122
)
2223

24+
// Temporary fix for github.com/micahhausler/aws-iam-policy. Awaiting for a-hilaly to send
25+
// a PR to micahhausler/aws-iam-policy to build Equal() method for PolicyDocument struct.
26+
replace github.com/micahhausler/aws-iam-policy => github.com/a-hilaly/aws-iam-policy v0.0.0-20231121054900-2c56e839ca53
27+
2328
require (
2429
github.com/aws/aws-sdk-go-v2/config v1.28.6 // indirect
2530
github.com/aws/aws-sdk-go-v2/credentials v1.17.47 // indirect

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
github.com/a-hilaly/aws-iam-policy v0.0.0-20231121054900-2c56e839ca53 h1:2uNM0nR2WUDN88EYFxjEaroH+PZJ6k/h9kl+KO0dWVc=
2+
github.com/a-hilaly/aws-iam-policy v0.0.0-20231121054900-2c56e839ca53/go.mod h1:Ojgst9ZFn+VEEJpqtuw/LxVGqEf2+hwWBlkYWvF/XWM=
13
github.com/aws-controllers-k8s/kms-controller v1.0.21 h1:ar8gCdl/l7qbXzr48YN5tNq4vJbB5UqnRH7pAIkP3tI=
24
github.com/aws-controllers-k8s/kms-controller v1.0.21/go.mod h1:tHFXV8lkrzautPPvQtPUJABPlJ9MXPRj8GB1UublGHQ=
35
github.com/aws-controllers-k8s/runtime v0.52.0 h1:Q5UIAn6SSBr60t/DiU/zr6NLBlUuK2AG3yy2ma/9gDU=

‎pkg/resource/table/hooks.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -694,14 +694,7 @@ func customPreCompare(
694694
delta.Add("Spec.ContributorInsights", a.ko.Spec.ContributorInsights, b.ko.Spec.ContributorInsights)
695695
}
696696
}
697-
698-
if ackcompare.HasNilDifference(a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy) {
699-
delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy)
700-
} else if a.ko.Spec.ResourcePolicy != nil && b.ko.Spec.ResourcePolicy != nil {
701-
if *a.ko.Spec.ResourcePolicy != *b.ko.Spec.ResourcePolicy {
702-
delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy)
703-
}
704-
}
697+
compareResourcePolicyDocument(delta, a, b)
705698

706699
}
707700

‎pkg/resource/table/hooks_resource_policy.go‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,16 @@ package table
1515

1616
import (
1717
"context"
18+
"encoding/json"
1819
"errors"
20+
"reflect"
1921

22+
ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
2023
ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
2124
ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
2225
svcsdk "github.com/aws/aws-sdk-go-v2/service/dynamodb"
2326
svcsdktypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
27+
awsiampolicy "github.com/micahhausler/aws-iam-policy/policy"
2428
)
2529

2630
// syncResourcePolicy updates a DynamoDB table's resource-based policy.
@@ -121,15 +125,53 @@ func (rm *resourceManager) getResourcePolicyWithContext(
121125
ResourceArn: tableARN,
122126
},
123127
)
124-
128+
rm.metrics.RecordAPICall("GET", "GetResourcePolicy", nil)
125129
if err != nil {
126130
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.ErrorCode() == "PolicyNotFoundException" {
127131
return nil, nil
128132
}
129-
rm.metrics.RecordAPICall("GET", "GetResourcePolicy", err)
130133
return nil, err
131134
}
132135

133-
rm.metrics.RecordAPICall("GET", "GetResourcePolicy", nil)
134136
return res.Policy, nil
135137
}
138+
139+
// compareResourcePolicyDocument is a custom comparison function for
140+
// ResourcePolicy documents. The reason why we need a custom function for
141+
// this field is to handle the variability in shapes of JSON objects representing
142+
// IAM policies, especially when it comes to statements, actions, and other fields.
143+
func compareResourcePolicyDocument(
144+
delta *ackcompare.Delta,
145+
a *resource,
146+
b *resource,
147+
) {
148+
// Handle cases where one policy is nil and the other is not.
149+
// This means one resource has a policy and the other doesn't - they're different.
150+
if ackcompare.HasNilDifference(a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy) {
151+
delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy)
152+
return
153+
}
154+
155+
// If both policies are nil, there's no difference - both resources have no policy.
156+
if a.ko.Spec.ResourcePolicy == nil && b.ko.Spec.ResourcePolicy == nil {
157+
return
158+
}
159+
160+
// At this point, both policies are non-nil. We need to compare their JSON content.
161+
// To handle the variability in shapes of JSON objects representing IAM policies,
162+
// especially when it comes to statements, actions, and other fields, we need
163+
// a custom json.Unmarshaller approach crafted to our specific needs. Luckily,
164+
// it happens that @micahhausler built a library dedicated to this very special
165+
// need: github.com/micahhausler/aws-iam-policy.
166+
//
167+
// Copied from IAM Controller: https://github.com/aws-controllers-k8s/iam-controller/blob/main/pkg/resource/role/hooks.go#L398-L432
168+
// Based on review feedback: https://github.com/aws-controllers-k8s/dynamodb-controller/pull/154#discussion_r2443876840
169+
var policyDocumentA awsiampolicy.Policy
170+
_ = json.Unmarshal([]byte(*a.ko.Spec.ResourcePolicy), &policyDocumentA)
171+
var policyDocumentB awsiampolicy.Policy
172+
_ = json.Unmarshal([]byte(*b.ko.Spec.ResourcePolicy), &policyDocumentB)
173+
174+
if !reflect.DeepEqual(policyDocumentA, policyDocumentB) {
175+
delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy)
176+
}
177+
}

‎pkg/resource/table/hooks_resource_policy_test.go‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License"). You may
4+
// not use this file except in compliance with the License. A copy of the
5+
// License is located at
6+
//
7+
// http://aws.amazon.com/apache2.0/
8+
//
9+
// or in the "license" file accompanying this file. This file is distributed
10+
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
11+
// express or implied. See the License for the specific language governing
12+
// permissions and limitations under the License.
13+
14+
package table
15+
16+
import (
17+
"testing"
18+
19+
"github.com/aws/aws-sdk-go/aws"
20+
21+
"github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1"
22+
compare "github.com/aws-controllers-k8s/runtime/pkg/compare"
23+
)
24+
25+
func Test_compareResourcePolicyDocument(t *testing.T) {
26+
type args struct {
27+
a *resource
28+
b *resource
29+
}
30+
tests := []struct {
31+
name string
32+
args args
33+
wantDifferent bool
34+
}{
35+
{
36+
name: "both policies are nil",
37+
args: args{
38+
a: &resource{
39+
ko: &v1alpha1.Table{
40+
Spec: v1alpha1.TableSpec{
41+
ResourcePolicy: nil,
42+
},
43+
},
44+
},
45+
b: &resource{
46+
ko: &v1alpha1.Table{
47+
Spec: v1alpha1.TableSpec{
48+
ResourcePolicy: nil,
49+
},
50+
},
51+
},
52+
},
53+
wantDifferent: false,
54+
},
55+
{
56+
name: "desired policy is set, latest policy is nil",
57+
args: args{
58+
a: &resource{
59+
ko: &v1alpha1.Table{
60+
Spec: v1alpha1.TableSpec{
61+
ResourcePolicy: aws.String(`{"Version": "2012-10-17"}`),
62+
},
63+
},
64+
},
65+
b: &resource{
66+
ko: &v1alpha1.Table{
67+
Spec: v1alpha1.TableSpec{
68+
ResourcePolicy: nil,
69+
},
70+
},
71+
},
72+
},
73+
wantDifferent: true,
74+
},
75+
{
76+
name: "identical policy documents",
77+
args: args{
78+
a: &resource{
79+
ko: &v1alpha1.Table{
80+
Spec: v1alpha1.TableSpec{
81+
ResourcePolicy: aws.String(`{
82+
"Version": "2012-10-17",
83+
"Statement": [
84+
{
85+
"Effect": "Allow",
86+
"Principal": {"AWS": "arn:aws:iam::123456789012:root"},
87+
"Action": ["dynamodb:GetItem", "dynamodb:Query"],
88+
"Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable"
89+
}
90+
]
91+
}`),
92+
},
93+
},
94+
},
95+
b: &resource{
96+
ko: &v1alpha1.Table{
97+
Spec: v1alpha1.TableSpec{
98+
ResourcePolicy: aws.String(`{
99+
"Version": "2012-10-17",
100+
"Statement": [
101+
{
102+
"Effect": "Allow",
103+
"Principal": {"AWS": "arn:aws:iam::123456789012:root"},
104+
"Action": ["dynamodb:GetItem", "dynamodb:Query"],
105+
"Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable"
106+
}
107+
]
108+
}`),
109+
},
110+
},
111+
},
112+
},
113+
wantDifferent: false,
114+
},
115+
{
116+
name: "same policy content with different whitespace formatting",
117+
args: args{
118+
a: &resource{
119+
ko: &v1alpha1.Table{
120+
Spec: v1alpha1.TableSpec{
121+
ResourcePolicy: aws.String(`{
122+
"Version": "2012-10-17",
123+
"Statement": [
124+
{
125+
"Effect": "Allow",
126+
"Principal": {"AWS": "arn:aws:iam::123456789012:root"},
127+
"Action": ["dynamodb:GetItem", "dynamodb:Query"],
128+
"Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable"
129+
}
130+
]
131+
}`),
132+
},
133+
},
134+
},
135+
b: &resource{
136+
ko: &v1alpha1.Table{
137+
Spec: v1alpha1.TableSpec{
138+
ResourcePolicy: aws.String(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":"arn:aws:iam::123456789012:root"},"Action":["dynamodb:GetItem","dynamodb:Query"],"Resource":"arn:aws:dynamodb:us-west-2:123456789012:table/MyTable"}]}`),
139+
},
140+
},
141+
},
142+
},
143+
wantDifferent: false,
144+
},
145+
{
146+
name: "different effect in statement",
147+
args: args{
148+
a: &resource{
149+
ko: &v1alpha1.Table{
150+
Spec: v1alpha1.TableSpec{
151+
ResourcePolicy: aws.String(`{
152+
"Version": "2012-10-17",
153+
"Statement": [
154+
{
155+
"Effect": "Allow",
156+
"Principal": {"AWS": "arn:aws:iam::123456789012:root"},
157+
"Action": ["dynamodb:GetItem"],
158+
"Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable"
159+
}
160+
]
161+
}`),
162+
},
163+
},
164+
},
165+
b: &resource{
166+
ko: &v1alpha1.Table{
167+
Spec: v1alpha1.TableSpec{
168+
ResourcePolicy: aws.String(`{
169+
"Version": "2012-10-17",
170+
"Statement": [
171+
{
172+
"Effect": "Deny",
173+
"Principal": {"AWS": "arn:aws:iam::123456789012:root"},
174+
"Action": ["dynamodb:GetItem"],
175+
"Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable"
176+
}
177+
]
178+
}`),
179+
},
180+
},
181+
},
182+
},
183+
wantDifferent: true,
184+
},
185+
{
186+
name: "different actions in statement",
187+
args: args{
188+
a: &resource{
189+
ko: &v1alpha1.Table{
190+
Spec: v1alpha1.TableSpec{
191+
ResourcePolicy: aws.String(`{
192+
"Version": "2012-10-17",
193+
"Statement": [
194+
{
195+
"Effect": "Allow",
196+
"Principal": {"AWS": "arn:aws:iam::123456789012:root"},
197+
"Action": ["dynamodb:GetItem"],
198+
"Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable"
199+
}
200+
]
201+
}`),
202+
},
203+
},
204+
},
205+
b: &resource{
206+
ko: &v1alpha1.Table{
207+
Spec: v1alpha1.TableSpec{
208+
ResourcePolicy: aws.String(`{
209+
"Version": "2012-10-17",
210+
"Statement": [
211+
{
212+
"Effect": "Allow",
213+
"Principal": {"AWS": "arn:aws:iam::123456789012:root"},
214+
"Action": ["dynamodb:Query"],
215+
"Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable"
216+
}
217+
]
218+
}`),
219+
},
220+
},
221+
},
222+
},
223+
wantDifferent: true,
224+
},
225+
}
226+
for _, tt := range tests {
227+
t.Run(tt.name, func(t *testing.T) {
228+
delta := &compare.Delta{}
229+
compareResourcePolicyDocument(delta, tt.args.a, tt.args.b)
230+
if got := delta.DifferentAt("Spec.ResourcePolicy"); got != tt.wantDifferent {
231+
t.Errorf("compareResourcePolicyDocument() difference = %v, want %v", got, tt.wantDifferent)
232+
}
233+
})
234+
}
235+
}

‎test/e2e/requirements.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@38ce32256cc2552ab54e190cc8a8618e93af9e0c
1+
acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@a53a1840e135ba800b4f2aa42b112ba9389f82a7

‎test/e2e/resources/table_resource_policy.yaml‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Table used to test resource policy functionality
2+
apiVersion: dynamodb.services.k8s.aws/v1alpha1
3+
kind: Table
4+
metadata:
5+
name: $TABLE_NAME
6+
spec:
7+
tableName: $TABLE_NAME
8+
billingMode: PAY_PER_REQUEST
9+
tableClass: STANDARD
10+
attributeDefinitions:
11+
- attributeName: Bill
12+
attributeType: S
13+
- attributeName: Total
14+
attributeType: S
15+
keySchema:
16+
- attributeName: Bill
17+
keyType: HASH
18+
- attributeName: Total
19+
keyType: RANGE
20+
resourcePolicy: '$RESOURCE_POLICY'

0 commit comments

Comments
 (0)