[extension/oauth2clientauth] Add EndpointParams #7307

tomsanbear · 2022-01-24T02:23:57Z

Description:
Adds support to pass additional token endpoint parameters, in particular support for some oauth providers (in my case Ory Hydra) require the client to request the audience.

Link to tracking Issue: N/A

Testing: Tested on a custom build of the otel collector, in addition to adding unit test coverage.

Documentation: Added description of the endpoint param config in addition to an example in the readme.

linux-foundation-easycla · 2022-01-24T02:24:04Z

The committers are authorized under a signed CLA.

✅ Thomas Santerre (256d098, 5456ac9, 79a9ae8)

pavankrish123 · 2022-01-24T03:45:14Z

@jpkrohling I can take a review for this? Please assign this to me.

pavankrish123 · 2022-01-24T04:34:47Z

extension/oauth2clientauthextension/extension.go

@@ -76,6 +76,7 @@ func newClientCredentialsExtension(cfg *Config, logger *zap.Logger) (*ClientCred
 			ClientSecret: cfg.ClientSecret,
 			TokenURL:     cfg.TokenURL,
 			Scopes:       cfg.Scopes,
+			EndpointParams: cfg.EndpointParams,


Thanks @tomsanbear. Can you please modify this test case as well https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/extension/oauth2clientauthextension/extension_test.go#L48 in addition to the configuration test you modified. This test is a proper e2e use case.

Everything else LGTM

Hey @pavankrish123 , thanks for the quick review, i've pushed the test up to the branch. Let me know if there is anything else you suggest to add 👍

Thank you. I have added a minor suggestion on the test. Please add the test and everything else LGTM

jpkrohling · 2022-01-24T14:15:28Z

@jpkrohling I can take a review for this? Please assign this to me.

It's yours!

pavankrish123 · 2022-01-25T03:50:46Z

extension/oauth2clientauthextension/extension_test.go

-				Timeout:      2,
+				ClientID:       "testclientid",
+				ClientSecret:   "testsecret",
+				EndpointParams: url.Values{"audience": []string{"someaudience"}},


Can you please add the equivalent assert to test the EndpointParams to complete the test -

opentelemetry-collector-contrib/extension/oauth2clientauthextension/extension_test.go

Line 135 in 549435e

assert.Equal(t, test.settings.Timeout, rc.client.Timeout)

I have update the test case with the assertion, thanks!

…tor-contrib

pavankrish123 · 2022-01-27T03:06:00Z

Thank you! @jpkrohling need help in kicking off the rest of the builds and on merge.

jpkrohling · 2022-01-27T16:49:15Z

extension/oauth2clientauthextension/README.md

@@ -58,6 +60,7 @@ Following are the configuration fields
 - [**token_url**](https://datatracker.ietf.org/doc/html/rfc6749#section-3.2) - The resource server's token endpoint URLs.
 - [**client_id**](https://datatracker.ietf.org/doc/html/rfc6749#section-2.2) - The client identifier issued to the client.
 - [**client_secret**](https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1) - The secret string associated with above identifier.
+- [**enndoint_params**](https://github.com/golang/oauth2/blob/master/clientcredentials/clientcredentials.go#L44) - Additional parameters that are sent to the token endpoint.


Suggested change

- [**enndoint_params**](https://github.com/golang/oauth2/blob/master/clientcredentials/clientcredentials.go#L44) - Additional parameters that are sent to the token endpoint.

- [**endpoint_params**](https://github.com/golang/oauth2/blob/master/clientcredentials/clientcredentials.go#L44) - Additional parameters that are sent to the token endpoint.

github-actions · 2022-02-12T05:15:12Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

tomsanbear · 2022-02-14T17:17:47Z

Anything we can do to get this merged in? CI fails don't seem related

bogdandrutu · 2022-02-14T17:53:24Z

would be good to rebase, and ensure test pass.

github-actions · 2022-03-01T05:15:35Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

tomsanbear · 2022-03-02T00:31:06Z

Apologies from the spam as a result of the rebase, PR should be up to date with latest main now.

jpkrohling · 2022-03-02T09:32:00Z

Once you add a changelog entry, the CI should pass and I'll get this merged. Thanks for your patience!

jpkrohling · 2022-03-02T09:32:39Z

Sorry, just noticed that the linting is also failing:

golangci-lint run --allow-parallel-runners
extension.go:75: File is not `gofmt`-ed with `-s` (gofmt)
			ClientID:     cfg.ClientID,
			ClientSecret: cfg.ClientSecret,
			TokenURL:     cfg.TokenURL,
			Scopes:       cfg.Scopes,
make[2]: *** [../../Makefile.Common:96: lint] Error 1

jpkrohling · 2022-03-09T15:03:06Z

@tomsanbear, sorry to ask this, but the PR seems to have been opened from your main branch: could you please fix the conflict? I'll then merge this on green.

tomsanbear · 2022-03-09T15:12:55Z

@tomsanbear, sorry to ask this, but the PR seems to have been opened from your main branch: could you please fix the conflict? I'll then merge this on green.

@jpkrohling updated!

Fix open-telemetry#7307 --------- Signed-off-by: Alex Boten <aboten@lightstep.com>

tomsanbear added 3 commits January 23, 2022 21:16

Implement new config

256d098

update readme

5456ac9

fix typo

79a9ae8

tomsanbear requested a review from jpkrohling as a code owner January 24, 2022 02:23

tomsanbear requested a review from a team January 24, 2022 02:23

github-actions bot assigned Aneurysm9 Jan 24, 2022

pavankrish123 suggested changes Jan 24, 2022

View reviewed changes

jpkrohling assigned pavankrish123 and unassigned Aneurysm9 Jan 24, 2022

update extension_test with e2e config test

549435e

pavankrish123 suggested changes Jan 25, 2022

View reviewed changes

tomsanbear added 2 commits January 26, 2022 21:59

add assertiont to test case

a64bc54

Merge branch 'main' of github.com:open-telemetry/opentelemetry-collec…

ee4d674

…tor-contrib

pavankrish123 approved these changes Jan 27, 2022

View reviewed changes

jpkrohling reviewed Jan 27, 2022

View reviewed changes

typo fix

8147bac

jpkrohling approved these changes Jan 28, 2022

View reviewed changes

jpkrohling changed the title ~~Add EndpointParams to oauth2clientauthentication~~ [extension/oauth2clientauth] Add EndpointParams Jan 28, 2022

jpkrohling enabled auto-merge (squash) January 28, 2022 18:55

github-actions bot added the Stale label Feb 12, 2022

github-actions bot removed the Stale label Feb 15, 2022

github-actions bot added the Stale label Mar 1, 2022

tomsanbear requested review from djaglowski, pmm-sumo, dashpole, jmacd, dmitryax, Aneurysm9, anuraaga, bogdandrutu, mx-psi, punya, jsuereth, arminru and alolita as code owners March 2, 2022 00:27

Merge branch 'main' into main

ab048f1

github-actions bot removed the Stale label Mar 2, 2022

jpkrohling approved these changes Mar 2, 2022

View reviewed changes

tomsanbear and others added 4 commits March 2, 2022 21:22

go fmt

cad1005

update changelog

6214f3a

Merge branch 'main' into main

f65fa4c

Merge branch 'main' into main

f0e49da

Merge branch 'main' into main

6ebc8d2

jpkrohling merged commit 748a7f4 into open-telemetry:main Mar 9, 2022

pavankrish123 mentioned this pull request Mar 13, 2022

REQUEST: New membership for @pavankrish123 open-telemetry/community#1003

Closed

1 task

ZahidMirza95 pushed a commit to ZahidMirza95/opentelemetry-collector-contrib that referenced this pull request Jun 21, 2023

Split processor into its own module (open-telemetry#7858)

64f3415

Fix open-telemetry#7307 --------- Signed-off-by: Alex Boten <aboten@lightstep.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[extension/oauth2clientauth] Add EndpointParams #7307

[extension/oauth2clientauth] Add EndpointParams #7307

tomsanbear commented Jan 24, 2022

linux-foundation-easycla bot commented Jan 24, 2022 •

edited

Loading

pavankrish123 commented Jan 24, 2022

pavankrish123 Jan 24, 2022

tomsanbear Jan 25, 2022

pavankrish123 Jan 25, 2022

jpkrohling commented Jan 24, 2022

pavankrish123 Jan 25, 2022

tomsanbear Jan 27, 2022

pavankrish123 commented Jan 27, 2022

jpkrohling Jan 27, 2022

github-actions bot commented Feb 12, 2022

tomsanbear commented Feb 14, 2022

bogdandrutu commented Feb 14, 2022

github-actions bot commented Mar 1, 2022

tomsanbear commented Mar 2, 2022

jpkrohling commented Mar 2, 2022

jpkrohling commented Mar 2, 2022

jpkrohling commented Mar 9, 2022

tomsanbear commented Mar 9, 2022

	- [enndoint_params](https://github.com/golang/oauth2/blob/master/clientcredentials/clientcredentials.go#L44) - Additional parameters that are sent to the token endpoint.
	- [endpoint_params](https://github.com/golang/oauth2/blob/master/clientcredentials/clientcredentials.go#L44) - Additional parameters that are sent to the token endpoint.

[extension/oauth2clientauth] Add EndpointParams #7307

[extension/oauth2clientauth] Add EndpointParams #7307

Conversation

tomsanbear commented Jan 24, 2022

linux-foundation-easycla bot commented Jan 24, 2022 • edited Loading

pavankrish123 commented Jan 24, 2022

pavankrish123 Jan 24, 2022

Choose a reason for hiding this comment

tomsanbear Jan 25, 2022

Choose a reason for hiding this comment

pavankrish123 Jan 25, 2022

Choose a reason for hiding this comment

jpkrohling commented Jan 24, 2022

pavankrish123 Jan 25, 2022

Choose a reason for hiding this comment

tomsanbear Jan 27, 2022

Choose a reason for hiding this comment

pavankrish123 commented Jan 27, 2022

jpkrohling Jan 27, 2022

Choose a reason for hiding this comment

github-actions bot commented Feb 12, 2022

tomsanbear commented Feb 14, 2022

bogdandrutu commented Feb 14, 2022

github-actions bot commented Mar 1, 2022

tomsanbear commented Mar 2, 2022

jpkrohling commented Mar 2, 2022

jpkrohling commented Mar 2, 2022

jpkrohling commented Mar 9, 2022

tomsanbear commented Mar 9, 2022

linux-foundation-easycla bot commented Jan 24, 2022 •

edited

Loading