-
Notifications
You must be signed in to change notification settings - Fork 258
feat(clover): add cloudformationonly prop to cloudformation aws assets #8283
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(clover): add cloudformationonly prop to cloudformation aws assets #8283
Conversation
Dependency Review✅ No vulnerabilities or OpenSSF Scorecard issues found.Scanned FilesNone |
c14592b to
6403fe8
Compare
|
Working with Module Index at: https://module-index.systeminit.com AWS - 0 new, 203 changed asset(s) |
|
/diff AWS::SNS::TopicPolicy |
|
Working with Module Index at: https://module-index.systeminit.com Diffed AWS::SNS::TopicPolicy with the module index:Added function Set attributes for building assets in CloudFormation:Added prop /root/domain/extra/CloudFormationOnly:Replaced value within leafFunctions binding:Removed leafFunctions binding |
| const [variant] = schema.variants; | ||
|
|
||
| if (!spec.name.includes("::") || variant.superSchema.handlers) { | ||
| if (!spec.name.includes("::") || variant.superSchema.handlers?.read) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about this. Why should we change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It adds the prop to the assets that aren't supported by cloudcontrol and also lack the read 'handler' - so if I remove this part, the prop isn't added to 5 assets. If we don't want this I could add overrides to those 5 assets instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we had this way originally as no handlers meant no support, but I suppose read only could mean the same thing
I guess we've always had these wrong then?
This PR:
CloudFormationOnlyboolean prop todomain/extrafor 202 AWS assets that lack CloudControl supportdefaultValue: "true"andhidden: trueso it doesn't appear in the UIScreenshots:
CloudFormation asset:

CloudControl asset:

How was it tested?
Does it require a docs change?
In short: 🔗