Backport of: Fix secrets redaction for CSI volumes to avoid state corruption (#27176) (#27183)

In #12583 we changed the serialization code for CSI volumes so that we were
using the extension method we use for topology and nodes. This reduces an
enormous amount of boilerplate code, but we introduced a state store corruption
bug in the process. The extension method sanitizes the volume without copying
it, so a read of the volume (such as getting an event from the event stream) can
cause the volume's secrets to be redacted in subsequent requests to publish or
mount the volume.

Move the sanitization code into a testable method on the volume, and add a copy
to the method.

Ref: #12583
Fixes: #26766

Co-authored-by: Tim Gross <[email protected]>

Author: hc-github-team-nomad-core

.changelog/27176.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+csi: Fixed a bug where reading a volume from the API or event stream could erase its secrets
+```

nomad/structs/csi.go

@@ -578,6 +578,27 @@ func (v *CSIVolume) Copy() *CSIVolume {
 	return out
 }
 
+// Sanitize returns a deep copy of the volume, with sensitive fields redacted
+func (v *CSIVolume) Sanitize() *CSIVolume {
+	if v == nil {
+		return nil
+	}
+
+	clean := v.Copy()
+
+	// would be better not to have at all but left in and redacted for backwards
+	// compatibility with the existing API
+	clean.Secrets = nil
+
+	// MountFlags can contain secrets, so we always redact it but want to show
+	// the user that we have the value
+	if v.MountOptions != nil {
+		clean.MountOptions = clean.MountOptions.Sanitize()
+	}
+
+	return clean
+}
+
 // Claim updates the allocations and changes the volume state
 func (v *CSIVolume) Claim(claim *CSIVolumeClaim, alloc *Allocation) error {
 	// COMPAT: volumes registered prior to 1.1.0 will be missing caps for the

nomad/structs/csi_test.go

@@ -1093,6 +1093,30 @@ func TestTaskCSIPluginConfig_Equal(t *testing.T) {
 	}})
 }
 
+func TestCSIVolumeSanitize(t *testing.T) {
+	ci.Parallel(t)
+
+	orig := &CSIVolume{
+		ID: "foo",
+		MountOptions: &CSIMountOptions{
+			FSType:     "ext4",
+			MountFlags: []string{"ro", "noatime"},
+		},
+		Secrets: CSISecrets{
+			"foo": "bar",
+			"baz": "qux",
+		},
+		Parameters: map[string]string{"example": "unchanged"},
+	}
+
+	sanitized := orig.Sanitize()
+	must.Eq(t, []string{"[REDACTED]"}, sanitized.MountOptions.MountFlags)
+	must.Nil(t, sanitized.Secrets)
+
+	orig.Parameters["example"] = "different"
+	must.Eq(t, "unchanged", sanitized.Parameters["example"])
+}
+
 func TestCSISecretsSanitize(t *testing.T) {
 	ci.Parallel(t)
 

nomad/structs/extensions.go

@@ -69,7 +69,7 @@ func nodeExt(v interface{}) interface{} {
 }
 
 func csiVolumeExt(v interface{}) interface{} {
-	vol := v.(*CSIVolume)
+	vol := v.(*CSIVolume).Sanitize()
 	type EmbeddedCSIVolume CSIVolume
 
 	allocCount := len(vol.ReadAllocs) + len(vol.WriteAllocs)
@@ -100,15 +100,5 @@ func csiVolumeExt(v interface{}) interface{} {
 		}
 	}
 
-	// MountFlags can contain secrets, so we always redact it but want
-	// to show the user that we have the value
-	if vol.MountOptions != nil && len(vol.MountOptions.MountFlags) > 0 {
-		apiVol.MountOptions.MountFlags = []string{"[REDACTED]"}
-	}
-
-	// would be better not to have at all but left in and redacted for
-	// backwards compatibility with the existing API
-	apiVol.Secrets = nil
-
 	return apiVol
 }