Skip to content
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

[6012]Update auth.go to support azure workload identity on AKS pod #6014

Closed

Conversation

superff
Copy link

@superff superff commented Jun 15, 2023

Why the changes in this PR are needed?

To support Azure AD workload identity ,

in order to get a token from Azure AD with workload identity

the code needs to fetch the client_assertion token from a file generated by K8s service account

AZURE_FEDERATED_TOKEN_FILE The path of the projected service account token file.

https://learn.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-client-creds-grant-flow#third-case-access-token-request-with-a-federated-credential

sample token request

POST /{tenant}/oauth2/v2.0/token HTTP/1.1               // Line breaks for clarity
Host: login.microsoftonline.com:443
Content-Type: application/x-www-form-urlencoded

scope=https%3A%2F%2Fgraph.microsoft.com%2F.default
&client_id=97e0a5b7-d745-40b6-94fe-5f77d35c6e05
&client_assertion_type=urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-bearer
&client_assertion=eyJhbGciOiJSUzI1NiIsIng1dCI6Imd4OHRHeXN5amNScUtqRlBuZDdSRnd2d1pJMCJ9.eyJ{a lot of characters here}M8U3bSUKKJDEg
&grant_type=client_credentials


What are the changes in this PR?

Notes to assist PR review:

Further comments:

Fred Feng and others added 2 commits June 14, 2023 22:44
@superff superff changed the title Update auth.go [6012]Update auth.go Jun 15, 2023
@superff superff changed the title [6012]Update auth.go [6012]Update auth.go to support azure workload identity on AKS pod Jun 15, 2023
@@ -340,6 +341,14 @@ func (ap *oauth2ClientCredentialsAuthPlugin) requestToken(ctx context.Context) (
if ap.ClientID != "" {
body.Add("client_id", ap.ClientID)
}
} else if ap.ClientAssertionFile != "" {
body.Add("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer")
Copy link
Member

Choose a reason for hiding this comment

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

We should probably add a ClientAssertionType field to the plugin struct and default it to urn:ietf:params:oauth:client-assertion-type:jwt-bearer. Also a ClientAssertion field to set the value directly and not a file path would be helpful too.

Copy link
Author

Choose a reason for hiding this comment

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

I will push an update

Copy link
Author

Choose a reason for hiding this comment

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

I will push an update

Copy link
Author

Choose a reason for hiding this comment

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

@ashutosh-narkar could you point me which test case is still failing? I couldn't find the failed test method

@ashutosh-narkar
Copy link
Member

@superff this seems like a good start. Please also add test cases that exercise the new functionality.

@superff
Copy link
Author

superff commented Jun 15, 2023

@superff this seems like a good start. Please also add test cases that exercise the new functionality.

Sounds good

1. Added the default value for client_assertion_type
2. Update the validation of client_credential

Signed-off-by: Fred Feng <[email protected]>
@netlify
Copy link

netlify bot commented Jun 16, 2023

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit 61f2c71
🔍 Latest deploy log https://app.netlify.com/sites/openpolicyagent/deploys/648e98a00f078000086cdb11
😎 Deploy Preview https://deploy-preview-6014--openpolicyagent.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

superff and others added 19 commits June 15, 2023 18:35
Fix for loop

Sign off by: Fred Feng<[email protected]>
Go lint

Sign off by: Fred Feng<[email protected]>
Previously if a data file was removed those changes would not be reflected on the store and OPA would continue using old data to run the test. This change attempts to fix that.

Fixes: open-policy-agent#5986

Signed-off-by: boranx <[email protected]>
Signed-off-by: Fred Feng <[email protected]>
Signed-off-by: Fred Feng <[email protected]>
Signed-off-by: Fred Feng <[email protected]>
1. Added the default value for client_assertion_type
2. Update the validation of client_credential

Signed-off-by: Fred Feng <[email protected]>
Sign off by: Fred Feng<[email protected]>

Signed-off-by: Fred Feng <[email protected]>
Fix for loop

Sign off by: Fred Feng<[email protected]>

Signed-off-by: Fred Feng <[email protected]>
Go lint

Sign off by: Fred Feng<[email protected]>

Signed-off-by: Fred Feng <[email protected]>
* Update awesome opa logo

Signed-off-by: Charlie Egan <[email protected]>

* Add awesome-opa to integrations list

Signed-off-by: Charlie Egan <[email protected]>

---------

Signed-off-by: Charlie Egan <[email protected]>
Signed-off-by: Fred Feng <[email protected]>
Concurrent evaluation of the http.send builtin for the
same object can sometimes result in the HTTP headers
map being concurrently accessed. This can happen for
example when a key already present in the inter-query
cache needs to be revalidated and multiple routines
may access the HTTP headers at the same time resulting
in a race.

This change adds a new Clone method to cache interface.
The idea is to give each routine its own copy of the cached object
which would mean it has a copy of the headers map and
thus should be able to avoid any sync issues.

Signed-off-by: Ashutosh Narkar <[email protected]>
Signed-off-by: Fred Feng <[email protected]>
Also update project description to match styra.com copy

Signed-off-by: Charlie Egan <[email protected]>
Signed-off-by: Fred Feng <[email protected]>
Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.15.1 to 1.16.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.15.1...v1.16.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Fred Feng <[email protected]>
Signed-off-by: Fred Feng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants