From 94f818c8c05e7cf3f5cfe57c4ec0e054a5aa3616 Mon Sep 17 00:00:00 2001 From: zou rui Date: Tue, 18 Jun 2024 08:36:23 +0800 Subject: [PATCH] add CEL validation for BackendRef Group (#3557) * helm: add envoy gateway addon helm chart support (#3470) * initial dashboard addon helm chart Signed-off-by: shawnh2 * rename addon name and remove gateway-helm support Signed-off-by: shawnh2 * remove /charts from .helmignore Signed-off-by: shawnh2 * rename to gateway-addons-helm and keep one source of truth Signed-off-by: shawnh2 * restore examples values and fix comments Signed-off-by: shawnh2 * rewrite helm makefile Signed-off-by: shawnh2 --------- Signed-off-by: shawnh2 Signed-off-by: phantooom * chore: Remove namespace restriction for EnvoyProxy parametersRef resource Signed-off-by: phantooom * chore: add CEL validation for BackendRef Group Signed-off-by: phantooom * chore: add CEL validation for BackendRef Group Signed-off-by: phantooom --------- Signed-off-by: shawnh2 Signed-off-by: phantooom Co-authored-by: sh2 Co-authored-by: zirain --- api/v1alpha1/accesslogging_types.go | 5 + api/v1alpha1/envoyproxy_metric_types.go | 1 + api/v1alpha1/ext_auth_types.go | 2 + api/v1alpha1/tracing_types.go | 1 + .../gateway.envoyproxy.io_envoyproxies.yaml | 13 +++ ...ateway.envoyproxy.io_securitypolicies.yaml | 4 + test/cel-validation/envoyproxy_test.go | 99 +++++++++++++++++++ 7 files changed, 125 insertions(+) diff --git a/api/v1alpha1/accesslogging_types.go b/api/v1alpha1/accesslogging_types.go index 4d075bfedaa..d1f60203138 100644 --- a/api/v1alpha1/accesslogging_types.go +++ b/api/v1alpha1/accesslogging_types.go @@ -11,6 +11,8 @@ type ProxyAccessLog struct { // Settings defines accesslog settings for managed proxies. // If unspecified, will send default format to stdout. // +optional + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=50 Settings []ProxyAccessLogSetting `json:"settings,omitempty"` } @@ -19,6 +21,7 @@ type ProxyAccessLogSetting struct { Format ProxyAccessLogFormat `json:"format"` // Sinks defines the sinks of accesslog. // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=50 Sinks []ProxyAccessLogSink `json:"sinks"` } @@ -120,6 +123,7 @@ type ALSEnvoyProxyAccessLog struct { // +kubebuilder:validation:MinItems=1 // +kubebuilder:validation:MaxItems=1 // +kubebuilder:validation:XValidation:message="BackendRefs only supports Service kind.",rule="self.all(f, f.kind == 'Service')" + // +kubebuilder:validation:XValidation:message="BackendRefs only supports Core group.",rule="self.all(f, f.group == '')" BackendRefs []BackendRef `json:"backendRefs"` // LogName defines the friendly name of the access log to be returned in // StreamAccessLogsMessage.Identifier. This allows the access log server @@ -176,6 +180,7 @@ type OpenTelemetryEnvoyProxyAccessLog struct { // +optional // +kubebuilder:validation:MaxItems=1 // +kubebuilder:validation:XValidation:message="only support Service kind.",rule="self.all(f, f.kind == 'Service')" + // +kubebuilder:validation:XValidation:message="BackendRefs only supports Core group.",rule="self.all(f, f.group == '')" BackendRefs []BackendRef `json:"backendRefs,omitempty"` // Resources is a set of labels that describe the source of a log entry, including envoy node info. // It's recommended to follow [semantic conventions](https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/). diff --git a/api/v1alpha1/envoyproxy_metric_types.go b/api/v1alpha1/envoyproxy_metric_types.go index bf321febcbb..8791ddbd490 100644 --- a/api/v1alpha1/envoyproxy_metric_types.go +++ b/api/v1alpha1/envoyproxy_metric_types.go @@ -75,6 +75,7 @@ type ProxyOpenTelemetrySink struct { // +optional // +kubebuilder:validation:MaxItems=1 // +kubebuilder:validation:XValidation:message="only support Service kind.",rule="self.all(f, f.kind == 'Service')" + // +kubebuilder:validation:XValidation:message="BackendRefs only supports Core group.",rule="self.all(f, f.group == '')" BackendRefs []BackendRef `json:"backendRefs,omitempty"` // TODO: add support for customizing OpenTelemetry sink in https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/stat_sinks/open_telemetry/v3/open_telemetry.proto#envoy-v3-api-msg-extensions-stat-sinks-open-telemetry-v3-sinkconfig diff --git a/api/v1alpha1/ext_auth_types.go b/api/v1alpha1/ext_auth_types.go index a1cd40bb141..13de5f9f6ac 100644 --- a/api/v1alpha1/ext_auth_types.go +++ b/api/v1alpha1/ext_auth_types.go @@ -71,6 +71,7 @@ type GRPCExtAuthService struct { // +optional // +kubebuilder:validation:MaxItems=1 // +kubebuilder:validation:XValidation:message="only support Service kind.",rule="self.all(f, f.kind == 'Service')" + // +kubebuilder:validation:XValidation:message="BackendRefs only supports Core group.",rule="self.all(f, f.group == '')" BackendRefs []BackendRef `json:"backendRefs,omitempty"` } @@ -92,6 +93,7 @@ type HTTPExtAuthService struct { // +optional // +kubebuilder:validation:MaxItems=1 // +kubebuilder:validation:XValidation:message="only support Service kind.",rule="self.all(f, f.kind == 'Service')" + // +kubebuilder:validation:XValidation:message="BackendRefs only supports Core group.",rule="self.all(f, f.group == '')" BackendRefs []BackendRef `json:"backendRefs,omitempty"` // Path is the path of the HTTP External Authorization service. diff --git a/api/v1alpha1/tracing_types.go b/api/v1alpha1/tracing_types.go index a2a3f7b5617..1b8b55edc47 100644 --- a/api/v1alpha1/tracing_types.go +++ b/api/v1alpha1/tracing_types.go @@ -56,6 +56,7 @@ type TracingProvider struct { // +optional // +kubebuilder:validation:MaxItems=1 // +kubebuilder:validation:XValidation:message="only support Service kind.",rule="self.all(f, f.kind == 'Service')" + // +kubebuilder:validation:XValidation:message="BackendRefs only supports Core group.",rule="self.all(f, f.group == '')" BackendRefs []BackendRef `json:"backendRefs,omitempty"` } diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml index 75ee139934c..2da9760560f 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml @@ -10373,6 +10373,9 @@ spec: - message: BackendRefs only supports Service kind. rule: self.all(f, f.kind == 'Service') + - message: BackendRefs only supports Core + group. + rule: self.all(f, f.group == '') http: description: HTTP defines additional configuration specific to HTTP access logs. @@ -10522,6 +10525,9 @@ spec: x-kubernetes-validations: - message: only support Service kind. rule: self.all(f, f.kind == 'Service') + - message: BackendRefs only supports Core + group. + rule: self.all(f, f.group == '') host: description: |- Host define the extension service hostname. @@ -10568,12 +10574,15 @@ spec: openTelemetry field needs to be set. rule: 'self.type == ''OpenTelemetry'' ? has(self.openTelemetry) : !has(self.openTelemetry)' + maxItems: 50 minItems: 1 type: array required: - format - sinks type: object + maxItems: 50 + minItems: 1 type: array type: object metrics: @@ -10748,6 +10757,8 @@ spec: x-kubernetes-validations: - message: only support Service kind. rule: self.all(f, f.kind == 'Service') + - message: BackendRefs only supports Core group. + rule: self.all(f, f.group == '') host: description: |- Host define the service hostname. @@ -10944,6 +10955,8 @@ spec: x-kubernetes-validations: - message: only support Service kind. rule: self.all(f, f.kind == 'Service') + - message: BackendRefs only supports Core group. + rule: self.all(f, f.group == '') host: description: |- Host define the provider service hostname. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 96da82b371a..74458a4ec48 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -424,6 +424,8 @@ spec: x-kubernetes-validations: - message: only support Service kind. rule: self.all(f, f.kind == 'Service') + - message: BackendRefs only supports Core group. + rule: self.all(f, f.group == '') type: object x-kubernetes-validations: - message: backendRef or backendRefs needs to be set @@ -618,6 +620,8 @@ spec: x-kubernetes-validations: - message: only support Service kind. rule: self.all(f, f.kind == 'Service') + - message: BackendRefs only supports Core group. + rule: self.all(f, f.group == '') headersToBackend: description: |- HeadersToBackend are the authorization response headers that will be added diff --git a/test/cel-validation/envoyproxy_test.go b/test/cel-validation/envoyproxy_test.go index a4c9621c418..5914e1dbc4b 100644 --- a/test/cel-validation/envoyproxy_test.go +++ b/test/cel-validation/envoyproxy_test.go @@ -587,6 +587,42 @@ func TestEnvoyProxyProvider(t *testing.T) { }, wantErrors: []string{"BackendRefs only supports Service Kind."}, }, + { + desc: "invalid-accesslog-ALS-backendrefs-group", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + AccessLog: &egv1a1.ProxyAccessLog{ + Settings: []egv1a1.ProxyAccessLogSetting{ + { + Format: egv1a1.ProxyAccessLogFormat{ + Type: "Text", + Text: ptr.To("[%START_TIME%]"), + }, + Sinks: []egv1a1.ProxyAccessLogSink{ + { + Type: egv1a1.ProxyAccessLogSinkTypeALS, + ALS: &egv1a1.ALSEnvoyProxyAccessLog{ + BackendRefs: []egv1a1.BackendRef{ + { + BackendObjectReference: gwapiv1.BackendObjectReference{ + Name: "fake-service", + Group: ptr.To(gwapiv1.Group("foo")), + }, + }, + }, + Type: egv1a1.ALSEnvoyProxyAccessLogTypeHTTP, + }, + }, + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{"BackendRefs only supports Core group."}, + }, { desc: "invalid-accesslog-ALS-no-backendrefs", mutate: func(envoy *egv1a1.EnvoyProxy) { @@ -749,6 +785,41 @@ func TestEnvoyProxyProvider(t *testing.T) { }, wantErrors: []string{"only support Service Kind."}, }, + { + desc: "invalid-accesslog-backendref-group", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + AccessLog: &egv1a1.ProxyAccessLog{ + Settings: []egv1a1.ProxyAccessLogSetting{ + { + Format: egv1a1.ProxyAccessLogFormat{ + Type: "Text", + Text: ptr.To("[%START_TIME%]"), + }, + Sinks: []egv1a1.ProxyAccessLogSink{ + { + Type: egv1a1.ProxyAccessLogSinkTypeOpenTelemetry, + OpenTelemetry: &egv1a1.OpenTelemetryEnvoyProxyAccessLog{ + BackendRefs: []egv1a1.BackendRef{ + { + BackendObjectReference: gwapiv1.BackendObjectReference{ + Name: "fake-service", + Group: ptr.To(gwapiv1.Group("foo")), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{"BackendRefs only supports Core group."}, + }, { desc: "accesslog-backendref", mutate: func(envoy *egv1a1.EnvoyProxy) { @@ -1058,6 +1129,34 @@ func TestEnvoyProxyProvider(t *testing.T) { }, wantErrors: []string{"only support Service Kind."}, }, + { + desc: "ProxyMetrics-sinks-invalid-backendref-group", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + Metrics: &egv1a1.ProxyMetrics{ + Sinks: []egv1a1.ProxyMetricSink{ + { + Type: egv1a1.MetricSinkTypeOpenTelemetry, + OpenTelemetry: &egv1a1.ProxyOpenTelemetrySink{ + BackendRefs: []egv1a1.BackendRef{ + { + BackendObjectReference: gwapiv1.BackendObjectReference{ + Name: "fake-service", + Group: ptr.To(gwapiv1.Group("foo")), + Port: ptr.To(gwapiv1.PortNumber(8080)), + }, + }, + }, + }, + }, + }, + }, + }, + } + }, + wantErrors: []string{"BackendRefs only supports Core group."}, + }, { desc: "invalid-tracing-backendref-invalid-kind", mutate: func(envoy *egv1a1.EnvoyProxy) {