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

Integrate Authorino with KServe (kserve/modelmesh) #128

Closed
Jooho opened this issue Nov 20, 2023 · 1 comment
Closed

Integrate Authorino with KServe (kserve/modelmesh) #128

Jooho opened this issue Nov 20, 2023 · 1 comment
Assignees
Labels
kind/feature New feature

Comments

@Jooho
Copy link

Jooho commented Nov 20, 2023

/kind feature

Describe the solution you'd like
[A clear and concise description of what you want to happen.]
Integrate Authorino with KServe (kserve/modelmesh)

This ticket is for tracking purpose.

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

@openshift-ci openshift-ci bot added the kind/feature New feature label Nov 20, 2023
@israel-hdez israel-hdez self-assigned this Dec 11, 2023
israel-hdez added a commit to israel-hdez/opendatahub-operator that referenced this issue Dec 11, 2023
This first iteration is to cover authentication needs for KServe

* Add templates to install Authorino
* Add templates to configure Service Mesh to use Authorino to delegate Authorization
* Add KServe-specific templates add ability to secure KServe Inference Services
* Add relevant fields to DSCInitialization resource
* Code for proper cleanup, in case of uninstalling

Most (if not all) of this code comes from pull request opendatahub-io#605. Attribution to original authors: @bartoszmajsak, @aslakknutsen, @cam-garrison, et. al.

Related opendatahub-io/kserve#128

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

@israel-hdez israel-hdez closed this as not planned Won't fix, can't repro, duplicate, stale Dec 20, 2023
israel-hdez added a commit to israel-hdez/opendatahub-operator that referenced this issue Jan 15, 2024
This first iteration is to cover authentication needs for KServe

* Add templates to install Authorino
* Add templates to configure Service Mesh to use Authorino to delegate Authorization
* Add KServe-specific templates add ability to secure KServe Inference Services
* Add relevant fields to DSCInitialization resource
* Code for proper cleanup, in case of uninstalling

Most (if not all) of this code comes from pull request opendatahub-io#605. Attribution to original authors: @bartoszmajsak, @aslakknutsen, @cam-garrison, et. al.

Related opendatahub-io/kserve#128

Signed-off-by: Edgar Hernández <[email protected]>
israel-hdez added a commit to israel-hdez/opendatahub-operator that referenced this issue Jan 18, 2024
This first iteration is to cover authentication needs for KServe

* Add templates to install Authorino
* Add templates to configure Service Mesh to use Authorino to delegate Authorization
* Add KServe-specific templates add ability to secure KServe Inference Services
* Add relevant fields to DSCInitialization resource
* Code for proper cleanup, in case of uninstalling

Most (if not all) of this code comes from pull request opendatahub-io#605. Attribution to original authors: @bartoszmajsak, @aslakknutsen, @cam-garrison, et. al.

Related opendatahub-io/kserve#128

Signed-off-by: Edgar Hernández <[email protected]>
bartoszmajsak pushed a commit to israel-hdez/opendatahub-operator that referenced this issue Jan 23, 2024
This first iteration is to cover authentication needs for KServe

* Add templates to install Authorino
* Add templates to configure Service Mesh to use Authorino to delegate Authorization
* Add KServe-specific templates add ability to secure KServe Inference Services
* Add relevant fields to DSCInitialization resource
* Code for proper cleanup, in case of uninstalling

Most (if not all) of this code comes from pull request opendatahub-io#605. Attribution to original authors: @bartoszmajsak, @aslakknutsen, @cam-garrison, et. al.

Related opendatahub-io/kserve#128

Signed-off-by: Edgar Hernández <[email protected]>
bartoszmajsak pushed a commit to israel-hdez/opendatahub-operator that referenced this issue Jan 23, 2024
This first iteration is to cover authentication needs for KServe

* Add templates to install Authorino
* Add templates to configure Service Mesh to use Authorino to delegate Authorization
* Add KServe-specific templates add ability to secure KServe Inference Services
* Add relevant fields to DSCInitialization resource
* Code for proper cleanup, in case of uninstalling

Most (if not all) of this code comes from pull request opendatahub-io#605. Attribution to original authors: @bartoszmajsak, @aslakknutsen, @cam-garrison, et. al.

Related opendatahub-io/kserve#128

Signed-off-by: Edgar Hernández <[email protected]>
VaishnaviHire pushed a commit to opendatahub-io/opendatahub-operator that referenced this issue Feb 19, 2024
* feat(authz): Authorino for Service Mesh

This first iteration is to cover authentication needs for KServe

* Add templates to install Authorino
* Add templates to configure Service Mesh to use Authorino to delegate Authorization
* Add KServe-specific templates add ability to secure KServe Inference Services
* Add relevant fields to DSCInitialization resource
* Code for proper cleanup, in case of uninstalling

Most (if not all) of this code comes from pull request #605. Attribution to original authors: @bartoszmajsak, @aslakknutsen, @cam-garrison, et. al.

Related opendatahub-io/kserve#128

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

* Fix linter issues

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

* Resolve feedback: Bartosz

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

* fix: Remove port from the authorization policy

Also, add `/metrics` to the ignored paths for auth.

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

* Fix feedback: Bartosz

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

* More feedback: Bartosz

Co-authored-by: Bartosz Majsak <[email protected]>

* Fix feedback: Reto - Adjust AuthorizationPolicy

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

* Fix more feedback: Bartosz

- Remove Authorino namespace field from DSCI.
- Move around some code in kserve.go to servicemesh_setup.go

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

* chore: adds sec. prefix to authorino label selector

* fix: adds base dir to manifest sources

* chore: uses security instead of sec as a prefix in authorino label

* fix: /healthz is called by _something_, skipp

* fix: adopt ODH-ADR-0006 for clean up label

* fix: uses correct CRD name for authconfigs

Co-authored-by: Cameron Garrison <[email protected]>

* Remove left-over file

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

* Feedback: remove auth-refs ConfigMap

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

* Add missing role.yaml changes

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

* Go back to installing Authorino on its own namespace

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

* Feedback: Add clean-up for KServe/OSSM-auth

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

* Feedback: Simplify namings

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

* fix: add auth-refs cm

* Feedback: adjust labels and a log message

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

* Bugfix: Extension provider terminating with error when SMCP is gone

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

* Fix: add missing RBAC for ConfigMaps func

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

* Fix: Run `make bundle` and commit resulting changes

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

* Feedback: Wen - Better feature namings

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

* Feedback: Bartosz

* Use feature logger
* Don't trim -applications suffix on ResolveAuthNamespace

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

* Feedback: Wen - revert image placeholder was replaced

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

---------

Signed-off-by: Edgar Hernández <[email protected]>
Co-authored-by: Bartosz Majsak <[email protected]>
Co-authored-by: Aslak Knutsen <[email protected]>
Co-authored-by: Cameron Garrison <[email protected]>
Jooho pushed a commit to Jooho/kserve that referenced this issue Feb 28, 2024
…tudio-purge-kserve-qpext-28

Red Hat Konflux purge kserve-qpext-28
VaishnaviHire pushed a commit to VaishnaviHire/opendatahub-operator that referenced this issue Mar 11, 2024
* feat(authz): Authorino for Service Mesh

This first iteration is to cover authentication needs for KServe

* Add templates to install Authorino
* Add templates to configure Service Mesh to use Authorino to delegate Authorization
* Add KServe-specific templates add ability to secure KServe Inference Services
* Add relevant fields to DSCInitialization resource
* Code for proper cleanup, in case of uninstalling

Most (if not all) of this code comes from pull request opendatahub-io#605. Attribution to original authors: @bartoszmajsak, @aslakknutsen, @cam-garrison, et. al.

Related opendatahub-io/kserve#128

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

* Fix linter issues

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

* Resolve feedback: Bartosz

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

* fix: Remove port from the authorization policy

Also, add `/metrics` to the ignored paths for auth.

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

* Fix feedback: Bartosz

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

* More feedback: Bartosz

Co-authored-by: Bartosz Majsak <[email protected]>

* Fix feedback: Reto - Adjust AuthorizationPolicy

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

* Fix more feedback: Bartosz

- Remove Authorino namespace field from DSCI.
- Move around some code in kserve.go to servicemesh_setup.go

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

* chore: adds sec. prefix to authorino label selector

* fix: adds base dir to manifest sources

* chore: uses security instead of sec as a prefix in authorino label

* fix: /healthz is called by _something_, skipp

* fix: adopt ODH-ADR-0006 for clean up label

* fix: uses correct CRD name for authconfigs

Co-authored-by: Cameron Garrison <[email protected]>

* Remove left-over file

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

* Feedback: remove auth-refs ConfigMap

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

* Add missing role.yaml changes

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

* Go back to installing Authorino on its own namespace

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

* Feedback: Add clean-up for KServe/OSSM-auth

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

* Feedback: Simplify namings

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

* fix: add auth-refs cm

* Feedback: adjust labels and a log message

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

* Bugfix: Extension provider terminating with error when SMCP is gone

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

* Fix: add missing RBAC for ConfigMaps func

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

* Fix: Run `make bundle` and commit resulting changes

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

* Feedback: Wen - Better feature namings

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

* Feedback: Bartosz

* Use feature logger
* Don't trim -applications suffix on ResolveAuthNamespace

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

* Feedback: Wen - revert image placeholder was replaced

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

---------

Signed-off-by: Edgar Hernández <[email protected]>
Co-authored-by: Bartosz Majsak <[email protected]>
Co-authored-by: Aslak Knutsen <[email protected]>
Co-authored-by: Cameron Garrison <[email protected]>
(cherry picked from commit e32a7c2)
zdtsw added a commit to red-hat-data-services/rhods-operator that referenced this issue Mar 12, 2024
* Update bundle

* feat(authz): Authorino for Service Mesh (opendatahub-io#784)

* feat(authz): Authorino for Service Mesh

This first iteration is to cover authentication needs for KServe

* Add templates to install Authorino
* Add templates to configure Service Mesh to use Authorino to delegate Authorization
* Add KServe-specific templates add ability to secure KServe Inference Services
* Add relevant fields to DSCInitialization resource
* Code for proper cleanup, in case of uninstalling

Most (if not all) of this code comes from pull request opendatahub-io#605. Attribution to original authors: @bartoszmajsak, @aslakknutsen, @cam-garrison, et. al.

Related opendatahub-io/kserve#128

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

* Fix linter issues

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

* Resolve feedback: Bartosz

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

* fix: Remove port from the authorization policy

Also, add `/metrics` to the ignored paths for auth.

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

* Fix feedback: Bartosz

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

* More feedback: Bartosz

Co-authored-by: Bartosz Majsak <[email protected]>

* Fix feedback: Reto - Adjust AuthorizationPolicy

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

* Fix more feedback: Bartosz

- Remove Authorino namespace field from DSCI.
- Move around some code in kserve.go to servicemesh_setup.go

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

* chore: adds sec. prefix to authorino label selector

* fix: adds base dir to manifest sources

* chore: uses security instead of sec as a prefix in authorino label

* fix: /healthz is called by _something_, skipp

* fix: adopt ODH-ADR-0006 for clean up label

* fix: uses correct CRD name for authconfigs

Co-authored-by: Cameron Garrison <[email protected]>

* Remove left-over file

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

* Feedback: remove auth-refs ConfigMap

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

* Add missing role.yaml changes

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

* Go back to installing Authorino on its own namespace

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

* Feedback: Add clean-up for KServe/OSSM-auth

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

* Feedback: Simplify namings

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

* fix: add auth-refs cm

* Feedback: adjust labels and a log message

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

* Bugfix: Extension provider terminating with error when SMCP is gone

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

* Fix: add missing RBAC for ConfigMaps func

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

* Fix: Run `make bundle` and commit resulting changes

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

* Feedback: Wen - Better feature namings

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

* Feedback: Bartosz

* Use feature logger
* Don't trim -applications suffix on ResolveAuthNamespace

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

* Feedback: Wen - revert image placeholder was replaced

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

---------

Signed-off-by: Edgar Hernández <[email protected]>
Co-authored-by: Bartosz Majsak <[email protected]>
Co-authored-by: Aslak Knutsen <[email protected]>
Co-authored-by: Cameron Garrison <[email protected]>
(cherry picked from commit e32a7c2)

* fix(authz): Fix broken external auth configuration

There are two misconfigurations being fixed:
* In the SMCP, the service hostname of Authorino was coded with `-authorization` suffix, but the right suffix is `-authorino-authorization`.
* In the `kserve-predictor` AuthorizationPolicy, the hardcoded `opendatahub-odh-auth-provider` provider name was used, but it should have been the template `{{ .AppNamespace }}-auth-provider`.

In `pkg/feature/feature.go` the patch manifests (i.e. the ones containing `.patch` in the filename) are always applied. Thus, the first bullet is solved by fixing the patch file that adds the `extensionProvider` to the SMCP.

For the second bullet, the faulty AuthorizationPolicy is created with a regular manifest template which is only applied if the resource does not exist. Thus, a patch manifest is added to properly fix the faulty policy (including operator upgrades).

Signed-off-by: Edgar Hernández <[email protected]>
(cherry picked from commit e4252a0)

* fix: Rework operator precondition checks (opendatahub-io#899)

* init commit

* tmp: switch to subsciption

* tmp

* fix up testing

* linter on import

* minor self nits

* add bracket, make

* use found,err for checking subscription

Co-authored-by: Bartosz Majsak <[email protected]>

* fix import + test error expected outputs

* directly return errs rather than log and ret

Co-authored-by: Bartosz Majsak <[email protected]>

* remove unused log var from condiitons

* move const fixtures to separate package

* move creating op subscription to function

* rename noop features in testing

* remove redundant comments

Co-authored-by: Bartosz Majsak <[email protected]>

* move CreateSubscription to fixtures

---------

Co-authored-by: Bartosz Majsak <[email protected]>
(cherry picked from commit f44528e)

* chore: follow up review comments from previous PR (opendatahub-io#858)

* update: follow up comments

- cleanup commented out code
- rename function
- cleanup unnecessary sleep

Signed-off-by: Wen Zhou <[email protected]>

* update: add check on return err + remove apierrs.IsNotFound check

Signed-off-by: Wen Zhou <[email protected]>

* Update pkg/deploy/deploy.go

Co-authored-by: Bartosz Majsak <[email protected]>

* update(review): create new function DeleteSubscription

Signed-off-by: Wen Zhou <[email protected]>

* update: return for get and delete subscription

- get: return 'sub, nil' or 'nil, err' here error can be real one or
notfound

Signed-off-by: Wen Zhou <[email protected]>

* Update pkg/deploy/deploy.go

Co-authored-by: Bartosz Majsak <[email protected]>

* fix(linter)

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
Co-authored-by: Bartosz Majsak <[email protected]>
(cherry picked from commit a81a3da)

* fix(authz): ensures extauthz provider is removed from control plane during cleanup (opendatahub-io#905)

### Renames migration folder
The reason for this is to have a simple naming convention instead of suggesting storing migration patches in dedicated folders named after tickets.

Additionally, the feature explicitly orders files instead of assuming that the underlying fsys implementation fulfills such a contract.

### Ports opendatahub-io#605 test for extension provider

This test ensures the addition of an extension provider for external authorization and that it is removed from the control plane properly using a custom cleanup function.
We have missed it in the original work.

### Fix: aligns provider name between template and cleanup logic

This is short-term fix for the existing codebase. In the long term (which is actively worked on) we need to improve the way of how we are storing config information to limit cases where we rely on pre/suffixes. Cases like this should be kept as its own thing instead, as it represents the concept in the infrastructure/authz setup.

* chore: indentation

Signed-off-by: Wen Zhou <[email protected]>

* fix: use old package path till we cherry-pick refactor commit

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
Co-authored-by: Edgar Hernández <[email protected]>
Co-authored-by: Edgar Hernández <[email protected]>
Co-authored-by: Cameron Garrison <[email protected]>
Co-authored-by: Wen Zhou <[email protected]>
Co-authored-by: Bartosz Majsak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature
Projects
Status: Done
Development

No branches or pull requests

2 participants