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

No caching for HTTP requests #1045

Open
claystation opened this issue Jan 21, 2025 · 4 comments · May be fixed by #1073
Open

No caching for HTTP requests #1045

claystation opened this issue Jan 21, 2025 · 4 comments · May be fixed by #1073
Labels
enhancement New feature or request

Comments

@claystation
Copy link

Hi!

This question was also posted in slack on 13th of january.

It seems Conftest does not support or do any caching for http.Send() requests, whilst OPA does.
I currently have a rule that needs to call an API endpoint to verify data against, e.g. see if the value in the YAML is in the list of values from the API endpoint.
Now to not be too tightly coupled with the API we want to check if the status code is 200 or not. If not, we put out a warn_ otherwise we actually evaluate a deny_ rule. This is to prevent failures if the API is down or any other network issue for example.

I tried a local setup with a simple http server in go exposing an endpoint and logging each hit on it.
And i have the following policy:

teams := http.send({
	"url": "http://localhost:8080/rego", // Just a test endpoint for debugging
	"method": "get",
	"headers": {
		"Authorization": concat(" ", ["Bearer", opa.runtime().env.apiToken])
	},
	"raise_error": false
})

team_names contains name if {
	name := teams.body[_].metadata.name
}

warn_team_request_failed contains msg if {
	teams.status_code != 200
	msg := "Request for teams failed. Skipping check."
}

deny_not_existing_team contains msg if {
	teams.status_code == 200
	parts := split(input.spec.owner, "/")
	parts[1]
	not parts[1] in team_names
	msg := sprintf("Owner value does not contain a valid team on %s %s", [input.kind, input.metadata.name])
}

When running this with conftest verify hits my http endpoint twice. With opa eval it only gets hit once.
Now hitting it twice is actually exactly what we dont want (the warn_ eval can succeed but the deny_ eval can error out and thus fail).

Is there reasoning for not doing caching on http requests?
If there is an actual reason for not doing caching, is there a way to handle this scenario?

Please let me know if you need more info/debugging! Happy to help. In the meantime, if i have the time, i might dive into the source to see if i can contribute myself!

@jalseth jalseth added enhancement New feature or request and removed enhancement New feature or request labels Feb 15, 2025
@jalseth
Copy link
Member

jalseth commented Feb 15, 2025

This can be added, but I'd like to better understand the problem first. Why is it an issue that the call is made twice? conftest verify is used to unit test the Rego policy, and caching in tests can lead to unexpected behavior.

@claystation
Copy link
Author

Hey @jalseth !

Thanks for the reply. I think i meant the test command, not verify (as there is no test in my described example).
Anyway, the caching helps reduce load on external API's and it makes it easier to check if the request failed and continue on error as i described.
If you have a better solution where caching is not involved but we still somehow can check if the request failed and ignore it if it did, please let me know!
If you need any more/other info, also let me know :)

@anderseknert
Copy link
Member

Adding

rego.InterQueryBuiltinCache(cache.NewInterQueryCacheWithContext(ctx, nil)),

at the end of the list of options here, should be enough (if no particular configuration / limit needs to be set).

@jalseth
Copy link
Member

jalseth commented Feb 19, 2025

Thanks for clarifying. I don't have any problem with adding the cache to the test command because it makes sense that a HTTP response should be considered valid for the duration of a conftest test run. I'll send a PR.

@jalseth jalseth linked a pull request Feb 19, 2025 that will close this issue
@jalseth jalseth added the enhancement New feature or request label Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants