Skip to content

Commit

Permalink
feat: set domain automatically in Openshift cluster, fixes RHOAIENG-5…
Browse files Browse the repository at this point in the history
…123 (opendatahub-io#115)

* feat: set domain automatically in Openshift cluster, fixes RHOAIENG-5123

* fix: add check for non-empty domain
  • Loading branch information
dhirajsb authored Jul 12, 2024
1 parent 4615648 commit 15a9698
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ $(ENVTEST): $(LOCALBIN)
.PHONY: certificates
certificates:
# generate TLS certs
scripts/generate_certs.sh $(DOMAIN)
scripts/generate_certs.sh $(or $(DOMAIN),$(shell oc get ingresses.config/cluster -o jsonpath='{.spec.domain}'))
# create secrets from TLS certs
$(KUBECTL) create secret -n istio-system generic modelregistry-sample-rest-credential \
--from-file=tls.key=certs/modelregistry-sample-rest.domain.key \
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ If Authorino provider is from a non Open Data Hub cluster, configure its selecto
To use the Istio model registry samples the following configuration data is needed in the [istio.env](config/samples/istio/components/istio.env) file:

* AUTH_PROVIDER - name of the authorino external auth provider configured in the Istio control plane (defaults to `opendatahub-auth-provider` for Open Data Hub data science cluster with OpenShift Service Mesh enabled).
* DOMAIN - hostname domain suffix for gateway endpoints.
* DOMAIN - hostname domain suffix for gateway endpoints. This field is optional in an OpenShift cluster and set automatically if left empty.
This depends upon your cluster's external load balancer config. In OpenShift clusters, it can be obtained with the command:
```shell
oc get ingresses.config/cluster -o jsonpath='{.spec.domain}'
Expand Down
8 changes: 5 additions & 3 deletions api/v1alpha1/modelregistry_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,12 @@ type ServerConfig struct {
}

type GatewayConfig struct {
//+kubebuilder:required
//+optional

// Domain name for Gateway configuration
Domain string `json:"domain"`
// Domain name for Gateway configuration.
// If not provided, it is set automatically using model registry operator env variable DEFAULT_DOMAIN.
// If the env variable is not set, it is set to the OpenShift `cluster` ingress domain in an OpenShift cluster.
Domain string `json:"domain,omitempty"`

//+kubebuilder:default=ingressgateway

Expand Down
5 changes: 5 additions & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
_ "k8s.io/client-go/plugin/pkg/client/auth"

oapi "github.com/openshift/api"
oapiconfig "github.com/openshift/api/config/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
Expand All @@ -53,12 +54,14 @@ var (
const (
EnableWebhooks = "ENABLE_WEBHOOKS"
CreateAuthResources = "CREATE_AUTH_RESOURCES"
DefaultDomain = "DEFAULT_DOMAIN"
)

func init() {
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
// openshift scheme
utilruntime.Must(oapi.Install(scheme))
utilruntime.Must(oapiconfig.Install(scheme))
// authorino scheme
utilruntime.Must(authorino.AddToScheme(scheme))
// istio security scheme
Expand Down Expand Up @@ -154,6 +157,7 @@ func main() {

enableWebhooks := os.Getenv(EnableWebhooks) != "false"
createAuthResources := os.Getenv(CreateAuthResources) != "false"
defaultDomain := os.Getenv(DefaultDomain)

if err = (&controller.ModelRegistryReconciler{
Client: client,
Expand All @@ -166,6 +170,7 @@ func main() {
HasIstio: hasAuthorino && hasIstio,
Audiences: tokenReview.Status.Audiences,
CreateAuthResources: createAuthResources,
DefaultDomain: defaultDomain,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "ModelRegistry")
os.Exit(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ spec:
description: Maistra/OpenShift Servicemesh control plane name
type: string
domain:
description: Domain name for Gateway configuration
description: Domain name for Gateway configuration. If not
provided, it is set automatically using model registry operator
env variable DEFAULT_DOMAIN. If the env variable is not
set, it is set to the OpenShift `cluster` ingress domain
in an OpenShift cluster.
type: string
grpc:
description: gRPC gateway server config
Expand Down Expand Up @@ -317,7 +321,6 @@ spec:
type: object
type: object
required:
- domain
- grpc
- rest
type: object
Expand Down
2 changes: 2 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ spec:
value: "false"
- name: CREATE_AUTH_RESOURCES
value: "true"
- name: DEFAULT_DOMAIN
value: ""
securityContext:
allowPrivilegeEscalation: false
capabilities:
Expand Down
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ rules:
- patch
- update
- watch
- apiGroups:
- config.openshift.io
resources:
- ingresses
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
2 changes: 1 addition & 1 deletion config/samples/istio/components/istio.env
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
AUTH_PROVIDER=opendatahub-auth-provider
DOMAIN=my-domain
DOMAIN=
ISTIO_INGRESS=ingressgateway
REST_CREDENTIAL_NAME=modelregistry-sample-rest-credential
GRPC_CREDENTIAL_NAME=modelregistry-sample-grpc-credential
46 changes: 45 additions & 1 deletion internal/controller/modelregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
authorino "github.com/kuadrant/authorino/api/v1beta2"
modelregistryv1alpha1 "github.com/opendatahub-io/model-registry-operator/api/v1alpha1"
"github.com/opendatahub-io/model-registry-operator/internal/controller/config"
configv1 "github.com/openshift/api/config/v1"
routev1 "github.com/openshift/api/route/v1"
userv1 "github.com/openshift/api/user/v1"
networking "istio.io/client-go/pkg/apis/networking/v1beta1"
Expand Down Expand Up @@ -69,6 +70,7 @@ type ModelRegistryReconciler struct {
HasIstio bool
Audiences []string
CreateAuthResources bool
DefaultDomain string
}

// Reconcile is part of the main kubernetes reconciliation loop which aims to
Expand Down Expand Up @@ -299,6 +301,7 @@ func (r *ModelRegistryReconciler) GetRegistryForRoute(ctx context.Context, objec
// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch
// +kubebuilder:rbac:groups=core,resources=services;serviceaccounts,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=config.openshift.io,resources=ingresses,verbs=get;list;watch
// +kubebuilder:rbac:groups=route.openshift.io,resources=routes,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=route.openshift.io,resources=routes/custom-host,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=user.openshift.io,resources=groups,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -598,7 +601,21 @@ func (r *ModelRegistryReconciler) handleGatewayRoute(ctx context.Context, params

func (r *ModelRegistryReconciler) createOrUpdateGateway(ctx context.Context, params *ModelRegistryParams,
registry *modelregistryv1alpha1.ModelRegistry, templateName string) (result OperationResult, err error) {

result = ResourceUnchanged

// autoconfigure domain if needed
domainUpdated := false
if len(registry.Spec.Istio.Gateway.Domain) == 0 {
err = r.setClusterDomain(ctx, registry)
if err != nil {
return result, err
}
// update current reconcile spec
params.Spec = registry.Spec
domainUpdated = true
}

var gateway networking.Gateway
if err = r.Apply(params, templateName, &gateway); err != nil {
return result, err
Expand All @@ -611,7 +628,12 @@ func (r *ModelRegistryReconciler) createOrUpdateGateway(ctx context.Context, par
if err != nil {
return result, err
}
return result, nil

if !domainUpdated {
return result, nil
} else {
return ResourceUpdated, nil
}
}

func (r *ModelRegistryReconciler) createOrUpdateAuthConfig(ctx context.Context, params *ModelRegistryParams,
Expand Down Expand Up @@ -961,3 +983,25 @@ func (r *ModelRegistryReconciler) logResultAsEvent(registry *modelregistryv1alph
// ignore
}
}

func (r *ModelRegistryReconciler) setClusterDomain(ctx context.Context, registry *modelregistryv1alpha1.ModelRegistry) (err error) {
if len(r.DefaultDomain) != 0 {
registry.Spec.Istio.Gateway.Domain = r.DefaultDomain
} else if r.IsOpenShift {
ingress := configv1.Ingress{}
namespacedName := types.NamespacedName{Name: "cluster"}
err = r.Client.Get(context.Background(), namespacedName, &ingress)
if err != nil {
return err
}

// remember default domain for next time
r.DefaultDomain = ingress.Spec.Domain
registry.Spec.Istio.Gateway.Domain = r.DefaultDomain
} else {
return fmt.Errorf("model registry %s is missing gateway domain and default domain is not configured",
registry.Name)
}
// update domain in model registry resource
return r.Client.Update(ctx, registry)
}
4 changes: 4 additions & 0 deletions scripts/generate_certs.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
DOMAIN=$1
if [ -z ${DOMAIN} ]; then
echo "Missing command line parameter for certificate domain"
exit 1
fi
mkdir -p certs
# create CA cert
openssl req -x509 -sha256 -nodes -days 365 -newkey rsa:2048 -subj "/O=modelregistry Inc./CN=$DOMAIN" -keyout certs/domain.key -out certs/domain.crt
Expand Down

0 comments on commit 15a9698

Please sign in to comment.