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

(events): ValidationError when creating an EventBus that has CrossAccount access. #22120

Closed
sennyeya opened this issue Sep 19, 2022 · 5 comments · Fixed by #22296
Closed

(events): ValidationError when creating an EventBus that has CrossAccount access. #22120

sennyeya opened this issue Sep 19, 2022 · 5 comments · Fixed by #22296
Assignees
Labels
@aws-cdk/aws-events Related to CloudWatch Events bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@sennyeya
Copy link
Contributor

sennyeya commented Sep 19, 2022

Describe the bug

Basically, the id that's created for the support stack statementId is 5 characters too long (69 characters). It then fails validation and the stack cannot be deployed.

It looks like this has been reported before: #19941.

Expected Behavior

The stack deploys as expected.

Current Behavior

Error Message: Stack Deployments Failed: Error: The stack named ci-cd-EventBusPolicy-support-us-west-2-{account2} failed creation, it may need to be manually deleted from the AWS console: ROLLBACK_COMPLETE: 1 validation error detected: Value 'Allow-account-{account2}-c884c8876055cffba97afb1bc5a28125a7cac73762' at 'statementId' failed to satisfy constraint: Member must have length less than or equal to 64 (Service: AWSEvents; Status Code: 400; Error Code: ValidationException; Request ID: c521bf0c-abec-4042-90ed-54823e58a58e; Proxy: null)

Reproduction Steps

import { Stack, StackProps } from 'aws-cdk-lib';
import { IRepository } from 'aws-cdk-lib/aws-codecommit';
import {
  CodePipeline,
  CodePipelineSource,
  ShellStep,
} from 'aws-cdk-lib/pipelines';
import { Construct } from 'constructs';

export interface ReproStackProps extends StackProps {
  codeRepository: IRepository;
}

export class CodeStack extends Stack {
  public codeRepository: aws_codecommit.IRepository;
  constructor(scope: Construct, props: StackProps) {
    super(scope, 'CodeStack', props);
    this.codeRepository = aws_codecommit.Repository.fromRepositoryName(
      this,
      'code-repo',
      'repo',
    );
  }
}

export class ReproStack extends Stack {
  constructor(scope: Construct, props: ReproStackProps) {
    super(scope, 'repro', props);

    new CodePipeline(this, 'code-pipeline', {
      crossAccountKeys: true,
      synth: new ShellStep('synth', {
        input: CodePipelineSource.codeCommit(props.codeRepository, 'mainline'),
        commands: ['yarn install', 'yarn build'],
      }),
    });
  }
}
const app = new App();

const codeStack = new CodeStack(app, {
  env: {
    region: 'us-west-2',
    account: 'account1',
  },
});

new ReproStack(app, {
  env: {
    region: 'us-west-2',
    account: 'account2',
  },
  codeRepository: codeStack.codeRepository,
});

Possible Solution

When trying to upgrade past version 1.150 we ran into ValidationException while trying to create a EventBusPolicy cross account. This failure is probably happening because of the following line

statementId: Allow-account-${sourceAccount}-${this.node.addr},

It can be found here: https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-events/lib/rule.ts#L400.

Additional Information/Context

No response

CDK CLI Version

2.41.0

Framework Version

No response

Node.js Version

v16.17.0

OS

MacOS Monterey 12.4

Language

Typescript

Language Version

Version 4.8.3

Other information

No response

@sennyeya sennyeya added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 19, 2022
@github-actions github-actions bot added the @aws-cdk/aws-events Related to CloudWatch Events label Sep 19, 2022
@peterwoodworth
Copy link
Contributor

peterwoodworth commented Sep 19, 2022

Thanks for reporting this @ara6893,

That line you've pointed out seems to guarantee failure due to length

statementId: `Allow-account-${sourceAccount}-${this.node.addr}`,

Allow-account- = 14 characters
accountId = 12 characters
- = 1 char
this.node.addr = 42 characters

Adding these together results in 69 characters, which breaks the limit set by the service. We don't have any integration tests for this... We need to fix this bug and add integration tests

@peterwoodworth
Copy link
Contributor

I am marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

Check out our contributing guide if you're interested in contributing yourself - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂

@peterwoodworth peterwoodworth added p2 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 Sep 19, 2022
@sennyeya
Copy link
Contributor Author

I'd be happy to take a stab at this. @peterwoodworth , do you have any pointers on direction for this? My first thought is to do a substring to hard cap the id to 64 characters (with .substring(0, 64)), but that might hit some issues with non-uniqueness.

@peterwoodworth
Copy link
Contributor

We have precedent set elsewhere in our codebase to cap a hash or otherwise randomly generated ID for the sake of character limit. That solution should be fine!

@mergify mergify bot closed this as completed in #22296 Oct 29, 2022
mergify bot pushed a commit that referenced this issue Oct 29, 2022
Fixes #22120, #21808.

Current setup does not allow deployment of the EventBus support stack due to StatementId being larger than 64 characters.

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

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

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

Successfully merging a pull request may close this issue.

3 participants