map: include full property diff in ErrMapIncompatible error string

Signed-off-by: Timo Beckers <timo@isovalent.com>
cilium · Nov 8, 2023 · efc5046 · efc5046
1 parent 4e460ee
commit efc5046
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 17 deletions.
diff --git a/map.go b/map.go
@@ -9,6 +9,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"time"
 	"unsafe"
 
@@ -190,26 +191,32 @@ func (ms *MapSpec) Compatible(m *Map) error {
 		return err
 	}
 
-	switch {
-	case m.typ != ms.Type:
-		return fmt.Errorf("expected type %v, got %v: %w", ms.Type, m.typ, ErrMapIncompatible)
-
-	case m.keySize != ms.KeySize:
-		return fmt.Errorf("expected key size %v, got %v: %w", ms.KeySize, m.keySize, ErrMapIncompatible)
-
-	case m.valueSize != ms.ValueSize:
-		return fmt.Errorf("expected value size %v, got %v: %w", ms.ValueSize, m.valueSize, ErrMapIncompatible)
+	diffs := []string{}
+	if m.typ != ms.Type {
+		diffs = append(diffs, fmt.Sprintf("Type: %s changed to %s", m.typ, ms.Type))
+	}
+	if m.keySize != ms.KeySize {
+		diffs = append(diffs, fmt.Sprintf("KeySize: %d changed to %d", m.keySize, ms.KeySize))
+	}
+	if m.valueSize != ms.ValueSize {
+		diffs = append(diffs, fmt.Sprintf("ValueSize: %d changed to %d", m.valueSize, ms.ValueSize))
+	}
+	if m.maxEntries != ms.MaxEntries {
+		diffs = append(diffs, fmt.Sprintf("MaxEntries: %d changed to %d", m.maxEntries, ms.MaxEntries))
+	}
 
-	case m.maxEntries != ms.MaxEntries:
-		return fmt.Errorf("expected max entries %v, got %v: %w", ms.MaxEntries, m.maxEntries, ErrMapIncompatible)
+	// BPF_F_RDONLY_PROG is set unconditionally for devmaps. Explicitly allow this
+	// mismatch.
+	if !((ms.Type == DevMap || ms.Type == DevMapHash) && m.flags^ms.Flags == unix.BPF_F_RDONLY_PROG) &&
+		m.flags != ms.Flags {
+		diffs = append(diffs, fmt.Sprintf("Flags: %d changed to %d", m.flags, ms.Flags))
+	}
 
-	// BPF_F_RDONLY_PROG is set unconditionally for devmaps. Explicitly allow
-	// this mismatch.
-	case !((ms.Type == DevMap || ms.Type == DevMapHash) && m.flags^ms.Flags == unix.BPF_F_RDONLY_PROG) &&
-		m.flags != ms.Flags:
-		return fmt.Errorf("expected flags %v, got %v: %w", ms.Flags, m.flags, ErrMapIncompatible)
+	if len(diffs) == 0 {
+		return nil
 	}
-	return nil
+
+	return fmt.Errorf("%s: %w", strings.Join(diffs, ", "), ErrMapIncompatible)
 }
 
 // Map represents a Map file descriptor.

diff --git a/map_test.go b/map_test.go
@@ -1729,6 +1729,7 @@ func TestMapPinning(t *testing.T) {
 	}
 
 	spec.KeySize = 8
+	spec.ValueSize = 8
 	m3, err := NewMapWithOptions(spec, MapOptions{PinPath: tmp})
 	if err == nil {
 		m3.Close()
@@ -1737,6 +1738,10 @@ func TestMapPinning(t *testing.T) {
 	if !errors.Is(err, ErrMapIncompatible) {
 		t.Fatalf("Opening a pinned map with a mismatching spec failed with the wrong error")
 	}
+
+	// Check if error string mentions both KeySize and ValueSize.
+	c.Assert(err.Error(), qt.Contains, "KeySize")
+	c.Assert(err.Error(), qt.Contains, "ValueSize")
 }
 
 func TestPerfEventArrayCompatible(t *testing.T) {