-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix(codebuild): remove oauthToken property from source #2252
Conversation
Auth field should not be set since it's ignored by CodeBuild https://docs.aws.amazon.com/codebuild/latest/APIReference/API_ProjectSource.html#CodeBuild-Type-ProjectSource-auth BREAKING CHANGE: customers who use GitHub, GitHubEnterprise or Bitbucket as source will need to remove the oauthToken field as it's no longer available Fixes aws#2199
@@ -261,7 +251,6 @@ export class GitHubSource extends GitBuildSource { | |||
|
|||
protected toSourceProperty(): any { | |||
return { | |||
auth: { type: 'OAUTH', resource: this.oauthToken }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is correct. I believe you still need to supply auth: { type: 'OAUTH' }
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kaixiang-AWS can you confirm whether the auth: { type: 'OAUTH' }
is still required here, even without the Token?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the PR and added an integ test against github source to make sure it can deploy.
I can confirm that auth
field is not required. There might be an issue with the doc. I will report it to the doc writer and ask him to update the doc.
@@ -103,7 +100,7 @@ This source type can be used to build code from a BitBucket repository. | |||
|
|||
## Environment | |||
|
|||
By default, projects use a small instance with an Ubuntu 14.04 image. You | |||
By default, projects use a small instance with an Ubuntu 18.04 image. You |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to update this, you will also need to update:
https://github.com/awslabs/aws-cdk/blob/huijbers/bump-jsii/packages/@aws-cdk/aws-codebuild/lib/project.ts#L944
https://github.com/awslabs/aws-cdk/blob/huijbers/bump-jsii/packages/@aws-cdk/aws-codebuild/lib/project.ts#L625
And add a new image here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for not mentioning that in the PR. I have already pushed a commit to change the image, but forgot to update the documentation in the previous commit.
I think it would also be good to add an example of an ImportCredentials call. Can you add this to the README in a section title "Using GitHub repositories" or something similar?
|
Auth field should not be set since it's ignored by CodeBuild
https://docs.aws.amazon.com/codebuild/latest/APIReference/API_ProjectSource.html#CodeBuild-Type-ProjectSource-auth
Refactored some unit tests as well
BREAKING CHANGE: customers who use GitHub, GitHubEnterprise or Bitbucket as source
will need to remove the oauthToken field as it's no longer available
Fixes #2199
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.