Add a aws_codepipeline_webhook resource for managing CodePipeline webhooks

[joestump]

What does this add?

Introduces the aws_codepipeline_webhook resource to manage AWS CodePipeline webhooks.

Example HCL

# A shared secret between GitHub and AWS that allows AWS
# CodePipeline to authenticate the request came from GitHub.
# Would probably be better to pull this from the environment
# or something like SSM Parameter Store.
locals {
  webhook_secret = "super-secret"
}

resource "aws_codepipeline_webhook" "bar" {
  name            = "test-webhook-github-bar" 
  authentication  = "GITHUB_HMAC" 
  target_action   = "Source"
  target_pipeline = "${aws_codepipeline.bar.name}"

  authentication_configuration {
    secret_token = "${local.webhook_secret}"
  }

  filter {
    json_path    = "$.ref"
    match_equals = "refs/heads/{Branch}"
  }
}

# Wire the CodePipeline webhook into a GitHub repository.
resource "github_repository_webhook" "bar" {
  repository = "${github_repository.repo.name}"

  name = "web"

  configuration {
    url          = "${aws_codepipeline_webhook.bar.url}"
    content_type = "form"
    insecure_ssl = true 
    secret       = "${local.webhook_secret}"
  }
  events = ["push"]
}

Test output

make testacc TESTARGS='-run=TestAccAWSCodePipelineWebhook'
TF_ACC=1 go test ./... -v -run=TestAccAWSCodePipelineWebhook -timeout 120m
=== RUN   TestAccAWSCodePipelineWebhook_basic
--- PASS: TestAccAWSCodePipelineWebhook_basic (31.78s)
=== RUN   TestAccAWSCodePipelineWebhook_ipAuth
--- PASS: TestAccAWSCodePipelineWebhook_ipAuth (33.43s)
=== RUN   TestAccAWSCodePipelineWebhook_unauthenticated
--- PASS: TestAccAWSCodePipelineWebhook_unauthenticated (29.54s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	95.993s

TODO

• Create an acceptance test
• Write documentation
• Clean up PR with HCL example

[joestump]

Here's a complete working example. Push a buildspec.yml to kick off a pipeline run/build.

https://gist.github.com/joestump/cac3abb94050186fcba1c57c8a880a71

[joestump]

@bflad I took another pass at this one after your comments on #6117. Would love to get 👀 on this one as well. 😄

[bflad]

Hi @joestump 👋 Thanks for submitting this! Initial feedback below. Please reach out with any questions or if you do not have time to implement these items.

[bflad]

SchemaVersion should be omitted for new resources (it defaults to 0)

[bflad]

We prefer to allow the schema and any API errors to dictate this behavior rather than implementing additional error handling in the resource functions. authentication_configuration is marked as Required: true and we can allow the API (or the handling in extractCodePipelineWebhookAuthConfig) to return errors if both Optional nested arguments are omitted.

[bflad]

The schema handling for the auth_type argument prevents this case from ever getting hit.

[bflad]

If the API returns a sufficient error message, we should remove this 👍

[bflad]

Nit: In this project we tend to have "flatten" functions which perform the first half of this function, then leave the ResourceData handling in the Read function. e.g.

func flattenCodePipelineWebhookFilters(filters []*codepipeline.WebhookFilterRule) []interface{} {
// ...
}

// in Read
if err := d.Set("filter", flattenCodePipelineWebhookFilters(webhook.Filters)); err != nil {
  return fmt.Errorf("error setting filter: %s", err)
}

[bflad]

Other than the recommendation to use the AWS Go SDK helper functions for these, its also important to note that d.Set() can also directly handle pointers to simplify things and not require dereferencing (if the types match), e.g.

if err := d.Set("target_action", found.TargetAction); err != nil {
  return fmt.Errorf("error setting target_action: %s", err)
}

[joestump]

Good to know. For clarification, which is preferred?

d.Set("target_action", found.TargetAction)

or...

d.Set("target_action", aws.String(found.TargetAction))

[bflad]

Calling d.SetId("") during the Delete function is extraneous. 👍

[bflad]

We should add testing for the other authentication types as well. 👍 You may find its easiest to just make the "base" configuration above its own function so it doesn't need to be duplicated each test configuration.

[bflad]

Is this required for authentication = "UNAUTHENTICATED"?

[joestump]

It's not. I'll mark it as Optional: true instead.

[joestump]

@bflad I believe all comments have been addressed. There's a test for each authentication_configuration iteration now. 👍

[bflad]

Great work, @joestump! LGTM with two little things I'm fixing on merge so we can release this today. 🚀

(after mentioned changes)

--- PASS: TestAccAWSCodePipelineWebhook_basic (15.72s)
--- PASS: TestAccAWSCodePipelineWebhook_ipAuth (24.27s)
--- PASS: TestAccAWSCodePipelineWebhook_unauthenticated (25.03s)

[bflad]

Nit: Since we never return any errors we can just leave off that return (and its downstream handling) 😄

[bflad]

We only want Terraform to recreate the resource if the error indicates the resource was not found, otherwise we need to return the actual error message to the operator (e.g. AuthorizationDenied, InternalServiceError messages). Utilizing resource.NotFoundError should be enough to do the trick so we can do the following here:

// in getCodePipelineWebhook()
	return nil, &resource.NotFoundError{
		Message: fmt.Sprintf("No webhook with ARN %s found", arn),
	}

// in resourceAwsCodePipelineWebhookRead()
if isResourceNotFoundError(err) {
  log.Printf("[WARN] CodePipeline Webhook (%s) not found, removing from state", d.Id())
  d.SetId("")
  return nil
}

if err != nil {
  return fmt.Errorf("error getting CodePipeline Webhook (%s): %s", d.Id(), err)
}

[bflad]

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

[ghost]

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!