Only check for layer changes when actually deploying

[ankon]

Addresses: #8187

---

The discussion in #8187 seems to be stuck, and proposes quite some changes. I think it would be beneficial to unbreak serverless package using this trivial change, and then look into the bigger picture (see also #8499 and #8666, for example.)

The layers part is clearly depending on the aws provider, and so (IMHO) should be free to hook into the aws:deploy:checkForChanges.

Practical reasons for this change for us: We're using package to produce the cloudformation template (for various combinations of parameters), which we then run through cfn-lint as a first step in sanity-checking before deploying.

[codecov]

Codecov Report

Merging #10170 (44be449) into master (39bdea0) will decrease coverage by 0.07%.
The diff coverage is 25.00%.

@@            Coverage Diff             @@
##           master   #10170      +/-   ##
==========================================
- Coverage   85.35%   85.28%   -0.08%     
==========================================
  Files         337      338       +1     
  Lines       13725    13815      +90     
==========================================
+ Hits        11715    11782      +67     
- Misses       2010     2033      +23     

Impacted Files	Coverage Δ
lib/plugins/aws/package/compile/layers.js	72.16% <25.00%> (-12.05%)	⬇️
bin/serverless.js	39.13% <0.00%> (-10.87%)	⬇️
lib/cli/triage.js	90.80% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39bdea0...44be449. Read the comment docs.

[pgrzesik]

Thanks @ankon for your proposition. I've talked with @medikoo to learn more about the motivation behind the previous change and unfortunatelly your proposal will result in un-doing the optimization to how layers are handled. During our packaging step, we're producing the final CloudFormation template and it's affected by the logic in compareWithLastLayer - it ensures to reuse already uploaded artifacts to S3. If we move that logic to a later point, it will be no longer effective. Please also see the last comment from @medikoo in this topic: #8187 (comment)

Given the above, would you be okay with closing this PR?

[ankon]

Given the above, would you be okay with closing this PR?

I guess, it's your product after all :)

With that said: What we want to achieve is that developers and do an npm test step locally, without having access to any particular AWS environment, and the test step verifies that the changes in serverless.yml and related resources (such as plugins, configuration, ...) are not "obviously breaking". Doing that by means of cfn-lint seems IMHO quite reasonable.
I guess the position of serverless is that this is a concern that will be addressed by #8499 "somehow", but possibly in a way that would not allow us to use cfn-lint but rather "something else".

For now we'll simply maintain that trivial patch in our fork, and I'll subscribe to #8499 to see if/how we can adapt eventually.

[pgrzesik]

Given the above, would you be okay with closing this PR?

I guess, it's your product after all :)

Sorry if it came out rude - I didn't mean it this way. The thing is, the solution most likely will have to be way different where we have the ability to separate build/package steps and potentially offer an "offline" approach for generating the CloudFormation template. The big question is though - how useful that is going to be, because in some cases it's just impossible to create the correct CF template in an offline manner (e.g. when using AWS-based variable sources or even in this case), so validating against such template might not be the best choice.

I understand what you're trying to achieve in your specific use case, but I don't see at this moment a simple and quick "win" that would allow to do so, as the proposed change breaks the layer optimization.