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

⚠️ Discontinue Kube RBAC Proxy in Default Kubebuilder Scaffolding #3899

Merged
merged 1 commit into from
May 15, 2024
Merged

⚠️ Discontinue Kube RBAC Proxy in Default Kubebuilder Scaffolding #3899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test-sample-go.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
sed -i '25s/^#//' $KUSTOMIZATION_FILE_PATH
sed -i '27s/^#//' $KUSTOMIZATION_FILE_PATH
sed -i '42s/^#//' $KUSTOMIZATION_FILE_PATH
sed -i '46,143s/^#//' $KUSTOMIZATION_FILE_PATH
sed -i '46,142s/^#//' $KUSTOMIZATION_FILE_PATH

- name: Test
run: |
Expand Down
8 changes: 4 additions & 4 deletions docs/book/src/component-config-tutorial/testdata/project/config/default/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ resources:
#- ../prometheus

patches:
# Protect the /metrics endpoint by putting it behind auth.
# If you want your controller-manager to expose the /metrics
# endpoint w/o any authn/z, please comment the following line.
- path: manager_auth_proxy_patch.yaml
# [METRICS] The following patch will enable the metrics endpoint. Ensure that you also protect this endpoint.
# More info: https://book.kubebuilder.io/reference/metrics
# If you want to expose the metric endpoint of your controller-manager uncomment the following line.
#- path: manager_metrics_patch.yaml

# Mount the controller config file for loading manager configurations
# through a ComponentConfig type
Expand Down
34 changes: 0 additions & 34 deletions ...c/component-config-tutorial/testdata/project/config/default/manager_auth_proxy_patch.yaml
View file
Open in desktop

This file was deleted.

13 changes: 13 additions & 0 deletions .../src/component-config-tutorial/testdata/project/config/default/manager_metrics_patch.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# This patch adds the args to allow exposing the metrics endpoint securely
apiVersion: apps/v1
kind: Deployment
metadata:
name: controller-manager
namespace: system
spec:
template:
spec:
containers:
- name: manager
args:
- "--metrics-bind-address=0.0.0.0:8080"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this patch is needed to enable metrics. We have our metrics scraped by Prometheus without this arg set. Ref. controller-runtime argument help.

-metrics-bind-address string
The address the metric endpoint binds to. (default ":8080")

So if you want to have a patch to enable the metrics endpoint, you have to disable it by default by setting it to 0, ref. https://github.com/kubernetes-sigs/controller-runtime/blob/479b723944e34ae42c9911fe01228ff34eb5ca81/pkg/metrics/server/server.go#L120-L122

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. thank you a lot for check and share it.
I think ideally we should not enable by default.
Not everybody want to use it and by enable the metrics is required to protect the endpoint
So, it would be better if that is a conscious decision.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done the changes and also added a test to ensure that the metrics endpoint will not be exposed in this case

It("should generate a runnable project without metrics exposed", func() {
			kbc.IsRestricted = false
			GenerateV4WithoutMetrics(kbc)
			Run(kbc, true, false, false)
})

In this case, we are using the curl pod to ensure that the connection will be refused such as:

$ kubectl logs curl -n e2e-vbau-system
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 10.96.136.215:8080...
* connect to 10.96.136.215 port 8080 failed: Connection refused
* Failed to connect to e2e-vbau-controller-manager-metrics-service.e2e-vbau-system.svc.cluster.local port 8080 after 3 ms: Connection refused
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
* Closing connection 0
curl: (7) Failed to connect to e2e-vbau-controller-manager-metrics-service.e2e-vbau-system.svc.cluster.local port 8080 after 3 ms: Connection refused

Thank you a lot.

7 changes: 2 additions & 5 deletions docs/book/src/component-config-tutorial/testdata/project/config/prometheus/monitor.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,8 @@ metadata:
spec:
endpoints:
- path: /metrics
port: https
scheme: https
bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
tlsConfig:
insecureSkipVerify: true
port: http # Ensure this is the name of the port that exposes HTTP metrics
scheme: http
selector:
matchLabels:
control-plane: controller-manager
12 changes: 0 additions & 12 deletions ...component-config-tutorial/testdata/project/config/rbac/auth_proxy_client_clusterrole.yaml
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions docs/book/src/component-config-tutorial/testdata/project/config/rbac/auth_proxy_role.yaml
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions ...k/src/component-config-tutorial/testdata/project/config/rbac/auth_proxy_role_binding.yaml
View file
Open in desktop

This file was deleted.

9 changes: 2 additions & 7 deletions docs/book/src/component-config-tutorial/testdata/project/config/rbac/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,11 @@ resources:
- role_binding.yaml
- leader_election_role.yaml
- leader_election_role_binding.yaml
# Comment the following 4 lines if you want to disable
# the auth proxy (https://github.com/brancz/kube-rbac-proxy)
# which protects your /metrics endpoint.
- auth_proxy_service.yaml
- auth_proxy_role.yaml
- auth_proxy_role_binding.yaml
- auth_proxy_client_clusterrole.yaml
- metrics_service.yaml
# For each CRD, "Editor" and "Viewer" roles are scaffolded by
# default, aiding admins in cluster management. Those roles are
# not used by the Project itself. You can comment the following lines
# if you do not want those helpers be installed with your Project.
- projectconfig_editor_role.yaml
- projectconfig_viewer_role.yaml

6 changes: 3 additions & 3 deletions ...oject/config/rbac/auth_proxy_service.yaml → .../project/config/rbac/metrics_service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ metadata:
namespace: system
spec:
ports:
- name: https
port: 8443
- name: http
port: 8080
protocol: TCP
targetPort: https
targetPort: 8080
selector:
control-plane: controller-manager
8 changes: 4 additions & 4 deletions docs/book/src/cronjob-tutorial/testdata/project/config/default/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ resources:
- ../prometheus

patches:
# Protect the /metrics endpoint by putting it behind auth.
# If you want your controller-manager to expose the /metrics
# endpoint w/o any authn/z, please comment the following line.
- path: manager_auth_proxy_patch.yaml
# [METRICS] The following patch will enable the metrics endpoint. Ensure that you also protect this endpoint.
# More info: https://book.kubebuilder.io/reference/metrics
# If you want to expose the metric endpoint of your controller-manager uncomment the following line.
#- path: manager_metrics_patch.yaml

# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
# crd/kustomization.yaml
Expand Down
55 changes: 0 additions & 55 deletions docs/book/src/cronjob-tutorial/testdata/project/config/default/manager_auth_proxy_patch.yaml
View file
Open in desktop

This file was deleted.

13 changes: 13 additions & 0 deletions docs/book/src/cronjob-tutorial/testdata/project/config/default/manager_metrics_patch.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# This patch adds the args to allow exposing the metrics endpoint securely
apiVersion: apps/v1
kind: Deployment
metadata:
name: controller-manager
namespace: system
spec:
template:
spec:
containers:
- name: manager
args:
- "--metrics-bind-address=0.0.0.0:8080"
4 changes: 3 additions & 1 deletion docs/book/src/cronjob-tutorial/testdata/project/config/manager/manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ spec:
- command:
- /manager
args:
- --leader-elect
- --leader-elect
- --health-probe-bind-address=:8081
- --metrics-bind-address=0
image: controller:latest
name: manager
securityContext:
Expand Down
7 changes: 2 additions & 5 deletions docs/book/src/cronjob-tutorial/testdata/project/config/prometheus/monitor.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,8 @@ metadata:
spec:
endpoints:
- path: /metrics
port: https
scheme: https
bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
tlsConfig:
insecureSkipVerify: true
port: http # Ensure this is the name of the port that exposes HTTP metrics
scheme: http
selector:
matchLabels:
control-plane: controller-manager
12 changes: 0 additions & 12 deletions ...book/src/cronjob-tutorial/testdata/project/config/rbac/auth_proxy_client_clusterrole.yaml
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions docs/book/src/cronjob-tutorial/testdata/project/config/rbac/auth_proxy_role.yaml
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions docs/book/src/cronjob-tutorial/testdata/project/config/rbac/auth_proxy_role_binding.yaml
View file
Open in desktop

This file was deleted.

9 changes: 2 additions & 7 deletions docs/book/src/cronjob-tutorial/testdata/project/config/rbac/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,11 @@ resources:
- role_binding.yaml
- leader_election_role.yaml
- leader_election_role_binding.yaml
# Comment the following 4 lines if you want to disable
# the auth proxy (https://github.com/brancz/kube-rbac-proxy)
# which protects your /metrics endpoint.
- auth_proxy_service.yaml
- auth_proxy_role.yaml
- auth_proxy_role_binding.yaml
- auth_proxy_client_clusterrole.yaml
- metrics_service.yaml
# For each CRD, "Editor" and "Viewer" roles are scaffolded by
# default, aiding admins in cluster management. Those roles are
# not used by the Project itself. You can comment the following lines
# if you do not want those helpers be installed with your Project.
- cronjob_editor_role.yaml
- cronjob_viewer_role.yaml

6 changes: 3 additions & 3 deletions ...oject/config/rbac/auth_proxy_service.yaml → .../project/config/rbac/metrics_service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ metadata:
namespace: system
spec:
ports:
- name: https
port: 8443
- name: http
port: 8080
protocol: TCP
targetPort: https
targetPort: 8080
selector:
control-plane: controller-manager
8 changes: 4 additions & 4 deletions docs/book/src/getting-started/testdata/project/config/default/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ resources:
#- ../prometheus

patches:
# Protect the /metrics endpoint by putting it behind auth.
# If you want your controller-manager to expose the /metrics
# endpoint w/o any authn/z, please comment the following line.
- path: manager_auth_proxy_patch.yaml
# [METRICS] The following patch will enable the metrics endpoint. Ensure that you also protect this endpoint.
# More info: https://book.kubebuilder.io/reference/metrics
# If you want to expose the metric endpoint of your controller-manager uncomment the following line.
#- path: manager_metrics_patch.yaml
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Metrics should not be enable by default as it was before.


# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
# crd/kustomization.yaml
Expand Down