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

Warn when input attributes is missing in /v1/data body #4386

Closed
anderseknert opened this issue Feb 25, 2022 · 4 comments · Fixed by #4416
Closed

Warn when input attributes is missing in /v1/data body #4386

anderseknert opened this issue Feb 25, 2022 · 4 comments · Fixed by #4416

Comments

@anderseknert
Copy link
Member

Every now and then we see users new to OPA missing the fact that /v1/data requires the JSON request body to wrap the input document, ala: {"input": {"x": "y"}}. Unless e.g. default assignment is used, OPA currently returns just an empty response for a request missing the "input" wrapper, i.e. {"x": "y"} ... it's easy to see why this would be, given how people likely have worked on their policy in the Rego Playground or using opa eval before running the server.

It would be an improvement if we informed the user of what's missing! Suggested response might look something like:

HTTP/1.1 200 OK
Content-Type: application/json

{
  "result": <JSON>,
  "warning": {
    "code": "api_usage_warning",
    "message": "The 'input' key was missing from the request. See <DOC_URL> for more details."
  }
}

Note that while it's tempting to return a 400 response, this would both break a lot of existing integrations, and would need to differentiate between input missing entirely (i.e. no JSON body) and JSON body but with missing input attribute. For that reason we'll keep the 200 response but with an added warning.

@aflmp
Copy link
Contributor

aflmp commented Mar 4, 2022

@anderseknert Can I work on this please? like others, I have also been bitten by this issue in the past.

@anderseknert
Copy link
Member Author

Sure! Let me know if you need any help along the way, either here or in the #development channel on the OPA slack.

@aflmp
Copy link
Contributor

aflmp commented Mar 4, 2022

from a quick parse, looks like we want to address the issue here ?

@anderseknert
Copy link
Member Author

anderseknert commented Mar 4, 2022

Yeah, at least you have the input variable there and can check if it's nil or not.. but since it shouldn't be treated as an error but rather just as a warning to add to the response, you'll probably want to do that check where the response is built, and append the warning to it... somewhere here.

aflmp pushed a commit to aflmp/opa that referenced this issue Mar 24, 2022
This change adds an api_usage_warning when input attribute is missing in the request to /v1/data

Fixes: open-policy-agent#4386
Signed-off-by: Alam <afaaqalam@gmail.com>

server: remove url from warning msg

Signed-off-by: Alam <afaaqalam@gmail.com>

server: remove warning for GET requests to v1/data; fix tests

Signed-off-by: Alam <afaaqalam@gmail.com>
anderseknert pushed a commit that referenced this issue Mar 24, 2022
This change adds an api_usage_warning when input attribute is missing in POST requests to /v1/data

Fixes: #4386
Signed-off-by: Alam <afaaqalam@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants