From 3af7430074cbcad85504a995fbcaad60fc74deac Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Sat, 10 Jan 2026 01:50:53 +0100 Subject: [PATCH] fix(registry): implement field selector filtering for label-based resources (#1845) ## What this PR does Fixes field selector filtering for registry resources (Applications, TenantModules, TenantSecrets) when using kubectl with field selectors like `--field-selector=metadata.namespace=tenant-kvaps` or `metadata.name=test`. Controller-runtime cache doesn't support field selectors natively, which caused incorrect filtering behavior. This PR implements manual filtering for `metadata.name` and `metadata.namespace` field selectors in List() and Watch() methods. Changes: - Created `pkg/registry/fields` package with `ParseFieldSelector` utility for common field selector parsing - Refactored field selector logic in application, tenantmodule, and tenantsecret registries to use the common implementation - Implemented manual post-processing filtering after label-based queries - Removed `Raw` field usage and field selectors from `client.ListOptions` ### Release note ```release-note [registry] Fix field selector filtering for kubectl queries with metadata.name and metadata.namespace selectors ``` --- pkg/registry/apps/application/rest.go | 132 +++++++++++++------------ pkg/registry/core/tenantmodule/rest.go | 117 +++++++++++----------- pkg/registry/core/tenantsecret/rest.go | 38 ++++--- pkg/registry/fields/filter.go | 70 +++++++++++++ 4 files changed, 227 insertions(+), 130 deletions(-) create mode 100644 pkg/registry/fields/filter.go diff --git a/pkg/registry/apps/application/rest.go b/pkg/registry/apps/application/rest.go index 9e8fb891..4f3204d5 100644 --- a/pkg/registry/apps/application/rest.go +++ b/pkg/registry/apps/application/rest.go @@ -28,7 +28,7 @@ import ( helmv2 "github.com/fluxcd/helm-controller/api/v2" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - fields "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/fields" labels "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -42,6 +42,7 @@ import ( appsv1alpha1 "github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1" "github.com/cozystack/cozystack/pkg/config" + fieldfilter "github.com/cozystack/cozystack/pkg/registry/fields" "github.com/cozystack/cozystack/pkg/registry/sorting" internalapiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -257,26 +258,32 @@ func (r *REST) List(ctx context.Context, options *metainternalversion.ListOption } // Initialize variables for selector mapping - var helmFieldSelector string - var helmLabelSelector string + var helmLabelSelector labels.Selector - // Process field.selector - if options.FieldSelector != nil { - fs, err := fields.ParseSelector(options.FieldSelector.String()) - if err != nil { - klog.Errorf("Invalid field selector: %v", err) - return nil, fmt.Errorf("invalid field selector: %v", err) - } - // Check if selector is for metadata.name - if name, exists := fs.RequiresExactMatch("metadata.name"); exists { - // Convert Application name to HelmRelease name - mappedName := r.releaseConfig.Prefix + name - // Create new field.selector for HelmRelease - helmFieldSelector = fields.OneTermEqualSelector("metadata.name", mappedName).String() - } else { - // If field.selector contains other fields, map them directly - helmFieldSelector = fs.String() - } + // Parse field selector for manual filtering + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + fieldFilter, err := fieldfilter.ParseFieldSelector(options.FieldSelector) + if err != nil { + klog.Errorf("Error parsing field selector: %v", err) + return nil, err + } + + // If field selector specifies namespace different from context, return empty list + if fieldFilter.Namespace != "" && namespace != "" && namespace != fieldFilter.Namespace { + klog.V(6).Infof("Field selector namespace %s doesn't match context namespace %s, returning empty list", fieldFilter.Namespace, namespace) + return &appsv1alpha1.ApplicationList{ + TypeMeta: metav1.TypeMeta{ + APIVersion: appsv1alpha1.SchemeGroupVersion.String(), + Kind: r.kindName + "List", + }, + }, nil + } + + // Convert Application name to HelmRelease name for manual filtering + var filterByName string + if fieldFilter.Name != "" { + filterByName = r.releaseConfig.Prefix + fieldFilter.Name } // Process label.selector @@ -315,21 +322,16 @@ func (r *REST) List(ctx context.Context, options *metainternalversion.ListOption labelRequirements = append(labelRequirements, prefixedReqs...) } } - helmLabelSelector = labels.NewSelector().Add(labelRequirements...).String() + helmLabelSelector = labels.NewSelector().Add(labelRequirements...) klog.V(6).Infof("Using label selector: %s for kind: %s, group: %s", helmLabelSelector, r.kindName, r.gvk.Group) - // Set ListOptions for HelmRelease with selector mapping - metaOptions := metav1.ListOptions{ - FieldSelector: helmFieldSelector, - LabelSelector: helmLabelSelector, - } - - // List HelmReleases with mapped selectors + // List HelmReleases with label selector only + // Field selectors are not supported by controller-runtime cache, so we filter manually below hrList := &helmv2.HelmReleaseList{} err = r.c.List(ctx, hrList, &client.ListOptions{ - Namespace: namespace, - Raw: &metaOptions, + Namespace: namespace, + LabelSelector: helmLabelSelector, }) if err != nil { klog.Errorf("Error listing HelmReleases: %v", err) @@ -346,6 +348,16 @@ func (r *REST) List(ctx context.Context, options *metainternalversion.ListOption for i := range hrList.Items { hr := &hrList.Items[i] + // Apply manual field selector filtering (metadata.name and metadata.namespace) + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + if filterByName != "" && hr.Name != filterByName { + continue + } + if !fieldFilter.MatchesNamespace(hr.Namespace) { + continue + } + app, err := r.ConvertHelmReleaseToApplication(hr) if err != nil { klog.Errorf("Error converting HelmRelease %s to Application: %v", hr.GetName(), err) @@ -569,27 +581,21 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio } // Initialize variables for selector mapping - var helmFieldSelector string - var helmLabelSelector string + var helmLabelSelector labels.Selector - // Process field.selector - if options.FieldSelector != nil { - fs, err := fields.ParseSelector(options.FieldSelector.String()) - if err != nil { - klog.Errorf("Invalid field selector: %v", err) - return nil, fmt.Errorf("invalid field selector: %v", err) - } + // Parse field selector for manual filtering + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + fieldFilter, err := fieldfilter.ParseFieldSelector(options.FieldSelector) + if err != nil { + klog.Errorf("Error parsing field selector: %v", err) + return nil, err + } - // Check if selector is for metadata.name - if name, exists := fs.RequiresExactMatch("metadata.name"); exists { - // Convert Application name to HelmRelease name - mappedName := r.releaseConfig.Prefix + name - // Create new field.selector for HelmRelease - helmFieldSelector = fields.OneTermEqualSelector("metadata.name", mappedName).String() - } else { - // If field.selector contains other fields, map them directly - helmFieldSelector = fs.String() - } + // Convert Application name to HelmRelease name for manual filtering + var filterByName string + if fieldFilter.Name != "" { + filterByName = r.releaseConfig.Prefix + fieldFilter.Name } // Process label.selector @@ -628,21 +634,15 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio labelRequirements = append(labelRequirements, prefixedReqs...) } } - helmLabelSelector = labels.NewSelector().Add(labelRequirements...).String() + helmLabelSelector = labels.NewSelector().Add(labelRequirements...) - // Set ListOptions for HelmRelease with selector mapping - metaOptions := metav1.ListOptions{ - Watch: true, - ResourceVersion: options.ResourceVersion, - FieldSelector: helmFieldSelector, - LabelSelector: helmLabelSelector, - } - - // Start watch on HelmRelease with mapped selectors + // Start watch on HelmRelease with label selector only + // Field selectors are not supported by controller-runtime cache + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 hrList := &helmv2.HelmReleaseList{} helmWatcher, err := r.w.Watch(ctx, hrList, &client.ListOptions{ - Namespace: namespace, - Raw: &metaOptions, + Namespace: namespace, + LabelSelector: helmLabelSelector, }) if err != nil { klog.Errorf("Error setting up watch for HelmReleases: %v", err) @@ -682,6 +682,16 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio continue } + // Apply manual field selector filtering (metadata.name and metadata.namespace) + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + if filterByName != "" && hr.Name != filterByName { + continue + } + if !fieldFilter.MatchesNamespace(hr.Namespace) { + continue + } + // Note: All HelmReleases already match the required labels due to server-side label selector filtering // Convert HelmRelease to Application app, err := r.ConvertHelmReleaseToApplication(hr) diff --git a/pkg/registry/core/tenantmodule/rest.go b/pkg/registry/core/tenantmodule/rest.go index 12da0e0d..b2cda2db 100644 --- a/pkg/registry/core/tenantmodule/rest.go +++ b/pkg/registry/core/tenantmodule/rest.go @@ -40,6 +40,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" corev1alpha1 "github.com/cozystack/cozystack/pkg/apis/core/v1alpha1" + fieldfilter "github.com/cozystack/cozystack/pkg/registry/fields" "github.com/cozystack/cozystack/pkg/registry/sorting" apierrors "k8s.io/apimachinery/pkg/api/errors" ) @@ -161,24 +162,26 @@ func (r *REST) List(ctx context.Context, options *metainternalversion.ListOption } // Initialize variables for selector mapping - var helmFieldSelector string - var helmLabelSelector string + var helmLabelSelector labels.Selector - // Process field.selector - if options.FieldSelector != nil { - fs, err := fields.ParseSelector(options.FieldSelector.String()) - if err != nil { - klog.Errorf("Invalid field selector: %v", err) - return nil, fmt.Errorf("invalid field selector: %v", err) - } - // Check if selector is for metadata.name - if name, exists := fs.RequiresExactMatch("metadata.name"); exists { - // Create new field.selector for HelmRelease - helmFieldSelector = fields.OneTermEqualSelector("metadata.name", name).String() - } else { - // If field.selector contains other fields, map them directly - helmFieldSelector = fs.String() - } + // Parse field selector for manual filtering + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + fieldFilter, err := fieldfilter.ParseFieldSelector(options.FieldSelector) + if err != nil { + klog.Errorf("Error parsing field selector: %v", err) + return nil, err + } + + // If field selector specifies namespace different from context, return empty list + if fieldFilter.Namespace != "" && namespace != "" && namespace != fieldFilter.Namespace { + klog.V(6).Infof("Field selector namespace %s doesn't match context namespace %s, returning empty list", fieldFilter.Namespace, namespace) + return &corev1alpha1.TenantModuleList{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1alpha1.SchemeGroupVersion.String(), + Kind: "TenantModuleList", + }, + }, nil } // Process label.selector - add the tenant module label requirement @@ -202,19 +205,15 @@ func (r *REST) List(ctx context.Context, options *metainternalversion.ListOption } } - helmLabelSelector = labels.NewSelector().Add(labelRequirements...).String() + helmLabelSelector = labels.NewSelector().Add(labelRequirements...) - // Set ListOptions for HelmRelease with selector mapping - metaOptions := metav1.ListOptions{ - FieldSelector: helmFieldSelector, - LabelSelector: helmLabelSelector, - } - - // List HelmReleases with mapped selectors + // List HelmReleases with label selector only + // Field selectors are not supported by controller-runtime cache + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 hrList := &helmv2.HelmReleaseList{} err = r.c.List(ctx, hrList, &client.ListOptions{ - Namespace: namespace, - Raw: &metaOptions, + Namespace: namespace, + LabelSelector: helmLabelSelector, }) if err != nil { klog.Errorf("Error listing HelmReleases: %v", err) @@ -226,6 +225,16 @@ func (r *REST) List(ctx context.Context, options *metainternalversion.ListOption // Iterate over HelmReleases and convert to TenantModules for i := range hrList.Items { + // Apply manual field selector filtering (metadata.name and metadata.namespace) + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + if !fieldFilter.MatchesName(hrList.Items[i].Name) { + continue + } + if !fieldFilter.MatchesNamespace(hrList.Items[i].Namespace) { + continue + } + // Double-check the label requirement if !r.hasTenantModuleLabel(&hrList.Items[i]) { continue @@ -305,25 +314,15 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio } // Initialize variables for selector mapping - var helmFieldSelector string - var helmLabelSelector string + var helmLabelSelector labels.Selector - // Process field.selector - if options.FieldSelector != nil { - fs, err := fields.ParseSelector(options.FieldSelector.String()) - if err != nil { - klog.Errorf("Invalid field selector: %v", err) - return nil, fmt.Errorf("invalid field selector: %v", err) - } - - // Check if selector is for metadata.name - if name, exists := fs.RequiresExactMatch("metadata.name"); exists { - // Create new field.selector for HelmRelease - helmFieldSelector = fields.OneTermEqualSelector("metadata.name", name).String() - } else { - // If field.selector contains other fields, map them directly - helmFieldSelector = fs.String() - } + // Parse field selector for manual filtering + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + fieldFilter, err := fieldfilter.ParseFieldSelector(options.FieldSelector) + if err != nil { + klog.Errorf("Error parsing field selector: %v", err) + return nil, err } // Process label.selector - add the tenant module label requirement @@ -347,21 +346,15 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio } } - helmLabelSelector = labels.NewSelector().Add(labelRequirements...).String() + helmLabelSelector = labels.NewSelector().Add(labelRequirements...) - // Set ListOptions for HelmRelease with selector mapping - metaOptions := metav1.ListOptions{ - Watch: true, - ResourceVersion: options.ResourceVersion, - FieldSelector: helmFieldSelector, - LabelSelector: helmLabelSelector, - } - - // Start watch on HelmRelease with mapped selectors + // Start watch on HelmRelease with label selector only + // Field selectors are not supported by controller-runtime cache + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 hrList := &helmv2.HelmReleaseList{} helmWatcher, err := r.w.Watch(ctx, hrList, &client.ListOptions{ - Namespace: namespace, - Raw: &metaOptions, + Namespace: namespace, + LabelSelector: helmLabelSelector, }) if err != nil { klog.Errorf("Error setting up watch for HelmReleases: %v", err) @@ -401,6 +394,16 @@ func (r *REST) Watch(ctx context.Context, options *metainternalversion.ListOptio continue } + // Apply manual field selector filtering (metadata.name and metadata.namespace) + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + if !fieldFilter.MatchesName(hr.Name) { + continue + } + if !fieldFilter.MatchesNamespace(hr.Namespace) { + continue + } + if !r.hasTenantModuleLabel(hr) { continue } diff --git a/pkg/registry/core/tenantsecret/rest.go b/pkg/registry/core/tenantsecret/rest.go index e5dbf9dd..d069c6ae 100644 --- a/pkg/registry/core/tenantsecret/rest.go +++ b/pkg/registry/core/tenantsecret/rest.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" corev1alpha1 "github.com/cozystack/cozystack/pkg/apis/core/v1alpha1" + fieldfilter "github.com/cozystack/cozystack/pkg/registry/fields" "github.com/cozystack/cozystack/pkg/registry/sorting" ) @@ -248,9 +249,22 @@ func (r *REST) List(ctx context.Context, opts *metainternal.ListOptions) (runtim } } - fieldSel := "" - if opts.FieldSelector != nil { - fieldSel = opts.FieldSelector.String() + // Parse field selector for manual filtering + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + fieldFilter, err := fieldfilter.ParseFieldSelector(opts.FieldSelector) + if err != nil { + return nil, err + } + + // If field selector specifies namespace different from context, return empty list + if fieldFilter.Namespace != "" && ns != "" && ns != fieldFilter.Namespace { + return &corev1alpha1.TenantSecretList{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1alpha1.SchemeGroupVersion.String(), + Kind: kindTenantSecretList, + }, + }, nil } list := &corev1.SecretList{} @@ -258,10 +272,6 @@ func (r *REST) List(ctx context.Context, opts *metainternal.ListOptions) (runtim &client.ListOptions{ Namespace: ns, LabelSelector: ls, - Raw: &metav1.ListOptions{ - LabelSelector: ls.String(), - FieldSelector: fieldSel, - }, }) if err != nil { return nil, err @@ -276,6 +286,15 @@ func (r *REST) List(ctx context.Context, opts *metainternal.ListOptions) (runtim } for i := range list.Items { + // Apply manual field selector filtering (metadata.name and metadata.namespace) + // controller-runtime cache doesn't support field selectors + // See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + if !fieldFilter.MatchesName(list.Items[i].Name) { + continue + } + if !fieldFilter.MatchesNamespace(list.Items[i].Namespace) { + continue + } out.Items = append(out.Items, *secretToTenant(&list.Items[i])) } sorting.ByNamespacedName[corev1alpha1.TenantSecret, *corev1alpha1.TenantSecret](out.Items) @@ -413,11 +432,6 @@ func (r *REST) Watch(ctx context.Context, opts *metainternal.ListOptions) (watch base, err := r.w.Watch(ctx, secList, &client.ListOptions{ Namespace: ns, LabelSelector: ls, - Raw: &metav1.ListOptions{ - Watch: true, - LabelSelector: ls.String(), - ResourceVersion: opts.ResourceVersion, - }, }) if err != nil { return nil, err diff --git a/pkg/registry/fields/filter.go b/pkg/registry/fields/filter.go new file mode 100644 index 00000000..9b1fc246 --- /dev/null +++ b/pkg/registry/fields/filter.go @@ -0,0 +1,70 @@ +// Copyright 2024 The Cozystack Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fields + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/fields" +) + +// Filter holds field selector filters extracted from a field selector string. +type Filter struct { + // Name is the value from metadata.name field selector, empty if not specified + Name string + // Namespace is the value from metadata.namespace field selector, empty if not specified + Namespace string +} + +// ParseFieldSelector parses a field selector and extracts metadata.name and metadata.namespace values. +// Other field selectors are silently ignored as controller-runtime cache doesn't support them. +// See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 +func ParseFieldSelector(fieldSelector fields.Selector) (*Filter, error) { + if fieldSelector == nil { + return &Filter{}, nil + } + + fs, err := fields.ParseSelector(fieldSelector.String()) + if err != nil { + return nil, fmt.Errorf("invalid field selector: %v", err) + } + + filter := &Filter{} + + // Check if selector is for metadata.name + if name, exists := fs.RequiresExactMatch("metadata.name"); exists { + filter.Name = name + } + + // Check if selector is for metadata.namespace + if namespace, exists := fs.RequiresExactMatch("metadata.namespace"); exists { + filter.Namespace = namespace + } + + // Note: Other field selectors are silently ignored as controller-runtime cache + // doesn't support them. See: https://github.com/kubernetes-sigs/controller-runtime/issues/612 + + return filter, nil +} + +// MatchesName returns true if the filter has no name constraint or if the name matches. +func (f *Filter) MatchesName(name string) bool { + return f.Name == "" || f.Name == name +} + +// MatchesNamespace returns true if the filter has no namespace constraint or if the namespace matches. +func (f *Filter) MatchesNamespace(namespace string) bool { + return f.Namespace == "" || f.Namespace == namespace +}