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

aws-eks: allow passing secret to cluster.addHelmChart and cluster.addManifest #16476

Open
2 tasks
trondhindenes opened this issue Sep 13, 2021 · 9 comments
Open
2 tasks
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1

Comments

@trondhindenes
Copy link

trondhindenes commented Sep 13, 2021

it would be good if CDK would resolve references to SecretsManager for values passed to addHelmChart

Use Case

I'm trying to do this: read a secret, apply a helm chart with values referencing that secret. Code:

    secret = secretsmanager.Secret.from_secret_name_v2(scope, 'linkerDSecret', '/kubernetescluster/linkerd')
    cluster.add_helm_chart(
        id='linkerd2',
        chart='linkerd2',
        repository='https://helm.linkerd.io/stable',
        create_namespace=True,
        release='linkerd',
        values={
            'identityTrustAnchorsPEM': secret.secret_value_from_json(key='identityTrustAnchorsPEM'),
            'identity': {
                'issuer': {
                    'crtExpiry': secret.secret_value_from_json(key='identityissuerCertExpiry'),
                    'tls': {
                        'crtPEM': secret.secret_value_from_json(key='identityissuertlscrtPEM'),
                        'keyPEM': secret.secret_value_from_json(key='identityissuertlskeyPEM'),
                    }
                }
            }
        }
    )

However, this seems to not be supported - secrets are not resolved. It would be good if this was supported, so that we have a way of sending secrets to Helm charts.

Proposed Solution

Other

cluster.addManifest also does not resolve secrets. As far as I can see, this means that there's no way to pass secrets to a helm template or kubernetes manifest managed by CDK. This is quite problematic for us, as we'd hoped to be using CDK for all "core cluster" helm charts (ingress controllers, linkerd setup, etc)

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

This is a 🚀 Feature Request

@trondhindenes trondhindenes added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Sep 13, 2021
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Sep 13, 2021
@trondhindenes trondhindenes changed the title aws-eks: allow passing secret to cluster.addHelmChart aws-eks: allow passing secret to cluster.addHelmChart and cluster.addManifest Sep 13, 2021
@otaviomacedo otaviomacedo added effort/small Small work item – less than a day of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Sep 24, 2021
@otaviomacedo otaviomacedo removed their assignment Sep 24, 2021
@otaviomacedo
Copy link
Contributor

I am unassigning and marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

@markussiebert
Copy link
Contributor

I like this issue, but it's somehow similar to the following ones:

#15853
#9815

The main problem is, that cloudformation can't handle this

Dynamic references for secure values, such as secretsmanager, aren't currently supported in custom resources.
cloudformation documentation

So i see no 'native' way of implementing this.

A workaround could be extending the kubectl/helm handler with vals. I use it a lot in combination with [helmfile]https://github.com/roboll/helmfile and it offers access for a lot backends.

Wht do you think about this solution?

The question would be: how do we handle the iam permissions that we have to attach to the kubectl/helm handler...

@trondhindenes
Copy link
Author

trondhindenes commented Oct 7, 2021

Hi,
We basically ended up making custom providers for helm/kubectl charts, where we implement this. We string the payload, and look for the syntax cloudformation uses for secretmanager references, and for each if found, we translate that to a "real" secret and write it back to the string. This works well for both helm and kubectl, and allows us to use cdk's regular secrets manager lookup syntax (since that just constructs regular cloudformation refs).

This is basically the code in the handler lambda that deals with replacing secrets manager refs with "real" secrets:

def parse_and_resolve_secretsmanager(input_str):
    client = boto3.client('secretsmanager')
    split_string = input_str.split(':')
    secrets_manager_arn = str.join(':', split_string[1:8])
    result = client.get_secret_value(SecretId=secrets_manager_arn)
    if split_string[9]:
        parsed_secret = json.loads(result['SecretString'])
        return parsed_secret[split_string[9]]
    return result['SecretString']


def parse_string(input_str):
    old_parsed_str = input_str
    parsed_str = ''
    parse_counter = 0
    while parsed_str != old_parsed_str:
        old_parsed_str = parsed_str
        if parsed_str == '':
            parsed_str = input_str
        a = [x for x in re.finditer('{{resolve:(.*?)}}', parsed_str)]
        if not a:
            return parsed_str
        item = a[0]
        match_string = item.string
        inner_str = item.groups()[0]
        if inner_str.startswith('secretsmanager:'):
            print(f'replacing string with secrets manager lookup: {inner_str}')
            resolved_str = parse_and_resolve_secretsmanager(inner_str)
            replace_str = parsed_str[item.regs[0][0]:item.regs[0][1]]
            parsed_str = parsed_str.replace(replace_str, resolved_str)
        parse_counter += 1

Here's one of the tests we use:

@mock_secretsmanager
def test_complex_secrets():
    os.environ['AWS_DEFAULT_REGION'] = 'eu-west-1'
    client = boto3.client('secretsmanager')
    simple_secret1 = client.create_secret(Name='simplesecret1', SecretString='IamSecrit1')
    simple_secret2 = client.create_secret(Name='simplesecret2', SecretString='IamSecrit2')

    res = secrets_parser.parse_string('{{resolve:secretsmanager:' + simple_secret1['ARN'] + ':::}}:{{resolve:secretsmanager:' + simple_secret2['ARN'] + ':::}}')
    assert res == 'IamSecrit1:IamSecrit2'

we had to add a secrets_manager_read_access to our custom provider, which controls IAM access to the necessary secrets for the lambda handler driving the custom resource.

@trondhindenes
Copy link
Author

(I thought about publishing this to the constructs catalog, but haven't quite finished making it open-source-friendly yet)

@github-actions github-actions bot added p1 and removed p2 labels Apr 2, 2023
@github-actions
Copy link

github-actions bot commented Apr 2, 2023

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

@iliapolo
Copy link
Contributor

Yeah this is an annoying one. We don't have an action plan yet but its on our radar now.

@vechorko
Copy link

vechorko commented Jun 14, 2023

any road map or estimation time for implementation it? it's really major feature in use helm charts in big infrastructure to pass passwords and secret date from another stacks and services.
Just as a temporary solution, can be used SSM Parameters.

@akmalharith
Copy link

@trondhindenes I wonder if you are using kubectlprovider by cdk, aren’t the manifests being applied actually printing the secrets in the Lambda logs. How are you suppressing them?

@stephanpelikan
Copy link

We would also need that feature, passing DB credentials. I will try to use external-secrets instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

7 participants