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

Support tag and status filters for netbox_prefixes data source #367

Merged
merged 3 commits into from
Mar 26, 2023

Conversation

kyle-burnett
Copy link
Contributor

@kyle-burnett kyle-burnett commented Mar 23, 2023

This PR adds support for using tag and status values as filters for the existing netbox_prefixes data source.

This PR also suggests removing the requirement that the data source must return at least 1 prefix as a result, and instead allows it to support returning 0 results:

	if *res.GetPayload().Count == int64(0) {
		return errors.New("no result")
	}

There seems to be some inconsistency across the data sources in regards to this behavior - some data sources support returning 0 results, and others don't.

I'll defer to the maintainers on whether or not we can remove the above check, but IMO, the provider should not error out of a plan just because a particular data source might return 0 results (especially since the Netbox API itself support this).

If a user has a requirement that a data source must return results, then IMO they should implement logic locally to account for that rather than having the provider force the behavior.

@fbreckle
Copy link
Collaborator

We already had a discussion about zero-result data sources in #42.
The points there still stand and eventually we should establish a consistent standard. I created a stub at https://github.com/e-breuninger/terraform-provider-netbox/wiki/General-data-source-structure

For the time being, let's just embrace inconsistency and merge, since it seems to be a requirement you need!

@fbreckle fbreckle merged commit 5a1ff75 into e-breuninger:master Mar 26, 2023
@kyle-burnett
Copy link
Contributor Author

Thanks @fbreckle! I had not seen that previous discussion so thanks for linking to it.

The latest word from Hashicorp that I can find on 0-result data sources is here - hashicorp/terraform#30291 (comment)

twink0r pushed a commit to twink0r/terraform-provider-netbox that referenced this pull request Sep 15, 2023
…uninger#367)

* Support tag and status filters for netbox_prefixes data source

* cleanup

* go generate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants