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

Validate OTLP Endpoint #1106

Closed
chrkl opened this issue May 22, 2024 · 4 comments
Closed

Validate OTLP Endpoint #1106

chrkl opened this issue May 22, 2024 · 4 comments
Assignees
Labels
area/metrics MetricPipeline area/traces TracePipeline kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@chrkl
Copy link
Contributor

chrkl commented May 22, 2024

Description

Recent changes to the OpenTelemetry collector may terminate the Collector ungracefully in the case of a malformed OTLP endpoint.

Expected result

An endpoint value in a MetricPipeline or TracePipeline should never result in a crashing Pod. If the user defines a value that will not be accepted by the Collector, the configuration should not be applied and the problem indicated in the pipeline status.

Ensure that pipeline validation logic is not duplicated in the individual reconcilers

Actual result

When a user defines such an endpoint in a MetricPipeline or TracePipeline, this results in a crashing pod. In older releases of the Collector, it was running but not shipping any data.

Steps to reproduce

Create a MetricPipeline with endpoint that does not have a port. E.g., set the value to /. The exact values that are accepted by the Collector have to be evaluated.

Troubleshooting

Release Notes


Similar but different ticket: #1106

@chrkl chrkl added kind/bug Categorizes issue or PR as related to a bug. area/metrics MetricPipeline area/traces TracePipeline labels May 22, 2024
@chrkl chrkl added this to the 1.17.0 milestone May 22, 2024
@k15r
Copy link
Contributor

k15r commented May 22, 2024

is it ok to handle this directly in the api-server using x-validation?

@chrkl
Copy link
Contributor Author

chrkl commented May 22, 2024

Cloud be a problem for pipelines that already have a forbidden endpoint value. Those could not be reconciled anymore.

@k15r
Copy link
Contributor

k15r commented May 23, 2024

actually quite the opposite:
if we prevent setting invalid values in the CR using x-validation, then existing pipelines will still (not) work as before. If we introduce a change in the reconciliation mechanism, then we have to introduce a new condition on the CR to transport this information to the user.

@chrkl chrkl assigned k15r and unassigned k15r May 24, 2024
@a-thaler a-thaler removed this from the 1.17.0 milestone Jun 3, 2024
@TeodorSAP TeodorSAP self-assigned this Sep 2, 2024
@a-thaler a-thaler added this to the 1.23.0 milestone Sep 6, 2024
@TeodorSAP
Copy link
Member

Endpoint validation improved, with different scenarios for OTLP: HTTP and GRPC protocols and Fluentd: HTTP, port validation, schema validation, etc. UTs and E2E tests were also improved to test as many scenarios as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics MetricPipeline area/traces TracePipeline kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

4 participants