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.

Sign up for GitHub

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

Jump to bottom

OPA server ready check doesn't account for discovery bundles #2015

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

OPA server ready check doesn't account for discovery bundles #2015

patrick-east opened this issue Jan 17, 2020 · 0 comments · Fixed by #2041
Assignees
@patrick-east
Labels
bug

Comments

@patrick-east
Copy link
Contributor

Expected Behavior

If configuring a ready check for OPA using /health?bundle=true and a discovery service that requires a bundle to be downloaded, and then additional normal bundles, I expect that it wouldn't return true until after all the bundles are activated for the first time.

Actual Behavior

The ready check including bundles /health?bundle=true doesn't account for discovery bundles, or bundles found after discovery.

Steps to Reproduce the Problem

  1. Run OPA with a bad discovery config (so it can never download a discovery bundle)
opa run --set services.local.url=http://localhost:8080 --set discovery.service=local --set discovery.name=local -s
  1. Curl OPA's health endpoint
curl -v 'localhost:8181/health?bundle=true'
  1. Observe a 200 OK response
@patrick-east patrick-east self-assigned this Jan 17, 2020
@tsandall tsandall mentioned this issue Jan 21, 2020
Closed
patrick-east added a commit to patrick-east/opa that referenced this issue Jan 28, 2020
@patrick-east
server: Update health check to use plugin status
109355d 
The health check now supports a `?plugin` option which will make the
response depend on whether or not all configured plugins are in an OK
state.

The `bundle` parameter will now use the bundle *and* discovery plugin
statuses to determine if the bundles are ready. This corrects an issue
where discovery bundles, and bundles defined by the discovery dynamic
config, were not included with `/health?bundle=true` checks.

The URL parameter parsing has also changed to allow for omitting the
value for the `bundle` option. It will default to `true` so that
`/health?bundle=true` can be shortened to `/health?bundle`.

Fixes: open-policy-agent#2010
Fixes: open-policy-agent#2015
Signed-off-by: Patrick East <east.patrick@gmail.com>
@patrick-east patrick-east mentioned this issue Jan 28, 2020
Merged
patrick-east added a commit to patrick-east/opa that referenced this issue Feb 7, 2020
@patrick-east
server: Update health check to use plugin status
cb05217 
The health check now supports a `?plugin` option which will make the
response depend on whether or not all configured plugins are in an OK
state.

The `bundle` parameter will now use the bundle *and* discovery plugin
statuses to determine if the bundles are ready. This corrects an issue
where discovery bundles, and bundles defined by the discovery dynamic
config, were not included with `/health?bundle=true` checks.

The URL parameter parsing has also changed to allow for omitting the
value for the `bundle` option. It will default to `true` so that
`/health?bundle=true` can be shortened to `/health?bundle`.

Fixes: open-policy-agent#2010
Fixes: open-policy-agent#2015
Signed-off-by: Patrick East <east.patrick@gmail.com>
tsandall pushed a commit that referenced this issue Feb 7, 2020
@patrick-east @tsandall
server: Update health check to use plugin status
c26db82 
The health check now supports a `?plugin` option which will make the
response depend on whether or not all configured plugins are in an OK
state.

The `bundle` parameter will now use the bundle *and* discovery plugin
statuses to determine if the bundles are ready. This corrects an issue
where discovery bundles, and bundles defined by the discovery dynamic
config, were not included with `/health?bundle=true` checks.

The URL parameter parsing has also changed to allow for omitting the
value for the `bundle` option. It will default to `true` so that
`/health?bundle=true` can be shortened to `/health?bundle`.

Fixes: #2010
Fixes: #2015
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
Assignees

@patrick-east patrick-east

Labels
bug
Projects
Open Policy Agent
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add plugin status plumbing and update server to use it patrick-east/opa
1 participant
@patrick-east