aws-cf-templates icon indicating copy to clipboard operation
aws-cf-templates copied to clipboard

replace hardcoded aws partition in ARNs with ${AWS::Partition}

Open PatMyron opened this issue 3 years ago • 3 comments

@michaelwittig https://github.com/widdix/aws-cf-templates/issues/191, https://github.com/aws-cloudformation/cfn-lint/pull/1805

https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html#arns-syntax https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-sub.html https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/pseudo-parameter-reference.html#cfn-pseudo-param-partition

brew install gnu-sed
gsed -i 's/arn:aws:/arn:${AWS::Partition}:/g' **/*.yaml

manually added !Sub for a few IAM ARNs

only a couple files left with hardcoded aws partition ARNs after this merges:

PatMyron avatar Dec 20 '21 07:12 PatMyron

@PatMyron Thanks a lot for your contribution. I do have two questions.

  1. What's the motivation for this change? I understand, that cfn-lint advises to use the pseudo-parameter for the AWS partition. Is that the only reason for this change? Or do you plan to deploy the templates in China or GovCloud?
  2. Is there a reason to keep the hardcoded partitions for s3.yaml and cloudtrail.yaml?

My only concern is, that we don't have the capability to run our test suite in China/GovCloud. I expect, that some templates break in these environments. For example, due to missing or limited features/services. What do you think about that? @PatMyron @michaelwittig?

andreaswittig avatar Dec 20 '21 19:12 andreaswittig

do you plan to deploy the templates in China or GovCloud?

Not personally.. for others to run into fewer issues deploying into other partitions

we don't have the capability to run our test suite in China/GovCloud. I expect, that some templates break in these environments

I also no longer have access to those partitions, so I can't test there either. Agree there are likely other issues not yet addressed, but this should be a decent first step

Is there a reason to keep the hardcoded partitions for s3.yaml and cloudtrail.yaml?

state/s3.yaml seems okay anyways since it looks region-specific already: https://github.com/widdix/aws-cf-templates/blob/a1a80efaf52b3859ac95c3525c00244dfee9f1ed/state/s3.yaml#L248-L252

security/cloudtrail.yaml just also needed Sub so I just pushed that too: https://github.com/widdix/aws-cf-templates/blob/b751cec5d12c65ca84695595da3bd7c2a7a7b6f6/security/cloudtrail.yaml#L170

https://github.com/widdix/aws-cf-templates/blob/b751cec5d12c65ca84695595da3bd7c2a7a7b6f6/security/cloudtrail.yaml#L187

PatMyron avatar Dec 20 '21 20:12 PatMyron

The reason why I never worked on #191 is that CloudFormation (sometimes/always?) believes that an attribute changes even if the resulting !Sub has the same value as the String before. So we might replace resources because of that. What we would need to do is this:

  1. Launch stack using old template.
  2. Update using new template and see what happens.

I also agree that we have no way to test the templates in other partitions which is a problem.

michaelwittig avatar Dec 21 '21 08:12 michaelwittig