WIP: fix: support intrinsics in Location property of Serverless Application #710
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #710 +/- ##
===========================================
+ Coverage 94.17% 94.25% +0.07%
===========================================
Files 67 67
Lines 2678 2680 +2
Branches 478 478
===========================================
+ Hits 2522 2526 +4
Misses 80 80
+ Partials 76 74 -2
Continue to review full report at Codecov.
|
keetonian
changed the title
| and self.APPLICATION_ID_KEY in app.properties[self.LOCATION_KEY] | ||
| and self.SEMANTIC_VERSION_KEY in app.properties[self.LOCATION_KEY]) | ||
| and isinstance(location, dict) | ||
| and list(location.keys())[0] not in resolver.DEFAULT_SUPPORTED_INTRINSICS |
There was a problem hiding this comment.
why it only checks for the first key?
There was a problem hiding this comment.
If it's an S3 location, it will only have one dictionary key at this level:
Location:
Ref: parameter
# or
Location:
Sub: s3://{something}
If it has something else, then it's probably not valid.
This brings up a good point though: this doesn't check for all possible intrinsics. I need to fix that.
| @@ -187,7 +190,8 @@ def on_before_transform_resource(self, logical_id, resource_type, resource_prope | |||
| self._check_for_dictionary_key(logical_id, resource_properties, [self.LOCATION_KEY]) | |||
|
|
|||
| # If location isn't a dictionary, don't modify the resource. | |||
| if not isinstance(resource_properties[self.LOCATION_KEY], dict): | |||
| if (not isinstance(resource_properties[self.LOCATION_KEY], dict) | |||
There was a problem hiding this comment.
Can you make it a util function to check these conditions? You have the same check in the function above.
keetonian
changed the title
|
Why was this closed? Don't we still need this functionality? I'm unable to use intrinsic functions in Location today in version 0.11.0 |
|
Still running into this issue as well while using an intrinsic function (Fn::Join and I've tried Fn::Sub) with the location property in Serverless::Application ""StatusReason": "Transform AWS::Serverless-2016-10-31 failed with: Invalid Serverless Application Specification document. Number of errors found: 11. Resource with id [someStack] is invalid. Resource is missing the required [ApplicationId] property" |
Issue #, if available:
#694
Description of changes:
Support intrinsics such as Ref and Sub in the Location parameter for s3 strings.
This does not add support for intrinsics in ApplicationId or SemanticVersion.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.