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

This addresses the oddity of the configuration #5764

Closed
wants to merge 1 commit into from

Conversation

psteininger
Copy link

This addresses the oddity of the configuration in #2796. This simply removes the code that deletes the OAuthToken passed in and arbitrary validation condition looking for GITHUB_TOKEN env variable. This behavior goes against Terraform convention, and was absolutely NOT well documented, it also drove me insane for a few days until I found an off-hand mention and code reference. There was also no rationale provided for why the code should delete what was provided and replace it with the value of arbitrary ENV variable.

A much better way to address this is to improve the schema of the in the project definition to mark OAuthToken as sensitive.

Fixes #2796

Changes proposed in this pull request:

  • Remove validation code requiring GITHUB_TOKEN env var
  • Remove code that deletes the value provided in the configuration per AWS official documentation

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSAvailabilityZones'
I can't get it to run, never worked with Go.
...

… in hashicorp#2796. This simply removes the code that deletes the token passed in and arbitrary validation condition looking for GITHUB_TOKEN env variable. This is behavior goes against Terraform convention, and is absolutely NOT documented, and there was no rationale provided.

A much better way to address this is to improve the schema of the in the project definition to mark OAuthToken as sensitive.
@bflad bflad added the service/codepipeline Issues and PRs that pertain to the codepipeline service. label Sep 4, 2018
@aeschright aeschright requested a review from a team June 25, 2019 21:27
@aeschright aeschright added the proposal Proposes new design or functionality. label Aug 19, 2019
@bflad bflad added this to the v3.0.0 milestone Jan 11, 2020
@gdavison gdavison self-assigned this Jul 13, 2020
@gdavison
Copy link
Contributor

Thanks for the PR, @psteininger. Unfortunately, the AWS API doesn't return the OAuthToken for security reasons, so the change is a bit more involved than not deleting the API response. We've created #14175 to address the issue.

@gdavison gdavison closed this Jul 14, 2020
@gdavison gdavison removed this from the v3.0.0 milestone Jul 14, 2020
@ghost
Copy link

ghost commented Aug 14, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Aug 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
proposal Proposes new design or functionality. service/codepipeline Issues and PRs that pertain to the codepipeline service.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Putting GITHUB_TOKEN in terraform config for aws_codepipeline
4 participants