Opt 12 v1beta1 CRDs into storage-version migration + CI guard

cmd/thv-operator/api/v1alpha1/types.go

@@ -281,6 +281,7 @@ type MCPTelemetryConfigList struct {
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:resource:shortName=mwc,categories=toolhive
 //+kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`
 //+kubebuilder:printcolumn:name="References",type=string,JSONPath=`.status.referencingWorkloads`

cmd/thv-operator/api/v1beta1/embeddingserver_types.go

@@ -211,6 +211,7 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:resource:shortName=emb;embedding,categories=toolhive
 //+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.phase"
 //+kubebuilder:printcolumn:name="Model",type="string",JSONPath=".spec.model"

cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go

@@ -1117,6 +1117,7 @@ type MCPExternalAuthConfigStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:storageversion
 // +kubebuilder:subresource:status
+// +kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 // +kubebuilder:resource:shortName=extauth;mcpextauth,categories=toolhive
 // +kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type`
 // +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`

cmd/thv-operator/api/v1beta1/mcpgroup_types.go

@@ -88,6 +88,7 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:resource:shortName=mcpg;mcpgroup,categories=toolhive
 //+kubebuilder:printcolumn:name="Servers",type="integer",JSONPath=".status.serverCount"
 //+kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase"

cmd/thv-operator/api/v1beta1/mcpoidcconfig_types.go

@@ -200,6 +200,7 @@ type MCPOIDCConfigStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:storageversion
 // +kubebuilder:subresource:status
+// +kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 // +kubebuilder:resource:shortName=mcpoidc,categories=toolhive
 // +kubebuilder:printcolumn:name="Source",type=string,JSONPath=`.spec.type`
 // +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`

cmd/thv-operator/api/v1beta1/mcpregistry_types.go

@@ -205,6 +205,7 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.phase"
 //+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
 //+kubebuilder:printcolumn:name="Replicas",type="integer",JSONPath=".status.readyReplicas"

cmd/thv-operator/api/v1beta1/mcpremoteproxy_types.go

@@ -356,6 +356,7 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:resource:shortName=rp;mcprp,categories=toolhive
 //+kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase"
 //+kubebuilder:printcolumn:name="Remote URL",type="string",JSONPath=".spec.remoteUrl"

cmd/thv-operator/api/v1beta1/mcpserver_types.go

@@ -929,6 +929,7 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:resource:shortName=mcpserver;mcpservers,categories=toolhive
 //+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.phase"
 //+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"

cmd/thv-operator/api/v1beta1/mcpserverentry_types.go

@@ -164,6 +164,7 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:resource:shortName=mcpentry,categories=toolhive
 //+kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase"
 //+kubebuilder:printcolumn:name="Transport",type="string",JSONPath=".spec.transport"

cmd/thv-operator/api/v1beta1/mcptelemetryconfig_types.go

@@ -139,6 +139,7 @@ type MCPTelemetryConfigStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:storageversion
 // +kubebuilder:subresource:status
+// +kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 // +kubebuilder:resource:shortName=mcpotel,categories=toolhive
 // +kubebuilder:printcolumn:name="Endpoint",type=string,JSONPath=`.spec.openTelemetry.endpoint`
 // +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`

cmd/thv-operator/api/v1beta1/toolconfig_types.go

@@ -112,6 +112,7 @@ type MCPToolConfigStatus struct {
 // +kubebuilder:object:root=true
 // +kubebuilder:storageversion
 // +kubebuilder:subresource:status
+// +kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 // +kubebuilder:resource:shortName=tc;toolconfig,categories=toolhive
 // +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`
 // +kubebuilder:printcolumn:name="References",type=integer,JSONPath=`.status.referenceCount`

cmd/thv-operator/api/v1beta1/virtualmcpcompositetooldefinition_types.go

@@ -100,6 +100,7 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:resource:shortName=vmcpctd;compositetool,categories=toolhive
 //+kubebuilder:printcolumn:name="Workflow",type="string",JSONPath=".spec.name",description="Workflow name"
 //+kubebuilder:printcolumn:name="Steps",type="integer",JSONPath=".spec.steps[*]",description="Number of steps"

cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go

@@ -467,6 +467,7 @@ const (
 //+kubebuilder:object:root=true
 //+kubebuilder:storageversion
 //+kubebuilder:subresource:status
+//+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
 //+kubebuilder:resource:shortName=vmcp;virtualmcp,categories=toolhive
 //+kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase",description="The phase of the VirtualMCPServer"
 //+kubebuilder:printcolumn:name="URL",type="string",JSONPath=".status.url",description="Virtual MCP server URL"

cmd/thv-operator/controllers/marker_coverage_test.go

@@ -0,0 +1,260 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package controllers
+
+import (
+	"bufio"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestStorageVersionRootMarkerCoverage guards against a subtle failure mode in
+// the opt-in-label design for storage-version migration: if a new CRD root
+// type is added under cmd/thv-operator/api/ without the
+//
+//	+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true
+//
+// marker, the StorageVersionMigrator controller silently excludes it from
+// reconciliation. The problem surfaces only when a future release tries to
+// drop a deprecated version — at which point it is far too late.
+//
+// The test walks every cmd/thv-operator/api/v*/ directory and scans the
+// *_types.go (and legacy types.go) files inside. For each type whose marker
+// block contains both +kubebuilder:object:root=true AND
+// +kubebuilder:storageversion (i.e., the current storage version's root
+// types), it requires the migrate marker. No escape hatch — every
+// storage-version root must opt in.
+//
+// Why no escape hatch? An "exclude" marker would let a single-version CRD
+// declare "don't auto-migrate me", but at graduation time the developer
+// must remember to swap exclude→migrate. Forgetting the swap reintroduces
+// the silent-skip failure mode the test is meant to prevent. If a CRD ever
+// legitimately needs to be excluded, handle it as a one-off PR conversation
+// (e.g. a hardcoded allowlist here) rather than a self-serve marker.
+//
+// Notes on the scope:
+//   - List types (+kubebuilder:object:root=true WITHOUT +kubebuilder:storageversion)
+//     are intentionally skipped — CRD-level labels are keyed on the root type,
+//     not the list type.
+//   - Served-but-not-storage versions (e.g. v1alpha1 during a v1alpha1→v1beta1
+//     graduation, or v1beta1 during a future v1beta1→v1beta2 graduation) are
+//     naturally filtered out by the storageversion-marker requirement.
+//   - When a future graduation moves +kubebuilder:storageversion from v1beta1
+//     to v1beta2, the test will automatically start scanning v1beta2's root
+//     types without code changes here.
+func TestStorageVersionRootMarkerCoverage(t *testing.T) {
+	t.Parallel()
+
+	apiDir := filepath.Join("..", "api")
+	versionDirs, err := os.ReadDir(apiDir)
+	require.NoError(t, err, "reading %s", apiDir)
+
+	const (
+		rootMarker         = "+kubebuilder:object:root=true"
+		storageMarker      = "+kubebuilder:storageversion"
+		migrateMarker      = "+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true"
+		groupversionInfoGo = "groupversion_info.go"
+		zzGeneratedPrefix  = "zz_generated"
+		// Match both the modern one-type-per-file convention (e.g.
+		// mcpserver_types.go) and the legacy multi-type single-file
+		// convention (api/v1alpha1/types.go) so no root type slips through.
+		exactTypesGo  = "types.go"
+		suffixTypesGo = "_types.go"
+	)
+
+	type rootType struct {
+		version     string // e.g. "v1beta1"
+		file        string
+		typeName    string
+		markerBlock []string
+	}
+
+	var roots []rootType
+
+	for _, vd := range versionDirs {
+		// Only descend into directories that look like an API version
+		// (v1alpha1, v1beta1, v1beta2, v1, ...). Anything else under api/ is
+		// not our concern.
+		if !vd.IsDir() || !strings.HasPrefix(vd.Name(), "v") {
+			continue
+		}
+		version := vd.Name()
+		typesDir := filepath.Join(apiDir, version)
+
+		entries, err := os.ReadDir(typesDir)
+		require.NoError(t, err, "reading %s", typesDir)
+
+		for _, e := range entries {
+			if e.IsDir() {
+				continue
+			}
+			name := e.Name()
+			if name != exactTypesGo && !strings.HasSuffix(name, suffixTypesGo) {
+				continue
+			}
+			if name == groupversionInfoGo || strings.HasPrefix(name, zzGeneratedPrefix) {
+				continue
+			}
+
+			path := filepath.Join(typesDir, name)
+			f, err := os.Open(path)
+			require.NoError(t, err, "open %s", path)
+
+			scanner := bufio.NewScanner(f)
+			// Raise the max token size — some of the *_types.go files have very
+			// long comment lines (printcolumn JSONPath expressions).
+			scanner.Buffer(make([]byte, 64*1024), 1024*1024)
+
+			var block []string
+			for scanner.Scan() {
+				line := strings.TrimSpace(scanner.Text())
+
+				switch {
+				case strings.HasPrefix(line, "//"):
+					// Accumulate every comment/marker line. kubebuilder convention
+					// often separates markers from the godoc comment with a blank
+					// line, so we keep the block alive across blanks and only
+					// reset on a non-comment, non-blank, non-type line.
+					block = append(block, strings.TrimSpace(strings.TrimPrefix(line, "//")))
+				case strings.HasPrefix(line, "type "):
+					// Only root-level types matter (must contain object:root=true
+					// AND storageversion markers). List types have only object:root=true.
+					if containsMarker(block, rootMarker) && containsMarker(block, storageMarker) {
+						typeName := extractTypeName(line)
+						if typeName != "" {
+							copied := append([]string(nil), block...)
+							roots = append(roots, rootType{
+								version:     version,
+								file:        name,
+								typeName:    typeName,
+								markerBlock: copied,
+							})
+						}
+					}
+					block = nil
+				case line == "":
+					// Blank line — keep block alive (comment-then-blank-then-type
+					// is idiomatic Go + kubebuilder).
+				default:
+					// Anything else (e.g. struct body, package clause, import) —
+					// drop any in-flight comment block.
+					block = nil
+				}
+			}
+			require.NoError(t, scanner.Err(), "scan %s", path)
+			require.NoError(t, f.Close())
+		}
+	}
+
+	require.NotEmpty(t, roots,
+		"no storage-version root types found under cmd/thv-operator/api/ — "+
+			"scanner likely broken or no version directory carries the "+
+			"+kubebuilder:storageversion marker; this test is meaningless without coverage")
+
+	for _, r := range roots {
+		key := r.version + "/" + r.file + "." + r.typeName
+		if _, allowed := excludedRootTypes[key]; allowed {
+			continue
+		}
+		assert.Truef(t, containsMarker(r.markerBlock, migrateMarker),
+			"root type %s is missing the storage-version migrate marker:\n"+
+				"  %s\n"+
+				"Every root type carrying +kubebuilder:storageversion must declare it.\n"+
+				"See cmd/thv-operator/controllers/storageversionmigrator_controller.go\n"+
+				"for context. There is no opt-out marker — if a CRD legitimately\n"+
+				"should not be auto-migrated, raise it as a PR review conversation\n"+
+				"and add an entry to excludedRootTypes in this file.",
+			key, migrateMarker)
+	}
+}
+
+// excludedRootTypes is the (intentionally empty) allowlist of storage-version
+// root types that are deliberately not auto-migrated. Entries are keyed by
+// "<version>/<file>.<TypeName>" (e.g. "v1alpha1/types.go.MCPSomething").
+//
+// Adding an entry here is a deliberate, reviewed decision — not a self-serve
+// escape hatch. If a future CRD legitimately cannot or should not participate
+// in storage-version migration:
+//
+//  1. Open a PR with the entry added here.
+//  2. Make the case in the PR description: why is this CRD different?
+//     What happens at version-drop time if no migration ever runs?
+//  3. The reviewer should push back hard. Excluding a CRD means accepting
+//     that it can never have a version dropped without manual intervention.
+var excludedRootTypes = map[string]struct{}{
+	// Example (do not enable):
+	// "v1alpha1/types.go.MCPSomething": {},
+}
+
+// containsMarker returns true if any line in block carries the given
+// kubebuilder marker as a complete token — the marker must be followed by
+// end-of-line or whitespace.
+//
+// Strict matching matters: the migrate marker ends in "=true". A typo such
+// as "=truee" would still be a substring match under strings.Contains, the
+// test would pass, the generated CRD YAML would have label value "truee",
+// and the controller's runtime check (`labels[key] == "true"`) would
+// silently exclude the CRD. The bug would only surface at version-drop time
+// months later. Whole-token matching catches the typo at PR time.
+func containsMarker(block []string, marker string) bool {
+	for _, l := range block {
+		idx := strings.Index(l, marker)
+		if idx < 0 {
+			continue
+		}
+		end := idx + len(marker)
+		if end == len(l) || l[end] == ' ' || l[end] == '\t' {
+			return true
+		}
+		// Marker appeared only as a prefix of a longer token — reject.
+	}
+	return false
+}
+
+// extractTypeName returns the identifier in a line of the form `type Foo struct {`.
+// Returns empty string if the line is not a type declaration we care about.
+func extractTypeName(line string) string {
+	fields := strings.Fields(line)
+	if len(fields) < 2 || fields[0] != "type" {
+		return ""
+	}
+	return fields[1]
+}
+
+// TestContainsMarkerStrictBoundary defends the whole-token match contract of
+// containsMarker. If someone "improves" it back to strings.Contains semantics,
+// the value-typo failure mode reopens silently. Keep this test as the
+// regression guard.
+func TestContainsMarkerStrictBoundary(t *testing.T) {
+	t.Parallel()
+
+	const marker = "+kubebuilder:metadata:labels=toolhive.stacklok.dev/auto-migrate-storage-version=true"
+
+	tests := []struct {
+		name string
+		line string
+		want bool
+	}{
+		{name: "exact match at end of line", line: marker, want: true},
+		{name: "marker followed by space", line: marker + " // trailing comment", want: true},
+		{name: "marker followed by tab", line: marker + "\t", want: true},
+		{name: "value typo =truee is a prefix match — must reject", line: marker + "e", want: false},
+		{name: "value typo =trueX is a prefix match — must reject", line: marker + "X", want: false},
+		{name: "marker absent", line: "+kubebuilder:object:root=true", want: false},
+		{name: "marker followed by alphanumeric — must reject", line: marker + "0", want: false},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			got := containsMarker([]string{tc.line}, marker)
+			assert.Equal(t, tc.want, got)
+		})
+	}
+}

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_embeddingservers.yaml

@@ -4,6 +4,8 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     controller-gen.kubebuilder.io/version: v0.17.3
+  labels:
+    toolhive.stacklok.dev/auto-migrate-storage-version: "true"
   name: embeddingservers.toolhive.stacklok.dev
 spec:
   group: toolhive.stacklok.dev

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml

@@ -4,6 +4,8 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     controller-gen.kubebuilder.io/version: v0.17.3
+  labels:
+    toolhive.stacklok.dev/auto-migrate-storage-version: "true"
   name: mcpexternalauthconfigs.toolhive.stacklok.dev
 spec:
   group: toolhive.stacklok.dev

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpgroups.yaml

@@ -4,6 +4,8 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     controller-gen.kubebuilder.io/version: v0.17.3
+  labels:
+    toolhive.stacklok.dev/auto-migrate-storage-version: "true"
   name: mcpgroups.toolhive.stacklok.dev
 spec:
   group: toolhive.stacklok.dev