patch lambda importing #3371

VenelinMartinov · 2024-02-01T20:28:08Z

Addresses #2392

Looks like the upstream provider does not support specifying a lambda only by source_code_hash in inputs:
https://github.com/hashicorp/terraform-provider-aws/blob/97c16f28e84ab7afda6b935e19b9243510931c81/internal/service/lambda/function.go#L169-L173
The schema has ExactlyOneOf on the other ways to provide the source code.

This might have something to do with how TF treats imports, since I don't believe they generally generate the code for it.

We can however generate the input for the lambda, so we should support specifying it by the source_code_hash.

I've tested that the issue no longer reproduces under this patch.

TODO:

Issue for patch
test
read more about tf imports

github-actions · 2024-02-01T20:47:03Z

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Maintainer note: consult the runbook for dealing with any breaking changes.

iwahbe

The change looks reasonable. TF does generate a generated.tf file during an import, so I would like to make sure we understand if this bug shows up in TF, or just in Pulumi.

We can merge as soon as we understand that and have a test.

patches/0041-allow-lambdas-with-source_code_hash.patch

t0yv0 · 2024-02-02T18:25:11Z

I'm struggling with understanding this one a bit, is it true that the bug also holds in pure TF, which is why we're patching? Is there an upstream issue open?

patches/0041-allow-lambdas-with-source_code_hash.patch

t0yv0 · 2024-02-02T18:32:31Z

So we document source_code_hash like this:

source_code_hash
str
Used to trigger updates. Must be set to a base64-encoded SHA256 hash of the package file specified with either filename or s3_key.

That sounds exactly right, the upstream provider does not support specifying a lambda only by source_code_hash. Looks like that's not what this field is for. The proposal here is to add a feature that would exist only in the Pulumi provider that changes the behavior of source_code_hash?

t0yv0 · 2024-02-02T18:33:55Z

There's more work to do this with testing and documentation. Before we go down this route, can we quickly look at any other ways to satisfy the original problem? Why does pulumi not import this cleanly as is (presumably pure TF does)? I suspect we have a Read/import gap for archives that's related here?

VenelinMartinov · 2024-02-02T18:39:09Z

@t0yv0 great question, I had a look at TF issues and I found this:
hashicorp/terraform-provider-aws#29043 (comment) - this has a workaround which I will try.

They've closed is as expected behaviour.

Same issue reported here: hashicorp/terraform#34374

I guess one thing to test is invalid source_code_hash - provisioning a new resource with just that should fail.

t0yv0 · 2024-02-02T18:53:38Z

Can you paste what Pulumi Read() call produces during import and consequently what code does import produce? Thanks!

VenelinMartinov · 2024-02-02T19:03:16Z

Read GRPC:

{
    "method": "/pulumirpc.ResourceProvider/Read",
    "request": {
        "id": "testLambda-4b3ee0e",
        "urn": "urn:pulumi:imported::repro_lambda::aws:lambda/function:Function::mylambda",
        "properties": {}
    },
    "response": {
        "id": "testLambda-4b3ee0e",
        "properties": {
            "__meta": "{\"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0\":{\"create\":600000000000,\"delete\":600000000000,\"update\":600000000000}}",
            "architectures": [
                "x86_64"
            ],
            "arn": "arn:aws:lambda:us-west-2:616138583583:function:testLambda-4b3ee0e",
            "codeSigningConfigArn": "",
            "deadLetterConfig": null,
            "description": "",
            "environment": {
                "variables": {
                    "foo": "bar"
                }
            },
            "ephemeralStorage": {
                "size": 512
            },
            "fileSystemConfig": null,
            "handler": "index.test",
            "id": "testLambda-4b3ee0e",
            "imageConfig": null,
            "imageUri": "",
            "invokeArn": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:616138583583:function:testLambda-4b3ee0e/invocations",
            "kmsKeyArn": "",
            "lastModified": "2024-02-02T19:01:15.948+0000",
            "layers": [],
            "loggingConfig": {
                "applicationLogLevel": "",
                "logFormat": "Text",
                "logGroup": "/aws/lambda/testLambda-4b3ee0e",
                "systemLogLevel": ""
            },
            "memorySize": 128,
            "name": "testLambda-4b3ee0e",
            "packageType": "Zip",
            "qualifiedArn": "arn:aws:lambda:us-west-2:616138583583:function:testLambda-4b3ee0e:$LATEST",
            "qualifiedInvokeArn": "arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/arn:aws:lambda:us-west-2:616138583583:function:testLambda-4b3ee0e:$LATEST/invocations",
            "reservedConcurrentExecutions": -1,
            "role": "arn:aws:iam::616138583583:role/iamForLambda-47093df",
            "runtime": "nodejs18.x",
            "signingJobArn": "",
            "signingProfileVersionArn": "",
            "skipDestroy": false,
            "snapStart": null,
            "sourceCodeHash": "WUsPYQdwiMj+sDZzl3tNaSzS42vqVfng2CZtgcy+TRs=",
            "sourceCodeSize": 156,
            "tags": {},
            "tagsAll": {},
            "timeout": 3,
            "tracingConfig": {
                "mode": "PassThrough"
            },
            "version": "$LATEST",
            "vpcConfig": null
        },
        "inputs": {
            "__defaults": [],
            "architectures": [
                "x86_64"
            ],
            "environment": {
                "__defaults": [],
                "variables": {
                    "__defaults": [],
                    "foo": "bar"
                }
            },
            "ephemeralStorage": {
                "__defaults": [],
                "size": 512
            },
            "handler": "index.test",
            "loggingConfig": {
                "__defaults": [],
                "logFormat": "Text",
                "logGroup": "/aws/lambda/testLambda-4b3ee0e"
            },
            "name": "testLambda-4b3ee0e",
            "packageType": "Zip",
            "role": "arn:aws:iam::616138583583:role/iamForLambda-47093df",
            "runtime": "nodejs18.x",
            "sourceCodeHash": "WUsPYQdwiMj+sDZzl3tNaSzS42vqVfng2CZtgcy+TRs=",
            "tracingConfig": {
                "__defaults": [],
                "mode": "PassThrough"
            }
        }
    },
    "metadata": {
        "kind": "resource",
        "mode": "client",
        "name": "aws"
    }
}

Generated program:

import pulumi
import pulumi_aws as aws

mylambda = aws.lambda_.Function("mylambda",
    architectures=["x86_64"],
    environment=aws.lambda_.FunctionEnvironmentArgs(
        variables={
            "foo": "bar",
        },
    ),
    ephemeral_storage=aws.lambda_.FunctionEphemeralStorageArgs(
        size=512,
    ),
    handler="index.test",
    logging_config=aws.lambda_.FunctionLoggingConfigArgs(
        log_format="Text",
        log_group="/aws/lambda/testLambda-4b3ee0e",
    ),
    name="testLambda-4b3ee0e",
    package_type="Zip",
    role="arn:aws:iam::616138583583:role/iamForLambda-47093df",
    runtime="nodejs18.x",
    source_code_hash="WUsPYQdwiMj+sDZzl3tNaSzS42vqVfng2CZtgcy+TRs=",
    tracing_config=aws.lambda_.FunctionTracingConfigArgs(
        mode="PassThrough",
    ),
    opts=pulumi.ResourceOptions(protect=True))

VenelinMartinov · 2024-02-02T19:07:59Z

Here is where the upstream code updating happens: https://github.com/hashicorp/terraform-provider-aws/blob/c42a6bbfd7f170bbb30254045c1657e2e9c973c2/internal/service/lambda/function.go#L935

They check that one of the code related attributes has changed:
https://github.com/hashicorp/terraform-provider-aws/blob/c42a6bbfd7f170bbb30254045c1657e2e9c973c2/internal/service/lambda/function.go#L1284-L1292

and then on
https://github.com/hashicorp/terraform-provider-aws/blob/c42a6bbfd7f170bbb30254045c1657e2e9c973c2/internal/service/lambda/function.go#L946-L971
they assume that it must be one of filename or image_uri or s3_bucket.

So it looks to me like as long as there is no code change, the lambda should be fine to edit other resources.

I think this means that if we are to ship this we need to add a check that source_code_hash has not changed - that wouldn't work. Is there a mechanism to validate a diff? Would the diff customization stuff work as validation?

VenelinMartinov · 2024-02-02T19:26:07Z

I tried the upstream workaround:

import pulumi
import pulumi_aws as aws
import pulumi_archive as archive


mylambda = aws.lambda_.Function("mylambda",
    architectures=["x86_64"],
    environment=aws.lambda_.FunctionEnvironmentArgs(
        variables={
            "foo": "bar",
        },
    ),
    code=pulumi.FileArchive("lambda_function_payload.zip"),
    ephemeral_storage=aws.lambda_.FunctionEphemeralStorageArgs(
        size=512,
    ),
    handler="index.test",
    logging_config=aws.lambda_.FunctionLoggingConfigArgs(
        log_format="Text",
        log_group="/aws/lambda/testLambda-4b3ee0e",
    ),
    name="testLambda-4b3ee0e",
    package_type="Zip",
    role="arn:aws:iam::616138583583:role/iamForLambda-47093df",
    runtime="nodejs18.x",
    source_code_hash="WUsPYQdwiMj+sDZzl3tNaSzS42vqVfng2CZtgcy+TRs=",
    tracing_config=aws.lambda_.FunctionTracingConfigArgs(
        mode="PassThrough",
    ),
    opts=pulumi.ResourceOptions(protect=True, ignore_changes=["filename", "code"]))

Still getting the same error.

Here is the Check call:

{
    "method": "/pulumirpc.ResourceProvider/Check",
    "request": {
        "urn": "urn:pulumi:imported::repro_lambda::aws:lambda/function:Function::mylambda",
        "olds": {
            "__defaults": [],
            "architectures": [
                "x86_64"
            ],
            "environment": {
                "__defaults": [],
                "variables": {
                    "__defaults": [],
                    "foo": "bar"
                }
            },
            "ephemeralStorage": {
                "__defaults": [],
                "size": 512
            },
            "handler": "index.test",
            "loggingConfig": {
                "__defaults": [],
                "logFormat": "Text",
                "logGroup": "/aws/lambda/testLambda-4b3ee0e"
            },
            "name": "testLambda-4b3ee0e",
            "packageType": "Zip",
            "role": "arn:aws:iam::616138583583:role/iamForLambda-47093df",
            "runtime": "nodejs18.x",
            "sourceCodeHash": "WUsPYQdwiMj+sDZzl3tNaSzS42vqVfng2CZtgcy+TRs=",
            "tracingConfig": {
                "__defaults": [],
                "mode": "PassThrough"
            }
        },
        "news": {
            "architectures": [
                "x86_64"
            ],
            "environment": {
                "variables": {
                    "foo": "bar"
                }
            },
            "ephemeralStorage": {
                "size": 512
            },
            "handler": "index.test",
            "loggingConfig": {
                "logFormat": "Text",
                "logGroup": "/aws/lambda/testLambda-4b3ee0e"
            },
            "name": "testLambda-4b3ee0e",
            "packageType": "Zip",
            "role": "arn:aws:iam::616138583583:role/iamForLambda-47093df",
            "runtime": "nodejs18.x",
            "sourceCodeHash": "WUsPYQdwiMj+sDZzl3tNaSzS42vqVfng2CZtgcy+TRs=",
            "tracingConfig": {
                "mode": "PassThrough"
            }
        },
        "randomSeed": "zV98gjQ1aCgjsPjXHJdU0XGgDUDT0FQoNtVUhKYFMmY="
    },
    "response": {
        "inputs": {
            "__defaults": [
                "memorySize",
                "publish",
                "reservedConcurrentExecutions",
                "skipDestroy",
                "timeout"
            ],
            "architectures": [
                "x86_64"
            ],
            "environment": {
                "__defaults": [],
                "variables": {
                    "foo": "bar"
                }
            },
            "ephemeralStorage": {
                "__defaults": [],
                "size": 512
            },
            "handler": "index.test",
            "loggingConfig": {
                "__defaults": [
                    "applicationLogLevel",
                    "systemLogLevel"
                ],
                "applicationLogLevel": "",
                "logFormat": "Text",
                "logGroup": "/aws/lambda/testLambda-4b3ee0e",
                "systemLogLevel": ""
            },
            "memorySize": 128,
            "name": "testLambda-4b3ee0e",
            "packageType": "Zip",
            "publish": false,
            "reservedConcurrentExecutions": -1,
            "role": "arn:aws:iam::616138583583:role/iamForLambda-47093df",
            "runtime": "nodejs18.x",
            "skipDestroy": false,
            "sourceCodeHash": "WUsPYQdwiMj+sDZzl3tNaSzS42vqVfng2CZtgcy+TRs=",
            "timeout": 3,
            "tracingConfig": {
                "__defaults": [],
                "mode": "PassThrough"
            }
        },
        "failures": [
            {
                "reason": "Invalid combination of arguments. \"image_uri\": one of `filename,image_uri,s3_bucket` must be specified. Examine values at 'mylambda.imageUri'."
            },
            {
                "reason": "Invalid combination of arguments. \"filename\": one of `filename,image_uri,s3_bucket` must be specified. Examine values at 'mylambda.code'."
            },
            {
                "reason": "Invalid combination of arguments. \"s3_bucket\": one of `filename,image_uri,s3_bucket` must be specified. Examine values at 'mylambda.s3Bucket'."
            }
        ]
    },
    "metadata": {
        "kind": "resource",
        "mode": "client",
        "name": "aws"
    }
}

t0yv0 · 2024-02-02T19:30:36Z

Reading upstream it seems that recovering the source code is not possible. Is that true?

VenelinMartinov · 2024-02-02T19:32:50Z

Not via the API, reads just return the hash - I think the console sometimes displays the code but that also fails for larger files.

VenelinMartinov · 2024-02-02T19:33:26Z

I wonder what TF ignoreChanges do different here which makes it work for TF but not for pulumi

t0yv0

Thanks for working with me @VenelinMartinov on this, appreciate it.

Approving for now.

t0yv0 · 2024-02-02T20:20:43Z

Filed #3376 - the import story still feels like it needs some work.

guineveresaenger

If I read this correctly, this is to fix a general import failure on AWS lambda that happens because of a field presence validation.

Would it be worth it to add a test here, especially given that a failure to pass could alert us to upstream changes?

VenelinMartinov · 2024-02-02T23:34:48Z

Yup, totally, I’ll add tests before merging!

VenelinMartinov · 2024-02-12T14:05:28Z

examples/examples_test.go

+func TestSourceCodeHashUpdatedLambdaFails(t *testing.T) {
+  // Update requires a Configure, otherwise the AWS provider segfaults.
+
+  // Error looks like this but contains a request ID so we can't match it.


Note that this correctly fails but the error is not at all readable. I'm wondering if there is a nice way for this to fail earlier - essentially we want to fail if a user inputs this and then updates the value.

VenelinMartinov · 2024-02-12T14:06:18Z

examples/examples_test.go

+            "role": "arn:aws:iam::616138583583:role/iamForLambda-d5757fe",
+            "runtime": "nodejs18.x",
+            "skipDestroy": false,
+            "sourceCodeHash": "WUsPYQdwiMj+sDZzl3tNaSzS42vqVfng2CZtgcy+TRt=",


This is the only change in inputs.

VenelinMartinov · 2024-02-12T19:29:48Z

Looks like this does not fix the original issue which motivated it. Turns out the original issue was about using the import resource option and not pulumi import.

Details here: #2392 (comment)

patch lambda importing

4e5882e

VenelinMartinov requested review from t0yv0, guineveresaenger and iwahbe February 1, 2024 20:28

iwahbe reviewed Feb 2, 2024

View reviewed changes

patches/0041-allow-lambdas-with-source_code_hash.patch Outdated Show resolved Hide resolved

patches/0041-allow-lambdas-with-source_code_hash.patch Outdated Show resolved Hide resolved

t0yv0 reviewed Feb 2, 2024

View reviewed changes

patches/0041-allow-lambdas-with-source_code_hash.patch Outdated Show resolved Hide resolved

fix foramtting stuff on patch

8a02e9c

t0yv0 self-requested a review February 2, 2024 19:55

t0yv0 approved these changes Feb 2, 2024

View reviewed changes

guineveresaenger reviewed Feb 2, 2024

View reviewed changes

VenelinMartinov added 5 commits February 4, 2024 19:07

Merge branch 'master' into vvm/fix_lambda_importing

e093bf1

Merge branch 'master' into vvm/fix_lambda_importing

ad8e82f

tests for new behaviour

141f85b

go mod tidy

75fa590

fix up patch

b745fcd

VenelinMartinov commented Feb 12, 2024

View reviewed changes

VenelinMartinov mentioned this pull request Feb 12, 2024

Importing AWS Lambda function can fail to validate #2392

Closed

VenelinMartinov closed this Feb 12, 2024

VenelinMartinov mentioned this pull request Feb 14, 2024

Patch lambda to allow imports #3420

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

patch lambda importing #3371

patch lambda importing #3371

VenelinMartinov commented Feb 1, 2024 •

edited

Loading

github-actions bot commented Feb 1, 2024

iwahbe left a comment

t0yv0 commented Feb 2, 2024

t0yv0 commented Feb 2, 2024

t0yv0 commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024 •

edited

Loading

t0yv0 commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024 •

edited

Loading

VenelinMartinov commented Feb 2, 2024

t0yv0 commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024 •

edited

Loading

t0yv0 left a comment

t0yv0 commented Feb 2, 2024

guineveresaenger left a comment

VenelinMartinov commented Feb 2, 2024

VenelinMartinov Feb 12, 2024

VenelinMartinov Feb 12, 2024

VenelinMartinov commented Feb 12, 2024

patch lambda importing #3371

patch lambda importing #3371

Conversation

VenelinMartinov commented Feb 1, 2024 • edited Loading

github-actions bot commented Feb 1, 2024

Does the PR have any schema changes?

Does the PR have any schema changes?

iwahbe left a comment

Choose a reason for hiding this comment

t0yv0 commented Feb 2, 2024

t0yv0 commented Feb 2, 2024

t0yv0 commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024 • edited Loading

t0yv0 commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024 • edited Loading

VenelinMartinov commented Feb 2, 2024

t0yv0 commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024

VenelinMartinov commented Feb 2, 2024 • edited Loading

t0yv0 left a comment

Choose a reason for hiding this comment

t0yv0 commented Feb 2, 2024

guineveresaenger left a comment

Choose a reason for hiding this comment

VenelinMartinov commented Feb 2, 2024

VenelinMartinov Feb 12, 2024

Choose a reason for hiding this comment

VenelinMartinov Feb 12, 2024

Choose a reason for hiding this comment

VenelinMartinov commented Feb 12, 2024

VenelinMartinov commented Feb 1, 2024 •

edited

Loading

VenelinMartinov commented Feb 2, 2024 •

edited

Loading

VenelinMartinov commented Feb 2, 2024 •

edited

Loading

VenelinMartinov commented Feb 2, 2024 •

edited

Loading