Skip to content
This repository was archived by the owner on Apr 17, 2025. It is now read-only.

Fix manifest kustomization for metrics service #202

Merged
merged 1 commit into from
Apr 22, 2022
Merged

Fix manifest kustomization for metrics service #202

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/manager/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
resources:
- manager.yaml
- metrics_service.yaml
3 changes: 0 additions & 3 deletions config/manager/manager.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ spec:
- /manager
args:
- "--webhook-server-port=9443"
# If /config/default/manager_auth_proxy_patch.yaml is used, the port in
# --metrics-addr must match the value of --upstream passed to
# kube-rbac-proxy.
- "--metrics-addr=:8080"
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't just changing this to 8443 fix our issue and still allow us to use HTTPS? I'm not sure we want to take a step back in security and not use HTTPS for metrics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't just changing this to 8443 fix our issue and still allow us to use HTTPS? I'm not sure we want to take a step back in security and not use HTTPS for metrics.

No, there is nothing providing a HTTPS listener (on port 8443). Then we need to include the kube-rbac-proxy sidecar and route all metrics scrape request through its HTTPS listener - forwarding the request to the controller-manager metrics HTTP endpoint. That is an alternative mentioned in the PR description, though more complex, and I don't see the true benefit of this. I expect many K8s distros to block namespace ingress traffic by default.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah we took out kube-rbac-proxy a while ago (it has a bunch of problems from a supply chain perspective) and I'd rather not put it back in without good justification.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need rbac proxy for https, that is only needed for authz. We already serve https traffic for the webhook on 9443. So I guess I don't see why we couldn't also use it (or have the option) for 8443 for metrics. However, that is probably out of scope of this PR if metrics are currently broken.

I created this to revisit this.

- "--max-reconciles=10"
- "--apiserver-qps-throttle=50"
Expand Down
10 changes: 5 additions & 5 deletions config/rbac/auth_proxy_service.yaml → config/manager/metrics_service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@ apiVersion: v1
kind: Service
metadata:
annotations:
prometheus.io/port: "8443"
prometheus.io/scheme: https
prometheus.io/port: "8080"
prometheus.io/scheme: http
prometheus.io/scrape: "true"
labels:
control-plane: controller-manager
name: controller-manager-metrics-service
namespace: system
spec:
ports:
- name: https
port: 8443
targetPort: https
- name: metrics
port: 8080
targetPort: metrics
selector:
control-plane: controller-manager
13 changes: 0 additions & 13 deletions config/rbac/auth_proxy_role.yaml
View file
Open in desktop

This file was deleted.

12 changes: 0 additions & 12 deletions config/rbac/auth_proxy_role_binding.yaml
View file
Open in desktop

This file was deleted.

6 changes: 0 additions & 6 deletions config/rbac/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,3 @@ resources:
- hnc_admin.yaml
- leader_election_role.yaml
- leader_election_role_binding.yaml
# Comment the following 3 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