Skip late initialization for several duplicate resource policy fields

mbbush commented 6 months ago

Description of your changes

In several places in the terraform aws provider, there are two ways to specify a resource policy: a field named "policy" on the "foo" resource, and a separate resource named "foo_policy".

If we'd realized this before we'd committed to a stable API, perhaps the best option would have been to only provide one of those options, but it's too late for that now.

This PR removes conflicts between the resource policy managed on the parent resource, and the resource policy managed on the Policy resource, by skipping Late Initialization of the policy field on the parent resource, for the following resources:

Queue.sqs
Topic.sns
Role.iam (inlinePolicies)
~Key.kms~
~VpcEndpoint.ec2~

I made a semi-manual pass through the terraform aws provider docs, and these were the only examples I found, although I was not systematic and may have missed something. It seems like the resources with this behavior are the older resources, added to the terraform aws provider before they adopted more uniform best practices.

I initially included the VPC endpoint and KMS key, which both have a spec.forProvider.policy field, and a separate resource in terraform provider aws that sets that policy. However, neither the aws_kms_key_policy nor aws_vpc_endpoint_policy resources exist in this provider yet. I'm undecided about whether it's better to skip late init for these parameters because we may someday implement the corresponding policy resource, whether we should avoid implementing those resources because the field already exists on the parent resource, or whether we should leave them be and defer the decision to the future. For now I've backed those commits out, but left them in the git history so they can be easily restored if desireable.

While I was going through I updated the config files to the latest standards as well.

Fixes #1207

I have:

[x] Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Running uptest on this PR.

I verified by looking at the uptest logs that the policy parameter was set in the status.atProvider and unset in the spec.forProvider, when that was expected, for:

[x] Topic.sns (default policy)
[x] Topic.sns (custom policy from a TopicPolicy resource)
[x] Queue.sqs (custom policy from a QueuePolicy resource)
[x] Role.iam (custom policy from a RolePolicy resource)
[x] Key.kms (on the commit that included it)
[x] Key.kms (now that it's excluded, I verified that the policy is being late initialized.)

SQS queues have a default policy of "", and IAM roles have a default inlinePolicies that is null/empty array, so there's nothing to verify in the default case for those resources.

mbbush commented 6 months ago

/test-examples="examples/sns/v1beta1/topic.yaml"

mbbush commented 6 months ago

/test-examples="examples/sns/v1beta1/topic.yaml"

mbbush commented 6 months ago

/test-examples="examples/iam/v1beta1/role.yaml,examples/iam/v1beta1/role-with-inline-policy.yaml,examples/iam/v1beta1/rolepolicy.yaml"

mbbush commented 6 months ago

/test-examples="examples/sqs/v1beta1/queue.yaml,examples/sqs/v1beta1/queuepolicy.yaml"

mbbush commented 6 months ago

/test-examples="examples/kms/v1beta1/key.yaml"

mbbush commented 6 months ago

/test-examples="examples/kms/v1beta1/key.yaml"