Skip to content

Commit 78a6f96

Browse files
shabbskagalwalaskagalwala
authored andcommitted
[feat]: allow adding resource policy to dynamodb tables
1 parent baa9115 commit 78a6f96

File tree

4 files changed

+52
-11
lines changed

4 files changed

+52
-11
lines changed

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: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ package table
1515

1616
import (
1717
"context"
18+
"encoding/json"
1819
"errors"
1920
"fmt"
21+
"reflect"
2022
"strings"
2123
"time"
2224

@@ -28,6 +30,7 @@ import (
2830
"github.com/aws/aws-sdk-go-v2/aws"
2931
svcsdk "github.com/aws/aws-sdk-go-v2/service/dynamodb"
3032
svcsdktypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
33+
awsiampolicy "github.com/micahhausler/aws-iam-policy/policy"
3134
corev1 "k8s.io/api/core/v1"
3235

3336
"github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1"
@@ -694,14 +697,7 @@ func customPreCompare(
694697
delta.Add("Spec.ContributorInsights", a.ko.Spec.ContributorInsights, b.ko.Spec.ContributorInsights)
695698
}
696699
}
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-
}
700+
compareResourcePolicyDocument(delta, a, b)
705701

706702
}
707703

@@ -917,3 +913,43 @@ func (rm *resourceManager) updateContributorInsights(
917913

918914
return nil
919915
}
916+
917+
// compareResourcePolicyDocument is a custom comparison function for
918+
// ResourcePolicy documents. The reason why we need a custom function for
919+
// this field is to handle the variability in shapes of JSON objects representing
920+
// IAM policies, especially when it comes to statements, actions, and other fields.
921+
func compareResourcePolicyDocument(
922+
delta *ackcompare.Delta,
923+
a *resource,
924+
b *resource,
925+
) {
926+
// Handle cases where one policy is nil and the other is not.
927+
// This means one resource has a policy and the other doesn't - they're different.
928+
if ackcompare.HasNilDifference(a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy) {
929+
delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy)
930+
return
931+
}
932+
933+
// If both policies are nil, there's no difference - both resources have no policy.
934+
if a.ko.Spec.ResourcePolicy == nil && b.ko.Spec.ResourcePolicy == nil {
935+
return
936+
}
937+
938+
// At this point, both policies are non-nil. We need to compare their JSON content.
939+
// To handle the variability in shapes of JSON objects representing IAM policies,
940+
// especially when it comes to statements, actions, and other fields, we need
941+
// a custom json.Unmarshaller approach crafted to our specific needs. Luckily,
942+
// it happens that @micahhausler built a library dedicated to this very special
943+
// need: github.com/micahhausler/aws-iam-policy.
944+
//
945+
// Copied from IAM Controller: https://github.com/aws-controllers-k8s/iam-controller/blob/main/pkg/resource/role/hooks.go#L398-L432
946+
// Based on review feedback: https://github.com/aws-controllers-k8s/dynamodb-controller/pull/154#discussion_r2443876840
947+
var policyDocumentA awsiampolicy.Policy
948+
_ = json.Unmarshal([]byte(*a.ko.Spec.ResourcePolicy), &policyDocumentA)
949+
var policyDocumentB awsiampolicy.Policy
950+
_ = json.Unmarshal([]byte(*b.ko.Spec.ResourcePolicy), &policyDocumentB)
951+
952+
if !reflect.DeepEqual(policyDocumentA, policyDocumentB) {
953+
delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy)
954+
}
955+
}

pkg/resource/table/hooks_resource_policy.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,13 @@ func (rm *resourceManager) getResourcePolicyWithContext(
121121
ResourceArn: tableARN,
122122
},
123123
)
124-
124+
rm.metrics.RecordAPICall("GET", "GetResourcePolicy", nil)
125125
if err != nil {
126126
if awsErr, ok := ackerr.AWSError(err); ok && awsErr.ErrorCode() == "PolicyNotFoundException" {
127127
return nil, nil
128128
}
129-
rm.metrics.RecordAPICall("GET", "GetResourcePolicy", err)
130129
return nil, err
131130
}
132131

133-
rm.metrics.RecordAPICall("GET", "GetResourcePolicy", nil)
134132
return res.Policy, nil
135133
}

0 commit comments

Comments
 (0)