Worker pool data source ignores the name field

[itaispiegel]

trafficstars

Describe the bug The worker pool data source can query the worker pool by its name, but this field is completely ignored.

Steps to reproduce Query some worker pool and output it, with the following blocks:

data "octopusdeploy_worker_pools" "my_worker_pool" {
  name = "my_worker_pool"
}

output "worker_pool" {
  value = data.octopusdeploy_worker_pools.my_worker_pool.worker_pools
}

Expected behavior I expect the output to contain a single worker pool with the queried name.

Logs and other supporting information Instead of the queried pool, it will receive a list of workers that don't necessarily have the expected name.

Screenshots Irrelevant.

Environment and versions:

• OS: Doesn't matter.
• Octopus Server Version: v2023.3 (Build 10333).
• Terraform Version: v1.5.2
• Octopus Terraform Provider Version: v0.12.5

Additional context To my understanding, it seems that the bug is caused because the Name parameter of the WorkerPoolsQuery struct is completely ignored here.

[itaispiegel]

Still waiting for a response on this...

[mjhilton]

Hey @itaispiegel, sorry for the massive delay on looking at this, and for the bad experience with this data source.

You're exactly right: the name field is ignored. I've had a look, and it's because the WorkerPools GET endpoint (https://<yourinstance>.octopus.app/swaggerui/index.html#operations-WorkerPools-getWorkerPoolsBySpace) doesn't actually have a name query param to map through to.

As I'm triaging the Issues in this repo, the consistency of support for name and partial_name is coming up as a repeated thing on a few data sources. I'd like us to improve this.

I've put a review and suggested change on your PR - if it's still something you're needing, please have a look and I'll help you get it merged.

We may need to change the approach in the longer term because client-side filtering isn't great for performance, but at least it would help you out for now.

[itaispiegel]

@mjhilton Thanks a lot for the response! I just fixed the logic error you found so I guess it's ready to be merged 😄