fix(AWS API Gateway): Support Fn::Split for vpcEndpointIds schema
#9455
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9455 +/- ##
==========================================
+ Coverage 86.89% 86.91% +0.02%
==========================================
Files 321 321
Lines 11980 11987 +7
==========================================
+ Hits 10410 10419 +9
+ Misses 1570 1568 -2
Continue to review full report at Codecov.
|
lib/plugins/aws/provider.js
Outdated
| type: 'array', | ||
| items: { $ref: '#/definitions/awsCfInstruction' }, | ||
| }, | ||
| { $ref: '#/definitions/awsCfSplit' }, |
There was a problem hiding this comment.
Why not include awsCfSplit as part of AWS CF instructions set?
There was a problem hiding this comment.
I was thinking about it, but to be honest, awsCfInstruction is a little bit different as in most cases the supported functions resolve to a single identifier, where the Fn::Split produces an array/list of identifiers/values. For example, in this case we don't want to accept Fn::Split as an item in array because that would result in invalid value for the VpcEndpointIds property (item in an array would become a nested array after Fn::Split)
There was a problem hiding this comment.
I see, yes it makes sense then to not include it.
But now I wonder whether it'll not be wise to introduce something as awsCfArrayInstruction that will be an alias to above.
As that construct feels pretty generic and having that, we may agree to use it for every array property where we allow CF intrinsic functions.
What do you think?
There was a problem hiding this comment.
On a second though, the current awsCfInstruction might be a bit misleading as it doesn't include Fn::Split, which is also a cfInstruction, but it's often not supported in the same places as the current definitions in cfInstruction - maybe we could rename the awsCfInstruction to awsCfInstructionSingleResult to be more explicit?
There was a problem hiding this comment.
Sorry, I was writing the above comment without seeing your response - I like the proposed idea 👍 I think it will be useful as I think we might have a few other places where we don't support Fn::Split by omission in schema
pgrzesik
force-pushed
the
fix-vpc-endpoints-ids-split-syntax
branch
from
81f0aa1
to
ab04b36
Compare
Closes: #9453