Skip to content

Commit d82a5cd

Browse files
theRealWardodilyevsky
theRealWardo
and
dilyevsky
authored
[api] lock down proxy object monitoring changes for cloud infra (#85)
Co-authored-by: dilyevsky <ilyevsky@gmail.com>
1 parent 560e629 commit d82a5cd

File tree

4 files changed

+211
-19
lines changed

4 files changed

+211
-19
lines changed

‎api/core/v1alpha2/proxy_types.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -356,10 +356,6 @@ func getProxyTelemetryInfo(proxy *Proxy) string {
356356
features = append(features, "Tracing")
357357
}
358358

359-
if proxy.Spec.Telemetry.ThirdPartySinks != nil {
360-
features = append(features, "3rdParty")
361-
}
362-
363359
if len(features) == 0 {
364360
return "Default"
365361
}

‎api/core/v1alpha2/proxy_types_table_test.go‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -353,17 +353,7 @@ func TestGetProxyTelemetryInfo(t *testing.T) {
353353
}},
354354
expected: "[Tracing]",
355355
},
356-
{
357-
name: "third party sinks",
358-
proxy: &Proxy{Spec: ProxySpec{
359-
Telemetry: &ProxyTelementry{
360-
ThirdPartySinks: &ThirdPartySinks{
361-
DatadogLogs: &APIKey{Key: "key"},
362-
},
363-
},
364-
}},
365-
expected: "[3rdParty]",
366-
},
356+
367357
{
368358
name: "multiple features",
369359
proxy: &Proxy{Spec: ProxySpec{
@@ -374,12 +364,9 @@ func TestGetProxyTelemetryInfo(t *testing.T) {
374364
Tracing: &ProxyTracing{
375365
Enabled: true,
376366
},
377-
ThirdPartySinks: &ThirdPartySinks{
378-
AxiomLogs: &APIKey{Key: "key"},
379-
},
380367
},
381368
}},
382-
expected: "[AccessLogs Tracing 3rdParty]",
369+
expected: "[AccessLogs Tracing]",
383370
},
384371
}
385372

‎api/core/v1alpha2/proxy_validate.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ func (r *Proxy) Default() {
2727
var _ resourcestrategy.Validater = &Proxy{}
2828
var _ resourcestrategy.ValidateUpdater = &Proxy{}
2929

30+
// isCloudProvider returns true if the proxy uses the cloud infrastructure provider.
31+
func isCloudProvider(p InfraProvider) bool {
32+
return p == InfraProviderCloud || p == ""
33+
}
34+
3035
func (r *Proxy) validate() field.ErrorList {
3136
errs := field.ErrorList{}
3237
spec := r.Spec
@@ -38,6 +43,27 @@ func (r *Proxy) validate() field.ErrorList {
3843
"minimumDrainTime must be less than or equal to drainTimeout"))
3944
}
4045

46+
// Telemetry settings are managed by CloudMonitoringIntegration for cloud proxies.
47+
if isCloudProvider(spec.Provider) && spec.Telemetry != nil {
48+
telPath := field.NewPath("spec", "telemetry")
49+
msg := "telemetry settings are not configurable for cloud proxies; use CloudMonitoringIntegration instead"
50+
if spec.Telemetry.AccessLogs != nil {
51+
errs = append(errs, field.Forbidden(telPath.Child("accessLogs"), msg))
52+
}
53+
if spec.Telemetry.ContentLogs != nil {
54+
errs = append(errs, field.Forbidden(telPath.Child("contentLogs"), msg))
55+
}
56+
if spec.Telemetry.Tracing != nil {
57+
errs = append(errs, field.Forbidden(telPath.Child("tracing"), msg))
58+
}
59+
if spec.Telemetry.OtelCollectorConfig != nil {
60+
errs = append(errs, field.Forbidden(telPath.Child("otelCollectorConfig"), msg))
61+
}
62+
if spec.Telemetry.ThirdPartySinks != nil {
63+
errs = append(errs, field.Forbidden(telPath.Child("thirdPartySinks"), msg))
64+
}
65+
}
66+
4167
return errs
4268
}
4369

‎api/core/v1alpha2/proxy_validate_test.go‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
package v1alpha2
2+
3+
import (
4+
"context"
5+
"testing"
6+
"time"
7+
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
)
10+
11+
func defaultedProxy(provider InfraProvider, tel *ProxyTelementry) *Proxy {
12+
p := &Proxy{
13+
Spec: ProxySpec{
14+
Provider: provider,
15+
Telemetry: tel,
16+
},
17+
}
18+
p.Default()
19+
return p
20+
}
21+
22+
func TestValidate_CloudTelemetryRejected(t *testing.T) {
23+
msg := "telemetry settings are not configurable for cloud proxies; use CloudMonitoringIntegration instead"
24+
25+
tests := []struct {
26+
name string
27+
provider InfraProvider
28+
telemetry *ProxyTelementry
29+
wantErrs int
30+
wantField string
31+
}{
32+
{
33+
name: "cloud provider with nil telemetry is valid",
34+
provider: InfraProviderCloud,
35+
wantErrs: 0,
36+
},
37+
{
38+
name: "empty provider (defaults to cloud) with nil telemetry is valid",
39+
provider: "",
40+
wantErrs: 0,
41+
},
42+
{
43+
name: "cloud provider with empty telemetry is valid",
44+
provider: InfraProviderCloud,
45+
telemetry: &ProxyTelementry{},
46+
wantErrs: 0,
47+
},
48+
{
49+
name: "cloud provider with accessLogs rejected",
50+
provider: InfraProviderCloud,
51+
telemetry: &ProxyTelementry{
52+
AccessLogs: &ProxyAccessLogs{
53+
JSON: map[string]string{"key": "value"},
54+
},
55+
},
56+
wantErrs: 1,
57+
wantField: "spec.telemetry.accessLogs",
58+
},
59+
{
60+
name: "cloud provider with contentLogs rejected",
61+
provider: InfraProviderCloud,
62+
telemetry: &ProxyTelementry{
63+
ContentLogs: &ProxyContentLogs{RequestBodyEnabled: true},
64+
},
65+
wantErrs: 1,
66+
wantField: "spec.telemetry.contentLogs",
67+
},
68+
{
69+
name: "cloud provider with tracing rejected",
70+
provider: InfraProviderCloud,
71+
telemetry: &ProxyTelementry{
72+
Tracing: &ProxyTracing{Enabled: true},
73+
},
74+
wantErrs: 1,
75+
wantField: "spec.telemetry.tracing",
76+
},
77+
{
78+
name: "cloud provider with otelCollectorConfig rejected",
79+
provider: InfraProviderCloud,
80+
telemetry: &ProxyTelementry{
81+
OtelCollectorConfig: &LocalObjectReference{Name: "cfg"},
82+
},
83+
wantErrs: 1,
84+
wantField: "spec.telemetry.otelCollectorConfig",
85+
},
86+
{
87+
name: "cloud provider with thirdPartySinks rejected",
88+
provider: InfraProviderCloud,
89+
telemetry: &ProxyTelementry{
90+
ThirdPartySinks: &ThirdPartySinks{
91+
DatadogLogs: &APIKey{Key: "key"},
92+
},
93+
},
94+
wantErrs: 1,
95+
wantField: "spec.telemetry.thirdPartySinks",
96+
},
97+
{
98+
name: "cloud provider with multiple telemetry fields rejected",
99+
provider: "",
100+
telemetry: &ProxyTelementry{
101+
AccessLogs: &ProxyAccessLogs{JSON: map[string]string{"k": "v"}},
102+
Tracing: &ProxyTracing{Enabled: true},
103+
ContentLogs: &ProxyContentLogs{RequestBodyEnabled: true},
104+
},
105+
wantErrs: 3,
106+
},
107+
{
108+
name: "kubernetes provider with telemetry is valid",
109+
provider: InfraProviderKubernetes,
110+
telemetry: &ProxyTelementry{
111+
Tracing: &ProxyTracing{Enabled: true},
112+
},
113+
wantErrs: 0,
114+
},
115+
{
116+
name: "unmanaged provider with telemetry is valid",
117+
provider: InfraProviderUnmanaged,
118+
telemetry: &ProxyTelementry{
119+
AccessLogs: &ProxyAccessLogs{JSON: map[string]string{"k": "v"}},
120+
OtelCollectorConfig: &LocalObjectReference{Name: "cfg"},
121+
},
122+
wantErrs: 0,
123+
},
124+
}
125+
126+
for _, tt := range tests {
127+
t.Run(tt.name, func(t *testing.T) {
128+
p := defaultedProxy(tt.provider, tt.telemetry)
129+
errs := p.Validate(context.Background())
130+
if len(errs) != tt.wantErrs {
131+
t.Errorf("Validate() returned %d errors, want %d: %v", len(errs), tt.wantErrs, errs)
132+
}
133+
if tt.wantField != "" && len(errs) > 0 {
134+
if errs[0].Field != tt.wantField {
135+
t.Errorf("Validate() error field = %q, want %q", errs[0].Field, tt.wantField)
136+
}
137+
}
138+
if tt.wantErrs > 0 {
139+
for _, e := range errs {
140+
if e.Detail != msg {
141+
t.Errorf("Validate() error detail = %q, want %q", e.Detail, msg)
142+
}
143+
}
144+
}
145+
})
146+
}
147+
}
148+
149+
func TestValidateUpdate_CloudTelemetryRejected(t *testing.T) {
150+
old := defaultedProxy(InfraProviderCloud, nil)
151+
updated := defaultedProxy(InfraProviderCloud, &ProxyTelementry{
152+
Tracing: &ProxyTracing{Enabled: true},
153+
})
154+
155+
errs := old.ValidateUpdate(context.Background(), updated)
156+
if len(errs) != 1 {
157+
t.Fatalf("ValidateUpdate() returned %d errors, want 1: %v", len(errs), errs)
158+
}
159+
if errs[0].Field != "spec.telemetry.tracing" {
160+
t.Errorf("ValidateUpdate() error field = %q, want %q", errs[0].Field, "spec.telemetry.tracing")
161+
}
162+
}
163+
164+
func TestValidate_DrainTimeout(t *testing.T) {
165+
p := &Proxy{
166+
Spec: ProxySpec{
167+
Provider: InfraProviderKubernetes,
168+
Shutdown: &ShutdownConfig{
169+
DrainTimeout: &metav1.Duration{Duration: 10 * time.Second},
170+
MinimumDrainTime: &metav1.Duration{Duration: 30 * time.Second},
171+
},
172+
},
173+
}
174+
p.Default()
175+
176+
errs := p.Validate(context.Background())
177+
if len(errs) != 1 {
178+
t.Fatalf("Validate() returned %d errors, want 1: %v", len(errs), errs)
179+
}
180+
if errs[0].Field != "spec.shutdown.minimumDrainTime" {
181+
t.Errorf("Validate() error field = %q, want %q", errs[0].Field, "spec.shutdown.minimumDrainTime")
182+
}
183+
}

0 commit comments

Comments
 (0)