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

Not able to use environment variables in command #5501

Closed
vc0sta opened this issue Nov 22, 2023 · 10 comments · Fixed by #5516
Closed

Not able to use environment variables in command #5501

vc0sta opened this issue Nov 22, 2023 · 10 comments · Fixed by #5516
Labels
area/manifest Issues about infrastructure-as-code templates. guidance Issue requesting guidance or information about usage

Comments

@vc0sta
Copy link

vc0sta commented Nov 22, 2023

I have one application that needs some files to be present during startup, as this files changes depending on the environment, the idea is to populate the service container whenever the container starts.

so I created an S3 bucket to store those files and the plan is to use a sidecar container:

image:
  location: image:${TAG}
    ...
  depends_on:
    startup: success

sidecars:
  startup:
    image: public.ecr.aws/aws-cli/aws-cli:2.13.37
    command:
      "s3 sync s3://bucket target/"

This initially worked, the problem is that we have different buckets depending on the environment, how should I do this properly?

1. Using environment variables

The first option was to use env vars for the sidecar container, but I couldnt find the proper way to do that:

sidecars:
  startup:
    image: public.ecr.aws/aws-cli/aws-cli:2.13.37
    command:
      "s3 sync s3://${BUCKET_NAME} ${TARGET}"

These variables are evaluated by copilot during deployment and I didn't found a way to properly scape them, is it possible?

2. Creating my own image

Second solution would be to create my own image, there I can define the entrypoint to do what I want.

To keep all files related to this deployment in the same place, I've defined a simple Dockerfile and pointed to it in the manifest:

sidecars:
  startup:
    image:
      build:
        dockerfile: copilot/sidecars/startup/Dockerfile

That part worked flawlessly, the issue then was that the main container is not using the provided ${TAG} anymore, but it's trying to use ":latest" instead. Is it a known issue? should I open a ticket only for that?

3. Overrides

We finally found a working solution, by using overrides and changing the cloudformation template directly:

- op: add
  path: /Resources/TaskDefinition/Properties/ContainerDefinitions/1/Command
  value:
    - s3
    - sync
    - !Sub s3://${AddonsStack.Outputs.configurationfilesNameBucketName}
    - /var/data

Keeping it like this is totally fine for us, but still looks like a workaround, please let me know if there is a better way to achieve this.

@vc0sta vc0sta changed the title Not able to use environment variables in Not able to use environment variables in command Nov 22, 2023
@KollaAdithya
Copy link
Contributor

KollaAdithya commented Nov 22, 2023

Hello @vc0sta 👋

  1. Yaml patches looks good to me in this case. One way I could think is to use environments to override the command value per environment.
environments:
   test:
     sidecars:
        startup:
          command: "s3 sync s3://bucket1 target1/"
   prod:
     sidecars:
        startup:
          command: "s3 sync s3://bucket2 target2/"
  1. With the issue on tag, when Copilot builds the image for you by default tags the image with latest.
    if you want to set your own tag on the image then you can run copilot svc deploy --tag ${TAG}

@KollaAdithya KollaAdithya added guidance Issue requesting guidance or information about usage area/manifest Issues about infrastructure-as-code templates. labels Nov 22, 2023
@vc0sta
Copy link
Author

vc0sta commented Nov 23, 2023

yes, the environment overrides should work as well, but the bucket name would be hardcoded. My goal is to get them dynamically, this is why I'm using cloudformation overrides.

Regarding the latest tag, I'll try to make it clearer:

We have 2 containers for the same application, first one is using a pre-built image and its expected for the tag to come from CI/CD.

image:
  location: myApplication:${TAG}

for the sidecars, I was building the image from the Dockerfile

sidecars:
  startup:
    image:
      build:
        dockerfile: copilot/sidecars/startup/Dockerfile

The deploy command I'm using is TAG=develop copilot svc deploy ..., then the expected behavior is to have two containers running, myApplication:develop and startup:latest, right? But the outcome is actually myApplication:latest and startup:latest.

I'll try with --tag to check whether the outcome is the same.

edit:
by using copilot svc deploy --tag=1.25 ... I got exactly the same behavior.
sidecar was published as startup-latest and the task definition is using the right reference, but the main container is still pointing to :latest.

@KollaAdithya
Copy link
Contributor

Hello @vc0sta !

In your case main container should be tagged as myApplication:develop.
Can you go to CloudFormation Console->Parameters->ContainerImage. What is the Value of that Parameter you are seeing over there?

@bencehornak
Copy link
Contributor

bencehornak commented Dec 2, 2023

In my interpretation there's one improvement possibility here: the ability of escaping dollar signs in the manifest.

At the moment this manifest doesn't work and cannot be fixed (AFAIK):

image:
  # ...
  command: echo "Hello ${name}"
variables:
  name: world

because Copilot will try to substitute ${name} when running copilot svc deploy, as opposed to letting the container evaluate it in its own shell.

If there was a way to escape the Copilot variable substitution syntax, @vc0sta could write a command shared across all environments and would only need to provide the environment variable with a cfn override:

manifest.yaml

sidecars:
  startup:
    image: public.ecr.aws/aws-cli/aws-cli:2.13.37
    command:
      "s3 sync s3://\\${BUCKET_NAME} \\${TARGET}"
  variables:
    TARGET: /var/data

cfn.overrides.yaml

- op: add
  path: /Resources/TaskDefinition/Properties/ContainerDefinitions/1/Environment/-
  value:
    Name: BUCKET_NAME
    Value: !Sub s3://${AddonsStack.Outputs.configurationfilesNameBucketName}

@vc0sta
Copy link
Author

vc0sta commented Dec 7, 2023

Hello @vc0sta !

In your case main container should be tagged as myApplication:develop. Can you go to CloudFormation Console->Parameters->ContainerImage. What is the Value of that Parameter you are seeing over there?

it was showing :latest

@Lou1415926
Copy link
Contributor

@vc0sta um, that's unexpected. Just to share - I tried the same thing:

image:
  location: some-image-locator:${TAG}

and ran TAG=develop copilot svc package --output-dir infra --upload-assets (which will generate the same template and parameters as copilot svc deploy would), the -.params.json shows "ContainerImage": "some-image-locator:develop".

What baffles me is that, even if Copilot had failed to substitute the ${TAG} part of location: myApplication:${TAG} (provided in your comment), it would have resulted a malformat-ed url myApplication:, without the "latest".

I know you've shared some manifest snippets - I'm sorry to ask for this again. Can you share the entire image section?

@vc0sta
Copy link
Author

vc0sta commented Dec 12, 2023

@Lou1415926 Sure.

Here is the manifest:

name: vcosta-test
type: Backend Service

image:
  location: public.ecr.aws/nginx/nginx:${TAG}
  depends_on:
    startup: success

sidecars:
  startup:
    image:
      build:
        dockerfile: copilot/vcosta-test/Dockerfile
    essential: false

The Dockerfile is just a placeholder:

FROM public.ecr.aws/aws-cli/aws-cli:2.13.37

This generates the following parameters:

{
  "Parameters": {
    "ContainerImage": ":latest",
  }
}

If I comment out the startup container, or if I change it to a pre-built image, then the TAG is correctly evaluated:

{
  "Parameters": {
    "ContainerImage": "public.ecr.aws/nginx/nginx:develop",
  }
}

mergify bot pushed a commit that referenced this issue Dec 12, 2023
Addresses: #5501 (comment)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
@Lou1415926
Copy link
Contributor

Lou1415926 commented Dec 12, 2023

@vc0sta Thanks for sharing!! What you said helped a ton - the comment of "If I comment out the startup container, or if I change it to a pre-built image..." made me realize what the bug may have been. If you use image.location for main container, and image.build for at least one of the sidecars, the image URL for the main container wouldn't be populated correctly.

I've sent out a fix #5555! Sorry for the trouble and the back and forth!

@vc0sta
Copy link
Author

vc0sta commented Dec 13, 2023

oh, that was actually pretty quick solution!

thank you very much!

@iamhopaul123
Copy link
Contributor

Hello! The fix is now released in v1.32.1: https://github.com/aws/copilot-cli/releases/tag/v1.32.1

mergify bot pushed a commit that referenced this issue Dec 21, 2023
Allows escaping of interpolated variables:

```yaml
command: echo hello \${name}
variable:
  name: world
```

Fixes #5501, fixes #5532

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/manifest Issues about infrastructure-as-code templates. guidance Issue requesting guidance or information about usage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants