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

OPA REST API decision log path incorrect for non-standard package names #2031

Closed
patrick-east opened this issue Jan 23, 2020 · 0 comments · Fixed by #2032
Closed

OPA REST API decision log path incorrect for non-standard package names #2031

patrick-east opened this issue Jan 23, 2020 · 0 comments · Fixed by #2032
Assignees
Labels

Comments

@patrick-east
Copy link
Contributor

Similar to #2027 but more specific to the HTTP server in OPA.

Expected Behavior

With a policy like:

package policy["com.foo.acme.ingress"].main

p := 1

When I make a REST API call to /v1/data/policy/com.foo.acme.ingress/main/p I should see a decision log entry with path set to policy/com.foo.acme.ingress/main/p

Actual Behavior

The decision log path swaps all .'s to /'s so the path ends up being incorrect:

{
  "client_addr": "[::1]:50880",
  "level": "info",
  "msg": "Received request.",
  "req_id": 5,
  "req_method": "GET",
  "req_path": "/v1/data/policy/com.foo.acme.ingress/main/p",
  "time": "2020-01-23T11:43:34-08:00"
}
{
  "decision_id": "9b5e3cf5-d1b1-4114-b680-32d6e5b533b8",
  "labels": {
    "id": "db93a45f-47f6-4a7c-8e43-9fd5aaeed43b",
    "version": "0.17.0-dev"
  },
  "level": "info",
  "metrics": {
    "counter_server_query_cache_hit": 0,
    "timer_rego_input_parse_ns": 362,
    "timer_rego_query_compile_ns": 83207,
    "timer_rego_query_eval_ns": 22537,
    "timer_rego_query_parse_ns": 693133,
    "timer_server_handler_ns": 844860
  },
  "msg": "Decision Log",
  "path": "policy/com/foo/acme/ingress/main/p",
  "requested_by": "[::1]:50880",
  "result": 1,
  "time": "2020-01-23T11:43:34-08:00",
  "timestamp": "2020-01-23T19:43:34.947269Z",
  "type": "openpolicyagent.org/decision_logs"
}

Steps to Reproduce the Problem

  1. Run opa server with policy as described above, and decision logger enabled:
    opa run -s --log-format json-pretty --set decision_logs.console=true /tmp/policy.rego
  2. Make a request with the data API path including the non-standard package name:
    curl -s localhost:8181/v1/data/policy/com.foo.acme.ingress/main/p
  3. Observe the decision log output from the server
@patrick-east patrick-east self-assigned this Jan 23, 2020
patrick-east added a commit to patrick-east/opa that referenced this issue Jan 23, 2020
The logger was swapping `.`'s with `/`'s but this isn't safe when a
valid path should be /foo/a.b.c/main. The server was already doing
the right thing by passing in the url path where applicable, or only
specifying a query instead of the path.

This might affect anyone using the decision logger golang API passing
in something in dot-notation and expecting it to come out with paths.
Anyone using the HTTP server should be unaffected.

Fixes: open-policy-agent#2031
Signed-off-by: Patrick East <east.patrick@gmail.com>
tsandall pushed a commit that referenced this issue Jan 30, 2020
The logger was swapping `.`'s with `/`'s but this isn't safe when a
valid path should be /foo/a.b.c/main. The server was already doing
the right thing by passing in the url path where applicable, or only
specifying a query instead of the path.

This might affect anyone using the decision logger golang API passing
in something in dot-notation and expecting it to come out with paths.
Anyone using the HTTP server should be unaffected.

Fixes: #2031
Signed-off-by: Patrick East <east.patrick@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant