Skip to content

Commit

Permalink
Powerflex secret zone validation (#833)
Browse files Browse the repository at this point in the history 
* ZoneValidation() to PowerFlex PreChecks that calls into
drivers.ExtractZones. Fixed up existing unit-tests to pass around a
proper secret, rather than the "csm" value currently used, as this
causes the ExtractZones() to fail on parsing during unit-testing.

* Removing node label code since we do not need to validate against
node labels anymore. Removing the directly associated unit test code as
well. Fixing powerflex validate zone code to check that all arrays
have zone detail if there is at least one array with zone detail and 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.
  • Loading branch information
@anathoodell
anathoodell authored Dec 19, 2024
1 parent a892d43 commit 57bbb0c
Show file tree
Hide file tree
Showing 10 changed files with 406 additions and 118 deletions.
17 changes: 11 additions & 6 deletions controllers/csm_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1334,6 +1334,11 @@ func (r *ContainerStorageModuleReconciler) PreChecks(ctx context.Context, cr *cs
if err != nil {
return fmt.Errorf("failed powerflex validation: %v", err)
}
// zoning initially applies only to pflex
err = r.ZoneValidation(ctx, cr)
if err != nil {
return fmt.Errorf("error during zone validation: %v", err)
}
case csmv1.PowerStore:
err := drivers.PrecheckPowerStore(ctx, cr, operatorConfig, r.GetClient())
if err != nil {
Expand Down Expand Up @@ -1524,12 +1529,12 @@ func (r *ContainerStorageModuleReconciler) GetK8sClient() kubernetes.Interface {
return r.K8sClient
}

func (r *ContainerStorageModuleReconciler) GetMatchingNodes(ctx context.Context, labelKey string, labelValue string) (*corev1.NodeList, error) {
nodeList := &corev1.NodeList{}
opts := []client.ListOption{
client.MatchingLabels{labelKey: labelValue},
// ZoneValidation - If zones are configured performs validation and returns an error if the zone validation fails
func (r *ContainerStorageModuleReconciler) ZoneValidation(ctx context.Context, cr *csmv1.ContainerStorageModule) error {
err := drivers.ValidateZones(ctx, cr, r.Client)
if err != nil {
return fmt.Errorf("zone validation failed with error: %v", err)
}
err := r.List(ctx, nodeList, opts...)

return nodeList, err
return err
}
66 changes: 27 additions & 39 deletions controllers/csm_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ func (suite *CSMControllerTestSuite) TestCsmDowngrade() {
if err != nil {
panic(err)
}
sec := shared.MakeSecret(csmName+"-config", suite.namespace, pFlexConfigVersion)
sec := shared.MakeSecretPowerFlex(csmName+"-config", suite.namespace, pFlexConfigVersion)
err = suite.fakeClient.Create(ctx, sec)
if err != nil {
panic(err)
Expand Down Expand Up @@ -653,7 +653,7 @@ func (suite *CSMControllerTestSuite) TestCsmDowngradeSkipVersion() {
if err != nil {
panic(err)
}
sec := shared.MakeSecret(csmName+"-config", suite.namespace, pFlexConfigVersion)
sec := shared.MakeSecretPowerFlex(csmName+"-config", suite.namespace, pFlexConfigVersion)
err = suite.fakeClient.Create(ctx, sec)
if err != nil {
panic(err)
Expand Down Expand Up @@ -2383,56 +2383,44 @@ func (suite *CSMControllerTestSuite) makeFakeRevProxyCSM(name string, ns string,
assert.Nil(suite.T(), err)
}

func (suite *CSMControllerTestSuite) TestGetNodeLabels() {
// TBD: crclient/client.go needs to be augmented to filter on labels during
// the List return for a viable thorough test. Since this functionality is
// missing, this test is quite elementary as a result.

func (suite *CSMControllerTestSuite) TestZoneValidation() {
csm := shared.MakeCSM(csmName, suite.namespace, configVersion)
csm.Spec.Driver.CSIDriverType = csmv1.PowerScale
csm.Spec.Driver.CSIDriverType = csmv1.PowerFlex
csm.Spec.Driver.Common.Image = "image"
csm.Annotations[configVersionKey] = configVersion

sec := shared.MakeSecret(csmName+"-creds", suite.namespace, configVersion)
err := suite.fakeClient.Create(ctx, sec)
if err != nil {
panic(err)
}

csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
err = suite.fakeClient.Create(ctx, &csm)
if err != nil {
panic(err)
}
err := suite.fakeClient.Create(ctx, &csm)
assert.Nil(suite.T(), err)

reconciler := suite.createReconciler()

// create node object, add to fakeclient, reconcile.GetMatchingNodes
node := shared.MakeNode("node1", suite.namespace)
node.Labels["topology.kubernetes.io/zone"] = "US-EAST"

err = suite.fakeClient.Create(ctx, &node)
// add secret with NO zone to the namespace
sec := shared.MakeSecretPowerFlex(csmName+"-config", suite.namespace, pFlexConfigVersion)
err = suite.fakeClient.Create(ctx, sec)
assert.Nil(suite.T(), err)

nodeList := &corev1.NodeList{}
err = suite.fakeClient.List(ctx, nodeList, nil)
err = reconciler.ZoneValidation(ctx, &csm)
assert.Nil(suite.T(), err)
}

nodeListMatching, err := reconciler.GetMatchingNodes(ctx, "topology.kubernetes.io/zone", "US-EAST")
ctrl.Log.Info("node list response (1)", "number of nodes is: ", len(nodeListMatching.Items))
func (suite *CSMControllerTestSuite) TestZoneValidation2() {
csm := shared.MakeCSM(csmName, suite.namespace, configVersion)
csm.Spec.Driver.CSIDriverType = csmv1.PowerFlex
csm.Spec.Driver.Common.Image = "image"
csm.Annotations[configVersionKey] = configVersion

// Check the len to be 1 else fail
if len(nodeListMatching.Items) != 1 {
ctrl.Log.Error(err, "Unexpected length on node list.", "length", len(nodeListMatching.Items))
panic(err)
}
csm.ObjectMeta.Finalizers = []string{CSMFinalizerName}
err := suite.fakeClient.Create(ctx, &csm)
assert.Nil(suite.T(), err)

for _, node := range nodeListMatching.Items {
ctrl.Log.Info("Matching node item", "name ", node.ObjectMeta.GetName())
}
if node.ObjectMeta.GetName() != "node1" {
ctrl.Log.Error(err, "Unmatched label on node.")
panic(err)
}
reconciler := suite.createReconciler()

// add secret with an invalid multi zone to the namespace
secretZone := shared.MakeSecretPowerFlexMultiZoneInvalid(csmName+"-config", suite.namespace, pFlexConfigVersion)
err = suite.fakeClient.Create(ctx, secretZone)
assert.Nil(suite.T(), err)

err = reconciler.ZoneValidation(ctx, &csm)
assert.NotNil(suite.T(), err)
}
8 changes: 4 additions & 4 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ require (
github.com/x448/float16 v0.8.4 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/exp v0.0.0-20240416160154-fe59bbe5cc7f // indirect
golang.org/x/net v0.31.0 // indirect
golang.org/x/net v0.33.0 // indirect
golang.org/x/oauth2 v0.21.0 // indirect
golang.org/x/sys v0.27.0 // indirect
golang.org/x/term v0.26.0 // indirect
golang.org/x/text v0.20.0 // indirect
golang.org/x/sys v0.28.0 // indirect
golang.org/x/term v0.27.0 // indirect
golang.org/x/text v0.21.0 // indirect
golang.org/x/time v0.5.0 // indirect
gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
google.golang.org/protobuf v1.35.1 // indirect
Expand Down
16 changes: 8 additions & 8 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
golang.org/x/net v0.31.0 h1:68CPQngjLL0r2AlUKiSxtQFKvzRVbnzLwMUn5SzcLHo=
golang.org/x/net v0.31.0/go.mod h1:P4fl1q7dY2hnZFxEk4pPSkDHF+QqjitcnDjUQyMM+pM=
golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I=
golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4=
golang.org/x/oauth2 v0.21.0 h1:tsimM75w1tF/uws5rbeHzIWxEqElMehnc+iW793zsZs=
golang.org/x/oauth2 v0.21.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand All @@ -130,14 +130,14 @@ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s=
golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.26.0 h1:WEQa6V3Gja/BhNxg540hBip/kkaYtRg3cxg4oXSw4AU=
golang.org/x/term v0.26.0/go.mod h1:Si5m1o57C5nBNQo5z1iq+XDijt21BDBDp2bK0QI8e3E=
golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA=
golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.27.0 h1:WP60Sv1nlK1T6SupCHbXzSaN0b9wUmsPoRS9b61A23Q=
golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.20.0 h1:gK/Kv2otX8gz+wn7Rmb3vT96ZwuoxnQlY+HlJVj7Qug=
golang.org/x/text v0.20.0/go.mod h1:D4IsuqiFMhST5bX19pQ9ikHC2GsaKyk/oF+pn3ducp4=
golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
Expand Down
72 changes: 54 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 @@ -347,52 +348,87 @@ func ModifyPowerflexCR(yamlString string, cr csmv1.ContainerStorageModule, fileT
return yamlString
}

// ExtractZonesFromSecret - Reads the array config secret and returns the zone label information
func ExtractZonesFromSecret(ctx context.Context, kube client.Client, namespace string, secret string) (map[string]string, error) {
// ValidateZones - zone validation for topology aware clusters
func ValidateZones(ctx context.Context, cr *csmv1.ContainerStorageModule, ct client.Client) error {
secretName := cr.Name + "-config"
err := ValidateZonesInSecret(ctx, ct, cr.Namespace, secretName)
return err
}

// ValidateZonesInSecret - inspects incoming secret for zone validity
func ValidateZonesInSecret(ctx context.Context, kube client.Client, namespace string, secret string) error {
log := logger.GetLogger(ctx)

arraySecret, err := utils.GetSecret(ctx, secret, namespace, kube)
if err != nil {
return nil, 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"`
} `json:"zone"`
SystemID string `json:"systemID"`
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 nil, 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 nil, fmt.Errorf("unable to unmarshal multi-array configuration[%v]", err)
return fmt.Errorf("unable to unmarshal array secret %v", err)
}

var labelKey string
var numArrays, numArraysWithZone int
numArrays = len(yamlConfig)
for _, configParam := range yamlConfig {
if configParam.SystemID == "" {
return nil, fmt.Errorf("invalid value for SystemID")
return fmt.Errorf("invalid value for SystemID")
}
if configParam.Zone.Name != "" {
zonesMapData[configParam.SystemID] = configParam.Zone.Name
log.Infof("Zoning information configured for systemID %s: %v ", configParam.SystemID, zonesMapData)
if reflect.DeepEqual(configParam.Zone, Zone{}) {
log.Infof("Zone is not specified for SystemID: %s", configParam.SystemID)
} else {
log.Info("Zoning information not found in the array config. Continue with topology-unaware driver installation mode")
return zonesMapData, nil
log.Infof("Zone is specified for SystemID: %s", 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)
}
numArraysWithZone++
}
}

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 secret. Continue with topology-unaware driver installation mode")
}
} else {
return nil, fmt.Errorf("Array details are not provided in vxflexos-config secret")
return fmt.Errorf("array details are not provided in secret")
}

return zonesMapData, nil
return nil
}
Loading

0 comments on commit 57bbb0c

Please sign in to comment.