Support Resource Based Policy for DynamoDB

[ShayYannay]

Description of your changes

Support Resource Based Policy for DynamoDB tables

I have:

• [x] Read and followed Crossplane's [contribution process].
• [ ] Run make reviewable to ensure this PR is ready for review.
• [ ] Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Tested locally using Uptest, verified that the resource policy get created for the DynamoDB table:

NAME                                    SYNCED   READY   EXTERNAL-NAME   AGE
table.dynamodb.aws.upbound.io/example   True     True    example         3m7s

NAME                                             SYNCED   READY   EXTERNAL-NAME   AGE
resourcepolicy.dynamodb.aws.upbound.io/example   True     True    example         3m7s

[mbbush]

/test-examples=examples/dynamodb/v1beta1/resourcepolicy.yaml

@ShayYannay Unfortunately you won't be able to trigger this yourself until after your first PR is merged.

[mbbush]

/test-examples="examples/dynamodb/v1beta1/resourcepolicy.yaml"

[mbbush]

Each individual commit needs to be signed off. The output of the DCO job will tell you how to fix it.

[ShayYannay]

/test-examples="examples/dynamodb/v1beta1/resourcepolicy.yaml"

[turkenf]

/test-examples="examples/dynamodb/v1beta1/resourcepolicy.yaml"

[ShayYannay]

@mbbush I have read some nice documentation on how upjet use cross resources dependencies, from what I understand the generated examples use the terraform registry in order to correlated between the resources (e.g resource policy to table in DynamoDB): https://github.com/crossplane/upjet/blob/main/docs/configuring-a-resource.md#cross-resource-referencing

1. Tried to add Type in config.Reference explicitly to DynamoDB table (e.g https://pkg.go.dev/github.com/upbound/provider-aws/apis/dynamodb/v1beta1#Table) saw the make generate did not changed anything since the generated code already automatically inferred the Type
2. Saw that in the terraform registry for dynamodb policy the example is missing aws_dynamodb_table e.g the generated example for resource policy does not include the table as depended manifest.
3. I suspect that the external name for resource policy create some trouble correlating between ResourcePolicy and Table managed resources. I used in my code "aws_dynamodb_resource_policy": config.IdentifierFromProvider, and put it under TerraformPluginFrameworkExternalNameConfigs (saw that its framework and not sdk according to the terraform source code), but could be that this configuration is also wrong since resource policy attached to table is not IAM resource rather than configuration to the Table, another assumption is the terraform state used to parse the arn is missed because of this configuration.

Please advise, thanks!

[ShayYannay]

@mbbush Finally figured it out!! The resource policy id in dynamodb is equal to the dybamodb table arn, e.g the external name should be from template (see my last commit), able to understand that by looking carefully at the terraform state file on the id property. I've ran Uptest locally and now it pass

[mbbush]

I've pieced together what the error message means now.

It strikes me as potentially relevant that none of the existing framework providers use the IdentifierFromProvider external name config. It looks like there's some bug in the upjet Framework client that sends invalid read requests when the resource has not yet been created. I can try to track that down separately from your PR.

As far as what external name configuration you should use, I'd recommend config.TemplatedStringAsIdentifier("", "{{ .parameters.resource_arn }}"),

This will result in the resource not reconciling at all until after it's resolved the reference to the dynamodb table it's referring to, and then as soon as that's done, it will know that's supposed to be the terraform id. The "" is simply to indicate that there's no field in the resource's spec that should be removed and replaced with the external name.

Can you make that change, rebase, and run codegen again, and then I can uptest it in github actions?

One development practice that I try to follow when working on this provider is to put all of the generated changes in separate commits. Then when I'm rebasing, I can just drop all of those commits, not have to deal with conflicts in generated files, and just rerun make generate.

[ShayYannay]

Thanks! @mbbush I will follow your recommendations and change that.

[ShayYannay]

@mbbush tried changing in config/externalname.go from "aws_dynamodb_resource_policy": config.TemplatedStringAsIdentifier("name", "arn:aws:dynamodb:{{ .setup.configuration.region }}:{{ .setup.client_metadata.account_id }}:table/{{ .external_name }}"),
to "aws_dynamodb_resource_policy": config.TemplatedStringAsIdentifier("", "{{ .parameters.resource_arn }}"),

However got this error when running make generate:

angryjet: error: error loading packages using pattern ./...: provider-upjet-aws-dynamodb-rbp/apis/dynamodb/v1beta1/zz_generated.resolvers.go:194:62: mg.Spec.InitProvider.ResourceArn undefined (type ResourcePolicyInitParameters has no field or method ResourceArn)
exit status 1
apis/generate.go:34: running "go": exit status 1
09:30:33 [FAIL]
make[1]: *** [go.generate] Error 1
make: *** [generate] Error 2

Do you know why?

[mbbush]

If you delete all the generated resolvers files before running make generate, you won't get that error.

[ShayYannay]

Got it I will check it out thanks