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

(secretsmanager): secret_name returns the end of the ARN with 6 character suffix #11727

Closed
melodyyangaws opened this issue Nov 26, 2020 · 7 comments · Fixed by #11736
Closed
Assignees
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. in-progress This issue is being actively worked on. needs-triage This issue or PR still needs to be triaged.

Comments

@melodyyangaws
Copy link

melodyyangaws commented Nov 26, 2020

Reopen the issue #11573

Need to create an external secret in EKS, which is retrieved from secrets manager. Currently, CDK returns a secret name with secrets manager's 6 character suffix. Hopefully it can be fixed. Or a workaround would be appreciated.

It caused the error: {"level":50,"time":1606359303455,"pid":17,"hostname":"external-secrets-kubernetes-external-secrets-5648bf6544-p87vp","message":"Secrets Manager can't find the specified secret.","code":"ResourceNotFoundException"

Reproduction Steps

  1. create a random secret in secrets manager:
    my_secret = Secret(self, 'xx', generate_secret_string=SecretStringGenerator(secret_string_template=json.dumps({'username': 'tester'}), generate_string_key="password") )
  2. create an external secret resource in EKS while replacing the place holder in the yaml file:
eks.KubernetesManifest(self,'xx',
            cluster=my_cluster,
            manifest=loadYamlReplaceVarLocal('ex-secret.yaml', 
                fields= {
                    "{{secret_name}}": my_secret.secret_name
                })
        )

The content of ex-secret.yaml:

apiVersion: kubernetes-client.io/v1
kind: ExternalSecret
metadata:
  name: jupyter-external-secret
  namespace: jupyter
spec:
  backendType: secretsManager
  data:
    - key: {{secret_name}}
      name: password
      property: password          

What did you expect to happen?

I want to get the correct secret name that matches the name in secrets manager console.

What actually happened?

in CDK, my_secret.secret_name returns the end of full ARN with 6 character suffix, ie.jHubSecretArn6E48ECC3-0YhYd3BbaFH5-qeNyUO. The external secret object in EKS is actually expecting jHubSecretArn6E48ECC3-0YhYd3BbaFH5

Environment

  • CDK CLI Version : 1.74.0 (build e86602f)
  • Framework Version: 1.74.0
  • Node.js Version: v12.16.2
  • OS : macOS Catalina 10.15.7
  • Language (Version) : Python 3.8.5

Other

in CFN, it looks like this. can we further split it by '-', then take the first & second parts to construct the secret name?
"Value": {
"Fn::Select": [6,{"Fn::Split": [":",{ "Ref": "jHubSecretArn6E48ECC3" }]}]
}

This is 🐛 Bug Report

@melodyyangaws melodyyangaws added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 26, 2020
@github-actions github-actions bot added the @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager label Nov 26, 2020
@njlynch
Copy link
Contributor

njlynch commented Nov 26, 2020

Hi @melodyyangaws ,

Any particular reason you're using the secretName over the secretArn here? Given this is an "owned" secret, where you control access to the secret and are using it to generate the manifest, using the name over ARN seems to have extremely marginal value.

in CFN, it looks like this. can we further split it by '-', then take the first & second parts to construct the secret name?

This is impossible to do generically with built-in CloudFormation tools, but if you happen to know the format of the secretName (e.g., how many total hyphens exist) you can use the Fn::Select and Fn::Split functions to get just the "pure" secretName.

import { Fn } from '@aws-cdk/core';
const secretNameParts = Fn.split('-', my_secret.secret_name);
const secretNameWithoutSuffix = Fn.join('-', [Fn.select(0, secretNameParts), Fn.select(1, secretNameParts)]);

@melodyyangaws
Copy link
Author

Hi @njlynch, thank you so much, your suggestion worked!
It looks like Python slicing & split functions, such as my_secret.secret_name[:-7], don't work in this case, CDK core functions work !

njlynch added a commit that referenced this issue Nov 26, 2020
…der feature flag)

Proper (tokenized) secret names for owned secrets was introduced in #11202. This
caused `Secret.secretName` to return the full resource name (secret name + suffix)
for owned secrets. This was done to ensure that the secretName was a Tokenized
value that would create a dependency between the Secret and its consumer.

