feat: create project level authorino authconfig #64
Conversation
* Setup namespace annotations for gatewway/host information used by this namesapce * Generate host pattern based on openshift ingress configs AppDomain * Generate a default AuthConfig file using ServiceAccounts TokenReview process as auth * Rely on endpoint in `osh-model-controller` for anonymous access support releated-to: maistra#58
aslakknutsen
force-pushed
the
auth_config
branch
from
9715d38
to
fad7109
Compare
| @@ -5,5 +5,5 @@ resources: | |||
| - service.yaml | |||
| images: | |||
| - name: controller | |||
| newName: quay.io/maistra-dev/odh-project-controller | |||
| newName: quay.io/aslakknutsen/odh-project-controller | |||
There was a problem hiding this comment.
- nit: we should not forget to change it back
| @@ -136,3 +135,18 @@ func (r *OpenshiftServiceMeshReconciler) findIstioIngress(ctx context.Context) ( | |||
|
|
|||
| return routes, nil | |||
| } | |||
|
|
|||
| func (r *OpenshiftServiceMeshReconciler) findAppDomain(ctx context.Context) (string, error) { | |||
There was a problem hiding this comment.
Is fetching Ingress resource the only way to find it?
There was a problem hiding this comment.
Maybe not the only way to find it, but it's the controlling value location.
| func getAuthAudience() []string { | ||
| aud := getEnvOr(AuthAudience, "https://kubernetes.default.svc") | ||
| audiences := strings.Split(aud, ",") | ||
|
|
||
| for i := range audiences { | ||
| audiences[i] = strings.TrimSpace(audiences[i]) | ||
| } | ||
|
|
||
| return audiences | ||
| } | ||
|
|
There was a problem hiding this comment.
So this was introduced specifically to address the case when OpenDataHub runs e.g. on ROSA environment and we need to use third-party token issuers in Istio (so also needs this config change in the cluster: https://access.redhat.com/documentation/en-us/openshift_container_platform/4.8/html-single/service_mesh/index#ossm-install-rosa_ossm-create-smcp)
Won't we need it with the new approach?
There was a problem hiding this comment.
That would be up to the user if needed. Depends on the external server they are integrating this with. It's certainly not a install wide option anymore.
| if namespace.ObjectMeta.Annotations[AnnotationPublicGatewayExternalHost] != "" && | ||
| namespace.ObjectMeta.Annotations[AnnotationPublicGatewayInternalHost] != "" && | ||
| namespace.ObjectMeta.Annotations[AnnotationPublicGatewayName] != "" { | ||
| namespace.ObjectMeta.Annotations[AnnotationPublicGatewayName] != "" && | ||
| namespace.ObjectMeta.Annotations[AnnotationProjectModelGatewayHostPatternExternal] != "" && | ||
| namespace.ObjectMeta.Annotations[AnnotationProjectModelGatewayHostPatternInternal] != "" { |
There was a problem hiding this comment.
It's getting a bit hard to read now. Maybe we can have a func such as
func isAnnotated(objectMeta, annotations... string) bool
which will improve readability?
| authJSON: http://some-service?redirect_to={context.request.http.path} | ||
| unauthorized: | ||
| message: | ||
| value: "Unauthorized" | ||
| code: 403 | ||
| headers: | ||
| - name: Location | ||
| valueFrom: | ||
| authJSON: http://some-service?redirect_to={context.request.http.path} |
There was a problem hiding this comment.
Are we planning to change those authJSON values? How and from where should we take those?
There was a problem hiding this comment.
This is a bit of an open question. I think in the current form these are more "examples" then real values as they are dummy and the chosen auth form does not support a kind of "start flow" type thing.
This is now a User controlled AuthConfig. If they use some external auth server, they could choose to auto redirect there if the request is not auth to e.g. to start the oauth flow.
But maybe these should be commented out or moved to docs.
| } | ||
|
|
||
| authConfig.Spec.Identity[0].KubernetesAuth.Audiences = getAuthAudience() | ||
| authConfig.Spec.Identity[0].KubernetesAuth.Audiences = []string{namespace.Name + "-api"} |
There was a problem hiding this comment.
How would it work for e.g. project with only workbench enabled? At this point it expects the user to be able to "get" notebooks in this namespace.
It's a similar approach in other components IIRC. Do we foresee some granularity of the roles? Can you help me understand how is this supposed to replace it?
There was a problem hiding this comment.
This is all Host based.
Workbenchs + Notebooks are bound to the "*.opendatahub.openshift.ai" domain which is the openatahub level authconfig with oauth/single sign on configured. Which namespace they belong to doesn't matter.
Models are bound to a various of different domains, aka ".namespace.srv.cluster.local", ".namespace.apps.openshift.ai". These are configured in the project level authconfig.
In the current impl the ProjectLevel authconfig would still exist in the Project namespace if only workbenches are used, but the workbench is reached on different URL pattern so it won't be active.
osh-model-controllerfor anonymous access supportreleated-to: #58