From 5779f0d89442be613d02392d4f7c664fe8c6c718 Mon Sep 17 00:00:00 2001 From: Aly Nathoo Date: Thu, 19 Dec 2024 17:32:11 -0500 Subject: [PATCH] Changed validation code to handle corner cases in zone struct where labelValue or Name are either not specified or are empty values. Cleaned up test code and added test cases for these scenarios. --- pkg/drivers/powerflex.go | 51 ++++++---- pkg/drivers/powerflex_test.go | 182 ++++++++++++++++++++++++++++++---- 2 files changed, 196 insertions(+), 37 deletions(-) diff --git a/pkg/drivers/powerflex.go b/pkg/drivers/powerflex.go index cb28d7b5..44b9e289 100644 --- a/pkg/drivers/powerflex.go +++ b/pkg/drivers/powerflex.go @@ -16,6 +16,7 @@ import ( "context" "fmt" "os" + "reflect" "regexp" "strings" @@ -360,30 +361,31 @@ func ValidateZonesInSecret(ctx context.Context, kube client.Client, namespace st arraySecret, err := utils.GetSecret(ctx, secret, namespace, kube) if err != nil { - return fmt.Errorf("reading secret [%s] error [%s]", secret, err) + return fmt.Errorf("reading secret [%s] error %v", secret, err) + } + + type Zone struct { + Name string `json:"name,omitempty"` + LabelKey string `json:"labelKey,omitempty"` } type StorageArrayConfig struct { SystemID string `json:"systemID"` - Zone struct { - Name string `json:"name"` - LabelKey string `json:"labelKey"` - } `json:"zone"` + Zone Zone `json:"zone,omitempty"` } data := arraySecret.Data configBytes := data["config"] - zonesMapData := make(map[string]string) if string(configBytes) != "" { yamlConfig := make([]StorageArrayConfig, 0) configs, err := yaml.JSONToYAML(configBytes) if err != nil { - return fmt.Errorf("unable to parse multi-array configuration %v", err) + return fmt.Errorf("malformed json in array secret - unable to parse multi-array configuration %v", err) } err = yaml.Unmarshal(configs, &yamlConfig) if err != nil { - return fmt.Errorf("unable to unmarshal multi-array configuration %v", err) + return fmt.Errorf("unable to unmarshal array secret %v", err) } var labelKey string @@ -393,26 +395,39 @@ func ValidateZonesInSecret(ctx context.Context, kube client.Client, namespace st if configParam.SystemID == "" { return fmt.Errorf("invalid value for SystemID") } - if labelKey == "" { - labelKey = configParam.Zone.LabelKey + if reflect.DeepEqual(configParam.Zone, Zone{}) { + log.Infof("Zone is not specified for SystemID:", configParam.SystemID) } else { - if labelKey != configParam.Zone.LabelKey { - return fmt.Errorf("labelKey not consistent across all arrays") + log.Infof("Zone is specified for SystemID:", configParam.SystemID) + if configParam.Zone.LabelKey == "" { + return fmt.Errorf("Zone LabelKey is empty or not specified for SystemID: %s", + configParam.SystemID) + } + + if labelKey == "" { + labelKey = configParam.Zone.LabelKey + } else { + if labelKey != configParam.Zone.LabelKey { + return fmt.Errorf("labelKey is not consistent across all arrays in secret") + } + } + + if configParam.Zone.Name == "" { + return fmt.Errorf("Zone name is empty or not specified for SystemID: %s", + configParam.SystemID) } - } - if configParam.Zone.Name != "" { numArraysWithZone++ - zonesMapData[configParam.SystemID] = configParam.Zone.Name - log.Infof("Zoning information configured for systemID %s: %v ", configParam.SystemID, zonesMapData) } } + + log.Infof("found %d arrays zoning on %d", numArrays, numArraysWithZone) if numArraysWithZone > 0 && numArrays != numArraysWithZone { return fmt.Errorf("not all arrays have zoning configured. Check the array info secret, zone key should be the same for all arrays") } else if numArraysWithZone == 0 { - log.Info("Zoning information not found in the array config. Continue with topology-unaware driver installation mode") + log.Info("Zoning information not found in the array secret. Continue with topology-unaware driver installation mode") } } else { - return fmt.Errorf("array details are not provided in vxflexos-config secret") + return fmt.Errorf("array details are not provided in secret") } return nil diff --git a/pkg/drivers/powerflex_test.go b/pkg/drivers/powerflex_test.go index 162f5cd2..6deb8ddd 100644 --- a/pkg/drivers/powerflex_test.go +++ b/pkg/drivers/powerflex_test.go @@ -257,6 +257,35 @@ func TestExtractZonesFromSecret(t *testing.T) { zone: name: "ZONE-2" labelKey: "zone.csi-vxflexos.dellemc.com" +` + zoneDataWithMultiArraySomeZone2 := ` +- username: "admin" + password: "password" + systemID: "2b11bb111111bb1b" + endpoint: "https://127.0.0.2" + skipCertificateValidation: true + mdm: "10.0.0.3,10.0.0.4" +- username: "admin" + password: "password" + systemID: "1a99aa999999aa9a" + endpoint: "https://127.0.0.1" + skipCertificateValidation: true + mdm: "10.0.0.5,10.0.0.6" +- username: "admin" + password: "password" + systemID: "1a99aa999999aa9a" + endpoint: "https://127.0.0.1" + skipCertificateValidation: true + mdm: "10.0.0.5,10.0.0.6" + zone: + name: "ZONE-2" + labelKey: "zone.csi-vxflexos.dellemc.com" +- username: "admin" + password: "password" + systemID: "1a99aa999999aa9a" + endpoint: "https://127.0.0.1" + skipCertificateValidation: true + mdm: "10.0.0.5,10.0.0.6" ` dataWithoutZone := ` - username: "admin" @@ -266,9 +295,69 @@ func TestExtractZonesFromSecret(t *testing.T) { skipCertificateValidation: true mdm: "10.0.0.3,10.0.0.4" ` + zoneDataWithMultiArrayPartialZone1 := ` +- username: "admin" + password: "password" + systemID: "2b11bb111111bb1b" + endpoint: "https://127.0.0.2" + skipCertificateValidation: true + mdm: "10.0.0.3,10.0.0.4" + zone: + name: "ZONE-1" + labelKey: "zone.csi-vxflexos.dellemc.com" +- username: "admin" + password: "password" + systemID: "1a99aa999999aa9a" + endpoint: "https://127.0.0.1" + skipCertificateValidation: true + mdm: "10.0.0.5,10.0.0.6" + zone: + name: "" + labelKey: "zone.csi-vxflexos.dellemc.com" +` + zoneDataWithMultiArrayPartialZone2 := ` +- username: "admin" + password: "password" + systemID: "2b11bb111111bb1b" + endpoint: "https://127.0.0.2" + skipCertificateValidation: true + mdm: "10.0.0.3,10.0.0.4" + zone: + name: "ZONE-1" + labelKey: "zone.csi-vxflexos.dellemc.com" +- username: "admin" + password: "password" + systemID: "1a99aa999999aa9a" + endpoint: "https://127.0.0.1" + skipCertificateValidation: true + mdm: "10.0.0.5,10.0.0.6" + zone: + name: "myname" +` + zoneDataWithMultiArrayPartialZone3 := ` +- username: "admin" + password: "password" + systemID: "2b11bb111111bb1b" + endpoint: "https://127.0.0.2" + skipCertificateValidation: true + mdm: "10.0.0.3,10.0.0.4" + zone: + name: "ZONE-1" + labelKey: "" +- username: "admin" + password: "password" + systemID: "1a99aa999999aa9a" + endpoint: "https://127.0.0.1" + skipCertificateValidation: true + mdm: "10.0.0.5,10.0.0.6" + zone: + name: "myname" + labelKey: "zone.csi-vxflexos.dellemc.com" +` + ctx := context.Background() - tests := map[string]func() (client.WithWatch, map[string]string, string, bool){ - "success with zone": func() (client.WithWatch, map[string]string, string, bool) { + tests := map[string]func() (client.WithWatch, string, bool){ + "success with zone": func() (client.WithWatch, string, bool) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "vxflexos-config", @@ -280,9 +369,9 @@ func TestExtractZonesFromSecret(t *testing.T) { } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, map[string]string{"2b11bb111111bb1b": "US-EAST"}, "vxflexos-config", false + return client, "vxflexos-config", false }, - "success with zone and multi array": func() (client.WithWatch, map[string]string, string, bool) { + "success with zone and multi array": func() (client.WithWatch, string, bool) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "vxflexos-config", @@ -294,9 +383,9 @@ func TestExtractZonesFromSecret(t *testing.T) { } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, map[string]string{"2b11bb111111bb1b": "ZONE-1", "1a99aa999999aa9a": "ZONE-2"}, "vxflexos-config", false + return client, "vxflexos-config", false }, - "fail multi array but only some zone": func() (client.WithWatch, map[string]string, string, bool) { + "fail multi array but only some zone": func() (client.WithWatch, string, bool) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "vxflexos-config", @@ -307,9 +396,22 @@ func TestExtractZonesFromSecret(t *testing.T) { }, } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, map[string]string{"2b11bb111111bb1b": "ZONE-1", "1a99aa999999aa9a": "ZONE-2"}, "vxflexos-config", true + return client, "vxflexos-config", true }, - "success no zone": func() (client.WithWatch, map[string]string, string, bool) { + "fail multi array but only some zone test two": func() (client.WithWatch, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(zoneDataWithMultiArraySomeZone2), + }, + } + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, "vxflexos-config", true + }, + "success no zone": func() (client.WithWatch, string, bool) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "vxflexos-config", @@ -321,13 +423,13 @@ func TestExtractZonesFromSecret(t *testing.T) { } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, map[string]string{}, "vxflexos-config", false + return client, "vxflexos-config", false }, - "error getting secret": func() (client.WithWatch, map[string]string, string, bool) { + "error getting secret": func() (client.WithWatch, string, bool) { client := fake.NewClientBuilder().Build() - return client, nil, "vxflexos-not-found", true + return client, "vxflexos-not-found", true }, - "error parsing empty secret": func() (client.WithWatch, map[string]string, string, bool) { + "error parsing empty secret": func() (client.WithWatch, string, bool) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "vxflexos-config", @@ -339,9 +441,9 @@ func TestExtractZonesFromSecret(t *testing.T) { } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, nil, "vxflexos-config", true + return client, "vxflexos-config", true }, - "error with no system id": func() (client.WithWatch, map[string]string, string, bool) { + "error with no system id": func() (client.WithWatch, string, bool) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "vxflexos-config", @@ -353,9 +455,10 @@ func TestExtractZonesFromSecret(t *testing.T) { } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, nil, "vxflexos-config", true + return client, "vxflexos-config", true }, - "error unmarshaling config": func() (client.WithWatch, map[string]string, string, bool) { + + "error unmarshaling config": func() (client.WithWatch, string, bool) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "vxflexos-config", @@ -367,19 +470,60 @@ func TestExtractZonesFromSecret(t *testing.T) { } client := fake.NewClientBuilder().WithObjects(secret).Build() - return client, nil, "vxflexos-config", true + return client, "vxflexos-config", true + }, + "Fail Partial Zone Config 1": func() (client.WithWatch, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(zoneDataWithMultiArrayPartialZone1), + }, + } + + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, "vxflexos-config", true + }, + "Fail Partial Zone Config 2": func() (client.WithWatch, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(zoneDataWithMultiArrayPartialZone2), + }, + } + + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, "vxflexos-config", true + }, + "Fail Partial Zone Config 3": func() (client.WithWatch, string, bool) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vxflexos-config", + Namespace: "vxflexos", + }, + Data: map[string][]byte{ + "config": []byte(zoneDataWithMultiArrayPartialZone3), + }, + } + + client := fake.NewClientBuilder().WithObjects(secret).Build() + return client, "vxflexos-config", true }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { - client, wantZones, secret, wantErr := tc() + client, secret, wantErr := tc() err := ValidateZonesInSecret(ctx, client, "vxflexos", secret) if wantErr { assert.NotNil(t, err) } else { assert.Nil(t, err) - _ = wantZones } }) }