kedacore / keda

KEDA is a Kubernetes-based Event Driven Autoscaling component. It provides event driven scale for any container running in Kubernetes
https://keda.sh
Apache License 2.0
8.41k stars 1.06k forks source link

Generic OAuth2 tokens provider declaration #4694

Open kratkyzobak opened 1 year ago

kratkyzobak commented 1 year ago

Proposal

Add new TriggerAuthentification property like oAuth2token with target's clientID / resource / scope (based on various oauth integrations) with helpers for scalers developers.

Example TriggerAuthentification

- spec:
     podIdentity:
          source: azure-workload
     oAuth2token:
          source: podIdentity  # or hashiCorpVault, keycloakInstance etc
          targetResource: "xxxxxx" # resource to ask access token for        

Example "helper" interface for scalers:

tokenProvider, err := NewOAuth2TokenProvider(config); // returns sthg like OAuthTokenProvider from azidentity, but generic

if err != nil {
    token, err := tokenProvider.GetValidToken(); // would request token / refresh if needed

    // here would go scaler specific token usage (Bearer auth via HTTP / use as password for MySQL / ....)
}

Use-Case

Based on trying to implement https://github.com/kedacore/keda/pull/4657.

Short-term JWT tokens are introduced in many resources now. There are lot of OIDC providers in current Kubernetes world (Kubernetes itself OIDC, two OIDC flows in AKS, Identity providers within EKS and GCK.... self managed Keycloacks, Hashicorp Vaults....).

If every scaler would have to implement each, there would be lot of garbage code with different quality and different configuration style. Just as in https://github.com/kedacore/keda/pull/4657 I have implemented OIDC only for AAD Workload Identity since it's only one I'm interested in (and only one I understand enough to be able to test it as "after hours job").

There is allways 3 parts of job:

I think, there is opportunity to implement first 2 parts in some standardized way to be shared between scalers and then provide scalers mantainers simple interface to support multiple identity servers by one implementation with reusing same configuration for users.

Is this a feature you are interested in implementing yourself?

No

Anything else?

I'm able to provide some hands-on with implementation, but I'm not storng enough in GO to implement this whole by myself. Also, there would be needed some triage before implementation (mainly to cross-check various identity providers requirements). Also, same as implementation itself, there would be need for some abstractions in test (some sort of parametrized test to test "all" identity providers?)

JorTurFer commented 1 year ago

I think that this could be a good addition, WDYT @kedacore/keda-core-contributors ?

zroubalik commented 1 year ago

Yeah, I like it too.

mingmcb commented 1 year ago

is there any update on this one? any ETA when this will be complete?

JorTurFer commented 1 year ago

Summer has been complex, I think that in September we will continue at 100%. Nobody is working on this at this moment, are you willing to start with this?

stale[bot] commented 11 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

JoshuaCuevasUofT commented 11 months ago

Hi, me and a team at University of Toronto would like to work on this issue and contribute to its implementation. We're mostly comprised of computer science students, and this would be the first open source contribution for many of us. If there's any that has worked on this or is still working on this please let us know.

zroubalik commented 11 months ago

@JoshuaCuevasUofT hi, this is great! I am not aware of any work on this.

@kedacore/keda-maintainers wdyt?

stale[bot] commented 11 months ago

This issue has been automatically closed due to inactivity.

JoshuaCuevasUofT commented 11 months ago

Hi, I have some clarifying questions about this proposal's scope.

A) I wanted to clarify whether or not this proposal is only for Oauth2 tokens.

B) Including something like the property “oAuth2token” into the TriggerAuthentication passing clientID / resource / scope sounds good. It should be straightforward (though a little tedious) to create a dictionary that refers the resource (for example Microsoft AAD) to a certain JWKS url (for example https://login.microsoftonline.com/%s/discovery/keys). And the scope to some specifications.

The dictionary would then contain many functions like these, varying with IDP. And be inherited by every sub-scaler.

func WithAzureADOAuth(tenantID string, clientID string) RabbitOAuthConfig {
    return RabbitOAuthConfig{
        Enable:    true,
        ClientID:  clientID,
        ScopesKey: "roles",
        JwksURI:   fmt.Sprintf("https://login.microsoftonline.com/%s/discovery/keys", tenantID),
    }
}

C) When a scaler parses this information from the ScaledObject yaml, and a triggerauth yaml, the parse function seems to be unique for every scaler. 1) It would be hard to implement a new property in triggerauth and change every scaler’s parse function. 2) Is this necessary if app-to-app authentication between a scaler and some Oauth2 token provider should require the same information/parameters across all scalers (given the dictionary above)? It makes sense to add some inherited function for all sub-scalers to parse the new “oauth2token” property from triggerauthentication yaml. But I can’t find an example of that. If this makes sense, could you point to something I could take a look at?

Am I missing anything to this proposal?

JoshuaCuevasUofT commented 11 months ago

Also @whyismynamerudy @VigneshNk and @kathy23980 are confirmed to be working with me on this issue over the school semester so tediousness shouldn't be too bad when split between 4 people. Again, apologies for the inconsistency. Depending on the scope, I think it's a fair timeline to say we should have this done within a month.

JoshuaCuevasUofT commented 11 months ago

@zroubalik do you have any comments sir?

stale[bot] commented 9 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

JorTurFer commented 8 months ago

I'm not an expert with OIDC stuff, but maybe we could implement this as a new CRD to handle all the OIDC information as reference it int he TriggerAuthentication., I see this as a service that we could inject into the scalers during the creation, configured for the given OIDC. In this case, the scaler would just need to adapt their logic to use the injected item to generate the tokens (we could use a pointer to know if it's configured or not, but maybe this is just an implementation detail).

I say about another service injected into the scalers, because I'm assuming that probably we won't be able to standarize all the OIDC provider. Sadly I'm quite sure that Azure/AWS/GCP include their **** flavour of the things, but maybe I'm wrong, making our lives easier xD.

This new CRD could allow us to have multiple "well known providers" with their own config and another generic one for generic OIDC providers. This is also the approach that Grafana does (but it's also true that they do more than just requesting a token).

But I fully agree that we need to standardize the process somehow, creating a mechanism to include OIDC support on scalers because nowadays almost any service can be protected behind an OIDC, and this could be extensible to almost all the scalers (except the cloud provider services)

jkremser commented 8 months ago

+1 using the same approach as Grafana folks. They require client_id and client_secret to be provided for each type of OAuth provider. Also

auth_url = https://foo/auth
token_url = https://foo/token
api_url = https://foo/userinfo 

seems to be common for all of them. Grafana also provides a simple way for authorization by running JMESPath queries over the json returned from /userinfo (api_url) endpoint. This part, together with the call to the /userinfo endpoint can be probably omitted, because all keda needs, is probably the jwt token and authN, but I am not OAuth expert.

Also not sure if the token refreshing logic is needed here or not. I guess it depends on the frequency of usage, maybe asking for a new token over and over again can be easier.

Maybe some code could be reused:

stale[bot] commented 6 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.