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

Putting GITHUB_TOKEN in terraform config for aws_codepipeline #2796

Closed
yacinehmito opened this issue Dec 28, 2017 · 20 comments · Fixed by #14175
Closed

Putting GITHUB_TOKEN in terraform config for aws_codepipeline #2796

yacinehmito opened this issue Dec 28, 2017 · 20 comments · Fixed by #14175
Assignees
Labels
proposal Proposes new design or functionality. service/codepipeline Issues and PRs that pertain to the codepipeline service.
Milestone

Comments

@yacinehmito
Copy link

When using AWS CodePipeline, if you want to fetch your source code from GitHub, you need to run terraform apply with the environment variable GITHUB_TOKEN (whose value should be a previously generated OAuth token).

This kinda goes against Terraform's philosophy. Would it be more sensible to have a parameter for the resource aws_codepipeline to set that token? Is there a compelling reason to use an environment variable instead?

I'm starting the discussion. If we end up deciding on a parameter I'm willing to have a go at a PR.

@jen20 jen20 added the proposal Proposes new design or functionality. label Dec 29, 2017
@radeksimko radeksimko added the service/codepipeline Issues and PRs that pertain to the codepipeline service. label Jan 28, 2018
@erothmayer
Copy link

erothmayer commented Feb 25, 2018

Ran across this as I'm working with the resource in question.

(Edited as I was previously mistaken about the level of access CodePipeline requires)

If you're working with private repos, CodePipeline requires that the token you give it have access to those private repositories. As such, it's not great to encourage users to check those tokens in to source control along with their config, or leak it into state.

I gather some other providers (RDS for example) can encrypt their inputs in state, and with a big warning around the feature to remember to keep secrets in local config overrides and not in source it might be ok. But it does seem a little risky.

Basically, it would need to be treated like other secrets, such as the access key id and secret in the AWS provider.

@yacinehmito
Copy link
Author

Basically, it would need to be treated like other secrets, such as the access key id and secret in the AWS provider.

Absolutely. It is an issue?

@psteininger
Copy link

@yacinehmito wow, this is ridiculous that I need to set the env variable!!! there is a config field for OAuthToken, and it does jack!

Can this be fixed somehow? I mean I have a separate CodeBuild project (non-pipeline) where I use a Personal Access Token, and it all works. Why can't it work through pipeline? Is this a terraform issue or AWS CLI issue?

@psteininger
Copy link

the precise code I meant in my earlier comment is like below. I store the PAT in SSM, so that it's easy to access and it is the same, no matter which developer is running terraform apply:


  stage {
    name = "Source"

    action {
      name             = "Source"
      category         = "Source"
      owner            = "ThirdParty"
      provider         = "GitHub"
      version          = "1"
      output_artifacts = ["code"]

      configuration {
        Owner                = "organization"
        Repo                 = "repo"
        Branch               = "${terraform.workspace == "staging" ? "master" : terraform.workspace}"
        OAuthToken           = "${data.aws_ssm_parameter.github_token.value}"
        PollForSourceChanges = false
      }
    }
  }

@psteininger
Copy link

psteininger commented Jun 21, 2018

I see it in the code: https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_codepipeline.go#L284-L291

But why in heavens do this? What is the rationale behind it? This isn't the case for CodeBuild projects.

@stack72 @comebackoneyear Could either of you provide some background as to why this was coded this way? Any warnings/objections to changing it, so that one could use something like this?

OAuthToken           = "${data.aws_ssm_parameter.github_token.value}"

@psteininger
Copy link

@yacinehmito @stack72 @comebackoneyear Bumping this. I'd like to get a resolution on this, and perhaps do a PR (though I haven't written a line of Go in my life).

I honestly think that we should not use an ENV var, because that does not prevent the token from being stored in the state. It is displayed in the plan, regardless of the way it is specified. That is a separate issue. I currently mitigate it by storing state on encrypted S3 bucket.

@yacinehmito
Copy link
Author

If we end up deciding on a parameter I'm willing to have a go at a PR.

I'll have to back down on this, sorry. I don't have this need anymore. (And I haven't written a line of Go in my life either).

Still interested in having this solved for the sake of it though.

psteininger added a commit to laborvoices/terraform-provider-aws that referenced this issue Sep 4, 2018
… 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.
@psteininger
Copy link

@yacinehmito I made a PR. I removed the frustrating code, but I kept the code that allows setting the value from ENV var. I am beginning to realie that this was an early hack to prevent it from being displayed. A much better way to hide the value of the token is to polish off the schema to mark it as sensitive.

@ldormoy
Copy link

ldormoy commented Oct 4, 2018

I'm using atlantis in Fargate, which also requires a github token.

I use chamber to fetch the token at runtime:
https://github.com/segmentio/chamber

It's a wrapper for SSM Parameter Store.

the chamber exec command can be used as entrypoint to load as env variables all secrets registered under a service "namespace".

This way you don't have to commit your token in git and it also won't appear in your terraform or CI output

@debuggerpk
Copy link

would the personal access token do ?

@smastrorocco
Copy link

The AWS CodeBuild resource does not require this and works nicely using the OAUTH already granted to CodeBuild through the UI. Can something similar be done here? Requiring this to be present in ENV VAR is pretty problematic, especially during development where a developer might not have this.

@bonovoxly
Copy link

bonovoxly commented Sep 24, 2019

This is a bummer. Just ran into this.

I liked how CodeBuild did this. Just need to walk through the connection via the GUI once, then reuse.

@hlarsen
Copy link

hlarsen commented Oct 6, 2019

looks like #2854 is related and #5764 would solve it - anyone have any thoughts?

@eshiri
Copy link

eshiri commented Oct 7, 2019

@hlarsen #5764 it looks like a solution, but the PR is opened for more than a year.
Who can approve it?

@yogin
Copy link

yogin commented Dec 5, 2019

Ran into this issue today.

I have a GITHUB_TOKEN and GITHUB_ORGANIZATION setup in my environment to configure the Github provider, these are my own personal tokens, that I used to setup the Github webhook for CodePipeline, however, my CodePipeline needs it's own token to access the source code in Github, it should not reuse my provider configuration.

I've stored the token for CodePipeline in AWS SecretsManager, and was trying to set the OAuthToken configuration property. I noticed the plan shows the correct token (the one stored in Secrets Manager) but looking at the linked code above, I see that the value gets replaced with the environment variable when applying the changes.

Can someone remove this? Seems like this issue has been open here for a while already.
At the very least, to avoid "breaking" the current behavior, the code should check if the configuration block as the OAuthToken set to some value, and if it doesn't, then use the environment variable, that way it will continue to work for those who are already using it, but we can start using separate tokens.

The state already stores database passwords and other sensitive information, so it should not be an issue for the state to store this token. If anything it should just be marked as sensitive so it doesn't print out in plan.

Those concerned about keeping secrets in the state should seriously consider using remote states

@errivera-cbssports
Copy link

Would be great if we could just use the solution proposed by psteininger

OAuthToken = "${data.aws_ssm_parameter.github_token.value}"

I also was under the impression the OAuthToken configuration parameter would work as expected in CodePipeline.

@bflad bflad added this to the v3.0.0 milestone Jan 11, 2020
@isen-ng
Copy link

isen-ng commented Feb 26, 2020

I have a similar use case as @yogin

Assuming if I had OAuthToken = "${data.aws_ssm_parameter.github_token.value}" and $ export GITHUB_TOKEN=<a different value for a different purpose>

Reasons

I think it's incorrect to overwrite the original OAuthToken's non-empty value with $GITHUB_TOKEN. I think this is incorrect on multiple reasons:

  1. In terraform plan, even if $GITHUB_TOKEN is set, the plan still shows OAuthToken => <value from ssm>. However, if I apply this, the value from $GITHUB_TOKEN is used instead.
    • The actual value applied will be different from the value presented in plan. This violates terraform at the most fundamental level
    • I think this creates confusion and increases difficultly in debugging when there are issues related to this
  2. If a developer has hardcoded OAuthToken to a certain value or passed the value in from another resource explicitly, I think that developer explicitly wants to use that specific value.
    • Having it silently overwritten by another value is just wrong
  3. The $GITHUB_TOKEN env var is used for many other applications, one of them the hub cli.
    • By forcing the config to use only this $GITHUB_TOKEN if it's present, means that you cannot have different tokens for the different apps without significant hacking

Proposals

To solve these issues I have 2 proposals:

  1. If OAuthToken is set, do not read from $GITHUB_TOKEN
    • This addresses the fact that developers have explicitly set the value, don't tamper with it.
    • This may result in backward compatibility issues.

or

  1. Introduce a new variable called, action.0.configuration.OAuthTokenVarName whose default value is GITHUB_TOKEN.
    • When set, the provider will read the token from the new env var instead of $GITHUB_TOKEN.
    • This is completely backward compatible.

Edit:
If a consensus can be reached to fix this before 3.0.0, I can send a PR

@rabidscorpio
Copy link

While removing the ability to reference the GITHUB_TOKEN environment variable is not backwards compatible, I don't know of a single other place in Terraform that explicitly references an external environment variable other than the usual TF_VAR_name convention. This should never have made it into the provider and should really be removed so that it doesn't cause confusion.

I can easily imagine a situation where a user didn't set the oauth token in terraform and then terraform then pulls it from the environment variable automatically causing much confusion. The oauth token should be set just like any other sensitive parameter is set:

OAuthToken = "${data.aws_ssm_parameter.github_token.value}"

@ghost
Copy link

ghost commented Jul 31, 2020

This has been released in version 3.0.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Aug 30, 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 30, 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