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

Setting otlp/http CORS allowed_headers replaces the default safelist #6513

Open
ahayworth opened this issue Nov 9, 2022 · 2 comments
Open
Labels
bug Something isn't working Stale

Comments

@ahayworth
Copy link

Describe the bug
The documentation around CORS allowed_headers for the otlpreceiver and confighttp are unintentionally misleading. The language implies that headers listed here will be allowed in addition to the default safelist; but the actual behavior is to replace the default safelist (except for Origin).

You can see this behavior in the upstream CORS code that we rely on.

Steps to reproduce

  1. Configure OTLP/HTTP+JSON, and run a test application that uses opentelemetry-js to send metrics to it.
  2. Try to set the allowed_headers to anything you'd like.
  3. Observe that CORS preflight requests (and consequently, metrics export requests) will fail.

What did you expect to see?
I expected to see requests succeeding. 😄

What did you see instead?
I saw all requests failing. 😓

What version did you use?
Version: v0.61.0, but this is not necessarily version-dependent

What config did you use?
Config:

(NB: the astute observer will note that we didn't need to set this option. We ... are now also aware of this fact 😄 )

  receivers:
    otlp:
      protocols:
        grpc:
          include_metadata: true
        http:
          endpoint:
          include_metadata: true
          cors:
            allowed_origins:
              - http://*
            allowed_headers:
              - X-Geo-Country-ISO-Code

  processors:
    batch:
    memory_limiter:
      limit_mib: 3072
      spike_limit_mib: 512
      check_interval: 5s
    resource/geo:
      attributes:
        - key: geo.country_iso_code
          from_context: metadata.X-Geo-Country-ISO-Code
          action: insert

  exporters:
    logging:
      loglevel: debug
    unreleased_internal_thing_sorry:
      endpoint: 0.0.0.0:9000
      tls:
        insecure: true

  service:
    pipelines:
      metrics:
        receivers: [otlp]
        processors: [memory_limiter, resource/geo, batch]
        exporters: [logging, unreleased_internal_thing_sorry]

Environment
OS: 🤫 I don't think I am allowed to say 🤫
Compiler(if manually compiled): go 1.19

Additional context

I believe this can be addressed in one of two ways:

  1. A simple documentation PR could be enough to clarify the actual behavior of setting the CORS allowed_headers option.
  2. Alternately, a slightly more involved PR could be opened that would always merge in the default "safe" CORS headers, as the documentation currently implies.

I'm happy to make either fix: please just let me know which one the maintainers would prefer.

@ahayworth ahayworth added the bug Something isn't working label Nov 9, 2022
@codeboten
Copy link
Contributor

@ahayworth if you'd like to open a PR to fix the behaviour, I believe #2 is the correct way to proceed

@ahayworth
Copy link
Author

Fantastic, I'll take that approach. I still intend to fix it (likely tonight or tomorrow night), I just haven't been able to squeeze in a few minutes to write any code in the past few days. 😆

@github-actions github-actions bot added the Stale label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Stale
Projects
None yet
Development

No branches or pull requests

2 participants