diff --git a/api/core/v1alpha2/proxy_types.go b/api/core/v1alpha2/proxy_types.go index 7d69a939..4e600a78 100644 --- a/api/core/v1alpha2/proxy_types.go +++ b/api/core/v1alpha2/proxy_types.go @@ -356,10 +356,6 @@ func getProxyTelemetryInfo(proxy *Proxy) string { features = append(features, "Tracing") } - if proxy.Spec.Telemetry.ThirdPartySinks != nil { - features = append(features, "3rdParty") - } - if len(features) == 0 { return "Default" } diff --git a/api/core/v1alpha2/proxy_types_table_test.go b/api/core/v1alpha2/proxy_types_table_test.go index efeb22cc..98949f52 100644 --- a/api/core/v1alpha2/proxy_types_table_test.go +++ b/api/core/v1alpha2/proxy_types_table_test.go @@ -353,17 +353,7 @@ func TestGetProxyTelemetryInfo(t *testing.T) { }}, expected: "[Tracing]", }, - { - name: "third party sinks", - proxy: &Proxy{Spec: ProxySpec{ - Telemetry: &ProxyTelementry{ - ThirdPartySinks: &ThirdPartySinks{ - DatadogLogs: &APIKey{Key: "key"}, - }, - }, - }}, - expected: "[3rdParty]", - }, + { name: "multiple features", proxy: &Proxy{Spec: ProxySpec{ @@ -374,12 +364,9 @@ func TestGetProxyTelemetryInfo(t *testing.T) { Tracing: &ProxyTracing{ Enabled: true, }, - ThirdPartySinks: &ThirdPartySinks{ - AxiomLogs: &APIKey{Key: "key"}, - }, }, }}, - expected: "[AccessLogs Tracing 3rdParty]", + expected: "[AccessLogs Tracing]", }, } diff --git a/api/core/v1alpha2/proxy_validate.go b/api/core/v1alpha2/proxy_validate.go index f968cd7b..20b01ffd 100644 --- a/api/core/v1alpha2/proxy_validate.go +++ b/api/core/v1alpha2/proxy_validate.go @@ -27,6 +27,11 @@ func (r *Proxy) Default() { var _ resourcestrategy.Validater = &Proxy{} var _ resourcestrategy.ValidateUpdater = &Proxy{} +// isCloudProvider returns true if the proxy uses the cloud infrastructure provider. +func isCloudProvider(p InfraProvider) bool { + return p == InfraProviderCloud || p == "" +} + func (r *Proxy) validate() field.ErrorList { errs := field.ErrorList{} spec := r.Spec @@ -38,6 +43,27 @@ func (r *Proxy) validate() field.ErrorList { "minimumDrainTime must be less than or equal to drainTimeout")) } + // Telemetry settings are managed by CloudMonitoringIntegration for cloud proxies. + if isCloudProvider(spec.Provider) && spec.Telemetry != nil { + telPath := field.NewPath("spec", "telemetry") + msg := "telemetry settings are not configurable for cloud proxies; use CloudMonitoringIntegration instead" + if spec.Telemetry.AccessLogs != nil { + errs = append(errs, field.Forbidden(telPath.Child("accessLogs"), msg)) + } + if spec.Telemetry.ContentLogs != nil { + errs = append(errs, field.Forbidden(telPath.Child("contentLogs"), msg)) + } + if spec.Telemetry.Tracing != nil { + errs = append(errs, field.Forbidden(telPath.Child("tracing"), msg)) + } + if spec.Telemetry.OtelCollectorConfig != nil { + errs = append(errs, field.Forbidden(telPath.Child("otelCollectorConfig"), msg)) + } + if spec.Telemetry.ThirdPartySinks != nil { + errs = append(errs, field.Forbidden(telPath.Child("thirdPartySinks"), msg)) + } + } + return errs } diff --git a/api/core/v1alpha2/proxy_validate_test.go b/api/core/v1alpha2/proxy_validate_test.go new file mode 100644 index 00000000..05b0f0c5 --- /dev/null +++ b/api/core/v1alpha2/proxy_validate_test.go @@ -0,0 +1,183 @@ +package v1alpha2 + +import ( + "context" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func defaultedProxy(provider InfraProvider, tel *ProxyTelementry) *Proxy { + p := &Proxy{ + Spec: ProxySpec{ + Provider: provider, + Telemetry: tel, + }, + } + p.Default() + return p +} + +func TestValidate_CloudTelemetryRejected(t *testing.T) { + msg := "telemetry settings are not configurable for cloud proxies; use CloudMonitoringIntegration instead" + + tests := []struct { + name string + provider InfraProvider + telemetry *ProxyTelementry + wantErrs int + wantField string + }{ + { + name: "cloud provider with nil telemetry is valid", + provider: InfraProviderCloud, + wantErrs: 0, + }, + { + name: "empty provider (defaults to cloud) with nil telemetry is valid", + provider: "", + wantErrs: 0, + }, + { + name: "cloud provider with empty telemetry is valid", + provider: InfraProviderCloud, + telemetry: &ProxyTelementry{}, + wantErrs: 0, + }, + { + name: "cloud provider with accessLogs rejected", + provider: InfraProviderCloud, + telemetry: &ProxyTelementry{ + AccessLogs: &ProxyAccessLogs{ + JSON: map[string]string{"key": "value"}, + }, + }, + wantErrs: 1, + wantField: "spec.telemetry.accessLogs", + }, + { + name: "cloud provider with contentLogs rejected", + provider: InfraProviderCloud, + telemetry: &ProxyTelementry{ + ContentLogs: &ProxyContentLogs{RequestBodyEnabled: true}, + }, + wantErrs: 1, + wantField: "spec.telemetry.contentLogs", + }, + { + name: "cloud provider with tracing rejected", + provider: InfraProviderCloud, + telemetry: &ProxyTelementry{ + Tracing: &ProxyTracing{Enabled: true}, + }, + wantErrs: 1, + wantField: "spec.telemetry.tracing", + }, + { + name: "cloud provider with otelCollectorConfig rejected", + provider: InfraProviderCloud, + telemetry: &ProxyTelementry{ + OtelCollectorConfig: &LocalObjectReference{Name: "cfg"}, + }, + wantErrs: 1, + wantField: "spec.telemetry.otelCollectorConfig", + }, + { + name: "cloud provider with thirdPartySinks rejected", + provider: InfraProviderCloud, + telemetry: &ProxyTelementry{ + ThirdPartySinks: &ThirdPartySinks{ + DatadogLogs: &APIKey{Key: "key"}, + }, + }, + wantErrs: 1, + wantField: "spec.telemetry.thirdPartySinks", + }, + { + name: "cloud provider with multiple telemetry fields rejected", + provider: "", + telemetry: &ProxyTelementry{ + AccessLogs: &ProxyAccessLogs{JSON: map[string]string{"k": "v"}}, + Tracing: &ProxyTracing{Enabled: true}, + ContentLogs: &ProxyContentLogs{RequestBodyEnabled: true}, + }, + wantErrs: 3, + }, + { + name: "kubernetes provider with telemetry is valid", + provider: InfraProviderKubernetes, + telemetry: &ProxyTelementry{ + Tracing: &ProxyTracing{Enabled: true}, + }, + wantErrs: 0, + }, + { + name: "unmanaged provider with telemetry is valid", + provider: InfraProviderUnmanaged, + telemetry: &ProxyTelementry{ + AccessLogs: &ProxyAccessLogs{JSON: map[string]string{"k": "v"}}, + OtelCollectorConfig: &LocalObjectReference{Name: "cfg"}, + }, + wantErrs: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := defaultedProxy(tt.provider, tt.telemetry) + errs := p.Validate(context.Background()) + if len(errs) != tt.wantErrs { + t.Errorf("Validate() returned %d errors, want %d: %v", len(errs), tt.wantErrs, errs) + } + if tt.wantField != "" && len(errs) > 0 { + if errs[0].Field != tt.wantField { + t.Errorf("Validate() error field = %q, want %q", errs[0].Field, tt.wantField) + } + } + if tt.wantErrs > 0 { + for _, e := range errs { + if e.Detail != msg { + t.Errorf("Validate() error detail = %q, want %q", e.Detail, msg) + } + } + } + }) + } +} + +func TestValidateUpdate_CloudTelemetryRejected(t *testing.T) { + old := defaultedProxy(InfraProviderCloud, nil) + updated := defaultedProxy(InfraProviderCloud, &ProxyTelementry{ + Tracing: &ProxyTracing{Enabled: true}, + }) + + errs := old.ValidateUpdate(context.Background(), updated) + if len(errs) != 1 { + t.Fatalf("ValidateUpdate() returned %d errors, want 1: %v", len(errs), errs) + } + if errs[0].Field != "spec.telemetry.tracing" { + t.Errorf("ValidateUpdate() error field = %q, want %q", errs[0].Field, "spec.telemetry.tracing") + } +} + +func TestValidate_DrainTimeout(t *testing.T) { + p := &Proxy{ + Spec: ProxySpec{ + Provider: InfraProviderKubernetes, + Shutdown: &ShutdownConfig{ + DrainTimeout: &metav1.Duration{Duration: 10 * time.Second}, + MinimumDrainTime: &metav1.Duration{Duration: 30 * time.Second}, + }, + }, + } + p.Default() + + errs := p.Validate(context.Background()) + if len(errs) != 1 { + t.Fatalf("Validate() returned %d errors, want 1: %v", len(errs), errs) + } + if errs[0].Field != "spec.shutdown.minimumDrainTime" { + t.Errorf("Validate() error field = %q, want %q", errs[0].Field, "spec.shutdown.minimumDrainTime") + } +}