amazon-ecs-render-task-definition icon indicating copy to clipboard operation
amazon-ecs-render-task-definition copied to clipboard

feat: Include Environment Variables in Task Definitions with JSON Fragments

Open rojaswestall opened this issue 5 years ago • 12 comments
trafficstars

Issue https://github.com/aws-actions/amazon-ecs-render-task-definition/issues/20

Description of changes:

Merge a JSON fragment of a task definition with a task definition using lodash's mergewith. containerDefinitions and name within each container definition are required in the fragment.

  • Merge JSON fragments specified with merge input with the task definition
  • Added lodash mergeWith dependency
  • Updated tests
  • Updated README
  • Updated actions.yml inputs (made image optional and added merge)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

rojaswestall avatar Nov 06 '20 20:11 rojaswestall

Hey folks,

Do you plan to merge this soon? :D

brunocascio avatar Dec 17 '20 15:12 brunocascio

@rojaswestall do you have any comment/response to the PR feedback? I would like to work towards getting this approved if you're still interested

allisaurus avatar Jan 04 '21 17:01 allisaurus

We need this, merge please. Thanks.

rajeevnaikte avatar Jan 18 '21 22:01 rajeevnaikte

I don't think you need this to solve the problem, if you want to have different environment variables per deploying environment you can simply load a different task-definition.json. Here's what I did:

task-definition: deploy/task-definition.${{ env.ENVIRONMENT }}.json

For those who use SSM, the following is not a problem. It actually works well with that trick above ☝️ . However, if you only want to use Github secrets and you want to add them to the environment variables I think it would be great if we had a way to inject those variables as we do with the <IMAGE> tag. Where we could set a specific key and add it to the inputs. Some example:

{
  "name": "adapter-service",
  "image": "<IMAGE>",
  "environment": [
    {
      "name": "NODE_ENV",
      "value": "<ENVIRONMENT>"
    },
  ]
}

There are probably different ways to do this, but I think we can simply search for values with <> and match them with env variables names:

      - name: Fill in the new image ID in the Amazon ECS task definition
        id: task-def
        uses: aws-actions/amazon-ecs-render-task-definition@v1
        with:
          task-definition: deploy/task-definition.${{ env.ENVIRONMENT }}.json
          container-name: container-name
          image: container-image
        env:
          ENVIRONMENT: ${{ env.ENVIRONMENT }} 

What do you think?

rntdrts avatar Feb 04 '21 17:02 rntdrts

@rntdrts Having 1 file per environment is not scalable IMHO.

I implemented a similar solution seeding variables into a template.

task-definition.myservice.template.json

{
    "family": "my-service-__ENVIRONMENT__",
    "networkMode": "awsvpc",
    "executionRoleArn": "arn:aws:iam::764292098539:role/ecsTaskExecutionRole",
    "containerDefinitions": [
        {
            "name": "my-service",
            "image": "",
            "portMappings": [
                {
                    "hostPort": 5000,
                    "protocol": "tcp",
                    "containerPort": 5000
                }
            ],
            "essential": true,
            "environment": [
                {
                    "name": "NODE_ENV",
                    "value": "production"
                },
                ...
            ],
            "secrets": [
                {
                    "valueFrom": "__ENVIRONMENT__.MY_SERVICE_API_KEY",
                    "name": "API_KEY"
                },
                ...
            ]
        }
    ],
    "requiresCompatibilities": [
        "FARGATE"
    ],
    "cpu": "256",
    "memory": "512"
}

workflow yml:

 - name: Sed "my-service" task definition with staging
   run: sed -i "s/__ENVIRONMENT__/staging/g" .github/workflows/task-definition.my-service.template.json

- name: Render "my-service" task definition
  id: rendered-my-service-task-definition
  uses: aws-actions/amazon-ecs-render-task-definition@v1
  with:
    task-definition: .github/workflows/task-definition.my-service.template.json
    container-name: my-service
    image: ${{ env.DTR_REGISTRY }}/my-service:${{ env.GIT_COMMIT }}

- name: Deploy "my-service" to staging
  uses: aws-actions/amazon-ecs-deploy-task-definition@v1
  with:
    task-definition: ${{ steps.rendered-my-service-task-definition.outputs.task-definition }}
    service: my-service-staging
    cluster: my-cluster

I get populated SSM before with staging.MY_SERVICE_API_KEY. If it's missing, so aws keep failing until it exists.

The goal here is to use the same task definition across all the environments (could be DEV, QA, PPD, INT, PROD, etc) and write the code once.

brunocascio avatar Feb 04 '21 19:02 brunocascio

The goal here is to use the same task definition across all the environments (could be DEV, QA, PPD, INT, PROD, etc) and write the code once.

Maybe I didn't understand, but I don't think that what was done here solves your problem (at least by looking at the code). What is done here is a way to merge JSON files with each other. You will end up having to create similar files for each deploying environment (staging.json prod.json), plus the generic task definition.

rntdrts avatar Feb 04 '21 21:02 rntdrts

The goal here is to use the same task definition across all the environments (could be DEV, QA, PPD, INT, PROD, etc) and write the code once.

Maybe I didn't understand, but I don't think that what was done here solves your problem (at least by looking at the code). What is done here is a way to merge JSON files with each other. You will end up having to create similar files for each deploying environment (staging.json prod.json), plus the generic task definition.

Yes and no, because your staging.json and prod.json would only contain the dynamic parts, like the following:

task-definition.json

{
    "networkMode": "awsvpc",
    "executionRoleArn": "arn:aws:iam::764292098539:role/ecsTaskExecutionRole",
    "containerDefinitions": [
        {
            "name": "my-service",
            "image": "",
            "portMappings": [
                {
                    "hostPort": 5000,
                    "protocol": "tcp",
                    "containerPort": 5000
                }
            ],
            "essential": true,
            "environment": [
                {
                    "name": "NODE_ENV",
                    "value": "production"
                },
                ...
            ],
            ]
        }
    ],
    "requiresCompatibilities": [
        "FARGATE"
    ],
    "cpu": "256",
    "memory": "512"
}

staging.json

{
    "family": "my-service-staging",
    "containerDefinitions": [
        {
            "name": "my-service",
            "secrets": [
                {
                    "valueFrom": "staging.MY_SERVICE_API_KEY",
                    "name": "API_KEY"
                },
                ...
            ]
        }
    ]
}

prod.json

{
    "family": "my-service-prod",
    "containerDefinitions": [
        {
            "name": "my-service",
            "secrets": [
                {
                    "valueFrom": "production.MY_SERVICE_API_KEY",
                    "name": "API_KEY"
                },
                ...
            ]
        }
    ]
}

Might be better to have a template file and replace it with sed, but for example, if you want to use a different logging driver for production, you have to duplicate the code, so here's where merge JSON files come into action.

brunocascio avatar Feb 04 '21 21:02 brunocascio

Hi. Any ideas when it can be merged? =)

dspv avatar Jun 10 '21 12:06 dspv

Hey folks. Any idea when it is going to be merged? It has been a while now.

alencarsouza avatar Apr 14 '23 19:04 alencarsouza

Really looking forward for this!

brunoenribeiro avatar Jun 16 '23 18:06 brunoenribeiro

Waiting for this issue to get merged

pravinfullstack avatar Jul 22 '23 19:07 pravinfullstack

Hi @rojaswestall, thank you so much for your contribution. Apologies on the delay. We will be working on reviewing Pull Requests on the repositories. In the mean time please ensure that below steps, if not already done, are taken care of in your PR:

  1. Verify if PR follows semantic pull request conventions.

  2. Please ensure any comments on the earlier reviews of the PR are addressed.

  3. Please run npm run package command to update dist/ folder with latest dependencies.

  4. Resolve merge conflicts on the PR if any.

amazreech avatar May 10 '24 18:05 amazreech