fix(isvc): post testing (#603)

* fix(isvc): bugs from local testing Signed-off-by: Alessio Pragliola <[email protected]> * chore(isvc): nit rename skip tls verify var in tests Signed-off-by: Alessio Pragliola <[email protected]> * fix(isvc): wrongly overriding the custom http client config Signed-off-by: Alessio Pragliola <[email protected]> * feat(isvc): make skip tls verify controller wise Signed-off-by: Alessio Pragliola <[email protected]> --------- Signed-off-by: Alessio Pragliola <[email protected]>
kubeflow · Dec 3, 2024 · 3eb4e9c · 3eb4e9c
1 parent d816da7
commit 3eb4e9c
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 10 deletions.
diff --git a/pkg/inferenceservice-controller/controller.go b/pkg/inferenceservice-controller/controller.go
@@ -2,6 +2,7 @@ package inferenceservicecontroller
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
 	"net/http"
 
@@ -18,37 +19,59 @@ import (
 
 type InferenceServiceController struct {
 	client                        client.Client
-	customHTTPClient              *http.Client
+	httpClient                    *http.Client
 	log                           logr.Logger
 	bearerToken                   string
 	inferenceServiceIDLabel       string
 	registeredModelIDLabel        string
 	modelVersionIDLabel           string
 	modelRegistryNamespaceLabel   string
 	modelRegistryNameLabel        string
-	modelRegistryURLLabel         string
+	modelRegistryURLAnnotation    string
 	modelRegistryFinalizer        string
 	defaultModelRegistryNamespace string
 }
 
-func NewInferenceServiceController(client client.Client, log logr.Logger, bearerToken, isIDLabel, regModelIDLabel, modelVerIDLabel, mrNamespaceLabel, mrNameLabel, mrURLLabel, mrFinalizer, defaultMRNamespace string) *InferenceServiceController {
+func NewInferenceServiceController(
+	client client.Client,
+	log logr.Logger,
+	skipTLSVerify bool,
+	bearerToken,
+	isIDLabel,
+	regModelIDLabel,
+	modelVerIDLabel,
+	mrNamespaceLabel,
+	mrNameLabel,
+	mrURLAnnotation,
+	mrFinalizer,
+	defaultMRNamespace string,
+) *InferenceServiceController {
+	httpClient := http.DefaultClient
+
+	if skipTLSVerify {
+		httpClient.Transport = &http.Transport{
+			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+		}
+	}
+
 	return &InferenceServiceController{
 		client:                        client,
+		httpClient:                    httpClient,
 		log:                           log,
 		bearerToken:                   bearerToken,
 		inferenceServiceIDLabel:       isIDLabel,
 		registeredModelIDLabel:        regModelIDLabel,
 		modelVersionIDLabel:           modelVerIDLabel,
 		modelRegistryNamespaceLabel:   mrNamespaceLabel,
 		modelRegistryNameLabel:        mrNameLabel,
-		modelRegistryURLLabel:         mrURLLabel,
+		modelRegistryURLAnnotation:    mrURLAnnotation,
 		modelRegistryFinalizer:        mrFinalizer,
 		defaultModelRegistryNamespace: defaultMRNamespace,
 	}
 }
 
 func (r *InferenceServiceController) OverrideHTTPClient(client *http.Client) {
-	r.customHTTPClient = client
+	r.httpClient = client
 }
 
 // Reconcile performs the reconciliation of the model registry based on Kubeflow InferenceService CRs
@@ -80,14 +103,20 @@ func (r *InferenceServiceController) Reconcile(ctx context.Context, req ctrl.Req
 	registeredModelId, okRegisteredModelId := isvc.Labels[r.registeredModelIDLabel]
 	modelVersionId := isvc.Labels[r.modelVersionIDLabel]
 	mrName, okMrName := isvc.Labels[r.modelRegistryNameLabel]
-	mrUrl := isvc.Labels[r.modelRegistryURLLabel]
+	mrUrl, okMrUrl := isvc.Annotations[r.modelRegistryURLAnnotation]
 
-	if !okMrIsvcId && !okRegisteredModelId && !okMrName {
+	if !okMrIsvcId && !okRegisteredModelId {
 		// Early check: no model registry specific labels set in the ISVC, ignore the CR
 		log.Error(fmt.Errorf("missing model registry specific label, unable to link ISVC to Model Registry, skipping InferenceService"), "Stop ModelRegistry InferenceService reconciliation")
 		return ctrl.Result{}, nil
 	}
 
+	if !okMrName && !okMrUrl {
+		// Early check: it's required to have the model registry name or url set in the ISVC
+		log.Error(fmt.Errorf("missing model registry name or url, unable to link ISVC to Model Registry, skipping InferenceService"), "Stop ModelRegistry InferenceService reconciliation")
+		return ctrl.Result{}, nil
+	}
+
 	if mrNSFromISVC, ok := isvc.Labels[r.modelRegistryNamespaceLabel]; ok {
 		mrNamespace = mrNSFromISVC
 	}
@@ -210,7 +239,7 @@ func (r *InferenceServiceController) initModelRegistryService(ctx context.Contex
 	}
 
 	cfg := &openapi.Configuration{
-		HTTPClient: r.customHTTPClient,
+		HTTPClient: r.httpClient,
 		Servers: openapi.ServerConfigurations{
 			{
 				URL: url,

diff --git a/pkg/inferenceservice-controller/suite_test.go b/pkg/inferenceservice-controller/suite_test.go
@@ -41,7 +41,8 @@ const (
 	modelVersionIDLabel     = "modelregistry.kubeflow.org/model-version-id"
 	namespaceLabel          = "modelregistry.kubeflow.org/namespace"
 	nameLabel               = "modelregistry.kubeflow.org/name"
-	urlLabel                = "modelregistry.kubeflow.org/url"
+	skipTLSVerify           = true
+	urlAnnotation           = "modelregistry.kubeflow.org/url"
 	finalizerLabel          = "modelregistry.kubeflow.org/finalizer"
 	defaultNamespace        = "default"
 	accessToken             = ""
@@ -135,13 +136,14 @@ var _ = BeforeSuite(func() {
 	inferenceServiceController := inferenceservicecontroller.NewInferenceServiceController(
 		cli,
 		ctrl.Log.WithName("controllers").WithName("ModelRegistry-InferenceService-Controller"),
+		skipTLSVerify,
 		accessToken,
 		inferenceServiceIDLabel,
 		registeredModelIDLabel,
 		modelVersionIDLabel,
 		namespaceLabel,
 		nameLabel,
-		urlLabel,
+		urlAnnotation,
 		finalizerLabel,
 		defaultNamespace,
 	)