-
Notifications
You must be signed in to change notification settings - Fork 258
fix(clover): cloudformation only asset overhaul #8336
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?
Conversation
Dependency Review✅ No vulnerabilities or OpenSSF Scorecard issues found.Scanned FilesNone |
dccdb35 to
f202397
Compare
|
Working with Module Index at: https://module-index.systeminit.com AWS - 2 new, 219 changed asset(s)Hetzner - 0 new, 11 changed asset(s) |
|
Seems to have picked up the same changes as the nightly run for some reason, that PR was merged a few hours ago. |
|
/diff AWS::Route53::RecordSet |
|
/diff AWS::EC2::NetworkAclEntry |
|
Working with Module Index at: https://module-index.systeminit.com Diffed AWS::Route53::RecordSet with the module index:Added function CloudFormation Lint Validation:Added function Code Gen for CloudFormation-only assets:Added leafFunctions binding:Added prop /root/domain/CloudFormationResourceBody:Added prop /root/domain/properties:Replaced value within actionFuncs binding:Replaced value within actionFuncs binding:Replaced value within function Create Route53 RecordSet:Replaced value within function Discover on AWS:Replaced value within function Import from AWS:Replaced value within function Refresh Route53 RecordSet:Replaced value within function Set attributes for building assets in CloudFormation:Replaced value within function Set attributes for building assets in CloudFormation:Replaced value within function Set attributes for building assets in CloudFormation:Replaced value within function Set attributes for building assets in CloudFormation:Replaced value within leafFunctions binding:Replaced value within managementFuncs binding:Replaced value within managementFuncs binding:Removed function Set attributes for building assets in CloudFormation |
|
Working with Module Index at: https://module-index.systeminit.com Diffed AWS::EC2::NetworkAclEntry with the module index:Added function CloudFormation Lint Validation:Added function Code Gen for CloudFormation-only assets:Added leafFunctions binding:Added prop /root/domain/CloudFormationResourceBody:Added prop /root/domain/properties:Replaced value within function Set attributes for building assets in CloudFormation:Replaced value within function Set attributes for building assets in CloudFormation:Replaced value within function Set attributes for building assets in CloudFormation:Replaced value within function Set attributes for building assets in CloudFormation:Replaced value within leafFunctions binding:Removed function Set attributes for building assets in CloudFormation |
| // CF-only specific funcs - NOT included in provider-wide specs | ||
| // These are only added to CF-only assets in pruneCfAssets.ts | ||
| export const CF_ONLY_FUNC_SPECS = { | ||
| awsCfOnlyCodeGen: { | ||
| id: "f66e1afc8ff0f43f3b8a15c87f0a916243904de894e33322beb7f84dc792305a", | ||
| backendKind: "jsAttribute", | ||
| responseType: "codeGeneration", | ||
| displayName: "Code Gen for CloudFormation-only assets", | ||
| path: "./src/pipelines/aws/funcs/code-gen/awsCfOnlyCodeGen.ts", | ||
| }, | ||
| awsCfOnlyLint: { | ||
| id: "dba872348ce39bfa8c15002037263b2528d81833c6c1f24fef5c9bc4115145b4", | ||
| backendKind: "jsAttribute", | ||
| responseType: "qualification", | ||
| displayName: "CloudFormation Lint Validation", | ||
| path: "./src/pipelines/aws/funcs/qualifications/awsCfOnlyLint.ts", | ||
| }, | ||
| } as const satisfies Record<string, FuncSpecInfo>; | ||
|
|
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's weird to me that these are here but the attribute func is in the other file. They should all be in the same place, here or there.
| /** | ||
| * Creates a JSON prop with CodeEditor widget | ||
| */ | ||
| function createJsonProp( |
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.
Would this be better alongside the other generic "create this kind of prop" methods we have?
| // Add CloudFormationOnly prop to extra (extra is always created by restructureDomainProps) | ||
| const extraProp = findPropByName( | ||
| variant.domain, | ||
| "extra", | ||
| ) as ExpandedPropSpecFor["object"]; | ||
| if (extraProp) { | ||
| const cfOnlyProp = createScalarProp( | ||
| "CloudFormationOnly", | ||
| "boolean", | ||
| extraProp.metadata.propPath, | ||
| false, | ||
| ); | ||
| cfOnlyProp.data.defaultValue = "true"; | ||
| cfOnlyProp.data.hidden = true; | ||
| extraProp.entries.push(cfOnlyProp); | ||
| } | ||
| const cfOnlyProp = createScalarProp( | ||
| "CloudFormationOnly", | ||
| "boolean", | ||
| extraProp.metadata.propPath, | ||
| false, | ||
| ); | ||
| cfOnlyProp.data.defaultValue = "true"; | ||
| cfOnlyProp.data.hidden = true; | ||
| extraProp.entries.push(cfOnlyProp); |
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.
Maybe just do this inside of restructure since you already create it there. I think we have a getOrCreate() method somewhere. We should, at least
This PR restructures how CloudFormation-only AWS assets (those without a Cloud Control read handler) are generated in Clover. Instead of directly exposing CloudFormation properties at the domain root, they are now nested under a properties object, with a computed CloudFormationResourceBody output that generates the full CF resource structure.
Changes
New Files:
Modified Files:
Route53::RecordSet Overrides (updated for new domain structure):
Domain Structure (Before → After)
Before:
After:
Impact
Affects ~200 CloudFormation-only AWS assets. Route53::RecordSet has custom overrides that were updated to work with the new structure.
How was it tested?
Does it require a docs change?
In short: 🔗