Repository navigation
Conversation
The plugin registered with no network flags, so use_tls defaulted to false and 'tls on' was rejected. The client only used TLS because the credential providers forced use_tls on as a side effect, which also meant 'tls off' could not be used to reach a plain-HTTP endpoint such as a local AWS emulator. Register with FLB_OUTPUT_NET | FLB_IO_TLS, like out_s3, so TLS stays on by default and the 'tls' option controls the CloudWatch client. Signed-off-by: Srinivas Chippagiri <cvas22@gmail.com>
The EC2 and HTTP credential providers turn use_tls off on the output instance while they set up their plain-HTTP upstream, then set it to true unconditionally. Every output that uses the standard chain ends up with use_tls on after init, so a user's 'tls off' is lost for anything that reads it later. Save the instance value before the call and restore it afterwards. Signed-off-by: Srinivas Chippagiri <cvas22@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
Add internal tests that check the EC2 and HTTP credential providers leave the output instance use_tls value unchanged after upstream_set(), for both true and false. Add out_cloudwatch runtime tests that check use_tls, the CloudWatch client flags and the upstream flags for the default, 'tls off' and 'tls on' settings. Before the fix, 'tls off' still produced a TLS client and 'tls on' was rejected as unsupported. Signed-off-by: Srinivas Chippagiri <cvas22@gmail.com>
cb5806b to
dc78202
Compare
Logs
valgrind-flb-it-aws_credentials_http.log
valgrind-flb-rt-out_cloudwatch.log
valgrind-fluent-bit.log
valgrind-flb-it-aws_credentials_ec2.log
cloudwatch_logsoutput always used TLS, whatevertlswas set to. That made plain-HTTP endpoints such as a local AWS emulator unusable.The cause was in the EC2 and HTTP credential providers. Their
upstream_setturnedins->use_tlsoff while setting up their plain-HTTP upstream, then set it toFLB_TRUEregardless of its previous value. Because the EC2 provider is always part of the standard chain, every output using it ended up withuse_tlson after init.out_cloudwatch_logsreadsuse_tlsafterupstream_set, sotls offwas lost. That plugin also registered with.flags = 0, so it only used TLS by default because of this side effect, andtls onwas rejected with "does not support TLS".Changes (one commit per component):
out_cloudwatch_logs: register withFLB_OUTPUT_NET | FLB_IO_TLS(same asout_s3), so TLS stays on by default andtls on/tls offare both accepted. This commit comes first so that every commit keeps TLS on by default.aws: inupstream_set_fn_ec2()andupstream_set_fn_http(), save the instance'suse_tlsand restore it afterflb_output_upstream_set()instead of forcing it to true.tests: new internal teststest_ec2_provider_upstream_set_keeps_use_tlsandtest_http_provider_upstream_set_keeps_use_tls, and newout_cloudwatchruntime casestls_default,tls_off,tls_on, which checkuse_tls, the CloudWatch client flags and the upstream flags.Other AWS outputs are unaffected.
out_kinesis_streamsandout_kinesis_firehosehardcode TLS.out_s3andout_bigqueryalready registerFLB_IO_TLS.out_esandout_opensearchreaduse_tlsbefore they callupstream_set.Fixes #12577
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
All runs were on Linux (Debian bookworm, GCC 12, in Docker), configured with
-DFLB_TESTS_RUNTIME=On -DFLB_TESTS_INTERNAL=On -DFLB_USE_SIMDUTF=No. I turned simdutf off only because its AVX2 code did not build with GCC 12 in that container. It is unrelated to this change.flb-it-aws_credentials_ec2,flb-it-aws_credentials_http,flb-rt-out_cloudwatch: all pass.tls_offthen fail (tls=off use_tls=1 client flags=2 upstream flags=26), andtls_onfails withcloudwatch_logs.0 does not support TLS.flb-it-aws_credentials,flb-it-aws_credentials_sts,flb-rt-out_s3,flb-rt-out_kinesisandflb-rt-out_firehoseall pass.--leak-check=full) on the five new test cases reportsERROR SUMMARY: 0 errors from 0 contexts.tests/integration/scenarios/out_s3(shares the AWS credential code): 19 passed, 6 skipped, both normally and withVALGRIND=1 VALGRIND_STRICT=1.out_cloudwatch_logshas no integration scenario.Manual check with the configuration from the issue against
python3 -m http.server 4566:The requests now reach the plain-HTTP server, which rejects them with 501 because it does not implement POST. Before the fix they failed in the TLS handshake with
wrong version number.http.server log:
Not tested: real AWS endpoints, ECS/EKS credential sources, and Windows/macOS builds.
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit