fix: resolves ns in init-job #22

bartoszmajsak · 2023-06-28T09:32:05Z

looks up ns names (both app and istio) so we don't rely on hardcoded names anymore
fixes recreation of the secret
trim roles needed for the job and split them by to read/write Role resource files

so we do not hardcode them

cam-garrison

A couple questions but overall looks good. Have not tested on my end - let me know if you'd like me to and I can.

cam-garrison · 2023-06-28T14:47:58Z

service-mesh/control-plane/base/init-job-rbac/job-rolebinding.yaml

Can we remove the explicit references to istio-system here? They should be are overwritten by KfDef parameters regardless.

Good catch!

Fixed in 83ea2e6

cam-garrison · 2023-06-28T14:53:25Z

service-mesh/control-plane/base/init-job-rbac/resource-read-roles.yaml

-      - pods/log
-      - pods/exec


Are these not needed to ensure that istio-ingressgateway has secrets mounted and restart if it does not?

Funny enough the job seems to work but that's because of how the if was written :) I fixed both in 83ea2e6

removes ns from kustomize resources as it will be anyway replaced when running the job

* feat(mvp): enabling service mesh for ODH Co-authored-by: Bartosz Majsak <[email protected]> Co-authored-by: Cameron Garrison <[email protected]> Co-authored-by: Wambugu “Innocent” Kironji <[email protected]> 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. * chore: re-add mistakenly removed cert secret (maistra#11) re-add mistakenly removed cert secret * fix: move secret to correct kustomization section (maistra#14) move secret to correct kustomization section * update OWNERS, add aslak * fix: update jq download link (maistra#15) fix jq download link * use service-mesh-integration branch for kfdef * fix: corrects kfdef file name in the docs * fix(manifests): switches to use patches and overlays (maistra#18) Co-authored-by: Cameron Garrison <[email protected]> * fix: create hmac and token secrets as secrets rather than cfgmap (maistra#19) Change from creating hmac and token secrets for oauth2 secrets * chore(init-job): adds annotation to root namespace (maistra#21) * feat: adds service-mesh env variables They are optionally fetched from a config map, otherwise controller uses defaults * chore: creates service-mesh specific OWNERS file * fix: resolves ns in init-job (maistra#22) * fix: extracts port and raw host (maistra#23) * fix(job): defaults OAUTH_PORT to 443 if not defined in issuer * chore: removes obsolete route (maistra#24) * feat: basic support of ModelMesh (maistra#17) * feat: migrate existing DS project namespaces to enable SM (maistra#25) * add ds proj migration to init job * feat: patches SMCP instead of creating one (maistra#26) Co-authored-by: Cameron Garrison <[email protected]> * fix: specifies host in ODH dashboard gateway and virtualservice (maistra#27) --------- Co-authored-by: Bartosz Majsak <[email protected]> Co-authored-by: Edgar Hernández <[email protected]>

* feat(mvp): enabling service mesh for ODH Co-authored-by: Bartosz Majsak <[email protected]> Co-authored-by: Cameron Garrison <[email protected]> Co-authored-by: Wambugu “Innocent” Kironji <[email protected]> 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. * chore: re-add mistakenly removed cert secret (#11) re-add mistakenly removed cert secret * fix: move secret to correct kustomization section (#14) move secret to correct kustomization section * update OWNERS, add aslak * fix: update jq download link (#15) fix jq download link * use service-mesh-integration branch for kfdef * fix: corrects kfdef file name in the docs * fix(manifests): switches to use patches and overlays (#18) Co-authored-by: Cameron Garrison <[email protected]> * fix: create hmac and token secrets as secrets rather than cfgmap (#19) Change from creating hmac and token secrets for oauth2 secrets * chore(init-job): adds annotation to root namespace (#21) * feat: adds service-mesh env variables They are optionally fetched from a config map, otherwise controller uses defaults * chore: creates service-mesh specific OWNERS file * fix: resolves ns in init-job (#22) * fix: extracts port and raw host (#23) * fix(job): defaults OAUTH_PORT to 443 if not defined in issuer * chore: removes obsolete route (#24) * feat: basic support of ModelMesh (#17) * feat: migrate existing DS project namespaces to enable SM (#25) * add ds proj migration to init job * feat: patches SMCP instead of creating one (#26) Co-authored-by: Cameron Garrison <[email protected]> * fix: specifies host in ODH dashboard gateway and virtualservice (#27) --------- Co-authored-by: Bartosz Majsak <[email protected]> Co-authored-by: Edgar Hernández <[email protected]>

bartoszmajsak added 3 commits June 27, 2023 20:51

fix: resolves ns in init-job

b897c78

so we do not hardcode them

chore: simplifies roles needed for the job to run

76d0b8c

fix: recreates secret if it exists

17d62c8

bartoszmajsak requested a review from cam-garrison June 28, 2023 09:32

cam-garrison approved these changes Jun 28, 2023

View reviewed changes

fix: properly checks presence of mounted secrets

83ea2e6

removes ns from kustomize resources as it will be anyway replaced when running the job

bartoszmajsak merged commit c095a34 into service-mesh-integration Jun 28, 2023

bartoszmajsak deleted the resolve_ns_in_init_job branch June 28, 2023 18:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: resolves ns in init-job #22

fix: resolves ns in init-job #22

bartoszmajsak commented Jun 28, 2023

cam-garrison left a comment

cam-garrison Jun 28, 2023

bartoszmajsak Jun 28, 2023

bartoszmajsak Jun 28, 2023

cam-garrison Jun 28, 2023

bartoszmajsak Jun 28, 2023

fix: resolves ns in init-job #22

fix: resolves ns in init-job #22

Conversation

bartoszmajsak commented Jun 28, 2023

cam-garrison left a comment

Choose a reason for hiding this comment

cam-garrison Jun 28, 2023

Choose a reason for hiding this comment

bartoszmajsak Jun 28, 2023

Choose a reason for hiding this comment

bartoszmajsak Jun 28, 2023

Choose a reason for hiding this comment

cam-garrison Jun 28, 2023

Choose a reason for hiding this comment

bartoszmajsak Jun 28, 2023

Choose a reason for hiding this comment