feat: add default cert for model registry, fixes RHOAIENG-9909

opendatahub-io / opendatahub-operator

Open Data Hub operator to manage ODH component integrations

https://opendatahub.io

Apache License 2.0

60 stars 139 forks source link

feat: add default cert for model registry, fixes RHOAIENG-9909 #1165

Closed dhirajsb closed 2 months ago

dhirajsb commented 2 months ago

Description

Add default cert for model registry in a secret named default-modelregistry-cert in the control plane namespace Fixes RHOAIENG-9909

How Has This Been Tested?

Added e2e tests to verify that the secret was created

Screenshot or short clip

Merge criteria

[X] You have read the contributors guide.
[X] Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
[X] Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
[X] Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
[X] The developer has manually tested the changes and verified that the changes work

dhirajsb commented 2 months ago

@VaishnaviHire fyi

dhirajsb commented 2 months ago

/retest

zdtsw commented 2 months ago

One more thing to confirm for the PR: if user have both Kserve and ModeReg enabled, and they set their certifcate.type: SelfSigned, it wont have anything to do with ModelReg? so basically ModelReg only support OpenshiftDefaultIngress as valid certifcate.type, right?

dhirajsb commented 2 months ago

@zdtsw @VaishnaviHire Since MR has been moved to 2.14, we can remove the rhoai-2.13 label.

dhirajsb commented 2 months ago

@zdtsw I added check for servicemesh feature and MR component state in e2e test. Please review and approve if it looks good to you.

dhirajsb commented 2 months ago

do you have some test result from e2e? esp. with this PR, and have both ModelReg and Kserve are in enabled case.

@zdtsw I tested the odh operator image in my private cluster, and this is the MR manifest it created in the operator container:

<<K9s-Shell>> Pod: openshift-operators/opendatahub-operator-controller-manager-7944cd5d66-5tz75 | Container: manager 
bash-4.4$ cat /opt/manifests/model-registry-operator/overlays/odh/params.env 
IMAGES_GRPC_SERVICE=quay.io/opendatahub/mlmd-grpc-server:latest
IMAGES_REST_SERVICE=quay.io/opendatahub/model-registry:latest
CREATE_AUTH_RESOURCES=true
DEFAULT_CERT=default-modelregistry-cert
IMAGES_MODELREGISTRY_OPERATOR=quay.io/opendatahub/model-registry-operator:latest
bash-4.4$

Looks like it is able to set the MR DEFAULT_CERT property correctly.

dhirajsb commented 2 months ago

@VaishnaviHire @zdtsw can you please approve this PR if you don't have any other changes?

dhirajsb commented 2 months ago

/retest

dhirajsb commented 2 months ago

I verified enabling kserve and model-registry in my local cluster and it's able to create certs for both:

[dbokde@dbokde opendatahub-operator (feat/default-mr-cert =)]$ kc get secrets -n istio-system | grep -E model\|knative
default-modelregistry-cert                             Opaque                                2      67m
knative-serving-cert                                   Opaque                                2      87m

dhirajsb commented 2 months ago

/retest

dhirajsb commented 2 months ago

/test opendatahub-operator-e2e

VaishnaviHire commented 2 months ago

/lgtm

openshift-ci[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: VaishnaviHire

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: - ~~[OWNERS](https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/OWNERS)~~ [VaishnaviHire] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment