Adding support for Lambda function updates

[robzienert]

Current AWS Lambda resource does not support updates.

[joemiller]

I tried this code but I'm not seeing what I expected. I have a tf module with lambdas defined using local zips as the code source. I make a change to the zip and run apply but the lambda resources were not updated.

I have an existing tfstate file, could that be the issue? I have not tried a fresh project to see if the source_code_hash is added to the lambdas in the state file.

[robzienert]

Thanks for the update. I've been pulled onto something else at work temporarily, but I'll loop back onto this in a week or so.

[robzienert]

Updated the PR to fix a few bugs.

[vancluever]

Wish I saw this PR before I started work on it myself!! ;)

Going to pull this down and see if it does want I want it to do.

[vancluever]

Hey Rob,

This code did not update the Lambda function when all that changed was the zip contents (so all it would have would be a local sum). I'm not too sure if this is enough to trigger an update, which is the challenge for me, and the wall I hit before trying your code.

Maybe a sum can be implemented for both local and remote ZIP code to ensure that resourceAwsLambdaFunctionRead can perform proper updates on both the local and live sum? I'm thinking that might be enough to trigger the update and still have consistent data on the state of the function.

[vancluever]

Everyone, I did this up, it looks like what would be necessary to get the function to update without tainting the resource. I understand that this could easily be broken by the user (depending on a modifiable attribute's default), unfortunately, it doesn't look like state change on computed resources alone is enough.

paybyphone@a014b6d

Note that updating a function's code is a non-destructive change, you should be able to do it without forcing a new resource (and further, this would be feature parity with CloudFormation). With that said, state change on the source sum resources didn't seem to trigger a new resource either, from what I saw.

I could not see in the code and haven't seen any examples so far of being able to force an update, versus taint - does such functionality exist?

[robzienert]

@vancluever Thanks for the addition, I pulled it into this PR.

On the forcing updates front, I'm not aware of anything that's available on the resource level for this sort of thing. Modifying state is getting a little into the weeds for me, I'm not entirely sure how that would be implemented.

[vancluever]

Wow thanks @robzienert! Yeah I think it needs some tweaking still. I want to send this to the mailing list to see if we can't get it some more attention and discussion on what to do. I don't like the idea of update_code just being a dummy option. Hopefully I get a chance to do that tonight.

[vancluever]

Posted https://groups.google.com/forum/#!topic/terraform-tool/cyOpssFL7Sc. Maybe someone not actively looking at this PR will see that and help!

[robzienert]

Appreciate you helping usher this work along!

[alexander-bobin]

Any movement on this? It'd be great to have this available

[vancluever]

Hey Alexander, unfortunately not :( no activity on the mailing list post either, maybe now that the holidays are done there will be some more movement soon.

[catsby]

Hey all –

I'm trying to get an understanding of what's not working here.

Given a minimal config like so:

resource "aws_lambda_function" "lambda_function_test_update" {
  filename      = "test-fixtures/lambdatest.zip"
  function_name = "example_lambda_name_update"
  role          = "${aws_iam_role.iam_for_lambda.arn}"
  handler       = "exports.example"
}

The problem then is if edit the contents, Terraform doesn't recognize new code, yeah?

One way we could do this is by exposing the hash, and having users specify the sha256 hash of their zip file. This is extra work for the user, they'd have to recalculate it. It would be great if we could have a config method that calculates the sha256 here for us (similar to sha1 in https://terraform.io/docs/configuration/interpolation.html).

Alternatively, we could add a file_folder or some attribute to point to a folder on disk, and do the zipping ourselves, and store the hash of the folder in the config, not the filename. I think this would need to be done with a StateFunc, though, it may actually be possible to just do this with filename itself, now that I think about it. Changing it in-place would require a migration for existing lambda users, but if someone wants to explore that route and see if it will work at all, I can then help you with the migration.

[mostman1043]

I've been zipping it myself - then adding an md5 sum on the zip filename, then updating the terraform config with the new filename. This lets me (outside of terraform) detect if the code has been changed. For now, I am calling update on my own. I would say providing zip functionality would be difficult, and probably out of scope. My repo, for example, has a number of lambda functions in it, and there is a bunch of dancing that needs to be done to get things ready for zipping. Terraform detects that the filename in config is different than the state, which is almost there. I just need it to call out to updateFunction.

[vancluever]

@mostman79 yeah, changing the filename as a workaround works, but should not be necessary, and in fact if you use the branch in this PR with my changes (@robzienert has rolled them in) it works by simply updating the ZIP. So we are almost there, just we need to figure out how to do it without having to add an unnecessary attribute as a workaround.

@catsby - Confirmed - updating the code ZIP contents does not trigger an update. I think I tried to use a StateFunc to pre-calculate the source_code_hash value, the only problem being that fully computed values don't seem to trigger updates, from what I recall. Hence the workaround in the above commit where I have added the update_code attribute with a default of false, and change it to true when there is a SHA256 mismatch.

Do you know if using StateFunc as mentioned should work? Maybe I was missing something.

[apparentlymart]

The way I've seen this handled elsewhere in Terraform is to use StateFunc on the attribute so that what's stored in the state is a hash of the contents rather than the filename.

To make this completely robust it would be necessary for the Read function on the resource to update the data in case someone uploaded a new file through some non-Terraform means. That's tricky though, because I don't think the ResourceData API provides a way to update the hash, but rather requires the resource implementation to update the value which will then be passed in to StateFunc. A new method could potentially be added to ResourceData to allow the resource implementation to get the hash directly from the API (assuming that's available) and write that directly instead.

But with the facilities we have today, you could detect when a change has been made within Terraform by making a StateFunc that reads the given file and hashes it. Then if the file changes Terraform will consider that a diff, even if the filename is still the same. Read wouldn't touch this attribute at all, so changes outside of Terraform would not be noticed and a manual taint would be required in that case.

The other gotcha with this design is that StateFunc isn't able to return an error, so if an invalid file is given it'd be necessary to return a sentinel value to represent that case.

[vancluever]

Hey @apparentlymart - yeah that's what I basically tried:

• A sha256sum StateFunc hook was pre-calculating the hash for source_code_hash
• I was reading AWS's hash with the Read call on the resource, and setting source_code_hash accordingly, but:
• Even though the two were different, an update wasn't forced.

If this sounds like it should have worked, I can give it another go.

[apparentlymart]

@vancluever I don't think it works quite like you tried because the when you set source_code_has inside the Read function Terraform is expecting that you will write the whole file payload, and then it will call the StateFunc with it to produce the hash. So I think you were creating a hash of the hash.

But I'd expect that to always show as a diff, and you said it never shows as a diff, so apparently Terraform is doing something different than what I expected.

[vancluever]

Thanks @apparentlymart - will do another sanity check in the next day or two here to make sure it was doing what I thought it was doing and apply your suggestions! If StateFunc fires every time the value is changed then there definitely could be an issue there.

Will update when I know more!

[radeksimko]

I spent some time today trying to get to the bottom of this.

There's a couple of things to solve/fix in the PR, until we can merge it.

Sha256 calculation & representation/comparison

The CodeSha256 from the AWS comes a base64-encoded sha256 sum, which we all probably already know. However the difference (and root cause of all troubles) is the representation of the sum.
Sha256 is typically represented in hexadecimal format (base 16), however the base64-encoded version of it comes from Amazon as base64-encoded raw sha256 value (raw bytes). So either we need to be comparing hexadecimal with hexadecimal representation or bytes with bytes, but not bytes with hex - that would never be equal.

See an example in Go playground to get better understanding of the problem: https://play.golang.org/p/1LCqlwnFM2

How to move forward with this PR?

As calculating the sha & base64 and doing the comparison internally inside the resource is a bit tricky, I'd rather suggest we just leave this up to the user. i.e.

resource "aws_lambda_function" "test" {
    filename = "lambdatest.zip"
    function_name = "lambda_function_name"
    role = "${aws_iam_role.iam_for_lambda.arn}"
    handler = "exports.test"
    source_code_hash = "${base64encode(sha256(file("lambdatest.zip")))}"
}

This also gives them the freedom calculating this completely outside of Terraform and providing it as variable.

However, as mentioned above, the newly introduced sha256() returns the SHA256 sum in hexadecimal format, which means we'll need to introduce another function, probably hexDecode() or rawsha256(). The final TF template would then look like this:

resource "aws_lambda_function" "test" {
    filename = "lambdatest.zip"
    function_name = "lambda_function_name"
    role = "${aws_iam_role.iam_for_lambda.arn}"
    handler = "exports.test"
    source_code_hash = "${base64encode(hexdecode(sha256(file("lambdatest.zip"))))}"
}

I'm hoping there are no hidden side-effects of the file() function (e.g. newline representation) or some side effects caused by handing over computed values between functions. If not, this should just work.

Schema changes

With the suggestions above, you could just omit all of the extra logic with update_code & remote_code_hash and just let Terraform do the base64+hex+sha256 calculation on-the-fly via interpolation functions before it even gets to the resource and then let it do the comparison.

I'd suggest to compare/save the base64 encoded string to source_code_hash which will help us to avoid any decode operations on whatever comes from Amazon.

---

I will have a look into what we can do with the interpolation functions in order to make the above happen.

[radeksimko]

@robzienert As base64sha256() is now supported (#4899 was merged), would you mind using that function to finish this PR?

We'll also need to add acceptance tests covering the update functionality, at least for the local ZIP option, but ideally for both local & S3 options.

If you don't have time to finish it or don't want to finish it for any reason that's totally 👌 , let me know, I can pick it up. If I do so, I will keep your git authorship as is and build changes on top of it - i.e. your contribution & effort you spent on this PR so far would be still credited to you! 😉

If I don't hear back until the end of next week, I will pick it up.

I see this as a chance to unblock all Lambda users who're holding off from using Terraform to manage their Lambda resources because of this bug.

[radeksimko]

Fixed via #5239 which was just merged.

Updates will be supported in the next release.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.