-
Notifications
You must be signed in to change notification settings - Fork 4k
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-codepipeline-actions: CodeStarConnectionsSourceAction.variables inaccurate #31000
Labels
@aws-cdk/aws-codepipeline-actions
bug
This issue is a bug.
effort/medium
Medium work item – several days of effort
p2
Comments
dleavitt
added
bug
This issue is a bug.
needs-triage
This issue or PR still needs to be triaged.
labels
Aug 1, 2024
ashishdhingra
added
p2
needs-reproduction
This issue needs reproduction.
and removed
needs-triage
This issue or PR still needs to be triaged.
labels
Aug 1, 2024
ashishdhingra
added
effort/medium
Medium work item – several days of effort
and removed
needs-reproduction
This issue needs reproduction.
labels
Aug 1, 2024
This was referenced Aug 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
@aws-cdk/aws-codepipeline-actions
bug
This issue is a bug.
effort/medium
Medium work item – several days of effort
p2
Describe the bug
If you try to use
CodeStarConnectionsSourceAction.variables.branchName
on an execution triggered by a pull request, your build will fail with:Detail
#5604 added a
.variables
getter to a number ofcodepipeline.Action
subclasses includingCodeStarConnectionsSourceAction
. It's hardcoded to return a particular set of variables, and seems to be the only public way to get at the action's variables.aws-cdk/packages/aws-cdk-lib/aws-codepipeline-actions/lib/codestar-connections/source-action.ts
Lines 119 to 128 in 9295a85
These variables are correct in some cases, but aren't correct if the execution was triggered by a pull request (see screenshots below.)
With a
pullRequestFilter
in place,BranchName
will be unavailable, but four additional variables will be set:DestinationBranchName
,PullRequestId
,PullRequestTitle
,SourceBranchName
. There's no good way to get at these right now.Expected Behavior
One of the below:
a.
variables
should return the variables that the source action actually exports (likely impossible to implement at CDK level.)b.
variables
should return only variables that are always safe to use. I'm not sure if there's a public spec indicating what variables are returned under what circumstances. ButBranchName
is not always defined by the source action and therefore isn't safe to us.More importantly, since the CDK doesn't/can't know what the possible variables are, I'd expect the underlying
variableExpression(variableName: string)
should be publicly accessible, like it is on some of the other actions:aws-cdk/packages/aws-cdk-lib/aws-codepipeline-actions/lib/codebuild/build-action.ts
Lines 145 to 147 in 9295a85
aws-cdk/packages/aws-cdk-lib/aws-codepipeline/lib/action.ts
Lines 428 to 431 in 9295a85
Current Behavior
The cdk-provided
variables
getter misses some variables and returns others that may not exist. It doesn't provide any way to get at the missing ones.For missed variables, a workaround is to call the protected
variableExpression
method viaaction["variableExpression"]("DestinationBranchName")
or similar.For returned variables that don't exist (
BranchName
), workaround is not to use it.Reproduction Steps
Below is a construct that will reproduce the issue. Unfortunately, due to the way CodePipeline works, the minimal reproduction is not especially concise.
To use it, you'll need a CDK stack with a VPC and a CodeConnection to a git repo (I used a Github repo.)
Once you've got the stack deployed, open a pull request in the repo. It should trigger an execution. The
UsesBranchName
build action will fail, because the source action doesn't export theBranchName
variable.Now try manually triggering a execution with the "Release Change" button in the console. Here, the
UsesBranchName
build action will succeed, but theUsesDestinationBranchName
will fail, because theDestinationBranchName
won't have been defined (this will also happen when the pipeline is first created.)Reproduction: https://gist.github.com/dleavitt/7950f5073bb0ebe2f3fa5049a2f44ab8
Possible Solution
variable(variableName: string): string
method toCodeStarConnectionsSourceAction
, with the same implementation like this:aws-cdk/packages/aws-cdk-lib/aws-codepipeline-actions/lib/codebuild/build-action.ts
Lines 145 to 147 in 9295a85
Maybe add it directly to
Action
(or makevariableExpression
public) if there are other actions where the list of variables could be dynamic.BranchName
fromCodeStarConnectionsSourceAction.variables()
, since it's not always present and if missing attempting to use it will cause the build to fail.Additional Information/Context
From what I can tell, there's an underlying issue with the implementation of the CodeStarSourceConnection Action provider in CodePipeline. For a given pipeline and action:
Therefore the only variables that can be safely used are ones available in all cases (which
BranchName
is not.)I would love to be wrong about this, let me know if there's a workaround!
Variables from a non-pr trigger
Variables from a pull request trigger
CDK CLI Version
2.150.0 (build 3f93027)
Framework Version
No response
Node.js Version
v20.11.1
OS
MacOS 14.3 (23D56)
Language
TypeScript
Language Version
Typescript (5.4.5)
Other information
No response
The text was updated successfully, but these errors were encountered: