Skip to content

chore(cli): CLI uses cdk-assets to upload templates and assets#6742

Merged
mergify[bot] merged 2 commits intomasterfrom
huijbers/cdk-assets-take-2
Mar 16, 2020
Merged

chore(cli): CLI uses cdk-assets to upload templates and assets#6742
mergify[bot] merged 2 commits intomasterfrom
huijbers/cdk-assets-take-2

Conversation

@rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Mar 16, 2020

Centralize all logic about how templates, files and container assets
are built and uploaded in the cdk-assets tool.

We need this change first because it's on the critical path of the CLI
being able to deploy using the new convention mode roles (which requires
the CLI to use the asset publishing role for uploading the
CloudFormation templates), which is required for the pipeline
being able to do self-mutation using cdk deploy.

We can roll this change out independently of the framework emitting
the asset manifest (the CLI can generate it) and we don't need
to assume any roles, while we still get to test the new code path.

Also importing a number of improvements to cdk-assets from the
proof-of-concept branch.

Centralize all logic about how templates, files and container assets
are built and uploaded in the `cdk-assets` tool.

We need this change first because it's on the critical path of the CLI
being able to deploy using the new convention mode roles (which requires
the CLI to use the asset publishing role for uploading the
CloudFormation templates), which is required for the pipeline
being able to do self-mutation using `cdk deploy`.

We can roll this change out independently of the framework emitting
the asset manifest (the CLI can generate it) and we don't need
to assume any roles, while we still get to test the new code path.

Also importing a number of improvements to `cdk-assets` from the
proof-of-concept branch.

-----------

In addition to the previous attempt at this feature: handle it
properly when asset manifest contains absolute instead of relative
paths.
@rix0rrr rix0rrr requested a review from eladb March 16, 2020 12:41
@rix0rrr rix0rrr self-assigned this Mar 16, 2020
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Mar 16, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 3227973
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Mar 16, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 1579da3
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Mar 16, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit a75f711 into master Mar 16, 2020
@mergify mergify bot deleted the huijbers/cdk-assets-take-2 branch March 16, 2020 14:12
@danilofuchs
Copy link
Contributor

I had an issue with this PR. Before, I could do this:

        const dockerImage = new DockerImageAsset(this, "DockerImageAsset", {
            directory: "..",
            file: "server/Dockerfile"
        });

And the file prop was relative to the directory (../server/Dockerfile).
Now it is relative to something, although it is not clear if it is relative to the file which declares it or the stack itself. I'm trying to fix it by using file: "../server/Dockerfile".
This could mean a breaking change and needs to be documented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants