enhancement(http provider): Add automatic bearer token acquisition in http-client [version 2] #21583

KowalczykBartek · 2024-10-22T22:12:57Z

PR to solve a #20635 feature request.
This change brings new section in configuration for HttpSinkConfig :

pub struct HttpClientAuthorizationConfig {
    auth: HttpClientAuthorizationStrategy,
    tls: Option<TlsConfig>,
}

this allows to explicit define a oauth2 server which will be used to acquire bearer token (also, this request allows to bring mtls via TlsConfig),

sources:
  my_source_id:
    scrape_interval_secs: 1
    type: http_client
    endpoint: http://127.0.0.1:9898/logs

sinks:
  my_sink_id:
    http_client_authorization_strategy:
      auth:
        strategy: "o_auth2"
        client_id: "your.clientid"
        client_secret: "verysecretstring"
        token_endpoint: "https://your.provider.com/oauth/token"
    type: http
    encoding:
      codec: raw_message
    inputs:
      - my_source_id
    uri: http://127.0.0.1:9091/output

Some basic error handling is also on place, so invalid credentials will not crash a vector instance :) you can expect an event and error message with body from oauth server (unfortunately I over engineered this error handling a little bit, I have limited rust knowledge and was unable to fully understand all patterns - so, suggestions, how to improve this would be appreciated :) )

2024-10-22T21:51:18.256079Z  WARN sink{component_kind="sink" component_id=my_sink_id component_type=http}:request{request_id=1}:http: vector::internal_events::http_client: HTTP Auth extension error. error=Server error from authentication server: {"error":"invalid_client","error_description":"Bad credentials"} error_type="request_failed" stage="processing" internal_log_rate_limit=true

So, as discussed in #21023 (comment) - @pront I would be grateful for review :)

KowalczykBartek · 2024-10-22T22:49:59Z

src/http.rs

+            //should request for token influence upstream service latency ?            
+            if let Some(auth_extension) = auth_extension {
+                let auth_span = tracing::info_span!("auth_extension");
+                let res = auth_extension.modify_request(&mut request)


I had no idea how to handle this

Box<dyn Error>

to some vector friendly error better 😞

I was able to improve it a little bit with snafu

auth_extension.modify_request(&mut request) .instrument(auth_span.clone().or_current()) .await .inspect_err(|error| { // Emit the error into the internal events system. emit!(http_client::GotAuthExtensionError{error}); }) .context(ExtensionAuthenticationSnafu)?;

src/http.rs

pront

Hi @KowalczykBartek thank you for this contribution!

I did a quick scan of the changes and left a few comments.

.gitignore

src/http.rs

pront · 2024-10-23T18:37:19Z

src/http.rs

+pub struct HttpClientAuthorizationConfig {
+    /// Define how to authorize against an upstream.
+    #[configurable]
+    auth: HttpClientAuthorizationStrategy,


Can we flatten this to avoid repetition in configs, from:

http_client_authorization_strategy: auth: strategy: "o_auth2" client_id: "your.clientid" client_secret: "verysecretstring" token_endpoint: "https://your.provider.com/oauth/token"

to

http_client_authorization_strategy: strategy: "o_auth2" client_id: "your.clientid" client_secret: "verysecretstring" token_endpoint: "https://your.provider.com/oauth/token"

I think this makes sense from a UX perspective cc @vectordotdev/ux-team

Honestly I was unable to figure out this - @pront can you pls point me some example with flattened enum ?

I took another look at this:

We can replace pub http_client_authorization_strategy: Option<HttpClientAuthorizationConfig> with pub authorization_config: Option<AuthorizationConfig>

Rename auth to strategy

then config will look like this

sinks: my_sink_id: authorization_config: strategy: strategy: "o_auth2" client_id: "some.client.id" client_secret: "ab!@#" grace_period: 780 token_endpoint: "https://127.0.0.1:8091/oauth/token"

is this double strategy ok ? maybe I will change also tag from

/// Configuration of the authentication strategy for HTTP requests. /// /// HTTP authentication should be used with HTTPS only, as the authentication credentials are passed as an /// HTTP header without any additional encryption beyond what is provided by the transport itself. #[configurable_component] #[derive(Clone, Debug)] #[serde(deny_unknown_fields, rename_all = "snake_case", tag = "strategy")] #[configurable(metadata(docs::enum_tag_description = "The authentication strategy to use."))]

to 'protocol' ? maybe 'provider' ?

together with fix for mtls extension I pushed renamed struct

#[configurable(derived)] pub authorization_config: Option<AuthorizationConfig>,

src/internal_events/http_client.rs

src/http.rs

KowalczykBartek · 2024-10-23T23:37:40Z

I introduced grace_period property so, it can be adjusted - before it was hard coded for 1min. I also played around this feature more and found bug around grace calculation (overflow), so also fixed this.
I also verified following config works (so use case from original issue, oauth2 + mtls)

sinks:
  my_sink_id:
    http_client_authorization_strategy:
      auth:
        strategy: "o_auth2"
        client_id: "clientid"
        client_secret: "secret"
        grace_period: 300
        token_endpoint: "https://127.0.0.1:8091/oauth/token"
      tls:
        crt_path: /config/cert.pem
        key_file: /config/server.key

what I need still to do is test against more providers (I tested with UAA, its pretty common).

Hi @singhbaljit, may I ask you to check if that integration works in your env ? if there will be any required change I could make an adjustments.

src/sinks/http/config.rs

pront · 2024-10-24T17:15:29Z

src/http.rs

+pub struct HttpClientAuthorizationConfig {
+    /// Define how to authorize against an upstream.
+    #[configurable]
+    auth: HttpClientAuthorizationStrategy,


I took another look at this:

We can replace pub http_client_authorization_strategy: Option<HttpClientAuthorizationConfig> with pub authorization_config: Option<AuthorizationConfig>

Rename auth to strategy

src/http.rs

pront

This is starting to look great. Left a few nits, these should improve config readability.

KowalczykBartek · 2024-10-28T19:32:32Z

after testing on real environments I found another issue, oauth2 + mtls should be strict to the rfc https://datatracker.ietf.org/doc/html/rfc8705, so, speaking in cURL
this is oauth2 + mtls extension

curl --cert cert.pem --key key.pem --request POST \
  --url https://something/oauth/token \
  --header 'Content-Type: application/x-www-form-urlencoded' \
  --data 'grant_type=client_credentials' \
  --data 'client_id=abcdabcdabcd'

this is oauth2

curl --request POST \
  --url https://something/oauth/token \
  --header 'Content-Type: application/x-www-form-urlencoded' \
  --data client_secret=abcdabcdabcd \
  --data grant_type=client_credentials \
  --data client_id=abcdabcdabcd

I also updated a comment around HttpClientAuthorizationStrategy. With those changes I think I can say: it works :)

pront · 2024-10-28T21:00:23Z

after testing on real environments I found another issue, oauth2 + mtls should be strict to the efc https://datatracker.ietf.org/doc/html/rfc8705, so, speaking in cURL this is oauth2 + mtls extension
curl --cert cert.pem --key key.pem --request POST \
  --url https://something/oauth/token \
  --header 'Content-Type: application/x-www-form-urlencoded' \
  --data 'grant_type=client_credentials' \
  --data 'client_id=abcdabcdabcd'
this is oauth2
curl --request POST \
  --url https://something/oauth/token \
  --header 'Content-Type: application/x-www-form-urlencoded' \
  --data client_secret=abcdabcdabcd \
  --data grant_type=client_credentials \
  --data client_id=abcdabcdabcd
I also updated a comment around HttpClientAuthorizationStrategy. With those changes I think I can say: it works :)

Thank you @KowalczykBartek, I will do another review tomorrow.

pront

👋 Left some comments. Most of them are nits but we should get rid of the unwraps.

Also, we lack tests here. We could mock the endpoint but we can also test some internal bits separately. Please see existing tests for inspiration.

Thank you again for this contribution!

src/http.rs

KowalczykBartek · 2024-10-31T19:42:52Z

Also, we lack tests here. We could mock the endpoint but we can also test some internal bits separately. Please see existing tests for inspiration.

just FYI (because I see some jobs running :D ) - I will provide tests, I just need few days more :)

pront · 2024-10-31T19:51:31Z

Also, we lack tests here. We could mock the endpoint but we can also test some internal bits separately. Please see existing tests for inspiration.

just FYI (because I see some jobs running :D ) - i will provide tests, I just need few days more :)

Thank you @KowalczykBartek, much appreciated. Also, I wanted to share a few notes to help with the CI checks:

we can add an http scope here
also see our changelog guide
cargo install -f --path vdev #cd to the repo root
Run cargo vdev fmt
Run cargo vdev check rust --clippy

pront · 2024-11-07T17:56:58Z

Thank you again @KowalczykBartek for this great PR. You went above and beyond 👍 I enqueued it for merging.

KowalczykBartek · 2024-11-07T18:00:27Z

Thank you @pront for support and patience :)

pront · 2024-11-07T18:45:39Z

One more thing here, I noticed this failure: https://github.com/vectordotdev/vector/actions/runs/11728482979/job/32672282844?pr=21583.

This might be helpful:

make generate-component-docs
# review any changes
# commit if they look good
make check-component-docs
git push

KowalczykBartek · 2024-11-07T19:03:46Z

One more thing here, I noticed this failure: https://github.com/vectordotdev/vector/actions/runs/11728482979/job/32672282844?pr=21583.

This might be helpful:
make generate-component-docs
# review any changes
# commit if they look good
make check-component-docs
git push

hmm, generate-component-docs fail with

vector/scripts/generate-component-docs.rb:1753:in `<main>': undefined method `filter_map' for #<Hash:0x00000001380c15a0> (NoMethodError)

jszwedko · 2024-11-07T19:05:27Z

hmm, generate-component-docs fail with

vector/scripts/generate-component-docs.rb:1753:in `<main>': undefined method `filter_map' for #<Hash:0x00000001380c15a0> (NoMethodError)

I think that means you are using Ruby 2, that script requires Ruby 3 (that should really be clearer 😓 ).

KowalczykBartek · 2024-11-07T19:06:51Z

you are right, I have ruby 2, let me try to fix this :D

KowalczykBartek · 2024-11-07T19:39:38Z

I was able to run this command,

# review any changes
# commit if they look good

but this this is tricky, I am not sure what this http.cue is :) It looks, good, but is it good ? not sure :)

btw, last time I saw ruby was like.. 6years ago when deploying database using bosh.io (not a drills company :)) :D

jszwedko · 2024-11-07T19:54:30Z

.github/workflows/semantic.yml

@@ -29,6 +29,7 @@ jobs:
            revert

          scopes: |
+            http


Suggested change

http

This change seems to affect the http and axiom sinks? Could we use those pre-existing scopes instead?

I don't know :) I did this because it was one of the review comments #21583 (comment) :)

I thought it is worth introducing a scope since this affects a lower level module used by multiple components. But I don't really have a strong opinion here.

Aha gotcha, sorry for the run-around 😅 Maybe http provider would be more consistent? We have other x providers already as scopes to use for lower level changes that affect multiple components that share a "provider".

I removed this http component from list as suggested

KowalczykBartek · 2024-11-07T22:54:34Z

Windows has some differences across error messages, so, I just removed error msg expectation for connection refused and internal server errors, its too much library and platform dependent

singhbaljit · 2024-11-08T18:03:52Z

Chiming in a bit late... this is highly appreciated! I'll certainly test this feature and report any issues I find. Thank you very much!

tareksha · 2024-11-12T17:05:15Z

many thanks @KowalczykBartek for the great work!

KowalczykBartek added 3 commits October 22, 2024 16:48

HTTP client auth as trait extension

e43a2e2

Use Client directly

8875329

Unavailable oauth server should not crash vector

0f15ea3

KowalczykBartek requested a review from a team as a code owner October 22, 2024 22:12

github-actions bot added the domain: sinks Anything related to the Vector's sinks label Oct 22, 2024

KowalczykBartek mentioned this pull request Oct 22, 2024

enhancement(http-client): Add automatic bearer token acquisition in http-client #21023

Closed

KowalczykBartek commented Oct 22, 2024

View reviewed changes

KowalczykBartek commented Oct 23, 2024

View reviewed changes

src/http.rs Show resolved Hide resolved

Fix snafu error to by sync + send

3c87e8d

pront self-assigned this Oct 23, 2024

pront reviewed Oct 23, 2024

View reviewed changes

review fixes

e37eaf1

KowalczykBartek added 2 commits October 24, 2024 01:40

review fixes

0c44809

review fixes

31d0327

pront reviewed Oct 24, 2024

View reviewed changes

src/http.rs Show resolved Hide resolved

pront reviewed Oct 24, 2024

View reviewed changes

KowalczykBartek added 2 commits October 28, 2024 20:24

acquire token with oauth2 and mtls as described in rfc

8f84adf

fix spell issues

9b7fd99

pront reviewed Oct 29, 2024

View reviewed changes

KowalczykBartek added 3 commits October 30, 2024 23:45

fix review comments

00ebf03

fix review comments

e840a2e

fix review comments

b92f587

pront changed the title ~~enhancement(http-client): Add automatic bearer token acquisition in http-client [version 2]~~ enhancement(http): Add automatic bearer token acquisition in http-client [version 2] Oct 31, 2024

fix review comments

5ef76fd

KowalczykBartek added 2 commits November 7, 2024 08:50

review fixes

051cc54

review fixes

52f1cc4

pront enabled auto-merge November 7, 2024 17:55

generate-component-docs

4e9500c

auto-merge was automatically disabled November 7, 2024 19:37
Head branch was pushed to by a user without write access

KowalczykBartek requested review from a team as code owners November 7, 2024 19:37

github-actions bot added the domain: external docs Anything related to Vector's external, public documentation label Nov 7, 2024

aliciascott approved these changes Nov 7, 2024

View reviewed changes

jszwedko reviewed Nov 7, 2024

View reviewed changes

remove http component

aac3949

pront changed the title ~~enhancement(http): Add automatic bearer token acquisition in http-client [version 2]~~ enhancement(http provider): Add automatic bearer token acquisition in http-client [version 2] Nov 7, 2024

pront enabled auto-merge November 7, 2024 20:09

pront added this pull request to the merge queue Nov 7, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 7, 2024

unable to expect a message under windows

9a51704

pront enabled auto-merge November 7, 2024 23:49

pront added this pull request to the merge queue Nov 8, 2024

Merged via the queue into vectordotdev:master with commit fb9c5c1 Nov 8, 2024
39 of 40 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enhancement(http provider): Add automatic bearer token acquisition in http-client [version 2] #21583

enhancement(http provider): Add automatic bearer token acquisition in http-client [version 2] #21583

KowalczykBartek commented Oct 22, 2024 •

edited

Loading

KowalczykBartek Oct 22, 2024 •

edited

Loading

KowalczykBartek Oct 23, 2024

pront left a comment

pront Oct 23, 2024

KowalczykBartek Oct 23, 2024

pront Oct 24, 2024

KowalczykBartek Oct 25, 2024 •

edited

Loading

KowalczykBartek Oct 28, 2024 •

edited

Loading

KowalczykBartek commented Oct 23, 2024 •

edited

Loading

pront Oct 24, 2024

pront left a comment

KowalczykBartek commented Oct 28, 2024 •

edited

Loading

pront commented Oct 28, 2024

pront left a comment

KowalczykBartek commented Oct 31, 2024 •

edited

Loading

pront commented Oct 31, 2024 •

edited

Loading

pront commented Nov 7, 2024

KowalczykBartek commented Nov 7, 2024

pront commented Nov 7, 2024

KowalczykBartek commented Nov 7, 2024

jszwedko commented Nov 7, 2024

KowalczykBartek commented Nov 7, 2024

KowalczykBartek commented Nov 7, 2024 •

edited

Loading

jszwedko Nov 7, 2024

KowalczykBartek Nov 7, 2024

pront Nov 7, 2024

jszwedko Nov 7, 2024

KowalczykBartek Nov 7, 2024

KowalczykBartek commented Nov 7, 2024 •

edited

Loading

singhbaljit commented Nov 8, 2024

tareksha commented Nov 12, 2024

enhancement(http provider): Add automatic bearer token acquisition in http-client [version 2] #21583

enhancement(http provider): Add automatic bearer token acquisition in http-client [version 2] #21583

Conversation

KowalczykBartek commented Oct 22, 2024 • edited Loading

KowalczykBartek Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pront left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KowalczykBartek Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

KowalczykBartek Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

KowalczykBartek commented Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

pront left a comment

Choose a reason for hiding this comment

KowalczykBartek commented Oct 28, 2024 • edited Loading

pront commented Oct 28, 2024

pront left a comment

Choose a reason for hiding this comment

KowalczykBartek commented Oct 31, 2024 • edited Loading

pront commented Oct 31, 2024 • edited Loading

pront commented Nov 7, 2024

KowalczykBartek commented Nov 7, 2024

pront commented Nov 7, 2024

KowalczykBartek commented Nov 7, 2024

jszwedko commented Nov 7, 2024

KowalczykBartek commented Nov 7, 2024

KowalczykBartek commented Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KowalczykBartek commented Nov 7, 2024 • edited Loading

singhbaljit commented Nov 8, 2024

tareksha commented Nov 12, 2024

KowalczykBartek commented Oct 22, 2024 •

edited

Loading

KowalczykBartek Oct 22, 2024 •

edited

Loading

KowalczykBartek Oct 25, 2024 •

edited

Loading

KowalczykBartek Oct 28, 2024 •

edited

Loading

KowalczykBartek commented Oct 23, 2024 •

edited

Loading

KowalczykBartek commented Oct 28, 2024 •

edited

Loading

KowalczykBartek commented Oct 31, 2024 •

edited

Loading

pront commented Oct 31, 2024 •

edited

Loading

KowalczykBartek commented Nov 7, 2024 •

edited

Loading

KowalczykBartek commented Nov 7, 2024 •

edited

Loading