GetAtt coverage - Githubissues

rhboyd commented 5 years ago

I'm not using the template on this one because it cuts across many services/resources/etc.....

If a resource has an Arn, Fn::GetAtt [myResource, Arn] should return said Arn. If I get a cfn failure one more time because !GetAtt myResource.Arn is undefined and !Ref myResource returns the Arn instead, I will write a VERY sternly worded tweet (maybe a couple).

I recognize that there's a bigger debate about "Arn all the thingz!!!" bu tthat's not what I'm asking for. All I want is GetAtt Arn to work if the Resource has an Arn.

mbonig commented 5 years ago

Whatever the !Ref returns should be available as a straight !GetAtt too.

rjlohan commented 5 years ago

Is there value in enabling the !GetAtt function to retrieve any attribute of a resource, rather than the current subset we offer?

!Ref has special meaning in that it returns the identifier of the physical resource, which is what CloudFormation uses internally to track your things so I don't think there's any concerns in how that function behaves.

rhboyd commented 5 years ago

Is there value in enabling the !GetAtt function to retrieve any attribute of a resource, rather than the current subset we offer?

Is +∞ an appropriate amount of value? That would make working with CloudFormation SO much easier. Personally, I avoid !Ref at all costs because I have to interrupt and go check the docs to see what !Ref returns for a specific resource type. Making the returned value explicit with !GetAtt would be YUGE!!!!

benkehoe commented 5 years ago

The CloudFormation team should enforce that when teams add new resources to CloudFormation, they have both a Name attribute (just "Name", not "FooName" for a Foo resource unless there is a very good reason) and an Arn attribute (case sensitive, I don't care if you choose Arn or ARN, but enforce one of them).

rjlohan commented 5 years ago

The CloudFormation team should enforce that when teams add new resources to CloudFormation, they have both a Name attribute (just "Name", not "FooName" for a Foo resource unless there is a very good reason) and an Arn attribute (case sensitive, I don't care if you choose Arn or ARN, but enforce one of them).

That enforcement is probably more a function of accepting a Name on the underlying service APIs, which plenty do not.

benkehoe commented 5 years ago

The attributes of a CloudFormation resource should not have to match the underlying APIs at the expense of user confusion.

rjlohan commented 5 years ago

The attributes of a CloudFormation resource should not have to match the underlying APIs at the expense of user confusion.

Hmmm... so maybe you could clarify for me where the Name data would reside? If it's not attached at the underlying service would it be metadata only available in CloudFormation? In that case, the existing logical resource ID of a resource is that same information?

benkehoe commented 5 years ago

The CFN resource developed by the service team would be responsible for mapping between the required "Name" attribute and the corresponding "Name" parameter in their APIs. Who knows, maybe it would eventually drive consistency in the APIs!

mdavis-xyz commented 5 years ago

More broadly, I think GetAtt should also work for all Properties.

For example, if I have:

    PublicDomain:
        Type: AWS::Route53::HostedZone
        Properties: 
          Name: !Join [ ".", [ !Ref "stage", "something", "other", "example", "com"] ]

I would like to be able to use !GetAtt ["PublicDomain", "Name"], and have it return the result of !Join [ ".", [ !Ref "stage", "something", "other", "example", "com"] ]. I don't want to have to copy and paste that !Join statement throughout my template. Copy-ing and pasting is almost always bad. (e.g. what happens when I modify one instance of that !Join expression and I forget to change the others.)

rhboyd commented 5 years ago

That !Join should be

!Sub "${stage}.something.other.example.com"

Your teammates and future-you will thank you.

mbonig commented 5 years ago

The attributes of a CloudFormation resource should not have to match the underlying APIs at the expense of user confusion.

It would create a disconnect between the CFN definition and the underlying API calls which itself is confusing.

rhboyd commented 5 years ago

CFN shouldn't be seen as "SDK in the cloud". That's what leads people to thinking that concepts like Terraform are a good idea. It's completely normal for the CFN Definition to deviate from the service's API as long as the CFN Definition is well documented. CFN has the capability to create many different layers of abstraction based on what the user needs in that case, while the API is usually constrained to the single lowest layer.

benkehoe commented 5 years ago

CFN shouldn't be seen as "SDK in the cloud"

Seconded. Service resources have lifecycles that are not always well-represented by mapping directly to API calls. CloudFormation needs to internalize this; the lack of this awareness is why we can't deploy two versions of a Lambda function in a single template.

reidca commented 5 years ago

More broadly, I think GetAtt should also work for all Properties.

For example, if I have:
    PublicDomain:
        Type: AWS::Route53::HostedZone
        Properties: 
          Name: !Join [ ".", [ !Ref "stage", "something", "other", "example", "com"] ] 
I would like to be able to use !GetAtt ["PublicDomain", "Name"], and have it return the result of !Join [ ".", [ !Ref "stage", "something", "other", "example", "com"] ]. I don't want to have to copy and paste that !Join statement throughout my template. Copy-ing and pasting is almost always bad. (e.g. what happens when I modify one instance of that !Join expression and I forget to change the others.)

I agree and furthermore it should allow properties from existing resources to be returned.

I have created a custom resource that takes in some uniquely identifiable characteristic of existing resources and returns anything which could be useful to other resources in the template. I would love this functionality to be built into Cfn without me having to maintain the python code and ecer expand it for new resources.

Without this I was finding it impossible to work with resources created outside of that particular template or created manually in the console. Using exports and imports is problematic because of the dependencies it puts on stacks and because each resource has to have individual properties exposed instead of the resource being treated as an object.

Any improvement in this area would be a major benefit.

fimbulvetr commented 5 years ago

The attributes of a CloudFormation resource should not have to match the underlying APIs at the expense of user confusion.

Disagreed. The fact that CF is extremely similar to the API is exactly what makes it easy to reason about.

PatMyron commented 5 years ago

Not up-to-date, but this quickly identifies lacking GetAtt ARN coverage: https://theburningmonk.com/cloudformation-ref-and-getatt-cheatsheet/

up-to-date command for GetAtt attributes:

curl -s --compressed https://d1uauaxba7bl26.cloudfront.net/latest/gzip/CloudFormationResourceSpecification.json | jq '.ResourceTypes' | jq 'with_entries(.value |= .Attributes)' | grep -v ': null' | grep -v 'Type": "'

ARNs can also frequently be constructed using other return values, Pseudo Parameters for Partition, Region, and AccountId, and the Sub intrinsic function like this if anyone needs workarounds: https://stackoverflow.com/a/59362496/4122849

rjlohan commented 4 years ago

One concern we've had with allowing !GetAtt for anything is that it could be inadvertently used on highly volatile properties leading to unexpected changes in stack updates. At this time, !GetAtt is mostly reserved to properties which are createOnly and so do not change on update (and then lead to a propagation of unexpected changes).

How do folks feel about this risk/concern?

benbridts commented 4 years ago

@rjlohan I think one way to reduce that risk is to make change sets smarter. AFAIK using GetAtt is always shown as a "conditional" replacement if the referenced resource gets updated. It could follow the same behaviour as a !Ref when the !GetAtt is used on an additionalIdentifier (like an Arn).

There might be value in a schema key the define properties that are not identifiers, but tied to the lifecycle of the resource (RequiresReplacementProperties?). For example the subnetId of an EC2 instance.

To give a more concrete answer:

I think there are certain properties that deserve a special case. Arn is definitely one of those. However, I think care should be taken that the change set behaviour for !Ref Resource and !GetAtt Resource.Arn should be the same
Some resources are missing output fields that make them almost unusable, this should be a priority, because it's even more frustrating to have something unusable in CloudFormation than have nothing at all. I think this is also a part of why people are asking to make everything an attribute (and with reason)
If the reason for being able to reference all attributes of a resource within a stack is DRY, I'd rather see a "Constants" section, that looks a bit like the one blow (bonus points for being able to use resolves in there). I think having them work similar to Conditions (so being able to use Parameters/Mappings) might already solve some of the issues raised here.
If adding more Attributes will turn every change set in "I don't know, conditional", this should be a reason to improve change sets, not to block attributes (although I'd prefer the change set improvement and constants first, before making everything an attribute)

Constants:
  MyDomainName: !Sub "${stage}.something.other.example.com"

rjlohan commented 4 years ago

Thinking more on this and the issue I'm still wrangling with is how/when to evaluate the !GetAtt - specifically during stack updates. I think it makes sense to continue operating as we currently do - which is that the !Ref and !GetAtt values are only evaluated if the target resource itself is updated by CloudFormation. This would mean that we don't go off reading the state of the world and having out-of-band changes to target properties turn into surprise updates as the dependency graph is re-evaluated.

Importantly, this means that your !GetAtt value would always be 'pinned' to whatever the value was when the target resource was last updated by CloudFormation.

Thoughts?

fimbulvetr commented 4 years ago

Thinking more on this and the issue I'm still wrangling with is how/when to evaluate the !GetAtt - specifically during stack updates. I think it makes sense to continue operating as we currently do - which is that the !Ref and !GetAtt values are only evaluated if the target resource itself is updated by CloudFormation. This would mean that we don't go off reading the state of the world and having out-of-band changes to target properties turn into surprise updates as the dependency graph is re-evaluated.

Importantly, this means that your !GetAtt value would always be 'pinned' to whatever the value was when the target resource was last updated by CloudFormation.

Thoughts?

@rjlohan

I am tending to agree with you on this. Trying to reword it here: It does help get more GetAtts support, but comes at the cost of the GetAtts not being refreshed if you changed the the property the getatt references in the console or api. It would only refresh the GetAtt if the update was done by CloudFormation because otherwise CloudFormation would have to crawl every resources property that's GetAtt'd to everytime there is an update - and that goes against one of (my) major complaints about CloudFormation changesets - they're so slow.

For instance, and this is a contrived example, let's say we have an SQS queue and a Lambda function as resources. The SQS has a VisibilityTimeout of 30. We wanted the lambda to have the same Timeout, so you gave us a GetAtt for VisibilityTimeout on the SQS queue. If we defined our lambda's Timeout as Timeout: !GetAtt sqsQueue.VisibilityTimeout, then the following is true:

If we change the SQS's VisibilityTimeout to 45 seconds in Cloudformation and redeploy, the Lambda's Timeout would also get updated to 45s.
If we change the SQS's VisibilityTimeout to 45 seconds in the web console, or via API, the Lambda's Timeout would not change because Cloudformation was unaware of the change. We would have do step 1 in order for the lambda to also change.

We see similar behavior in using a {{resolve...secretmanager...secretid}} where secretId is the generic path without the version id. (i.e. /foo/bar/secret instead of /foo/bar/secret-kabcd1) - because CF is never aware the secret has changed as you are always using it's generic path and not using an explicit version. We are fine with this behavior because the positives outweigh the negatives for our particular use cases.

benkehoe commented 4 years ago

I think I'd agree for "refresh of attributes only happens when resource is updated", but I'd follow that with "drift detection should detect that attributes in use have changed".

As for @ikben's Constants section, that's open as aws-cloudformation/cfn-language-discussion#55.

PatMyron commented 4 years ago

From parsing the resource specification, there are currently 496 existing GetAtt attributes total: 100 are not readOnlyProperties. Of the 100 non-readOnlyProperties, 67 are createOnlyProperties and 33 are not:

import requests
r = requests.get('https://d1uauaxba7bl26.cloudfront.net/latest/gzip/CloudFormationResourceSpecification.json')
for type, value in r.json()['ResourceTypes'].items():
  if 'Attributes' in value:
    for attribute in value['Attributes']:
      if attribute in value['Properties']:
        print(value['Properties'][attribute]['UpdateType'] + ': ' + type + '.' + attribute)

non-readOnlyProperties-GetAtt-attributes.txt (sorted data from July and September 2020 combined)

To properly migrate more of those resource types to the CloudFormation Registry while maintaining GetAtt backwards compatibility, I would like the CloudFormation Registry resource types to expand automatic GetAtt support beyond just the currently supported readOnlyProperties and support at least createOnlyProperties as well (although all writeOnlyProperties should be excluded if we expand beyond readOnlyProperties)

Supporting beyond that raises a more difficult question of whether mutable properties that could have changed out-of-band should read from templates or live state, but we shouldn't need to deal with that to expand to createOnlyProperties, except for the possibility of createOnlyProperties that become non-createOnlyProperties later on

https://github.com/aws-cloudformation/aws-cloudformation-resource-schema/#resource-semantics

onegovee commented 3 years ago

What Amazon wants you to do is create a lambda that does a describe on the resource(s) in question then parse the attributes you want and return them as a cfn custom resource.

mayormaier commented 1 year ago

Hi All, I'd like to bump this discussion, but specifically for a GetAtt on the 'arn' value of a resource. I'm thinking about this in terms of EC2::Instance resources, and other resources that follow a similar model. For resources where the resource ID is assigned by AWS (e.g., a EC2 Instance ID), a GetAtt should be supported for the ARN of that resource. Thoughts?

r-heimann commented 1 year ago

All of my opened issues are in regards to

Whatever the !Ref returns should be available as a straight !GetAtt too.

These are, according to the CFN Docs, not available as a !GetAtt and only usable through !Ref. Since those already exist as !Ref it should be easy for the CFN Devs to implement those as !GetAtt and update the CFN Docs. If i find more CFN Resources which have !Ref but are missing its !GetAtt i'll reference them here. Believe it or not but some of the CFN Resources actually have this properly implemented... Here's hoping that in the future this will be supportes out-of-the-box for new services or this'll be a nerver-ending issue.

srcndogan commented 1 year ago

We should be able to retrieve arns for all resource types that have ARNs. While requesting on all of them wanna identify below resources as well for the FR ASAP please. AWS::Glue::Database AWS::Glue::Table AWS::Logs::LogStream

tb3088 commented 1 year ago

The previous is mine. It is unconscionable that 4 years in and the team has yet to squash ALL of their published object types. You (AWS) hire summer interns by the bushel-load. No more perfect opportunity to do the grunt work of validating EVERY object for essential attribute support and updating the documentation. What is so hard about REFUSING acceptance from a service team that tries to deliver an incomplete implementation? A thousand eyes in Seattle might as well be struck with blindness how utter crap API consistency and coverage is. You're supposed to write tooling to force compliance.

The various official blog and 'help' pages with their example templates intended for customer consumption are shockingly badly written, and Arn being unavailable is no little part of that.

Any competent API team also knows you HIDE implementation-specific quirks from the user. So yes, a "Name" attribute absolutely should be supported and transparently mapped to whatever private XXXName the Service team chose to use. You can support the private one too, but having to open the Service API in order to decipher CF is nuts!

ex-AWS employee

aws-cloudformation / cloudformation-coverage-roadmap

GetAtt coverage #68