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): using the same repo more than once as a connection in a pipeline causes duplicate id error #23916

Closed
andreprawira opened this issue Jan 30, 2023 · 7 comments · Fixed by #27602
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 p1

Comments

@andreprawira
Copy link

andreprawira commented Jan 30, 2023

Describe the bug

I'm trying to check out two different branches of the same repo, but are getting the "Node with duplicate id" error message. Below is the code that I have

Expected Behavior

I'm expecting CDK will checkout the code from the same repo except it will use different branch and synth the code as usual

Current Behavior

jsii.errors.JavaScriptError: 
  @jsii/kernel.RuntimeError: Error: Node with duplicate id: questek/icmd-platform-test
      at Kernel._ensureSync (C:\Users\andre\AppData\Local\Temp\tmpt9ddaen9\lib\program.js:8428:27)
      at Kernel.invoke (C:\Users\andre\AppData\Local\Temp\tmpt9ddaen9\lib\program.js:7840:34)
      at KernelHost.processRequest (C:\Users\andre\AppData\Local\Temp\tmpt9ddaen9\lib\program.js:11017:36)
      at KernelHost.run (C:\Users\andre\AppData\Local\Temp\tmpt9ddaen9\lib\program.js:10977:22)
      at Immediate._onImmediate (C:\Users\andre\AppData\Local\Temp\tmpt9ddaen9\lib\program.js:10978:46)
      at processImmediate (node:internal/timers:466:21)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "D:\Work\AllCloud\QuesTek\questek-saas-cdk\app.py", line 26, in <module>
    app.synth()
  File "D:\Work\AllCloud\QuesTek\questek-saas-cdk\.venv\lib\site-packages\aws_cdk\__init__.py", line 20667, in synth
    return typing.cast(_CloudAssembly_c693643e, jsii.invoke(self, "synth", [options]))
  File "D:\Work\AllCloud\QuesTek\questek-saas-cdk\.venv\lib\site-packages\jsii\_kernel\__init__.py", line 148, in wrapped
    return _recursize_dereference(kernel, fn(kernel, *args, **kwargs))
  File "D:\Work\AllCloud\QuesTek\questek-saas-cdk\.venv\lib\site-packages\jsii\_kernel\__init__.py", line 386, in invoke
    response = self.provider.invoke(
  File "D:\Work\AllCloud\QuesTek\questek-saas-cdk\.venv\lib\site-packages\jsii\_kernel\providers\process.py", line 365, in invoke
    return self._process.send(request, InvokeResponse)
  File "D:\Work\AllCloud\QuesTek\questek-saas-cdk\.venv\lib\site-packages\jsii\_kernel\providers\process.py", line 331, in send
    raise RuntimeError(resp.error) from JavaScriptError(resp.stack)
RuntimeError: Error: Node with duplicate id: my-organization/my-branch

Reproduction Steps

additional_inputs={
                        "source-1": pipelines.CodePipelineSource.connection(
                            repo_string="my-organization/my-repo1",
                            branch=f"pipeline/{version}/frontend",
                            connection_arn=infra.code_star_connection,
                            code_build_clone_output=True,
                            trigger_on_push=True,
                        ),
                        "source-2": pipelines.CodePipelineSource.connection(
                            repo_string="my-organization/my-repo1", # this is supposed to be organization-name/repo-name but CDK is reading it as Node ID thus it needs to be different than above 
                            branch=f"pipeline/{version}/backend",
                            connection_arn=infra.code_star_connection,
                            code_build_clone_output=True,
                            trigger_on_push=True,
                        ),
                        "source-3": pipelines.CodePipelineSource.connection(
                            repo_string="my-organization/my-repo3",
                            branch="main",
                            connection_arn=infra.code_star_connection,
                            code_build_clone_output=True,
                            trigger_on_push=False,
                        ),

then run cdk deploy and you will then see the error

Possible Solution

No response

Additional Information/Context

If you try to change the source-2 repo name from my-repo1 to my-repo2 for example it would work. I think there is a bug here where CDK is reading that line as Node ID instead of repo name

CDK CLI Version

2.51.0 (build a87259f)

Framework Version

No response

Node.js Version

16.18.0

OS

Windows

Language

Python

Language Version

No response

Other information

No response

@andreprawira andreprawira added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 30, 2023
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Jan 30, 2023
@jbcursol
Copy link

jbcursol commented Feb 2, 2023

I believe I'm having the same issue when trying to do exactly what OP is. Except my issue comes from executing cdk synth with a .ts pipeline definition.

synth: new ShellStep('Synth', {
                input: CodePipelineSource.codeCommit(repos, 'main'),
                commands: [
                    'npm ci',
                    'npm run build',
                    'npx cdk synth main-TestFunctions-PipelineStack'
                ],
                additionalInputs: {
                    "../release": CodePipelineSource.codeCommit(repos, 'release')
                }
            }),

@peterwoodworth
Copy link
Contributor

I've looked into this a bit, and it makes sense that this is happening. Thanks for reporting this!

CodePipelineSource.connection() will end up creating a new CodeStarConnectionSource using the data you've passed in. It will call the super class Step with the repo string.

constructor(repoString: string, readonly branch: string, readonly props: ConnectionSourceOptions) {
super(repoString);

Step uses whatever was passed in as the identifier, so the id will be identical if the same repository was used twice despite the branch being different

constructor(
/** Identifier for this step */
public readonly id: string) {

We might want to take the branch name into account here (might be a breaking change), or, we might want to find some way to identify this scenario and adjust the id if so.

@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Feb 7, 2023
@peterwoodworth peterwoodworth changed the title aws_cdk.pipelines: Node with duplicate id: aws_cdk.pipelines: Using the same repo at least twice as a connection in a pipeline causes duplicate id error Feb 7, 2023
@andreprawira
Copy link
Author

andreprawira commented Feb 7, 2023

@peterwoodworth thanks for replying, i appreciate it, i'd suggest to have the branch name not to be used as the logical name, cause i think that is what's happening here (and please correct me if im wrong, i just started CDK not long ago), here is my suggestion

"source-1": pipelines.CodePipelineSource.connection(self, "logical-name-for-source-1",
                            repo_string="my-organization/my-repo1",
                            branch=f"pipeline/{version}/frontend",
                            connection_arn=infra.code_star_connection,
                            code_build_clone_output=True,
                            trigger_on_push=True,
                        ),
"source-2": pipelines.CodePipelineSource.connection(self, "logical-name-for-source-2",
                            repo_string="my-organization/my-repo1",
                            branch=f"pipeline/{version}/backend",
                            connection_arn=infra.code_star_connection,
                            code_build_clone_output=True,
                            trigger_on_push=True,
                        ),

That way it would allow checking out the same repo with multiple branches, also out of curiosity are you working for AWS?

@npvisual
Copy link

npvisual commented Jul 27, 2023

I believe there's an additional parameter, action_name, which has the following description :

The action name used for this source in the CodePipeline. Default: - The repository string

which should probably be used for the id if provided in the constructor. Unfortunately only the repo string is passed.

There was a similar comment provided here :
#24767 (review)

This is problematic, as several folks have mentioned, because you can't create a CodePipeline source for different branches of the same repo.

@clafollett
Copy link

I just hit this issue after a major refactor to upgrade all our packages and to finally get our deployments back up to speed. We were using GitHub Version 1 and upgraded to the CodeStarConnections and we can not longer deploy our environment pipelines that relied on a strict branch naming convention. Any idea when this fix will be available?

@scanlonp scanlonp changed the title aws_cdk.pipelines: Using the same repo at least twice as a connection in a pipeline causes duplicate id error (pipelines): using the same repo more than once as a connection in a pipeline causes duplicate id error Oct 5, 2023
@mergify mergify bot closed this as completed in #27602 Oct 20, 2023
mergify bot pushed a commit that referenced this issue Oct 20, 2023
… in a pipeline causes duplicate id error (#27602)

Differentiates between sources of the same repository by appending the branch name onto the node id and input/output artifacts. This avoids the duplicate id errors for different branches of the same repository, as well as validating that each source is a unique repository & branch combination.

The only change to the CFN template is the input & output artifacts, but since these are not stateful resources, they can be modified without breaking changes. The artifacts are also updates in tandem, so the pipeline source behavior will stay the same.

This change impacts these `CodePipelineSource`s:
- `s3()` - `objectKey` appended
- `connection()`
- `codeCommit()`

Does not change `ecr()` behavior as there is no notion of folders or branches.
Does not change `github()` behavior as doing so would cause destructive changes of webhooks.

Closes #23916 and #19875.

----

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

@HansFalkenberg-Visma
Copy link

This issue being closed with the last mention being a merged PR is thoroughly misleading. The merged fix was reverted 6 days later:

History: https://github.com/aws/aws-cdk/commits/main/packages/aws-cdk-lib/pipelines/lib/codepipeline/codepipeline-source.ts
Revert PR: #27700
Revert explanation: #27701

Apparently nobody has tried fixing it again since, and this remains an issue. But it seems it would have been easy enough to fix -- at least for those without tokens in their branch names -- by adding the appropriate amount of Token.isUnresolved(branch).

Roll your own as a workaround:

// replace
// CodePipelineSource.connection(`${owner}/${repository}`, branch, { ... });
// with
// new ConnectionSource27602(`${owner}/${repository}`, branch, { ... });


import { Token } from 'aws-cdk-lib';
import { Artifact } from 'aws-cdk-lib/aws-codepipeline';
import { CodeStarConnectionsSourceAction } from 'aws-cdk-lib/aws-codepipeline-actions';
import { CodePipelineSource, ConnectionSourceOptions, FileSet } from 'aws-cdk-lib/pipelines';

// workaround for https://github.com/aws/aws-cdk/issues/23916
// Based on
// https://github.com/aws/aws-cdk/blob/10357c0ab6be105e0d988b9045bcfe99faf69cbd/packages/aws-cdk-lib/pipelines/lib/codepipeline/codepipeline-source.ts#L420
// and
// https://github.com/aws/aws-cdk/blob/70acc844e2a652aea4f1328e4e758c3c5030d501/packages/aws-cdk-lib/pipelines/lib/codepipeline/codepipeline-source.ts#L420
export class ConnectionSource27602 extends CodePipelineSource {
  private readonly owner: string;
  private readonly repo: string;

  constructor(
    repoString: string,
    readonly branch: string,
    readonly props: ConnectionSourceOptions,
  ) {
    super(`${repoString}-${ConnectionSource27602.validateBranch(branch)}`);

    if (!this.isValidRepoString(repoString)) {
      throw new Error(
        `Connection repository name should be a resolved string like '<owner>/<repo>' or '<owner>/<group1>/<group2>/.../<repo>', got '${repoString}'`,
      );
    }

    const parts = repoString.split('/');

    this.owner = parts[0];
    this.repo = parts.slice(1).join('/');
    this.configurePrimaryOutput(new FileSet('Source', this));
  }

  private static validateBranch(branch: string): string {
    if (Token.isUnresolved(branch)) {
      throw new Error(`Connection branch name should be a resolved string', got '${branch}'`);
    }
    return branch;
  }

  private isValidRepoString(repoString: string) {
    if (Token.isUnresolved(repoString)) {
      return false;
    }

    const parts = repoString.split('/');

    // minimum length is 2 (owner/repo) and
    // maximum length is 22 (owner/parent group/twenty sub groups/repo).
    // maximum length is based on limitation of GitLab, see https://docs.gitlab.com/ee/user/group/subgroups/
    if (parts.length < 2 || parts.length > 23) {
      return false;
    }

    // check if all element in parts is not empty
    return parts.every((element) => element !== '');
  }

  protected getAction(
    output: Artifact,
    actionName: string,
    runOrder: number,
    variablesNamespace?: string,
  ) {
    return new CodeStarConnectionsSourceAction({
      output,
      actionName: this.props.actionName ?? actionName,
      runOrder,
      connectionArn: this.props.connectionArn,
      owner: this.owner,
      repo: this.repo,
      branch: this.branch,
      codeBuildCloneOutput: this.props.codeBuildCloneOutput,
      triggerOnPush: this.props.triggerOnPush,
      variablesNamespace,
    });
  }
}

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