Skip to content

Commit

Permalink
feat: add CEL validation for EnvoyProxy telemetry (#2050)
Browse files Browse the repository at this point in the history 
* add missing test cases for envoyproxy validate methods

Signed-off-by: sh2 <shawnhxh@outlook.com>

* typo: correct method comment

Signed-off-by: sh2 <shawnhxh@outlook.com>

* add CEL for envoy proxy telemetry and test cases

Signed-off-by: sh2 <shawnhxh@outlook.com>

---------

Signed-off-by: sh2 <shawnhxh@outlook.com>
  • Loading branch information
@shawnh2
shawnh2 authored Nov 13, 2023
1 parent f2e12a5 commit 7835e2f
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 10 deletions.
9 changes: 9 additions & 0 deletions api/v1alpha1/accesslogging_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ const (
// ProxyAccessLogFormat defines the format of accesslog.
// By default accesslogs are written to standard output.
// +union
//
// +kubebuilder:validation:XValidation:rule="self.type == 'Text' ? has(self.text) : !has(self.text)",message="If AccessLogFormat type is Text, text field needs to be set."
// +kubebuilder:validation:XValidation:rule="self.type == 'JSON' ? has(self.json) : !has(self.json)",message="If AccessLogFormat type is JSON, json field needs to be set."
type ProxyAccessLogFormat struct {
// Type defines the type of accesslog format.
// +kubebuilder:validation:Enum=Text;JSON
Expand Down Expand Up @@ -65,9 +68,15 @@ const (
ProxyAccessLogSinkTypeOpenTelemetry ProxyAccessLogSinkType = "OpenTelemetry"
)

// ProxyAccessLogSink defines the sink of accesslog.
// +union
//
// +kubebuilder:validation:XValidation:rule="self.type == 'File' ? has(self.file) : !has(self.file)",message="If AccessLogSink type is File, file field needs to be set."
// +kubebuilder:validation:XValidation:rule="self.type == 'OpenTelemetry' ? has(self.openTelemetry) : !has(self.openTelemetry)",message="If AccessLogSink type is OpenTelemetry, openTelemetry field needs to be set."
type ProxyAccessLogSink struct {
// Type defines the type of accesslog sink.
// +kubebuilder:validation:Enum=File;OpenTelemetry
// +unionDiscriminator
Type ProxyAccessLogSinkType `json:"type,omitempty"`
// File defines the file accesslog sink.
// +optional
Expand Down
7 changes: 7 additions & 0 deletions api/v1alpha1/envoyproxy_metric_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,21 @@ type ProxyMetrics struct {
EnableVirtualHostStats bool `json:"enableVirtualHostStats,omitempty"`
}

// ProxyMetricSink defines the sink of metrics.
// Default metrics sink is OpenTelemetry.
// +union
//
// +kubebuilder:validation:XValidation:rule="self.type == 'OpenTelemetry' ? has(self.openTelemetry) : !has(self.openTelemetry)",message="If MetricSink type is OpenTelemetry, openTelemetry field needs to be set."
type ProxyMetricSink struct {
// Type defines the metric sink type.
// EG currently only supports OpenTelemetry.
// +kubebuilder:validation:Enum=OpenTelemetry
// +kubebuilder:default=OpenTelemetry
// +unionDiscriminator
Type MetricSinkType `json:"type"`
// OpenTelemetry defines the configuration for OpenTelemetry sink.
// It's required if the sink type is OpenTelemetry.
// +optional
OpenTelemetry *ProxyOpenTelemetrySink `json:"openTelemetry,omitempty"`
}

Expand Down
8 changes: 4 additions & 4 deletions api/v1alpha1/shared_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,16 +260,16 @@ type StringMatch struct {
type StringMatchType string

const (
// MatchExact :the input string must match exactly the match value.
// StringMatchExact :the input string must match exactly the match value.
StringMatchExact StringMatchType = "Exact"

// MatchPrefix :the input string must start with the match value.
// StringMatchPrefix :the input string must start with the match value.
StringMatchPrefix StringMatchType = "Prefix"

// MatchSuffix :the input string must end with the match value.
// StringMatchSuffix :the input string must end with the match value.
StringMatchSuffix StringMatchType = "Suffix"

// MatchRegularExpression :The input string must match the regular expression
// StringMatchRegularExpression :The input string must match the regular expression
// specified in the match value.
// The regex string must adhere to the syntax documented in
// https://github.com/google/re2/wiki/Syntax.
Expand Down
25 changes: 25 additions & 0 deletions charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5350,9 +5350,18 @@ spec:
- JSON
type: string
type: object
x-kubernetes-validations:
- message: If AccessLogFormat type is Text, text field
needs to be set.
rule: 'self.type == ''Text'' ? has(self.text) : !has(self.text)'
- message: If AccessLogFormat type is JSON, json field
needs to be set.
rule: 'self.type == ''JSON'' ? has(self.json) : !has(self.json)'
sinks:
description: Sinks defines the sinks of accesslog.
items:
description: ProxyAccessLogSink defines the sink of
accesslog.
properties:
file:
description: File defines the file accesslog sink.
Expand Down Expand Up @@ -5397,6 +5406,15 @@ spec:
- OpenTelemetry
type: string
type: object
x-kubernetes-validations:
- message: If AccessLogSink type is File, file field
needs to be set.
rule: 'self.type == ''File'' ? has(self.file) :
!has(self.file)'
- message: If AccessLogSink type is OpenTelemetry,
openTelemetry field needs to be set.
rule: 'self.type == ''OpenTelemetry'' ? has(self.openTelemetry)
: !has(self.openTelemetry)'
minItems: 1
type: array
required:
Expand Down Expand Up @@ -5459,6 +5477,8 @@ spec:
description: Sinks defines the metric sinks where metrics
are sent to.
items:
description: ProxyMetricSink defines the sink of metrics.
Default metrics sink is OpenTelemetry.
properties:
openTelemetry:
description: OpenTelemetry defines the configuration
Expand Down Expand Up @@ -5489,6 +5509,11 @@ spec:
required:
- type
type: object
x-kubernetes-validations:
- message: If MetricSink type is OpenTelemetry, openTelemetry
field needs to be set.
rule: 'self.type == ''OpenTelemetry'' ? has(self.openTelemetry)
: !has(self.openTelemetry)'
type: array
type: object
tracing:
Expand Down
4 changes: 2 additions & 2 deletions site/content/en/latest/api/extension_types.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,7 @@ _Appears in:_




ProxyAccessLogSink defines the sink of accesslog.

_Appears in:_
- [ProxyAccessLogSetting](#proxyaccesslogsetting)
Expand Down Expand Up @@ -1227,7 +1227,7 @@ _Appears in:_




ProxyMetricSink defines the sink of metrics. Default metrics sink is OpenTelemetry.

_Appears in:_
- [ProxyMetrics](#proxymetrics)
Expand Down
223 changes: 219 additions & 4 deletions test/cel-validation/envoyproxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,8 @@ func TestEnvoyProxyProvider(t *testing.T) {
wantErrors []string
}{
{
desc: "nil provider",
mutate: func(envoy *egv1a1.EnvoyProxy) {

},
desc: "nil provider",
mutate: func(envoy *egv1a1.EnvoyProxy) {},
wantErrors: []string{},
},
{
Expand Down Expand Up @@ -204,6 +202,223 @@ func TestEnvoyProxyProvider(t *testing.T) {
},
wantErrors: []string{"loadBalancerIP can only be set for LoadBalancer type"},
},
{
desc: "invalid-ProxyAccessLogFormat",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
AccessLog: &egv1a1.ProxyAccessLog{
Settings: []egv1a1.ProxyAccessLogSetting{
{
Format: egv1a1.ProxyAccessLogFormat{
Type: "foo",
},
},
},
},
},
}
},
wantErrors: []string{"Unsupported value: \"foo\": supported values: \"Text\", \"JSON\""},
},
{
desc: "ProxyAccessLogFormat-with-TypeText-but-no-text",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
AccessLog: &egv1a1.ProxyAccessLog{
Settings: []egv1a1.ProxyAccessLogSetting{
{
Format: egv1a1.ProxyAccessLogFormat{
Type: egv1a1.ProxyAccessLogFormatTypeText,
},
Sinks: []egv1a1.ProxyAccessLogSink{
{
Type: egv1a1.ProxyAccessLogSinkTypeFile,
File: &egv1a1.FileEnvoyProxyAccessLog{
Path: "foo/bar",
},
},
},
},
},
},
},
}
},
wantErrors: []string{"If AccessLogFormat type is Text, text field needs to be set"},
},
{
desc: "ProxyAccessLogFormat-with-TypeJSON-but-no-json",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
AccessLog: &egv1a1.ProxyAccessLog{
Settings: []egv1a1.ProxyAccessLogSetting{
{
Format: egv1a1.ProxyAccessLogFormat{
Type: egv1a1.ProxyAccessLogFormatTypeJSON,
},
Sinks: []egv1a1.ProxyAccessLogSink{
{
Type: egv1a1.ProxyAccessLogSinkTypeFile,
File: &egv1a1.FileEnvoyProxyAccessLog{
Path: "foo/bar",
},
},
},
},
},
},
},
}
},
wantErrors: []string{"If AccessLogFormat type is JSON, json field needs to be set"},
},
{
desc: "ProxyAccessLogFormat-with-TypeJSON-but-got-text",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
AccessLog: &egv1a1.ProxyAccessLog{
Settings: []egv1a1.ProxyAccessLogSetting{
{
Format: egv1a1.ProxyAccessLogFormat{
Type: egv1a1.ProxyAccessLogFormatTypeJSON,
Text: ptr.To("[%START_TIME%]"),
},
Sinks: []egv1a1.ProxyAccessLogSink{
{
Type: egv1a1.ProxyAccessLogSinkTypeFile,
File: &egv1a1.FileEnvoyProxyAccessLog{
Path: "foo/bar",
},
},
},
},
},
},
},
}
},
wantErrors: []string{"If AccessLogFormat type is JSON, json field needs to be set"},
},
{
desc: "ProxyAccessLogSink-with-TypeFile-but-no-file",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
AccessLog: &egv1a1.ProxyAccessLog{
Settings: []egv1a1.ProxyAccessLogSetting{
{
Format: egv1a1.ProxyAccessLogFormat{
Type: egv1a1.ProxyAccessLogFormatTypeText,
Text: ptr.To("[%START_TIME%]"),
},
Sinks: []egv1a1.ProxyAccessLogSink{
{
Type: egv1a1.ProxyAccessLogSinkTypeFile,
},
},
},
},
},
},
}
},
wantErrors: []string{"If AccessLogSink type is File, file field needs to be set"},
},
{
desc: "ProxyAccessLogSink-with-TypeOpenTelemetry-but-no-openTelemetry",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
AccessLog: &egv1a1.ProxyAccessLog{
Settings: []egv1a1.ProxyAccessLogSetting{
{
Format: egv1a1.ProxyAccessLogFormat{
Type: egv1a1.ProxyAccessLogFormatTypeText,
Text: ptr.To("[%START_TIME%]"),
},
Sinks: []egv1a1.ProxyAccessLogSink{
{
Type: egv1a1.ProxyAccessLogSinkTypeOpenTelemetry,
},
},
},
},
},
},
}
},
wantErrors: []string{"If AccessLogSink type is OpenTelemetry, openTelemetry field needs to be set"},
},
{
desc: "ProxyAccessLog-settings-pass",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
AccessLog: &egv1a1.ProxyAccessLog{
Settings: []egv1a1.ProxyAccessLogSetting{
{
Format: egv1a1.ProxyAccessLogFormat{
Type: egv1a1.ProxyAccessLogFormatTypeText,
Text: ptr.To("[%START_TIME%]"),
},
Sinks: []egv1a1.ProxyAccessLogSink{
{
Type: egv1a1.ProxyAccessLogSinkTypeFile,
File: &egv1a1.FileEnvoyProxyAccessLog{
Path: "foo/bar",
},
},
},
},
},
},
},
}
},
wantErrors: []string{},
},
{
desc: "ProxyMetricSink-with-TypeOpenTelemetry-but-no-openTelemetry",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
Metrics: &egv1a1.ProxyMetrics{
Sinks: []egv1a1.ProxyMetricSink{
{
Type: egv1a1.MetricSinkTypeOpenTelemetry,
},
},
},
},
}
},
wantErrors: []string{"If MetricSink type is OpenTelemetry, openTelemetry field needs to be set"},
},
{
desc: "ProxyMetrics-sinks-pass",
mutate: func(envoy *egv1a1.EnvoyProxy) {
envoy.Spec = egv1a1.EnvoyProxySpec{
Telemetry: &egv1a1.ProxyTelemetry{
Metrics: &egv1a1.ProxyMetrics{
Sinks: []egv1a1.ProxyMetricSink{
{
Type: egv1a1.MetricSinkTypeOpenTelemetry,
OpenTelemetry: &egv1a1.ProxyOpenTelemetrySink{
Host: "0.0.0.0",
Port: 3217,
},
},
},
},
},
}
},
wantErrors: []string{},
},
}

for _, tc := range cases {
Expand Down

0 comments on commit 7835e2f

Please sign in to comment.