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): assets with deploy-time values #12903

Closed
1 of 2 tasks
johnameyer opened this issue Feb 6, 2021 · 15 comments · Fixed by #18659
Closed
1 of 2 tasks

(aws-s3-deployment): assets with deploy-time values #12903

johnameyer opened this issue Feb 6, 2021 · 15 comments · Fixed by #18659
Assignees
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package @aws-cdk/aws-s3-deployment effort/medium Medium work item – several days of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. p2

Comments

@johnameyer
Copy link

johnameyer commented Feb 6, 2021

Allow for deploying of an inline file to an s3 bucket, like as can be done for Lambda functions.

Use Case

This is one way to allow for resources generated at synth time to be aware of resources that are created in a stack at deploy time, e.g. allowing a static frontend (with a config file that is deployed by this mechanism) to know the URIs of backend resources like a Cognito auth endpoint or an API Gateway. Since normally static resources would be built at the asset generation phase, it would normally be impossible for them to obtain these attributes of resources generated later in the lifecycle. Instead, you can build your code with the assumption of a config file being generated at deploy time and deployed to the bucket and postpone actually needing those URIs.

This is particularly helpful in CDK Pipelines, where it is desired to have all the assets generated up-front and then configure a stage at deploy time, to prevent extensive customization of the Pipeline.

Proposed Solution

From the description of how the s3-deployments module works under the hood, it's not exactly clear at what point the files are uploaded to the intermediary asset bucket, but it seems at least naively a custom resource could create a resource in a bucket from a parameter (resolved by cloudformation) from which a SourceConfig could be generated.

The workaround currently is to create a custom resource which deploys this file to the bucket, which seems to be how s3-deployments already work, but does not have the in-built feature of invalidating Cloudfront.

Other

S3 Deployments module: https://docs.aws.amazon.com/cdk/api/latest/docs/aws-s3-deployment-readme.html
Lambda InlineCode: https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-lambda.InlineCode.html
StackOverflow question: https://stackoverflow.com/questions/60074546/aws-cdk-passing-api-gateway-url-to-static-site-in-same-stack/65799623#65799623
Example usage:

const sources = [
  s3deploy.Source.asset('./website-dist'),
  s3deploy.Source.inline('config.js', `exports.config = { cognitoUserPool: '${ cognitoUserPool.userPoolClientId }', apiURI: '${ api.apiId }'`)
];
new s3deploy.BucketDeployment(this, 'DeployWebsite', {
  sources,
  destinationBucket: websiteBucket,
  destinationKeyPrefix: 'web/static'
}); // my file should exist at web/static/config.js and have the tokens resolved
  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@johnameyer johnameyer added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Feb 6, 2021
@johnameyer johnameyer changed the title aws-s3-deployment: inline asset deployment (aws-s3-deployment): inline asset deployment Feb 6, 2021
@johnameyer
Copy link
Author

Implemented current workaround in https://stackoverflow.com/a/66084764/2088936 as proof of feasibility/starting point for discussion.

@jogold
Copy link
Contributor

jogold commented Feb 7, 2021

I'm offering a solution for this here https:/jogold/cloudstructs/tree/master/src/static-website (static website with backend config saved to the bucket).

@iliapolo
Copy link
Contributor

@johnameyer I'm not opposed to having this functionality, seems useful. So the current proposal is to:

  1. Custom resource accept a string that is resolved at deploy time.
  2. Custom resource lambda zips up the resolved string and uploads to an intermediary bucket.
  3. That bucket is used to bind SourceConfig

Right?

P.S Is what @jogold referenced fully addresses your use-case?

@iliapolo iliapolo added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. needs-discussion This issue/PR requires more discussion with community. labels Feb 10, 2021
@johnameyer
Copy link
Author

That's the current workaround and I think it is the least disagreeable. However, it is not a particularly efficient solution since it's adding a custom resource and a bucket that just feeds into another custom resource and bucket under the hood. I think there a few ways to make it less heavy-handed.
One way (call it proposal B) is to allow passing a non-zip file to the ISource (since we just go and unzip on the other side), as it would make the inline source custom resource no longer need to zip up the file (no extra library needed, and now just a simple upload so it can just be an AwsCustomResource that puts to s3).
Potentially (proposal C), we could also add inline as an option onto ISource like Lambda does and then have the s3-deployment custom resource handle everything (which would require a little more work), but this then saves us making an intermediate bucket and another custom handler altogether.
There's potentially a few other options as well but curious as to what you think of those two.

@jogold's solution is pretty good, but it doesn't directly integrate with s3-deployments (and as such things like the cloudfront invalidation that I need).

@iliapolo
Copy link
Contributor

@johnameyer Yeap, I think option 3 is the best, just to make sure we aligned, basically adding a new property in SourceConfig:

export interface SourceConfig {
/**
* The source bucket to deploy from.
*/
readonly bucket: s3.IBucket;
/**
* An S3 object key in the source bucket that points to a zip file.
*/
readonly zipObjectKey: string;
}

And take it from there. Yes?

@johnameyer
Copy link
Author

Oh yep yep SourceConfig is the one.

@iliapolo
Copy link
Contributor

@johnameyer Sounds great. feel free to pick this up whenever you get a chance :)

@iliapolo iliapolo added effort/medium Medium work item – several days of effort p2 and removed needs-discussion This issue/PR requires more discussion with community. needs-triage This issue or PR still needs to be triaged. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Feb 10, 2021
@ericzbeard ericzbeard added the feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. label Apr 2, 2021
@iliapolo iliapolo removed their assignment Jun 27, 2021
@eladb eladb changed the title (aws-s3-deployment): inline asset deployment (aws-s3-deployment): assets with deploy-time values Sep 14, 2021
@github-actions github-actions bot added the @aws-cdk/assets Related to the @aws-cdk/assets package label Sep 14, 2021
@otaviomacedo otaviomacedo removed their assignment Sep 14, 2021
@fjmacagno
Copy link

I found a simple way to do this, but it doesn't use aws-s3-deployment:

// Returns a construct which deploys a string to an s3 object. The contents can include references to other resources.
export function resourceStringToS3(scope: Construct, id: string, contents: string, bucket: Bucket, key: string): Construct {
  return new custom_resources.AwsCustomResource(
    scope,
    id,
    {
      onUpdate: {
        service: "S3",
        action: "putObject",
        parameters: {
          Body: contents,
          Bucket: bucket.bucketName,
          Key: key,
        },
        physicalResourceId: PhysicalResourceId.of(key),
      },
      onDelete: {
        service: "S3",
        action: "deleteObject",
        parameters: {
          Bucket: bucket.bucketName,
          Key: key,
        },
      },
      policy: AwsCustomResourcePolicy.fromSdkCalls({
        resources: [`${bucket.bucketArn}/${key}`],
      }),
    }
  );
}

@Dreamescaper
Copy link

@fjmacagno
Unfortunately, this approach works for smaller files only.
I've tried to use it, but I have payload limit error:

byte payload is too large for the Event invocation type (limit 262144 bytes)

@fjmacagno
Copy link

Unfortunate, though not susprising

@edcarter
Copy link

edcarter commented Nov 25, 2021

I believe the root of the problem is deeper than what has been discussed. In the CDK it is assumed that "Assets" are dependencies for the constructs in the stack (for example a ZIP asset containing lambda handler code is a dependency of the lambda construct). Since "Assets" are assumed to be dependencies of the constructs they are built and deployed before the rest of the stack.

This design breaks down when the dependencies are flipped: constructs in the stack are dependencies of the assets. For example, an "Asset" containing the files for a static s3 website depend on the stack constructs since the API gateway/cloudfront distrubution needs to be baked into the frontend "Asset".

To "properly" solve this issue I think that "Assets" should have an optional argument to determine when they are deployed. If the assets are dependencies for the constructs they should be deployed before the constructs -- as they are currently. If the assets depend on the constructs they should be deployed at the same time as the constructs so any dynamic values can be injected into the assets.

I realize this is a very large change, but I think this is the "correct" solution.

@eladb
Copy link
Contributor

eladb commented Jan 25, 2022

This is a fun challenge, and can be a major enabler, so definitely worth looking into if anyone is interested to pick this up.

Here's a proposed approach: materialize "deploy-time values" into the asset file during synthesis (e.g. replace tokens with a set of markers) and then substitute these markers with their deploy-time value inside the s3-deployment custom resource when the file is copied from the assets bucket to its final destination.

I think it will be simpler to support this for JSON first (due to how token resolution works), but we can add text files in later:

Sketch:

const files = Source.json({
  'config.json': {
    Foo: {
      Bar: sns.topicArn // <-- { Ref }
    }
  },
});

new BucketDeployment(this, 'BucketDeployment', {
  sources: [files]
});

The Source.inline() factory will synthesize will resolve the tokens inside the values, and replace them with unique markers, so the asset that will get uploaded by the CLI to the assets bucket will look something like this:

config.json:

{
  "Foo": {
    "Bar": "{{MARKER_11122128378}}"
  }
}

The BucketDeployment custom resource will be passed additional properties that will assign markers to values. Then during deployment, the custom resource will simply replace the markers with their actual values.

eladb pushed a commit that referenced this issue Jan 26, 2022
Allow deploying test-based content that can potentially include deploy-time values such as attributes of cloud resources.

Introduce a `Source.content(objectKey, text)` where `text` can naturally include tokens that resolve only at deploy time.
This is implemented by replacing the deploy-time tokens with markers that are replaced inside the s3-deployment custome resource.

Fixes #12903

TODO:

- [ ] Implement + test the custom resource code
- [ ] Documentation
- [ ] Finalize integration test
@eladb
Copy link
Contributor

eladb commented Jan 26, 2022

Started to work on this here in case anyone is interested to track/comment: #18659

@mergify mergify bot closed this as completed in #18659 Feb 3, 2022
mergify bot pushed a commit that referenced this issue Feb 3, 2022
Allow deploying test-based content that can potentially include deploy-time values such as attributes of cloud resources.

Introduce a `Source.data(objectKey, text)` and `Source.jsonData(objectKey, obj)` where the data can naturally include deploy-time tokens such as references to resources (`Ref`) or to resource attributes (`Fn::GetAtt`).

For example:

```ts
const appConfig = {
  topic_arn: topic.topicArn,
  base_url: 'https://my-endpoint',
};

new s3deploy.BucketDeployment(this, 'BucketDeployment', {
  sources: [s3deploy.Source.jsonData('config.json', config)],
  destinationBucket: destinationBucket,
});
```

This is implemented by replacing the deploy-time tokens with markers that are replaced inside the s3-deployment custom resource.

Fixes #12903

----

*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 Feb 3, 2022

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

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
Allow deploying test-based content that can potentially include deploy-time values such as attributes of cloud resources.

Introduce a `Source.data(objectKey, text)` and `Source.jsonData(objectKey, obj)` where the data can naturally include deploy-time tokens such as references to resources (`Ref`) or to resource attributes (`Fn::GetAtt`).

For example:

```ts
const appConfig = {
  topic_arn: topic.topicArn,
  base_url: 'https://my-endpoint',
};

new s3deploy.BucketDeployment(this, 'BucketDeployment', {
  sources: [s3deploy.Source.jsonData('config.json', config)],
  destinationBucket: destinationBucket,
});
```

This is implemented by replacing the deploy-time tokens with markers that are replaced inside the s3-deployment custom resource.

Fixes aws#12903

----

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

@eladb I am seeing a problem when values are passed cross-stack... it doesn't seem to generate the proper cfn-imports/exports and so throws an error because the generated code expects the resource in the same stack

Unresolved resource dependencies [XYZ] in the Resources block of the template

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package @aws-cdk/aws-s3-deployment effort/medium Medium work item – several days of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants