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

topdown/http: Respect raise_error flag during input validation #6553

Merged

Conversation

ashutosh-narkar
Copy link
Member

Why the changes in this PR are needed?

Currently the raise_error flag is not honored during the input validation step. So http.send will return an error if input validation fails irrespective of the raise_error flag status.

What are the changes in this PR?

This change takes into account raise_error flag status during the input validation phase.

Further comments:

The description of the raise_error flag is updated to reflect actual behavior.

Copy link

netlify bot commented Jan 26, 2024

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit 8a8b9ff
🔍 Latest deploy log https://app.netlify.com/sites/openpolicyagent/deploys/65b7df4c1ebc730008d66d0d
😎 Deploy Preview https://deploy-preview-6553--openpolicyagent.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

johanfylling
johanfylling previously approved these changes Jan 29, 2024
Copy link
Contributor

@johanfylling johanfylling left a comment

Choose a reason for hiding this comment

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

Is this change in response to a raised user issue?

Naïvely, I'd expected raise_error to only affect issues when making the HTTP call, and not to function argument validation. I.e. any error in the argument that can be detected before a call is made would always raise an error, regardless of the value of raise_error, as this would more likely fall within the categories of programmer- or configuration error.

I see, however, that this isn't the precedence, as e.g. force_cache_duration_seconds/force_cache error reporting already is affected by raise_error.

topdown/http_test.go Show resolved Hide resolved
docs/content/policy-reference.md Show resolved Hide resolved
@anderseknert
Copy link
Member

I agree with @johanfylling — I would not expect raise_error to impact input validation, but something that strict-builtin-errors would surface.

Currently the `raise_error` flag is not honored during the
input validation step. So `http.send` will return an error if
input validation fails irrespective of the `raise_error` flag
status. This change attempts to fix that.
Also the description of the `raise_error` flag is updated to
reflect actual behavior.

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
@ashutosh-narkar ashutosh-narkar merged commit c99f599 into open-policy-agent:main Jan 29, 2024
25 checks passed
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.

None yet

3 participants