Skip to content

Commit 3c4e4ed

Browse files
scotwellsscotwells
authored
fix(notes): resolve subject API version dynamically instead of hardcoding (#508)
## Summary - Fixes error when creating notes on Domain resources: "no matches for kind Domain in version networking.datumapis.com/v1alpha1" - The webhook was assuming all resources use `v1alpha1`, but Domain uses `v1alpha` - Now the webhook automatically discovers the correct API version ## Test plan - [x] Unit tests passing - [ ] Create a note on a Domain resource and verify it succeeds 🤖 Generated with [Claude Code](https://claude.com/claude-code)
2 parents 4f6296b + 4dd68fa commit 3c4e4ed

5 files changed

Lines changed: 70 additions & 24 deletions

File tree

‎cmd/milo/controller-manager/controllermanager.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,11 @@ func Run(ctx context.Context, c *config.CompletedConfig, opts *Options) error {
466466
Metrics: metricsserver.Options{
467467
BindAddress: "0",
468468
},
469+
// Use the same REST mapper as the controller context to share the
470+
// cached API discovery across the webhook manager and controllers.
471+
MapperProvider: func(c *restclient.Config, httpClient *http.Client) (meta.RESTMapper, error) {
472+
return controllerContext.RESTMapper, nil
473+
},
469474
WebhookServer: webhook.NewServer(webhook.Options{
470475
Port: opts.ControllerRuntimeWebhookPort,
471476
CertDir: opts.SecureServing.ServerCert.CertDirectory,

‎internal/webhooks/notes/v1alpha1/clusternote_webhook.go‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1"
99
notesv1alpha1 "go.miloapis.com/milo/pkg/apis/notes/v1alpha1"
1010
"k8s.io/apimachinery/pkg/api/errors"
11+
"k8s.io/apimachinery/pkg/api/meta"
1112
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1213
"k8s.io/apimachinery/pkg/runtime"
1314
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -27,8 +28,9 @@ func SetupClusterNoteWebhooksWithManager(mgr ctrl.Manager) error {
2728
return ctrl.NewWebhookManagedBy(mgr).
2829
For(&notesv1alpha1.ClusterNote{}).
2930
WithDefaulter(&ClusterNoteMutator{
30-
Client: mgr.GetClient(),
31-
Scheme: mgr.GetScheme(),
31+
Client: mgr.GetClient(),
32+
Scheme: mgr.GetScheme(),
33+
RESTMapper: mgr.GetRESTMapper(),
3234
}).
3335
WithValidator(&ClusterNoteValidator{
3436
Client: mgr.GetClient(),
@@ -39,8 +41,9 @@ func SetupClusterNoteWebhooksWithManager(mgr ctrl.Manager) error {
3941
// +kubebuilder:webhook:path=/mutate-notes-miloapis-com-v1alpha1-clusternote,mutating=true,failurePolicy=fail,sideEffects=None,groups=notes.miloapis.com,resources=clusternotes,verbs=create,versions=v1alpha1,name=mclusternote.notes.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system
4042

4143
type ClusterNoteMutator struct {
42-
Client client.Client
43-
Scheme *runtime.Scheme
44+
Client client.Client
45+
Scheme *runtime.Scheme
46+
RESTMapper meta.RESTMapper
4447
}
4548

4649
var _ admission.CustomDefaulter = &ClusterNoteMutator{}
@@ -83,15 +86,19 @@ func (m *ClusterNoteMutator) setSubjectOwnerReference(ctx context.Context, clust
8386
return nil // Subject is namespaced, can't set owner reference on cluster-scoped resource
8487
}
8588

86-
// Get the subject resource
87-
gvk := schema.GroupVersionKind{
88-
Group: clusterNote.Spec.SubjectRef.APIGroup,
89-
Version: "v1alpha1", // Assuming v1alpha1, this could be made more flexible
90-
Kind: clusterNote.Spec.SubjectRef.Kind,
89+
// Resolve the GVK using REST mapper to discover the correct API version
90+
groupKind := schema.GroupKind{
91+
Group: clusterNote.Spec.SubjectRef.APIGroup,
92+
Kind: clusterNote.Spec.SubjectRef.Kind,
93+
}
94+
95+
mapping, err := m.RESTMapper.RESTMapping(groupKind)
96+
if err != nil {
97+
return fmt.Errorf("failed to get REST mapping for %s: %w", groupKind, err)
9198
}
9299

93100
subject := &unstructured.Unstructured{}
94-
subject.SetGroupVersionKind(gvk)
101+
subject.SetGroupVersionKind(mapping.GroupVersionKind)
95102

96103
if err := m.Client.Get(ctx, types.NamespacedName{
97104
Name: clusterNote.Spec.SubjectRef.Name,

‎internal/webhooks/notes/v1alpha1/clusternote_webhook_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import (
1616
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
1717
)
1818

19+
// Note: newTestRESTMapper is defined in note_webhook_test.go and shared across tests in this package
20+
1921
func TestClusterNoteMutator_Default(t *testing.T) {
2022
tests := map[string]struct {
2123
clusterNote *notesv1alpha1.ClusterNote
@@ -124,8 +126,9 @@ func TestClusterNoteMutator_Default(t *testing.T) {
124126

125127
fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(objects...).Build()
126128
mutator := &ClusterNoteMutator{
127-
Client: fakeClient,
128-
Scheme: testScheme,
129+
Client: fakeClient,
130+
Scheme: testScheme,
131+
RESTMapper: newTestRESTMapper(),
129132
}
130133

131134
// Create admission request context

‎internal/webhooks/notes/v1alpha1/note_webhook.go‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1"
99
notesv1alpha1 "go.miloapis.com/milo/pkg/apis/notes/v1alpha1"
1010
"k8s.io/apimachinery/pkg/api/errors"
11+
"k8s.io/apimachinery/pkg/api/meta"
1112
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1213
"k8s.io/apimachinery/pkg/runtime"
1314
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -27,8 +28,9 @@ func SetupNoteWebhooksWithManager(mgr ctrl.Manager) error {
2728
return ctrl.NewWebhookManagedBy(mgr).
2829
For(&notesv1alpha1.Note{}).
2930
WithDefaulter(&NoteMutator{
30-
Client: mgr.GetClient(),
31-
Scheme: mgr.GetScheme(),
31+
Client: mgr.GetClient(),
32+
Scheme: mgr.GetScheme(),
33+
RESTMapper: mgr.GetRESTMapper(),
3234
}).
3335
WithValidator(&NoteValidator{
3436
Client: mgr.GetClient(),
@@ -39,8 +41,9 @@ func SetupNoteWebhooksWithManager(mgr ctrl.Manager) error {
3941
// +kubebuilder:webhook:path=/mutate-notes-miloapis-com-v1alpha1-note,mutating=true,failurePolicy=fail,sideEffects=None,groups=notes.miloapis.com,resources=notes,verbs=create,versions=v1alpha1,name=mnote.notes.miloapis.com,admissionReviewVersions={v1,v1beta1},serviceName=milo-controller-manager,servicePort=9443,serviceNamespace=milo-system
4042

4143
type NoteMutator struct {
42-
Client client.Client
43-
Scheme *runtime.Scheme
44+
Client client.Client
45+
Scheme *runtime.Scheme
46+
RESTMapper meta.RESTMapper
4447
}
4548

4649
var _ admission.CustomDefaulter = &NoteMutator{}
@@ -83,15 +86,19 @@ func (m *NoteMutator) setSubjectOwnerReference(ctx context.Context, note *notesv
8386
return nil
8487
}
8588

86-
// Get the subject resource
87-
gvk := schema.GroupVersionKind{
88-
Group: note.Spec.SubjectRef.APIGroup,
89-
Version: "v1alpha1", // Assuming v1alpha1, this could be made more flexible
90-
Kind: note.Spec.SubjectRef.Kind,
89+
// Resolve the GVK using REST mapper to discover the correct API version
90+
groupKind := schema.GroupKind{
91+
Group: note.Spec.SubjectRef.APIGroup,
92+
Kind: note.Spec.SubjectRef.Kind,
93+
}
94+
95+
mapping, err := m.RESTMapper.RESTMapping(groupKind)
96+
if err != nil {
97+
return fmt.Errorf("failed to get REST mapping for %s: %w", groupKind, err)
9198
}
9299

93100
subject := &unstructured.Unstructured{}
94-
subject.SetGroupVersionKind(gvk)
101+
subject.SetGroupVersionKind(mapping.GroupVersionKind)
95102

96103
if err := m.Client.Get(ctx, types.NamespacedName{
97104
Name: note.Spec.SubjectRef.Name,

‎internal/webhooks/notes/v1alpha1/note_webhook_test.go‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ import (
88
"github.com/stretchr/testify/require"
99
iamv1alpha1 "go.miloapis.com/milo/pkg/apis/iam/v1alpha1"
1010
notesv1alpha1 "go.miloapis.com/milo/pkg/apis/notes/v1alpha1"
11+
"k8s.io/apimachinery/pkg/api/meta"
1112
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1213
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1314
"k8s.io/apimachinery/pkg/runtime"
15+
"k8s.io/apimachinery/pkg/runtime/schema"
1416
"k8s.io/apimachinery/pkg/types"
1517
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1618
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -25,6 +27,27 @@ func init() {
2527
utilruntime.Must(notesv1alpha1.AddToScheme(testScheme))
2628
}
2729

30+
// newTestRESTMapper creates a RESTMapper for testing that knows about common test resources
31+
func newTestRESTMapper() meta.RESTMapper {
32+
mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{
33+
{Group: "networking.datumapis.com", Version: "v1alpha1"},
34+
{Group: "resourcemanager.miloapis.com", Version: "v1alpha1"},
35+
})
36+
// Register Domain as a namespaced resource
37+
mapper.Add(schema.GroupVersionKind{
38+
Group: "networking.datumapis.com",
39+
Version: "v1alpha1",
40+
Kind: "Domain",
41+
}, meta.RESTScopeNamespace)
42+
// Register Organization as a cluster-scoped resource
43+
mapper.Add(schema.GroupVersionKind{
44+
Group: "resourcemanager.miloapis.com",
45+
Version: "v1alpha1",
46+
Kind: "Organization",
47+
}, meta.RESTScopeRoot)
48+
return mapper
49+
}
50+
2851
func TestNoteMutator_Default(t *testing.T) {
2952
tests := map[string]struct {
3053
note *notesv1alpha1.Note
@@ -137,8 +160,9 @@ func TestNoteMutator_Default(t *testing.T) {
137160

138161
fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(objects...).Build()
139162
mutator := &NoteMutator{
140-
Client: fakeClient,
141-
Scheme: testScheme,
163+
Client: fakeClient,
164+
Scheme: testScheme,
165+
RESTMapper: newTestRESTMapper(),
142166
}
143167

144168
// Create admission request context

0 commit comments

Comments
 (0)