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

(@aws-cdk/aws-codestarnotifications): CodeCommit repository not supported as source for Notification Rule #15653

Closed
badfun opened this issue Jul 19, 2021 · 16 comments · Fixed by #15739
Labels
@aws-cdk/aws-codestarnotifications effort/small Small work item – less than a day of effort feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature-request A feature should be added or improved. p2

Comments

@badfun
Copy link
Contributor

badfun commented Jul 19, 2021

Currently @aws-cdk/aws-codestarnotifications module does not seem to support using repository as the source. However, this does appear to be supported in Cloudformation.
From the CDK api docs:
"Currently, Supported sources include pipelines in AWS CodePipeline and build projects in AWS CodeBuild in this L2 constructor."

and from CloudFormation template reference:
"Supported resources include pipelines in AWS CodePipeline, repositories in AWS CodeCommit, and build projects in AWS CodeBuild."

In the console notification rules can easily be created for repository events and sent to SNS or Chatbot targets. I would like to replicate that with the CDK.

Environment

  • CDK CLI Version: 1.109.0
  • Module Version: 1.109.0
  • Node.js Version: 14.17.0
  • OS: Windows 10
  • Language (Version): 4.3.4

Other information

According to CloudFormation, an ARN is required, however re-creating an ARN won't work INotificationRuleSource won't accept a string. Ideally it should work something like this:

    const repositoryNotification = new NotificationRule(this, 'PullRequestNotificationRule', {
      source: repository.repositoryArn,
      events: [
        'codecommit-repository-pull-request-created',
        'codecommit-repository-pull-request-merged'
      ],
      targets: [chatbot],
      detailType: DetailType.FULL,
      notificationRuleName: 'pull-request-status'
    })
    repositoryNotification.addTarget(chatbot)
@badfun badfun added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Jul 19, 2021
@skinny85
Copy link
Contributor

Thanks for opening the issue @badfun! We encourage community contributions, so if you'd be interested in submitting us a Pull Request adding this functionality, here's our "Contributing guide": https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md.

@skinny85
Copy link
Contributor

Also tagging @luckily for visibility, as he's the author of this functionality in the CDK.

@skinny85
Copy link
Contributor

skinny85 commented Jul 19, 2021

In the meantime, to unblock yourself, you should be able to write a class that implements the codestarnotifications.INotificationRuleSource interface, and takes in a CodeCommit IRepository, yourself:

import * as notifications from '@aws-cdk/aws-codestarnotifications';
import * as codecommit from '@aws-cdk/aws-codecommit';

export class CodeCommitNotification implements notifications.INotificationRuleSource {
  constructor(private readonly repository: codecommit.IRepository) {
  }

  public bindAsNotificationRuleSource(_scope: Construct): notifications.NotificationRuleSourceConfig {
    return {
      sourceArn: this.repository.repositoryArn,
    };
  }
}

@skinny85 skinny85 added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs p2 and removed @aws-cdk/aws-codecommit Related to AWS CodeCommit guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Jul 19, 2021
@skinny85 skinny85 removed their assignment Jul 19, 2021
@luckily
Copy link
Contributor

luckily commented Jul 20, 2021

Hi @badfun, can you try something like this?

import * as cdk from '@aws-cdk/core';
import * as codecommit from '@aws-cdk/aws-codecommit';
import * as notifications from '@aws-cdk/aws-codestarnotifications';
import * as sns from '@aws-cdk/aws-sns';
import * as subs from '@aws-cdk/aws-sns-subscriptions';

const repo = new codecommit.Repository(this, 'MyRepo', {
  repositoryName: 'MyTestRepo',
  description: 'For CDK testing',
});

const topic = new sns.Topic(this, 'MySns', {
  topicName: 'NotifyFromCodecommit',
});

topic.addSubscription(new subs.EmailSubscription('my@email.address'));

