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_codepipeline with Github OAuth still breaking auth #15200

Closed
ybron opened this issue Sep 17, 2020 · 17 comments · Fixed by #16959
Closed

aws_codepipeline with Github OAuth still breaking auth #15200

ybron opened this issue Sep 17, 2020 · 17 comments · Fixed by #16959
Assignees
Labels
bug Addresses a defect in current functionality. service/codepipeline Issues and PRs that pertain to the codepipeline service.
Milestone

Comments

@ybron
Copy link

ybron commented Sep 17, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform CLI and Terraform AWS Provider Version

Terraform v0.12.28
+ provider.archive v1.3.0
+ provider.aws v3.0.0
+ provider.template v2.1.2

Affected Resource(s)

  • aws_codepipeline

Terraform Configuration Files

resource "aws_codepipeline" "pipeline" {
    name      = "${var.ecs_service_name}-codepipeline"
    role_arn  = var.codepipeline_role

    artifact_store {
        location = var.artifact_store_bucket
        type     = "S3"
    }

    stage {
        name = "Source"

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

            configuration = {
                Owner                  = "XXXXXX"
                Repo                   = var.repository_name
                Branch                 = "main"
                PollForSourceChanges   = "true"
                #OAuthToken             = "*"
            }
        }
     }

     stage {
         name = "Build"

         action {
             name             = "Build"
             category         = "Build"
             owner            = "AWS"
             provider         = "CodeBuild"
             version          = "1"
             input_artifacts  = ["SourceArtifact"]
             output_artifacts = ["BuildArtifact"]

             configuration = {
                ProjectName             = "${var.ecs_service_name}-codebuild"
             }
          }
      }
    stage {
        name = "Deploy"

        dynamic action {
        for_each = var.dedup_conf
            content {
                category         = "Deploy"
                configuration    = {
                    ClusterName  = var.ecs_cluster_name
                    ServiceName  = "${var.ecs_service_name}-${action.key}"
                    FileName     = "imagedefinitions.json"
                }
                input_artifacts  = [
                    "BuildArtifact",
                ]
                name             = "${var.ecs_service_name}-${action.key}"
                output_artifacts = []
                owner            = "AWS"
                provider         = "ECS"
                run_order        = 1
                version          = "1"
            }
        }
    }
}

Debug Output

N/A

Panic Output

N/A

Expected Behavior

The existing OAuthToken should have been left in-place when the aws_codepipeline resource was updated.

Actual Behavior

Same behaviour as before, when you were forced to push a broken placeholder OAuthToken resource in order to update it. Only now you don't have to comment-in the OAuthToken placeholder to update the resource.

Steps to Reproduce

  1. terraform apply (after triggering authorizing to github, and forcing an update to the aws_codepipeline resource

Important Factoids

N/A

References

@ghost ghost added the service/codepipeline Issues and PRs that pertain to the codepipeline service. label Sep 17, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Sep 17, 2020
@anGie44 anGie44 added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 21, 2020
@gdavison gdavison self-assigned this Sep 21, 2020
@gdavison
Copy link
Contributor

Hi @ybron. I'm not sure I fully understand the error you're encountering. I have a few questions that should help clarify this.

  1. When you say you're updating the aws_codepipeline, is that updating from the AWS provider 2.x to 3.x, or is it modifying the resource using the same version of the provider?

  2. I notice that you have the OAuthToken field commented out and with a dummy value. With the changes introduced in v3.0 of the AWS provider, the field needs to be kept in the Terraform configuration. If it is not kept in the configuration, the existing value will be removed. (I've just noticed that there is some missing text in the upgrade instructions. Before the line $ TF_VAR_github_token=<token> terraform apply, it should say that the next two blocks are the v3.0 approach.)

If it is neither of these cases, can you provide the before-and-after Terraform configurations as well as indicate how the Github token is being passed to Terraform in both cases?

@luhn
Copy link

luhn commented Sep 22, 2020

I'm experiencing this too.

  1. Updating the resource, to trigger a change.
  2. Here's my configuration, field not commented:

data "aws_ssm_parameter" "token" {
  name = "/github/personal_access_token"
}

resource "aws_codepipeline" "main" {
  name     = local.name
  role_arn = aws_iam_role.codepipeline.arn

  artifact_store {
    location = aws_s3_bucket.artifacts.bucket
    type     = "S3"
  }

  stage {
    name = "Source"

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

      configuration = {
        Owner      = "owner"
        Repo       = "repo"
        Branch     = "master"
        OAuthToken = data.aws_ssm_parameter.token.value
      }
    }
  }

	# ...
}

On initial creation, source works fine. Change to resource and apply the update, now source is broken:

Screen Shot 2020-09-22 at 11 53 46 AM

If I taint and recreate the pipeline, the source works again.

@ybron
Copy link
Author

ybron commented Sep 24, 2020

Hi @gdavison, thanks for commenting.

Re: #1, This is after I have updated the provider to 3.0.0 and re-initialized. It is when I update the resource (by making changes to a step, adding actions, etc.) that the OAuthToken is removed.

Re: #2, that would explain why #1 happens... I had hoped that not providing it would leave it in-place as already established, or at least a notice that it was clearing it (maybe I just missed this in the output though?). Nobody wants to dig up the/an OAuthToken any/every time they make a change to the pipeline, nor check it into the code. From that perspective, it sounds like @luhn has a great solution, utilizing a data block. However, it seems like that may have a hitch or two remaining as well.

I probably misunderstood what was being addressed in #2854 - I thought that the hashed value of the token (or some similar non-plaintext version) would be kept in the state, without needing to keep providing it on every single apply.

@mchoi-truework
Copy link

Hello folks. I wanted to report a bug regarding this issue.

Below is a TF_LOG=TRACE of me going through an exercise of creating a CodePipeline via Terraform and then making an update to it (changing a name of an action). In the process, I'm seeing that Terraform module is sending the literal "hash-<value>" as the OAuthToken request parameter. Don't worry about the GitHub token value in the logs that token was only used for this exercise and has since been deleted.

Step 1: I'm creating a CodePipeline

2020-09-24T19:37:03.635-0700 [DEBUG] plugin.terraform-provider-aws_v3.8.0_x5: 2020/09/24 19:37:03 [DEBUG] [aws-sdk-go] DEBUG: Request codepipeline/CreatePipeline Details:
2020-09-24T19:37:03.636-0700 [DEBUG] plugin.terraform-provider-aws_v3.8.0_x5: {"pipeline":{"artifactStore":{"location":"mchoi-test-bucket","type":"S3"},"name":"mchoi-test","roleArn":"arn:aws:iam::<REDACTED>:role/mchoi-test-role","stages":[{"actions":[{"actionTypeId":{"category":"Source","owner":"ThirdParty","provider":"GitHub","version":"1"},"configuration":{"Branch":"master","OAuthToken":"4ead838fabf7f7474617d54dbd4d66c27bd5e31c","Owner":"mchoi-truework","PollForSourceChanges":"true","Repo":"mchoi-test"},"name":"Source","outputArtifacts":[{"name":"MyApp"}],"runOrder":1}],"name":"Source"},{"actions":[{"actionTypeId":{"category":"Approval","owner":"AWS","provider":"Manual","version":"1"},"configuration":{"CustomData":"n/a","ExternalEntityLink":"http://example.com","NotificationArn":"arn:aws:sns:us-east-2:<REDACTED>:mchoi-test-topic"},"name":"Approve","runOrder":1}],"name":"Approve"}]},"tags":[]}

It seems to do the correct thing and my CodePipeline is working well.

Step 2: Update the name of an action from "Approve" to "Approve2"

2020-09-24T19:44:44.834-0700 [DEBUG] plugin.terraform-provider-aws_v3.8.0_x5: 2020/09/24 19:44:44 [DEBUG] [aws-sdk-go] DEBUG: Request codepipeline/UpdatePipeline Details:
2020-09-24T19:44:44.834-0700 [DEBUG] plugin.terraform-provider-aws_v3.8.0_x5: {"pipeline":{"artifactStore":{"location":"mchoi-test-bucket","type":"S3"},"name":"mchoi-test","roleArn":"arn:aws:iam::<REDACTED>:role/mchoi-test-role","stages":[{"actions":[{"actionTypeId":{"category":"Source","owner":"ThirdParty","provider":"GitHub","version":"1"},"configuration":{"Branch":"master","OAuthToken":"hash-ddc67fffbc06433cf61b8d87cfa1b6ad7f88cd00d2b60a305f7643736d9ecfcb","Owner":"mchoi-truework","PollForSourceChanges":"true","Repo":"mchoi-test"},"name":"Source","outputArtifacts":[{"name":"MyApp"}],"runOrder":1}],"name":"Source"},{"actions":[{"actionTypeId":{"category":"Approval","owner":"AWS","provider":"Manual","version":"1"},"configuration":{"CustomData":"n/a","ExternalEntityLink":"http://example.com","NotificationArn":"arn:aws:sns:us-east-2:<REDACTED>:mchoi-test-topic"},"name":"Approve2","runOrder":1}],"name":"Approve"}]}}

As you can see it's sending the "hash-<value>" literal as the OAuthToken parameter. Here's a check:

$ echo -n '4ead838fabf7f7474617d54dbd4d66c27bd5e31c' | openssl sha256
ddc67fffbc06433cf61b8d87cfa1b6ad7f88cd00d2b60a305f7643736d9ecfcb

Step 3: Run terraform plan

No changes. Infrastructure is up-to-date.

I'm not sure what the correct fix would be but I wanted demonstrate that this is a bug and consequently when you configure a bogus token in AWS CodePipeline, the GitHub Source stage/action will not work. I look forward to what TF folks think. Thank you!

@CarloSimacan
Copy link

I face similar problems as described above.

Also, I noticed that Terraform shows the OAuthToken in its CLI/log output, but I think it should be marked as a sensitive value. Is this a small enough issue that can be addressed at the same time or should I make a new issue for it?

@woodrow
Copy link

woodrow commented Sep 29, 2020

I've also been affected by the bug mentioned by @mchoi-truework and @luhn. Please let us know if we should create a separate issue for it.

This bug unfortunately seems difficult to work around based on the current design of the aws_codepipeline resource provider. The unhashed OAuthToken value is needed for the AWS UpdatePipeline API call, but by Terraform's design it's not possible to access both the config value (unhashed token) and state value (hashed token) via the resourceData struct. Thus as @mchoi-truework called out above in #15200 (comment), once the aws_codepipeline resource is created and the hashed OAuthToken value is stored in the statefile, any further updates to the resource will pass the hashed token to UpdatePipeline and cause the CodePipeline's Github SourceAction to fail with the error shown in @luhn's comment #15200 (comment).

It seems like a useful fix for this bug could be removing the hashing and instead creating provider-specific configuration blocks to represent configuration objects, as mentioned here #14175 (comment), rather than treating the config as a map. This would then allow the OAuthToken to be marked as sensitive and hidden from output, which seems like it was the intent of #14175 in the first place. The token would still be stored in cleartext in the statefile, but that seems unavoidable based on the current design of Terraform.

@mwarkentin
Copy link
Contributor

mwarkentin commented Oct 6, 2020

AWS CodeStarConnections now support Github: #15453

Once you have a connection set up there you can use that as your source connection to bypass the entire OAuth song and dance:

stage {
    name = "Source"

    action {
      name             = "Source"
      category         = "Source"
      owner            = "AWS"
      provider         = "CodeStarSourceConnection"
      version          = "1"
      output_artifacts = ["source_output"]

      configuration = {
        ConnectionArn    = var.codestar_connection_arn
        FullRepositoryId = "${var.github_organization}/${var.repo_name}"
        BranchName       = var.branch_name
      }
    }
}

Edit: According the the AWS Docs this old way of connecting to Github is deprecated and will be unsupported "soon" (not sure what that means).

@CarloSimacan
Copy link

AWS CodeStarConnections now support Github: #15453

Once you have a connection set up there you can use that as your source connection to bypass the entire OAuth song and dance:

stage {
    name = "Source"

    action {
      name             = "Source"
      category         = "Source"
      owner            = "AWS"
      provider         = "CodeStarSourceConnection"
      version          = "1"
      output_artifacts = ["source_output"]

      configuration = {
        ConnectionArn    = var.codestar_connection_arn
        FullRepositoryId = "${var.github_organization}/${var.repo_name}"
        BranchName       = var.branch_name
      }
    }
}

Edit: According the the AWS Docs this old way of connecting to Github is deprecated and will be unsupported "soon" (not sure what that means).

Well, can't argue with AWS here, but this solution requires some manual steps to set up the connection. This is might be a minor inconvenience for one AWS account + connection. It gets troublesome when dealing with automation across multiple AWS accounts (and connections), which is common in larger organizations/enterprises.

@mwarkentin
Copy link
Contributor

You can reuse the one installed app on your GitHub org across all of your AWS accounts. Not sure if that's what you meant? If it's the manual set up process of the connection each one takes a minute or two, so even large-ish orgs should be doable.

Maybe Oauth Device Flow could simplify the initial set up if they supported that?

@CarloSimacan
Copy link

You can reuse the one installed app on your GitHub org across all of your AWS accounts. Not sure if that's what you meant? If it's the manual set up process of the connection each one takes a minute or two, so even large-ish orgs should be doable.

Maybe Oauth Device Flow could simplify the initial set up if they supported that?

Yes, it's the manual set-up process. Formerly, I worked at an enterprise where AWS accounts were created on-demand, in full automation and multiple times per day. In such an environment the central team managing AWS can quickly become a bottleneck if any manual steps get involved in the process. It's true that I describe a very specific situation here, but I think a central repository in GitHub with multiple CodePipeline 'listeners' would scale very well to do maintenance over many accounts in (such) an enterprise.

I had never heard of Oauth Device Flow tbh. I looked into it a bit and it seems quite some hassle to set-up. Especially considering that the same functionality could 'just work' out of the box by providing a token to CodePipeline.

@bhudgens
Copy link

Is there a mechanism to taint a record if TF detects a change or a lifecycle declaration we can set that forces a 'delete' on change to overcome this bug?

@gauravachatrath
Copy link

Hi Any update on fix of this bug ?

@bhudgens
Copy link

bhudgens commented Nov 5, 2020

It looks like the Github V1 provider in codepipeline is being deprecated which will likely result in this bug going away:

https://docs.aws.amazon.com/codepipeline/latest/userguide/update-github-action-connections.html

image

Related Issue(s):

#16042
#15453
#15960

@gdavison
Copy link
Contributor

gdavison commented Dec 10, 2020

Thanks for the deprecation information, @bhudgens. For better or worse, deprecated with AWS almost never means it's actually going away, so the provider will still likely need to be able to handle this.

There are several positive notes, however:

  1. When I made this change, I was trying to find a simple way to keep sensitive values out of the Terraform state, since we don't currently encrypt it beyond what the backend provides. This is discussed in https://www.terraform.io/docs/state/sensitive-data.html and https://www.terraform.io/docs/extend/best-practices/sensitive-state.html. The new features in Terraform 0.14 for better sensitive value handling (https://www.hashicorp.com/blog/announcing-hashicorp-terraform-0-14-general-availability#sensitive-input-variables-and-extended-provider-schema-sensitivity) will mask values in the output, but not state. The fix for this particular issue is actually a lot more simple than first thought.

  2. There is an open PR Add aws_codestarconnections_connection resource #15990 (based on #11389 Add codestar connection resource #12637) to add a CodeStar connection resource which can be used instead of the legacy GitHub v1 action provider.

@Zordrak
Copy link

Zordrak commented Dec 17, 2020

This absolutely remains a critical bug. There are significant permissions issues when using the CodeStar Connections approach with a GitHub Organization. The only way to provide discrete permission granularity without delegating your GitHub Organizations permissions to IAM is to use Personal Access Tokens, or some kind of nasty hack that pre-checks-out code with a Deploy Key and puts it into a bucket for the CodePipeline source stage to use, and the manual configuration requirement is also a problem.

Where this leaves us is the need to continue to use GitHub v1 connections in CodePipeline, and this is something we are addressing with our Technical Account Manager.

The problem we have is that any time we update a pipeline, we have to manually taint the pipeline so that it gets re-created because on every update the OAuth token is being broken.

Any support in getting this fixed so that we can continue to use this mechanism without breaking our pipelines would be fantastic.

@ghost
Copy link

ghost commented Jan 15, 2021

This has been released in version 3.24.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 Feb 14, 2021

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 as resolved and limited conversation to collaborators Feb 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/codepipeline Issues and PRs that pertain to the codepipeline service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.