From e827530aa4bbdac3d2b629a385b64c1000dbdec2 Mon Sep 17 00:00:00 2001
From: Lorenz Bauer <lmb@isovalent.com>
Date: Mon, 15 Jul 2024 11:51:44 +0100
Subject: [PATCH] info: make it harder to add backwards incompatible API

People adding new fields to the Info API always get tripped up by
how it works. Add a comment which explains why it works the way it does.
Also add a test which ensures that new exported fields are only
added when they are really warranted.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
---
 info.go      | 17 +++++++++++++++++
 info_test.go | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/info.go b/info.go
index 25adbb94e..04c60c64b 100644
--- a/info.go
+++ b/info.go
@@ -20,6 +20,23 @@ import (
 	"github.com/cilium/ebpf/internal/unix"
 )
 
+// The *Info structs expose metadata about a program or map. Most
+// fields are exposed via a getter:
+//
+//     func (*MapInfo) ID() (MapID, bool)
+//
+// This is because the metadata available changes based on kernel version.
+// The second boolean return value indicates whether a particular field is
+// available on the current kernel.
+//
+// Always add new metadata as such a getter, unless you can somehow get the
+// value of the field on all supported kernels. Also document which version
+// a particular field first appeared in.
+//
+// Some metadata is a buffer which needs additional parsing. In this case,
+// store the undecoded data in the Info struct and provide a getter which
+// decodes it when necessary. See ProgramInfo.Instructions for an example.
+
 // MapInfo describes a map.
 type MapInfo struct {
 	Type       MapType
diff --git a/info_test.go b/info_test.go
index b7653ff44..90725f550 100644
--- a/info_test.go
+++ b/info_test.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -436,3 +437,35 @@ func TestProgInfoExtBTF(t *testing.T) {
 		}
 	}
 }
+
+func TestInfoExportedFields(t *testing.T) {
+	// It is highly unlikely that you should be adjusting the asserts below.
+	// See the comment at the top of info.go for more information.
+
+	var names []string
+	for _, field := range reflect.VisibleFields(reflect.TypeOf(MapInfo{})) {
+		if field.IsExported() {
+			names = append(names, field.Name)
+		}
+	}
+	qt.Assert(t, qt.ContentEquals(names, []string{
+		"Type",
+		"KeySize",
+		"ValueSize",
+		"MaxEntries",
+		"Flags",
+		"Name",
+	}))
+
+	names = nil
+	for _, field := range reflect.VisibleFields(reflect.TypeOf(ProgramInfo{})) {
+		if field.IsExported() {
+			names = append(names, field.Name)
+		}
+	}
+	qt.Assert(t, qt.ContentEquals(names, []string{
+		"Type",
+		"Tag",
+		"Name",
+	}))
+}