Skip to content

Commit

Permalink
Changed validation code to handle corner cases in zone struct where
Browse files Browse the repository at this point in the history 
labelValue or Name are either not specified or are empty values. Cleaned
up test code and added test cases for these scenarios.
  • Loading branch information
@anathoodell
anathoodell committed Dec 19, 2024
1 parent af9f31a commit 5779f0d
Show file tree
Hide file tree
Showing 2 changed files with 196 additions and 37 deletions.
51 changes: 33 additions & 18 deletions pkg/drivers/powerflex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"context"
"fmt"
"os"
"reflect"
"regexp"
"strings"

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
182 changes: 163 additions & 19 deletions pkg/drivers/powerflex_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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
}
})
}
Expand Down

0 comments on commit 5779f0d

Please sign in to comment.