aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.59k stars 3.89k forks source link

[aws-codepipeline] Cyclic reference error when importing an S3 bucket in a code pipeline defined in a different stack #10896

Closed harshchhatwani closed 2 years ago

harshchhatwani commented 4 years ago

:question: General Issue

Hi- I am using CDK to create a code pipeline with an S3 bucket as its source. The S3 bucket and code pipeline are defined in different stacks and per the AWS documentation we can access resources in a different stack as long as they are in the same account and AWS region: https://docs.aws.amazon.com/cdk/latest/guide/resources.html#resource_stack

The Question

This doesn't seem to be working and I am getting a "Cyclic reference" error. Does this mean the AWS documentation is wrong? Or is there something in particular that's missing to make this work? Sample code below

        const pipelineSourceActionOutput = new Artifact();
        const pipelineSourceAction = new S3SourceAction({
            actionName: "S3SourceAction",
            bucket: s3Bucket, // Versioned Bucket (and CloudTrail) defined in a different stack
            bucketKey: `${id}/upload.zip`,
            output: pipelineSourceActionOutput,
            trigger: S3Trigger.EVENTS,
            runOrder: 1,
        });
        new Pipeline(this, pipelineId, {
            pipelineName: pipelineId,
            stages: [
                {
                    stageName: "Source",
                    actions: [pipelineSourceAction],
                }
            ],
        });

I think this line of code is causing the dependency of my stack containing S3 bucket on the stack that defined code pipeline infra: https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-codepipeline-actions/lib/s3/source-action.ts#L120-L124

Environment

Other information

skinny85 commented 4 years ago

Hello @harshchhatwani ,

can you show the exact error that you get?

Thanks, Adam

harshchhatwani commented 4 years ago

Hi @skinny85: The error message contains confidential details of the project so I can't print that. Here's the obfuscated version

Error: 'Persistence-stack' depends on 'CodePipeline-stack' (
  PersistenceStack -> CodePipeline-stack/PipelineName/StageName/DeployToPrereleaseBucketAction/CodePipelineActionRole/Resource.Arn, 
  PersistenceStack -> CodePipeline-stack/PipelineName/StageName/DeployToReleaseBucketAction/CodePipelineActionRole/Resource.Arn, 
  PersistenceStack -> CodePipeline-stack/PipelineName/StageName/S3SourceAction/CodePipelineActionRole/Resource.Arn,
  PersistenceStack -> CodePipeline-stack/PipelineName/StageName/S3ExtractBinaryAction/CodePipelineActionRole/Resource.Arn).
Adding this dependency (CodePipeline-stack -> Persistence-stack/UploadsBucket/Resource.Ref) would create a cyclic reference.
skinny85 commented 4 years ago

@harshchhatwani can you try the solution mentioned here: https://github.com/aws/aws-cdk/issues/5657#issuecomment-571358504 , and see if that works for you?

harshchhatwani commented 4 years ago

Thanks for the pointer @skinny85: This did not work for me. I was able to get rid of cyclic error during build time with this, and even deploy the stack successfully but when I upload an object to the S3 bucket the pipeline fails with "Insufficient permissions The provided role does not have permissions to perform this action."

Looks like this particular line of code is causing this. Without the above workaround, I get a "cyclic reference" error during the build and with the above workaround the role doesn't have required permissions https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-codepipeline-actions/lib/s3/source-action.ts#L120-L124

skinny85 commented 4 years ago

Oh I know why that is I think. It's because of this line:

            trigger: S3Trigger.EVENTS,

Can you comment out that line from your S3SourceAction @harshchhatwani , and see if that helps?

harshchhatwani commented 4 years ago

@skinny85 : That line is in place to use CloudTrail events to trigger the pipeline instead of having the pipeline poll S3 for updates, which is the recommended way per the AWS documentation. What would be the reason to comment this out?

Also, if possible, can we get this reproduced on your end to find the fix and thereby avoid the feedback loop of me trying potential workarounds? It'll save me the churn due to back and forth here. :)

skinny85 commented 4 years ago

@skinny85 : That line is in place to use CloudTrail events to trigger the pipeline instead of having the pipeline poll S3 for updates, which is the recommended way per the AWS documentation. What would be the reason to comment this out?

The reason would be it's the Event that is causing this cycle I believe.

Also, if possible, can we get this reproduced on your end to find the fix and thereby avoid the feedback loop of me trying potential workarounds? It'll save me the churn due to back and forth here. :)

I did, and it did help locally.

harshchhatwani commented 4 years ago

@skinny85 So is the proposal to not use Event and instead have code pipeline poll the S3 bucket for changes?

skinny85 commented 3 years ago

That's a temporary fix while I work on this more deeply, yes 🙂.

You can also try creating the Event explicitly in the Pipeline Stack, something like:

        const pipelineSourceAction = new S3SourceAction({
            actionName: "S3SourceAction",
            bucket: s3Bucket, // Versioned Bucket (and CloudTrail) defined in a different stack
            bucketKey: `${id}/upload.zip`,
            output: pipelineSourceActionOutput,
            trigger: S3Trigger.NONE,
            runOrder: 1,
        });

        const importedBucket = s3.Bucket.fromBucketName(this, 'ImportedBucket', s3Bucket.bucketName);
        importedBucket.onCloudTrailWriteObject('EventId', {
          target: new event_targets.CodePipeline(pipeline),
          paths: [`${id}/upload.zip`],
        });

That should make it work with Events, without the need for polling.

rantoniuk commented 2 years ago

Reference documentation - here

@skinny85 unfortunately not, 1.129.0 and below code still yields circular dependency for me:

    const s3key = 'src.zip';
    const trail = new Trail(this, 'Trail', {
      isMultiRegionTrail: false,
    });

    trail.addS3EventSelector(
      [
        {
          bucket: props.bucket,
          objectPrefix: s3key,
        },
      ],
      {
        readWriteType: ReadWriteType.WRITE_ONLY,
      },
    );

    const s3SourceArtifact = new codepipeline.Artifact('source-code-artifact');

    pipeline.addStage({
      stageName: 'Source',
      actions: [
        new codepipelineActions.S3SourceAction({
          actionName: 'S3Source',
          role: pipelineRole,
          bucket: props.bucket,
          bucketKey: `/${s3key}`,
          output: s3SourceArtifact,
          trigger: S3Trigger.NONE,
        }),
      ],
    });

    // when this is commented out, there is no circular dependency
    props.bitbucketStore.onCloudTrailWriteObject('EventId', {
      target: new CodePipelineTarget(pipeline),
      paths: [s3key],
    });

Some additional bugs I see:

  1. trail.addS3EventSelector([
    {
    bucket: props.bucket,
    objectPrefix: '/prefix',
    }], {
    readWriteType: ReadWriteType.WRITE_ONLY,
    });

creates a trail with double slash instead of single one (which is inconsistent with bucketKey from S3SourceAction.

  1. Creating a trail with no options:
const trail = new Trail(this, 'Trail', { });

and then updating the CDK to:

    const trail = new Trail(this, 'Trail', {
      isMultiRegionTrail: false,
    });

results in cdk diff showing no difference.

Let me know if you'd like the last two to be reported separately and if you have any idea for another workaround for setting up Trail events to CodePipeline.


Manual workaround:

skinny85 commented 2 years ago

Let me know if you'd like the last two to be reported separately and if you have any idea for another workaround for setting up Trail events to CodePipeline.

Yes please - those sound like separate problems with the @aws-cdk/aws-cloudtrail library.

skinny85 commented 2 years ago

What's props.bitbucketStore in your code?

rantoniuk commented 2 years ago

It's an S3 Bucket created from another stack and passed as props, like this:

const bucketStack = new PipelineBucketsStack(app, 'pipelinesBucketsStack', {
});

...

new DevopsPipelineStack(app, 'DevopsPipelineStack', {
  bitbucketStore: bucketStack.bitbucketStoreBucket,
...
});

where StackProps are defined like this:

interface ExtendedStackProps extends StackProps {
  bitbucketStore: Bucket;
...
}
skinny85 commented 2 years ago

I have a little trouble following your intent here, unfortunately.

You have a CodePipeline with an S3 source:

    pipeline.addStage({
      stageName: 'Source',
      actions: [
        new codepipelineActions.S3SourceAction({
          actionName: 'S3Source',
          role: pipelineRole,
          bucket: props.bucket,
          bucketKey: `/${s3key}`,
          output: s3SourceArtifact,
          trigger: S3Trigger.NONE,
        }),
      ],
    });

But you also want to trigger the pipeline on changes to a different S3 Bucket...?

    props.bitbucketStore.onCloudTrailWriteObject('EventId', {
      target: new CodePipelineTarget(pipeline),
      paths: [s3key],
    });

Can you explain your reasoning behind this setup?

rantoniuk commented 2 years ago

No, that's the same bucket and a standard scenario. I have an S3 Bucket and I want events on that bucket to be emitted and trigger exactly that pipeline. The props.bucket is created from another stack and then passed as a param to the source stage of that pipeline.

What I'm trying to achieve is a full CDK code that:

skinny85 commented 2 years ago

So, the reason for the cycle here is quite clear:

  1. Stack1 needs the CodePipeline name (because it has a Rule to trigger it).
  2. Stack2 needs the Bucket name (because it uses it as a source).

You can break the cycle by moving the Event that triggers the CodePipeline from Stack1 to Stack2 (you'll need to create it explicitly, like new events.Rule(stack2, id, { ... });).

rantoniuk commented 2 years ago

Yes, indeed, the cycle is clear and I was trying to follow the suggested code, but that also creates the circular dependency.

So again, just to be clear:

    const trail = new Trail(this, 'BitBucketUploadsTrail', {
      isMultiRegionTrail: false,
    });

    trail.addS3EventSelector(
      [
        {
          bucket: props.bitbucketStore,
          objectPrefix: s3key,
        },
      ],
      {
        readWriteType: ReadWriteType.WRITE_ONLY,
      },
    );

    pipeline.addStage({
      stageName: 'Source',
      actions: [
        new S3SourceAction({
          actionName: 'S3Source',
          runOrder: 1,
          role: devOpsPipelineRole,
          bucket: props.bitbucketStore,
          bucketKey: `${s3key}`,
          output:  new Artifact();,
          trigger: S3Trigger.NONE,
        }),
      ],
    });

    // props.bitbucketStore.onCloudTrailWriteObject('BitBucketZipUploadEvent', {
    //   target: new CodePipelineTarget(pipeline),
    //   paths: [s3key],
    // });

Now:

I am looking at aws-events-targets-readme.html#start-a-codepipeline-pipeline but I don't see a method to create an S3-based/CloudTrail-based rule example. Do you have any code example for this that could be added also to the docs in the scope of this issue?

rantoniuk commented 2 years ago

I have figured out a way, though I'd say it's not CDK native, as it's a mix of CDK code and JSON from the tutorial page, but it works:

new Rule(this, 'rule', {
      eventPattern: {
        source: ['aws.s3'],
        detailType: ['AWS API Call via CloudTrail'],
        detail: {
          eventSource: ['s3.amazonaws.com'],
          eventName: ['CopyObject', 'PutObject', 'CompleteMultipartUpload'],
          requestParameters: {
            bucketName: [props.bitbucketStore.bucketName],
            key: [s3key],
          },
        },
      },
      targets: [new CodePipelineTarget(pipeline)],
    });

(I guess it could be added to the samples I mentioned above, happy to provide a PR if you tell me where).

skinny85 commented 2 years ago

Yes, that's exactly what I meant in https://github.com/aws/aws-cdk/issues/10896#issuecomment-953248261 🙂.

jzybert commented 2 years ago

Had this same issue, but inside a single stack.

export class BuildStack extends DeploymentStack {
    constructor(scope: App, id: string, props: DeploymentStackProps) {
        super(scope, id, props);

        const repository = new Repository(this, 'Repository', {
            repositoryName: 'Application',
            description: 'Read-only CodeCommit repo.'
        });

        const accessLogsKey = new SecureKey(this, 'AccessLogsKey', {
            enableKeyRotation: true,
            trustAccountIdentities: true,
            alias: 'app-encryption-key'
        });
        accessLogsKey.addToResourcePolicy(new SecurePolicyStatement({
            effect: Effect.ALLOW,
            principals: [new SecureAccountRootPrincipal()],
            actions: [
                'kms:Decrypt',
                'kms:DecryptKey'
            ],
            resources: ['*']
        }));

        const accessLogsBucket = new SecureBucket(this, 'AccessLogsBucket', {
            bucketName: 'app-access-logs',
            blockPublicAccess: SecureBlockPublicAccess.BLOCK_ALL,
            encryption: BucketEncryption.KMS,
            encryptionKey: accessLogsKey,
            enforceSSL: true,
            // The access logs bucket can be exempt from requiring its own access logs bucket
            serverAccessLogsBucket: Exempt(undefined)
        });
        accessLogsBucket.addToResourcePolicy(new SecurePolicyStatement({
            effect: Effect.ALLOW,
            principals: [new SecureAccountRootPrincipal()],
            actions: [
                's3:GetObject*',
                's3:GetBucket*',
                's3:List*'
            ],
            resources: [accessLogsBucket.bucketArn, `${accessLogsBucket.bucketArn}/*`]
        }));

        const artifactBucket = new SecureBucket(this, 'ArtifactBucket', {
            bucketName: 'app-artifacts',
            encryption: Exempt(BucketEncryption.S3_MANAGED),
            enforceSSL: true,
            serverAccessLogsBucket: accessLogsBucket
        });
        artifactBucket.addToResourcePolicy(new SecurePolicyStatement({
            effect: Effect.ALLOW,
            principals: [new SecureAccountRootPrincipal()],
            actions: [
                's3:GetObject*',
                's3:GetBucket*',
                's3:List*'
            ],
            resources: [artifactBucket.bucketArn, `${artifactBucket.bucketArn}/*`]
        }));

        const project = new Project(this, 'Project', {
            projectName: 'Application-AutomatedBuild',
            source: Source.codeCommit({
                repository: repository
            }),
            environment: {
                buildImage: WindowsBuildImage.WIN_SERVER_CORE_2019_BASE,
                computeType: WindowsBuildImage.WIN_SERVER_CORE_2019_BASE.defaultComputeType,
            },
            role: new SecureRole(this, 'ProjectRole', {
                roleName: 'CodeBuildProjectRole-DO-NOT-DELETE',
                assumedBy: new SecureServicePrincipal('codebuild.amazonaws.com')
            }),
            artifacts: Artifacts.s3({
                bucket: artifactBucket,
                includeBuildId: false,
                packageZip: false
            })
        });

        // Required policies to access goshawk (in order to pull CodeArtifact NPM packages)
        project.addToRolePolicy(new SecurePolicyStatement({
            effect: Effect.ALLOW,
            actions: ['s3:GetObject'],
            resources: ['arn:aws:s3:::goshawk/*']
        }));
        project.addToRolePolicy(new SecurePolicyStatement({
            effect: Effect.ALLOW,
            actions: [
                'goshawk:GetAuthorizationToken',
                'goshawk:ReadFromRepository'
            ],
            resources: ['*']
        }));

        const pipeline = new Pipeline(this, 'Pipeline', {
            pipelineName: 'Application'
        });
        pipeline.node.addDependency(repository);
        pipeline.node.addDependency(project);

        const sourceOutput = new Artifact('sourceOutput');
        pipeline.addStage({
            stageName: 'Source',
            actions: [
                new CodeCommitSourceAction({
                    actionName: 'source',
                    repository: repository,
                    branch: 'mainline',
                    output: sourceOutput
                })
            ]
        });

        const buildOutput = new Artifact('BuildOutput');
        pipeline.addStage({
            stageName: 'Build',
            actions: [
                new CodeBuildAction({
                    actionName: 'build',
                    project: project,
                    input: sourceOutput,
                    outputs: [buildOutput]
                })
            ]
        });
    }
}

Error:

Deployment b4e1194a-cc3b-47ed-8e0c-a3248f68655d failed because Couldn't call cloudformation for target arn:aws:cloudformation:**hiding for privacy**:stack/BuildStack because of a validation error. The error was Circular dependency between resources: [PipelineArtifactsBucket4F67921F, PipelineBuildbuildCodePipelineActionRole11965005, PipelineRoleDefaultPolicy0AA25D2C, PipelineArtifactsBucketEncryptionKeyBC3B247B, PipelineRoleD3FFD1A8, PipelineE0CAC50E, PipelineSourcesourceCodePipelineActionRole57B371C3, ProjectRoleDefaultPolicyB1A21FB4, PipelineEventsRoleDefaultPolicyF485194A, PipelineBuildbuildCodePipelineActionRoleDefaultPolicyD6B23CEB, PipelineArtifactsBucketEncryptionKeyAliasC972AB70, RepositoryBuildStackPipeline69086EB1mainlineEventRule74A5046C, PipelineEventsRole8A77C29A, PipelineSourcesourceCodePipelineActionRoleDefaultPolicyE2FCF43C, ProjectC78D97AD] (Service: AmazonCloudFormation; Status Code: 400; Error Code: ValidationError; Request ID: 2955d618-68f2-4cb6-b488-91415d536a22; Proxy: null)
skinny85 commented 2 years ago

@jzybert in the pipeline, you shouldn't provide source and artifacts for a CodeBuild Project, because those are controlled through the pipeline itself.

Use the PipelineProject class instead of Project, and that will become much more clear 🙂.

github-actions[bot] commented 2 years ago

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