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

Feature Request: add health checks to oidc-discovery-provider #3121

Closed
chdalski opened this issue May 31, 2022 · 3 comments
Closed

Feature Request: add health checks to oidc-discovery-provider #3121

chdalski opened this issue May 31, 2022 · 3 comments
Labels
help wanted Issues with this label are ready to start work but are in need of someone to do it priority/backlog Issue is approved and in the backlog

Comments

@chdalski
Copy link
Contributor

Currently the oidc-discovery-provider has no purposive health check endpoints.

It would be nice if the oidc-discovery-provider would get similar configurable endpoints as the server/agent, which currently are:

health_checks {
        listener_enabled = true
        bind_address = "localhost"
        bind_port = "8080"
        live_path = "/live"
        ready_path = "/ready"
}

See: https://github.com/spiffe/spire/blob/main/doc/spire_server.md#health-check-configuration

  • Version: 1.3.0
  • Platform: k8s
  • Subsystem: oidc-discovery-provider
@azdagron azdagron added help wanted Issues with this label are ready to start work but are in need of someone to do it priority/backlog Issue is approved and in the backlog labels May 31, 2022
@azdagron
Copy link
Member

I think this is very reasonable. Happy to discuss a design here.

@chdalski
Copy link
Contributor Author

chdalski commented Jun 8, 2022

@azdagron From my understanding so far the oidc-discovery-provider is, contrary to the spire server/agent, a single web-service process.
So for "ready" I would expect something as simple as http 200 if the service is up and running.
For the "live" part my understanding would be that the web-service can successfully access the workload api, meaning that /keys and /.well-known/openid-configuration endpoints are served, while it's not an assurance that the configuration is correct at all.

Would that be sufficient?

@azdagron
Copy link
Member

azdagron commented Jun 8, 2022

That sounds good to me. As far as the configuration shape, SPIREs configuration shape here has some historic choices that we can probably avoid until necessary. I think to start out we could probably elide listener_enabled and bind_address. The former can be assumed if the health_checks section is present at all, and the latter should probably always be localhost unless there is a compelling reason to expose the health endpoint off-box.

I think the other configuration keys could have reasonable defaults as well, so at a minimum, to enable health checks, all you'd have to do is add the following:

health_checks {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues with this label are ready to start work but are in need of someone to do it priority/backlog Issue is approved and in the backlog
Projects
None yet
Development

No branches or pull requests

2 participants