protosanitizer: variant of StripSecrets for CSI <= 0.3.0

The variant for the older CSI spec is relying on the naming of the
secret fields to determine which ones need to be removed. As all field
names in the older spec are known and fixed, that's good enough.

Author: pohly

protosanitizer/protosanitizer.go

@@ -36,15 +36,27 @@ import (
 // Instead of the secret value(s), the string "***stripped***" is
 // included in the result.
 //
+// StripSecrets relies on an extension in CSI 1.0 and thus can only
+// be used for messages based on that or a more recent spec!
+//
 // StripSecrets itself is fast and therefore it is cheap to pass the
 // result to logging functions which may or may not end up serializing
 // the parameter depending on the current log level.
 func StripSecrets(msg interface{}) fmt.Stringer {
-	return &stripSecrets{msg}
+	return &stripSecrets{msg, isCSI1Secret}
+}
+
+// StripSecretsCSI03 is like StripSecrets, except that it works
+// for messages based on CSI 0.3 and older. It does not work
+// for CSI 1.0, use StripSecrets for that.
+func StripSecretsCSI03(msg interface{}) fmt.Stringer {
+	return &stripSecrets{msg, isCSI03Secret}
 }
 
 type stripSecrets struct {
 	msg interface{}
+
+	isSecretField func(field *protobuf.FieldDescriptorProto) bool
 }
 
 func (s *stripSecrets) String() string {
@@ -60,7 +72,7 @@ func (s *stripSecrets) String() string {
 	}
 
 	// Now remove secrets from the generic representation of the message.
-	strip(parsed, s.msg)
+	s.strip(parsed, s.msg)
 
 	// Re-encoded the stripped representation and return that.
 	b, err = json.Marshal(parsed)
@@ -70,7 +82,7 @@ func (s *stripSecrets) String() string {
 	return string(b)
 }
 
-func strip(parsed interface{}, msg interface{}) {
+func (s *stripSecrets) strip(parsed interface{}, msg interface{}) {
 	protobufMsg, ok := msg.(descriptor.Message)
 	if !ok {
 		// Not a protobuf message, so we are done.
@@ -93,8 +105,7 @@ func strip(parsed interface{}, msg interface{}) {
 	fields := md.GetField()
 	if fields != nil {
 		for _, field := range fields {
-			ex, err := proto.GetExtension(field.Options, csi.E_CsiSecret)
-			if err == nil && ex != nil && *ex.(*bool) {
+			if s.isSecretField(field) {
 				// Overwrite only if already set.
 				if _, ok := parsedFields[field.GetName()]; ok {
 					parsedFields[field.GetName()] = "***stripped***"
@@ -126,13 +137,26 @@ func strip(parsed interface{}, msg interface{}) {
 				if slice, ok := entry.([]interface{}); ok {
 					// Array of values, like VolumeCapabilities in CreateVolumeRequest.
 					for _, entry := range slice {
-						strip(entry, i)
+						s.strip(entry, i)
 					}
 				} else {
 					// Single value.
-					strip(entry, i)
+					s.strip(entry, i)
 				}
 			}
 		}
 	}
 }
+
+// isCSI1Secret uses the csi.E_CsiSecret extension from CSI 1.0 to
+// determine whether a field contains secrets.
+func isCSI1Secret(field *protobuf.FieldDescriptorProto) bool {
+	ex, err := proto.GetExtension(field.Options, csi.E_CsiSecret)
+	return err == nil && ex != nil && *ex.(*bool)
+}
+
+// isCSI03Secret relies on the naming convention in CSI <= 0.3
+// to determine whether a field contains secrets.
+func isCSI03Secret(field *protobuf.FieldDescriptorProto) bool {
+	return strings.HasSuffix(field.GetName(), "_secrets")
+}

protosanitizer/protosanitizer_test.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/golang/protobuf/proto"
+	csi03 "github.com/kubernetes-csi/csi-lib-utils/protosanitizer/test/csi03"
 	"github.com/kubernetes-csi/csi-lib-utils/protosanitizer/test/csitest"
 	"github.com/stretchr/testify/assert"
 )
@@ -30,6 +31,42 @@ func TestStripSecrets(t *testing.T) {
 	secretName := "secret-abc"
 	secretValue := "123"
 
+	// CSI 0.3.0.
+	createVolumeCSI03 := &csi03.CreateVolumeRequest{
+		AccessibilityRequirements: &csi03.TopologyRequirement{
+			Requisite: []*csi03.Topology{
+				&csi03.Topology{
+					Segments: map[string]string{
+						"foo": "bar",
+						"x":   "y",
+					},
+				},
+				&csi03.Topology{
+					Segments: map[string]string{
+						"a": "b",
+					},
+				},
+			},
+		},
+		Name: "foo",
+		VolumeCapabilities: []*csi03.VolumeCapability{
+			&csi03.VolumeCapability{
+				AccessType: &csi03.VolumeCapability_Mount{
+					Mount: &csi03.VolumeCapability_MountVolume{
+						FsType: "ext4",
+					},
+				},
+			},
+		},
+		CapacityRange: &csi03.CapacityRange{
+			RequiredBytes: 1024,
+		},
+		ControllerCreateSecrets: map[string]string{
+			secretName:   secretValue,
+			"secret-xyz": "987",
+		},
+	}
+
 	// Current spec.
 	createVolume := &csi.CreateVolumeRequest{
 		AccessibilityRequirements: &csi.TopologyRequirement{
@@ -142,6 +179,7 @@ func TestStripSecrets(t *testing.T) {
 			AccessibilityRequirements: &csi.TopologyRequirement{},
 		}, `{"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"}}}]}`},
+		{createVolumeCSI03, `{"accessibility_requirements":{"requisite":[{"segments":{"foo":"bar","x":"y"}},{"segments":{"a":"b"}}]},"capacity_range":{"required_bytes":1024},"controller_create_secrets":"***stripped***","name":"foo","volume_capabilities":[{"AccessType":{"Mount":{"fs_type":"ext4"}}}]}`},
 		{&csitest.CreateVolumeRequest{}, `{}`},
 		{createVolumeFuture,
 			// Secrets are *not* removed from all fields yet. This will have to be fixed one way or another
@@ -165,7 +203,12 @@ func TestStripSecrets(t *testing.T) {
 
 	for _, c := range cases {
 		before := fmt.Sprint(c.original)
-		stripped := StripSecrets(c.original)
+		var stripped fmt.Stringer
+		if _, ok := c.original.(*csi03.CreateVolumeRequest); ok {
+			stripped = StripSecretsCSI03(c.original)
+		} else {
+			stripped = StripSecrets(c.original)
+		}
 		if assert.Equal(t, c.stripped, fmt.Sprintf("%s", stripped), "unexpected result for fmt s of %s", c.original) {
 			assert.Equal(t, c.stripped, fmt.Sprintf("%v", stripped), "unexpected result for fmt v of %s", c.original)
 		}