-
Notifications
You must be signed in to change notification settings - Fork 479
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
feat(converter/otelcol): support converting sigv4authextension
#6713
feat(converter/otelcol): support converting sigv4authextension
#6713
Conversation
…tdata` Signed-off-by: hainenber <dotronghai96@gmail.com>
Signed-off-by: hainenber <dotronghai96@gmail.com>
…authextension` Signed-off-by: hainenber <dotronghai96@gmail.com>
sigv4authextension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Will approve after the two comments are resolved.
I'm OK with the ConvertWithoutValidation workaround for now, this one seems like an edge case and it's probably not worth doing a more extensive change just to get the converter working.
internal/converter/internal/otelcolconvert/testdata/otelcol_without_validation/sigv4auth.river
Outdated
Show resolved
Hide resolved
…nd not the top-level folder Signed-off-by: hainenber <dotronghai96@gmail.com>
…er result Signed-off-by: hainenber <dotronghai96@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
PR Description
Which issue(s) this PR fixes
Closes #6442
Notes to the Reviewer
I'm not sure this is the correct approach as the newly added
ConvertWithoutValidation
doesn't really reflect production usage and only for testing. I might open a new PR on the upstream to see if OtelCol folks are OK with opening the struct and/or add a setter to inject mock SigV4 credential provider.PR Checklist