protosanitizer: add test for converted message

One real use case is that users of an older spec can receive a message
based on a new spec (possible as long as backwards compatibility is
preserved) without logging any secrets added in the new spec. It is
unlikely, but it is nonetheless better to have a test for it.

Author: pohly

protosanitizer/protosanitizer_test.go

@@ -21,13 +21,16 @@ import (
 	"testing"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
+	"github.com/golang/protobuf/proto"
 	"github.com/kubernetes-csi/csi-lib-utils/protosanitizer/test/csitest"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestStripSecrets(t *testing.T) {
 	secretName := "secret-abc"
 	secretValue := "123"
+
+	// Current spec.
 	createVolume := &csi.CreateVolumeRequest{
 		AccessibilityRequirements: &csi.TopologyRequirement{
 			Requisite: []*csi.Topology{
@@ -63,9 +66,50 @@ func TestStripSecrets(t *testing.T) {
 		},
 	}
 
-	cases := []struct {
+	// Revised spec with more secret fields.
+	createVolumeFuture := &csitest.CreateVolumeRequest{
+		CapacityRange: &csitest.CapacityRange{
+			RequiredBytes: 1024,
+		},
+		MaybeSecretMap: map[int64]*csitest.VolumeCapability{
+			1: &csitest.VolumeCapability{ArraySecret: "aaa"},
+			2: &csitest.VolumeCapability{ArraySecret: "bbb"},
+		},
+		Name:         "foo",
+		NewSecretInt: 42,
+		Seecreets: map[string]string{
+			secretName:   secretValue,
+			"secret-xyz": "987",
+		},
+		VolumeCapabilities: []*csitest.VolumeCapability{
+			&csitest.VolumeCapability{
+				AccessType: &csitest.VolumeCapability_Mount{
+					Mount: &csitest.VolumeCapability_MountVolume{
+						FsType: "ext4",
+					},
+				},
+				ArraySecret: "knock knock",
+			},
+			&csitest.VolumeCapability{
+				ArraySecret: "Who's there?",
+			},
+		},
+		VolumeContentSource: &csitest.VolumeContentSource{
+			Type: &csitest.VolumeContentSource_Volume{
+				Volume: &csitest.VolumeContentSource_VolumeSource{
+					VolumeId:         "abc",
+					OneofSecretField: "hello",
+				},
+			},
+			NestedSecretField: "world",
+		},
+	}
+
+	type testcase struct {
 		original, stripped interface{}
-	}{
+	}
+
+	cases := []testcase{
 		{nil, "null"},
 		{1, "1"},
 		{"hello world", `"hello world"`},
@@ -99,43 +143,7 @@ func TestStripSecrets(t *testing.T) {
 		}, `{"accessibility_requirements":{},"capacity_range":{"limit_bytes":1024,"required_bytes":1024},"name":"test-volume","parameters":{"param1":"param1","param2":"param2"},"secrets":"***stripped***","volume_capabilities":[{"AccessType":{"Mount":{"fs_type":"ext4","mount_flags":["flag1","flag2","flag3"]}},"access_mode":{"mode":5}}],"volume_content_source":{"Type":null}}`},
 		{createVolume, `{"accessibility_requirements":{"requisite":[{"segments":{"foo":"bar","x":"y"}},{"segments":{"a":"b"}}]},"capacity_range":{"required_bytes":1024},"name":"foo","secrets":"***stripped***","volume_capabilities":[{"AccessType":{"Mount":{"fs_type":"ext4"}}}]}`},
 		{&csitest.CreateVolumeRequest{}, `{}`},
-		{&csitest.CreateVolumeRequest{
-			CapacityRange: &csitest.CapacityRange{
-				RequiredBytes: 1024,
-			},
-			MaybeSecretMap: map[int64]*csitest.VolumeCapability{
-				1: &csitest.VolumeCapability{ArraySecret: "aaa"},
-				2: &csitest.VolumeCapability{ArraySecret: "bbb"},
-			},
-			Name:         "foo",
-			NewSecretInt: 42,
-			Seecreets: map[string]string{
-				secretName:   secretValue,
-				"secret-xyz": "987",
-			},
-			VolumeCapabilities: []*csitest.VolumeCapability{
-				&csitest.VolumeCapability{
-					AccessType: &csitest.VolumeCapability_Mount{
-						Mount: &csitest.VolumeCapability_MountVolume{
-							FsType: "ext4",
-						},
-					},
-					ArraySecret: "knock knock",
-				},
-				&csitest.VolumeCapability{
-					ArraySecret: "Who's there?",
-				},
-			},
-			VolumeContentSource: &csitest.VolumeContentSource{
-				Type: &csitest.VolumeContentSource_Volume{
-					Volume: &csitest.VolumeContentSource_VolumeSource{
-						VolumeId:         "abc",
-						OneofSecretField: "hello",
-					},
-				},
-				NestedSecretField: "world",
-			},
-		},
+		{createVolumeFuture,
 			// Secrets are *not* removed from all fields yet. This will have to be fixed one way or another
 			// before the CSI spec can start using secrets there (currently it doesn't).
 			// The test is still useful because it shows that also complicated fields get serialized.
@@ -144,6 +152,17 @@ func TestStripSecrets(t *testing.T) {
 		},
 	}
 
+	// Message from revised spec as received by a sidecar based on the current spec.
+	// The XXX_unrecognized field contains secrets and must not get logged.
+	unknownFields := &csi.CreateVolumeRequest{}
+	data, err := proto.Marshal(createVolumeFuture)
+	if assert.NoError(t, err, "marshall future message") &&
+		assert.NoError(t, proto.Unmarshal(data, unknownFields), "unmarshal with unknown fields") {
+		cases = append(cases, testcase{unknownFields,
+			`{"capacity_range":{"required_bytes":1024},"name":"foo","secrets":"***stripped***","volume_capabilities":[{"AccessType":{"Mount":{"fs_type":"ext4"}}},{"AccessType":null}],"volume_content_source":{"Type":{"Volume":{"volume_id":"abc"}}}}`,
+		})
+	}
+
 	for _, c := range cases {
 		before := fmt.Sprint(c.original)
 		stripped := StripSecrets(c.original)