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

SecretOptions for FireLens #8174

Closed
2 tasks
MartinLoeper opened this issue May 24, 2020 · 5 comments · Fixed by #15351
Closed
2 tasks

SecretOptions for FireLens #8174

MartinLoeper opened this issue May 24, 2020 · 5 comments · Fixed by #15351
Assignees
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1

Comments

@MartinLoeper
Copy link

MartinLoeper commented May 24, 2020

It is currently possible to define secrets for FireLens logging configurations. [1]
This is not implemented in the FireLensLogDriver from @aws-cdk/aws-ecs.

[1] https://github.com/aws-samples/amazon-ecs-firelens-examples/blob/master/examples/fluent-bit/datadog/task-definition.json

Use Case

It is important to define secrets via secretOptions otherwise the API keys such as the DataDog API key are displayed as plain text in some logs.

Proposed Solution

I implemented a workaround as a custom class (which extends the current aws-cdk implementation), but I guess that the property should simply be added to the FireLensLogDriver.

import { FireLensLogDriver, FireLensLogDriverProps, LogDriverConfig, ContainerDefinition } from "@aws-cdk/aws-ecs";
import { Construct } from "@aws-cdk/core";
import { IStringParameter } from "@aws-cdk/aws-ssm";

interface FireLensSecret {
    valueFrom: IStringParameter;
    name: string;
}

interface FireLensSecretAsString {
    valueFrom: string;
    name: string;
}

/**
 * Specifies the firelens log driver configuration options.
 */
export interface FireLensLogDriverWithSecretsProps extends FireLensLogDriverProps {
    /**
     * The configuration options to send to the log driver.
     * @default - the log driver options
     */
    readonly secretOptions?: FireLensSecret[];
}

export interface FireLensWithSecretLogDriverConfig extends LogDriverConfig {
    readonly secretOptions: FireLensSecretAsString[];
}

export class FireLensLogDriverWithSecrets extends FireLensLogDriver {
    private readonly secretOptions: FireLensSecret[];

    /**
     * Constructs a new instance of the FireLensLogDriver class.
     * @param props the awsfirelens log driver configuration options.
     */
    constructor(props: FireLensLogDriverWithSecretsProps) {
        super(props);

        this.secretOptions = props.secretOptions || [];
    }

    /**
     * Called when the log driver is configured on a container
     */
    public bind(_scope: Construct, _containerDefinition: ContainerDefinition): FireLensWithSecretLogDriverConfig {
        const config = super.bind(_scope, _containerDefinition);

        return {
            secretOptions: this.secretOptions.map((mapping) => {
                return {
                    name: mapping.name,
                    valueFrom: mapping.valueFrom.parameterArn,
                };
            }),
            ...config,
        };
    }
}

My sample implementation above does not take into account Secrets Manager credentials.
It is for SSM parameters only.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@MartinLoeper MartinLoeper added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 24, 2020
@tobli
Copy link

tobli commented May 24, 2020

See also #7264. I was looking into a PR for that, but support for secret log options should be applicable to all log drivers.

@SomayaB SomayaB added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label May 27, 2020
@piradeepk piradeepk assigned kohidave and hencrice and unassigned uttarasridhar Jun 1, 2020
@piradeepk piradeepk added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 1, 2020
@SoManyHs
Copy link
Contributor

Hi @MartinLoeper, did you still have plans to try implementing this? Thanks!

@MartinLoeper
Copy link
Author

@SoManyHs As @tobli mentioned this issue should probably be fixed at a bigger scope - for all log drivers not the FireLensLogDriver only. That is an effort which I am currently unable to invest. Sorry =(

@epvanhouten
Copy link

I am needing this functionality as well.

@upparekh upparekh self-assigned this Jun 4, 2021
@mergify mergify bot closed this as completed in #15351 Jun 29, 2021
mergify bot pushed a commit that referenced this issue Jun 29, 2021
…15351)

This PR adds support for specifying secrets for the Firelens log driver. It adds the `secretOptions` property to the `LogConfiguration` of log drivers.

Fixes [#8174](#8174)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️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.

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…ws#15351)

This PR adds support for specifying secrets for the Firelens log driver. It adds the `secretOptions` property to the `LogConfiguration` of log drivers.

Fixes [aws#8174](aws#8174)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.