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

Ability to strip headers for AWS Proxy #7596

Closed
sukrit007wawa opened this issue Feb 7, 2022 · 10 comments · Fixed by #16818
Closed

Ability to strip headers for AWS Proxy #7596

sukrit007wawa opened this issue Feb 7, 2022 · 10 comments · Fixed by #16818
Labels
comp:aws AWS components

Comments

@sukrit007wawa
Copy link

Is your feature request related to a problem? Please describe.
If we deploy a istio (envoy) proxy in front of the collector, the signing process fails because proxy sets x-forwarded-proto to 'http' causing a mismatch in the signing process.

Similar to aws-sig4-proxy, we need the ability to strip out or exclude headers from signing. See https://github.com/awslabs/aws-sigv4-proxy#examples

Describe the solution you'd like
Ability to pass an environment variable/configuration to exclude headers that would include headers that needs to be removed when using awsproxy. The default value of this configuration can include headers like x-forwarded-port,x-forwarded-proto

Describe alternatives you've considered
We are considering suppressing this header using envoy filters.

Additional context
To reproduce this issue, you can simply exec inside otel-collector, and run the curl command

curl -X POST \
-H 'x-forwarded-proto: http' \
http://localhost:2000/GetSamplingRules
@jpkrohling
Copy link
Member

cc @anuraaga @Aneurysm9 @mxiamxia

@alolita alolita added the comp:aws AWS components label Feb 8, 2022
@sethAmazon
Copy link
Contributor

Can you please put your collector config and logs

@sukrit007wawa
Copy link
Author

sukrit007wawa commented Feb 11, 2022

curl -X POST
-H 'x-forwarded-proto: http'
http://localhost:2000/GetSamplingRules

Unfortunately , there are no errors in collector logs. We have set this using Istio, however, to isolate the issue you can run ADOT Collector (0.16.0), and run simple curl command:
curl -X POST \ -H 'x-forwarded-proto: http' \ http://localhost:2000/GetSamplingRules

if you remove 'x-forwarded-proto', it works. The idea being, if you add a proxy in front of the collector, it will add x-forwarded-proto to the header. Since original request was made over 'http' , the value of 'x-forwarded-proto' will be http. This causes signing in awsproxy to break, and we get errors on client-side.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2022

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Nov 8, 2022
@rapphil
Copy link
Contributor

rapphil commented Nov 17, 2022

This is still relevant

@jpkrohling
Copy link
Member

@kovrus, do you think this would be suitable for the header setter?

@kovrus
Copy link
Member

kovrus commented Nov 23, 2022

@jpkrohling It could be a good place to implement that, we can change the header setter config to look like the following:

extensions:
  headers_setter:
    headers:
      - key: <header>
        action: insert
        from_context: ...  
      - key: <header>
        action: insert
        value: ...
      - key: <header>
        action: drop    

what do you think? The name of the extension wouldn't reflect what it does though

@jpkrohling
Copy link
Member

I wouldn't be against another rename, given that it's an organic evolution of the component. Consider also the upsert and update operations, in addition to insert and drop.

@kovrus
Copy link
Member

kovrus commented Nov 24, 2022

I'll open an issue for the proposed change.

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Jan 24, 2023
@jpkrohling jpkrohling removed the Stale label Jan 25, 2023
ZahidMirza95 pushed a commit to ZahidMirza95/opentelemetry-collector-contrib that referenced this issue Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:aws AWS components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants