Skip to content

fix: sam build fails when layer logicalID shared across stacks#2760

Merged
aahung merged 4 commits intoaws:developfrom
aahung:delay-layer-validation
Mar 24, 2021
Merged

fix: sam build fails when layer logicalID shared across stacks#2760
aahung merged 4 commits intoaws:developfrom
aahung:delay-layer-validation

Conversation

@aahung
Copy link
Contributor

@aahung aahung commented Mar 23, 2021

Which issue(s) does this change fix?

fix #2724

Why is this change necessary?

sam-cli does not support resolving intrinsic functions in parameters/outputs, which results in incorrect layer reference when the layer is passed from some other stacks using intrinsic functions.

Before we have a great support of intrinsic functions, customers should still be able to build their stacks. Incorrect resolution of layers should only affect local *.

How does it address the issue?

Delay the validation process and only raise exception when certain layer properties are used.
I added two integration tests to cover both using parameters (pass down) and outputs (pass up).

*pass up: template-pass-up.yaml, pass down: template-pass-down.yaml

  • pass up before:
    • sam build works,
    • sam local invoke works
  • pass up now:
    • sam build works,
    • sam local invoke: Error: {'Fn::GetAtt': ['ChildApp', 'Outputs.LayerVersion']} is an Unsupported Intrinsic works
  • pass down before:
    • sam build failed with invalid layer arn error
    • sam local invoke failed with invalid layer arn error
  • pass down now:
    • sam build works
    • sam local invoke failed with invalid layer arn error

What side effects does this change have?

for the "pass up" case, customers won't be able to local invoke anymore. This should be the expected behavior.

Checklist

  • Add input/output type hints to new functions/methods
  • Write design document (Do I need to write a design document?)
  • Write unit tests
  • Write/update functional tests
  • Write/update integration tests
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed
  • Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Comment on lines 148 to 149
if not isinstance(arn, str):
raise UnsupportedIntrinsic("{} is an Unsupported Intrinsic".format(arn))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was originally here, just reverting

@aahung
Copy link
Contributor Author

aahung commented Mar 24, 2021

just discussed with @mndeveci, for the change that prevents local invoking a function containing unresolvable layer, the risk is unknown and the benefit does not justify the risk. I just reverted that part and use a LOG.debug instead.

@aahung aahung merged commit 63fcba5 into aws:develop Mar 24, 2021
elbayaaa pushed a commit to elbayaaa/aws-sam-cli that referenced this pull request Mar 25, 2021
)

* fix: sam build fails when layer logicalID shared across stacks

* Add flanky annotation to integ test

* Allow local invoke the pass-up case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Layers shared in multiple stacks cause build/deploy to fail

4 participants