aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.5k stars 3.84k forks source link

aws-batch-alpha: Unable to specify ECS container env vars containing secrets #25559

Closed Malanius closed 1 year ago

Malanius commented 1 year ago

Describe the bug

Since the changes in the batch-alpha module in 2.74.0, it is impossible to define environment variables holding values from secrets. This is due to the changed interface between the old JobDefinitionContainer and new IEcsContainerDefinition. The old interface correctly uses key: value from ECS secret with control of which env variable the secret is assigned to and also which secret field is used. The new one just expects an array of Secrets Manager secrets that don't provide any way to specify these things.

Expected Behavior

To be able to define and use ECS secrets in the same way as with previous constructs.

Current Behavior

The new one just expects an array of Secrets Manager secrets that don't provide any way to assign the secret value and field to the container env variable. I didn't even try to synthesise and deploy this to see what would happen as this clearly doesn't do what's needed.

Reproduction Steps

This is working pre 2.74.0:

import * as batch from '@aws-cdk/aws-batch-alpha';
import * as ecr from 'aws-cdk-lib/aws-ecr';
import * as esc from 'aws-cdk-lib/aws-ecs';
import * as sm from 'aws-cdk-lib/aws-secretsmanager';
import { Construct } from 'constructs';

declare const ecrRepo: ecr.IRepository;
declare const secret: sm.ISecret;

export class JobDefBug extends Construct {
  constructor(scope: Construct, id: string, _props: {}) {
    super(scope, id);

    new batch.JobDefinition(this, 'JobDefinition', {
      container: {
        image: esc.ContainerImage.fromEcrRepository(ecrRepo),
        secrets: {
          SOME_SECRET: esc.Secret.fromSecretsManager(secret, 'secret-field'),
        },
      },
    });
  }
}

Trying to update to the new, post 2.74.0 constructs, the secrets property is incompatible:

import * as batch from '@aws-cdk/aws-batch-alpha';
import * as ecr from 'aws-cdk-lib/aws-ecr';
import * as esc from 'aws-cdk-lib/aws-ecs';
import * as sm from 'aws-cdk-lib/aws-secretsmanager';
import { Construct } from 'constructs';

declare const ecrRepo: ecr.IRepository;
declare const secret: sm.ISecret;

export class JobDefBug extends Construct {
  constructor(scope: Construct, id: string, _props: {}) {
    super(scope, id);

    new batch.EcsJobDefinition(this, 'JobDefinition', {
      container: {
        image: esc.ContainerImage.fromEcrRepository(ecrRepo),
        secrets: {
          SOME_SECRET: esc.Secret.fromSecretsManager(secret, 'secret-field'),
        },
      },
    });
  }
}

The new interface also requires some properties that were optional and had default values in the old one like CPU and memory resources.

Possible Solution

Using the correct interface for secrets property in order to be able to use ECS secrets as before and be able to upgrade existing job definitions is probably the way.

Additional Information/Context

No response

CDK CLI Version

2.79.1

Framework Version

2.74.0

Node.js Version

16.20.0

OS

Linux

Language

Typescript

Language Version

Typescript (4.9.5)

Other information

No response

pahud commented 1 year ago

Yeah I noticed the type is secretsmanager.ISecret[]; now. Thank you for your feedback.

kellertobias commented 1 year ago

Our hotifx for this issue was:

    const job = new batchAlpha.EcsJobDefinition(..., {container: {secrets: Object.values(secretsMap)}} /* Define as usual */)
    const cfnContainerDef = job.node.defaultChild as CfnJobDefinition;
    const secrets: { Name: string; ValueFrom: string }[] = [];
    Object.entries(secretsMap).forEach(([Name, secret]) => {
        secrets.push({ Name, ValueFrom: secret.secretArn });
    });
    cfnContainerDef.addPropertyOverride('ContainerProperties.Secrets', secrets);
github-actions[bot] commented 1 year ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.