mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-26 21:17:23 +00:00
Merge pull request #70742 from ddebroy/ddebroy-csinode1
Add validation of CSINodeInfo fields before Create/Update actions
This commit is contained in:
commit
a04e854428
@ -21,6 +21,7 @@ package nodeinfomanager // import "k8s.io/kubernetes/pkg/volume/csi/nodeinfomana
|
|||||||
import (
|
import (
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
|
||||||
csipb "github.com/container-storage-interface/spec/lib/go/csi/v0"
|
csipb "github.com/container-storage-interface/spec/lib/go/csi/v0"
|
||||||
"k8s.io/api/core/v1"
|
"k8s.io/api/core/v1"
|
||||||
@ -458,7 +459,11 @@ func (nim *nodeInfoManager) installDriverToCSINodeInfo(
|
|||||||
nodeInfo.Spec.Drivers = newDriverSpecs
|
nodeInfo.Spec.Drivers = newDriverSpecs
|
||||||
nodeInfo.Status.Drivers = newDriverStatuses
|
nodeInfo.Status.Drivers = newDriverStatuses
|
||||||
|
|
||||||
_, err := csiKubeClient.CsiV1alpha1().CSINodeInfos().Update(nodeInfo)
|
err := validateCSINodeInfo(nodeInfo)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
_, err = csiKubeClient.CsiV1alpha1().CSINodeInfos().Update(nodeInfo)
|
||||||
return err // do not wrap error
|
return err // do not wrap error
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -494,7 +499,10 @@ func (nim *nodeInfoManager) uninstallDriverFromCSINodeInfo(csiDriverName string)
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO (verult) make sure CSINodeInfo has validation logic to prevent duplicate driver names
|
err = validateCSINodeInfo(nodeInfo)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
_, updateErr := nodeInfoClient.Update(nodeInfo)
|
_, updateErr := nodeInfoClient.Update(nodeInfo)
|
||||||
return updateErr // do not wrap error
|
return updateErr // do not wrap error
|
||||||
|
|
||||||
@ -557,3 +565,50 @@ func removeMaxAttachLimit(driverName string) nodeUpdateFunc {
|
|||||||
return node, true, nil
|
return node, true, nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// validateCSINodeInfo ensures members of CSINodeInfo object satisfies map and set semantics.
|
||||||
|
// Before calling CSINodeInfoInterface.Update(), validateCSINodeInfo() should be invoked to
|
||||||
|
// make sure the CSINodeInfo is compliant
|
||||||
|
func validateCSINodeInfo(nodeInfo *csiv1alpha1.CSINodeInfo) error {
|
||||||
|
if len(nodeInfo.Status.Drivers) < 1 {
|
||||||
|
return fmt.Errorf("at least one Driver entry is required in driver statuses")
|
||||||
|
}
|
||||||
|
if len(nodeInfo.Spec.Drivers) < 1 {
|
||||||
|
return fmt.Errorf("at least one Driver entry is required in driver specs")
|
||||||
|
}
|
||||||
|
if len(nodeInfo.Status.Drivers) != len(nodeInfo.Spec.Drivers) {
|
||||||
|
return fmt.Errorf("")
|
||||||
|
}
|
||||||
|
// check for duplicate entries for the same driver in statuses
|
||||||
|
var errors []string
|
||||||
|
driverNamesInStatuses := make(sets.String)
|
||||||
|
for _, driverInfo := range nodeInfo.Status.Drivers {
|
||||||
|
if driverNamesInStatuses.Has(driverInfo.Name) {
|
||||||
|
errors = append(errors, fmt.Sprintf("duplicate entries found for driver: %s in driver statuses", driverInfo.Name))
|
||||||
|
}
|
||||||
|
driverNamesInStatuses.Insert(driverInfo.Name)
|
||||||
|
}
|
||||||
|
// check for duplicate entries for the same driver in specs
|
||||||
|
driverNamesInSpecs := make(sets.String)
|
||||||
|
for _, driverInfo := range nodeInfo.Spec.Drivers {
|
||||||
|
if driverNamesInSpecs.Has(driverInfo.Name) {
|
||||||
|
errors = append(errors, fmt.Sprintf("duplicate entries found for driver: %s in driver specs", driverInfo.Name))
|
||||||
|
}
|
||||||
|
driverNamesInSpecs.Insert(driverInfo.Name)
|
||||||
|
topoKeys := make(sets.String)
|
||||||
|
for _, key := range driverInfo.TopologyKeys {
|
||||||
|
if topoKeys.Has(key) {
|
||||||
|
errors = append(errors, fmt.Sprintf("duplicate topology keys %s found for driver %s in driver specs", key, driverInfo.Name))
|
||||||
|
}
|
||||||
|
topoKeys.Insert(key)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// check all entries in specs and status match
|
||||||
|
if !driverNamesInSpecs.Equal(driverNamesInStatuses) {
|
||||||
|
errors = append(errors, fmt.Sprintf("list of drivers in specs: %v does not match list of drivers in statuses: %v", driverNamesInSpecs.List(), driverNamesInStatuses.List()))
|
||||||
|
}
|
||||||
|
if len(errors) == 0 {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
return fmt.Errorf(strings.Join(errors, ", "))
|
||||||
|
}
|
||||||
|
@ -637,6 +637,296 @@ func TestInstallCSIDriverExistingAnnotation(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestValidateCSINodeInfo(t *testing.T) {
|
||||||
|
testcases := []struct {
|
||||||
|
name string
|
||||||
|
nodeInfo *csiv1alpha1.CSINodeInfo
|
||||||
|
expectErr bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "multiple drivers with different node IDs, topology keys and status",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1, key2"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driverB",
|
||||||
|
NodeID: "nodeA",
|
||||||
|
TopologyKeys: []string{"keyA", "keyB"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "in-tree",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driverB",
|
||||||
|
Available: false,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "multiple drivers with same node IDs, topology keys and status",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver2",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver2",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "duplicate drivers in driver specs",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1", "key2"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "nodeX",
|
||||||
|
TopologyKeys: []string{"keyA", "keyB"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "duplicate drivers in driver statuses",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1", "key2"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "in-tree",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: false,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "single driver with duplicate topology keys in driver specs",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1", "key1"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "multiple drivers with one set of duplicate topology keys in driver specs",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver2",
|
||||||
|
NodeID: "nodeX",
|
||||||
|
TopologyKeys: []string{"keyA", "keyA"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver2",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "mismatch between drivers in specs and status (null intersection)",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver2",
|
||||||
|
NodeID: "nodeX",
|
||||||
|
TopologyKeys: []string{"keyA", "keyA"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver3",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "mismatch between drivers in specs and status (specs superset of status)",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver2",
|
||||||
|
NodeID: "nodeX",
|
||||||
|
TopologyKeys: []string{"keyA", "keyA"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "mismatch between drivers in specs and status (specs subset of status)",
|
||||||
|
nodeInfo: &csiv1alpha1.CSINodeInfo{
|
||||||
|
Spec: csiv1alpha1.CSINodeInfoSpec{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoSpec{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
NodeID: "node1",
|
||||||
|
TopologyKeys: []string{"key1"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Status: csiv1alpha1.CSINodeInfoStatus{
|
||||||
|
Drivers: []csiv1alpha1.CSIDriverInfoStatus{
|
||||||
|
{
|
||||||
|
Name: "driver1",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Name: "driver2",
|
||||||
|
Available: true,
|
||||||
|
VolumePluginMechanism: "csi",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
expectErr: true,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
for _, tc := range testcases {
|
||||||
|
t.Logf("test case: %q", tc.name)
|
||||||
|
err := validateCSINodeInfo(tc.nodeInfo)
|
||||||
|
if err != nil && !tc.expectErr {
|
||||||
|
t.Errorf("expected no errors from validateCSINodeInfo but got error %v", err)
|
||||||
|
}
|
||||||
|
if err == nil && tc.expectErr {
|
||||||
|
t.Errorf("expected error from validateCSINodeInfo but got no errors")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []testcase) {
|
func test(t *testing.T, addNodeInfo bool, csiNodeInfoEnabled bool, testcases []testcase) {
|
||||||
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSINodeInfo, csiNodeInfoEnabled)()
|
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSINodeInfo, csiNodeInfoEnabled)()
|
||||||
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AttachVolumeLimit, true)()
|
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AttachVolumeLimit, true)()
|
||||||
|
Loading…
Reference in New Issue
Block a user