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

[v0.14] Enable forwarding Authorization header in InferenceGraphs #467

Conversation

israel-hdez
Copy link

What this PR does / why we need it:

This will configure all InferenceGraph workloads to forward the standard HTTP Authorization header to the backing InferenceServices.

The Authorization header is used to receive/send credentials and let ODH stack to validate access.

By enabling forwarding of the Authorization header, we cover the case when there is an (some) auth-protected InferenceService(s) as part of an InferenceGraph. Access is fine-grained, so access to an InferenceGraph does not guarantee access to some InferenceService. The user needs to provide credentials to all resources (IGs and ISVC) that the request needs to go through. Since each workload validates credentials on its own, credentials need to be forwarded to all workloads of an InferenceGraph.

Which issue(s) this PR fixes
Fixes https://issues.redhat.com/browse/RHOAIENG-17827

Type of changes
Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Feature/Issue validation/testing:

  1. Deploy an InferenceGraph with some of the ISVCs with auth enabled. The IG doesn't need to be auth-protected.
  2. Test that requests that don't require to go through the auth-protected ISVCs will succeed.
  3. Test that requests that require to go through the auth-protected ISVCs will fail with no credentials, or credentials without enough privileges.
  4. Test that requests that require to go through the auth-protected ISVCs will succeed with credentials having enough privileges.

Checklist:

  • N/A Have you added unit/e2e tests that prove your fix is effective or that this feature works?
  • Has code been commented, particularly in hard-to-understand areas?
  • N/A Have you made corresponding changes to the documentation?

This will configure all `InferenceGraph` workloads to forward the standard HTTP `Authorization` header to the backing `InferenceServices`.

The `Authorization` header is used to receive/send credentials and let ODH stack to validate access.

By enabling forwarding of the `Authorization` header, we cover the case when there is an (some) auth-protected InferenceService(s) as part of an InferenceGraph. Access is fine-grained, so access to an InferenceGraph does not guarantee access to some InferenceService. The user needs to provide credentials to all resources (IGs and ISVC) that the request needs to go through. Since each workload validates credentials on its own, credentials need to be forwarded to all workloads of an InferenceGraph.

Signed-off-by: Edgar Hernández <[email protected]>
Copy link

openshift-ci bot commented Jan 16, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: israel-hdez

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

"cpuLimit": "1",
"headers": {
"propagate": [
"Authorization"
Copy link
Author

Choose a reason for hiding this comment

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

@Jooho @danielezonca Please, advice if this is fine.
I don't see an issue as long as everything runs over TLS.

Copy link

Choose a reason for hiding this comment

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

I think it is a good idea to add token to downstream endpoints.

@israel-hdez israel-hdez requested review from Jooho and danielezonca and removed request for terrytangyuan and mholder6 January 16, 2025 22:12
@Jooho
Copy link

Jooho commented Jan 20, 2025

LGTM from jooho

@Jooho
Copy link

Jooho commented Jan 22, 2025

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jan 22, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit bce3a4e into opendatahub-io:release-v0.14 Jan 22, 2025
20 checks passed
@israel-hdez israel-hdez deleted the j17827-ig-forward-token branch January 22, 2025 17:13
israel-hdez added a commit to israel-hdez/kserve that referenced this pull request Jan 22, 2025
…b-io#467)

This will configure all `InferenceGraph` workloads to forward the standard HTTP `Authorization` header to the backing `InferenceServices`.

The `Authorization` header is used to receive/send credentials and let ODH stack to validate access.

By enabling forwarding of the `Authorization` header, we cover the case when there is an (some) auth-protected InferenceService(s) as part of an InferenceGraph. Access is fine-grained, so access to an InferenceGraph does not guarantee access to some InferenceService. The user needs to provide credentials to all resources (IGs and ISVC) that the request needs to go through. Since each workload validates credentials on its own, credentials need to be forwarded to all workloads of an InferenceGraph.

Signed-off-by: Edgar Hernández <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants