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

Decouple asset packaging and publishing #668

Closed
rix0rrr opened this issue Sep 5, 2018 · 4 comments
Closed

Decouple asset packaging and publishing #668

rix0rrr opened this issue Sep 5, 2018 · 4 comments
Assignees
Labels
@aws-cdk/assets Related to the @aws-cdk/assets package feature-request A feature should be added or improved.

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 5, 2018

This came up while looking at our current asset implementation.

Currently

  • assets.Asset is a concrete class that does a number of things, such as adding metadata and parameters to the template. The presence of these fields leads to the toolkit uploading data to S3.
  • lambda.AssetCode is a concrete class that instantiates an assets.Asset class with a path and packaging parameter.

First observation: there is no way to treat something as Lambda AssetCode UNLESS this involves packaging and uploading some files somewhere. In particular, some files that are published out-of-band could and should be treated like assets as well, but they don't involve the upload machinery of current assets, plus they are referenced differently. Because of the concrete class dependency, there is also no way to make a PipelineAsset, because MUST extend assets.Asset, and MUST call super() which will generate metadata that will make the toolkit try to do something. We don't want that.

Second observation: but is introducing a class that behaves differently actually desirable? We probably still want to say new ZipDirectoryAsset({ path: './my-files' }) regardless of the publishing method. In fact, that is desirable so that the same project can be deployed via a pipeline for production accounts as well as from the command-line for developer accounts.

So we want the assets.Asset class to say something about packaging, but not about publishing. In fact, we want to defer the choice of what publishing method to use to synthesis time.

This is all well and achievable, but the next problem is that we want to refer to the published artifacts as well; it's helpful if the assets.Asset class is aware of what to inject into the CloudFormation template to refer to the published artifact--unfortunately (but also obviously), this depends on the publishing method.

What happens

DEPLOYING USING COMMAND LINE

  • App requests directory to be zipped
  • App adds parameters to template, refers to these parameters when constructing assets reference
  • CDK deploy: synthesizes, zips directory, uploads to S3
  • Calls CloudFormation with parameter values obtained in previous step

CODEPIPELINE

  • App requests directory to be zipped
  • App uses magic intrinsics to refer to assets ({Fn::GetArtifactAtt})
  • Pipeline build: synthesizes, zips directory to special place, declare as output artifact
  • There has to be a CodeBuild step for every asset, and the only supported packaging method can be ZipDirectory! (since an Artifact is always a zip of a whole directory acted upon by CodeBuild)
  • Pipeline: Uploads to S3
  • Pipeline: Substitutes magic intrinsics in template with actual parameter value for the given artifact when calling CloudFormation

(In practice, we're probably going to use the toolkit to upload to S3 as well on CodeBuild because of the restrictions on artifacts--but we still want this to be extensible to support more types of publishing)

Asset references

COMMAND LINE

Add 2 parameters to template
bucket    = { "Ref": "BucketParameter"}
objectKey = { "Ref": "ObjectParameter" }.split('||')[0] + { "Ref": "ObjectParameter" }.split(||)[1]
prefix    = { "Ref": "ObjectParameter" }.split('||')[0]
objectUrl = *Some string substitution magic*

CODEPIPELINE

bucket          = { "Fn::GetArtifactAtt" : ["ArtifactName", "BucketName"]}
objectKey       = { "Fn::GetArtifactAtt" : ["ArtifactName", "ObjectKey"]}
objectUrl       = { "Fn::GetArtifactAtt" : ["ArtifactName", "URL"]}
prefix          = ""  // all files in bucket then? Don't know if URL structure will be guaranteed...

Tricky part here is that ArtifactName is decided in the pipeline, so we should pass it into the build step in some way so that it can be properly output in the synthesis step.

Proposal

To be most generic, we should probably split asset handling into 2 phases (just like we do with missing context).

In the first synthesis phase, the app reports the assets and their expected packaging methods.

The CDK then packages and publishes according to the selected publishing method (--publish=direct, --publish=codepipeline or similar). The publishing step may not do anything other than put files in a certain location.

In the second synthesis phase, synthesis is run again and the toolkit uses context to pass data on how to reference the assets to the Asset class, which uses the context to return the appropriate attributes.

If we remove the S3ObjectKeyParameter from the command-line assets implementation, and just pass in the literal hash of the ZIP file that was produced during packaging, we'll immediately have solved #395 as well.

If the publishing method starts with the magic string plugin-, as in:

cdk --publish=plugin-something synth

Then the NPM package aws-cdk-plugin-something is loaded and used for publishing.

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Sep 5, 2018

I suppose in order to support #659 maybe the app can request a particular type of publishing (on a per-asset basis) which overrides the default publishing method selected by running the CDK.

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Sep 17, 2018

@RomainMuller

@fulghum fulghum added the @aws-cdk/assets Related to the @aws-cdk/assets package label Sep 20, 2018
@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
@eladb eladb self-assigned this Aug 12, 2019
@asyschikov
Copy link

Are there any updates on this issue? The inability to control the packaging makes certain scenarios difficult, for example cross-account deployment with CloudFormation deployment target.

@eladb
Copy link
Contributor

eladb commented Jan 23, 2020

Superseded by #3437

@eladb eladb closed this as completed Jan 23, 2020
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 feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

5 participants