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

(pipelines): --output Argument leaks into SimpleSynthAction when deploying the Pipeline #13303

Closed
aax opened this issue Feb 26, 2021 · 3 comments · Fixed by #14211
Closed

(pipelines): --output Argument leaks into SimpleSynthAction when deploying the Pipeline #13303

aax opened this issue Feb 26, 2021 · 3 comments · Fixed by #14211
Assignees
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md needs-triage This issue or PR still needs to be triaged. p1

Comments

@aax
Copy link

aax commented Feb 26, 2021

I ran into this issue when synthesizing an app containing a CdkPipeline with a SimpleSynthAction. For my needs, I have to specify a custom --output folder when deploying the app. But this output folder path somehow leaks into the CdkPipeline itself. The SimpleSynthAction expects the cloud assembly artifact to be at the same location within the CodeBuild project.

Reproduction Steps

Define a CdkPipeline with a SimpleSynthAction:

cloud_assembly_artifact = codepipeline.Artifact()

CdkPipeline(stack, "Pipeline",
    cloud_assembly_artifact=cloud_assembly_artifact,
    source_action=...,
    synth_action=SimpleSynthAction(
        source_artifact=...,
        cloud_assembly_artifact=cloud_assembly_artifact,
        install_command='npm install -g aws-cdk@1.91.0',
        synth_command='cdk synth'
    )
)

Deploy the app with a custom output argument:

cdk deploy --output /tmp/cdk

What did you expect to happen?

I would expect the pipeline to be deployed and executed successfully.

What actually happened?

The synth action in the pipeline fails. The build spec generated for the synth action looks as follows. Note the primary artifact's base-directory is /tmp/cdk, but the synth command writes its output to cdk.out by default. Therefore the action has an empty output artifact and the build fails.

{
  "version": "0.2",
  "phases": {
    "pre_build": {
      "commands": [
        "npm install -g aws-cdk@1.91.0"
      ]
    },
    "build": {
      "commands": [
        "pipenv run cdk synth"
      ]
    }
  },
  "artifacts": {
    "base-directory": "/tmp/cdk",
    "files": "**/*"
  }
}

Environment

  • CDK CLI Version : 1.91.0
  • Framework Version: 1.91.0
  • Node.js Version: v15.5.0
  • OS : Windows
  • Language (Version): Python 3.7.9

Other

I can't see how this behaviour is intended. But if it is, I would need to change it for my case. Can I somehow specify the location of the cloud assembly artifact within the SimpleSynthAction?


This is 🐛 Bug Report

@aax aax added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 26, 2021
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Feb 26, 2021
@aax
Copy link
Author

aax commented Mar 1, 2021

I was able to find a workaround:

SimpleSynthAction(
    source_artifact=...,
    cloud_assembly_artifact=cloud_assembly_artifact,
    install_command='npm install -g aws-cdk@1.91.0',
    synth_command=f'cdk synth --output {scope.node.root.outdir}'
)

This sets the output directory of the synth call within CodeBuild to the same path that is assumed in for the build artifact.

I'm lucky this works for me. But if you need to use an exotic output directory when you deploy your CDK Pipeline or if you are on a different system than within your CodeBuild project (Windows vs Linux) it might not be possible to use the same output dir like this. Hence, I still consider this behaviour as undesirable.

@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 3, 2021

The relative paths we use in the ACTUAL synth dir are still correct, but the base output directory should ALWAYS be cdk.out, and not depend on deploy settings.

@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1 labels Mar 3, 2021
@rix0rrr rix0rrr added this to the [GA] CDK Pipelines milestone Mar 3, 2021
@mergify mergify bot closed this as completed in #14211 Apr 19, 2021
mergify bot pushed a commit that referenced this issue Apr 19, 2021
… `--output` (#14211)

Previously, we were taking the directory passed by the user to the CLI in the `--output` option and writing it to the BuildSpec. Since the pipeline will always build with the default output, we should also keep the default output directory in the BuildSpec. 

Fixes #13303


----

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

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
… `--output` (aws#14211)

Previously, we were taking the directory passed by the user to the CLI in the `--output` option and writing it to the BuildSpec. Since the pipeline will always build with the default output, we should also keep the default output directory in the BuildSpec. 

Fixes aws#13303


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md needs-triage This issue or PR still needs to be triaged. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants