feat(mvp): enabling service mesh for ODH

[cam-garrison]

⚠️ This PR should be rebased onto the target branch as it keeps a lean history of fixes and new features introduced since it's been open

Description

This PR implements our MVP of enabling service mesh with ODH, using Authorino combined with service mesh to facilitate authorization to ODH components.

To deploy ODH with Service Mesh, follow along with our instructions found in enabling_ossm.md to find a reference KfDef along with other steps.

Notes:

• Current implementation relies on some hardcoded namespaces: using opendatahub for the KfDef namespace, and istio-system + auth-provider for the OSSM SMCP and Authorino resources respectively. Ideally we would not be hardcoding opendatahub, we are looking for suggestions on how to avoid this.
• We use an initJob to dynamically create resources based on cluster configuration + to allow for secret propagation between requisite resources. In the future, this will ideally be implemented by a more sophisticated controller/control plane to create and reconcile these resources.
• We do not support a migration path for pre-existing ODH deployments yet.
• We do not use the opendatahub-operator secret-generator to create our OauthClient resource as we need the secret to be propagated to another resource - in the future, we may extend the secret-generator to perform this functionality rather than handle it in our initJob or in a controller. Is this something that folks well-versed in the operator think could be possible?
• We configure our service mesh deployment to use autoInject, injecting to all pods found in the mesh namespaces by default for convenience. This may cause problems if future components not yet considered should not be in the mesh, so this may be something to revisit.
• The resources generated in istio-system and auth-provider generated by the KfDef creation are not deleted by the operator upon KfDef deletion. This could be something to address in the future ODH operator rewrite.

How Has This Been Tested?

Tested by creating our KfDef in CRC clusters.

• Ensured that while accessing the dashboard for the first time user was redirected to openshift login which initiated the oauth flow. After successful login the dashboard is presented to the user.
• Tested that SMCP is created in istio-system, that minimal ODH components are created in opendatahub and have istio-proxy injected, and that authorino pod is created in auth-provider and is injected with istio-proxy container.
• Tested that when notebooks created, they are injected with istio-proxy and contain necessary annotations for service-mesh.
• Tested that data science projects created have a ServiceMeshMember resource and requisite service mesh annotations added to namespace.

Merge criteria:

• The commits are squashed in a cohesive manner and have meaningful messages.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

[danielezonca]

Current implementation relies on some hardcoded namespaces: using opendatahub for the KfDef namespace, and istio-system + auth-provider for the OSSM SMCP and Authorino resources respectively. Ideally we would not be hardcoding opendatahub, we are looking for suggestions on how to avoid this.

I'm not doing a full review, I just checked the description of the PR and I would like to better understand this point. Can you please clarify the implication of this? For example is it possible to create two different KfDef objects in different namespaces?

[cam-garrison]

I'm not doing a full review, I just checked the description of the PR and I would like to better understand this point. Can you please clarify the implication of this?

@danielezonca - The implication of this point was pointing out the limitation that when using this PR/manifests, the KfDef can only be created in the namespace opendatahub. From my understanding, typically users are able to create a KfDef/ODH deployment in any namespace of their choosing, so I wanted to make sure reviewers/testers were aware of this limitation and look for suggestions on how to fix this.

As for:

For example is it possible to create two different KfDef objects in different namespaces?

I believe it is (could be wrong on this - experts please correct me if so) - although it is worth noting that currently, service mesh integration does not support multiple KfDefs in one cluster.

[bartoszmajsak]

For example is it possible to create two different KfDef objects in different namespaces?

@danielezonca is it a requirement or a supported scenario? I am wondering if you can have multiple controllers supporting this - as you will end up with e.g. nb ctrl deployed for both namespaces but watching for the same resources cluster-wide.

[danielezonca]

@danielezonca - The implication of this point was pointing out the limitation that when using this PR/manifests, the KfDef can only be created in the namespace opendatahub. From my understanding, typically users are able to create a KfDef/ODH deployment in any namespace of their choosing, so I wanted to make sure reviewers/testers were aware of this limitation and look for suggestions on how to fix this.

Yes I think this usually is up to the user so we will probably need to find some alternative solution
Thanks for the clarification :)

@danielezonca is it a requirement or a supported scenario?

I think it is supported and expected to work but I will let others to confirm because I'm not 100% sure :)

[israel-hdez]

I was following instructions in enabling_ossm.md in a fresh cluster. It fails with the following status in the KfDef:

Status:
  Conditions:
    Last Update Time:  2023-06-07T23:29:05Z
    Reason:             (kubeflow.error): Code 500 with message: kfApp Apply failed for kustomize:  (kubeflow.error): Code 500 with message: error evaluating kustomization manifest for control-plane: error mapping rest config no matches for kind "ServiceMeshControlPlane" in version "maistra.io/v2"
    Status:            True
    Type:              Degraded
    Last Update Time:  2023-06-07T23:29:05Z
    Reason:            Kubeflow Deployment completed
    Status:            True
    Type:              Available

---

EDIT: So, for whatever reason, I was running createSubscription commands in different order. So, I ran first operator-sdk run bundle quay.io/cgarriso/opendatahub-operator-bundle:dev-0.0.2 --namespace openshift-operators --timeout 5m0s, then I ran createSubscription "servicemeshoperator". I ended up with the following message in the OS console:

So, for now, invoking operator-sdk run should be the last one to allow proper installation of the other things.

---

EDIT: After everything is installed and up, when trying to access the dashboard I just see a blank page. Authorino logs have:

{"level":"info","ts":1686182958.5667002,"logger":"authorino.service.auth","msg":"incoming authorization request","request id":"5be7790e-3986-9ff7-97d2-4ebbb4f2149b","object":{"source":{"address":{"Address":{"SocketAddress":{"address":"10.128.10.19","PortSpecifier":{"PortValue":34824}}}}},"destination":{"address":{"Address":{"SocketAddress":{"address":"10.128.8.105","PortSpecifier":{"PortValue":8443}}}},"principal":"apps-crc.testing"},"request":{"http":{"id":"5be7790e-3986-9ff7-97d2-4ebbb4f2149b","method":"GET","path":"/","host":"opendatahub.apps.ehernand-tsdemo.osapps.com","scheme":"https"}}}}
{"level":"info","ts":1686182958.5893471,"logger":"authorino.service.auth","msg":"outgoing authorization response","request id":"5be7790e-3986-9ff7-97d2-4ebbb4f2149b","authorized":false,"response":"PERMISSION_DENIED","object":{"code":7,"message":"Unauthorized"}}

Maybe the principal not matching the host in the request is an issue?

[bartoszmajsak]

Thanks for review @israel-hdez.

As for the authorino logs - I will have a look. Haven't seen this one before. Perhaps you could share a bit more details of what's your exact environment? That might help me reproducing it.

[israel-hdez]

Perhaps you could share a bit more details of what's your exact environment?

It was a fresh OSD cluster. I was following the instructions and had no luck.
When trying to access the dashboard I'm redirected fine to the authentication form, but once I authenticate and it redirects me back to the dashboard url, I get a blank page and those log lines appear.

[bartoszmajsak]

Update

With the 8ca7a6d we changed the approach of overwriting upstream settings (such as oauth-proxy container and related resources). Instead, it now uses service-mesh overlay which is patching manifests relevant to service mesh. In some cases, it will delete certain resources which are not relevant anymore. Other than that everything (resources and init-job) is the same.

In addition, odh-dashboard has now a dev overlay provided which limits resource consumption for certain deployments. This is particularly useful for constrained (namely local) environments such as crc.

How to use it

Sample `KfDef` manifest

apiVersion: kfdef.apps.kubeflow.org/v1
kind: KfDef
metadata:
 name: odh-mesh
spec:
 applications:
 - kustomizeConfig:
      parameters:
        - name: namespace
          value: istio-system
      repoRef:
        name: manifests
        path: service-mesh/control-plane
   name: control-plane
 - kustomizeConfig:
      overlays:
        - service-mesh
      repoRef:
        name: manifests
        path: odh-common
   name: odh-common
 - kustomizeConfig:
      overlays:
        - service-mesh
        - dev
      repoRef:
        name: manifests
        path: odh-dashboard
   name: odh-dashboard
 - kustomizeConfig:
      overlays:
        - service-mesh
      repoRef:
        name: manifests
        path: odh-notebook-controller
   name: odh-notebook-controller
 - kustomizeConfig:
      repoRef:
        name: manifests
        path: odh-project-controller
   name: odh-project-controller
 - kustomizeConfig:
      repoRef:
        name: manifests
        path: notebook-images
   name: notebook-images
 - kustomizeConfig:
      parameters:
        - name: namespace
          value: auth-provider
      repoRef:
        name: manifests
        path: service-mesh/authorino
   name: authorino
 repos:
 - name: manifests
   uri: https://github.com/maistra/odh-manifests/tarball/service-mesh-integration
 version: service-mesh-integration

Important

Loose ends

(pretty much all that is mentioned in the PR description), but worth noting are:

• we still have namespaces hardcoded for the init-job, but we working on the parametrized approach
• we are not able to $patch: delete certain resources (such as openshift routes), as they're not supported by kustomize - see related thread this has been done as part of init-job

About manifests structure

One important thing for all the manifest files which are going to be used in KfDef:

• regular manifests shouldn't use bases but resources instead in kustomization.yaml. The former is deprecated anyway, but the main point is that with overlays it results in unexpected errors.
• overlay kustomization.yaml files should be the only ones using bases to process and apply patches correctly. But if the point above is not fulfilled the operator will end up in recursive resolution of accumulated manifests and terminate with error.

Please take note of the comments - KfDef way of using kustomize is quite buggy and certain things were not trivial to make them work :)

[bartoszmajsak]

@harshad16 PTAL #818 (comment)

[shalberd]

For example is it possible to create two different KfDef objects in different namespaces?

@LaVLaS Someone in odh slack once told me this is not supported. I don't know why exactly, but I suppose having multiple instances of central software like notebook controller, odh notebook controller, dashboard could create problems. Of course, if that is so, then the question is why opendatahub-operator in namespace openshift-operators is not running in singleNamespace mode, but in mode "allNamespaces" when it comes to e.g. KfDef evaluation.

[harshad16]

Review in progress
a small change suggested for time being

[bartoszmajsak]

With c095a34 we do not have namespaces hardcoded anymore.

[bartoszmajsak]

In specific cluster setups (e.g. ROSA), the audience for token review is configured not to be kuberentes.default.svc. We're working on fixing it, but till then you can use the following workaround to patch AuthConfig:

TOKEN=YOUR_USER_TOKEN
ODH_NS=opendatahub
result=$(kubectl create -o jsonpath='{.status.audiences[0]}' -f -<<EOF
apiVersion: authentication.k8s.io/v1
kind: TokenReview
spec:
  token: "$TOKEN"
  audiences: []
EOF
)

kubectl patch authconfig odh-dashboard-protection -n ${ODH_NS} --type='json' -p="[{'op': 'replace', 'path': '/spec/identity/0/kubernetes/audiences', 'value': ['${result}']}]"

/cc @harshad16

Related ticket: https://issues.redhat.com/browse/OSSM-4356

[bartoszmajsak]

One downside of the current approach is that instead of applying patches for the SMCP we are installing a new control plane. This won't work for everyone - e.g. on ROSA you might need to set spec.security.indentity.type: ThirdParty.

We are changing this approach to apply patches that are relevant for us instead and starting from the assumption you have smcp deployed.

Related ticket: https://issues.redhat.com/browse/OSSM-4359

[bartoszmajsak]

In 313cba2 @israel-hdez brings basic support of ModelMesh

[cam-garrison]

In 312c623 we introduce migrating pre-existing data science projects into the mesh in our init-job.

[bartoszmajsak]

c99a385 changes the approach to patching SMCP instead of creating one. Therefore you should install SMCP for your particular cluster setup first (we provided basic one in the doc) before applying KfDef.

[bartoszmajsak]

f47da8f addresses a concern related to the use of a wildcard "*" in the hosts field of the ODH-dashboard's Gateway and VirtualService resources. By using the wildcard, traffic from unintended sources could potentially be routed to our services. This is now narrowed to the host of the dashboard instead.

[LaVLaS]

@bartoszmajsak This deployment of ServiceMesh with the dashboard worked as intended. I did have an issue with the dashboard being able to spawn a notebook but I haven't had a chance to check any logs to see where the issue is located. Outside of that we should be free to merge this into the feature branch and iterate from there for anyone needing to do a dev deployment of ODH w/ ServiceMesh

The current layout of the manifests, initJob and code changes provide a good template for what we need to port over to the new odh operator codebase.

[bartoszmajsak]

Thanks for validating it @LaVLaS.

I did have an issue with the dashboard being able to spawn a notebook but I haven't had a chance to check any logs to see where the issue is located.

We will have a look at this. Can you provide more details on how exactly you tried to spawn a notebook and what were the symptoms?

[bartoszmajsak]

@harshad16 I would like to "rebase and merge" this work, but since you said that your review is in progress I wanted to check if there's more feedback you could share.

[harshad16]

/lgtm

Thanks for waiting on my review
In the next pr , we can switch to image based on notebook-controller:
https://github.com/opendatahub-io/kubeflow/releases/tag/v1.6.1-3

[LaVLaS]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LaVLaS

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 [LaVLaS]

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