New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix lambda version generation when only function config changes #4510

Merged
merged 3 commits into from Dec 3, 2017

Conversation

Projects
None yet
3 participants
@HyperBrain
Member

HyperBrain commented Dec 1, 2017

What did you implement:

There is a bug in the lambda version creation. Currently only the ZIP file hash is used to
create the logical id for the lambda version resource.
If you just change the function configuration (e.g. memory size, VPC or an environment
variable), the version resource created in the CF template will have the same logical id as before.

This leads to a bug, because if you now deploy, the function is deployed as $LATEST, but no
new version is created
, because CF will not create one as the logical id of the version resource
is the same as before.

Only if you do source changes, a new version will be created.
This behavior is wrong. Also any change in the function configuration creates a differently
behaving deployment and thus must lead to a new function version created.

The bug gets visible and breaks your deployments, if you rely on the last version instead
of $LATEST (e.g. if you use the alias plugin, tag the latest versions or use the versions to
rollback).

How did you implement it:

I added the function configuration to the hash used for generating the logical id
of the function version resource -> H(zip + config). With that change a new
version is now correctly created as soon as (1) the source (zip) changed and (2)
the function configuration changed.

Additionally I switched to reading the ZIP asynchronously (we should not use synchronous functions at all) in chunks to comply with Node so that the Node event loop is not unnecessarily stalled during this lengthy operation. This also lowers the memory footprint to a minimum and is much faster - to generate the hashes it is unnecessary to read the whole file at once!

Example:

A deployment creates this version:

    "ApiLambdaVersionU7fLjAh4hB6aBdBfab0OMjP36BIJFx0JvnYXBQ6H3w": {
      "Type": "AWS::Lambda::Version",
      "DeletionPolicy": "Retain",
      "Properties": {
        "FunctionName": {
          "Ref": "ApiLambdaFunction"
        },
        "CodeSha256": "Zqaq1qsB5+UrPXUlIKBgIlQJsKD0t9vRWDj8NkYqQGw=",
        "Description": "General API"
      }
    },

After change of an environment variable in the function configuration. Notice, that
the ZIP file hash did not change:

    "ApiLambdaVersionGgi0NalwGjBwqrM3eh5DjS1hsCW64EG7ePcOEGYaRCs": {
      "Type": "AWS::Lambda::Version",
      "DeletionPolicy": "Retain",
      "Properties": {
        "FunctionName": {
          "Ref": "ApiLambdaFunction"
        },
        "CodeSha256": "Zqaq1qsB5+UrPXUlIKBgIlQJsKD0t9vRWDj8NkYqQGw=",
        "Description": "General API"
      }
    },

As you see, although the code hash (zip file) stays the same, the version
resource id changed, what means, that a new version will be created now. This
is fully correct, because the last version that would have been in there before
the fix would have been substantially different than, the real last deployed
function. If you'd have tried to restore that one you're system would have been
broken badly.

Now the last Lambda version will resemble the last changed version. If you deploy
without any changes, of course no new versions will be created. That's exactly the
expected behavior.

How can we verify it:

Deploy any service. Open one of the service's functions in the AWS Lambda console and check the generated version (it should be of the same configuration as the $LATEST version).

Now change something in the configuration of the function, but leave its sources untouched.
Redeploy -> CF should now create a new version including the changed configuration. The version
should be shown in the AWS console.

I tested it with multiple of our complex projects and the version creation worked as expected.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@HyperBrain HyperBrain requested a review from pmuens Dec 1, 2017


// Read the file in chunks and add them to the hash (saves memory and performance)
return BbPromise.fromCallback(cb => {
fs.readFile(artifactFilePath, cb);

This comment has been minimized.

@HyperBrain

HyperBrain Dec 1, 2017

Member

To myself: Switch to chunked reading as commented.

@HyperBrain HyperBrain requested review from RafalWilinski and horike37 Dec 1, 2017

@HyperBrain HyperBrain self-assigned this Dec 1, 2017

HyperBrain added some commits Dec 1, 2017

@HyperBrain HyperBrain added this to the 1.25 milestone Dec 2, 2017

@horike37

That's a fantastic work 🎉 Thanks @HyperBrain
I tested locally and see creating a new version even if an only configuration of Lambda is changed.

LGTM 💯

@RafalWilinski

Works like a charm! 🎉 LGTM

@HyperBrain

This comment has been minimized.

Member

HyperBrain commented Dec 3, 2017

Thanks for double checking and reviewing. I'll merge it now.

@HyperBrain HyperBrain merged commit 4863835 into master Dec 3, 2017

5 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 87.866%
Details

@HyperBrain HyperBrain deleted the fix-version-logical-id branch Dec 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment