Skip to content
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

Feature request / bug: Improve checks for file updates #4897

Closed
night0wl opened this issue Jan 29, 2016 · 11 comments
Closed

Feature request / bug: Improve checks for file updates #4897

night0wl opened this issue Jan 29, 2016 · 11 comments

Comments

@night0wl
Copy link

It would be helpful if when given a file as a resource (such as in aws_lambda_function) that it actually checks the contents of the file. Currently, the resource update is only triggered if the location of file changes.

For example I have:

resource "aws_lambda_function" "my_lambda" {
  filename = "${path.module}/provisioning/files/my_lambda_src.zip"
  function_name = "my_lambda"
  role = "${aws_iam_role.iam_my_lambda.arn}"
  handler = "lambda_function.lambda_handler"
  runtime = "python2.7"
  timeout = 10
}

This runs fine, and works as expected, however, there are two issues I have with the current implementation.

  1. When I update the contents of the .zip file, terraform does not pick this up, and I have to taint the resource
  2. The path that is interpolated from $path.module is the full path on my local system. This means that if another member of my team runs terraform, it sees a different path, causing the resource to be updated unnecessarily.

It would be useful to be able to (either by default or via some option) to determine if the payload is actually different (e.g. checksum), instead of using the path.

@jonapich
Copy link

Noted in the referenced issue, but I want to make sure this isn't missed: updating the lambda's code does not require a destroy of the resource. AWS creates additional aliases in this case. However, I personally find it annoying in situations where you're not interested in keeping old versions - it would be nice to be able to control the behavior from terraform (i.e.: provide a "create_alias" bool kind of option).

@radeksimko
Copy link
Member

Hi,
updates for Lambda function have been implemented as part of #5239 and will be part of the next release.

It will be possible to trigger an update of a Lambda function by specifying an optional attribute source_code_hash, e.g.

resource "aws_lambda_function" "test_lambda" {
  filename = "lambda_function_payload.zip"
  function_name = "lambda_function_name"
  role = "${aws_iam_role.iam_for_lambda.arn}"
  handler = "exports.test"
  source_code_hash = "${base64sha256(file("lambda_function_payload.zip"))}"
}

The update mechanism is also documented and docs update will be released along with the next version which makes it possible to update Lambda function.

@muikrad I'm not entirely sure you can keep old versions of Lambda functions, although I know there's the concept of aliases that can be effectively used for versioning. In regards to keeping old versions of code I'd suggest deploying from versioned S3 bucket. If you have more specific suggestions on how Terraform could work with aliases for versioning feel free to describe that in a separate issue.

@jonapich
Copy link

@radeksimko to update a lambda function and use aliases, you must use the "update code" API call instead of deleting the lambda and recreating it again. There is no reason to delete a lambda version - this might actually create an unnecessary downtime! The AWS API has all these features, I don't understand why Terraform would do it another way if it can avoid deleting it.

@radeksimko
Copy link
Member

@muikrad if you have had a look at the linked PR, you would see that it is doing exactly what you described - using UpdateFunctionCode API call to update the function in-place. 😉 It is not destroying/recreating it anymore.

However my note was regarding Lambda aliases which can be used as an extra versioning layer within Lambda. If you have suggestions for solutions regarding that, please open a new issue with those suggestions.

Thanks.

@jonapich
Copy link

Oh, awesome, then you are already creating versions. Sorry for not checking the PR. I guess the resource needs to output the version number of the updated code (probably from the update code response) into an output attribute. That way one could use it as an interpolation in other resources. I don't personally have a use case for this, but I guess having it handy would at least be a good start.

@radeksimko
Copy link
Member

@muikrad not exactly. The updates (as implemented in the linked PR) are currently performed in a way that a Lambda function is updated in-place (that's all the UpdateFunctionCode API call does).

needs to output the version number of the updated code (probably from the update code response) into an output attribute.

We do not expose the Lambda version (yet), but it's basically always $LATEST anyway.
I assume we could add an extra attribute publish_new_versions = true/false that would be calling lambda.PublishVersion API call. If someone has a use case for this (I personally don't atm), they can create a separate issue.

@jonapich
Copy link

I'm sorry @radeksimko, but you are wrong. The version returned by the UpdateFunctionCode is never $LATEST as you seem to imply, and that API call does not replace the code in-line (I was also mislead by this at first). Or maybe it does when it's a simple text file; I don't use this scenario so I wouldn't know. The explications below assume that you are uploading a zip file to AWS Lambda, which becomes the only possible use case if you're packaging 3rd party libraries with your code.

$LATEST is only a builtin alias to simplify accessing the most recent version; it is not a version per se and should only be used by calling entities if they don't care about which version they're calling. You can indeed use the PublishVersion API to create a new version from $LATEST, but that's another issue, as you point out.

So here's the catch: when you use the UpdateFunctionCode API call, AWS actually publishes a new version of the code. I learned it the hard way when I ran into storage issue because I did not clean up my old versions. The response always contains a Version key that contains the real version number of the newly updated code. Also important to note, the returned ARN actually includes the version number, which is never $LATEST (even though at this point, they both point to the same version). If Terraform's arn output attribute uses the arn from the response, it could cause unnecessary cascading updates if the customer is not interested in targetting specific versions, because Terraform will notice the change and will want to update all dependant entities.

Here's a sample response for the UpdateFunctionCode API call that should clarify this even more:

{
    "CodeSha256": "640iOU1Jmz6ExMHTIsQzszkd/Yi5wuMxBC8Tu6igRCU=", 
    "FunctionName": "ndev-testing", 
    "ResponseMetadata": {
        "HTTPStatusCode": 200, 
        "RequestId": "f2d7167a-e9db-11e5-97af-c198fa9341ba"
    }, 
    "VpcConfig": {
        "SubnetIds": ["subnet-ffda3eaa", "subnet-264f7d0d", "subnet-40cb95b7"], 
        "VpcId": "vpc-3a729cfb", 
        "SecurityGroupIds": ["sg-fa68cc81"]
    }, 
    "MemorySize": 128, 
    "FunctionArn": "arn:aws:lambda:us-east-1:******:function:ndev-testing:15", 
    "Version": "15",
    "Role": "arn:aws:iam::*******:role/nifd-role-lambda", 
    "Timeout": 120, 
    "LastModified": "2016-03-14T11:57:32.121+0000", 
    "Handler": "testlambda.main_handler", 
    "Runtime": "python2.7", 
    "CodeSize": 8022181, 
    "Description": "testing"
}

As you see, the FunctionArn contains the version (:15) as well as the Version field and doesn't not make any mention of $LATEST. To allow full flexibility over this, Terraform really should offer 2 distinct arn properties; the one from the response and an extra one that removes the version so that dependant entities can be configured to always use $LATEST. If the Terraform docs are clear on this, Terraform users will be able to make an educated decision over the version features and prevent cascading updates of resources if they are not interested in the versionning features.

@radeksimko
Copy link
Member

@muikrad I'm not sure how you created/updated that function ^ but I'm pretty sure it wasn't via the latest version of Terraform in master.

Admittedly I wasn't entirely right (u could say I was wrong 😉 ) about UpdateFunctionCode not publishing versions. I will correct myself and say "It does not publish versions by default". There is an attribute Publish (bool) which is essentially false by default (if it's missing in the request). So there is no need to call PublishVersion (contrary to what I said before).

aws --region=us-west-2 lambda get-function --function-name=lambda_function_name
{
    "Code": {
        "RepositoryType": "S3",
        "Location": "https://***-us-west-2-tasks.s3-us-west-2.amazonaws.com/snapshots/*REDACTED*"
    },
    "Configuration": {
        "Version": "$LATEST",
        "CodeSha256": "60hXsKdYkfeql*REDACTED*mZDbGIrPnUHc=",
        "FunctionName": "lambda_function_name",
        "MemorySize": 128,
        "CodeSize": 339,
        "FunctionArn": "arn:aws:lambda:us-west-2:***:function:lambda_function_name",
        "Handler": "exports.test",
        "Role": "arn:aws:iam::***:role/iam_for_lambda",
        "Timeout": 3,
        "LastModified": "2016-03-14T13:42:07.081+0000",
        "Runtime": "nodejs",
        "Description": ""
    }
}

when I manually add Publish = aws.Bool(true) into the code & recompile Terraform, I get pretty much the same output as you do.

@jonapich
Copy link

Oh my, I apologize; I completely overlooked the Publish parameter. It was indeed set to True, which explains the results I'm having. I feel embarrassed now. Sorry to have taken so much of your time.

@radeksimko
Copy link
Member

I feel embarrassed now. Sorry to have taken so much of your time.

No need to be embarrassed or apologise! I think it was worth discussing, so that anyone else searching for the same feature will find a thorough explanation here. 😉

@ghost
Copy link

ghost commented Apr 27, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants