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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker images: cannot use tokens in buildArgs - improve validation and errors #3981

Closed
PeterBengtson opened this issue Sep 7, 2019 · 13 comments 路 Fixed by #3989
Closed

docker images: cannot use tokens in buildArgs - improve validation and errors #3981

PeterBengtson opened this issue Sep 7, 2019 · 13 comments 路 Fixed by #3989
Assignees
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@PeterBengtson
Copy link

馃悰 Bug Report

What is the problem?

We all know that this works as intended:

    const myLambda = new lambda.Function(this, 'MyLambda', {
      ...
      environment: {
        FOO: mySqsQueue.queueArn,
      },
    })

The SQS queue ARN token is evaluated as expected and its final value used.
The following, however, does not work:

    const myImage = new ecrAssets.DockerImageAsset(this, 'MyImage', {
      ...
      buildArgs: {
        FOO: mySqsQueue.queueArn,
      },
    })

In the last example, the token is never expanded.

Environment

  • CDK CLI Version: 1.7
  • OS: all
  • Language: all
@PeterBengtson PeterBengtson added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 7, 2019
@jogold
Copy link
Contributor

jogold commented Sep 7, 2019

The buildArgs are used when building your image (docker build ...). This happens before your stack gets deployed. At this stage mySqsQueue.queueArn cannot be resolved, it's a deploy time value.

@PeterBengtson
Copy link
Author

PeterBengtson commented Sep 7, 2019

So how would the above be rewritten to actually work? You must agree that the symmetry of the above is misleading and unintuitive, as both the above are assets: one a lambda, the other one an ECR image. Conceptually, the difference is very slight. Yet, CDK handles these cases differently.

@jogold
Copy link
Contributor

jogold commented Sep 7, 2019

Can you detail your use case? How about using environment in your ContainerDefinition (taskDefinition.addContainer()) ?

@PeterBengtson
Copy link
Author

There is no TaskDefinition here; you're assuming the use case involves ECS. The ECR container is used by a fleet of EC2 spot instances.

@PeterBengtson
Copy link
Author

To my mind, CDK is simply wrong here. The two examples above are structurally almost identical, and I see no reason why both examples shouldn't evaluate their args in a similar way. Are you saying the current behaviour is intentional? If so, how should the code be written to resolve the args as expected?

@jogold
Copy link
Contributor

jogold commented Sep 7, 2019

Why does your image needs your queue arn at build time?

@PeterBengtson
Copy link
Author

For various security reasons the ARN can't be passed in at run-time. But isn't that really beside the point? What I'm wondering about is the asymmetry of the behaviour in the above two cases. Is it intentional, and if so, isn't it misleading? And, again if so, how should it be rewritten to work?

@jogold
Copy link
Contributor

jogold commented Sep 7, 2019

There's no real asymmetry. Both aren't assets. A Lambda function is a resource, its code is an asset (zipped and uploaded to S3 before CF deploy). You would face the same problem if you'd want your code to be aware of a deploy time queue arn.

You cannot compare buildArgs with environment.

@rhboyd
Copy link
Contributor

rhboyd commented Sep 7, 2019

To my mind, CDK is simply wrong here. The two examples above are structurally almost identical, and I see no reason why both examples shouldn't evaluate their args in a similar way. Are you saying the current behaviour is intentional? If so, how should the code be written to resolve the args as expected?

A hospital and an office complex are structurally almost identical but you'd be advised against coming to work with a broken leg and expecting it to feel better when you leave.

A Docker container and a Lambda Function are rather different, even though they share a similar API. A Docker image needs to be built before it can be used, a Lambda function does not. It makes sense that building/deploying a Docker Image would involve more steps than building/deploying a Lambda Function. I would argue that building a Docker Image is closer to building an EC2 AMI than it is to packaging a Lambda Function.

@eladb
Copy link
Contributor

eladb commented Sep 8, 2019

I agree with @PeterBengtson that this is confusing, without knowing better, I would expect this to also work.

But, sadly, there is not much we can do in this particular case besides a runtime error (which I think would at least reduce some of the grief). Building on @jogold's suggestion, maybe you can export a QUEUE_URL environment in a user-data script in your EC2 instance instead of baking it into the build image?

Tokens are pretty magical. They allow us to treat late-bound values as first class in all programming languages, and in many cases, they "Just Work" and users don't need to understand their details. In some (hopefully edge cases), their magic "leaks" and we need to be more explicit about validation and user education.

I am repurposing this issue to improve the error message in case tokens are used for buildArgs.

@eladb eladb changed the title Tokens not evaluated docker images: cannot use tokens in buildArgs - improve validation and errors Sep 8, 2019
eladb pushed a commit that referenced this issue Sep 8, 2019
Since `buildArgs` are used before deployment, if tokens are used,
we will fail quickly with a nice message.

Fixes #3981
eladb pushed a commit that referenced this issue Sep 8, 2019
Since `buildArgs` are used before deployment, if tokens are used,
we will fail quickly with a nice message.

Fixes #3981
@eladb
Copy link
Contributor

eladb commented Sep 8, 2019

See #3989

@eladb eladb self-assigned this Sep 8, 2019
@PeterBengtson
Copy link
Author

Thanks, Elad, for clarifying the situation. I will forward your response to the developer who wrote the code in question. As a workaround, I had already advised him to store the URL in Parameter Store instead. A better error message would have reduced the confusion, so it's good it's forthcoming. Thanks.

@PeterBengtson PeterBengtson reopened this Sep 8, 2019
mergify bot pushed a commit that referenced this issue Sep 9, 2019
* feat(ecr-assets): fail if tokens are used in buildArgs

Since `buildArgs` are used before deployment, if tokens are used,
we will fail quickly with a nice message.

Fixes #3981

* do not import Token, since its not used
@kgunny
Copy link

kgunny commented Aug 10, 2020

Hey peops, i hit this issue today, but it makes no sense why i can't use the references in this instance. If this is just the wrong way to code this please let me know. I'm creating an ECR repo, then trying to use DockerImageAsset to get an image into the repo.
self.my_repo = ecr.Repository( self, 'my-repo', repository_name='my-repo', image_scan_on_push=True ) my_image_asset = ecr_assets.DockerImageAsset( self, 'my-image', directory='dockerfile-dir', repository_name=self.my_rep.repository_name, target=self.my_rep.repository_uri_for_tag('my-image:latest') )
Errors i get:
`
jsii.errors.JavaScriptError:
Error: Cannot use Token as value of 'repositoryName': this value is used before deployment starts
at validateProps (C:\Users\kgunn\AppData\Local\Temp\jsii-kernel-e21lP5\node_modules@aws-cdk\aws-ecr-assets\lib\image-asset.js:91:19)
at new DockerImageAsset (C:\Users\kgunn\AppData\Local\Temp\jsii-kernel-e21lP5\node_modules@aws-cdk\aws-ecr-assets\lib\image-asset.js:19:9)
at obj._wrapSandboxCode (C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_embedded\jsii\jsii-runtime.js:7922:49)
at Kernel._wrapSandboxCode (C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_embedded\jsii\jsii-runtime.js:8398:19)
at Kernel._create (C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_embedded\jsii\jsii-runtime.js:7922:26)
at Kernel.create (C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_embedded\jsii\jsii-runtime.js:7666:21)
at KernelHost.processRequest (C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_embedded\jsii\jsii-runtime.js:7446:28)
at KernelHost.run (C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_embedded\jsii\jsii-runtime.js:7384:14)
at Immediate.setImmediate [as _onImmediate] (C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_embedded\jsii\jsii-runtime.js:7387:37)
at runCallback (timers.js:705:18)
at tryOnImmediate (timers.js:676:5)
at processImmediate (timers.js:658:5)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "app.py", line 72, in
ecr_stack = EcrStack(app,
File "C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_runtime.py", line 66, in call
inst = super().call(*args, **kwargs)
File "C:_work\ProjectsCloud\data-platform-service-api\data_platform_service_api\ecr_stack.py", line 17, in init
my_image_asset = ecr_assets.DockerImageAsset(
File "C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_runtime.py", line 66, in call
inst = super().call(*args, **kwargs)
File "C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\aws_cdk\aws_ecr_assets_init_.py", line 149, in init
jsii.create(DockerImageAsset, self, [scope, id, props])
File "C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_kernel_init_.py", line 224, in create
response = self.provider.create(
File "C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_kernel\providers\process.py", line 333, in create
return self._process.send(request, CreateResponse)
File "C:_work\ProjectsCloud\data-platform-service-api.env\lib\site-packages\jsii_kernel\providers\process.py", line 318, in send
raise JSIIError(resp.error) from JavaScriptError(resp.stack)
jsii.errors.JSIIError: Cannot use Token as value of 'repositoryName': this value is used before deployment starts
Subprocess exited with error 1
`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants