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

Remove insecure kube_rbac_proxy #111

Merged

Conversation

JamesLaverack
Copy link
Contributor

@JamesLaverack JamesLaverack commented Dec 4, 2019

Fixes #108

  • Passing around one's service account token is very insecure.
  • Binding extra permissions to the default service account in the eco-system namespace is discouraged.

General feeling in the team is that it's better to just expose /metrics openly. If protection is desired from other things in the Kubernetes cluster is what service meshes are for.

* Passing around one's service account token is very insecure.
* Binding extra permissions to the default service account in the
  `eco-system` namespace is discouraged.

General feeling in the team is that it's better to just expose
`/metrics` openly. If protection is desired from other things in the
Kubernetes cluster is what service meshes are for.
@JamesLaverack JamesLaverack self-assigned this Dec 4, 2019
@wallrj
Copy link
Contributor

wallrj commented Dec 6, 2019

I'd be inclined to just comment out the lines, as recommended in the commented yaml and not delete the patches.

I don't know if Kubebuilder currently updates these manifests after they have been created, but I imagine that the kubebuilder will in future attempt to upgrade the manifests to the latest version of kube-rbac-proxy and then things will break.

The kube-rbac-proxy maintainer explains here https://github.com/brancz/kube-rbac-proxy#notes-on-serviceaccount-token-security that you can use a client certificate instead of a token and you'd give promethes a limited token with privileges to only read metrics presumably.

@wallrj
Copy link
Contributor

wallrj commented Dec 6, 2019

Maybe there aren't any plans for Kubebuilder to update config/ after all. I found a few discussions about it:

We generally don't update scaffolded files because we have no way to know if the user has significantly changed them.
-- kubernetes-sigs/kubebuilder#1197 (comment)

We've got a section for migration in the docs now. I don't foresee scaffolding changes being automatic, so closing for now.
-- kubernetes-sigs/kubebuilder#650 (comment)

For existing projects, please follow this instruction to migrate the config directory structure to the new structure, which works with kustomize 2.0.0
-- kubernetes-sigs/kubebuilder#595 (comment)

Migrating between project structures in KubeBuilder generally involves a bit of manual work.
-- https://book.kubebuilder.io/migrations.html

@JamesLaverack JamesLaverack merged commit 730ede4 into improbable-eng:master Dec 9, 2019
@JamesLaverack JamesLaverack deleted the remove-metrics-proxy branch December 9, 2019 10:33
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 this pull request may close these issues.

Expose metrics from the etcd operator
4 participants