diff --git a/api/policies/v1/policyserver_types.go b/api/policies/v1/policyserver_types.go index 52cd73b13..413d8034e 100644 --- a/api/policies/v1/policyserver_types.go +++ b/api/policies/v1/policyserver_types.go @@ -175,7 +175,7 @@ type PolicyServerSpec struct { // Port exposed by the metrics Service for this policy server. // When unset, defaults to the controller-wide default - // (KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var, or 8080). + // (--policy-server-metrics-port CLI flag, deprecated KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var, or 8080). // Only relevant when metrics are enabled. // // Use this field to customize which port Prometheus scrapes for this diff --git a/charts/kubewarden-controller/templates/deployment.yaml b/charts/kubewarden-controller/templates/deployment.yaml index 290c8e2f1..abdab6bf9 100644 --- a/charts/kubewarden-controller/templates/deployment.yaml +++ b/charts/kubewarden-controller/templates/deployment.yaml @@ -83,6 +83,9 @@ spec: {{- if and (eq .Values.telemetry.mode "sidecar") }} - --enable-otel-sidecar {{- end }} + {{- if and .Values.telemetry.metrics (eq .Values.telemetry.mode "sidecar") }} + - --policy-server-metrics-port={{ .Values.telemetry.sidecar.metrics.port | default 8080 }} + {{- end }} {{- if .Values.telemetry.metrics }} - --enable-metrics {{- end }} @@ -100,8 +103,6 @@ spec: - /controller env: {{- if and .Values.telemetry.metrics (eq .Values.telemetry.mode "sidecar") }} - - name: KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT - value: "{{ .Values.telemetry.sidecar.metrics.port | default 8080 }}" - name: OTEL_EXPORTER_OTLP_ENDPOINT value: "https://localhost:4317" - name: OTEL_EXPORTER_OTLP_INSECURE diff --git a/charts/kubewarden-controller/tests/telemetry_configuration_test.yaml b/charts/kubewarden-controller/tests/telemetry_configuration_test.yaml index 4339e11b8..1d240ad13 100644 --- a/charts/kubewarden-controller/tests/telemetry_configuration_test.yaml +++ b/charts/kubewarden-controller/tests/telemetry_configuration_test.yaml @@ -37,10 +37,9 @@ tests: content: --opentelemetry-client-certificate-secret=my-client-cert - notContains: - path: spec.template.spec.containers[0].env + path: spec.template.spec.containers[0].args content: - name: KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT - value: "8080" + --policy-server-metrics-port=8080 - contains: path: spec.template.spec.containers[0].env content: @@ -138,10 +137,9 @@ tests: content: --opentelemetry-client-certificate-secret= - contains: - path: spec.template.spec.containers[0].env + path: spec.template.spec.containers[0].args content: - name: KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT - value: "8080" + --policy-server-metrics-port=8080 - contains: path: spec.template.spec.containers[0].env content: @@ -202,3 +200,26 @@ tests: path: tls.crt - key: tls.key path: tls.key + - it: "sidecar tracing should not set policy server metrics port when metrics are disabled" + set: + telemetry: + mode: "sidecar" + metrics: false + tracing: true + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: + --enable-otel-sidecar + - notContains: + path: spec.template.spec.containers[0].args + content: + --enable-metrics + - contains: + path: spec.template.spec.containers[0].args + content: + --enable-tracing + - notContains: + path: spec.template.spec.containers[0].args + content: + --policy-server-metrics-port=8080 diff --git a/charts/kubewarden-crds/templates/crds/policies.kubewarden.io_policyservers.yaml b/charts/kubewarden-crds/templates/crds/policies.kubewarden.io_policyservers.yaml index 0b0bf5d2b..106132fb3 100644 --- a/charts/kubewarden-crds/templates/crds/policies.kubewarden.io_policyservers.yaml +++ b/charts/kubewarden-crds/templates/crds/policies.kubewarden.io_policyservers.yaml @@ -1177,7 +1177,7 @@ spec: description: |- Port exposed by the metrics Service for this policy server. When unset, defaults to the controller-wide default - (KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var, or 8080). + (--policy-server-metrics-port CLI flag, deprecated KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var, or 8080). Only relevant when metrics are enabled. Use this field to customize which port Prometheus scrapes for this diff --git a/cmd/controller/main.go b/cmd/controller/main.go index d2ae3ad32..f5c614f79 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -116,10 +116,16 @@ func main() { var openTelemetryClientCertificateSecret string var openTelemetryCertificateSecret string var imagePullSecretsFlag string + var policyServerMetricsPort int flag.StringVar(&mgrOpts.MetricsAddr, "metrics-bind-address", ":8088", "The address the controller-runtime metric endpoint binds to.") flag.StringVar(&mgrOpts.ProbeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.IntVar(&mgrOpts.WebhookServerPort, "webhook-server-port", 9443, "The port the webhook server listens on.") + flag.IntVar(&policyServerMetricsPort, + "policy-server-metrics-port", + constants.PolicyServerMetricsPort, + "The default port exposed by every PolicyServer metrics Service. "+ + "Per-PolicyServer overrides (spec.metricsPort) always take priority.") flag.BoolVar(&mgrOpts.EnableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") @@ -159,6 +165,12 @@ func main() { opts := zap.Options{} opts.BindFlags(flag.CommandLine) flag.Parse() + policyServerMetricsPortFlagSet := false + flag.Visit(func(f *flag.Flag) { + if f.Name == "policy-server-metrics-port" { + policyServerMetricsPortFlagSet = true + } + }) mgrOpts.EnableMutualTLS = config.ClientCAConfigMapName != "" config.ImagePullSecrets = parseImagePullSecrets(imagePullSecretsFlag) ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) @@ -195,30 +207,54 @@ func main() { "Use only when the Kubernetes API server cannot reach pod-network webhook endpoints.") } - // Read the global default metrics port for PolicyServer services from the - // environment variable, falling back to the hardcoded constant. - policyServerMetricsPort := int32(constants.PolicyServerMetricsPort) + // Validate --policy-server-metrics-port range. + if int64(policyServerMetricsPort) < minAllowedPort || + int64(policyServerMetricsPort) > maxAllowedPort { + setupLog.Error( + errors.New("port must be between 1 and 65535"), + "invalid policy server metrics port", + "flag", "--policy-server-metrics-port", + "value", policyServerMetricsPort, + "min", minAllowedPort, + "max", maxAllowedPort, + ) + retcode = 1 + return + } if envPort := os.Getenv(constants.PolicyServerMetricsPortEnvVar); envPort != "" { - parsed, err := strconv.ParseInt(envPort, 10, 32) - if err != nil { - setupLog.Error(err, "cannot parse env var as integer port", - "envVar", constants.PolicyServerMetricsPortEnvVar, "value", envPort) - retcode = 1 - return - } - if parsed < minAllowedPort || parsed > maxAllowedPort { - setupLog.Error( - errors.New("port must be between 1 and 65535"), - "invalid env var port value", + if policyServerMetricsPortFlagSet { + setupLog.Info( + "WARNING: deprecated environment variable ignored because --policy-server-metrics-port was set", "envVar", constants.PolicyServerMetricsPortEnvVar, - "value", envPort, - "min", minAllowedPort, - "max", maxAllowedPort, + "flag", "--policy-server-metrics-port", + ) + } else { + envPortParsed, err := strconv.ParseInt(envPort, 10, 32) + if err != nil { + setupLog.Error(err, "invalid policy server metrics port environment variable", + "envVar", constants.PolicyServerMetricsPortEnvVar, "value", envPort) + retcode = 1 + return + } + policyServerMetricsPort = int(envPortParsed) + setupLog.Info( + "WARNING: deprecated environment variable used as fallback; use --policy-server-metrics-port instead", + "envVar", constants.PolicyServerMetricsPortEnvVar, + "flag", "--policy-server-metrics-port", ) - retcode = 1 - return } - policyServerMetricsPort = int32(parsed) + } + if int64(policyServerMetricsPort) < minAllowedPort || + int64(policyServerMetricsPort) > maxAllowedPort { + setupLog.Error( + errors.New("port must be between 1 and 65535"), + "invalid policy server metrics port", + "value", policyServerMetricsPort, + "min", minAllowedPort, + "max", maxAllowedPort, + ) + retcode = 1 + return } if enableMetrics { @@ -267,7 +303,7 @@ func main() { mgrOpts.DeploymentsNamespace, config, otelConfiguration, - policyServerMetricsPort, + int32(policyServerMetricsPort), ); err != nil { setupLog.Error(err, "unable to create controllers") retcode = 1 diff --git a/docs/crds/CRD-docs-for-docs-repo.adoc b/docs/crds/CRD-docs-for-docs-repo.adoc index 370a7a659..2fc6645ee 100644 --- a/docs/crds/CRD-docs-for-docs-repo.adoc +++ b/docs/crds/CRD-docs-for-docs-repo.adoc @@ -959,7 +959,7 @@ Optional: \{} + | *`metricsPort`* __integer__ | Port exposed by the metrics Service for this policy server. + When unset, defaults to the controller-wide default + -(KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var, or 8080). + +(--policy-server-metrics-port CLI flag, deprecated KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var, or 8080). + Only relevant when metrics are enabled. + Use this field to customize which port Prometheus scrapes for this + @@ -1588,4 +1588,3 @@ _Underlying type:_ _string_ - diff --git a/docs/crds/CRD-docs-for-docs-repo.md b/docs/crds/CRD-docs-for-docs-repo.md index 0c78a1c4b..8f8cf572d 100644 --- a/docs/crds/CRD-docs-for-docs-repo.md +++ b/docs/crds/CRD-docs-for-docs-repo.md @@ -542,7 +542,7 @@ _Appears in:_ | `namespacedPoliciesCapabilities` _string array_ | NamespacedPoliciesCapabilities lists host capability API calls allowed
for namespaced policies running on this PolicyServer. When not set,
no host capabilities are granted to namespaced policies.
Supported wildcard patterns:
- "*": allow all host capabilities
- "category/*": allow all capabilities in a category (e.g. "oci/*")
- "category/version/*": allow all capabilities of a specific version (e.g. "oci/v1/*")
- Specific capability paths (e.g. "oci/v1/verify", "net/v1/dns_lookup_host") | | Optional: \{\}
| | `webhookPort` _integer_ | Port where the policy server listens for incoming webhook requests.
When unset, defaults to 8443. This is the port the Kubernetes API server
reaches when evaluating admission requests. | | Maximum: 65535
Minimum: 1
Optional: \{\}
| | `readinessProbePort` _integer_ | Port used by the policy server to expose the readiness probe endpoint.
When unset, defaults to 8081. | | Maximum: 65535
Minimum: 1
Optional: \{\}
| -| `metricsPort` _integer_ | Port exposed by the metrics Service for this policy server.
When unset, defaults to the controller-wide default
(KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var, or 8080).
Only relevant when metrics are enabled.
Use this field to customize which port Prometheus scrapes for this
PolicyServer's metrics Service (e.g. to match naming conventions or
avoid Service-level port collisions).
NOTE: this field controls only the Service Port (the externally visible
scrape port). The Service TargetPort — the port the pod actually listens
on — is always the controller-wide default and is not affected by this
field. This is intentional: when the OpenTelemetry sidecar mode is
enabled, each pod gets its own injected sidecar, but the pod-side
Prometheus listener port is determined by controller-wide/injection
configuration, not per PolicyServer. Therefore, changing this field does
not change the pod listener port and will not resolve pod-port conflicts
such as those caused by hostNetwork. | | Maximum: 65535
Minimum: 1
Optional: \{\}
| +| `metricsPort` _integer_ | Port exposed by the metrics Service for this policy server.
When unset, defaults to the controller-wide default
(--policy-server-metrics-port CLI flag, deprecated KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var, or 8080).
Only relevant when metrics are enabled.
Use this field to customize which port Prometheus scrapes for this
PolicyServer's metrics Service (e.g. to match naming conventions or
avoid Service-level port collisions).
NOTE: this field controls only the Service Port (the externally visible
scrape port). The Service TargetPort — the port the pod actually listens
on — is always the controller-wide default and is not affected by this
field. This is intentional: when the OpenTelemetry sidecar mode is
enabled, each pod gets its own injected sidecar, but the pod-side
Prometheus listener port is determined by controller-wide/injection
configuration, not per PolicyServer. Therefore, changing this field does
not change the pod listener port and will not resolve pod-port conflicts
such as those caused by hostNetwork. | | Maximum: 65535
Minimum: 1
Optional: \{\}
| @@ -878,4 +878,3 @@ _Appears in:_ - diff --git a/internal/controller/policyserver_controller.go b/internal/controller/policyserver_controller.go index b8a2d042b..baaaec487 100644 --- a/internal/controller/policyserver_controller.go +++ b/internal/controller/policyserver_controller.go @@ -75,8 +75,10 @@ type PolicyServerReconciler struct { // so that in-cluster DNS resolution keeps working. HostNetwork bool // PolicyServerMetricsPort is the global default metrics port for PolicyServer - // Service objects. It is populated from the KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT - // environment variable at startup, falling back to constants.PolicyServerMetricsPort. + // Service objects. It is populated from the --policy-server-metrics-port CLI + // flag at startup, falling back to the deprecated + // KUBEWARDEN_POLICY_SERVER_SERVICES_METRICS_PORT env var and then to + // constants.PolicyServerMetricsPort. // A per-PolicyServer CRD field (spec.metricsPort) always takes priority. PolicyServerMetricsPort int32 } diff --git a/internal/controller/policyserver_controller_service_test.go b/internal/controller/policyserver_controller_service_test.go index 7c34b4022..56eaacbfb 100644 --- a/internal/controller/policyserver_controller_service_test.go +++ b/internal/controller/policyserver_controller_service_test.go @@ -72,7 +72,7 @@ func findServicePort(ports []corev1.ServicePort, name string) *corev1.ServicePor // TestUpdateServiceMetricsPortPriorityChain validates the 3-tier priority // chain for the metrics Service Port: // -// CRD field (spec.metricsPort) > env var (PolicyServerMetricsPort) > constant (8080) +// CRD field (spec.metricsPort) > --policy-server-metrics-port CLI flag > deprecated env var > constant (8080) // // It also verifies that the Service TargetPort is always fixed at the // controller-wide PolicyServerMetricsPort regardless of any CRD override. @@ -87,14 +87,14 @@ func TestUpdateServiceMetricsPortPriorityChain(t *testing.T) { tests := []struct { name string metricsEnabled bool - policyServerMetricPort int32 // simulates env var / constant + policyServerMetricPort int32 // simulates controller-wide CLI flag / env var / constant crdMetricsPort *int32 expectMetricsPort bool expectedPort int32 // Service Port expectedTargetPort int32 // Service TargetPort }{ { - name: "env var only (no CRD override)", + name: "CLI flag only (no CRD override)", metricsEnabled: true, policyServerMetricPort: 9090, crdMetricsPort: nil, @@ -103,7 +103,7 @@ func TestUpdateServiceMetricsPortPriorityChain(t *testing.T) { expectedTargetPort: 9090, }, { - name: "CRD overrides env var", + name: "CRD overrides CLI flag", metricsEnabled: true, policyServerMetricPort: 9090, crdMetricsPort: int32Ptr(9091), @@ -170,7 +170,7 @@ func TestUpdateServiceMetricsPortPriorityChain(t *testing.T) { require.NotNil(t, metricsPort, "metrics port must exist when metrics are enabled") assert.Equal(t, tc.expectedPort, metricsPort.Port, - "Service Port should respect CRD > env var > constant priority") + "Service Port should respect CRD > CLI flag > constant priority") assert.Equal(t, intstr.FromInt32(tc.expectedTargetPort), metricsPort.TargetPort, "Service TargetPort must always equal the global PolicyServerMetricsPort (fixed regardless of spec.metricsPort, to avoid breaking OTel sidecar mode)") assert.Equal(t, corev1.ProtocolTCP, metricsPort.Protocol)