diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml index 69f4ae6..994533a 100755 --- a/apis/v1alpha1/ack-generate-metadata.yaml +++ b/apis/v1alpha1/ack-generate-metadata.yaml @@ -1,13 +1,13 @@ ack_generate_info: - build_date: "2025-09-19T17:14:03Z" + build_date: "2025-10-21T04:38:02Z" build_hash: 6b4211163dcc34776b01da9a18217bac0f4103fd go_version: go1.24.6 version: v0.52.0 -api_directory_checksum: bcdceff2d7ddf7c98141572260ef2e6cee8bf23f +api_directory_checksum: d2887bf57c4e94a2687e17c41f74c875131c0beb api_version: v1alpha1 aws_sdk_go_version: v1.32.6 generator_config_info: - file_checksum: cc3489b53a45170d339a4de0d7d7ec0aa788955e + file_checksum: 3c4832feff83bc9c29b40bc73bafc1d7e75ab1cd original_file_name: generator.yaml last_modification: reason: API generation diff --git a/apis/v1alpha1/generator.yaml b/apis/v1alpha1/generator.yaml index 47dc75c..42bbe2a 100644 --- a/apis/v1alpha1/generator.yaml +++ b/apis/v1alpha1/generator.yaml @@ -11,7 +11,6 @@ ignore: # Replica of Spec.SSESpecification - TableDescription.SSEDescription - TableDescription.TableClassSummary - - CreateTableInput.ResourcePolicy - CreateTableInput.WarmThroughput operations: UpdateGlobalTable: @@ -29,7 +28,13 @@ resources: custom_field: list_of: CreateReplicationGroupMemberAction compare: - is_ignored: true + is_ignored: true + ResourcePolicy: + from: + operation: PutResourcePolicy + path: Policy + compare: + is_ignored: true GlobalSecondaryIndexesDescriptions: custom_field: list_of: GlobalSecondaryIndexDescription @@ -74,7 +79,7 @@ resources: from: operation: UpdateContributorInsights path: ContributorInsightsAction - compare: + compare: is_ignored: true exceptions: errors: diff --git a/apis/v1alpha1/table.go b/apis/v1alpha1/table.go index 5c10191..d5d60ab 100644 --- a/apis/v1alpha1/table.go +++ b/apis/v1alpha1/table.go @@ -144,6 +144,19 @@ type TableSpec struct { // Account, and Table Quotas (https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Limits.html) // in the Amazon DynamoDB Developer Guide. ProvisionedThroughput *ProvisionedThroughput `json:"provisionedThroughput,omitempty"` + // An Amazon Web Services resource-based policy document in JSON format. + // + // - The maximum size supported for a resource-based policy document is 20 + // KB. DynamoDB counts whitespaces when calculating the size of a policy + // against this limit. + // + // - Within a resource-based policy, if the action for a DynamoDB service-linked + // role (SLR) to replicate data for a global table is denied, adding or deleting + // a replica will fail with an error. + // + // For a full list of all considerations that apply while attaching a resource-based + // policy, see Resource-based policy considerations (https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/rbac-considerations.html). + ResourcePolicy *string `json:"resourcePolicy,omitempty"` // Represents the settings used to enable server-side encryption. SSESpecification *SSESpecification `json:"sseSpecification,omitempty"` // The settings for DynamoDB Streams on the table. These settings consist of: diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 3fed78f..9f3beeb 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -2744,6 +2744,11 @@ func (in *TableSpec) DeepCopyInto(out *TableSpec) { *out = new(ProvisionedThroughput) (*in).DeepCopyInto(*out) } + if in.ResourcePolicy != nil { + in, out := &in.ResourcePolicy, &out.ResourcePolicy + *out = new(string) + **out = **in + } if in.SSESpecification != nil { in, out := &in.SSESpecification, &out.SSESpecification *out = new(SSESpecification) diff --git a/config/crd/bases/dynamodb.services.k8s.aws_tables.yaml b/config/crd/bases/dynamodb.services.k8s.aws_tables.yaml index 5cb696d..efb3ac7 100644 --- a/config/crd/bases/dynamodb.services.k8s.aws_tables.yaml +++ b/config/crd/bases/dynamodb.services.k8s.aws_tables.yaml @@ -325,6 +325,21 @@ spec: format: int64 type: integer type: object + resourcePolicy: + description: |- + An Amazon Web Services resource-based policy document in JSON format. + + * The maximum size supported for a resource-based policy document is 20 + KB. DynamoDB counts whitespaces when calculating the size of a policy + against this limit. + + * Within a resource-based policy, if the action for a DynamoDB service-linked + role (SLR) to replicate data for a global table is denied, adding or deleting + a replica will fail with an error. + + For a full list of all considerations that apply while attaching a resource-based + policy, see Resource-based policy considerations (https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/rbac-considerations.html). + type: string sseSpecification: description: Represents the settings used to enable server-side encryption. properties: diff --git a/generator.yaml b/generator.yaml index 47dc75c..42bbe2a 100644 --- a/generator.yaml +++ b/generator.yaml @@ -11,7 +11,6 @@ ignore: # Replica of Spec.SSESpecification - TableDescription.SSEDescription - TableDescription.TableClassSummary - - CreateTableInput.ResourcePolicy - CreateTableInput.WarmThroughput operations: UpdateGlobalTable: @@ -29,7 +28,13 @@ resources: custom_field: list_of: CreateReplicationGroupMemberAction compare: - is_ignored: true + is_ignored: true + ResourcePolicy: + from: + operation: PutResourcePolicy + path: Policy + compare: + is_ignored: true GlobalSecondaryIndexesDescriptions: custom_field: list_of: GlobalSecondaryIndexDescription @@ -74,7 +79,7 @@ resources: from: operation: UpdateContributorInsights path: ContributorInsightsAction - compare: + compare: is_ignored: true exceptions: errors: diff --git a/go.mod b/go.mod index c41001d..06d20d6 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/dynamodb v1.39.8 github.com/aws/smithy-go v1.22.2 github.com/go-logr/logr v1.4.2 + github.com/micahhausler/aws-iam-policy v0.4.2 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 k8s.io/api v0.32.1 @@ -20,6 +21,10 @@ require ( sigs.k8s.io/controller-runtime v0.20.4 ) +// Temporary fix for github.com/micahhausler/aws-iam-policy. Awaiting for a-hilaly to send +// a PR to micahhausler/aws-iam-policy to build Equal() method for PolicyDocument struct. +replace github.com/micahhausler/aws-iam-policy => github.com/a-hilaly/aws-iam-policy v0.0.0-20231121054900-2c56e839ca53 + require ( github.com/aws/aws-sdk-go-v2/config v1.28.6 // indirect github.com/aws/aws-sdk-go-v2/credentials v1.17.47 // indirect diff --git a/go.sum b/go.sum index 9422e84..abb0332 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/a-hilaly/aws-iam-policy v0.0.0-20231121054900-2c56e839ca53 h1:2uNM0nR2WUDN88EYFxjEaroH+PZJ6k/h9kl+KO0dWVc= +github.com/a-hilaly/aws-iam-policy v0.0.0-20231121054900-2c56e839ca53/go.mod h1:Ojgst9ZFn+VEEJpqtuw/LxVGqEf2+hwWBlkYWvF/XWM= github.com/aws-controllers-k8s/kms-controller v1.0.21 h1:ar8gCdl/l7qbXzr48YN5tNq4vJbB5UqnRH7pAIkP3tI= github.com/aws-controllers-k8s/kms-controller v1.0.21/go.mod h1:tHFXV8lkrzautPPvQtPUJABPlJ9MXPRj8GB1UublGHQ= github.com/aws-controllers-k8s/runtime v0.52.0 h1:Q5UIAn6SSBr60t/DiU/zr6NLBlUuK2AG3yy2ma/9gDU= diff --git a/helm/crds/dynamodb.services.k8s.aws_tables.yaml b/helm/crds/dynamodb.services.k8s.aws_tables.yaml index 951d6b8..e6d3e86 100644 --- a/helm/crds/dynamodb.services.k8s.aws_tables.yaml +++ b/helm/crds/dynamodb.services.k8s.aws_tables.yaml @@ -329,6 +329,21 @@ spec: format: int64 type: integer type: object + resourcePolicy: + description: |- + An Amazon Web Services resource-based policy document in JSON format. + + - The maximum size supported for a resource-based policy document is 20 + KB. DynamoDB counts whitespaces when calculating the size of a policy + against this limit. + + - Within a resource-based policy, if the action for a DynamoDB service-linked + role (SLR) to replicate data for a global table is denied, adding or deleting + a replica will fail with an error. + + For a full list of all considerations that apply while attaching a resource-based + policy, see Resource-based policy considerations (https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/rbac-considerations.html). + type: string sseSpecification: description: Represents the settings used to enable server-side encryption. properties: diff --git a/pkg/resource/table/hooks.go b/pkg/resource/table/hooks.go index dca3307..f380fad 100644 --- a/pkg/resource/table/hooks.go +++ b/pkg/resource/table/hooks.go @@ -166,17 +166,6 @@ func (rm *resourceManager) customUpdateTable( setSyncedCondition(desired, corev1.ConditionFalse, &msg, nil) return desired, requeueWaitWhileCreating } - if isTableUpdating(latest) { - msg := "table is currently being updated" - setSyncedCondition(desired, corev1.ConditionFalse, &msg, nil) - return desired, requeueWaitWhileUpdating - } - if tableHasTerminalStatus(latest) { - msg := "table is in '" + *latest.ko.Status.TableStatus + "' status" - setTerminalCondition(desired, corev1.ConditionTrue, &msg, nil) - setSyncedCondition(desired, corev1.ConditionTrue, nil, nil) - return desired, nil - } // Merge in the information we read from the API call above to the copy of // the original Kubernetes object we passed to the function @@ -188,10 +177,36 @@ func (rm *resourceManager) customUpdateTable( return nil, err } } - if !delta.DifferentExcept("Spec.Tags") { + + // ResourcePolicy can be updated independently of table state + if delta.DifferentAt("Spec.ResourcePolicy") { + if latest.ko.Status.ACKResourceMetadata == nil || latest.ko.Status.ACKResourceMetadata.ARN == nil { + rlog.Debug("skipping ResourcePolicy sync - table ARN not available yet") + return &resource{ko}, requeueWaitWhileCreating + } + + err = rm.syncResourcePolicy(ctx, desired, latest) + if err != nil { + return nil, fmt.Errorf("cannot update table resource policy %v", err) + } + } + + if !delta.DifferentExcept("Spec.Tags", "Spec.ResourcePolicy") { return &resource{ko}, nil } + if isTableUpdating(latest) { + msg := "table is currently being updated" + setSyncedCondition(desired, corev1.ConditionFalse, &msg, nil) + return desired, requeueWaitWhileUpdating + } + if tableHasTerminalStatus(latest) { + msg := "table is in '" + *latest.ko.Status.TableStatus + "' status" + setTerminalCondition(desired, corev1.ConditionTrue, &msg, nil) + setSyncedCondition(desired, corev1.ConditionTrue, nil, nil) + return desired, nil + } + if delta.DifferentAt("Spec.TimeToLive") { if err := rm.syncTTL(ctx, desired, latest); err != nil { // Ignore "already disabled errors" @@ -522,6 +537,15 @@ func (rm *resourceManager) setResourceAdditionalFields( } else { ko.Spec.ContinuousBackups = pitrSpec } + + if ko.Status.ACKResourceMetadata != nil && ko.Status.ACKResourceMetadata.ARN != nil { + policy, err := rm.getResourcePolicyWithContext(ctx, (*string)(ko.Status.ACKResourceMetadata.ARN)) + if err != nil { + return err + } + ko.Spec.ResourcePolicy = policy + } + return nil } @@ -670,6 +694,7 @@ func customPreCompare( delta.Add("Spec.ContributorInsights", a.ko.Spec.ContributorInsights, b.ko.Spec.ContributorInsights) } } + compareResourcePolicyDocument(delta, a, b) } diff --git a/pkg/resource/table/hooks_resource_policy.go b/pkg/resource/table/hooks_resource_policy.go new file mode 100644 index 0000000..7a9f914 --- /dev/null +++ b/pkg/resource/table/hooks_resource_policy.go @@ -0,0 +1,177 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package table + +import ( + "context" + "encoding/json" + "errors" + "reflect" + + ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare" + ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors" + ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log" + svcsdk "github.com/aws/aws-sdk-go-v2/service/dynamodb" + svcsdktypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" + awsiampolicy "github.com/micahhausler/aws-iam-policy/policy" +) + +// syncResourcePolicy updates a DynamoDB table's resource-based policy. +func (rm *resourceManager) syncResourcePolicy( + ctx context.Context, + desired *resource, + latest *resource, +) (err error) { + rlog := ackrtlog.FromContext(ctx) + exit := rlog.Trace("rm.syncResourcePolicy") + defer func(err error) { exit(err) }(err) + + if desired.ko.Spec.ResourcePolicy == nil { + return rm.deleteResourcePolicy(ctx, latest) + } + + return rm.putResourcePolicy(ctx, desired) +} + +// putResourcePolicy attaches or updates a resource-based policy to a DynamoDB table. +func (rm *resourceManager) putResourcePolicy( + ctx context.Context, + r *resource, +) (err error) { + rlog := ackrtlog.FromContext(ctx) + exit := rlog.Trace("rm.putResourcePolicy") + defer func(err error) { exit(err) }(err) + + if r.ko.Spec.ResourcePolicy == nil { + return nil + } + + tableARN := (*string)(r.ko.Status.ACKResourceMetadata.ARN) + if tableARN == nil || *tableARN == "" { + return errors.New("table ARN is required to put resource policy") + } + + _, err = rm.sdkapi.PutResourcePolicy( + ctx, + &svcsdk.PutResourcePolicyInput{ + ResourceArn: tableARN, + Policy: r.ko.Spec.ResourcePolicy, + }, + ) + rm.metrics.RecordAPICall("UPDATE", "PutResourcePolicy", err) + return err +} + +// deleteResourcePolicy removes a resource-based policy from a DynamoDB table. +func (rm *resourceManager) deleteResourcePolicy( + ctx context.Context, + r *resource, +) (err error) { + rlog := ackrtlog.FromContext(ctx) + exit := rlog.Trace("rm.deleteResourcePolicy") + defer func(err error) { exit(err) }(err) + + tableARN := (*string)(r.ko.Status.ACKResourceMetadata.ARN) + if tableARN == nil || *tableARN == "" { + return errors.New("table ARN is required to delete resource policy") + } + + _, err = rm.sdkapi.DeleteResourcePolicy( + ctx, + &svcsdk.DeleteResourcePolicyInput{ + ResourceArn: tableARN, + }, + ) + rm.metrics.RecordAPICall("DELETE", "DeleteResourcePolicy", err) + if err != nil { + var policyNotFoundErr *svcsdktypes.PolicyNotFoundException + if errors.As(err, &policyNotFoundErr) { + // Policy already doesn't exist, this is a success case + return nil + } + } + + return err +} + +// getResourcePolicyWithContext retrieves the resource-based policy of a DynamoDB table. +func (rm *resourceManager) getResourcePolicyWithContext( + ctx context.Context, + tableARN *string, +) (*string, error) { + var err error + rlog := ackrtlog.FromContext(ctx) + exit := rlog.Trace("rm.getResourcePolicyWithContext") + defer func(err error) { exit(err) }(err) + + if tableARN == nil || *tableARN == "" { + return nil, errors.New("table ARN is required to get resource policy") + } + + res, err := rm.sdkapi.GetResourcePolicy( + ctx, + &svcsdk.GetResourcePolicyInput{ + ResourceArn: tableARN, + }, + ) + rm.metrics.RecordAPICall("GET", "GetResourcePolicy", nil) + if err != nil { + if awsErr, ok := ackerr.AWSError(err); ok && awsErr.ErrorCode() == "PolicyNotFoundException" { + return nil, nil + } + return nil, err + } + + return res.Policy, nil +} + +// compareResourcePolicyDocument is a custom comparison function for +// ResourcePolicy documents. The reason why we need a custom function for +// this field is to handle the variability in shapes of JSON objects representing +// IAM policies, especially when it comes to statements, actions, and other fields. +func compareResourcePolicyDocument( + delta *ackcompare.Delta, + a *resource, + b *resource, +) { + // Handle cases where one policy is nil and the other is not. + // This means one resource has a policy and the other doesn't - they're different. + if ackcompare.HasNilDifference(a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy) { + delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy) + return + } + + // If both policies are nil, there's no difference - both resources have no policy. + if a.ko.Spec.ResourcePolicy == nil && b.ko.Spec.ResourcePolicy == nil { + return + } + + // At this point, both policies are non-nil. We need to compare their JSON content. + // To handle the variability in shapes of JSON objects representing IAM policies, + // especially when it comes to statements, actions, and other fields, we need + // a custom json.Unmarshaller approach crafted to our specific needs. Luckily, + // it happens that @micahhausler built a library dedicated to this very special + // need: github.com/micahhausler/aws-iam-policy. + // + // Copied from IAM Controller: https://github.com/aws-controllers-k8s/iam-controller/blob/main/pkg/resource/role/hooks.go#L398-L432 + // Based on review feedback: https://github.com/aws-controllers-k8s/dynamodb-controller/pull/154#discussion_r2443876840 + var policyDocumentA awsiampolicy.Policy + _ = json.Unmarshal([]byte(*a.ko.Spec.ResourcePolicy), &policyDocumentA) + var policyDocumentB awsiampolicy.Policy + _ = json.Unmarshal([]byte(*b.ko.Spec.ResourcePolicy), &policyDocumentB) + + if !reflect.DeepEqual(policyDocumentA, policyDocumentB) { + delta.Add("Spec.ResourcePolicy", a.ko.Spec.ResourcePolicy, b.ko.Spec.ResourcePolicy) + } +} diff --git a/pkg/resource/table/hooks_resource_policy_test.go b/pkg/resource/table/hooks_resource_policy_test.go new file mode 100644 index 0000000..42cde3f --- /dev/null +++ b/pkg/resource/table/hooks_resource_policy_test.go @@ -0,0 +1,235 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package table + +import ( + "testing" + + "github.com/aws/aws-sdk-go/aws" + + "github.com/aws-controllers-k8s/dynamodb-controller/apis/v1alpha1" + compare "github.com/aws-controllers-k8s/runtime/pkg/compare" +) + +func Test_compareResourcePolicyDocument(t *testing.T) { + type args struct { + a *resource + b *resource + } + tests := []struct { + name string + args args + wantDifferent bool + }{ + { + name: "both policies are nil", + args: args{ + a: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + ResourcePolicy: nil, + }, + }, + }, + b: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + ResourcePolicy: nil, + }, + }, + }, + }, + wantDifferent: false, + }, + { + name: "desired policy is set, latest policy is nil", + args: args{ + a: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + ResourcePolicy: aws.String(`{"Version": "2012-10-17"}`), + }, + }, + }, + b: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + ResourcePolicy: nil, + }, + }, + }, + }, + wantDifferent: true, + }, + { + name: "identical policy documents", + args: args{ + a: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + 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" + } + ] + }`), + }, + }, + }, + b: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + 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" + } + ] + }`), + }, + }, + }, + }, + wantDifferent: false, + }, + { + name: "same policy content with different whitespace formatting", + args: args{ + a: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + 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" + } + ] + }`), + }, + }, + }, + b: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + 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"}]}`), + }, + }, + }, + }, + wantDifferent: false, + }, + { + name: "different effect in statement", + args: args{ + a: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + ResourcePolicy: aws.String(`{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": {"AWS": "arn:aws:iam::123456789012:root"}, + "Action": ["dynamodb:GetItem"], + "Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable" + } + ] + }`), + }, + }, + }, + b: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + ResourcePolicy: aws.String(`{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Deny", + "Principal": {"AWS": "arn:aws:iam::123456789012:root"}, + "Action": ["dynamodb:GetItem"], + "Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable" + } + ] + }`), + }, + }, + }, + }, + wantDifferent: true, + }, + { + name: "different actions in statement", + args: args{ + a: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + ResourcePolicy: aws.String(`{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": {"AWS": "arn:aws:iam::123456789012:root"}, + "Action": ["dynamodb:GetItem"], + "Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable" + } + ] + }`), + }, + }, + }, + b: &resource{ + ko: &v1alpha1.Table{ + Spec: v1alpha1.TableSpec{ + ResourcePolicy: aws.String(`{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": {"AWS": "arn:aws:iam::123456789012:root"}, + "Action": ["dynamodb:Query"], + "Resource": "arn:aws:dynamodb:us-west-2:123456789012:table/MyTable" + } + ] + }`), + }, + }, + }, + }, + wantDifferent: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + delta := &compare.Delta{} + compareResourcePolicyDocument(delta, tt.args.a, tt.args.b) + if got := delta.DifferentAt("Spec.ResourcePolicy"); got != tt.wantDifferent { + t.Errorf("compareResourcePolicyDocument() difference = %v, want %v", got, tt.wantDifferent) + } + }) + } +} diff --git a/pkg/resource/table/sdk.go b/pkg/resource/table/sdk.go index 3657409..62b1716 100644 --- a/pkg/resource/table/sdk.go +++ b/pkg/resource/table/sdk.go @@ -948,28 +948,31 @@ func (rm *resourceManager) newCreateRequestPayload( } res.ProvisionedThroughput = f6 } + if r.ko.Spec.ResourcePolicy != nil { + res.ResourcePolicy = r.ko.Spec.ResourcePolicy + } if r.ko.Spec.SSESpecification != nil { - f7 := &svcsdktypes.SSESpecification{} + f8 := &svcsdktypes.SSESpecification{} if r.ko.Spec.SSESpecification.Enabled != nil { - f7.Enabled = r.ko.Spec.SSESpecification.Enabled + f8.Enabled = r.ko.Spec.SSESpecification.Enabled } if r.ko.Spec.SSESpecification.KMSMasterKeyID != nil { - f7.KMSMasterKeyId = r.ko.Spec.SSESpecification.KMSMasterKeyID + f8.KMSMasterKeyId = r.ko.Spec.SSESpecification.KMSMasterKeyID } if r.ko.Spec.SSESpecification.SSEType != nil { - f7.SSEType = svcsdktypes.SSEType(*r.ko.Spec.SSESpecification.SSEType) + f8.SSEType = svcsdktypes.SSEType(*r.ko.Spec.SSESpecification.SSEType) } - res.SSESpecification = f7 + res.SSESpecification = f8 } if r.ko.Spec.StreamSpecification != nil { - f8 := &svcsdktypes.StreamSpecification{} + f9 := &svcsdktypes.StreamSpecification{} if r.ko.Spec.StreamSpecification.StreamEnabled != nil { - f8.StreamEnabled = r.ko.Spec.StreamSpecification.StreamEnabled + f9.StreamEnabled = r.ko.Spec.StreamSpecification.StreamEnabled } if r.ko.Spec.StreamSpecification.StreamViewType != nil { - f8.StreamViewType = svcsdktypes.StreamViewType(*r.ko.Spec.StreamSpecification.StreamViewType) + f9.StreamViewType = svcsdktypes.StreamViewType(*r.ko.Spec.StreamSpecification.StreamViewType) } - res.StreamSpecification = f8 + res.StreamSpecification = f9 } if r.ko.Spec.TableClass != nil { res.TableClass = svcsdktypes.TableClass(*r.ko.Spec.TableClass) @@ -978,18 +981,18 @@ func (rm *resourceManager) newCreateRequestPayload( res.TableName = r.ko.Spec.TableName } if r.ko.Spec.Tags != nil { - f11 := []svcsdktypes.Tag{} - for _, f11iter := range r.ko.Spec.Tags { - f11elem := &svcsdktypes.Tag{} - if f11iter.Key != nil { - f11elem.Key = f11iter.Key + f12 := []svcsdktypes.Tag{} + for _, f12iter := range r.ko.Spec.Tags { + f12elem := &svcsdktypes.Tag{} + if f12iter.Key != nil { + f12elem.Key = f12iter.Key } - if f11iter.Value != nil { - f11elem.Value = f11iter.Value + if f12iter.Value != nil { + f12elem.Value = f12iter.Value } - f11 = append(f11, *f11elem) + f12 = append(f12, *f12elem) } - res.Tags = f11 + res.Tags = f12 } return res, nil diff --git a/test/e2e/requirements.txt b/test/e2e/requirements.txt index 8d94056..bf13ec2 100644 --- a/test/e2e/requirements.txt +++ b/test/e2e/requirements.txt @@ -1 +1 @@ -acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@38ce32256cc2552ab54e190cc8a8618e93af9e0c +acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@a53a1840e135ba800b4f2aa42b112ba9389f82a7 diff --git a/test/e2e/resources/table_resource_policy.yaml b/test/e2e/resources/table_resource_policy.yaml new file mode 100644 index 0000000..d4f1a1e --- /dev/null +++ b/test/e2e/resources/table_resource_policy.yaml @@ -0,0 +1,20 @@ +# Table used to test resource policy functionality +apiVersion: dynamodb.services.k8s.aws/v1alpha1 +kind: Table +metadata: + name: $TABLE_NAME +spec: + tableName: $TABLE_NAME + billingMode: PAY_PER_REQUEST + tableClass: STANDARD + attributeDefinitions: + - attributeName: Bill + attributeType: S + - attributeName: Total + attributeType: S + keySchema: + - attributeName: Bill + keyType: HASH + - attributeName: Total + keyType: RANGE + resourcePolicy: '$RESOURCE_POLICY' diff --git a/test/e2e/table.py b/test/e2e/table.py index c85a0ae..dc45b7a 100644 --- a/test/e2e/table.py +++ b/test/e2e/table.py @@ -274,6 +274,27 @@ def get_point_in_time_recovery_enabled(table_name): return None +def get_resource_policy(table_arn): + """Returns the resource policy for the table with a supplied ARN. + + Args: + table_arn: the ARN of the DynamoDB table + + Returns: + A dict representing the JSON policy document and Revision Id of the policy or None if no policy exists + """ + c = boto3.client('dynamodb', region_name=get_region()) + try: + resp = c.get_resource_policy(ResourceArn=table_arn) + if 'Policy' in resp and resp['Policy']: + return resp + return None + except c.exceptions.PolicyNotFoundException: + return None + except c.exceptions.ResourceNotFoundException: + return None + + class ReplicaMatcher: def __init__(self, expected_regions): self.expected_regions = expected_regions diff --git a/test/e2e/tests/test_table.py b/test/e2e/tests/test_table.py index cad55f6..63e8253 100644 --- a/test/e2e/tests/test_table.py +++ b/test/e2e/tests/test_table.py @@ -14,6 +14,7 @@ """Integration tests for the DynamoDB Table API. """ +import json import logging import time from typing import Dict, Tuple @@ -21,6 +22,7 @@ import boto3 import pytest from acktest import tags +from acktest.aws.identity import get_region, get_account_id from acktest.k8s import resource as k8s from acktest.resources import random_suffix_name from e2e import (CRD_GROUP, CRD_VERSION, condition, get_resource_tags, @@ -162,12 +164,85 @@ def table_basic_pay_per_request(): except: pass + +@pytest.fixture(scope="function") +def table_resource_policy(): + resource_name = random_suffix_name("table-resource-policy", 32) + account_id = get_account_id() + region = get_region() + resource_policy = { + "Version": "2012-10-17", + "Id": "ack-table-with-policy", + "Statement": [ + { + "Sid": "EnableResourcePolicyOnTable", + "Effect": "Allow", + "Principal": { + "AWS": f'arn:aws:iam::{account_id}:root' + }, + "Action": [ + "dynamodb:GetItem", + "dynamodb:PutItem", + "dynamodb:Query", + "dynamodb:Scan" + ], + "Resource": f'arn:aws:dynamodb:{region}:{account_id}:table/{resource_name}' + } + ] + } + + replacements = REPLACEMENT_VALUES.copy() + replacements["TABLE_NAME"] = resource_name + replacements["RESOURCE_POLICY"] = json.dumps(resource_policy) + + # Create the k8s resource + resource_data = load_dynamodb_resource( + "table_resource_policy", + additional_replacements=replacements, + ) + + ref = k8s.CustomResourceReference( + CRD_GROUP, CRD_VERSION, "tables", + resource_name, namespace="default", + ) + + time.sleep(CREATE_WAIT_AFTER_SECONDS) + + # Create table + k8s.create_custom_resource(ref, resource_data) + cr = k8s.wait_resource_consumed_by_controller(ref) + + assert cr is not None + assert k8s.get_resource_exists(ref) + + # Wait for the resource to be synced (table created and policy applied) + # Resource policy is applied after table creation + assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=90) + + # Now wait for table to be ACTIVE + wait_for_cr_status( + ref, + "tableStatus", + "ACTIVE", + 90, + 3, + ) + + yield (ref, cr, resource_policy) + + try: + _, deleted = k8s.delete_custom_resource(ref, wait_periods=3, period_length=10) + assert deleted + except: + pass + + @service_marker @pytest.mark.canary class TestTable: def table_exists(self, table_name: str) -> bool: return table.get(table_name) is not None - + def table_insight_status(self, table_name: str, status: str) -> bool: return table.get(table_name) is not None @@ -218,7 +293,6 @@ def test_table_update_tags(self, table_lsi): actual=table_tags, ) - cr = k8s.wait_resource_consumed_by_controller(ref) # make multiple updates at once new_tags = [ @@ -471,7 +545,7 @@ def test_update_provisioned_throughput(self, table_lsi): timeout_seconds=MODIFY_WAIT_AFTER_SECONDS, interval_seconds=3, ) - + def test_update_insights(self, table_insights): (ref, res) = table_insights @@ -497,7 +571,6 @@ def test_update_insights(self, table_insights): cr = k8s.get_resource(ref) assert cr['spec']['contributorInsights'] == "DISABLE" assert self.table_insight_status(table_name, "DISABLED") - def test_enable_sse_specification(self, table_lsi): (ref, res) = table_lsi @@ -574,7 +647,6 @@ def test_update_class(self, table_lsi): # Set table class cr["spec"]["tableClass"] = "STANDARD" - # Patch k8s resource k8s.patch_custom_resource(ref, cr) @@ -689,7 +761,6 @@ def test_create_delete_gsi(self, table_gsi): new_gsi_dict("office-per-country", "Country", 5), ] - # Patch k8s resource k8s.patch_custom_resource(ref, cr) @@ -731,7 +802,6 @@ def test_create_update_gsi(self, table_gsi): new_gsi_dict("office-per-city", "City", 10), # update op new_gsi_dict("office-per-country", "Country", 5), # new gsi ] - # Patch k8s resource k8s.patch_custom_resource(ref, cr) @@ -880,6 +950,7 @@ def test_create_gsi_pay_per_request(self, table_basic_pay_per_request): timeout_seconds=MODIFY_WAIT_AFTER_SECONDS*40, interval_seconds=15, ) + def test_create_gsi_same_attributes(self, table_basic): (ref, res) = table_basic @@ -912,7 +983,7 @@ def test_create_gsi_same_attributes(self, table_basic): cr["spec"]['globalSecondaryIndexes'] = [ gsi, ] - + # Patch k8s resource k8s.patch_custom_resource(ref, cr) k8s.wait_resource_consumed_by_controller(ref) @@ -927,3 +998,45 @@ def test_create_gsi_same_attributes(self, table_basic): interval_seconds=15, ) + def test_resource_policy(self, table_resource_policy): + (ref, res, resource_policy) = table_resource_policy + table_name = res["spec"]["tableName"] + + assert self.table_exists(table_name) + + # https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_GetResourcePolicy.html + # Need to wait and use an arn to query if the resource policy is added to the table + condition.assert_synced(ref) + cr = k8s.wait_resource_consumed_by_controller(ref) + table_arn = cr["status"]["ackResourceMetadata"]["arn"] + + policy = table.get_resource_policy(table_arn) + assert policy is not None + assert 'ack-table-with-policy' in policy['Policy'] + + # Update resource policy - add statement ID to verify update + resource_policy['Id'] = 'updated-table-policy' + updates = { + "spec": { + "resourcePolicy": json.dumps(resource_policy) + } + } + + k8s.patch_custom_resource(ref, updates) + time.sleep(MODIFY_WAIT_AFTER_SECONDS) + assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=10) + + # Verify policy was updated + policy = table.get_resource_policy(table_arn) + assert 'updated-table-policy' in policy['Policy'] + + # Delete resource policy + cr = k8s.wait_resource_consumed_by_controller(ref) + cr["spec"]["resourcePolicy"] = None + + k8s.patch_custom_resource(ref, cr) + time.sleep(MODIFY_WAIT_AFTER_SECONDS) + + # Verify policy was deleted + deleted_policy = table.get_resource_policy(table_arn) + assert deleted_policy is None