However, Secrets Manager's DescribeSecret API does not accept the resourceName
as an input; it accepts the full ARN, a partial ARN (without the suffix), or the
secret name (without the suffix). This leads to several integrations with other
services or custom scripts to fail when using the secretName from an owned
Secret.

For owned secrets, we can formulate how to parse the resourceName to get the
secretName based on either (a) knowing the secret name or (b) knowing the format
that CloudFormation uses when secret names aren't specified. This fix introduces
proper parsing of secret names for owned secrets (behind a feature flag).

BREAKING CHANGE: (feature flag) Secret.secretName for owned secrets will now
only the secret name (without suffix) and not the full resource name. This is
enabled through the `@aws-cdk/secretsmanager:parseOwnedSecretName` flag.

fixes #11727
njlynch added a commit that referenced this issue Nov 26, 2020
…der feature flag)

Proper (tokenized) secret names for owned secrets was introduced in #11202. This
caused `Secret.secretName` to return the full resource name (secret name + suffix)
for owned secrets. This was done to ensure that the secretName was a Tokenized
value that would create a dependency between the Secret and its consumer.

However, Secrets Manager's DescribeSecret API does not accept the resourceName
as an input; it accepts the full ARN, a partial ARN (without the suffix), or the
secret name (without the suffix). This leads to several integrations with other
services or custom scripts to fail when using the secretName from an owned
Secret.

For owned secrets, we can parse the resourceName to get the secretName based on
either (a) knowing the secret name or (b) knowing the format that CloudFormation
uses when secret names aren't specified. This fix introduces proper parsing of
secret names for owned secrets (behind a feature flag).

BREAKING CHANGE: (feature flag) Secret.secretName for owned secrets will now
only the secret name (without suffix) and not the full resource name. This is
enabled through the `@aws-cdk/secretsmanager:parseOwnedSecretName` flag.

fixes #11727
njlynch added a commit that referenced this issue Nov 26, 2020
…der feature flag)

Proper (tokenized) secret names for owned secrets was introduced in #11202. This
caused `Secret.secretName` to return the full resource name (secret name + suffix)
for owned secrets. This was done to ensure that the secretName was a Tokenized
value that would create a dependency between the Secret and its consumer.

However, Secrets Manager's DescribeSecret API does not accept the resourceName
as an input; it accepts the full ARN, a partial ARN (without the suffix), or the
secret name (without the suffix). This leads to several integrations with other
services or custom scripts to fail when using the secretName from an owned
Secret.

For owned secrets, we can parse the resourceName to get the secretName based on
either (a) knowing the secret name or (b) knowing the format that CloudFormation
uses when secret names aren't specified. This fix introduces proper parsing of
secret names for owned secrets (behind a feature flag).

BREAKING CHANGE: (feature flag) Secret.secretName for owned secrets will now
only the secret name (without suffix) and not the full resource name. This is
enabled through the `@aws-cdk/secretsmanager:parseOwnedSecretName` flag.

fixes #11727
@njlynch
Copy link
Contributor

njlynch commented Nov 26, 2020

As a general rule, attributes of a construct (e.g., things assigned at deploy time) are Tokens, which are placeholders that can't be directly manipulated. The only way to handle them is through Cfn-native functions.

For what it's worth, while I think you should use secretArn in this case, the fact that secretName is somewhat useless for "owned" secrets is ... annoying. This is the third or fourth complaint I've received on this behavior since launching secretName and finishing #11202. This bug was the final metaphorical straw. I've coded up a fix here: #11736. Thanks for the inspiration! :)

@njlynch njlynch added the in-progress This issue is being actively worked on. label Nov 26, 2020
@melodyyangaws
Copy link
Author

HI @njlynch , I am new to EKS, maybe I am wrong. The secretName is a compulsory property to make the secret retrieval working in my case. Base on the blog, the kubermetes external secrets controller pull out the credentials from secrets manager using its name as the key. It doesn't support secretArn yet.

@mergify mergify bot closed this as completed in #11736 Nov 30, 2020
mergify bot pushed a commit that referenced this issue Nov 30, 2020
…der feature flag) (#11736)

Proper (tokenized) secret names for owned secrets was introduced in #11202. This
caused `Secret.secretName` to return the full resource name (secret name + suffix)
for owned secrets. This was done to ensure that the secretName was a Tokenized
value that would create a dependency between the Secret and its consumer.

However, Secrets Manager's DescribeSecret API does not accept the resourceName
as an input; it accepts the full ARN, a partial ARN (without the suffix), or the
secret name (without the suffix). This leads to several integrations with other
services or custom scripts to fail when using the secretName from an owned
Secret.

For owned secrets, we can parse the resourceName to get the secretName based on
either (a) knowing the secret name or (b) knowing the format that CloudFormation
uses when secret names aren't specified. This fix introduces proper parsing of
secret names for owned secrets (behind a feature flag).

BREAKING CHANGE: (feature flag) Secret.secretName for owned secrets will now
only the secret name (without suffix) and not the full resource name. This is
enabled through the `@aws-cdk/secretsmanager:parseOwnedSecretName` flag.

fixes #11727

----

*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.

@jdimeo
Copy link

jdimeo commented May 1, 2021

Hello @njlynch I think this is still an issue and we can't use ARN either.

In our case, we're using the path to the secret as the (plain) value of an SSM parameter. That way we can organize our SSM parameters in a hierarchical way (as controlled by a configuration class structure) and use indirection to do a second lookup when the value is a reference to a secret.

Given this secret:

ads_master_secret = aws_secretsmanager.Secret(
    self,
    id="AdsMasterSecret",
    generate_secret_string=aws_secretsmanager.SecretStringGenerator(
        secret_string_template=json.dumps({"username": ads_master_username}),
        generate_string_key="password",
        exclude_punctuation=True,
    ),
)

and this SSM parameter:

aws_ssm.StringParameter(
    self,
    id="AdsMasterCredentialsSSMParam",
    string_value="/aws/reference/secretsmanager/" + ads_master_secret.secret_name,
    parameter_name="/databases/main/password",
)

and this cdk.context.json:

  "@aws-cdk/secretsmanager:parseOwnedSecretName": "true"

We then load the config this way (at runtime):

log.debug("Looking for parameter {}", path);
val v = client.getParameter(req -> req.name(path).withDecryption(true)).parameter().value();
if (v.startsWith(SECRET_PATH)) { // /aws/reference/secretsmanager/
	// If the parameter refers to a secrets path, perform another lookup to get that value
	log.debug("Looking for secret {}...", v);
	val secret = client.getParameter(req -> req.name(v).withDecryption(true)).parameter().value();
	val creds = new ObjectMapper().readValue(secret, Credentials.class);
	map.put(path, creds.getPassword());
} else {
	map.put(path, v);
}

but are getting the error:

Looking for secret /aws/reference/secretsmanager/AdsMasterSecretC055F632-tdDfbCusyUCt-QyOlpI...
SSM parameter lookup failed for path /databases/main/password: An error occurred while calling one AWS dependency service. (Service: Ssm, Status Code: 400, Request ID: b34036a7-a652-4ca1-8f3c-9cfd8a815fad, Extended Request ID: null)

even though IAM stuff should be set up correctly... (and as you can see, it's using the full suffix)

Unless there's a better way of bridging the "flat" nature of the secrets manager with the "tree" of SSM parameters?

@cnechita-bwi
Copy link

Hello @njlynch
I have two stacks which create a secret, and then pass the secret.getSecretName as an environment variable to a Fargate task. From one of the stacks the proper secret name is passed, for the other stack the name + suffix is passed.
What is stranger is that when I deploy the stack from my local machine, .getSecretName return the proper secret name.

I have checked and the Feature toggle "@aws-cdk/aws-secretsmanager:parseOwnedSecretName": true is correct in both places.

I simply do not understand what is happening there.

Regards,
Cristian.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-secretsmanager Related to AWS Secrets Manager bug This issue is a bug. in-progress This issue is being actively worked on. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants