Skip to content

Commit e826fd7

Browse files
rakesh-garimellaskhalash
rakesh-garimella
and
skhalash
authored
fix: dont allow paths in grpc protocol (#3309)
Co-authored-by: Stanislav Khalash <stanislav.khalash@sap.com>
1 parent f238222 commit e826fd7

File tree

6 files changed

+218
-119
lines changed

6 files changed

+218
-119
lines changed

‎internal/validators/endpoint/endpoint_validator.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ var (
3737
ErrUnsupportedScheme = errors.New("missing or unsupported protocol scheme")
3838
ErrGRPCOAuth2NoTLS = errors.New("OAuth2 requires TLS when using gRPC protocol")
3939
ErrHTTPWithTLS = errors.New("HTTP scheme with TLS not allowed")
40+
ErrGRPCWithPath = errors.New("gRPC endpoints cannot contain paths")
4041
)
4142

4243
type EndpointInvalidError struct {
@@ -89,6 +90,13 @@ func (v *Validator) Validate(ctx context.Context, params EndpointValidationParam
8990
}
9091
}
9192

93+
// path validation for gRPC
94+
if params.Protocol == OTLPProtocolGRPC {
95+
if err := validateGRPCPath(u.Path); err != nil {
96+
return err
97+
}
98+
}
99+
92100
// OAuth2 validation
93101
if params.OTLPOAuth2 != nil {
94102
var validationFunc func(string, *telemetryv1beta1.OutputTLS) error
@@ -178,6 +186,15 @@ func validateSchemeHTTP(scheme string) error {
178186
return nil
179187
}
180188

189+
func validateGRPCPath(path string) error {
190+
// OTel Collector's gRPC exporter does not accept any path, including trailing slashes.
191+
if path != "" {
192+
return &EndpointInvalidError{Err: ErrGRPCWithPath}
193+
}
194+
195+
return nil
196+
}
197+
181198
func validateGRPCWithOAuth2(scheme string, tls *telemetryv1beta1.OutputTLS) error {
182199
// Insecure TLS config
183200
if tls != nil && tls.Insecure {

‎internal/validators/endpoint/endpoint_validator_test.go‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const (
2121
errMsgUnsupportedScheme = "missing or unsupported protocol scheme"
2222
errMsgGRPCOAuth2NoTLS = "OAuth2 requires TLS when using gRPC protocol"
2323
errMsgHTTPWithTLS = "HTTP scheme with TLS not allowed"
24+
errMsgGRPCWithPath = "gRPC endpoints cannot contain paths"
2425
)
2526

2627
var testScenarios = []struct {
@@ -36,12 +37,19 @@ var testScenarios = []struct {
3637
errFluentdHTTP error
3738
errMsgFluentdHTTP string
3839
}{
40+
{
41+
name: "with scheme: endpoint with trailing slash and port",
42+
endpoint: "https://foo.bar:4317/",
43+
44+
errOTLPGRPC: ErrGRPCWithPath,
45+
errMsgOTLPGRPC: errMsgGRPCWithPath,
46+
},
3947
{
4048
name: "with scheme: valid endpoint with path and port",
4149
endpoint: "https://foo.bar:4317/foo/bar",
4250

43-
errOTLPGRPC: nil,
44-
errMsgOTLPGRPC: "",
51+
errOTLPGRPC: ErrGRPCWithPath,
52+
errMsgOTLPGRPC: errMsgGRPCWithPath,
4553

4654
errOTLPHTTP: nil,
4755
errMsgOTLPHTTP: "",
@@ -53,8 +61,8 @@ var testScenarios = []struct {
5361
name: "with scheme: valid endpoint with path, query params, and port",
5462
endpoint: "https://foo.bar:4317/ingest?token=abc&query=param",
5563

56-
errOTLPGRPC: nil,
57-
errMsgOTLPGRPC: "",
64+
errOTLPGRPC: ErrGRPCWithPath,
65+
errMsgOTLPGRPC: errMsgGRPCWithPath,
5866

5967
errOTLPHTTP: nil,
6068
errMsgOTLPHTTP: "",
@@ -66,8 +74,8 @@ var testScenarios = []struct {
6674
name: "with IPv4: valid IPv4 endpoint with path and port",
6775
endpoint: "https://10.108.183.198:4317/foo/bar",
6876

69-
errOTLPGRPC: nil,
70-
errMsgOTLPGRPC: "",
77+
errOTLPGRPC: ErrGRPCWithPath,
78+
errMsgOTLPGRPC: errMsgGRPCWithPath,
7179

7280
errOTLPHTTP: nil,
7381
errMsgOTLPHTTP: "",
@@ -79,8 +87,8 @@ var testScenarios = []struct {
7987
name: "with IPv6: valid IPv6 endpoint with path and port",
8088
endpoint: "https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:4317/foo/bar",
8189

82-
errOTLPGRPC: nil,
83-
errMsgOTLPGRPC: "",
90+
errOTLPGRPC: ErrGRPCWithPath,
91+
errMsgOTLPGRPC: errMsgGRPCWithPath,
8492

8593
errOTLPHTTP: nil,
8694
errMsgOTLPHTTP: "",

‎test/e2e/logs/shared/single_pipeline_test.go‎

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -71,42 +71,64 @@ func TestSinglePipeline_OTel(t *testing.T) {
7171
},
7272
}
7373

74-
for _, tc := range tests {
75-
t.Run(tc.name, func(t *testing.T) {
76-
suite.SetupTestWithOptions(t, tc.labels, tc.opts...)
77-
78-
var (
79-
uniquePrefix = unique.Prefix(tc.name)
80-
pipelineName = uniquePrefix()
81-
backendNs = uniquePrefix("backend")
82-
genNs = uniquePrefix("gen")
83-
)
84-
85-
backend := kitbackend.New(backendNs, kitbackend.SignalTypeLogsOTel)
86-
87-
pipeline := testutils.NewLogPipelineBuilder().
88-
WithName(pipelineName).
89-
WithInput(tc.inputBuilder(genNs)).
90-
WithOTLPOutput(testutils.OTLPEndpoint(backend.EndpointHTTP())).
91-
Build()
92-
93-
resources := []client.Object{
94-
kitk8sobjects.NewNamespace(backendNs).K8sObject(),
95-
kitk8sobjects.NewNamespace(genNs).K8sObject(),
96-
&pipeline,
97-
tc.logGeneratorBuilder(genNs),
98-
}
99-
resources = append(resources, backend.K8sObjects()...)
100-
101-
Expect(kitk8s.CreateObjects(t, resources...)).To(Succeed())
102-
103-
assert.BackendReachable(t, backend)
104-
105-
tc.readinessCheckFunc(t, tc.resourceName)
74+
protocols := []struct {
75+
name string
76+
protocol telemetryv1beta1.OTLPProtocol
77+
endpoint func(backend *kitbackend.Backend) string
78+
}{
79+
{
80+
name: "grpc",
81+
protocol: "grpc",
82+
endpoint: func(b *kitbackend.Backend) string { return b.EndpointHTTP() },
83+
},
84+
{
85+
name: "http",
86+
protocol: "http",
87+
endpoint: func(b *kitbackend.Backend) string { return b.EndpointOTLPHTTP() },
88+
},
89+
}
10690

107-
assert.OTelLogPipelineHealthy(t, pipelineName)
108-
assert.OTelLogsFromNamespaceDelivered(t, backend, genNs)
109-
})
91+
for _, tc := range tests {
92+
for _, proto := range protocols {
93+
t.Run(tc.name+"/"+proto.name, func(t *testing.T) {
94+
suite.SetupTestWithOptions(t, tc.labels, tc.opts...)
95+
96+
var (
97+
uniquePrefix = unique.Prefix(tc.name, proto.name)
98+
pipelineName = uniquePrefix()
99+
backendNs = uniquePrefix("backend")
100+
genNs = uniquePrefix("gen")
101+
)
102+
103+
backend := kitbackend.New(backendNs, kitbackend.SignalTypeLogsOTel)
104+
105+
pipeline := testutils.NewLogPipelineBuilder().
106+
WithName(pipelineName).
107+
WithInput(tc.inputBuilder(genNs)).
108+
WithOTLPOutput(
109+
testutils.OTLPEndpoint(proto.endpoint(backend)),
110+
testutils.OTLPProtocol(proto.protocol),
111+
).
112+
Build()
113+
114+
resources := []client.Object{
115+
kitk8sobjects.NewNamespace(backendNs).K8sObject(),
116+
kitk8sobjects.NewNamespace(genNs).K8sObject(),
117+
&pipeline,
118+
tc.logGeneratorBuilder(genNs),
119+
}
120+
resources = append(resources, backend.K8sObjects()...)
121+
122+
Expect(kitk8s.CreateObjects(t, resources...)).To(Succeed())
123+
124+
assert.BackendReachable(t, backend)
125+
126+
tc.readinessCheckFunc(t, tc.resourceName)
127+
128+
assert.OTelLogPipelineHealthy(t, pipelineName)
129+
assert.OTelLogsFromNamespaceDelivered(t, backend, genNs)
130+
})
131+
}
110132
}
111133
}
112134

‎test/e2e/metrics/shared/single_pipeline_test.go‎

Lines changed: 70 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -57,54 +57,76 @@ func TestSinglePipeline(t *testing.T) {
5757
},
5858
}
5959

60+
protocols := []struct {
61+
name string
62+
protocol telemetryv1beta1.OTLPProtocol
63+
endpoint func(backend *kitbackend.Backend) string
64+
}{
65+
{
66+
name: "grpc",
67+
protocol: "grpc",
68+
endpoint: func(b *kitbackend.Backend) string { return b.EndpointHTTP() },
69+
},
70+
{
71+
name: "http",
72+
protocol: "http",
73+
endpoint: func(b *kitbackend.Backend) string { return b.EndpointOTLPHTTP() },
74+
},
75+
}
76+
6077
for _, tc := range tests {
61-
t.Run(tc.name, func(t *testing.T) {
62-
suite.SetupTest(t, tc.labels...)
63-
64-
var (
65-
uniquePrefix = unique.Prefix(tc.name)
66-
pipelineName = uniquePrefix()
67-
backendNs = uniquePrefix("backend")
68-
genNs = uniquePrefix("gen")
69-
)
70-
71-
backend := kitbackend.New(backendNs, kitbackend.SignalTypeMetrics)
72-
pipeline := testutils.NewMetricPipelineBuilder().
73-
WithName(pipelineName).
74-
WithInput(tc.inputBuilder(genNs)).
75-
WithOTLPOutput(testutils.OTLPEndpoint(backend.EndpointHTTP())).
76-
Build()
77-
78-
resources := []client.Object{
79-
kitk8sobjects.NewNamespace(backendNs).K8sObject(),
80-
kitk8sobjects.NewNamespace(genNs).K8sObject(),
81-
&pipeline,
82-
}
83-
resources = append(resources, tc.generatorBuilder(genNs)...)
84-
resources = append(resources, backend.K8sObjects()...)
85-
86-
Expect(kitk8s.CreateObjects(t, resources...)).To(Succeed())
87-
88-
assert.BackendReachable(t, backend)
89-
assert.DeploymentReady(t, kitkyma.MetricGatewayName)
90-
91-
if suite.ExpectAgent(tc.labels...) {
92-
assert.DaemonSetReady(t, kitkyma.MetricAgentName)
93-
}
94-
95-
assert.MetricPipelineHealthy(t, pipelineName)
96-
97-
if suite.ExpectAgent(tc.labels...) {
98-
assert.MetricsFromNamespaceDelivered(t, backend, genNs, runtime.DefaultMetricsNames)
99-
100-
agentMetricsURL := suite.ProxyClient.ProxyURLForService(kitkyma.MetricAgentMetricsService.Namespace, kitkyma.MetricAgentMetricsService.Name, "metrics", ports.Metrics)
101-
assert.EmitsOTelCollectorMetrics(t, agentMetricsURL)
102-
} else {
103-
assert.MetricsFromNamespaceDelivered(t, backend, genNs, telemetrygen.MetricNames)
104-
105-
gatewayMetricsURL := suite.ProxyClient.ProxyURLForService(kitkyma.MetricGatewayMetricsService.Namespace, kitkyma.MetricGatewayMetricsService.Name, "metrics", ports.Metrics)
106-
assert.EmitsOTelCollectorMetrics(t, gatewayMetricsURL)
107-
}
108-
})
78+
for _, proto := range protocols {
79+
t.Run(tc.name+"/"+proto.name, func(t *testing.T) {
80+
suite.SetupTest(t, tc.labels...)
81+
82+
var (
83+
uniquePrefix = unique.Prefix(tc.name, proto.name)
84+
pipelineName = uniquePrefix()
85+
backendNs = uniquePrefix("backend")
86+
genNs = uniquePrefix("gen")
87+
)
88+
89+
backend := kitbackend.New(backendNs, kitbackend.SignalTypeMetrics)
90+
pipeline := testutils.NewMetricPipelineBuilder().
91+
WithName(pipelineName).
92+
WithInput(tc.inputBuilder(genNs)).
93+
WithOTLPOutput(
94+
testutils.OTLPEndpoint(proto.endpoint(backend)),
95+
testutils.OTLPProtocol(proto.protocol),
96+
).
97+
Build()
98+
99+
resources := []client.Object{
100+
kitk8sobjects.NewNamespace(backendNs).K8sObject(),
101+
kitk8sobjects.NewNamespace(genNs).K8sObject(),
102+
&pipeline,
103+
}
104+
resources = append(resources, tc.generatorBuilder(genNs)...)
105+
resources = append(resources, backend.K8sObjects()...)
106+
107+
Expect(kitk8s.CreateObjects(t, resources...)).To(Succeed())
108+
109+
assert.BackendReachable(t, backend)
110+
assert.DeploymentReady(t, kitkyma.MetricGatewayName)
111+
112+
if suite.ExpectAgent(tc.labels...) {
113+
assert.DaemonSetReady(t, kitkyma.MetricAgentName)
114+
}
115+
116+
assert.MetricPipelineHealthy(t, pipelineName)
117+
118+
if suite.ExpectAgent(tc.labels...) {
119+
assert.MetricsFromNamespaceDelivered(t, backend, genNs, runtime.DefaultMetricsNames)
120+
121+
agentMetricsURL := suite.ProxyClient.ProxyURLForService(kitkyma.MetricAgentMetricsService.Namespace, kitkyma.MetricAgentMetricsService.Name, "metrics", ports.Metrics)
122+
assert.EmitsOTelCollectorMetrics(t, agentMetricsURL)
123+
} else {
124+
assert.MetricsFromNamespaceDelivered(t, backend, genNs, telemetrygen.MetricNames)
125+
126+
gatewayMetricsURL := suite.ProxyClient.ProxyURLForService(kitkyma.MetricGatewayMetricsService.Namespace, kitkyma.MetricGatewayMetricsService.Name, "metrics", ports.Metrics)
127+
assert.EmitsOTelCollectorMetrics(t, gatewayMetricsURL)
128+
}
129+
})
130+
}
109131
}
110132
}

0 commit comments

Comments
 (0)