From e92b05dba0be4f367ebeb56b25c6f971101c3266 Mon Sep 17 00:00:00 2001 From: Ryan Phillips Date: Thu, 13 Nov 2025 14:50:23 -0600 Subject: [PATCH] network policies and metrics tweaks --- bindata/assets.go | 2 +- .../networkpolicy/99-deny-all.yaml | 19 -- .../01-allow-egress-api.yaml} | 6 +- .../02-allow-egress-cluster-dns.yaml} | 5 +- .../03-allow-ingress-webhook.yaml} | 3 +- .../05-allow-ingress-metrics.yaml} | 9 +- .../networkpolicy/operand/99-deny-all.yaml | 15 ++ .../operator/01-allow-egress-api.yaml | 20 ++ .../operator/02-allow-egress-cluster-dns.yaml | 29 +++ .../operator/03-allow-ingress-metrics.yaml | 30 +++ .../networkpolicy/operator/99-deny-all.yaml | 18 ++ .../kueue-operator/prometheus-rbac/role.yaml | 17 ++ .../prometheus-rbac/rolebinding.yaml | 13 ++ .../servicemonitor/operator-metrics.yaml | 25 ++ .../kueue-operator.clusterserviceversion.yaml | 21 +- ...ift-kueue-operator-metrics_v1_service.yaml | 19 ++ deploy/02_clusterrole.yaml | 12 + deploy/07_deployment.yaml | 9 +- deploy/08_service.yaml | 17 ++ deploy/08a_metrics_certificate.yaml | 18 ++ deploy/08b_metrics_issuer.yaml | 7 + pkg/operator/target_config_reconciler.go | 216 +++++++++++++++++- test/e2e/e2e_operator_test.go | 168 +++++++++++--- 23 files changed, 630 insertions(+), 68 deletions(-) delete mode 100644 bindata/assets/kueue-operator/networkpolicy/99-deny-all.yaml rename bindata/assets/kueue-operator/networkpolicy/{10-allow-egress-api.yaml => operand/01-allow-egress-api.yaml} (69%) rename bindata/assets/kueue-operator/networkpolicy/{10-allow-egress-cluster-dns.yaml => operand/02-allow-egress-cluster-dns.yaml} (78%) rename bindata/assets/kueue-operator/networkpolicy/{10-allow-ingress-egress-webhook.yaml => operand/03-allow-ingress-webhook.yaml} (91%) rename bindata/assets/kueue-operator/networkpolicy/{10-allow-ingress-egress-metrics.yaml => operand/05-allow-ingress-metrics.yaml} (65%) create mode 100644 bindata/assets/kueue-operator/networkpolicy/operand/99-deny-all.yaml create mode 100644 bindata/assets/kueue-operator/networkpolicy/operator/01-allow-egress-api.yaml create mode 100644 bindata/assets/kueue-operator/networkpolicy/operator/02-allow-egress-cluster-dns.yaml create mode 100644 bindata/assets/kueue-operator/networkpolicy/operator/03-allow-ingress-metrics.yaml create mode 100644 bindata/assets/kueue-operator/networkpolicy/operator/99-deny-all.yaml create mode 100644 bindata/assets/kueue-operator/prometheus-rbac/role.yaml create mode 100644 bindata/assets/kueue-operator/prometheus-rbac/rolebinding.yaml create mode 100644 bindata/assets/kueue-operator/servicemonitor/operator-metrics.yaml create mode 100644 bundle/manifests/openshift-kueue-operator-metrics_v1_service.yaml create mode 100644 deploy/08_service.yaml create mode 100644 deploy/08a_metrics_certificate.yaml create mode 100644 deploy/08b_metrics_issuer.yaml diff --git a/bindata/assets.go b/bindata/assets.go index 318f43548..e4c6ed9cb 100644 --- a/bindata/assets.go +++ b/bindata/assets.go @@ -5,7 +5,7 @@ import ( "fmt" ) -//go:embed assets/* +//go:embed assets var f embed.FS // Asset reads and returns the content of the named file. diff --git a/bindata/assets/kueue-operator/networkpolicy/99-deny-all.yaml b/bindata/assets/kueue-operator/networkpolicy/99-deny-all.yaml deleted file mode 100644 index d95733f81..000000000 --- a/bindata/assets/kueue-operator/networkpolicy/99-deny-all.yaml +++ /dev/null @@ -1,19 +0,0 @@ -# it's the default deny-all policy, applies to both ingress and egress traffic -# it matches the kueue pod running in the shared operator namespace. - -# TODO: ideally kueue operator should reside in an isolated (not shared with -# other operators) namespace where the operator has full control of the -# namespace, this enables the operator to select all pod(s) in the namespace -# to enforce a namespace-wide deny all policy. -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: kueue-deny-all - namespace: openshift-kueue-operator -spec: - podSelector: - matchLabels: - app.openshift.io/name: kueue # applies to both the operator and kueue pod - policyTypes: - - Ingress - - Egress diff --git a/bindata/assets/kueue-operator/networkpolicy/10-allow-egress-api.yaml b/bindata/assets/kueue-operator/networkpolicy/operand/01-allow-egress-api.yaml similarity index 69% rename from bindata/assets/kueue-operator/networkpolicy/10-allow-egress-api.yaml rename to bindata/assets/kueue-operator/networkpolicy/operand/01-allow-egress-api.yaml index 3302db939..6db833cc8 100644 --- a/bindata/assets/kueue-operator/networkpolicy/10-allow-egress-api.yaml +++ b/bindata/assets/kueue-operator/networkpolicy/operand/01-allow-egress-api.yaml @@ -1,4 +1,4 @@ -# allow outbound traffic to kube-apiserver +# allow outbound traffic to kube-apiserver from kueue operand pod apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: @@ -7,11 +7,11 @@ metadata: spec: podSelector: matchLabels: - app.openshift.io/name: kueue # applies to both the operator and kueue pod + app.kubernetes.io/name: kueue + control-plane: controller-manager egress: - ports: - protocol: TCP port: 6443 # we can not use a name because it is not on the pod network policyTypes: - Egress - diff --git a/bindata/assets/kueue-operator/networkpolicy/10-allow-egress-cluster-dns.yaml b/bindata/assets/kueue-operator/networkpolicy/operand/02-allow-egress-cluster-dns.yaml similarity index 78% rename from bindata/assets/kueue-operator/networkpolicy/10-allow-egress-cluster-dns.yaml rename to bindata/assets/kueue-operator/networkpolicy/operand/02-allow-egress-cluster-dns.yaml index bc8879f2e..e516be92d 100644 --- a/bindata/assets/kueue-operator/networkpolicy/10-allow-egress-cluster-dns.yaml +++ b/bindata/assets/kueue-operator/networkpolicy/operand/02-allow-egress-cluster-dns.yaml @@ -1,4 +1,4 @@ -# allow traffic to cluster DNS service +# allow traffic to cluster DNS service from kueue operand pod apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: @@ -7,7 +7,8 @@ metadata: spec: podSelector: matchLabels: - app.openshift.io/name: kueue # applies to both the operator and kueue pod + app.kubernetes.io/name: kueue + control-plane: controller-manager egress: - to: - namespaceSelector: diff --git a/bindata/assets/kueue-operator/networkpolicy/10-allow-ingress-egress-webhook.yaml b/bindata/assets/kueue-operator/networkpolicy/operand/03-allow-ingress-webhook.yaml similarity index 91% rename from bindata/assets/kueue-operator/networkpolicy/10-allow-ingress-egress-webhook.yaml rename to bindata/assets/kueue-operator/networkpolicy/operand/03-allow-ingress-webhook.yaml index 1a9c2509b..bda96f708 100644 --- a/bindata/assets/kueue-operator/networkpolicy/10-allow-ingress-egress-webhook.yaml +++ b/bindata/assets/kueue-operator/networkpolicy/operand/03-allow-ingress-webhook.yaml @@ -7,7 +7,8 @@ metadata: spec: podSelector: matchLabels: - app.kubernetes.io/name: kueue # applies to the kueue pod only + app.kubernetes.io/name: kueue + control-plane: controller-manager ingress: - from: - namespaceSelector: diff --git a/bindata/assets/kueue-operator/networkpolicy/10-allow-ingress-egress-metrics.yaml b/bindata/assets/kueue-operator/networkpolicy/operand/05-allow-ingress-metrics.yaml similarity index 65% rename from bindata/assets/kueue-operator/networkpolicy/10-allow-ingress-egress-metrics.yaml rename to bindata/assets/kueue-operator/networkpolicy/operand/05-allow-ingress-metrics.yaml index 48fd43777..6c831715b 100644 --- a/bindata/assets/kueue-operator/networkpolicy/10-allow-ingress-egress-metrics.yaml +++ b/bindata/assets/kueue-operator/networkpolicy/operand/05-allow-ingress-metrics.yaml @@ -1,4 +1,10 @@ +<<<<<<<< HEAD:bindata/assets/kueue-operator/networkpolicy/10-allow-ingress-egress-metrics.yaml # allow ingress and egress traffic to/from metrics endpoint +|||||||| parent of e772e32c (network policies and metrics tweaks):bindata/assets/kueue-operator/networkpolicy/allow-ingress-metrics.yaml +# allow ingress traffic to metrics endpoint +======== +# allow ingress traffic to metrics endpoint on kueue operand pod +>>>>>>>> e772e32c (network policies and metrics tweaks):bindata/assets/kueue-operator/networkpolicy/operand/05-allow-ingress-metrics.yaml apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: @@ -7,7 +13,8 @@ metadata: spec: podSelector: matchLabels: - app.openshift.io/name: kueue # applies to both the operator and kueue pod + app.kubernetes.io/name: kueue + control-plane: controller-manager ingress: - from: - namespaceSelector: diff --git a/bindata/assets/kueue-operator/networkpolicy/operand/99-deny-all.yaml b/bindata/assets/kueue-operator/networkpolicy/operand/99-deny-all.yaml new file mode 100644 index 000000000..6ca328d53 --- /dev/null +++ b/bindata/assets/kueue-operator/networkpolicy/operand/99-deny-all.yaml @@ -0,0 +1,15 @@ +# default deny-all policy for kueue operand pod +# Applies to kueue deployment pods only +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: kueue-deny-all + namespace: openshift-kueue-operator +spec: + podSelector: + matchLabels: + app.kubernetes.io/name: kueue + control-plane: controller-manager + policyTypes: + - Ingress + - Egress diff --git a/bindata/assets/kueue-operator/networkpolicy/operator/01-allow-egress-api.yaml b/bindata/assets/kueue-operator/networkpolicy/operator/01-allow-egress-api.yaml new file mode 100644 index 000000000..a67bc42a2 --- /dev/null +++ b/bindata/assets/kueue-operator/networkpolicy/operator/01-allow-egress-api.yaml @@ -0,0 +1,20 @@ +# allow outbound traffic to kube-apiserver from operator pod +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: kueue-operator-allow-egress-kube-apiserver + namespace: openshift-kueue-operator + labels: + app.kubernetes.io/managed-by: kueue-operator + app.kubernetes.io/component: operator-network-policy +spec: + podSelector: + matchLabels: + app.openshift.io/component: operator + app.openshift.io/name: kueue + egress: + - ports: + - protocol: TCP + port: 6443 # we can not use a name because it is not on the pod network + policyTypes: + - Egress diff --git a/bindata/assets/kueue-operator/networkpolicy/operator/02-allow-egress-cluster-dns.yaml b/bindata/assets/kueue-operator/networkpolicy/operator/02-allow-egress-cluster-dns.yaml new file mode 100644 index 000000000..04bdd1e00 --- /dev/null +++ b/bindata/assets/kueue-operator/networkpolicy/operator/02-allow-egress-cluster-dns.yaml @@ -0,0 +1,29 @@ +# allow traffic to cluster DNS service from operator pod +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: kueue-operator-allow-egress-cluster-dns + namespace: openshift-kueue-operator + labels: + app.kubernetes.io/managed-by: kueue-operator + app.kubernetes.io/component: operator-network-policy +spec: + podSelector: + matchLabels: + app.openshift.io/component: operator + app.openshift.io/name: kueue + egress: + - to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: openshift-dns + podSelector: + matchLabels: + dns.operator.openshift.io/daemonset-dns: default + ports: + - protocol: TCP + port: dns-tcp + - protocol: UDP + port: dns + policyTypes: + - Egress diff --git a/bindata/assets/kueue-operator/networkpolicy/operator/03-allow-ingress-metrics.yaml b/bindata/assets/kueue-operator/networkpolicy/operator/03-allow-ingress-metrics.yaml new file mode 100644 index 000000000..7fba4d145 --- /dev/null +++ b/bindata/assets/kueue-operator/networkpolicy/operator/03-allow-ingress-metrics.yaml @@ -0,0 +1,30 @@ +# allow ingress traffic to metrics endpoint on operator pod +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: kueue-operator-allow-ingress-metrics + namespace: openshift-kueue-operator + labels: + app.kubernetes.io/managed-by: kueue-operator + app.kubernetes.io/component: operator-network-policy +spec: + podSelector: + matchLabels: + app.openshift.io/component: operator + app.openshift.io/name: kueue + ingress: + - from: + - namespaceSelector: + matchLabels: + openshift.io/cluster-monitoring: "true" + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: openshift-monitoring + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: openshift-user-workload-monitoring + ports: + - protocol: TCP + port: 8443 + policyTypes: + - Ingress diff --git a/bindata/assets/kueue-operator/networkpolicy/operator/99-deny-all.yaml b/bindata/assets/kueue-operator/networkpolicy/operator/99-deny-all.yaml new file mode 100644 index 000000000..a65f1fcc1 --- /dev/null +++ b/bindata/assets/kueue-operator/networkpolicy/operator/99-deny-all.yaml @@ -0,0 +1,18 @@ +# default deny-all policy for operator pod +# Applied LAST to ensure allow policies are in place first +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: kueue-operator-deny-all + namespace: openshift-kueue-operator + labels: + app.kubernetes.io/managed-by: kueue-operator + app.kubernetes.io/component: operator-network-policy +spec: + podSelector: + matchLabels: + app.openshift.io/component: operator + app.openshift.io/name: kueue + policyTypes: + - Ingress + - Egress diff --git a/bindata/assets/kueue-operator/prometheus-rbac/role.yaml b/bindata/assets/kueue-operator/prometheus-rbac/role.yaml new file mode 100644 index 000000000..54ede4465 --- /dev/null +++ b/bindata/assets/kueue-operator/prometheus-rbac/role.yaml @@ -0,0 +1,17 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: prometheus-k8s-metrics + namespace: openshift-kueue-operator +rules: + - apiGroups: + - "" + resources: + - services + - endpoints + - pods + - secrets + verbs: + - get + - list + - watch diff --git a/bindata/assets/kueue-operator/prometheus-rbac/rolebinding.yaml b/bindata/assets/kueue-operator/prometheus-rbac/rolebinding.yaml new file mode 100644 index 000000000..1c2b8adeb --- /dev/null +++ b/bindata/assets/kueue-operator/prometheus-rbac/rolebinding.yaml @@ -0,0 +1,13 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: prometheus-k8s-metrics + namespace: openshift-kueue-operator +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: prometheus-k8s-metrics +subjects: + - kind: ServiceAccount + name: prometheus-k8s + namespace: openshift-monitoring diff --git a/bindata/assets/kueue-operator/servicemonitor/operator-metrics.yaml b/bindata/assets/kueue-operator/servicemonitor/operator-metrics.yaml new file mode 100644 index 000000000..bdae7bc4a --- /dev/null +++ b/bindata/assets/kueue-operator/servicemonitor/operator-metrics.yaml @@ -0,0 +1,25 @@ +apiVersion: monitoring.coreos.com/v1 +kind: ServiceMonitor +metadata: + name: openshift-kueue-operator-metrics + namespace: openshift-kueue-operator + labels: + app.openshift.io/component: operator + app.openshift.io/name: kueue +spec: + endpoints: + - interval: 30s + path: /metrics + port: metrics + scheme: https + bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token + tlsConfig: + ca: + secret: + name: openshift-kueue-operator-metrics-tls + key: ca.crt + serverName: openshift-kueue-operator-metrics.openshift-kueue-operator.svc + selector: + matchLabels: + app.openshift.io/component: operator + app.openshift.io/name: kueue diff --git a/bundle/manifests/kueue-operator.clusterserviceversion.yaml b/bundle/manifests/kueue-operator.clusterserviceversion.yaml index 9cd68640f..c41cca6d9 100644 --- a/bundle/manifests/kueue-operator.clusterserviceversion.yaml +++ b/bundle/manifests/kueue-operator.clusterserviceversion.yaml @@ -236,6 +236,18 @@ spec: - get - create - update + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create serviceAccountName: openshift-kueue-operator deployments: - name: openshift-kueue-operator @@ -272,7 +284,7 @@ spec: imagePullPolicy: Always name: openshift-kueue-operator ports: - - containerPort: 60000 + - containerPort: 8443 name: metrics resources: {} securityContext: @@ -284,6 +296,9 @@ spec: volumeMounts: - mountPath: /tmp name: tmp + - mountPath: /var/run/secrets/serving-cert + name: metrics-tls + readOnly: true priorityClassName: system-cluster-critical securityContext: runAsNonRoot: true @@ -293,6 +308,10 @@ spec: volumes: - emptyDir: {} name: tmp + - name: metrics-tls + secret: + optional: true + secretName: openshift-kueue-operator-metrics-tls permissions: - rules: - apiGroups: diff --git a/bundle/manifests/openshift-kueue-operator-metrics_v1_service.yaml b/bundle/manifests/openshift-kueue-operator-metrics_v1_service.yaml new file mode 100644 index 000000000..a669a1d20 --- /dev/null +++ b/bundle/manifests/openshift-kueue-operator-metrics_v1_service.yaml @@ -0,0 +1,19 @@ +apiVersion: v1 +kind: Service +metadata: + creationTimestamp: null + labels: + app.openshift.io/component: operator + app.openshift.io/name: kueue + name: openshift-kueue-operator-metrics +spec: + ports: + - name: metrics + port: 60000 + protocol: TCP + targetPort: 8443 + selector: + name: openshift-kueue-operator + type: ClusterIP +status: + loadBalancer: {} diff --git a/deploy/02_clusterrole.yaml b/deploy/02_clusterrole.yaml index 19e8094a0..91b035913 100644 --- a/deploy/02_clusterrole.yaml +++ b/deploy/02_clusterrole.yaml @@ -164,3 +164,15 @@ rules: - get - create - update + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create diff --git a/deploy/07_deployment.yaml b/deploy/07_deployment.yaml index 6bb6cab75..3a0c6a76a 100644 --- a/deploy/07_deployment.yaml +++ b/deploy/07_deployment.yaml @@ -31,8 +31,11 @@ spec: volumeMounts: - name: tmp mountPath: "/tmp" + - name: metrics-tls + mountPath: "/var/run/secrets/serving-cert" + readOnly: true ports: - - containerPort: 60000 + - containerPort: 8443 name: metrics command: - kueue-operator @@ -54,3 +57,7 @@ spec: volumes: - name: tmp emptyDir: {} + - name: metrics-tls + secret: + secretName: openshift-kueue-operator-metrics-tls + optional: true diff --git a/deploy/08_service.yaml b/deploy/08_service.yaml new file mode 100644 index 000000000..83dd08f7d --- /dev/null +++ b/deploy/08_service.yaml @@ -0,0 +1,17 @@ +apiVersion: v1 +kind: Service +metadata: + name: openshift-kueue-operator-metrics + namespace: openshift-kueue-operator + labels: + app.openshift.io/component: operator + app.openshift.io/name: kueue +spec: + ports: + - name: metrics + port: 60000 + protocol: TCP + targetPort: 8443 + selector: + name: openshift-kueue-operator + type: ClusterIP diff --git a/deploy/08a_metrics_certificate.yaml b/deploy/08a_metrics_certificate.yaml new file mode 100644 index 000000000..e329558dd --- /dev/null +++ b/deploy/08a_metrics_certificate.yaml @@ -0,0 +1,18 @@ +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: openshift-kueue-operator-metrics + namespace: openshift-kueue-operator +spec: + secretName: openshift-kueue-operator-metrics-tls + commonName: openshift-kueue-operator-metrics.openshift-kueue-operator.svc + dnsNames: + - openshift-kueue-operator-metrics.openshift-kueue-operator.svc + - openshift-kueue-operator-metrics.openshift-kueue-operator.svc.cluster.local + issuerRef: + name: openshift-kueue-operator-issuer + kind: Issuer + usages: + - digital signature + - key encipherment + - server auth diff --git a/deploy/08b_metrics_issuer.yaml b/deploy/08b_metrics_issuer.yaml new file mode 100644 index 000000000..7268e0362 --- /dev/null +++ b/deploy/08b_metrics_issuer.yaml @@ -0,0 +1,7 @@ +apiVersion: cert-manager.io/v1 +kind: Issuer +metadata: + name: openshift-kueue-operator-issuer + namespace: openshift-kueue-operator +spec: + selfSigned: {} diff --git a/pkg/operator/target_config_reconciler.go b/pkg/operator/target_config_reconciler.go index 2c9f19877..c43c7dea4 100644 --- a/pkg/operator/target_config_reconciler.go +++ b/pkg/operator/target_config_reconciler.go @@ -66,6 +66,7 @@ import ( "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" "k8s.io/utils/clock" + "sigs.k8s.io/yaml" ) const ( @@ -186,6 +187,48 @@ func NewTargetConfigReconciler( // Detect platform type (OpenShift vs kind/vanilla k8s) c.isOpenShift = c.detectOpenShift() + // Create operator ServiceMonitor and Prometheus RBAC if CRD is available + if c.serviceMonitorSupport { + // Create Prometheus RBAC first (needed for Prometheus to scrape metrics) + if err := c.ensurePrometheusRBAC(ctx); err != nil { + klog.Errorf("Failed to create Prometheus RBAC: %v", err) + c.eventRecorder.Warningf( + "PrometheusRBACCreateFailed", + "Failed to create Prometheus RBAC: %v", err, + ) + // Don't fail controller startup if RBAC creation fails + } else { + klog.Info("Prometheus RBAC ensured successfully") + } + + // Create ServiceMonitor + if err := c.ensureOperatorServiceMonitor(ctx); err != nil { + klog.Errorf("Failed to create operator ServiceMonitor: %v", err) + c.eventRecorder.Warningf( + "ServiceMonitorCreateFailed", + "Failed to create operator ServiceMonitor: %v", err, + ) + // Don't fail controller startup if ServiceMonitor creation fails + // This is optional functionality + } else { + klog.Info("Operator ServiceMonitor ensured successfully") + } + } else { + klog.Info("ServiceMonitor CRD not available, skipping operator monitoring setup") + } + + // Ensure operator NetworkPolicies + if err := c.ensureOperatorNetworkPolicies(ctx); err != nil { + klog.Errorf("Failed to create operator NetworkPolicies: %v", err) + c.eventRecorder.Warningf( + "NetworkPolicyCreateFailed", + "Failed to create operator NetworkPolicies: %v", err, + ) + // Don't fail controller startup if NetworkPolicy creation fails + } else { + klog.Info("Operator NetworkPolicies ensured successfully") + } + return factory.New().WithInformers( kueueClient.Informer(), kubeInformersForNamespaces.InformersFor(c.operatorNamespace).Apps().V1().Deployments().Informer(), @@ -1330,25 +1373,22 @@ func (c *TargetConfigReconciler) manageClusterRoles(ctx context.Context, specAnn return nil } +// manageNetworkPolicies applies NetworkPolicies for the kueue operand (deployment) pods. +// These policies are applied during the sync loop and have owner references to the Kueue CR. +// For operator NetworkPolicies, see ensureOperatorNetworkPolicies() which is called during +// controller initialization. func (c *TargetConfigReconciler) manageNetworkPolicies(ctx context.Context, specAnnotations map[string]string, ownerReference metav1.OwnerReference) error { - networkPolicyDir := "assets/kueue-operator/networkpolicy" + // Use operand subdirectory - operator policies are applied separately during init + networkPolicyDir := "assets/kueue-operator/networkpolicy/operand" files, err := bindata.AssetDir(networkPolicyDir) if err != nil { return fmt.Errorf("failed to read networkpolicy from directory %q: %w", networkPolicyDir, err) } - // TODO: does the order of the creation of these policies matter? - // TODO: Since OLM does not support networkpolicy resource yet the - // operator Pod is creating policies for self isolation (in addition - // to operand isolation). let's say our operator creates the following - // network policy manifests for self and the operand in the following - // order: a) deny-all, b) allow-egress-api, c) allow egress cluster-dns, - // and d) allow-ingress-metrics; while creating these manifests in order, - // if there is a delay between a and b, long enough that deny-all takes - // effect and creation of b fails. If this can happen then the operator - // has lost access to the apiserver in a self inflicted manner. Should - // the operator create the deny-all policy last to avoid this issue? + // Sort files to ensure consistent ordering (allow policies before deny-all) + slices.Sort(files) + var hash string for _, file := range files { assetPath := filepath.Join(networkPolicyDir, file) @@ -1474,6 +1514,158 @@ func (c *TargetConfigReconciler) adjustWebhookNetworkPolicyForPlatform(policy *n return policy } +// ensurePrometheusRBAC creates RBAC resources to allow Prometheus to scrape operator metrics. +// This includes a Role granting permissions to list pods/services/endpoints and a RoleBinding +// binding the prometheus-k8s service account to the Role. +func (c *TargetConfigReconciler) ensurePrometheusRBAC(ctx context.Context) error { + klog.Info("Creating Prometheus RBAC resources...") + + // Create Role + roleAssetPath := "assets/kueue-operator/prometheus-rbac/role.yaml" + roleData, err := bindata.Asset(roleAssetPath) + if err != nil { + return fmt.Errorf("failed to load Prometheus Role asset: %w", err) + } + + role := resourceread.ReadRoleV1OrDie(roleData) + role.Namespace = c.operatorNamespace + + _, _, err = resourceapply.ApplyRole(ctx, c.kubeClient.RbacV1(), c.eventRecorder, role) + if err != nil { + return fmt.Errorf("failed to apply Prometheus Role: %w", err) + } + + // Create RoleBinding + roleBindingAssetPath := "assets/kueue-operator/prometheus-rbac/rolebinding.yaml" + roleBindingData, err := bindata.Asset(roleBindingAssetPath) + if err != nil { + return fmt.Errorf("failed to load Prometheus RoleBinding asset: %w", err) + } + + roleBinding := resourceread.ReadRoleBindingV1OrDie(roleBindingData) + roleBinding.Namespace = c.operatorNamespace + + _, _, err = resourceapply.ApplyRoleBinding(ctx, c.kubeClient.RbacV1(), c.eventRecorder, roleBinding) + if err != nil { + return fmt.Errorf("failed to apply Prometheus RoleBinding: %w", err) + } + + klog.Info("Prometheus RBAC resources created successfully") + return nil +} + +// ensureOperatorServiceMonitor creates a ServiceMonitor for the operator's metrics endpoint +// if the ServiceMonitor CRD is available. This is optional functionality that enables +// Prometheus integration on clusters with Prometheus Operator installed. +func (c *TargetConfigReconciler) ensureOperatorServiceMonitor(ctx context.Context) error { + klog.Info("Creating operator ServiceMonitor...") + + assetPath := "assets/kueue-operator/servicemonitor/operator-metrics.yaml" + data, err := bindata.Asset(assetPath) + if err != nil { + return fmt.Errorf("failed to load ServiceMonitor asset: %w", err) + } + + // Parse the ServiceMonitor YAML + obj := &unstructured.Unstructured{} + if err := yaml.Unmarshal(data, obj); err != nil { + return fmt.Errorf("failed to unmarshal ServiceMonitor: %w", err) + } + + // Set the namespace + obj.SetNamespace(c.operatorNamespace) + + // Create or update the ServiceMonitor using dynamic client + gvr := schema.GroupVersionResource{ + Group: "monitoring.coreos.com", + Version: "v1", + Resource: "servicemonitors", + } + + existing, err := c.dynamicClient.Resource(gvr).Namespace(c.operatorNamespace).Get( + ctx, + obj.GetName(), + metav1.GetOptions{}, + ) + + if errors.IsNotFound(err) { + klog.Infof("Creating ServiceMonitor: %s", obj.GetName()) + _, err = c.dynamicClient.Resource(gvr).Namespace(c.operatorNamespace).Create( + ctx, + obj, + metav1.CreateOptions{}, + ) + if err != nil { + return fmt.Errorf("failed to create ServiceMonitor: %w", err) + } + klog.Info("Operator ServiceMonitor created successfully") + } else if err != nil { + return fmt.Errorf("failed to get ServiceMonitor: %w", err) + } else { + // Update if spec has changed + existingSpec, _, _ := unstructured.NestedMap(existing.Object, "spec") + requiredSpec, _, _ := unstructured.NestedMap(obj.Object, "spec") + + existingHash, _ := computeSpecHash(existingSpec) + requiredHash, _ := computeSpecHash(requiredSpec) + + if existingHash != requiredHash { + klog.Infof("Updating ServiceMonitor: %s", obj.GetName()) + obj.SetResourceVersion(existing.GetResourceVersion()) + _, err = c.dynamicClient.Resource(gvr).Namespace(c.operatorNamespace).Update( + ctx, + obj, + metav1.UpdateOptions{}, + ) + if err != nil { + return fmt.Errorf("failed to update ServiceMonitor: %w", err) + } + klog.Info("Operator ServiceMonitor updated successfully") + } else { + klog.V(4).Info("Operator ServiceMonitor already up to date") + } + } + + return nil +} + +// ensureOperatorNetworkPolicies creates NetworkPolicies for the operator pod. +// These are applied once during controller initialization and do not have owner references +// since they need to persist even when no Kueue CR exists. +func (c *TargetConfigReconciler) ensureOperatorNetworkPolicies(ctx context.Context) error { + klog.Info("Creating operator NetworkPolicies...") + + networkPolicyDir := "assets/kueue-operator/networkpolicy/operator" + files, err := bindata.AssetDir(networkPolicyDir) + if err != nil { + return fmt.Errorf("failed to read networkpolicy from directory %q: %w", networkPolicyDir, err) + } + + // Sort files to ensure consistent ordering (allow policies before deny-all) + // Note: AssetDir already filters out directories, so only files are returned + slices.Sort(files) + + for _, file := range files { + assetPath := filepath.Join(networkPolicyDir, file) + want := utilresourceapply.ReadNetworkPolicyV1OrDie(bindata.MustAsset(assetPath)) + want.Namespace = c.operatorNamespace + + // Apply without owner reference - these policies persist independently + policy, updated, err := c.applyNetworkPolicyWithCache(ctx, want) + if err != nil { + return fmt.Errorf("failed to apply NetworkPolicy %s: %w", want.Name, err) + } + if updated { + klog.Infof("Operator NetworkPolicy %s updated", policy.Name) + } else { + klog.V(4).Infof("Operator NetworkPolicy %s already up to date", policy.Name) + } + } + + klog.Info("Operator NetworkPolicies ensured successfully") + return nil +} + func (c *TargetConfigReconciler) manageOpenshiftClusterRolesBindingForKueue(ctx context.Context, ownerReference metav1.OwnerReference) (*rbacv1.ClusterRoleBinding, bool, error) { clusterRoleBinding := &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ diff --git a/test/e2e/e2e_operator_test.go b/test/e2e/e2e_operator_test.go index eb3792dbb..b2397df53 100644 --- a/test/e2e/e2e_operator_test.go +++ b/test/e2e/e2e_operator_test.go @@ -306,19 +306,8 @@ var _ = Describe("Kueue Operator", Label("operator"), Ordered, func() { }, testutils.OperatorReadyTime, testutils.OperatorPoll).Should(Succeed(), "webhook configurations are not ready") }) - It("verify that deny-all network policy is present", func() { - // Skip this test if not running on OpenShift - _, err := kubeClient.Discovery().ServerResourcesForGroupVersion("route.openshift.io/v1") - if err != nil { - Skip("Skipping deny-all network policy test - not running on OpenShift") - } - + It("verify that deny-all network policy is present for operand", Label("network-policy"), func() { Eventually(func() error { - const ( - denyLabelKey = "app.openshift.io/name" - denyLabelValue = "kueue" - ) - deny, err := kubeClient.NetworkingV1().NetworkPolicies(testutils.OperatorNamespace).Get( context.Background(), "kueue-deny-all", @@ -330,8 +319,11 @@ var _ = Describe("Kueue Operator", Label("operator"), Ordered, func() { // deny-all policy should prohibit all traffic // (ingress and egress) for pods that has the - // label 'app.openshift.io/name: kueue' - Expect(deny.Spec.PodSelector.MatchLabels).To(Equal(map[string]string{denyLabelKey: denyLabelValue})) + // label 'control-plane: controller-manager' (operand pods) + Expect(deny.Spec.PodSelector.MatchLabels).To(Equal(map[string]string{ + "app.kubernetes.io/name": "kueue", + "control-plane": "controller-manager", + })) Expect(deny.Spec.Ingress).To(BeEmpty()) Expect(deny.Spec.Egress).To(BeEmpty()) Expect(deny.Spec.PolicyTypes).To(Equal([]networkingv1.PolicyType{ @@ -339,23 +331,123 @@ var _ = Describe("Kueue Operator", Label("operator"), Ordered, func() { networkingv1.PolicyTypeEgress, })) - // make sure that both our operator pod and the - // operand pod have the right label + // make sure that operand pods have the right label pods, err := kubeClient.CoreV1().Pods(testutils.OperatorNamespace).List(context.Background(), metav1.ListOptions{}) if err != nil { return err // retry } - operatorPods := findOperatorPods(testutils.OperatorNamespace, pods) - Expect(len(operatorPods)).To(BeNumerically(">=", 1), "no operator pod seen") kueuePods := findKueuePods(pods) Expect(len(kueuePods)).To(BeNumerically(">=", 1), "no kueue pod seen") - for _, pod := range append(operatorPods, kueuePods...) { - Expect(pod.Labels).To(HaveKeyWithValue(denyLabelKey, denyLabelValue)) + for _, pod := range kueuePods { + Expect(pod.Labels).To(HaveKeyWithValue("control-plane", "controller-manager")) } return nil - }, testutils.OperatorReadyTime, testutils.OperatorPoll).Should(Succeed(), "network policy has not been setup") + }, testutils.OperatorReadyTime, testutils.OperatorPoll).Should(Succeed(), "operand network policy has not been setup") + }) + + It("verify operator NetworkPolicies are created on startup", Label("network-policy"), func() { + ctx := context.Background() + operatorNetPols := []string{ + "kueue-operator-allow-egress-kube-apiserver", + "kueue-operator-allow-egress-cluster-dns", + "kueue-operator-allow-ingress-metrics", + "kueue-operator-deny-all", + } + + for _, name := range operatorNetPols { + netpol, err := kubeClient.NetworkingV1().NetworkPolicies(testutils.OperatorNamespace).Get( + ctx, + name, + metav1.GetOptions{}, + ) + Expect(err).NotTo(HaveOccurred(), "Operator NetworkPolicy %s should exist", name) + + // Verify pod selector targets operator pods + Expect(netpol.Spec.PodSelector.MatchLabels).To(HaveKeyWithValue("app.openshift.io/component", "operator")) + Expect(netpol.Spec.PodSelector.MatchLabels).To(HaveKeyWithValue("app.openshift.io/name", "kueue")) + + // Verify labels for tracking + Expect(netpol.Labels).To(HaveKeyWithValue("app.kubernetes.io/managed-by", "kueue-operator")) + Expect(netpol.Labels).To(HaveKeyWithValue("app.kubernetes.io/component", "operator-network-policy")) + } + }) + + It("verify operator NetworkPolicies target correct pods", Label("network-policy"), func() { + ctx := context.Background() + pods, err := kubeClient.CoreV1().Pods(testutils.OperatorNamespace).List(ctx, metav1.ListOptions{}) + Expect(err).NotTo(HaveOccurred()) + + operatorPods := findOperatorPods(testutils.OperatorNamespace, pods) + Expect(len(operatorPods)).To(BeNumerically(">=", 1), "no operator pod seen") + + // Verify operator pods have the correct label for NetworkPolicy selection + for _, pod := range operatorPods { + Expect(pod.Labels).To(HaveKeyWithValue("name", testutils.OperatorNamespace)) + } + }) + + It("verify operand NetworkPolicies have correct pod selectors", Label("network-policy"), func() { + ctx := context.Background() + operandNetPols := []string{ + "kueue-allow-egress-kube-apiserver", + "kueue-allow-egress-cluster-dns", + "kueue-allow-ingress-webhook", + "kueue-allow-ingress-visibility", + "kueue-allow-ingress-metrics", + "kueue-deny-all", + } + + for _, name := range operandNetPols { + netpol, err := kubeClient.NetworkingV1().NetworkPolicies(testutils.OperatorNamespace).Get( + ctx, + name, + metav1.GetOptions{}, + ) + Expect(err).NotTo(HaveOccurred(), "Operand NetworkPolicy %s should exist", name) + + // Verify pod selector targets operand pods + Expect(netpol.Spec.PodSelector.MatchLabels).To(HaveKeyWithValue("app.kubernetes.io/name", "kueue")) + Expect(netpol.Spec.PodSelector.MatchLabels).To(HaveKeyWithValue("control-plane", "controller-manager")) + } + }) + + It("verify separation between operator and operand NetworkPolicies", Label("network-policy"), func() { + ctx := context.Background() + + // Get all NetworkPolicies + netpolList, err := kubeClient.NetworkingV1().NetworkPolicies(testutils.OperatorNamespace).List( + ctx, + metav1.ListOptions{}, + ) + Expect(err).NotTo(HaveOccurred()) + + // Count operator vs operand policies + operatorPolicies := 0 + operandPolicies := 0 + + for _, netpol := range netpolList.Items { + if netpol.Labels != nil { + if component, ok := netpol.Labels["app.kubernetes.io/component"]; ok && component == "operator-network-policy" { + operatorPolicies++ + // Verify operator policies target operator pods + Expect(netpol.Spec.PodSelector.MatchLabels).To(HaveKeyWithValue("app.openshift.io/component", "operator")) + Expect(netpol.Spec.PodSelector.MatchLabels).To(HaveKeyWithValue("app.openshift.io/name", "kueue")) + } + } + + // Operand policies target control-plane pods + if selector := netpol.Spec.PodSelector.MatchLabels; selector != nil { + if _, ok := selector["control-plane"]; ok { + operandPolicies++ + } + } + } + + // We expect 4 operator policies and 6 operand policies + Expect(operatorPolicies).To(Equal(4), "Expected 4 operator NetworkPolicies") + Expect(operandPolicies).To(Equal(6), "Expected 6 operand NetworkPolicies") }) }) @@ -1210,17 +1302,39 @@ var _ = Describe("Kueue Operator", Label("operator"), Ordered, func() { return fmt.Errorf("ServiceAccount kueue-controller-manager still exists") } - klog.Infof("Verifying removal of all NetworkPolicies in namespace: %s", testutils.OperatorNamespace) + klog.Infof("Verifying removal of operand NetworkPolicies (operator NetworkPolicies should persist)") networkPolicyList, err := kubeClient.NetworkingV1().NetworkPolicies(testutils.OperatorNamespace).List(ctx, metav1.ListOptions{}) if err != nil { return fmt.Errorf("Failed to list NetworkPolicies: %v", err) } - if len(networkPolicyList.Items) > 0 { - var networkPolicyNames []string - for _, netpl := range networkPolicyList.Items { - networkPolicyNames = append(networkPolicyNames, netpl.Name) + + // Separate operator policies from operand policies + var operandPolicies []string + var operatorPolicies []string + for _, netpol := range networkPolicyList.Items { + isOperatorPolicy := false + if netpol.Labels != nil { + if component, ok := netpol.Labels["app.kubernetes.io/component"]; ok && component == "operator-network-policy" { + isOperatorPolicy = true + operatorPolicies = append(operatorPolicies, netpol.Name) + } + } + if !isOperatorPolicy { + operandPolicies = append(operandPolicies, netpol.Name) } - return fmt.Errorf("NetworkPolicies still exist in namespace %s: %v", testutils.OperatorNamespace, networkPolicyNames) + } + + // Operand NetworkPolicies should be deleted (they have owner references) + if len(operandPolicies) > 0 { + return fmt.Errorf("Operand NetworkPolicies still exist in namespace %s: %v", testutils.OperatorNamespace, operandPolicies) + } + + // Operator NetworkPolicies should persist (no owner references) + klog.Infof("Operator NetworkPolicies correctly persisted: %v", operatorPolicies) + expectedOperatorPolicies := 4 + if len(operatorPolicies) != expectedOperatorPolicies { + return fmt.Errorf("Expected %d operator NetworkPolicies to persist, found %d: %v", + expectedOperatorPolicies, len(operatorPolicies), operatorPolicies) } klog.Infof("Verifying removal of all Services in namespace: %s", testutils.OperatorNamespace)