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-s3-deployment: Source.jsonData fails for certain types of deploy-time values #25504

Closed
ktmayes-amzn opened this issue May 9, 2023 · 7 comments · Fixed by #27237
Closed
Labels
@aws-cdk/aws-s3-deployment effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@ktmayes-amzn
Copy link

Describe the bug

Some types of deploy-time values cause Source.jsonData (and presumably the other deploy-time static methods for Source) to fail since these deploy-time values contain CloudFormation functions other than Ref or Fn::Get.

For example, the secretName field on a Secret becomes an Fn::Join function call which gets rejected by Source.jsonData (see Reproduction Steps for example code).

Expected Behavior

For the rendered JSON file containing the secret ID to be deployed to the S3 bucket.

Current Behavior

For the code for reproducing the issue, I get this error:

Error: Invalid CloudFormation reference. "Ref" or "Fn::GetAtt". Got {"Fn::Join":["-",[{"Fn::Select":[0,{"Fn::Split":["-",{"Fn::Select":[6,{"Fn::Split":[":",{"Ref":"testsecretF8BBC644"}]}]}]}]},{"Fn::Select":[1,{"Fn::Split":["-",{"Fn::Select":[6,{"Fn::Split":[":",{"Ref":"testsecretF8BBC644"}]}]}]}]}]]}

Reproduction Steps

const bucket = new Bucket(this, 'test-bucket');
const secret = new Secret(this, 'test-secret');
new BucketDeployment(this, 'test-bucket-deployment', {
  destinationBucket: bucket,
  sources: [Source.jsonData('key.json', { secretId: secret.secretName })],
});

Possible Solution

No response

Additional Information/Context

Seems to be rejected by this code:

function addMarker(part: Ref | GetAtt) {
const keys = Object.keys(part);
if (keys.length !== 1 || (keys[0] != 'Ref' && keys[0] != 'Fn::GetAtt')) {
throw new Error(`Invalid CloudFormation reference. "Ref" or "Fn::GetAtt". Got ${JSON.stringify(part)}`);
}
const marker = `<<marker:0xbaba:${markerIndex++}>>`;
result.push(marker);
markers[marker] = part;
}
return { text: result.join(''), markers };
}

CDK CLI Version

2.75.0 (build 37c53d6)

Framework Version

No response

Node.js Version

18

OS

macOS

Language

Typescript

Language Version

TypeScript (4.9.5)

Other information

No response

@ktmayes-amzn ktmayes-amzn added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 9, 2023
@pahud
Copy link
Contributor

pahud commented May 10, 2023

Thanks for your report. Can you share your content of key.json so I can rapidly reproduce it in my account?

@pahud pahud added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels May 10, 2023
@pahud
Copy link
Contributor

pahud commented May 10, 2023

And you are right. The addMarker() seems only support Fn::GetAtt and Fn::GetAtt while you were passing Fn::Join and Fn::Select which are not supported yet.

@pahud pahud added feature-request A feature should be added or improved. and removed bug This issue is a bug. labels May 10, 2023
@ktmayes-amzn
Copy link
Author

ktmayes-amzn commented May 10, 2023

Thank you for the quick response!

I'm not sure what you mean about the contents of key.json, I think my code sample should be self-contained in that it triggers the bug when synthesizing a template for a stack containing that code.

If you want what { secretId: secret.secretName } from the code sample becomes in CloudFormation-speak, it is this:

Example CloudFormation JSON rejected by `Source.jsonData`
{
  "Fn::Join": [
    "",
    [
      "{\"secretId\":\"",
      {
        "Fn::Join": [
          "-",
          [
            {
              "Fn::Select": [
                0,
                {
                  "Fn::Split": [
                    "-",
                    {
                      "Fn::Select": [
                        6,
                        {
                          "Fn::Split": [
                            ":",
                            {
                              "Ref": "testsecretF8BBC644"
                            }
                          ]
                        }
                      ]
                    }
                  ]
                }
              ]
            },
            {
              "Fn::Select": [
                1,
                {
                  "Fn::Split": [
                    "-",
                    {
                      "Fn::Select": [
                        6,
                        {
                          "Fn::Split": [
                            ":",
                            {
                              "Ref": "testsecretF8BBC644"
                            }
                          ]
                        }
                      ]
                    }
                  ]
                }
              ]
            }
          ]
        ]
      },
      "\"}"
    ]
  ]
}

I added console.log(JSON.stringify(obj, null, " ")); to the renderData function in the aws-cdk-lib/aws-s3-deployment/lib/render-data.js file in my node_modules.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label May 10, 2023
@Justinon
Copy link
Contributor

Also seeing this issue:

Error: Invalid CloudFormation reference. "Ref" or "Fn::GetAtt". Got {"Fn::Select":[1,{"Fn::Split":["/",{"Ref":"SsmParameterValue<REDACTED>Parameter"}]}]}

@Justinon
Copy link
Contributor

Justinon commented Sep 20, 2023

Also seeing this issue:

Error: Invalid CloudFormation reference. "Ref" or "Fn::GetAtt". Got {"Fn::Select":[1,{"Fn::Split":["/",{"Ref":"SsmParameterValue<REDACTED>Parameter"}]}]}

This comes from the following:

let tg: ApplicationTargetGroup // Assume this is defined
const str = tg.firstLoadBalancerFullName + '/' + tg.targetGroupFullName

Followed by then utilizing that str as a value in a JSON blob being passed as obj for Source.jsonData.

@Justinon
Copy link
Contributor

Played around w/ my local copy of node_modules/aws-cdk-lib/aws-s3-deployment/lib/render-data.js and removed the conditional in addMarker from this:

if (keys.length !== 1 || (keys[0] != 'Ref' && keys[0] != 'Fn::GetAtt')) {

to this:

if (keys.length !== 1) {

and...it worked beautifully. The rendered value in the S3 object itself is 100% correct. Why have this condition at all? Perhaps either removing it, or adding && keys[0] != 'Fn::Select', && keys[0] != 'Fn::Join', etc. will solve this.

@mergify mergify bot closed this as completed in #27237 Oct 4, 2023
mergify bot pushed a commit that referenced this issue Oct 4, 2023
Closes #25504 

The reason for this change is to support more complex Cloudformation references used within `Source.data` in `aws-s3-deployment`. The objects today only support `Ref` or `Fn::GetAtt` Cfn references, which is limiting when it comes to attempting to manipulate Cfn references at deploy-time, such as via `Fn::Split` or `Fn::Select`. Many AWS CDK functions return tokens that must be evaluated using these complex Cfn functions (see [ApplicationTargetGroup's firstLoadBalancerFullName attribute](https://github.com/aws/aws-cdk/blob/3edd2400bc0c8a86366a29d3a7eef1ef4fa5e016/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-target-group.ts#L438)), but they are incompatible with `renderData`!

This is a blocking issue for CDK projects which rely on generating S3 objects using `BucketDeployment`, wherein the rendered data is generated from native functions which utilize Cfn functions under-the-hood to dynamically construct values at deploy-time.

----

*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

github-actions bot commented Oct 4, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3-deployment effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants