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

Add authorization for metrics endpoint #2073

Closed
sbueringer opened this issue Nov 28, 2022 · 6 comments · Fixed by #2407
Closed

Add authorization for metrics endpoint #2073

sbueringer opened this issue Nov 28, 2022 · 6 comments · Fixed by #2407
Assignees

Comments

@sbueringer
Copy link
Member

sbueringer commented Nov 28, 2022

It would be great if controller-runtime would be able to authorize requests to the /metrics endpoint.

Some background information:

  • Kubernetes core components protect the /metrics endpoint via authentication (user/group/SA) and authorization (via RBAC verb: get, nonResourceURLs: /metrics) (docs).
  • The kube-rbac-proxy from @brancz can provide this functionality as a sidecar. (non-resource-url example)

It would be great to have this capability built into controller-runtime as it would make it possible to have this functionality without a dependency to an additional project and a sidecar container.

Is there interest in the controller-runtime community to support the nonResourceURL-based authorization (like core Kubernetes components)?

Notes:

  • Not sure how other projects are handling this but in cluster-api we removed kube-rbac-proxy after a few lengthy disussions (I think main reason was that it's not community-owned, xref: ⚠️ Remove kube-rbac-proxy and expose metrics on localhost:8080 cluster-api#4640). Since then metrics are basically disabled per default in Cluster API since we also didn't want to expose them unauthorized per default for security reasons.
  • ~ related the metrics endpoint should also be served on https instead of http (with the same cert as the webhook?)
  • Not sure if other endpoints should be protected as well, but /metrics would be the most important one for us.
@sbueringer
Copy link
Member Author

cc @alvaroaleman @joelanford @vincepri What do you think about it?

cc @fabriziopandini @killianmuldoon (given our recent discussions about metrics in Cluster API)

@alvaroaleman
Copy link
Member

@sbueringer in general doing that seems ok to me but I also do not have a super precise picture of how much complexity this actually entails. From a cursory look at kube-rbac-proxy, most of the logic here can be imported from upstream?

@sbueringer
Copy link
Member Author

Probably.

I'll try to get back to this as soon as I have time and implement a prototype or something so we have more data to decide this.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 20, 2023
@sbueringer
Copy link
Member Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 20, 2023
@sbueringer
Copy link
Member Author

/assign

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 a pull request may close this issue.

4 participants