Merge pull request #309 from Jooho/cp_RHOAIENG-16070

[Cherry-Pick] update global ca bundle logic and storage-config logic
opendatahub-io · Nov 21, 2024 · 516e192 · 516e192
2 parents 8a19341 + b5de317
commit 516e192
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 39 deletions.
diff --git a/controllers/kserve_customcacert_controller.go b/controllers/kserve_customcacert_controller.go
@@ -18,6 +18,7 @@ package controllers
 import (
 	"context"
 	"reflect"
+	"strings"
 
 	"github.com/go-logr/logr"
 	"github.com/opendatahub-io/odh-model-controller/controllers/constants"
@@ -56,7 +57,7 @@ func (r *KServeCustomCACertReconciler) reconcileConfigMap(configmap *corev1.Conf
 		}
 		configmap = odhCustomCertConfigMap
 	}
-	odhCustomCertData = configmap.Data[constants.ODHCustomCACertFileName]
+	odhCustomCertData = strings.TrimSpace(configmap.Data[constants.ODHCustomCACertFileName])
 
 	// Create Desired resource
 	configData := map[string]string{kserveCustomCACertFileName: odhCustomCertData}

diff --git a/controllers/kserve_customcacert_controller_test.go b/controllers/kserve_customcacert_controller_test.go
@@ -18,22 +18,36 @@ package controllers
 import (
 	"context"
 	"reflect"
+	"strings"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/opendatahub-io/odh-model-controller/controllers/constants"
 	corev1 "k8s.io/api/core/v1"
 )
 
 const (
-	odhtrustedcabundleConfigMapUpdatedPath = "./testdata/configmaps/odh-trusted-ca-bundle-configmap-updated.yaml"
-	kservecustomcacertConfigMapUpdatedPath = "./testdata/configmaps/odh-kserve-custom-ca-cert-configmap-updated.yaml"
+	odhtrustedcabundleConfigMapUpdatedPath         = "./testdata/configmaps/odh-trusted-ca-bundle-configmap-updated.yaml"
+	kserveCustomCACustomBundleConfigMapUpdatedPath = "./testdata/configmaps/odh-kserve-custom-ca-cert-configmap-updated.yaml"
 )
 
 var _ = Describe("KServe Custom CA Cert ConfigMap controller", func() {
 	ctx := context.Background()
 
+	AfterEach(func() {
+		configmap := &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "odh-trusted-ca-bundle",
+				Namespace: "default",
+			},
+		}
+
+		Expect(cli.Delete(ctx, configmap)).Should(Succeed())
+	})
+
 	Context("when a configmap 'odh-trusted-ca-bundle' exists", func() {
 		It("should create a configmap that is for kserve custom ca cert", func() {
 			By("creating odh-trusted-ca-bundle configmap")
@@ -69,8 +83,10 @@ var _ = Describe("KServe Custom CA Cert ConfigMap controller", func() {
 			kserveCACertConfigmap, err := waitForConfigMap(cli, WorkingNamespace, constants.KServeCACertConfigMapName, 30, 1*time.Second)
 			Expect(err).NotTo(HaveOccurred())
 			expectedKserveCACertConfigmap := &corev1.ConfigMap{}
-			err = convertToStructuredResource(kservecustomcacertConfigMapUpdatedPath, expectedKserveCACertConfigmap)
+			err = convertToStructuredResource(kserveCustomCACustomBundleConfigMapUpdatedPath, expectedKserveCACertConfigmap)
 			Expect(err).NotTo(HaveOccurred())
+			// Trim out the last \n in the updated file
+			expectedKserveCACertConfigmap.Data["cabundle.crt"] = strings.TrimSpace(expectedKserveCACertConfigmap.Data["cabundle.crt"])
 
 			Expect(compareConfigMap(kserveCACertConfigmap, expectedKserveCACertConfigmap)).Should((BeTrue()))
 		})

diff --git a/controllers/storageconfig_controller.go b/controllers/storageconfig_controller.go
@@ -104,19 +104,18 @@ func (r *StorageSecretReconciler) reconcileSecret(secret *corev1.Secret,
 	odhCustomCertData := ""
 	odhGlobalCertConfigMap := &corev1.ConfigMap{}
 	err = r.Get(ctx, types.NamespacedName{
-		Name:      constants.ODHGlobalCertConfigMapName,
+		Name:      constants.KServeCACertConfigMapName,
 		Namespace: secret.Namespace,
 	}, odhGlobalCertConfigMap)
 
 	if err != nil {
 		if apierrs.IsNotFound(err) {
 			log.Info("unable to fetch the ODH Global Cert ConfigMap", "error", err)
-
 		} else {
 			return err
 		}
 	} else {
-		odhCustomCertData = odhGlobalCertConfigMap.Data[constants.ODHCustomCACertFileName]
+		odhCustomCertData = odhGlobalCertConfigMap.Data[constants.KServeCACertFileName]
 	}
 
 	// Generate desire Storage Config Secret

diff --git a/controllers/storageconfig_controller_test.go b/controllers/storageconfig_controller_test.go
@@ -34,7 +34,6 @@ import (
 const (
 	dataconnectionStringPath            = "./testdata/secrets/dataconnection-string.yaml"
 	storageconfigEncodedPath            = "./testdata/secrets/storageconfig-encoded.yaml"
-	storageconfigCertString             = "./testdata/secrets/storageconfig-cert-string.yaml"
 	storageconfigEncodedUnmanagedPath   = "./testdata/secrets/storageconfig-encoded-unmanaged.yaml"
 	storageconfigCertEncodedPath        = "./testdata/secrets/storageconfig-cert-encoded.yaml"
 	storageconfigUpdatedCertEncodedPath = "./testdata/secrets/storageconfig-updated-cert-encoded.yaml"
@@ -144,15 +143,15 @@ var _ = Describe("StorageConfig controller", func() {
 		})
 	})
 
-	Context("when a configmap 'odh-trusted-ca-bundle' exists or updates", func() {
+	Context("when a configmap 'odh-kserve-custom-ca-bundle' exists or updates", func() {
 		It("should add/update certificate keys into storage-config secret", func() {
 			dataconnectionStringSecret := &corev1.Secret{}
 
-			By("creating odh-trusted-ca-bundle configmap")
-			odhtrustedcabundleConfigMap := &corev1.ConfigMap{}
-			err := convertToStructuredResource(odhtrustedcabundleConfigMapPath, odhtrustedcabundleConfigMap)
+			By("creating odh-kserve-custom-ca-bundle configmap")
+			odhKserveCustomCABundleConfigmap := &corev1.ConfigMap{}
+			err := convertToStructuredResource(odhKserveCustomCABundleConfigMapPath, odhKserveCustomCABundleConfigmap)
 			Expect(err).NotTo(HaveOccurred())
-			Expect(cli.Create(ctx, odhtrustedcabundleConfigMap)).Should(Succeed())
+			Expect(cli.Create(ctx, odhKserveCustomCABundleConfigmap)).Should(Succeed())
 
 			By("creating dataconnection secret")
 			err = convertToStructuredResource(dataconnectionStringPath, dataconnectionStringSecret)
@@ -168,24 +167,28 @@ var _ = Describe("StorageConfig controller", func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(compareSecrets(storageconfigSecret, expectedStorageConfigSecret)).Should((BeTrue()))
 
-			By("updating odh-trusted-ca-bundle configmap")
-			updatedOdhtrustedcacertConfigMap := &corev1.ConfigMap{}
-			err = convertToStructuredResource(odhtrustedcabundleConfigMapUpdatedPath, updatedOdhtrustedcacertConfigMap)
+			By("updating odh-kserve-custom-ca-bundle configmap")
+			updatedOdhKserveCustomCABundleConfigmap := &corev1.ConfigMap{}
+			err = convertToStructuredResource(kserveCustomCACustomBundleConfigMapUpdatedPath, updatedOdhKserveCustomCABundleConfigmap)
 			Expect(err).NotTo(HaveOccurred())
-			Expect(cli.Update(ctx, updatedOdhtrustedcacertConfigMap)).Should(Succeed())
+			Expect(cli.Update(ctx, updatedOdhKserveCustomCABundleConfigmap)).Should(Succeed())
+
+			// Delete existing storage-config secret
+			// This will be done by kserve_customcacert_controller but for this test, it needs to be delete manully to update the storage-config
+			Expect(cli.Delete(ctx, storageconfigSecret)).Should(Succeed())
 
 			// Check updated storage-config secret
 			updatedStorageconfigSecret, err := waitForSecret(cli, WorkingNamespace, constants.DefaultStorageConfig, 30, 3*time.Second)
 			Expect(err).NotTo(HaveOccurred())
 			expectedUpdatedStorageConfigSecret := &corev1.Secret{}
 			err = convertToStructuredResource(storageconfigUpdatedCertEncodedPath, expectedUpdatedStorageConfigSecret)
 			Expect(err).NotTo(HaveOccurred())
-
+			
 			Expect(compareSecrets(updatedStorageconfigSecret, expectedUpdatedStorageConfigSecret)).Should((BeTrue()))
 		})
 	})
 
-	Context("when a configmap odh-trusted-ca-bundle does not exists", func() {
+	Context("when a configmap odh-kserve-custom-ca-bundle does not exists", func() {
 		It("should not return error", func() {
 			dataconnectionStringSecret := &corev1.Secret{}
 

diff --git a/controllers/suite_test.go b/controllers/suite_test.go
@@ -71,24 +71,25 @@ var (
 )
 
 const (
-	WorkingNamespace                = "default"
-	MonitoringNS                    = "monitoring-ns"
-	RoleBindingPath                 = "./testdata/results/model-server-ns-role.yaml"
-	ServingRuntimePath1             = "./testdata/deploy/test-openvino-serving-runtime-1.yaml"
-	KserveServingRuntimePath1       = "./testdata/deploy/kserve-openvino-serving-runtime-1.yaml"
-	ServingRuntimePath2             = "./testdata/deploy/test-openvino-serving-runtime-2.yaml"
-	InferenceService1               = "./testdata/deploy/openvino-inference-service-1.yaml"
-	InferenceServiceNoRuntime       = "./testdata/deploy/openvino-inference-service-no-runtime.yaml"
-	KserveInferenceServicePath1     = "./testdata/deploy/kserve-openvino-inference-service-1.yaml"
-	InferenceServiceConfigPath1     = "./testdata/configmaps/inferenceservice-config.yaml"
-	ExpectedRoutePath               = "./testdata/results/example-onnx-mnist-route.yaml"
-	ExpectedRouteNoRuntimePath      = "./testdata/results/example-onnx-mnist-no-runtime-route.yaml"
-	DSCIWithAuthorization           = "./testdata/dsci-with-authorino-enabled.yaml"
-	DSCIWithoutAuthorization        = "./testdata/dsci-with-authorino-missing.yaml"
-	KServeAuthorizationPolicy       = "./testdata/kserve-authorization-policy.yaml"
-	odhtrustedcabundleConfigMapPath = "./testdata/configmaps/odh-trusted-ca-bundle-configmap.yaml"
-	timeout                         = time.Second * 20
-	interval                        = time.Millisecond * 10
+	WorkingNamespace                     = "default"
+	MonitoringNS                         = "monitoring-ns"
+	RoleBindingPath                      = "./testdata/results/model-server-ns-role.yaml"
+	ServingRuntimePath1                  = "./testdata/deploy/test-openvino-serving-runtime-1.yaml"
+	KserveServingRuntimePath1            = "./testdata/deploy/kserve-openvino-serving-runtime-1.yaml"
+	ServingRuntimePath2                  = "./testdata/deploy/test-openvino-serving-runtime-2.yaml"
+	InferenceService1                    = "./testdata/deploy/openvino-inference-service-1.yaml"
+	InferenceServiceNoRuntime            = "./testdata/deploy/openvino-inference-service-no-runtime.yaml"
+	KserveInferenceServicePath1          = "./testdata/deploy/kserve-openvino-inference-service-1.yaml"
+	InferenceServiceConfigPath1          = "./testdata/configmaps/inferenceservice-config.yaml"
+	ExpectedRoutePath                    = "./testdata/results/example-onnx-mnist-route.yaml"
+	ExpectedRouteNoRuntimePath           = "./testdata/results/example-onnx-mnist-no-runtime-route.yaml"
+	DSCIWithAuthorization                = "./testdata/dsci-with-authorino-enabled.yaml"
+	DSCIWithoutAuthorization             = "./testdata/dsci-with-authorino-missing.yaml"
+	KServeAuthorizationPolicy            = "./testdata/kserve-authorization-policy.yaml"
+	odhtrustedcabundleConfigMapPath      = "./testdata/configmaps/odh-trusted-ca-bundle-configmap.yaml"
+	odhKserveCustomCABundleConfigMapPath = "./testdata/configmaps/odh-kserve-custom-ca-cert-configmap.yaml"
+	timeout                              = time.Second * 20
+	interval                             = time.Millisecond * 10
 )
 
 func init() {

diff --git a/go.mod b/go.mod
@@ -4,13 +4,15 @@ go 1.21
 
 require (
 	github.com/go-logr/logr v1.3.0
+	github.com/hashicorp/errwrap v1.1.0
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/kserve/kserve v0.12.1
 	github.com/kuadrant/authorino v0.15.0
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.30.0
 	github.com/opendatahub-io/model-registry v0.1.1
 	github.com/openshift/api v3.9.0+incompatible
+	github.com/pkg/errors v0.9.1
 	github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.64.1
 	github.com/tidwall/gjson v1.17.0
 	go.uber.org/zap v1.26.0
@@ -62,7 +64,6 @@ require (
 	github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect
 	github.com/googleapis/gax-go/v2 v2.12.0 // indirect
 	github.com/googleapis/google-cloud-go-testing v0.0.0-20210719221736-1c9a4c676720 // indirect
-	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/imdario/mergo v0.3.16 // indirect
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
@@ -74,7 +75,6 @@ require (
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/nxadm/tail v1.4.8 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
-	github.com/pkg/errors v0.9.1 // indirect
 	github.com/prometheus/client_golang v1.17.0 // indirect
 	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common v0.45.0 // indirect