diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8bde3ce7c..34ce93f50 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,7 +6,7 @@ repos: exclude: README.md - id: check-merge-conflict - id: end-of-file-fixer - exclude: controllers/testdata/tls/ca-bundle.crt + exclude: controllers/testdata/tls - id: check-added-large-files - id: check-case-conflict - id: check-json diff --git a/Makefile b/Makefile index 1251aee39..aead30fa7 100644 --- a/Makefile +++ b/Makefile @@ -223,7 +223,7 @@ $(CONTROLLER_GEN): $(LOCALBIN) .PHONY: envtest envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. $(ENVTEST): $(LOCALBIN) - test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest + test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@release-0.16 .PHONY: bundle bundle: manifests kustomize ## Generate bundle manifests and metadata, then validate generated files. diff --git a/controllers/dspipeline_fake_controller.go b/controllers/dspipeline_fake_controller.go index 37d5a7622..f431131c2 100644 --- a/controllers/dspipeline_fake_controller.go +++ b/controllers/dspipeline_fake_controller.go @@ -42,9 +42,9 @@ func NewFakeController() *DSPAReconciler { // Create Scheme FakeScheme := scheme.Scheme utilruntime.Must(clientgoscheme.AddToScheme(FakeScheme)) - utilruntime.Must(buildv1.AddToScheme(FakeScheme)) - utilruntime.Must(imagev1.AddToScheme(FakeScheme)) - utilruntime.Must(routev1.AddToScheme(FakeScheme)) + utilruntime.Must(buildv1.Install(FakeScheme)) + utilruntime.Must(imagev1.Install(FakeScheme)) + utilruntime.Must(routev1.Install(FakeScheme)) utilruntime.Must(dspav1alpha1.AddToScheme(FakeScheme)) FakeBuilder.WithScheme(FakeScheme) diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go index 192132a3b..f775834d6 100644 --- a/controllers/dspipeline_params.go +++ b/controllers/dspipeline_params.go @@ -468,7 +468,7 @@ func (p *DSPAParams) SetupObjectParams(ctx context.Context, dsp *dspa.DataScienc } -func (p *DSPAParams) SetupMLMD(ctx context.Context, dsp *dspa.DataSciencePipelinesApplication, client client.Client, log logr.Logger) error { +func (p *DSPAParams) SetupMLMD(dsp *dspa.DataSciencePipelinesApplication, log logr.Logger) error { if p.UsingV2Pipelines(dsp) { if p.MLMD == nil { log.Info("MLMD not specified, but is a required component for V2 Pipelines. Including MLMD with default specs.") @@ -666,7 +666,10 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip if sysCertsErr != nil { return sysCertsErr } - p.APICustomPemCerts = append(p.APICustomPemCerts, certs) + + if len(certs) != 0 { + p.APICustomPemCerts = append(p.APICustomPemCerts, certs) + } } p.CustomCABundle = &dspa.CABundle{ @@ -758,7 +761,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip setStringDefault(argoExecImageFromConfig, &p.WorkflowController.ArgoExecImage) } - err := p.SetupMLMD(ctx, dsp, client, log) + err := p.SetupMLMD(dsp, log) if err != nil { return err } diff --git a/controllers/dspipeline_params_test.go b/controllers/dspipeline_params_test.go new file mode 100644 index 000000000..8bb367449 --- /dev/null +++ b/controllers/dspipeline_params_test.go @@ -0,0 +1,228 @@ +//go:build test_all || test_unit + +/* + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controllers + +import ( + dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" + "github.com/opendatahub-io/data-science-pipelines-operator/controllers/testutil" + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "testing" +) + +type Client struct { + Clientset kubernetes.Interface +} + +func TestExtractParams_WithEmptyDSPA(t *testing.T) { + dspa := testutil.CreateEmptyDSPA() + ctx, params, reconciler := CreateNewTestObjects() + err := params.ExtractParams(ctx, dspa, reconciler.Client, reconciler.Log) + assert.Nil(t, err) +} + +func TestExtractParams_CABundle(t *testing.T) { + + ctx, _, client := CreateNewTestObjects() + + tt := []struct { + msg string + dsp *dspav1alpha1.DataSciencePipelinesApplication + CustomCABundleRootMountPath string + CustomSSLCertDir *string + PiplinesCABundleMountPath string + SSLCertFileEnv string + APICustomPemCerts [][]byte + CustomCABundle *dspav1alpha1.CABundle + ConfigMapPreReq []*v1.ConfigMap + errorMsg string + }{ + { + msg: "no bundle provided", + dsp: testutil.CreateEmptyDSPA(), + CustomCABundleRootMountPath: "/dsp-custom-certs", + CustomSSLCertDir: nil, + PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt", + APICustomPemCerts: nil, + CustomCABundle: nil, + }, + { + msg: "user bundle provided, but no configmap", + dsp: testutil.CreateDSPAWithAPIServerCABundle("testcakey", "testcaname"), + CustomCABundleRootMountPath: "/dsp-custom-certs", + CustomSSLCertDir: nil, + PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt", + APICustomPemCerts: nil, + CustomCABundle: nil, + ConfigMapPreReq: []*v1.ConfigMap{}, + errorMsg: "configmaps \"testcaname\" not found", + }, + { + msg: "user bundle provided", + dsp: testutil.CreateDSPAWithAPIServerCABundle("testcakey", "testcaname"), + CustomCABundleRootMountPath: "/dsp-custom-certs", + CustomSSLCertDir: strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"), + PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt", + APICustomPemCerts: [][]byte{[]byte("bundle-contents")}, + CustomCABundle: &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"}, + ConfigMapPreReq: []*v1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: "testcaname", Namespace: "testnamespace"}, + Data: map[string]string{"testcakey": "bundle-contents"}, + }, + }, + }, + { + msg: "odh-trusted-ca bundle provided", + dsp: testutil.CreateEmptyDSPA(), + CustomCABundleRootMountPath: "/dsp-custom-certs", + CustomSSLCertDir: strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"), + PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt", + APICustomPemCerts: [][]byte{[]byte("odh-bundle-contents")}, + CustomCABundle: &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"}, + ConfigMapPreReq: []*v1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: "odh-trusted-ca-bundle", Namespace: "testnamespace"}, + Data: map[string]string{"testcakey": "odh-bundle-contents"}, + }, + }, + }, + { + msg: "some empty values in odh-trusted-ca bundle provided", + dsp: testutil.CreateEmptyDSPA(), + CustomCABundleRootMountPath: "/dsp-custom-certs", + CustomSSLCertDir: strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"), + PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt", + APICustomPemCerts: [][]byte{[]byte("odh-bundle-contents-2")}, + CustomCABundle: &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"}, + ConfigMapPreReq: []*v1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: "odh-trusted-ca-bundle", Namespace: "testnamespace"}, + Data: map[string]string{"ca-bundle.crt": "", "odh-ca-bundle.crt": "odh-bundle-contents-2"}, + }, + }, + }, + { + msg: "some empty values in odh-trusted-ca bundle provided", + dsp: testutil.CreateEmptyDSPA(), + CustomCABundleRootMountPath: "/dsp-custom-certs", + CustomSSLCertDir: nil, + PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt", + APICustomPemCerts: nil, + CustomCABundle: nil, + ConfigMapPreReq: []*v1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: "odh-trusted-ca-bundle", Namespace: "testnamespace"}, + Data: map[string]string{"ca-bundle.crt": "", "odh-ca-bundle.crt": ""}, + }, + }, + }, + { + msg: "both user and odh-trusted-ca bundle provided", + dsp: testutil.CreateDSPAWithAPIServerCABundle("testcakey", "testcaname"), + CustomCABundleRootMountPath: "/dsp-custom-certs", + CustomSSLCertDir: strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"), + PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt", + APICustomPemCerts: [][]byte{[]byte("odh-bundle-contents"), []byte("bundle-contents")}, + CustomCABundle: &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"}, + ConfigMapPreReq: []*v1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: "odh-trusted-ca-bundle", Namespace: "testnamespace"}, + Data: map[string]string{"testcakey": "odh-bundle-contents"}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "testcaname", Namespace: "testnamespace"}, + Data: map[string]string{"testcakey": "bundle-contents"}, + }, + }, + }, + { + msg: "both user and odh-trusted-ca bundle provided with non empty SSL_CERT_FILE", + dsp: testutil.CreateDSPAWithAPIServerCABundle("testcakey", "testcaname"), + CustomCABundleRootMountPath: "/dsp-custom-certs", + CustomSSLCertDir: strPtr("/dsp-custom-certs:/etc/ssl/certs:/etc/pki/tls/certs"), + PiplinesCABundleMountPath: "/dsp-custom-certs/dsp-ca.crt", + APICustomPemCerts: [][]byte{[]byte("odh-bundle-contents"), []byte("bundle-contents"), []byte("dummycontent")}, + CustomCABundle: &dspav1alpha1.CABundle{ConfigMapKey: "dsp-ca.crt", ConfigMapName: "dsp-trusted-ca-testdspa"}, + ConfigMapPreReq: []*v1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: "odh-trusted-ca-bundle", Namespace: "testnamespace"}, + Data: map[string]string{"testcakey": "odh-bundle-contents"}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "testcaname", Namespace: "testnamespace"}, + Data: map[string]string{"testcakey": "bundle-contents"}, + }, + }, + SSLCertFileEnv: "testdata/tls/dummy-ca-bundle.crt", + }, + } + + for _, test := range tt { + t.Run(test.msg, func(t *testing.T) { + if test.SSLCertFileEnv == "" { + t.Setenv("SSL_CERT_FILE", "testdata/tls/empty-ca-bundle.crt") + } else { + t.Setenv("SSL_CERT_FILE", test.SSLCertFileEnv) + } + + if test.ConfigMapPreReq != nil && len(test.ConfigMapPreReq) > 0 { + for _, cfg := range test.ConfigMapPreReq { + err := client.Create(ctx, cfg) + assert.Nil(t, err) + } + } + + actualParams := &DSPAParams{} + extractError := actualParams.ExtractParams(ctx, test.dsp, client.Client, client.Log) + if test.errorMsg != "" { + assert.Contains(t, extractError.Error(), test.errorMsg) + } else { + assert.Nil(t, extractError) + } + + actualCustomCABundleRootMountPath := actualParams.CustomCABundleRootMountPath + assert.Equal(t, actualCustomCABundleRootMountPath, test.CustomCABundleRootMountPath) + + actualCustomSSLCertDir := actualParams.CustomSSLCertDir + assert.Equal(t, actualCustomSSLCertDir, test.CustomSSLCertDir) + + actualPipelinesCABundleMountPath := actualParams.PiplinesCABundleMountPath + assert.Equal(t, actualPipelinesCABundleMountPath, test.PiplinesCABundleMountPath) + + actualAPICustomPemCerts := actualParams.APICustomPemCerts + assert.Equal(t, actualAPICustomPemCerts, test.APICustomPemCerts) + + actualCustomCABundle := actualParams.CustomCABundle + assert.Equal(t, actualCustomCABundle, test.CustomCABundle) + + if test.ConfigMapPreReq != nil && len(test.ConfigMapPreReq) > 0 { + for _, cfg := range test.ConfigMapPreReq { + err := client.Delete(ctx, cfg) + assert.Nil(t, err) + } + } + }) + } +} + +func strPtr(v string) *string { + return &v +} diff --git a/controllers/testdata/tls/dummy-ca-bundle.crt b/controllers/testdata/tls/dummy-ca-bundle.crt new file mode 100644 index 000000000..a03db2fbf --- /dev/null +++ b/controllers/testdata/tls/dummy-ca-bundle.crt @@ -0,0 +1 @@ +dummycontent \ No newline at end of file diff --git a/controllers/testdata/tls/empty-ca-bundle.crt b/controllers/testdata/tls/empty-ca-bundle.crt new file mode 100644 index 000000000..e69de29bb diff --git a/controllers/testutil/util.go b/controllers/testutil/util.go index 0de35fe87..0e7c28402 100644 --- a/controllers/testutil/util.go +++ b/controllers/testutil/util.go @@ -19,7 +19,7 @@ package testutil import ( "context" "fmt" - + dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" "os" "time" @@ -210,3 +210,42 @@ func GenerateDeclarativeTestCases() []Case { return testcases } + +func CreateEmptyDSPA() *dspav1alpha1.DataSciencePipelinesApplication { + dspa := &dspav1alpha1.DataSciencePipelinesApplication{ + Spec: dspav1alpha1.DSPASpec{ + APIServer: &dspav1alpha1.APIServer{Deploy: false}, + MLMD: &dspav1alpha1.MLMD{Deploy: false}, + PersistenceAgent: &dspav1alpha1.PersistenceAgent{Deploy: false}, + ScheduledWorkflow: &dspav1alpha1.ScheduledWorkflow{Deploy: false}, + MlPipelineUI: &dspav1alpha1.MlPipelineUI{ + Deploy: false, + Image: "testimage-MlPipelineUI:test", + }, + WorkflowController: &dspav1alpha1.WorkflowController{Deploy: false}, + Database: &dspav1alpha1.Database{DisableHealthCheck: false, MariaDB: &dspav1alpha1.MariaDB{Deploy: false}}, + ObjectStorage: &dspav1alpha1.ObjectStorage{ + DisableHealthCheck: false, + Minio: &dspav1alpha1.Minio{ + Deploy: false, + Image: "testimage-Minio:test", + }, + }, + }, + } + dspa.Name = "testdspa" + dspa.Namespace = "testnamespace" + return dspa +} + +func CreateDSPAWithAPIServerCABundle(key string, cfgmapName string) *dspav1alpha1.DataSciencePipelinesApplication { + dspa := CreateEmptyDSPA() + dspa.Spec.APIServer = &dspav1alpha1.APIServer{ + Deploy: true, + CABundle: &dspav1alpha1.CABundle{ + ConfigMapKey: key, + ConfigMapName: cfgmapName, + }, + } + return dspa +}