remove deprecated kube-rbac-proxy (#294)

* remove deprecated kube-rbac-proxy * add metric to count appwrapper phase transitions per namespace * normalize config files * run Metrics tests in CI
project-codeflare · Jan 2, 2025 · e926f00 · e926f00
1 parent ed8bee9
commit e926f00
Show file tree

Hide file tree

Showing 23 changed files with 286 additions and 126 deletions.
diff --git a/.github/workflows/CI-standalone.yaml b/.github/workflows/CI-standalone.yaml
@@ -59,4 +59,4 @@ jobs:
         make deploy -e GIT_BRANCH=${{ env.GIT_BRANCH }} TAG=${{ env.GIT_BRANCH }}-${{ env.TAG }} ENV=standalone
 
     - name: Run E2E tests
-      run: LABEL_FILTER="Standalone,Webhook" ./hack/run-tests-on-cluster.sh
+      run: LABEL_FILTER="Metrics,Standalone,Webhook" ./hack/run-tests-on-cluster.sh
diff --git a/cmd/main.go b/cmd/main.go
@@ -42,13 +42,15 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/yaml"
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 
 	workloadv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2"
+	"github.com/project-codeflare/appwrapper/internal/metrics"
 	"github.com/project-codeflare/appwrapper/pkg/config"
 	"github.com/project-codeflare/appwrapper/pkg/controller"
 	"github.com/project-codeflare/appwrapper/pkg/logger"
@@ -123,15 +125,19 @@ func main() {
 		tlsOpts = append(tlsOpts, disableHTTP2)
 	}
 
+	metrics.Register()
+
 	mgr, err := ctrl.NewManager(k8sConfig, ctrl.Options{
 		Scheme: scheme,
 		Metrics: metricsserver.Options{
-			BindAddress:   cfg.ControllerManager.Metrics.BindAddress,
-			SecureServing: cfg.ControllerManager.Metrics.SecureServing,
-			TLSOpts:       tlsOpts,
+			BindAddress:    cfg.ControllerManager.Metrics.BindAddress,
+			FilterProvider: filters.WithAuthenticationAndAuthorization,
+			SecureServing:  true,
+			TLSOpts:        tlsOpts,
 		},
 		WebhookServer: webhook.NewServer(webhook.Options{
 			TLSOpts: tlsOpts,
+			Port:    9443,
 		}),
 		HealthProbeBindAddress: cfg.ControllerManager.Health.BindAddress,
 		LeaderElection:         cfg.ControllerManager.LeaderElection,

diff --git a/config/default/config.yaml b/config/default/config.yaml
@@ -10,5 +10,5 @@ data:
       health:
         bindAddress: ":8081"
       metrics:
-        bindAddress: "127.0.0.1:8080"
+        bindAddress: ":8443"
       leaderElection: true
diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml
@@ -8,30 +8,30 @@ namespace: appwrapper-system
 # field above.
 namePrefix: appwrapper-
 
-# Labels to add to all resources and selectors.
-#labels:
-#- includeSelectors: true
-#  pairs:
-#    someName: someValue
+labels:
+- pairs:
+    app.kubernetes.io/name: appwrapper
+    app.kubernetes.io/component: controller
+  includeTemplates: true
+- pairs:
+    control-plane: controller-manager
+  includeSelectors: true
 
 resources:
 - config.yaml
 - ../crd
 - ../rbac
 - ../manager
+- ../internalcert
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
 # crd/kustomization.yaml
 - ../webhook
-- ../internalcert
 # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'.
 #- ../prometheus
+# [METRICS] Expose the controller manager metrics service.
+- metrics_service.yaml
 
 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
-
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
 # crd/kustomization.yaml
 - path: manager_webhook_patch.yaml

diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml
diff --git a/config/default/metrics_service.yaml b/config/default/metrics_service.yaml
@@ -0,0 +1,16 @@
+apiVersion: v1
+kind: Service
+metadata:
+  labels:
+    control-plane: controller-manager
+    app.kubernetes.io/name: appwrapper
+  name: controller-manager-metrics-service
+  namespace: system
+spec:
+  ports:
+  - name: https
+    port: 8443
+    protocol: TCP
+    targetPort: 8443
+  selector:
+    control-plane: controller-manager
diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -1,28 +1,19 @@
 apiVersion: v1
 kind: Namespace
 metadata:
-  labels:
-    control-plane: controller-manager
   name: system
 ---
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: controller-manager
   namespace: system
-  labels:
-    control-plane: controller-manager
 spec:
-  selector:
-    matchLabels:
-      control-plane: controller-manager
   replicas: 1
   template:
     metadata:
       annotations:
         kubectl.kubernetes.io/default-container: manager
-      labels:
-        control-plane: controller-manager
     spec:
       affinity:
         nodeAffinity:
@@ -47,6 +38,8 @@ spec:
       containers:
       - command:
         - /manager
+        args:
+        - "--zap-log-level=2"
         image: controller:latest
         name: manager
         securityContext:
@@ -68,10 +61,10 @@ spec:
           periodSeconds: 10
         resources:
           limits:
-            cpu: 500m
+            cpu: "2"
             memory: 128Mi
           requests:
-            cpu: 10m
+            cpu: 100m
             memory: 64Mi
       serviceAccountName: controller-manager
       terminationGracePeriodSeconds: 10
diff --git a/config/rbac/kustomization.yaml b/config/rbac/kustomization.yaml
@@ -12,10 +12,13 @@ resources:
 - user_role.yaml
 - editor_role.yaml
 - viewer_role.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
+
+# The following RBAC configurations are used to protect
+# the metrics endpoint with authn/authz. These configurations
+# ensure that only authorized users and service accounts
+# can access the metrics endpoint. Comment the following
+# permissions if you want to disable this protection.
+# More info: https://book.kubebuilder.io/reference/metrics.html
+- metrics_auth_role.yaml
+- metrics_auth_role_binding.yaml
+- metrics_reader_role.yaml
diff --git a/config/rbac/auth_proxy_role.yaml → config/rbac/metrics_auth_role.yaml b/config/rbac/auth_proxy_role.yaml → config/rbac/metrics_auth_role.yaml
@@ -1,7 +1,7 @@
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
 metadata:
-  name: proxy-role
+  name: metrics-auth-role
 rules:
 - apiGroups:
   - authentication.k8s.io

diff --git a/config/rbac/auth_proxy_role_binding.yaml → config/rbac/metrics_auth_role_binding.yaml b/config/rbac/auth_proxy_role_binding.yaml → config/rbac/metrics_auth_role_binding.yaml
@@ -1,11 +1,11 @@
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
 metadata:
-  name: proxy-rolebinding
+  name: metrics-auth-rolebinding
 roleRef:
   apiGroup: rbac.authorization.k8s.io
   kind: ClusterRole
-  name: proxy-role
+  name: metrics-auth-role
 subjects:
 - kind: ServiceAccount
   name: controller-manager

diff --git a/...g/rbac/auth_proxy_client_clusterrole.yaml → config/rbac/metrics_reader_role.yaml b/...g/rbac/auth_proxy_client_clusterrole.yaml → config/rbac/metrics_reader_role.yaml
diff --git a/config/standalone/config.yaml b/config/standalone/config.yaml
@@ -10,5 +10,5 @@ data:
       health:
         bindAddress: ":8081"
       metrics:
-        bindAddress: "127.0.0.1:8080"
+        bindAddress: ":8443"
       leaderElection: true
diff --git a/config/standalone/kustomization.yaml b/config/standalone/kustomization.yaml
@@ -8,30 +8,30 @@ namespace: appwrapper-system
 # field above.
 namePrefix: appwrapper-
 
-# Labels to add to all resources and selectors.
-#labels:
-#- includeSelectors: true
-#  pairs:
-#    someName: someValue
+labels:
+- pairs:
+    app.kubernetes.io/name: appwrapper
+    app.kubernetes.io/component: controller
+  includeTemplates: true
+- pairs:
+    control-plane: controller-manager
+  includeSelectors: true
 
 resources:
 - config.yaml
 - ../crd
 - ../rbac
 - ../manager
+- ../internalcert
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
 # crd/kustomization.yaml
 - ../webhook
-- ../internalcert
 # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'.
 #- ../prometheus
+# [METRICS] Expose the controller manager metrics service.
+- metrics_service.yaml
 
 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
-
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in
 # crd/kustomization.yaml
 - path: manager_webhook_patch.yaml
diff --git a/config/standalone/manager_auth_proxy_patch.yaml b/config/standalone/manager_auth_proxy_patch.yaml
diff --git a/config/rbac/auth_proxy_service.yaml → config/standalone/metrics_service.yaml b/config/rbac/auth_proxy_service.yaml → config/standalone/metrics_service.yaml
@@ -8,6 +8,6 @@ spec:
   - name: https
     port: 8443
     protocol: TCP
-    targetPort: https
+    targetPort: 8443
   selector:
     control-plane: controller-manager