Skip to content

Remove TLS support from firehose receiver#10409

Merged
wu-sheng merged 2 commits intomasterfrom
rm-aws-tls
Feb 19, 2023
Merged

Remove TLS support from firehose receiver#10409
wu-sheng merged 2 commits intomasterfrom
rm-aws-tls

Conversation

@wu-sheng
Copy link
Copy Markdown
Member

Notice, no HTTPS/TLS setup support. By following AWS Firehose request, it uses proxy request (https://... instead of /aws/firehose/metrics), there must be a proxy(Nginx, Envoy, etc.).

  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
  • Update the CHANGES log.

@pg-yang I think we need to update Firehose and S3 document to add a proxy requirement in the metrics forward path

Notice, no HTTPS/TLS setup support. By following AWS Firehose request, it uses [proxy request](https://en.wikipedia.org/wiki/Proxy_server#Web_proxy_servers) (`https://...` instead of `/aws/firehose/metrics`), there must be a proxy(Nginx, Envoy, etc.).
@wu-sheng wu-sheng added the backend OAP backend related. label Feb 19, 2023
@wu-sheng wu-sheng added this to the 9.4.0 milestone Feb 19, 2023
@wu-sheng wu-sheng requested a review from pg-yang February 19, 2023 14:24
@wu-sheng
Copy link
Copy Markdown
Member Author

Upstream discussion on Armeria to locate this.

https://line-armeria.slack.com/archives/C1NGPBUH2/p1676811880163349

@wu-sheng
Copy link
Copy Markdown
Member Author

Debug logs from @pg-yang
image

image

@pg-yang
Copy link
Copy Markdown
Member

pg-yang commented Feb 19, 2023

We should remove the configurations from

enableTLS: ${SW_RECEIVER_AWS_FIREHOSE_HTTP_ENABLE_TLS:false}
tlsKeyPath: ${SW_RECEIVER_AWS_FIREHOSE_HTTP_TLS_KEY_PATH:}
tlsCertChainPath: ${SW_RECEIVER_AWS_FIREHOSE_HTTP_TLS_CERT_CHAIN_PATH:}
as well

Copy link
Copy Markdown
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configs in yaml file should be removed too

@pg-yang
Copy link
Copy Markdown
Member

pg-yang commented Feb 19, 2023

And configuration-vocabulary.md

@kezhenxu94
Copy link
Copy Markdown
Member

@wu-sheng
Copy link
Copy Markdown
Member Author

Debug logs from @pg-yang
image

image

And doc https://github.com/apache/skywalking/blob/cabe40eb47fc83d681f1f40359798915f8ff7428/docs/en/setup/backend/aws-firehose-receiver.md#notice

I will update other docs, and leave AWS doc to @pg-yang. He should have a more accurate version than I could do.

@wu-sheng wu-sheng requested a review from kezhenxu94 February 19, 2023 14:59
Copy link
Copy Markdown
Member

@pg-yang pg-yang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, wait for GHA

@wu-sheng wu-sheng merged commit eda83dc into master Feb 19, 2023
@wu-sheng wu-sheng deleted the rm-aws-tls branch February 19, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend OAP backend related.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants