Generating random variable names for intrinsic functions breaks Serverless caching

[FilipPyrek]

This is a Bug Report

Description

For bug reports:

• What went wrong?

I redeployed a service with no changes, and expected Serverless to skip the deployment - Service files not changed. Skipping deployment... - but it didn't - it deployed it as a new version.

So I started looking into it and I discovered that only services which are using serverless-step-functions plugin have this issue, others are skipping the deployment properly.

So I looked at a diff of cloud formation files generated by Serverless:

"DefinitionString": {
  "Fn::Sub": [
+    "{\n  \"StartAt\": \"firstStep\",\n  \"States\": {\n    \"firstStep\": {\n      \"Type\": \"Task\",\n      \"Resource\": \"${uq9Cg4DzU0}\",\n      \"Next\": \"secondStep\"\n    },\n    \"secondStep\": {\n      \"Type\": \"Task\",\n      \"Resource\": \"${KLDanpHdfJ}\",\n      \"End\": true\n    }\n  }\n}",
-    "{\n  \"StartAt\": \"firstStep\",\n  \"States\": {\n    \"firstStep\": {\n      \"Type\": \"Task\",\n      \"Resource\": \"${rtMpxQlwFk}\",\n      \"Next\": \"secondStep\"\n    },\n    \"secondStep\": {\n      \"Type\": \"Task\",\n      \"Resource\": \"${stQOtQttz5}\",\n      \"End\": true\n    }\n  }\n}",
    {
+     "uq9Cg4DzU0": {
-     "rtMpxQlwFk": {
        "Fn::GetAtt": [
          "FirstLambdaFucntion",
          "Arn"
        ]
      },
+     "KLDanpHdfJ": {
-     "stQOtQttz5": {
        "Fn::GetAtt": [
          "SecondLambdaFunction",
          "Arn"
        ]
      }
    }
  ]
}

Service is redeployed when source code or Cloud Formation code differs from the version deployed on AWS: lib/plugins/aws/deploy/lib/checkForChanges.js, lib/plugins/aws/lib/normalizeFiles.js

And then I looked into source code of serverless-step-functions and I discovered this:
lib/deploy/stepFunctions/compileStateMachines.js#L47-L49

if (isIntrinsic(element)) {
	const paramName = randomName();
	value[idx] = `\${${paramName}}`;
	yield [paramName, element];
} else {
	const innerFuncs = Array.from(getIntrinsicFunctions(element));
	for (const x of innerFuncs) {
		yield x;
	}
}

This shows that every time service is packaged, new - unique - version of CloudFormation is generated because of usage of randomName function. And this prevents Serverless from skipping the deployment.

---

* What did you expect should have happened?

When redeploying same code with no changes, Serverless should skip the deployment saying Service files not changed. Skipping deployment....

Additional Data

• Serverless Framework Core Version you're using: 1.61.0
• The Plugin Version you're using: 2.17.0
• Operating System: macOS Catalina for local development, circleci/node:10.16.0 docker image on CI

[FilipPyrek]

I think that this could be solved by replacing randomName function by some hash function...

I can prepare a PR if you guys want. 🙂

[theburningmonk]

@FilipPyrek that's a good idea! do you mind putting in a PR for this? as a test I'd like to see if it handles the same function being called in multiple states correctly, should work as it is when we turn the array of vars into obj

[FilipPyrek]

Hi @theburningmonk just a kind reminder - the PR is ready for review 🙂