Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add CEL validation for EnvoyProxy telemetry #2050

Merged
merged 5 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/v1alpha1/accesslogging_types.go
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
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
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
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
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
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