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

r/aws_appsync_resolver: Support direct resolver #14710

Merged
merged 2 commits into from
Apr 29, 2021

Conversation

shuheiktgw
Copy link
Collaborator

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment 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 pull request followers and do not help prioritize the request

Closes #14488

Release note for CHANGELOG:

r/aws_appsync_resolver: Support direct resolver

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAwsAppsyncResolver'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAwsAppsyncResolver -timeout 120m
=== RUN   TestAccAwsAppsyncResolver_basic
=== PAUSE TestAccAwsAppsyncResolver_basic
=== RUN   TestAccAwsAppsyncResolver_disappears
=== PAUSE TestAccAwsAppsyncResolver_disappears
=== RUN   TestAccAwsAppsyncResolver_DataSource
=== PAUSE TestAccAwsAppsyncResolver_DataSource
=== RUN   TestAccAwsAppsyncResolver_DataSource_lambda
=== PAUSE TestAccAwsAppsyncResolver_DataSource_lambda
=== RUN   TestAccAwsAppsyncResolver_RequestTemplate
=== PAUSE TestAccAwsAppsyncResolver_RequestTemplate
=== RUN   TestAccAwsAppsyncResolver_ResponseTemplate
=== PAUSE TestAccAwsAppsyncResolver_ResponseTemplate
=== RUN   TestAccAwsAppsyncResolver_multipleResolvers
=== PAUSE TestAccAwsAppsyncResolver_multipleResolvers
=== RUN   TestAccAwsAppsyncResolver_PipelineConfig
=== PAUSE TestAccAwsAppsyncResolver_PipelineConfig
=== RUN   TestAccAwsAppsyncResolver_CachingConfig
=== PAUSE TestAccAwsAppsyncResolver_CachingConfig
=== CONT  TestAccAwsAppsyncResolver_basic
=== CONT  TestAccAwsAppsyncResolver_ResponseTemplate
=== CONT  TestAccAwsAppsyncResolver_disappears
=== CONT  TestAccAwsAppsyncResolver_CachingConfig
=== CONT  TestAccAwsAppsyncResolver_DataSource_lambda
=== CONT  TestAccAwsAppsyncResolver_RequestTemplate
=== CONT  TestAccAwsAppsyncResolver_PipelineConfig
=== CONT  TestAccAwsAppsyncResolver_multipleResolvers
=== CONT  TestAccAwsAppsyncResolver_DataSource
    TestAccAwsAppsyncResolver_disappears: resource_aws_appsync_resolver_test.go:50: [INFO] Got non-empty plan, as expected
--- PASS: TestAccAwsAppsyncResolver_disappears (44.45s)
--- PASS: TestAccAwsAppsyncResolver_CachingConfig (49.63s)
--- PASS: TestAccAwsAppsyncResolver_basic (58.12s)
--- PASS: TestAccAwsAppsyncResolver_multipleResolvers (71.85s)
--- PASS: TestAccAwsAppsyncResolver_DataSource_lambda (77.05s)
--- PASS: TestAccAwsAppsyncResolver_DataSource (81.25s)
--- PASS: TestAccAwsAppsyncResolver_ResponseTemplate (88.90s)
--- PASS: TestAccAwsAppsyncResolver_PipelineConfig (92.33s)
--- PASS: TestAccAwsAppsyncResolver_RequestTemplate (94.25s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	95.983s
...

Thank you for your review! 👍

@shuheiktgw shuheiktgw requested a review from a team August 17, 2020 23:48
@ghost ghost added size/M Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. service/appsync Issues and PRs that pertain to the appsync service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. labels Aug 17, 2020
Copy link

@liamross liamross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just about to open a PR with these changes before searching and finding yours... This is exactly how I implemented it though, looks great! (only difference was that I put the two template GetOk checks above pipeline_config since they are in that order within the schema, but that's a nitpick).

Can we please get this PR through? I'm trying to use Terraform with AppSync and this is a blocker for me.

@liamross
Copy link

If anyone finds this and wants a hacky solution in the meantime, they can use the following templates to basically imitate the direct resolver pass-through object (taken from this repo):

resource "aws_appsync_resolver" "resolver" {
  # ...other properties

  request_template = <<-EOF
  {
    "version" : "2017-02-28",
    "operation": "Invoke",
    "payload": {
      "arguments": $util.toJson($ctx.arguments),
      "identity": $util.toJson($ctx.identity),
      "source": $util.toJson($ctx.source),
      "request": $util.toJson($ctx.request),
      "prev": $util.toJson($ctx.prev),
      "info": {
          "selectionSetList": $util.toJson($ctx.info.selectionSetList),
          "selectionSetGraphQL": $util.toJson($ctx.info.selectionSetGraphQL),
          "parentTypeName": $util.toJson($ctx.info.parentTypeName),
          "fieldName": $util.toJson($ctx.info.fieldName),
          "variables": $util.toJson($ctx.info.variables)
      },
      "stash": $util.toJson($ctx.stash)
    }
  }
  EOF

  response_template = <<-EOF
  $util.toJson($ctx.result)
  EOF
}

Comment on lines 111 to 112
* `request_template` - (Optional) The request mapping template for UNIT resolver or 'before mapping template' for PIPELINE resolver.
* `response_template` - (Optional) The response mapping template for UNIT resolver or 'after mapping template' for PIPELINE resolver.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually might be worth stating that these are only really option if type = "AWS_LAMBDA" (although there's no code-level check for that atm -- not sure if needed since it should just fail on AWS side during apply, but nice if we want feedback within terraform). Also might be nice to add an example to the code blocks on this page for "Direct to lambda resolver" or something.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree here - we should probably note the requirement for non-Lambda resolvers. 👍

@ghost
Copy link

ghost commented Jan 21, 2021

@liamross thank you for development. It is also an issue for me. Can you @mitchellh please accept this PR?

Base automatically changed from master to main January 23, 2021 00:58
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:58
@bflad bflad self-assigned this Apr 29, 2021
Copy link
Member

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, @shuheiktgw, just going to make these minor adjustments and re-verify 👍

aws/resource_aws_appsync_resolver_test.go Show resolved Hide resolved
Comment on lines 111 to 112
* `request_template` - (Optional) The request mapping template for UNIT resolver or 'before mapping template' for PIPELINE resolver.
* `response_template` - (Optional) The response mapping template for UNIT resolver or 'after mapping template' for PIPELINE resolver.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree here - we should probably note the requirement for non-Lambda resolvers. 👍

website/docs/r/appsync_resolver.html.markdown Outdated Show resolved Hide resolved
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Apr 29, 2021
@@ -129,6 +127,14 @@ func resourceAwsAppsyncResolverCreate(d *schema.ResourceData, meta interface{})
}
}

if v, ok := d.GetOk("request_template"); ok {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, we will also need to do the same in Update 👍

@bflad
Copy link
Member

bflad commented Apr 29, 2021

Re-verified 🚀

Output from acceptance testing:

--- PASS: TestAccAwsAppsyncResolver_CachingConfig (21.06s)
--- PASS: TestAccAwsAppsyncResolver_disappears (22.72s)
--- PASS: TestAccAwsAppsyncResolver_PipelineConfig (30.25s)
--- PASS: TestAccAwsAppsyncResolver_DataSource (35.13s)
--- PASS: TestAccAwsAppsyncResolver_multipleResolvers (38.57s)
--- PASS: TestAccAwsAppsyncResolver_basic (46.53s)
--- PASS: TestAccAwsAppsyncResolver_ResponseTemplate (47.66s)
--- PASS: TestAccAwsAppsyncResolver_DataSource_lambda (50.22s)
--- PASS: TestAccAwsAppsyncResolver_RequestTemplate (58.62s)

@bflad bflad merged commit 0679f4c into hashicorp:main Apr 29, 2021
@github-actions github-actions bot added this to the v3.38.0 milestone Apr 29, 2021
@ghost
Copy link

ghost commented Apr 30, 2021

This has been released in version 3.38.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!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/appsync Issues and PRs that pertain to the appsync service. size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support direct resolvers in aws_appsync_resolver resource
3 participants