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

(codepipeline): Imported pipeline has incorrect account id #27816

Closed
daschaa opened this issue Nov 2, 2023 · 9 comments
Closed

(codepipeline): Imported pipeline has incorrect account id #27816

daschaa opened this issue Nov 2, 2023 · 9 comments
Assignees
Labels
@aws-cdk/aws-codepipeline Related to AWS CodePipeline bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@daschaa
Copy link
Contributor

daschaa commented Nov 2, 2023

Describe the bug

When importing a pipeline from an pipeline arn, the imported pipeline has the environment from the stack its imported to, not from the pipeline arn.

Expected Behavior

The env property has the same account id than the arn where it is imported from.

Current Behavior

The env property has the account id from the scope.

Reproduction Steps

import { App, CfnOutput, Stack } from 'aws-cdk-lib';
import { Pipeline } from 'aws-cdk-lib/aws-codepipeline';

const app = new App();
const stack = new Stack(app, 'Stack', {
  env: {
    account: '123456789012',
    region: 'eu-central-1',
  },
});

const pipeline = Pipeline.fromPipelineArn(stack, 'ImportedPipeline', 'arn:aws:states:eu-central-1:123456789015:Pipeline/MyPipeline');

new CfnOutput(stack, 'Output', {
  value: pipeline.env.account,
});

app.synth();

Possible Solution

Change the fromPipelineArn method to use the environmentFromArn parameter from the Import class.

public static fromPipelineArn(scope: Construct, id: string, pipelineArn: string): IPipeline {
class Import extends PipelineBase {
public readonly pipelineName = Stack.of(scope).splitArn(pipelineArn, ArnFormat.SLASH_RESOURCE_NAME).resource;
public readonly pipelineArn = pipelineArn;
}
return new Import(scope, id);
}

to following:

public static fromPipelineArn(scope: Construct, id: string, pipelineArn: string): IPipeline {
    class Import extends PipelineBase {
      public readonly pipelineName = Stack.of(scope).splitArn(pipelineArn, ArnFormat.SLASH_RESOURCE_NAME).resource;
      public readonly pipelineArn = pipelineArn;
    }

    return new Import(scope, id, {
      environmentFromArn: pipelineArn,
    });
  }

Additional Information/Context

No response

CDK CLI Version

2.103.1

Framework Version

No response

Node.js Version

18.12.1

OS

Mac

Language

TypeScript

Language Version

No response

Other information

No response

@daschaa daschaa added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 2, 2023
@github-actions github-actions bot added the @aws-cdk/aws-codepipeline Related to AWS CodePipeline label Nov 2, 2023
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 2, 2023
@daschaa
Copy link
Contributor Author

daschaa commented Nov 6, 2023

@khushail Any Updates? This issue is blocking me currently

@khushail khushail added p1 needs-review effort/small Small work item – less than a day of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Nov 6, 2023
@pahud
Copy link
Contributor

pahud commented Nov 6, 2023

I am not sure if the imported pipeline.env should return the imported pipeline env because fromPipelineArn() basically returns IPipeline which implements IResource that comes with the env of this resource interface(details).

@daschaa
Copy link
Contributor Author

daschaa commented Nov 6, 2023

@pahud I'm not sure if I can follow. Shouldn't the pipeline return the env that it belongs to? For example: I create a pipeline in the AWS console and it is deployed in eu-west-1. Then I call the fromPipelineArn method and pass the Arn of the Pipeline to the method so that I can use it in my IaC.
Shouldn't CDK then treat the Pipeline as it is deployed in eu-west-1? For me, as an user of the CDK, I would expect that it always reflects the reality.

To see why this is an issue, see the recent comments in the pull request #27799

@pahud
Copy link
Contributor

pahud commented Nov 6, 2023

@daschaa yes I agree with you.

Generally, when we import existing resource with the fromXxx() method, it returns the IXxx interface that carries some information we pass in such as resource arn and properties that could be split from the ARN. And the IXxx interface actually implements IResource which always has the env property representing the ResourceEnvironment. Unfortunately, the IPipeline.env is now resolving to your current account rather than the owner account of the imported pipeline and this could be very confusing. However, this is the current implementation of the IResource that IPipeline implements, which I believe is a universal behavior across multiple imported resources in CDK.

Now, if IPipeline.env represents the env of the imported pipeline from another account, we should assume all other existing CDK imported resources would have exactly the same behavior but I doubt that. For example, the imported Lambda function IFunction.env would not represent the resource environment of the cross-account function owner.

For me, as an user of the CDK, I would expect that it always reflects the reality.

I guess maybe we can create something like IPipeline.ownerEnv that represents the pipeline owner env which can be retrieved from the pipeline arn while IPipeline.env remains to represent current owner env, which has been implemented as a consistent behavior across all imported resources. But this definitely need more discussion.

We will bring this to the core team this week.

@vinayak-kukreja
Copy link
Contributor

Hey @daschaa , could you please tell us more about your use case and what is blocking you in the current implementation?

@daschaa
Copy link
Contributor Author

daschaa commented Nov 8, 2023

@vinayak-kukreja I'm working on #27799 where a reviewer suggests I should implement a fix for this issue. There is a workaround available (as done in the PR) but I agree with the reviewer that it should probably be solved in the fromPipelineArn method.

@vinayak-kukreja
Copy link
Contributor

I agree. This makes sense.
We will take a look at the PR. Thank you for opening a PR for this.

@GavinZZ
Copy link
Contributor

GavinZZ commented May 6, 2024

I believe this issue is also fixed in #27799. Going to mark this issue as resolved. Feel free to re-open if you have additional questions.

@GavinZZ GavinZZ closed this as completed May 6, 2024
Copy link

github-actions bot commented May 6, 2024

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-codepipeline Related to AWS CodePipeline bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

No branches or pull requests

5 participants