From 3a883fcb0b69ea254b812d33e36ee78d45be09df Mon Sep 17 00:00:00 2001 From: Christian Coffield Date: Wed, 23 Feb 2022 19:01:10 -0500 Subject: [PATCH] Add tests for PrecheckPowerScale() (#48) --- .github/workflows/actions.yml | 2 +- controllers/csm_controller.go | 2 +- pkg/drivers/commonconfig_test.go | 61 +++++++++++++++++++++++++++++--- pkg/drivers/powerscale.go | 9 +++-- tests/shared/crclient/client.go | 5 +++ 5 files changed, 67 insertions(+), 12 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index dff00a316..c20a1942a 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -54,7 +54,7 @@ jobs: - name: Run unit tests and check package coverage uses: dell/common-github-actions/go-code-tester@csm-operator-drivers with: - threshold: 80 + threshold: 90 go_security_scan: name: Go security runs-on: ubuntu-latest diff --git a/controllers/csm_controller.go b/controllers/csm_controller.go index 753f26237..ab6c62ad1 100644 --- a/controllers/csm_controller.go +++ b/controllers/csm_controller.go @@ -690,7 +690,7 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs switch cr.Spec.Driver.CSIDriverType { case csmv1.PowerScale: - err := drivers.PrecheckPowerScale(ctx, cr, r) + err := drivers.PrecheckPowerScale(ctx, cr, r.GetClient()) if err != nil { return fmt.Errorf("failed powerscale validation: %v", err) } diff --git a/pkg/drivers/commonconfig_test.go b/pkg/drivers/commonconfig_test.go index 7d6491d3a..5a38c2d6e 100644 --- a/pkg/drivers/commonconfig_test.go +++ b/pkg/drivers/commonconfig_test.go @@ -7,8 +7,11 @@ import ( csmv1 "github.com/dell/csm-operator/api/v1alpha1" "github.com/dell/csm-operator/pkg/utils" "github.com/dell/csm-operator/tests/shared" + "github.com/dell/csm-operator/tests/shared/crclient" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log/zap" ) @@ -20,6 +23,9 @@ var ( powerScaleCSM = csmForPowerScale() powerScaleCSMBadSkipCert = csmForPowerScaleBadSkipCert() powerScaleCSMBadCertCnt = csmForPowerScaleBadCertCnt() + objects = map[shared.StorageKey]runtime.Object{} + powerScaleClient = crclient.NewFakeClientNoInjector(objects) + powerScaleSecret = shared.MakeSecret("csm", "driver-test", shared.ConfigVersion) // where to find all the yaml files config = utils.OperatorConfig{ @@ -48,12 +54,30 @@ var ( name string // csm object csm csmv1.ContainerStorageModule + // client + ct client.Client + sec *corev1.Secret // expected error expectedErr string }{ - {"happy path", powerScaleCSM, ""}, - {"invalid value for skip cert validation", powerScaleCSMBadSkipCert, "is an invalid value for X_CSI_ISI_SKIP_CERTIFICATE_VALIDATION"}, - {"invalid value for cert secret cnt", powerScaleCSMBadCertCnt, "is an invalid value for CERT_SECRET_COUNT"}, + {"happy path", powerScaleCSM, powerScaleClient, powerScaleSecret, ""}, + {"invalid value for skip cert validation", powerScaleCSMBadSkipCert, powerScaleClient, powerScaleSecret, "is an invalid value for X_CSI_ISI_SKIP_CERTIFICATE_VALIDATION"}, + {"invalid value for cert secret cnt", powerScaleCSMBadCertCnt, powerScaleClient, powerScaleSecret, "is an invalid value for CERT_SECRET_COUNT"}, + } + + preCheckPowerScaleTest = []struct { + // every single unit test name + name string + // csm object + csm csmv1.ContainerStorageModule + // client + ct client.Client + // secret + sec *corev1.Secret + // expected error + expectedErr string + }{ + {"missing secret", powerScaleCSM, powerScaleClient, powerScaleSecret, "failed to find secret"}, } opts = zap.Options{ @@ -79,6 +103,32 @@ func TestGetApplyCertVolume(t *testing.T) { } } +func TestPrecheckPowerScale(t *testing.T) { + ctx := context.Background() + for _, tt := range preCheckPowerScaleTest { + t.Run(tt.name, func(t *testing.T) { + err := PrecheckPowerScale(ctx, &tt.csm, tt.ct) + if tt.expectedErr == "" { + assert.Nil(t, err) + } else { + assert.Containsf(t, err.Error(), tt.expectedErr, "expected error containing %q, got %s", tt.expectedErr, err) + } + }) + } + + for _, tt := range powerScaleTests { + tt.ct.Create(ctx, tt.sec) + t.Run(tt.name, func(t *testing.T) { + err := PrecheckPowerScale(ctx, &tt.csm, tt.ct) + if tt.expectedErr == "" { + assert.Nil(t, err) + } else { + assert.Containsf(t, err.Error(), tt.expectedErr, "expected error containing %q, got %s", tt.expectedErr, err) + } + }) + } +} + func TestGetCsiDriver(t *testing.T) { ctx := context.Background() for _, tt := range tests { @@ -193,9 +243,10 @@ func csmForPowerScale() csmv1.ContainerStorageModule { res := shared.MakeCSM("csm", "driver-test", shared.ConfigVersion) // Add log level to cover some code in GetConfigMap - envVarLogLevel1 := corev1.EnvVar{Name: "CERT_SECRET_COUNT", Value: "2"} - envVarLogLevel2 := corev1.EnvVar{Name: "X_CSI_ISI_SKIP_CERTIFICATE_VALIDATION", Value: "true"} + envVarLogLevel1 := corev1.EnvVar{Name: "CERT_SECRET_COUNT", Value: "0"} + envVarLogLevel2 := corev1.EnvVar{Name: "X_CSI_ISI_SKIP_CERTIFICATE_VALIDATION", Value: "false"} res.Spec.Driver.Common.Envs = []corev1.EnvVar{envVarLogLevel1, envVarLogLevel2} + res.Spec.Driver.AuthSecret = "csm-creds" return res } diff --git a/pkg/drivers/powerscale.go b/pkg/drivers/powerscale.go index bedb9e597..891aa84cb 100644 --- a/pkg/drivers/powerscale.go +++ b/pkg/drivers/powerscale.go @@ -7,12 +7,12 @@ import ( csmv1 "github.com/dell/csm-operator/api/v1alpha1" "github.com/dell/csm-operator/pkg/logger" - utils "github.com/dell/csm-operator/pkg/utils" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" acorev1 "k8s.io/client-go/applyconfigurations/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" // +kubebuilder:scaffold:imports ) @@ -28,7 +28,7 @@ const ( ) // PrecheckPowerScale do input validation -func PrecheckPowerScale(ctx context.Context, cr *csmv1.ContainerStorageModule, r utils.ReconcileCSM) error { +func PrecheckPowerScale(ctx context.Context, cr *csmv1.ContainerStorageModule, ct client.Client) error { log := logger.GetLogger(ctx) // Check for secrete only config := cr.Name + "-creds" @@ -69,13 +69,12 @@ func PrecheckPowerScale(ctx context.Context, cr *csmv1.ContainerStorageModule, r for _, name := range secrets { found := &corev1.Secret{} - err := r.GetClient().Get(ctx, types.NamespacedName{Name: name, - Namespace: cr.GetNamespace()}, found) + err := ct.Get(ctx, types.NamespacedName{Name: name, Namespace: cr.GetNamespace()}, found) if err != nil { + log.Error(err, "Failed query for secret %s", name) if errors.IsNotFound(err) { return fmt.Errorf("failed to find secret %s", name) } - log.Error(err, "Failed to query for secret. Warning - the controller pod may not start") } } diff --git a/tests/shared/crclient/client.go b/tests/shared/crclient/client.go index aa7113658..157ec9720 100644 --- a/tests/shared/crclient/client.go +++ b/tests/shared/crclient/client.go @@ -36,6 +36,11 @@ func NewFakeClient(objectMap map[shared.StorageKey]runtime.Object, errorInjector } } +// NewFakeClientNoInjector creates a new client without an error injector +func NewFakeClientNoInjector(objectMap map[shared.StorageKey]runtime.Object) *Client { + return &Client{Objects: objectMap} +} + // Get implements client.Client. func (f Client) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error { if f.ErrorInjector != nil {