Fix dirty node publish enforcement

This commit changes Trident to deny new CSI ControllerPublishVolume calls for CO nodes that have potentially unclean attachment states.

Author: jwebster7

storage_drivers/ontap/ontap_asa.go

@@ -1,4 +1,4 @@
-// Copyright 2024 NetApp, Inc. All Rights Reserved.
+// Copyright 2025 NetApp, Inc. All Rights Reserved.
 
 package ontap
 
@@ -1323,6 +1323,10 @@ func (d *ASAStorageDriver) CanEnablePublishEnforcement() bool {
 	return true
 }
 
+func (d *ASAStorageDriver) HealVolumePublishEnforcement(ctx context.Context, volume *storage.Volume) bool {
+	return HealSANPublishEnforcement(ctx, d, volume)
+}
+
 // CreateASALUNInternalID creates a string in the format /svm/<svm_name>/lun/<lun_name>
 func (d *ASAStorageDriver) CreateASALUNInternalID(svm, name string) string {
 	return fmt.Sprintf("/svm/%s/lun/%s", svm, name)

storage_drivers/ontap/ontap_asa_nvme.go

@@ -1479,6 +1479,10 @@ func (d *ASANVMeStorageDriver) CanEnablePublishEnforcement() bool {
 	return true
 }
 
+func (d *ASANVMeStorageDriver) HealVolumePublishEnforcement(ctx context.Context, volume *storage.Volume) bool {
+	return HealSANPublishEnforcement(ctx, d, volume)
+}
+
 // CreateASANVMeNamespaceInternalID creates a string in the format /svm/<svm_name>/<namespace_name>
 func (d *ASANVMeStorageDriver) CreateASANVMeNamespaceInternalID(svm, name string) string {
 	return fmt.Sprintf("/svm/%s/namespace/%s", svm, name)

storage_drivers/ontap/ontap_common.go

@@ -4576,6 +4576,33 @@ func EnableSANPublishEnforcement(
 	return nil
 }
 
+// HealSANPublishEnforcement is a no-op for ONTAP-SAN volumes because ONTAP-SAN already properly sets
+// the LUN mappings during publish/unpublish,
+// operations. This function is implemented to satisfy interface assertions on the drivers.
+func HealSANPublishEnforcement(_ context.Context, _ storage.Driver, _ *storage.Volume) bool {
+	return false
+}
+
+// HealNASPublishEnforcement checks if publish enforcement should be enabled on the given NAS volume
+// and updates the volume config accordingly. It returns true if the volume config was updated.
+func HealNASPublishEnforcement(ctx context.Context, driver storage.Driver, volume *storage.Volume) bool {
+	var updated bool
+	// Check if publish enforcement is already set.
+	if volume.Config.AccessInfo.PublishEnforcement {
+		// If publish enforcement is already enabled on the volume, nothing to do.
+		return updated
+	}
+
+	policy := volume.Config.ExportPolicy
+	driverConfig := driver.GetCommonConfig(ctx)
+	if policy == getEmptyExportPolicyName(*driverConfig.StoragePrefix) ||
+		policy == volume.Config.InternalName {
+		volume.Config.AccessInfo.PublishEnforcement = true
+		updated = true
+	}
+	return updated
+}
+
 func ValidateStoragePrefixEconomy(storagePrefix string) error {
 	// Ensure storage prefix is compatible with ONTAP
 	matched, err := regexp.MatchString(`^$|^[a-zA-Z0-9_.-]*$`, storagePrefix)

storage_drivers/ontap/ontap_common_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/netapp/trident/storage"
 	sa "github.com/netapp/trident/storage_attribute"
 	drivers "github.com/netapp/trident/storage_drivers"
+	fakeDriver "github.com/netapp/trident/storage_drivers/fake"
 	"github.com/netapp/trident/storage_drivers/ontap/api"
 	"github.com/netapp/trident/storage_drivers/ontap/api/azgo"
 	"github.com/netapp/trident/storage_drivers/ontap/api/rest/client/networking"
@@ -10370,3 +10371,102 @@ func TestCleanupFailedCloneFlexVol(t *testing.T) {
 		})
 	}
 }
+
+func TestHealNASPublishEnforcement(t *testing.T) {
+	tt := map[string]struct {
+		makeDriver func() storage.Driver
+		volume     *storage.Volume
+		assertBool assert.BoolAssertionFunc
+	}{
+		"enables publish enforcement when policy is the same as internal name": {
+			makeDriver: func() storage.Driver {
+				config := drivers.FakeStorageDriverConfig{
+					CommonStorageDriverConfig: &drivers.CommonStorageDriverConfig{
+						StorageDriverName: "fakeDriver",
+						StoragePrefix:     convert.ToPtr("fake_"),
+					},
+				}
+				return fakeDriver.NewFakeStorageDriver(ctx, config)
+			},
+			volume: &storage.Volume{
+				Config: &storage.VolumeConfig{
+					InternalName: "pvc-test-name",
+					ExportPolicy: "pvc-test-name",
+					AccessInfo: tridentmodels.VolumeAccessInfo{
+						PublishEnforcement: false,
+					},
+				},
+			},
+			assertBool: assert.True,
+		},
+		"enables publish enforcement when policy is the empty export policy": {
+			makeDriver: func() storage.Driver {
+				config := drivers.FakeStorageDriverConfig{
+					CommonStorageDriverConfig: &drivers.CommonStorageDriverConfig{
+						StorageDriverName: "fakeDriver",
+						StoragePrefix:     convert.ToPtr("fake_"),
+					},
+				}
+				return fakeDriver.NewFakeStorageDriver(ctx, config)
+			},
+			volume: &storage.Volume{
+				Config: &storage.VolumeConfig{
+					InternalName: "pvc-test-name",
+					ExportPolicy: getEmptyExportPolicyName("fake_"),
+					AccessInfo: tridentmodels.VolumeAccessInfo{
+						PublishEnforcement: false,
+					},
+				},
+			},
+			assertBool: assert.True,
+		},
+		"does not enable publish enforcement when policy is not empty and policy is not the same as the volume": {
+			makeDriver: func() storage.Driver {
+				config := drivers.FakeStorageDriverConfig{
+					CommonStorageDriverConfig: &drivers.CommonStorageDriverConfig{
+						StorageDriverName: "fakeDriver",
+						StoragePrefix:     convert.ToPtr("fake_"),
+					},
+				}
+				return fakeDriver.NewFakeStorageDriver(ctx, config)
+			},
+			volume: &storage.Volume{
+				Config: &storage.VolumeConfig{
+					InternalName: "pvc-test-name",
+					ExportPolicy: "trident-export-policy",
+					AccessInfo: tridentmodels.VolumeAccessInfo{
+						PublishEnforcement: false,
+					},
+				},
+			},
+			assertBool: assert.False,
+		},
+		"does not update if publish enforcement is alread set": {
+			makeDriver: func() storage.Driver {
+				config := drivers.FakeStorageDriverConfig{
+					CommonStorageDriverConfig: &drivers.CommonStorageDriverConfig{
+						StorageDriverName: "fakeDriver",
+						StoragePrefix:     convert.ToPtr("fake_"),
+					},
+				}
+				return fakeDriver.NewFakeStorageDriver(ctx, config)
+			},
+			volume: &storage.Volume{
+				Config: &storage.VolumeConfig{
+					InternalName: "pvc-test-name",
+					ExportPolicy: "trident-export-policy",
+					AccessInfo: tridentmodels.VolumeAccessInfo{
+						PublishEnforcement: true,
+					},
+				},
+			},
+			assertBool: assert.False,
+		},
+	}
+
+	for name, fixture := range tt {
+		t.Run(name, func(t *testing.T) {
+			fixture.assertBool(t, HealNASPublishEnforcement(ctx, fixture.makeDriver(), fixture.volume))
+		})
+	}
+}

storage_drivers/ontap/ontap_nas.go

@@ -1991,19 +1991,5 @@ func (d *NASStorageDriver) CanEnablePublishEnforcement() bool {
 func (d *NASStorageDriver) HealVolumePublishEnforcement(
 	ctx context.Context, vol *storage.Volume,
 ) bool {
-	var updated bool
-	// Check of publish enforcment is already set
-	if vol.Config.AccessInfo.PublishEnforcement {
-		// If publish enforcement is already enabled on the volume, nothing to do.
-		return updated
-	}
-
-	policy := vol.Config.ExportPolicy
-	driverConfig := d.GetCommonConfig(ctx)
-	if policy == getEmptyExportPolicyName(*driverConfig.StoragePrefix) ||
-		policy == vol.Config.InternalName {
-		vol.Config.AccessInfo.PublishEnforcement = true
-		updated = true
-	}
-	return updated
+	return HealNASPublishEnforcement(ctx, d, vol)
 }

storage_drivers/ontap/ontap_nas_qtree.go

@@ -2789,19 +2789,5 @@ func (d *NASQtreeStorageDriver) CanEnablePublishEnforcement() bool {
 func (d *NASQtreeStorageDriver) HealVolumePublishEnforcement(
 	ctx context.Context, vol *storage.Volume,
 ) bool {
-	var updated bool
-	// Check of publish enforcment is already set
-	if vol.Config.AccessInfo.PublishEnforcement {
-		// If publish enforcement is already enabled on the volume, nothing to do.
-		return updated
-	}
-
-	policy := vol.Config.ExportPolicy
-	driverConfig := d.GetCommonConfig(ctx)
-	if policy == getEmptyExportPolicyName(*driverConfig.StoragePrefix) ||
-		policy == vol.Config.InternalName {
-		vol.Config.AccessInfo.PublishEnforcement = true
-		updated = true
-	}
-	return updated
+	return HealNASPublishEnforcement(ctx, d, vol)
 }

storage_drivers/ontap/ontap_san.go

@@ -1807,3 +1807,7 @@ func (d *SANStorageDriver) EnablePublishEnforcement(ctx context.Context, volume
 func (d *SANStorageDriver) CanEnablePublishEnforcement() bool {
 	return true
 }
+
+func (d *SANStorageDriver) HealVolumePublishEnforcement(ctx context.Context, volume *storage.Volume) bool {
+	return HealSANPublishEnforcement(ctx, d, volume)
+}

storage_drivers/ontap/ontap_san_economy.go

@@ -2809,6 +2809,10 @@ func (d *SANEconomyStorageDriver) CanEnablePublishEnforcement() bool {
 	return true
 }
 
+func (d *SANEconomyStorageDriver) HealVolumePublishEnforcement(ctx context.Context, volume *storage.Volume) bool {
+	return HealSANPublishEnforcement(ctx, d, volume)
+}
+
 // ParseLunInternalID parses the passed string which is in the format /svm/<svm_name>/flexvol/<flexvol_name>/lun/<lun_name>
 // and returns svm, flexvol and LUN name.
 func (d SANEconomyStorageDriver) ParseLunInternalID(internalId string) (svm, flexvol, lun string, err error) {

storage_drivers/ontap/ontap_san_nvme.go

@@ -1833,3 +1833,7 @@ func (d *NVMeStorageDriver) EnablePublishEnforcement(_ context.Context, volume *
 func (d *NVMeStorageDriver) CanEnablePublishEnforcement() bool {
 	return true
 }
+
+func (d *NVMeStorageDriver) HealVolumePublishEnforcement(ctx context.Context, volume *storage.Volume) bool {
+	return HealSANPublishEnforcement(ctx, d, volume)
+}