const notification = new notifications.NotificationRule(this, 'MyNotificaitonRule', {
  source: {
    bindAsNotificationRuleSource: () => {
      return { sourceArn: repo.repositoryArn };
    },
  },
  events: [
    'codecommit-repository-pull-request-created',
  ],
});

notification.addTarget(topic);

You can also use aws-chatbot instead.

This is a quick workaround. It works.

I would like to know If you want to contribute for this, I will be happy to give you suggestions and assist in the review. 🙂 If you don't want to contribute, I may try to contribute in a few days, but I'm not sure.

@badfun
Copy link
Contributor Author

badfun commented Jul 20, 2021

thanks @skinny85 and @luckily. I will give this a shot. @luckily I will see if I can put some time in on the solution. Have quite a bit on my plate at the moment (as do we all I'm sure) but I'd like to contribute.

@luckily
Copy link
Contributor

luckily commented Jul 20, 2021

I will wait for your contribution @badfun! 😄

@badfun
Copy link
Contributor Author

badfun commented Jul 20, 2021

@luckily I create a pull request: #15679
I spent most of my time trying to figure out how to deal with Gitpod. What a pain. Let me know if I need to make more changes.

@skinny85
Copy link
Contributor

I spent most of my time trying to figure out how to deal with Gitpod. What a pain.

What were the problems with Gitpod @badfun?

@badfun
Copy link
Contributor Author

badfun commented Jul 20, 2021

I couldn't figure out how to do the pull request from it. I think it may have been browser issues, but not sure. It seemed to hang when trying to authorize, etc.
I ended up cloning the repo locally and then copying the changes from Gitpod to the local one, then pushing that.
I did test it with Gitpod on the pull request page and that worked fine.

@skinny85
Copy link
Contributor

Hmm, weird... It always worked very smoothly for me. Sorry for the troubles!

@luckily
Copy link
Contributor

luckily commented Jul 21, 2021

@badfun You can develop on your local machine, it's very easy too, reference here.

@badfun
Copy link
Contributor Author

badfun commented Jul 21, 2021

that sounds like a commitment! ;-)

@badfun
Copy link
Contributor Author

badfun commented Jul 21, 2021

Sorry fellas, I tried. I could not get an environment to work for me. Gitpod has some kind of access issues with my Github account and I could not push to my repo from the Workspace. I tried the local build, as per the instructions here: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md, but it failed on the '../../../scripts/buildup' step. I tried resuming, restarting, etc. but I always ended up with this error:

/c/Users/Ken/Cloud/DEV/AWS-CDK/aws-cdk/scripts/foreach.sh: line 36: printf: missing unicode digit for \U
C:\Users\Ken\Cloud\DEV\AWS-CDKws-cdk\packageswslint: yarn build  (24 remaining)
yarn run v1.22.10
$ tsc -b && eslint . --ext=.ts && pkglint && chmod +x bin/awslint
In package package.json
- [package-info/repository] repository.directory should be "packages\\awslint", is "packages/awslint" (fixable)
Error: Some package.json files had errors
    at main (C:\Users\Ken\Cloud\DEV\AWS-CDK\aws-cdk\tools\pkglint\bin\pkglint.js:29:15)
    at Object.<anonymous> (C:\Users\Ken\Cloud\DEV\AWS-CDK\aws-cdk\tools\pkglint\bin\pkglint.js:32:1)
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:933:32)
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)
    at Module.require (internal/modules/cjs/loader.js:957:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (C:\Users\Ken\Cloud\DEV\AWS-CDK\aws-cdk\tools\pkglint\bin\pkglint:2:1)
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error: last command failed. fix problem and resume by executing: /c/Users/Ken/Cloud/DEV/AWS-CDK/aws-cdk/scripts/foreach.sh
/c/Users/Ken/Cloud/DEV/AWS-CDK/aws-cdk/scripts/foreach.sh: line 40: printf: missing unicode digit for \U
directory:    C:\Users\Ken\Cloud\DEV\AWS-CDKws-cdk\packageswslint

I'm guessing it is something to do with packages not being correct versions or whatnot.

I have done a bunch of work which I think should be correct, you can access it in this branch of my fork:
https://github.com/badfun/aws-cdk/tree/add-codecommit-to-notification-source

Too frustrated to carry on right now.
Ken

ps @luckily your code above worked fine and that is what I am using for now.

@luckily
Copy link
Contributor

luckily commented Jul 22, 2021

@badfun Sorry, I didn't expect that you might use Windows environment, but I think you can solve this problem with Docker, and you should mount CDK source of local directory into Docker to execute some commands, and always execute scripts via Docker.

Gitpod is a good choice if you don't have suitable development environment.

@badfun
Copy link
Contributor Author

badfun commented Jul 22, 2021

@luckily Back in business. I followed the blog post you referenced and used WSL2 on WIndows. Got the environment working. Now I can go back to working on the code.

@aws-cdk/aws-codecommit: yarn run v1.22.10
@aws-cdk/aws-codecommit: $ cdk-test
@aws-cdk/aws-codecommit: test.codecommit.js
@aws-cdk/aws-codecommit: ✔ CodeCommit Repositories - add an SNS trigger to repository
@aws-cdk/aws-codecommit: ✔ CodeCommit Repositories - fails when triggers have duplicate names
@aws-cdk/aws-codecommit: ✔ CodeCommit Repositories - can be imported using a Repository ARN
@aws-cdk/aws-codecommit: ✔ CodeCommit Repositories - can be imported using a Repository ARN and respect the region in clone urls
@aws-cdk/aws-codecommit: ✔ CodeCommit Repositories - can be imported using just a Repository name (the ARN is deduced)
@aws-cdk/aws-codecommit: ✔ CodeCommit Repositories - grant push
@aws-cdk/aws-codecommit: ✔ CodeCommit Repositories - HTTPS (GRC) clone URL
@aws-cdk/aws-codecommit: OK: 13 assertions (10484ms)
@aws-cdk/aws-codecommit: =============================== Coverage summary ===============================
@aws-cdk/aws-codecommit: Statements   : 57.38% ( 70/122 )
@aws-cdk/aws-codecommit: Branches     : 57.78% ( 26/45 )
@aws-cdk/aws-codecommit: Functions    : 31.11% ( 14/45 )
@aws-cdk/aws-codecommit: Lines        : 57.02% ( 69/121 )
@aws-cdk/aws-codecommit: ================================================================================
@aws-cdk/aws-codecommit: Verifying integ.codecommit-events.js against integ.codecommit-events.expected.json ... OK.
@aws-cdk/aws-codecommit: Tests successful. Total time (42.1s) | /mnt/c/Users/Ken/cloud/dev/aws-cdk/aws-cdk/node_modules/nyc/bin/nyc.js (28.8s) | cdk-integ-assert (13.1s)
@aws-cdk/aws-codecommit: Done in 44.00s.
lerna success run Ran npm script 'test' in 1 package in 44.5s:
lerna success - @aws-cdk/aws-codecommit

mergify bot pushed a commit that referenced this issue Aug 25, 2021
…#15739)

----
Fixes #15653

Notification rule can be created for CodeCommit Repository events using @aws-cdk/aws-codestarnotifications

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

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…aws#15739)

----
Fixes aws#15653

Notification rule can be created for CodeCommit Repository events using @aws-cdk/aws-codestarnotifications

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Sep 6, 2021
…aws#15739)

----
Fixes aws#15653

Notification rule can be created for CodeCommit Repository events using @aws-cdk/aws-codestarnotifications

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
david-doyle-as24 pushed a commit to david-doyle-as24/aws-cdk that referenced this issue Sep 7, 2021
…aws#15739)

----
Fixes aws#15653

Notification rule can be created for CodeCommit Repository events using @aws-cdk/aws-codestarnotifications

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-codestarnotifications effort/small Small work item – less than a day of effort feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants