consul icon indicating copy to clipboard operation
consul copied to clipboard

Allow reading/updating/deleting a PreparedQuery by exact name

Open jamestran201 opened this issue 3 years ago • 6 comments

Related to: #10531

Background context

When a PreparedQuery is created and managed by a tool such as Ansible, its ID is not known ahead of time. This can make it difficult for the user to manage this query in their scripts. For this use case, the user currently manages these queries manually.

The issue above proposes adding endpoints to read/update/delete a query using its exact name to help the user with this use case, see this comment for more details.

Changes in this PR

This PR adds the ability to get a query using its exact name. The implementation is adapted from the code that retrieves a query by its ID.

The current changes in this PR:

  • Add a REST endpoint to retrieve the query using its exact name. The endpoint path is /v1/query/exact-name/{query-name}.
  • Add a RPC endpoint to retrieve the query using its exact name. This RPC endpoint is called PreparedQuery.GetByExactName.
  • Add a function to retrieve the query from the state store using its name.
  • Add a function to the api package to call the new endpoint
  • Add API doc for GET endpoint

To do

  • Implement endpoint to update a query given its name
  • Implement endpoint to delete a query given its name

Questions

  • ~~The implementation for reading a query given its name has a lot of duplication with reading the query by its ID. Should the common code be extracted?~~ Answered by https://github.com/hashicorp/consul/pull/12053#discussion_r795790995
  • ~~Should the code for updating and deleting a query be implemented in this PR or separate ones?~~ Answered by https://github.com/hashicorp/consul/pull/12053#discussion_r795790995

jamestran201 avatar Jan 13 '22 01:01 jamestran201

As for your questions, 1: We're fine with duplicate code for now; we prefer not to prematurely refactor code until necessary (to avoid coupling code that may diverge later - we might see some of that with ACL auth here) 2: This PR is fine; update/delete can come after we've reviewed the ACL implications in this PR.

Thank you for your patience!

kisunji avatar Jan 31 '22 15:01 kisunji

Hey @tmnhat2001 thanks for your patience here.

Our inclination is that we probably want to have ACL permissions checking for prepared query read when getting a prepared query by name but we want to do more research into this.

acpana avatar Feb 08 '22 20:02 acpana

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes


1 out of 2 committers have signed the CLA.

  • [x] jamestran201
  • [ ] gandalf-rick-and-morty

Have you signed the CLA already but the status is still pending? Recheck it.

hashicorp-cla avatar Mar 12 '22 16:03 hashicorp-cla

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

hashicorp-cla avatar Mar 12 '22 16:03 hashicorp-cla

@markan @kisunji Sorry for the late reply 🙇‍♂️ . I recently changed jobs so haven't been able to work on this. Thank you for you guidance with ACL! I will start implementing it.

jamestran201 avatar Apr 17 '22 02:04 jamestran201

This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions.

github-actions[bot] avatar Sep 02 '22 01:09 github-actions[bot]

This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions.

github-actions[bot] avatar Jul 10 '24 01:07 github-actions[bot]