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

Add support for elasticsearch outbound connection and relevant accepter #22988

Merged

Conversation

Symbianx
Copy link
Contributor

@Symbianx Symbianx commented Feb 7, 2022

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 #13609

Output from acceptance testing:

$ make testacc TESTS="TestAccElasticsearchOutboundConnection_basic|TestAccElasticsearchInboundConnectionAccepter_basic" PKG=elasticsearch

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/elasticsearch/... -v -count 1 -parallel 20 -run='TestAccElasticsearchOutboundConnection_basic|TestAccElasticsearchInboundConnectionAccepter_basic'  -timeout 180m
=== RUN   TestAccElasticsearchInboundConnectionAccepter_basic
=== PAUSE TestAccElasticsearchInboundConnectionAccepter_basic
=== RUN   TestAccElasticsearchOutboundConnection_basic
=== PAUSE TestAccElasticsearchOutboundConnection_basic
=== CONT  TestAccElasticsearchInboundConnectionAccepter_basic
=== CONT  TestAccElasticsearchOutboundConnection_basic
--- PASS: TestAccElasticsearchInboundConnectionAccepter_basic (1406.07s)
--- PASS: TestAccElasticsearchOutboundConnection_basic (1441.27s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/elasticsearch      1443.891s

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. client-connections Pertains to the AWS Client and service connections. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/elasticsearch Issues and PRs that pertain to the elasticsearch service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/XL Managed by automation to categorize the size of a PR. labels Feb 7, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @Symbianx 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@justinretzolk justinretzolk 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 Feb 15, 2022
@Symbianx
Copy link
Contributor Author

Symbianx commented Mar 3, 2022

@justinretzolk I see you added a needs-triage label. What does this mean? Is there anything I can do to accelerate the review of this PR?

@justinretzolk
Copy link
Member

Hey @Symbianx 👋 Thank you for checking in on this, and thank you for your contribution! I was actually removing the needs-triage label in this case. That label is added automatically so that we have a chance to look at everything that comes in to determine whether or not it needs to be addressed urgently.

Unfortunately, I'm not able to provide an estimate on when this will be reviewed due to the potential of shifting priorities (we prioritize work by count of ":+1:" reactions, as well as a few other things). A larger prioritization document is in the works, but in the meantime additional information may be found in the FAQ. Outside of that, as soon as time permits, one of the engineers on the team will work on a full review so that we can get this merged in 🙂.

@Symbianx Symbianx force-pushed the f-elasticsearch-cross-cluster-search-support branch 2 times, most recently from c0e59e9 to ca6abcc Compare April 19, 2022 08:17
@github-actions github-actions bot added service/opensearch Issues and PRs that pertain to the opensearch service. and removed client-connections Pertains to the AWS Client and service connections. service/elasticsearch Issues and PRs that pertain to the elasticsearch service. labels Apr 19, 2022
@Symbianx Symbianx force-pushed the f-elasticsearch-cross-cluster-search-support branch from ca6abcc to f1c6ff6 Compare April 19, 2022 08:19
@Symbianx
Copy link
Contributor Author

Just rebased this PR with master which should make it easier to review

@Symbianx
Copy link
Contributor Author

Symbianx commented May 30, 2022

Hey @justinretzolk , sorry to bother you again.

This PR has been here for quite a while and I know you have a lot more PRs and issues to look at so I have one simple query.

Is there a known way for us to hook into the provider lifecycle to add our own resources/extensions without having to fork and build the whole repo?

Cloudformation doesn't support cross-cluster search either so the fact that we can't configure this via code in any way is really setting us back.

@justinretzolk
Copy link
Member

Hey @Symbianx 👋 No bother at all! That's what I'm here for, and I very much appreciate your understanding regarding prioritization 🙂.

The best way to be able to incorporate changes like this prior to them being merged in would indeed be to build the provider from source. Given that you've got a fork and the proposed change written, you could build and use a developer version of the provider using the steps outlined in the development environment setup documentation (which is further detailed in the Development Overrides for Provider Developers documentation).

I would recommend that if you decide to go this route, you only use this development version of the provider for the exact resources you need it for, so that you don't have too much work to move back to the main provider once the resources have been added (you'll need to do some terraform import and terraform state rm work at that point to reconcile the state).

@riy
Copy link

riy commented Jul 1, 2022

Hey @Symbianx,

Great job you did on this issue! I would love to help out with the feedback @justinretzolk gave, but unfortunately I'm quite new to the "building an AWS provider" world.
Do you have an ETA on when the PR could move ahead? In return I'd be more than happy to use this feature right away and give you feedback on how it went for our infra, if that's of any use to you. Thanks a bunch! :-)

@Symbianx
Copy link
Contributor Author

Symbianx commented Jul 5, 2022

@riy Heya
Unfortunately I'm also waiting for the PR to be reviewed, the way I see it the PR is ready to go but it needs to be looked at by the maintainers 😞
We haven't tried to build it ourselves yet since other priorities came through. Would be great to know if it did work out for you or not!

@jar-b jar-b self-assigned this Nov 11, 2022
@jar-b jar-b force-pushed the f-elasticsearch-cross-cluster-search-support branch from f1c6ff6 to a314303 Compare November 11, 2022 21:13
@jar-b jar-b force-pushed the f-elasticsearch-cross-cluster-search-support branch from 943362a to 59f43bc Compare November 11, 2022 21:46
Copy link
Member

@jar-b jar-b left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

$ make testacc PKG=opensearch TESTS="TestAccOpenSearchOutboundConnection_|TestAccOpenSearchInboundConnectionAccepter_"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/opensearch/... -v -count 1 -parallel 20 -run='TestAccOpenSearchOutboundConnection_|TestAccOpenSearchInboundConnectionAccepter_'  -timeout 180m
=== RUN   TestAccOpenSearchInboundConnectionAccepter_basic
=== PAUSE TestAccOpenSearchInboundConnectionAccepter_basic
=== RUN   TestAccOpenSearchInboundConnectionAccepter_disappears
=== PAUSE TestAccOpenSearchInboundConnectionAccepter_disappears
=== RUN   TestAccOpenSearchOutboundConnection_basic
=== PAUSE TestAccOpenSearchOutboundConnection_basic
=== RUN   TestAccOpenSearchOutboundConnection_disappears
=== PAUSE TestAccOpenSearchOutboundConnection_disappears
=== CONT  TestAccOpenSearchInboundConnectionAccepter_basic
=== CONT  TestAccOpenSearchOutboundConnection_basic
=== CONT  TestAccOpenSearchOutboundConnection_disappears
=== CONT  TestAccOpenSearchInboundConnectionAccepter_disappears
--- PASS: TestAccOpenSearchOutboundConnection_disappears (1622.82s)
--- PASS: TestAccOpenSearchOutboundConnection_basic (1636.86s)
--- PASS: TestAccOpenSearchInboundConnectionAccepter_disappears (1803.42s)
--- PASS: TestAccOpenSearchInboundConnectionAccepter_basic (1877.88s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/opensearch 1880.647s

@jar-b jar-b merged commit 4424d42 into hashicorp:main Nov 11, 2022
@jar-b
Copy link
Member

jar-b commented Nov 11, 2022

Hey @Symbianx, thanks for your contribution! 👏 👏

This was in great shape, I just made a few small adjustments:

  • Updated acceptance tests to use ProtoV5ProviderFactories
  • Added ImportVerify step to _basic tests
  • Added _disappears tests
  • Added some extra nil handling in the domain info flattener
  • Minor documentation cleanup
  • Addressed various linter findings

@github-actions github-actions bot added this to the v4.40.0 milestone Nov 11, 2022
@github-actions
Copy link

This functionality has been released in v4.40.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. Thank you!

@LeonD9
Copy link

LeonD9 commented Dec 13, 2022

Hey @Symbianx Is it possible to import inbound connection in a different region then of the local opensearch?
I get provider error when trying to import inbound connection between USE and EUC regions:

│ Error: Request cancelled
│
│ The plugin.(*GRPCProvider).ReadResource request was cancelled.
╵

Releasing state lock. This may take a few moments...

Stack trace from the terraform-provider-aws_v4.46.0_x5 plugin:

panic: interface conversion: interface {} is nil, not *opensearchservice.InboundConnection

goroutine 370 [running]:
github.com/hashicorp/terraform-provider-aws/internal/service/opensearch.resourceInboundConnectionRead({0xe71f7c8, 0xc0054be5a0}, 0xc0054b7c00, {0xc7f8d80?, 0xc0003b8c00?})
	github.com/hashicorp/terraform-provider-aws/internal/service/opensearch/inbound_connection_accepter.go:86 +0x2a8
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0xc001842c40, {0xe71f800, 0xc0054eba70}, 0xd?, {0xc7f8d80, 0xc0003b8c00})
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.24.1/helper/schema/resource.go:724 +0x12e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).RefreshWithoutUpgrade(0xc001842c40, {0xe71f800, 0xc0054eba70}, 0xc00522cf70, {0xc7f8d80, 0xc0003b8c00})
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.24.1/helper/schema/resource.go:1015 +0x585
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadResource(0xc000010f48, {0xe71f800?, 0xc0054eb950?}, 0xc000f5db00)
	github.com/hashicorp/terraform-plugin-sdk/v2@v2.24.1/helper/schema/grpc_provider.go:613 +0x4a5
github.com/hashicorp/terraform-plugin-mux/tf5muxserver.muxServer.ReadResource({0xc0030f6bd0, 0xc0030f6c30, {0xc00313f860, 0x2, 0x2}, 0xc0030f6c00, 0xc002560b00, 0xc0032d6900, 0xc0030f6c60}, {0xe71f800, ...}, ...)
	github.com/hashicorp/terraform-plugin-mux@v0.7.0/tf5muxserver/mux_server_ReadResource.go:26 +0x142
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0xc00091ea00, {0xe71f800?, 0xc0054eaf30?}, 0xc0054be2a0)
	github.com/hashicorp/terraform-plugin-go@v0.14.2/tfprotov5/tf5server/server.go:748 +0x4b1
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadResource_Handler({0xd161ce0?, 0xc00091ea00}, {0xe71f800, 0xc0054eaf30}, 0xc00099f180, 0x0)
	github.com/hashicorp/terraform-plugin-go@v0.14.2/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:349 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0xc00053d680, {0xe72a200, 0xc000f42680}, 0xc0054f0360, 0xc003d3b2f0, 0x14acbbf0, 0x0)
	google.golang.org/grpc@v1.51.0/server.go:1340 +0xd23
google.golang.org/grpc.(*Server).handleStream(0xc00053d680, {0xe72a200, 0xc000f42680}, 0xc0054f0360, 0x0)
	google.golang.org/grpc@v1.51.0/server.go:1713 +0xa2f
google.golang.org/grpc.(*Server).serveStreams.func1.2()
	google.golang.org/grpc@v1.51.0/server.go:965 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
	google.golang.org/grpc@v1.51.0/server.go:963 +0x28a

Error: The terraform-provider-aws_v4.46.0_x5 plugin crashed!

@github-actions
Copy link

github-actions bot commented Feb 3, 2023

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 Feb 3, 2023
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. provider Pertains to the provider itself, rather than any interaction with AWS. service/opensearch Issues and PRs that pertain to the opensearch service. size/XL 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.

Elasticsearch cross-cluster search
6 participants