Lambda S3 object version defaults to $LATEST if unspecified

[Bowbaq]

Turns out S3ObjectVersion is not required by the S3 API. Since there's no way to get an object's version in terraform (afaik), it's useful to be able to leave it out.

[radeksimko]

When you don't specify the version in the API, it is set to $LATEST by default, right?
So I assume this is just to avoid the following as it's default behaviour?

resource "aws_lambda_function" "test" {
    ...
    s3_object_version = "$LATEST"
    ...
}

It would be nice to have an acceptance test confirming this behaviour.

Otherwise I'm happy to merge this, but I'd hold off until we merge #3825

[Bowbaq]

If I recall, you can only explicitly set a 'proper' object version, and $LATEST isn't valid. I'll see if I can write a test tomorrow (haven't gotten round to looking at the test infrastructure yet)

[radeksimko]

Understood, I wonder what comes back from the GetFunction API endpoint after not specifying the version in CreateFunction - $LATEST or empty string?

If user decides to manage versions outside of terraform, yet still reference/output/display current version we also may want to make s3_object_version Computed and read it back from the API.

Ping me if you need any help with writing the test.

[Bowbaq]

@radeksimko pretty busy these days, it's probably going to take a couple more days before I can get around to finishing this up

[radeksimko]

No problem, I'm also busy, currently Fosdem + CfgMgmt Camp, I'll probably get back to Lambda PRs by the end of this week.

[Bowbaq]

@radeksimko I took a stab at it, lmk

[radeksimko]

It would be nice to separate this function out into more generic ones instead of hardcoding values, e.g.

• testAccCheckAWSLambdaFunctionName(expectedName string, function *lambda.GetFunctionOutput)
• testAccCheckAWSLambdaFunctionVersion(expectedVersion string, function *lambda.GetFunctionOutput)

[radeksimko]

Except the nitpick in the acceptance test case this is looking good. 👍

I spent some time reading the Lambda docs, specifically the part about versioning and I'd like to note though that s3_object_version and that version which has $LATEST are two different things. There doesn't seem to be an easy way to get s3_object_version back from the API. Version attribute can either be an alias or a version which has been published when uploading a new zip file or bumping the s3_object_version.

i.e. That acceptance test doesn't do any harm, it tests the schema at least, but checking $LATEST in the context of omitting s3_object_version is irrelevant since you can get Version=$LATEST even if the user submits s3_object_version=specificVersionID.

We don't expose Version as version nor we publish versions (yet) and updating code in place works without it, so it's not really an issue, but I wanted to clarify this.

[Bowbaq]

@radeksimko refactored as per your comments. I don't have too much more time to put in this, so feel free to fork & make any necessary edits

[radeksimko]

Thanks for the modifications.

I don't have too much more time to put in this, so feel free to fork & make any necessary edits

No problem, I think this now looks ok on the first sight. I will do a final review by the end of this week, after #3825 is merged.

[leetrout]

@radeksimko is this blocked by #3825?

[radeksimko]

@leetrout I reran Travis tests and that brought up some failures, but I'll fix this and merge in separately, before #3825 , or #5239 respectively is merged. This PR looks like smaller thing that can sneak in and I can resolve any conflicts with #5239 afterwards.

[radeksimko]

Merged via #5370

[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.