diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go index d8922956f..de8aff7de 100644 --- a/api/v1alpha1/dspipeline_types.go +++ b/api/v1alpha1/dspipeline_types.go @@ -108,6 +108,10 @@ type APIServer struct { // provide a PEM formatted CA bundle to be injected into the DSP // server pod to trust this connection. CA Bundle should be provided // as values within configmaps, mapped to keys. + // + // Note that if a global cert via ODH or the User is provided in this DSPA's + // namespace with the name "odh-trusted-ca-bundle", then that configmap + // is automatically used instead, and "caBundle" is ignored. CABundle *CABundle `json:"cABundle,omitempty"` } @@ -169,6 +173,19 @@ type MlPipelineUI struct { type Database struct { *MariaDB `json:"mariaDB,omitempty"` *ExternalDB `json:"externalDB,omitempty"` + + // +kubebuilder:validation:Optional + // CustomExtraParams allow users to further customize the sql dsn parameters used by the Pipeline Server + // when opening a connection with the Database. + // ref: https://github.com/go-sql-driver/mysql?tab=readme-ov-file#dsn-data-source-name + // + // Value must be a JSON string. For example, to disable tls for Pipeline Server DB connection + // the user can provide a string: {"tls":"true"} + // + // If updating post DSPA deployment, then a manual restart of the pipeline server pod will be required + // so the new configmap may be consumed. + CustomExtraParams *string `json:"customExtraParams,omitempty"` + // Default: false // +kubebuilder:default:=false // +kubebuilder:validation:Optional diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 864bddb54..185a264fa 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -230,6 +230,11 @@ func (in *Database) DeepCopyInto(out *Database) { *out = new(ExternalDB) (*in).DeepCopyInto(*out) } + if in.CustomExtraParams != nil { + in, out := &in.CustomExtraParams, &out.CustomExtraParams + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Database. diff --git a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml index 5bbb80be5..ffb5567de 100644 --- a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml +++ b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml @@ -62,11 +62,15 @@ spec: description: 'Default: true' type: boolean cABundle: - description: If the Object store/DB is behind a TLS secured connection + description: "If the Object store/DB is behind a TLS secured connection that is unrecognized by the host OpenShift/K8s cluster, then you can provide a PEM formatted CA bundle to be injected into the DSP server pod to trust this connection. CA Bundle should - be provided as values within configmaps, mapped to keys. + be provided as values within configmaps, mapped to keys. \n + Note that if a global cert via ODH or the User is provided in + this DSPA's namespace with the name \"odh-trusted-ca-bundle\", + then that configmap is automatically used instead, and \"caBundle\" + is ignored." properties: configMapKey: description: Key should map to a CA bundle. The key is also @@ -177,6 +181,16 @@ spec: DS Pipelines metadata tracking. Specify either the default MariaDB deployment, or configure your own External SQL DB. properties: + customExtraParams: + description: "CustomExtraParams allow users to further customize + the sql dsn parameters used by the Pipeline Server when opening + a connection with the Database. ref: https://github.com/go-sql-driver/mysql?tab=readme-ov-file#dsn-data-source-name + \n Value must be a JSON string. For example, to disable tls + for Pipeline Server DB connection the user can provide a string: + {\"tls\":\"true\"} \n If updating post DSPA deployment, then + a manual restart of the pipeline server pod will be required + so the new configmap may be consumed." + type: string disableHealthCheck: default: false description: 'Default: false' diff --git a/config/internal/apiserver/deployment.yaml.tmpl b/config/internal/apiserver/deployment.yaml.tmpl index ae0d064f4..3e00ec6aa 100644 --- a/config/internal/apiserver/deployment.yaml.tmpl +++ b/config/internal/apiserver/deployment.yaml.tmpl @@ -155,6 +155,9 @@ spec: {{ end }} {{ if or .APIServer.EnableSamplePipeline .APIServer.CABundle }} volumeMounts: + - name: server-config + mountPath: /config/config.json + subPath: config.json {{ if .APIServer.EnableSamplePipeline }} - name: sample-config mountPath: /config/sample_config.json @@ -220,6 +223,9 @@ spec: - name: proxy-tls secret: secretName: ds-pipelines-proxy-tls-{{.Name}} + - name: server-config + configMap: + name: pipeline-server-config-{{.Name}} {{ if .APIServer.CABundle }} - name: ca-bundle configMap: diff --git a/config/internal/apiserver/server-config.yaml.tmpl b/config/internal/apiserver/server-config.yaml.tmpl new file mode 100644 index 000000000..f90f27f36 --- /dev/null +++ b/config/internal/apiserver/server-config.yaml.tmpl @@ -0,0 +1,18 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: pipeline-server-config-{{.Name}} + namespace: {{.Namespace}} + labels: + app: {{.APIServerDefaultResourceName}} + component: data-science-pipelines +data: + config.json: | + { + "DBConfig": { + "DriverName": "mysql", + "ConMaxLifeTime": "120s", + "ExtraParams": {{ .DBConnection.ExtraParams }} + }, + "InitConnectionTimeout": "6m" + } diff --git a/config/overlays/make-deploy/kustomization.yaml b/config/overlays/make-deploy/kustomization.yaml index 7814f52a5..f04e186c2 100644 --- a/config/overlays/make-deploy/kustomization.yaml +++ b/config/overlays/make-deploy/kustomization.yaml @@ -1,6 +1,6 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization -namespace: odh-applications +namespace: opendatahub resources: - ../../base patchesStrategicMerge: diff --git a/controllers/apiserver.go b/controllers/apiserver.go index 4e12b3967..b22a3be43 100644 --- a/controllers/apiserver.go +++ b/controllers/apiserver.go @@ -28,6 +28,7 @@ const apiServerDefaultResourceNamePrefix = "ds-pipeline-" var apiServerTemplates = []string{ "apiserver/artifact_script.yaml.tmpl", + "apiserver/server-config.yaml.tmpl", "apiserver/role_ds-pipeline.yaml.tmpl", "apiserver/role_pipeline-runner.yaml.tmpl", "apiserver/role_ds-pipeline-user-access.yaml.tmpl", diff --git a/controllers/config/defaults.go b/controllers/config/defaults.go index b13ef3c78..9a16dcf2a 100644 --- a/controllers/config/defaults.go +++ b/controllers/config/defaults.go @@ -27,13 +27,23 @@ const ( DefaultImageValue = "MustSetInConfig" APIServerPiplinesCABundleMountPath = "/etc/pki/tls/certs" PiplinesCABundleMountPath = "/etc/pki/tls/certs" - MLPipelineUIConfigMapPrefix = "ds-pipeline-ui-configmap-" - ArtifactScriptConfigMapNamePrefix = "ds-pipeline-artifact-script-" - ArtifactScriptConfigMapKey = "artifact_script" - DSPServicePrefix = "ds-pipeline" - DBSecretNamePrefix = "ds-pipeline-db-" - DBSecretKey = "password" + // GlobalCaBundleConfigMapName key and label values are a contract with + // ODH Platform https://github.com/opendatahub-io/architecture-decision-records/pull/28 + GlobalCaBundleConfigMapName = "odh-trusted-ca-bundle" + + // GlobalCaBundleConfigMapKey is the key provided by the configmap created via OCP Cluster Network Operator + // https://docs.openshift.com/container-platform/4.14/networking/configuring-a-custom-pki.html#certificate-injection-using-operators_configuring-a-custom-pki + GlobalCaBundleConfigMapKey = "ca-bundle.crt" + + MLPipelineUIConfigMapPrefix = "ds-pipeline-ui-configmap-" + ArtifactScriptConfigMapNamePrefix = "ds-pipeline-artifact-script-" + ArtifactScriptConfigMapKey = "artifact_script" + DSPServicePrefix = "ds-pipeline" + + DBSecretNamePrefix = "ds-pipeline-db-" + DBSecretKey = "password" + DBDefaultExtraParams = "{\"tls\":\"%t\"}" MariaDBName = "mlpipeline" MariaDBHostPrefix = "mariadb" diff --git a/controllers/database.go b/controllers/database.go index 33a083a53..92c7bd0cd 100644 --- a/controllers/database.go +++ b/controllers/database.go @@ -17,12 +17,18 @@ package controllers import ( "context" + cryptoTls "crypto/tls" + "crypto/x509" "database/sql" b64 "encoding/base64" "fmt" + "github.com/go-logr/logr" + "github.com/go-sql-driver/mysql" _ "github.com/go-sql-driver/mysql" dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" "github.com/opendatahub-io/data-science-pipelines-operator/controllers/config" + "k8s.io/apimachinery/pkg/util/json" + "os" ) const dbSecret = "mariadb/secret.yaml.tmpl" @@ -35,22 +41,115 @@ var dbTemplates = []string{ dbSecret, } -// extract to var for mocking in testing -var ConnectAndQueryDatabase = func(host, port, username, password, dbname string) bool { +func tLSClientConfig(pem []byte) (*cryptoTls.Config, error) { + rootCertPool := x509.NewCertPool() + + if f := os.Getenv("SSL_CERT_FILE"); f != "" { + data, err := os.ReadFile(f) + if err == nil { + rootCertPool.AppendCertsFromPEM(data) + } + } + + if ok := rootCertPool.AppendCertsFromPEM(pem); !ok { + return nil, fmt.Errorf("error parsing CA Certificate, ensure provided certs are in valid PEM format") + } + tlsConfig := &cryptoTls.Config{ + RootCAs: rootCertPool, + } + return tlsConfig, nil +} + +func createMySQLConfig(user, password string, mysqlServiceHost string, + mysqlServicePort string, dbName string, mysqlExtraParams map[string]string) *mysql.Config { + + params := map[string]string{ + "charset": "utf8", + "parseTime": "True", + "loc": "Local", + } + + for k, v := range mysqlExtraParams { + params[k] = v + } + + return &mysql.Config{ + User: user, + Passwd: password, + Net: "tcp", + Addr: fmt.Sprintf("%s:%s", mysqlServiceHost, mysqlServicePort), + Params: params, + DBName: dbName, + AllowNativePasswords: true, + } +} + +var ConnectAndQueryDatabase = func( + host string, + log logr.Logger, + port, username, password, dbname, tls string, + pemCerts []byte, + extraParams map[string]string) (bool, error) { + + mysqlConfig := createMySQLConfig( + username, + password, + host, + port, + "", + extraParams, + ) + // Create a context with a timeout of 1 second ctx, cancel := context.WithTimeout(context.Background(), config.DefaultDBConnectionTimeout) defer cancel() - connectionString := fmt.Sprintf("%s:%s@tcp(%s:%s)/%s", username, password, host, port, dbname) - db, err := sql.Open("mysql", connectionString) + var tlsConfig *cryptoTls.Config + switch tls { + case "false", "": + // don't set anything + case "true": + if len(pemCerts) != 0 { + var err error + tlsConfig, err = tLSClientConfig(pemCerts) + if err != nil { + log.Info(fmt.Sprintf("Encountered error when processing custom ca bundle, Error: %v", err)) + return false, err + } + } + case "skip-verify", "preferred": + tlsConfig = &cryptoTls.Config{InsecureSkipVerify: true} + default: + // Unknown config, default to don't set anything + } + + // Only register tls config in the case of: "true", "skip-verify", "preferred" + if tlsConfig != nil { + err := mysql.RegisterTLSConfig("custom", tlsConfig) + // If ExtraParams{"tls": ".."} is set, that takes precedent over mysqlConfig.TLSConfig + // so we need to make sure we're setting our tls config to be used instead if it exists + if _, ok := mysqlConfig.Params["tls"]; ok { + mysqlConfig.Params["tls"] = "custom" + } + // Just to be safe, we also set it here, fallback from mysqlConfig.Params["tls"] not being set + mysqlConfig.TLSConfig = "custom" + if err != nil { + return false, err + } + } + + db, err := sql.Open("mysql", mysqlConfig.FormatDSN()) if err != nil { - return false + return false, err } defer db.Close() testStatement := "SELECT 1;" _, err = db.QueryContext(ctx, testStatement) - return err == nil + if err != nil { + return false, err + } + return true, nil } func (r *DSPAReconciler) isDatabaseAccessible(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication, @@ -72,16 +171,50 @@ func (r *DSPAReconciler) isDatabaseAccessible(ctx context.Context, dsp *dspav1al } decodePass, _ := b64.StdEncoding.DecodeString(params.DBConnection.Password) - dbHealthCheckPassed := ConnectAndQueryDatabase(params.DBConnection.Host, + + var extraParamsJson map[string]string + err := json.Unmarshal([]byte(params.DBConnection.ExtraParams), &extraParamsJson) + if err != nil { + log.Info(fmt.Sprintf("Could not parse tls config in ExtraParams, if setting CustomExtraParams, ensure the JSON string is well-formed. Error: %v", err)) + return false + } + + // tls can be true, false, skip-verify, preferred + // we default to true if it's an externalDB, false otherwise + // (if not specified via CustomExtraParams) + tls := "false" + if usingExternalDB { + tls = "true" + } + + // Override tls with the value in ExtraParams, if specified + // If users have specified a CustomExtraParams field, have to + // check for "tls" existence because users may choose to leave + // out the "tls" param + if val, ok := extraParamsJson["tls"]; ok { + tls = val + } + + dbHealthCheckPassed, err := ConnectAndQueryDatabase( + params.DBConnection.Host, + log, params.DBConnection.Port, params.DBConnection.Username, string(decodePass), - params.DBConnection.DBName) + params.DBConnection.DBName, + tls, + params.APICustomPemCerts, + extraParamsJson, + ) + if err != nil { + log.Info(fmt.Sprintf("Unable to connect to Database: %v", err)) + return false + } + if dbHealthCheckPassed { log.Info("Database Health Check Successful") - } else { - log.Info("Unable to connect to Database") } + return dbHealthCheckPassed } diff --git a/controllers/dspipeline_controller.go b/controllers/dspipeline_controller.go index d92275ab7..f32255a5b 100644 --- a/controllers/dspipeline_controller.go +++ b/controllers/dspipeline_controller.go @@ -516,6 +516,38 @@ func (r *DSPAReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&rbacv1.Role{}). Owns(&rbacv1.RoleBinding{}). Owns(&routev1.Route{}). + // Watch for global ca bundle, if one is added to this namespace + // we need to reconcile on all the dspa's in this namespace + // so they may mount this cert in the appropriate containers + Watches(&source.Kind{Type: &corev1.ConfigMap{}}, + handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request { + thisNamespace := o.GetNamespace() + log := r.Log.WithValues("namespace", thisNamespace) + + if o.GetName() != "odh-trusted-ca-bundle" { + return []reconcile.Request{} + } + + log.V(1).Info(fmt.Sprintf("Reconcile event triggered by change in event on Global CA Bundle: %s", o.GetName())) + + var dspaList dspav1alpha1.DataSciencePipelinesApplicationList + ctx := context.Background() + if err := r.List(ctx, &dspaList, client.InNamespace(thisNamespace)); err != nil { + log.Error(err, "unable to list DSPA's when attempting to handle Global CA Bundle event.") + return []reconcile.Request{} + } + + var reconcileRequests []reconcile.Request + for _, dspa := range dspaList.Items { + namespacedName := types.NamespacedName{ + Name: dspa.Name, + Namespace: thisNamespace, + } + reconcileRequests = append(reconcileRequests, reconcile.Request{NamespacedName: namespacedName}) + } + + return reconcileRequests + })). // Watch for Pods belonging to DSPA Watches(&source.Kind{Type: &corev1.Pod{}}, handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request { diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go index 1d8d8d465..d476fa1a8 100644 --- a/controllers/dspipeline_params.go +++ b/controllers/dspipeline_params.go @@ -20,6 +20,7 @@ import ( "context" "encoding/base64" "fmt" + "k8s.io/apimachinery/pkg/util/json" "math/rand" "time" @@ -65,6 +66,7 @@ type DBConnection struct { DBName string CredentialsSecret *dspa.SecretKeyValue Password string + ExtraParams string } type ObjectStorageConnection struct { @@ -87,7 +89,7 @@ func (p *DSPAParams) UsingExternalDB(dsp *dspa.DataSciencePipelinesApplication) return false } -// StorageHealthCheckDisabled will return the value if the Database has disableHealthCheck specified in the CR, otherwise false. +// DatabaseHealthCheckDisabled will return the value if the Database has disableHealthCheck specified in the CR, otherwise false. func (p *DSPAParams) DatabaseHealthCheckDisabled(dsp *dspa.DataSciencePipelinesApplication) bool { if dsp.Spec.Database != nil { return dsp.Spec.Database.DisableHealthCheck @@ -149,6 +151,9 @@ func (p *DSPAParams) SetupDBParams(ctx context.Context, dsp *dspa.DataSciencePip p.DBConnection.Port = dsp.Spec.Database.ExternalDB.Port p.DBConnection.Username = dsp.Spec.Database.ExternalDB.Username p.DBConnection.DBName = dsp.Spec.Database.ExternalDB.DBName + // Assume default external connection is tls enabled + // user can override this via CustomExtraParams field + p.DBConnection.ExtraParams = fmt.Sprintf(config.DBDefaultExtraParams, true) customCreds = dsp.Spec.Database.ExternalDB.PasswordSecret } else { // If no externalDB or mariaDB is specified, DSPO assumes @@ -180,11 +185,25 @@ func (p *DSPAParams) SetupDBParams(ctx context.Context, dsp *dspa.DataSciencePip p.DBConnection.Port = config.MariaDBHostPort p.DBConnection.Username = p.MariaDB.Username p.DBConnection.DBName = p.MariaDB.DBName + // By Default OOB mariadb is not tls enabled + p.DBConnection.ExtraParams = fmt.Sprintf(config.DBDefaultExtraParams, false) if p.MariaDB.PasswordSecret != nil { customCreds = p.MariaDB.PasswordSecret } } + // User specified custom Extra parameters will always take precedence + if dsp.Spec.Database.CustomExtraParams != nil { + // Validate CustomExtraParams is a valid params json + var validParamsJson map[string]string + err := json.Unmarshal([]byte(*dsp.Spec.Database.CustomExtraParams), &validParamsJson) + if err != nil { + log.Info(fmt.Sprintf("Encountered error when validationg CustomExtraParams field in DSPA, please ensure the params are well-formed: Error: %v", err)) + return err + } + p.DBConnection.ExtraParams = *dsp.Spec.Database.CustomExtraParams + } + // Secret where DB credentials reside on cluster var credsSecretName string var credsPasswordKey string @@ -428,7 +447,7 @@ func setResourcesDefault(defaultValue dspa.ResourceRequirements, value **dspa.Re } } -func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePipelinesApplication, client client.Client, log logr.Logger) error { +func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePipelinesApplication, client client.Client, loggr logr.Logger) error { p.Name = dsp.Name p.Namespace = dsp.Namespace p.Owner = dsp @@ -447,6 +466,8 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip p.APIServerPiplinesCABundleMountPath = config.APIServerPiplinesCABundleMountPath p.PiplinesCABundleMountPath = config.PiplinesCABundleMountPath + log := loggr.WithValues("namespace", p.Namespace).WithValues("dspa_name", p.Name) + if p.APIServer != nil { serverImageFromConfig := config.GetStringConfigWithDefault(config.APIServerImagePath, config.DefaultImageValue) @@ -468,16 +489,39 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip } } - // If a Custom CA Bundle is specified for injection into DSP API Server Pod - // then retrieve the bundle to utilize during storage health check - if p.APIServer.CABundle != nil { - cfgKey, cfgName := p.APIServer.CABundle.ConfigMapKey, p.APIServer.CABundle.ConfigMapName - err, val := util.GetConfigMapValue(ctx, cfgKey, cfgName, p.Namespace, client, log) - if err != nil { - log.Error(err, "Encountered error when attempting to retrieve CABundle from configmap") - return err + // Check for Global certs + // If it exists, we use this cert + // If no global cert provided, check if a custom bundle is provided via the DSPA + globalCABundleCFGMapKey, globalCABundleCFGMapName := config.GlobalCaBundleConfigMapKey, config.GlobalCaBundleConfigMapName + err, globalCerVal := util.GetConfigMapValue(ctx, globalCABundleCFGMapKey, globalCABundleCFGMapName, p.Namespace, client, log) + if err != nil && apierrs.IsNotFound(err) { + // If the global cert configmap is not available, that is OK + // proceed to check if the user has provided their + // own configmap via DSPA config + if p.APIServer.CABundle != nil { + dspaCaBundleCfgKey, dspaCaBundleCfgName := p.APIServer.CABundle.ConfigMapKey, p.APIServer.CABundle.ConfigMapName + dspaCACfgErr, dspaProvidedCABundle := util.GetConfigMapValue(ctx, dspaCaBundleCfgKey, dspaCaBundleCfgName, p.Namespace, client, log) + if dspaCACfgErr != nil && apierrs.IsNotFound(dspaCACfgErr) { + log.Info(fmt.Sprintf("ConfigMap [%s] was not found in namespace [%s]", dspaCaBundleCfgKey, p.Namespace)) + return dspaCACfgErr + } else if dspaCACfgErr != nil { + log.Info(fmt.Sprintf("Encountered error when attempting to fetch ConfigMap: [%s], Error: %v", dspaCaBundleCfgName, dspaCACfgErr)) + return dspaCACfgErr + } + p.APICustomPemCerts = []byte(dspaProvidedCABundle) + } + } else if err != nil { + log.Info(fmt.Sprintf("Encountered error when attempting to fetch ConfigMap: [%s], Error: %v", globalCABundleCFGMapKey, err)) + return err + } else { + // Found a global cert, consume this cert, takes precedence over "cABundle" provided via DSPA + log.Info(fmt.Sprintf("Found global CA Bundle %s present in this namespace %s, this cert will be "+ + "included to verify external tls connections in this DSPA.", config.GlobalCaBundleConfigMapName, p.Namespace)) + p.APICustomPemCerts = []byte(globalCerVal) + p.APIServer.CABundle = &dspa.CABundle{ + ConfigMapName: config.GlobalCaBundleConfigMapName, + ConfigMapKey: config.GlobalCaBundleConfigMapKey, } - p.APICustomPemCerts = []byte(val) } } diff --git a/controllers/storage.go b/controllers/storage.go index fedc3a2b9..b6e1d4845 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -130,10 +130,10 @@ var ConnectAndQueryObjStore = func(ctx context.Context, log logr.Logger, endpoin } if util.IsX509UnknownAuthorityError(err) { - log.Error(err, "Encountered x509 UnknownAuthorityError when connecting to ObjectStore. "+ + log.Info(fmt.Sprintf("Encountered x509 UnknownAuthorityError when connecting to ObjectStore. "+ "If using an tls S3 connection with self-signed certs, you may specify a custom CABundle "+ "to mount on the DSP API Server via the DSPA cr under the spec.cABundle field. If you have already "+ - "provided a CABundle, verify the validity of the provided CABundle.") + "provided a CABundle, verify the validity of the provided CABundle. Error: %v", err)) return false } diff --git a/controllers/suite_test.go b/controllers/suite_test.go index c8e9c2801..8adfd9d55 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -73,8 +73,8 @@ func TestAPIs(t *testing.T) { var _ = BeforeEach(func() { By("Overriding the Database and Object Store live connection functions with trivial stubs") - ConnectAndQueryDatabase = func(host string, port string, username string, password string, dbname string) bool { - return true + ConnectAndQueryDatabase = func(host string, log logr.Logger, port, username, password, dbname, tls string, pemCerts []byte, extraParams map[string]string) (bool, error) { + return true, nil } ConnectAndQueryObjStore = func(ctx context.Context, log logr.Logger, endpoint, bucket string, accesskey, secretkey []byte, secure bool, pemCerts []byte) bool { return true diff --git a/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml index fa277a796..4be9291a6 100644 --- a/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml @@ -136,6 +136,9 @@ spec: cpu: 500m memory: 1Gi volumeMounts: + - name: server-config + mountPath: /config/config.json + subPath: config.json - mountPath: /config/sample_config.json name: sample-config subPath: sample_config.json @@ -193,6 +196,10 @@ spec: secret: secretName: ds-pipelines-proxy-tls-testdsp0 defaultMode: 420 + - name: server-config + configMap: + defaultMode: 420 + name: pipeline-server-config-testdsp0 - configMap: defaultMode: 420 name: sample-config-testdsp0 @@ -201,5 +208,4 @@ spec: defaultMode: 420 name: sample-pipeline-testdsp0 name: sample-pipeline - serviceAccountName: ds-pipeline-testdsp0 diff --git a/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml index 5c1263828..c2205a899 100644 --- a/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml @@ -136,6 +136,9 @@ spec: cpu: 2522m memory: 5Gi volumeMounts: + - name: server-config + mountPath: /config/config.json + subPath: config.json - mountPath: /config/sample_config.json name: sample-config subPath: sample_config.json @@ -193,6 +196,10 @@ spec: secret: secretName: ds-pipelines-proxy-tls-testdsp2 defaultMode: 420 + - name: server-config + configMap: + defaultMode: 420 + name: pipeline-server-config-testdsp2 - configMap: defaultMode: 420 name: sample-config-testdsp2 diff --git a/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml index 0b617788d..79e78d123 100644 --- a/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml @@ -187,4 +187,8 @@ spec: secret: secretName: ds-pipelines-proxy-tls-testdsp3 defaultMode: 420 + - name: server-config + configMap: + name: pipeline-server-config-testdsp3 + defaultMode: 420 serviceAccountName: ds-pipeline-testdsp3 diff --git a/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml index 94524294c..eabb4b6fe 100644 --- a/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml @@ -187,4 +187,8 @@ spec: secret: secretName: ds-pipelines-proxy-tls-testdsp4 defaultMode: 420 + - name: server-config + configMap: + defaultMode: 420 + name: pipeline-server-config-testdsp4 serviceAccountName: ds-pipeline-testdsp4 diff --git a/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml index 92f6ac5b9..1f59a95e9 100644 --- a/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml @@ -136,6 +136,9 @@ spec: cpu: 500m memory: 1Gi volumeMounts: + - name: server-config + mountPath: /config/config.json + subPath: config.json - mountPath: /config/sample_config.json name: sample-config subPath: sample_config.json @@ -193,6 +196,10 @@ spec: secret: secretName: ds-pipelines-proxy-tls-testdsp5 defaultMode: 420 + - configMap: + defaultMode: 420 + name: pipeline-server-config-testdsp5 + name: server-config - configMap: defaultMode: 420 name: sample-config-testdsp5 diff --git a/controllers/testdata/declarative/case_6/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_6/expected/created/apiserver_deployment.yaml index 96c5967e8..585b0db80 100644 --- a/controllers/testdata/declarative/case_6/expected/created/apiserver_deployment.yaml +++ b/controllers/testdata/declarative/case_6/expected/created/apiserver_deployment.yaml @@ -142,6 +142,9 @@ spec: cpu: 500m memory: 1Gi volumeMounts: + - name: server-config + mountPath: /config/config.json + subPath: config.json - name: ca-bundle mountPath: /etc/pki/tls/certs - name: oauth-proxy @@ -196,6 +199,10 @@ spec: secret: secretName: ds-pipelines-proxy-tls-testdsp6 defaultMode: 420 + - name: server-config + configMap: + name: pipeline-server-config-testdsp6 + defaultMode: 420 - name: ca-bundle configMap: name: testcabundleconfigmap6 diff --git a/controllers/util/util.go b/controllers/util/util.go index 555ce0938..5e001583b 100644 --- a/controllers/util/util.go +++ b/controllers/util/util.go @@ -23,7 +23,6 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" - apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "net/url" @@ -72,16 +71,12 @@ func GetConfigMapValue(ctx context.Context, cfgKey, cfgName, ns string, client c Namespace: ns, } err := client.Get(ctx, namespacedName, cfgMap) - if err != nil && apierrs.IsNotFound(err) { - log.Error(err, fmt.Sprintf("ConfigMap [%s] was not found in namespace [%s]", cfgName, ns)) - return err, "" - } else if err != nil { - log.Error(err, fmt.Sprintf("Encountered error when attempting to fetch ConfigMap. [%s]..", cfgName)) + if err != nil { return err, "" } if val, ok := cfgMap.Data[cfgKey]; ok { return nil, val } else { - return fmt.Errorf("ConfigMap %s sdoes not contain specified key %s", cfgName, cfgKey), "" + return fmt.Errorf("ConfigMap %s does not contain expected key %s", cfgName, cfgKey), "" } }