Skip to content

Commit cd2cd0a

Browse files
authored
fix(mysql): fall back to scheme for empty GVK in getTableName (#1118)
## What Convert `getTableName` to a method on `*mysqlMetadataStorage` so it can fall back to `scheme.ObjectKinds()` when the object's TypeMeta has no Kind populated. Adds 4 unit tests. ## Why Controller-runtime returns objects with empty `TypeMeta` (see [kubernetes-sigs/controller-runtime#1517](kubernetes-sigs/controller-runtime#1517)). Without this fallback, every TR/PR/Pipeline reconcile that hits the metadata-storage path errors with `unable to determine table name for object type`. This blocks any environment with `enableMetadataStorage: true` (e.g. the OSS sandbox) and is a prerequisite for the cascade-delete PR stack (#1104#1114). The same scheme-fallback pattern already exists in `groupVersionForObject` further down in the same file — this fix mirrors it. ## How to test \`\`\`bash ./tools/bazel test //go/storage/mysql:go_default_test \`\`\` 4 unit tests cover: GVK populated, empty GVK with scheme fallback, nil scheme, unknown-to-scheme. End-to-end verified against a sandbox cluster running the full cascade-delete stack: prior to this fix the TR controller error-loops with the table-name error; with it, reconciles complete normally. Full cascade-delete e2e results are attached on #1114. ## Breaking changes None. `getTableName` is package-private; the receiver-conversion is a pure refactor.
1 parent 4c120e7 commit cd2cd0a

3 files changed

Lines changed: 74 additions & 5 deletions

File tree

go/storage/mysql/BUILD.bazel

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load("@io_bazel_rules_go//go:def.bzl", "go_library")
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "go_default_library",
@@ -15,3 +15,15 @@ go_library(
1515
"@io_k8s_apimachinery//pkg/runtime/schema:go_default_library",
1616
],
1717
)
18+
19+
go_test(
20+
name = "go_default_test",
21+
srcs = ["mysql_test.go"],
22+
embed = [":go_default_library"],
23+
deps = [
24+
"//proto/api/v2:go_default_library",
25+
"@com_github_stretchr_testify//require:go_default_library",
26+
"@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library",
27+
"@io_k8s_apimachinery//pkg/runtime:go_default_library",
28+
],
29+
)

go/storage/mysql/mysql.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func (m *mysqlMetadataStorage) Upsert(ctx context.Context, object runtime.Object
8787
return err
8888
}
8989

90-
tableName := getTableName(object)
90+
tableName := m.getTableName(object)
9191
if tableName == "" {
9292
return fmt.Errorf("unable to determine table name for object type")
9393
}
@@ -148,7 +148,7 @@ func (m *mysqlMetadataStorage) Upsert(ctx context.Context, object runtime.Object
148148

149149
// GetByName retrieves an object by its namespace and name
150150
func (m *mysqlMetadataStorage) GetByName(ctx context.Context, namespace string, name string, object runtime.Object) error {
151-
tableName := getTableName(object)
151+
tableName := m.getTableName(object)
152152
if tableName == "" {
153153
return fmt.Errorf("unable to determine table name for object type")
154154
}
@@ -183,7 +183,7 @@ func (m *mysqlMetadataStorage) GetByName(ctx context.Context, namespace string,
183183

184184
// GetByID retrieves an object by its UID
185185
func (m *mysqlMetadataStorage) GetByID(ctx context.Context, uid string, object runtime.Object) error {
186-
tableName := getTableName(object)
186+
tableName := m.getTableName(object)
187187
if tableName == "" {
188188
return fmt.Errorf("unable to determine table name for object type")
189189
}
@@ -438,8 +438,17 @@ func getObjectMeta(object runtime.Object) (metav1.Object, error) {
438438
return metaObj, nil
439439
}
440440

441-
func getTableName(object runtime.Object) string {
441+
// getTableName returns the lowercased Kind for the object's table. When the
442+
// object's TypeMeta is empty (a known controller-runtime quirk —
443+
// https://github.com/kubernetes-sigs/controller-runtime/issues/1517), it falls
444+
// back to scheme.ObjectKinds, mirroring the pattern in groupVersionForObject.
445+
func (m *mysqlMetadataStorage) getTableName(object runtime.Object) string {
442446
gvk := object.GetObjectKind().GroupVersionKind()
447+
if gvk.Kind == "" && m.scheme != nil {
448+
if gvks, _, err := m.scheme.ObjectKinds(object); err == nil && len(gvks) > 0 {
449+
gvk = gvks[0]
450+
}
451+
}
443452
return strings.ToLower(gvk.Kind)
444453
}
445454

go/storage/mysql/mysql_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package mysql
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
"k8s.io/apimachinery/pkg/runtime"
9+
10+
v2pb "github.com/michelangelo-ai/michelangelo/proto-go/api/v2"
11+
)
12+
13+
func newSchemeWithV2(t *testing.T) *runtime.Scheme {
14+
t.Helper()
15+
s := runtime.NewScheme()
16+
require.NoError(t, v2pb.AddToScheme(s))
17+
return s
18+
}
19+
20+
func TestGetTableName_GVKPopulated(t *testing.T) {
21+
m := &mysqlMetadataStorage{scheme: newSchemeWithV2(t)}
22+
obj := &v2pb.TriggerRun{
23+
TypeMeta: metav1.TypeMeta{Kind: "TriggerRun", APIVersion: "michelangelo.api/v2"},
24+
}
25+
require.Equal(t, "triggerrun", m.getTableName(obj))
26+
}
27+
28+
func TestGetTableName_GVKEmpty_SchemeFallback(t *testing.T) {
29+
// controller-runtime returns objects with empty TypeMeta (issue #1517);
30+
// the scheme fallback must resolve the Kind from the registered type.
31+
m := &mysqlMetadataStorage{scheme: newSchemeWithV2(t)}
32+
obj := &v2pb.TriggerRun{}
33+
require.Equal(t, "triggerrun", m.getTableName(obj))
34+
}
35+
36+
func TestGetTableName_GVKEmpty_NilScheme(t *testing.T) {
37+
// No scheme configured + empty GVK = "" (caller decides). Must not panic.
38+
m := &mysqlMetadataStorage{scheme: nil}
39+
obj := &v2pb.TriggerRun{}
40+
require.Equal(t, "", m.getTableName(obj))
41+
}
42+
43+
func TestGetTableName_GVKEmpty_UnknownToScheme(t *testing.T) {
44+
// Scheme exists but doesn't know this type → falls through to "".
45+
m := &mysqlMetadataStorage{scheme: runtime.NewScheme()}
46+
obj := &v2pb.TriggerRun{}
47+
require.Equal(t, "", m.getTableName(obj))
48+
}

0 commit comments

Comments
 (0)