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

feat(pipelines): add support for CodePipeline namespaced variables #17189

Closed
wants to merge 4 commits into from
Closed

feat(pipelines): add support for CodePipeline namespaced variables #17189

wants to merge 4 commits into from

Conversation

igilham
Copy link
Contributor

@igilham igilham commented Oct 27, 2021

Firstly, I'm new to this project. I'm not used to the contributing guide and PR rules etc. and I don't expect this to work immediately. I also haven't set up a working build environment yet. This is all coming from a position of ignorance with the hope that it can become useful.

My team is attempting to build a pipeline that needs to pass a version number from one step to another using an exported environment variable. For this to work, the CodeBuildStep needs to support variablesNamespace similar to CodeBuildAction in @aws-cdk_aws-codepipeline-actions.

There are a couple of open issues talking about this and an existing PR which may be a little out of date now. I've borrowed from #15964 (by @berenddeboer) to implement the changes and addressed the request for a variable method in CodeBuildStep. I'm not certain if that existing PR is still being worked on.

Ideally, I'd like to get some feedback on how to proceed before investing much time in locking-in this approach by modifying tests and documentation. I've written a few questions, below, but feel free to raise any issues you can see.

To do:

  • Add variablesNamespace to CodeBuildStep
  • Add variable function to get a fully qualified CodePipeline variable name from CodeBuildStep instances
  • Add support for variables namespace to CodeBuild factory
  • Update existing affected tests, specifically the integration tests
  • Add tests around the variable method
  • Update relevant documentation

Open questions

  • Is it safe to always define variablesNamespace or do we need to maintain the current behaviour of keeping it undefined by default?
  • Is the approach taken to automatically define the namespace name sufficiently unique?
  • Do we need to allow the user to specify their own namespace?

Fixes #15943

Partially addresses #16407


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

- Add variables namespace to CodeBuildStep
- Add `variable` function to get a fully qualified CodePipeline variable name from CodeBuildStep instances
- Add support for variables namespace to CodeBuild factory

Refer to #15964 for a previous attempt by @berenddeboer, from which I've copied some implementation details.

Fixes #15943

Partially addresses #16407
@gitpod-io
Copy link

gitpod-io bot commented Oct 27, 2021

@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Oct 27, 2021
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 03e9861
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@igilham
Copy link
Contributor Author

igilham commented Nov 9, 2021

Looks like there's either no interest in addressing this issue or no time to consider it at present.

My team has abandoned CDK Pipelines due to it missing features we need.

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 24, 2021

Apparently PR is abandoned.

@rix0rrr rix0rrr closed this Nov 24, 2021
@markusl
Copy link
Contributor

markusl commented Nov 24, 2021

@rix0rrr would you be able to merge this PR as it is highly relevant for other customers?

To me, it sounds weird that there is a PR specifically requested by the AWS CDK team, and then nothing is done for the PR for a month, and after that, it is labeled as "abandoned".

Please correct me if there is a misunderstanding related to this issue.

Best Regards,
Markus

@igilham
Copy link
Contributor Author

igilham commented Nov 24, 2021

I am not the one who abandoned it. The CDK team has so far ignored it and the request for feedback.

Several implementations of this feature are possible and I wanted to get some consensus on the approach before spending a lot of time polishing it. Since there has been no response of any kind, it seems I was wise not to waste too much time on it.

@igilham
Copy link
Contributor Author

igilham commented Feb 16, 2022

Is there an interest in picking this back up? The open questions and request for feedback are still unanswered at present.

rix0rrr added a commit that referenced this pull request Feb 17, 2022
feat(pipelines): step outputs

Make it possible to export environment variables from a CodeBuildStep,
and pipeline sources, and use them in the environment variables of
a CodeBuildStep or ShellStep.

Closes #17189, closes #18893, closes #15943, closes #16407.
@mergify mergify bot closed this in #19024 Feb 23, 2022
mergify bot pushed a commit that referenced this pull request Feb 23, 2022
Make it possible to export environment variables from a CodeBuildStep,
and pipeline sources, and use them in the environment variables of
a CodeBuildStep or ShellStep.

Closes #17189, closes #18893, closes #15943, closes #16407.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@igilham igilham deleted the ig-pipeline-export-vars branch February 24, 2022 11:55
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(pipelines): Specifying variable namespace for pipelines.CodeBuildStep
4 participants