Skip to content

Commit

Permalink
fix: fix checkObjectNamespaceLabels func param type (#2477)
Browse files Browse the repository at this point in the history 
edit: fix checkObjectNamespaceLabels func param type

Signed-off-by: ShyunnY <1147212064@qq.com>
  • Loading branch information
@ShyunnY
ShyunnY authored Jan 23, 2024
1 parent 9359b78 commit 45c03a2
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 29 deletions.
12 changes: 6 additions & 6 deletions internal/provider/kubernetes/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,11 +580,11 @@ func (r *gatewayAPIReconciler) findReferenceGrant(ctx context.Context, from, to
if len(r.namespaceLabels) != 0 {
var rgs []gwapiv1b1.ReferenceGrant
for _, refGrant := range refGrants {
ns := refGrant.GetNamespace()
ok, err := r.checkObjectNamespaceLabels(ns)
refGrant := refGrant
ok, err := r.checkObjectNamespaceLabels(&refGrant)
if err != nil {
// TODO: should return? or just proceed?
return nil, fmt.Errorf("failed to check namespace labels for ReferenceGrant %s in namespace %s: %w", refGrant.GetName(), ns, err)
return nil, fmt.Errorf("failed to check namespace labels for ReferenceGrant %s in namespace %s: %w", refGrant.GetName(), refGrant.GetNamespace(), err)
}
if !ok {
// TODO: should log?
Expand Down Expand Up @@ -624,11 +624,11 @@ func (r *gatewayAPIReconciler) processGateways(ctx context.Context, acceptedGC *
if len(r.namespaceLabels) != 0 {
var gtws []gwapiv1.Gateway
for _, gtw := range gateways {
ns := gtw.GetNamespace()
ok, err := r.checkObjectNamespaceLabels(ns)
gtw := gtw
ok, err := r.checkObjectNamespaceLabels(&gtw)
if err != nil {
// TODO: should return? or just proceed?
return fmt.Errorf("failed to check namespace labels for gateway %s in namespace %s: %w", gtw.GetName(), ns, err)
return fmt.Errorf("failed to check namespace labels for gateway %s in namespace %s: %w", gtw.GetName(), gtw.GetNamespace(), err)
}

if ok {
Expand Down
6 changes: 3 additions & 3 deletions internal/provider/kubernetes/filters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ func (r *gatewayAPIReconciler) getExtensionRefFilters(ctx context.Context) ([]un
if len(r.namespaceLabels) != 0 {
var extRs []unstructured.Unstructured
for _, extR := range uExtResources {
ns := extR.GetNamespace()
ok, err := r.checkObjectNamespaceLabels(ns)
extR := extR
ok, err := r.checkObjectNamespaceLabels(&extR)
if err != nil {
// TODO: should return? or just proceed?
return nil, fmt.Errorf("failed to check namespace labels for ExtensionRefFilter %s in namespace %s: %w", extR.GetName(), ns, err)
return nil, fmt.Errorf("failed to check namespace labels for ExtensionRefFilter %s in namespace %s: %w", extR.GetName(), extR.GetNamespace(), err)
}
if ok {
extRs = append(extRs, extR)
Expand Down
15 changes: 10 additions & 5 deletions internal/provider/kubernetes/predicates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
corev1 "k8s.io/api/core/v1"
discoveryv1 "k8s.io/api/discovery/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
matav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -45,7 +46,7 @@ func (r *gatewayAPIReconciler) hasMatchingController(obj client.Object) bool {
// hasMatchingNamespaceLabels returns true if the namespace of provided object has
// the provided labels or false otherwise.
func (r *gatewayAPIReconciler) hasMatchingNamespaceLabels(obj client.Object) bool {
ok, err := r.checkObjectNamespaceLabels(obj.GetNamespace())
ok, err := r.checkObjectNamespaceLabels(obj)
if err != nil {
r.log.Error(
err, "failed to get Namespace",
Expand All @@ -61,10 +62,14 @@ type NamespaceGetter interface {
}

// checkObjectNamespaceLabels checks if labels of namespace of the object is a subset of namespaceLabels
// TODO: check if param can be an interface, so the caller doesn't need to get the namespace before calling
// this function.
func (r *gatewayAPIReconciler) checkObjectNamespaceLabels(nsString string) (bool, error) {
// TODO: add validation here because some objects don't have namespace
func (r *gatewayAPIReconciler) checkObjectNamespaceLabels(obj matav1.Object) (bool, error) {

var nsString string
// TODO: it requires extra condition validate cluster resources or resources without namespace?
if nsString = obj.GetNamespace(); len(nsString) == 0 {
return false, nil
}

ns := &corev1.Namespace{}
if err := r.client.Get(
context.Background(),
Expand Down
100 changes: 100 additions & 0 deletions internal/provider/kubernetes/predicates_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,103 @@ func TestValidateDeploymentForReconcile(t *testing.T) {
})
}
}

func TestCheckObjectNamespaceLabels(t *testing.T) {

testCases := []struct {
name string
object client.Object
reconcileLabels []string
ns *corev1.Namespace
expect bool
}{
{
name: "matching labels of namespace of the object is a subset of namespaceLabels",
object: test.GetHTTPRoute(
types.NamespacedName{
Name: "foo-route",
Namespace: "foo",
},
"eg",
types.NamespacedName{
Name: "foo-svc",
Namespace: "foo",
},
8080,
),
ns: &corev1.Namespace{
ObjectMeta: v1.ObjectMeta{
Name: "foo",
Labels: map[string]string{
"label-1": "",
},
},
},
reconcileLabels: []string{"label-1"},
expect: true,
},
{
name: "non-matching labels of namespace of the object is a subset of namespaceLabels",
object: test.GetHTTPRoute(
types.NamespacedName{
Name: "bar-route",
Namespace: "bar",
},
"eg",
types.NamespacedName{
Name: "bar-svc",
Namespace: "bar",
},
8080,
),
ns: &corev1.Namespace{
ObjectMeta: v1.ObjectMeta{
Name: "bar",
Labels: map[string]string{
"label-2": "",
},
},
},
reconcileLabels: []string{"label-1"},
expect: false,
},
{
name: "non-matching labels of namespace of the cluster-level object is a subset of namespaceLabels",
object: &corev1.Namespace{
ObjectMeta: v1.ObjectMeta{
Name: "foo-1",
Labels: map[string]string{
"label-1": "",
},
},
},
ns: &corev1.Namespace{
ObjectMeta: v1.ObjectMeta{
Name: "bar-1",
Labels: map[string]string{
"label-1": "",
},
},
},
reconcileLabels: []string{"label-1"},
expect: false,
},
}

// Create the reconciler.
logger := logging.DefaultLogger(v1alpha1.LogLevelInfo)

r := gatewayAPIReconciler{
classController: v1alpha1.GatewayControllerName,
log: logger,
}

for _, tc := range testCases {
tc := tc
r.client = fakeclient.NewClientBuilder().WithObjects(tc.ns).Build()
r.namespaceLabels = tc.reconcileLabels
ok, err := r.checkObjectNamespaceLabels(tc.object)
require.NoError(t, err)
require.Equal(t, tc.expect, ok)
}
}
30 changes: 15 additions & 15 deletions internal/provider/kubernetes/routes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ func (r *gatewayAPIReconciler) processTLSRoutes(ctx context.Context, gatewayName
if len(r.namespaceLabels) != 0 {
var rts []gwapiv1a2.TLSRoute
for _, rt := range tlsRoutes {
ns := rt.GetNamespace()
ok, err := r.checkObjectNamespaceLabels(ns)
rt := rt
ok, err := r.checkObjectNamespaceLabels(&rt)
if err != nil {
// TODO: should return? or just proceed?
return fmt.Errorf("failed to check namespace labels for TLSRoute %s in namespace %s: %w", rt.GetName(), ns, err)
return fmt.Errorf("failed to check namespace labels for TLSRoute %s in namespace %s: %w", rt.GetName(), rt.GetNamespace(), err)
}

if ok {
Expand Down Expand Up @@ -118,11 +118,11 @@ func (r *gatewayAPIReconciler) processGRPCRoutes(ctx context.Context, gatewayNam
if len(r.namespaceLabels) != 0 {
var grs []gwapiv1a2.GRPCRoute
for _, gr := range grpcRoutes {
ns := gr.GetNamespace()
ok, err := r.checkObjectNamespaceLabels(ns)
gr := gr
ok, err := r.checkObjectNamespaceLabels(&gr)
if err != nil {
// TODO: should return? or just proceed?
return fmt.Errorf("failed to check namespace labels for GRPCRoute %s in namespace %s: %w", gr.GetName(), ns, err)
return fmt.Errorf("failed to check namespace labels for GRPCRoute %s in namespace %s: %w", gr.GetName(), gr.GetNamespace(), err)
}
if ok {
grs = append(grs, gr)
Expand Down Expand Up @@ -247,11 +247,11 @@ func (r *gatewayAPIReconciler) processHTTPRoutes(ctx context.Context, gatewayNam
if len(r.namespaceLabels) != 0 {
var hrs []gwapiv1.HTTPRoute
for _, hr := range httpRoutes {
ns := hr.GetNamespace()
ok, err := r.checkObjectNamespaceLabels(ns)
hr := hr
ok, err := r.checkObjectNamespaceLabels(&hr)
if err != nil {
// TODO: should return? or just proceed?
return fmt.Errorf("failed to check namespace labels for HTTPRoute %s in namespace %s: %w", hr.GetName(), ns, err)
return fmt.Errorf("failed to check namespace labels for HTTPRoute %s in namespace %s: %w", hr.GetName(), hr.GetNamespace(), err)
}

if ok {
Expand Down Expand Up @@ -420,11 +420,11 @@ func (r *gatewayAPIReconciler) processTCPRoutes(ctx context.Context, gatewayName
if len(r.namespaceLabels) != 0 {
var trs []gwapiv1a2.TCPRoute
for _, tr := range tcpRoutes {
ns := tr.GetNamespace()
ok, err := r.checkObjectNamespaceLabels(ns)
tr := tr
ok, err := r.checkObjectNamespaceLabels(&tr)
if err != nil {
// TODO: should return? or just proceed?
return fmt.Errorf("failed to check namespace labels for TCPRoute %s in namespace %s: %w", tr.GetName(), ns, err)
return fmt.Errorf("failed to check namespace labels for TCPRoute %s in namespace %s: %w", tr.GetName(), tr.GetNamespace(), err)
}

if ok {
Expand Down Expand Up @@ -501,11 +501,11 @@ func (r *gatewayAPIReconciler) processUDPRoutes(ctx context.Context, gatewayName
if len(r.namespaceLabels) != 0 {
var urs []gwapiv1a2.UDPRoute
for _, ur := range udpRoutes {
ns := ur.GetNamespace()
ok, err := r.checkObjectNamespaceLabels(ns)
ur := ur
ok, err := r.checkObjectNamespaceLabels(&ur)
if err != nil {
// TODO: should return? or just proceed?
return fmt.Errorf("failed to check namespace labels for UDPRoute %s in namespace %s: %w", ur.GetName(), ns, err)
return fmt.Errorf("failed to check namespace labels for UDPRoute %s in namespace %s: %w", ur.GetName(), ur.GetNamespace(), err)
}

if ok {
Expand Down

0 comments on commit 45c03a2

Please sign in to comment.