Skip to content

Commit

Permalink
Fix issues raised from the CI tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Boomatang committed May 30, 2024
1 parent 805f896 commit 71a3057
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 45 deletions.
4 changes: 2 additions & 2 deletions controllers/authpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,12 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
Watches(
&gatewayapiv1.HTTPRoute{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return httpRouteEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
return httpRouteEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
}),
).
Watches(&gatewayapiv1.Gateway{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
}),
).
Complete(r)
Expand Down
2 changes: 1 addition & 1 deletion controllers/dnspolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
Watches(
&gatewayapiv1.Gateway{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "DNSPolicy"})
return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "v1alpha1", Kind: "DNSPolicy"})
}),
)
return ctrlr.Complete(r)
Expand Down
4 changes: 2 additions & 2 deletions controllers/ratelimitpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,15 +234,15 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
Watches(
&gatewayapiv1.HTTPRoute{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return httpRouteEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"})
return httpRouteEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"})
}),
).
// Currently the purpose is to generate events when rlp references change in gateways
// so the status of the rlps targeting a route can be keep in sync
Watches(
&gatewayapiv1.Gateway{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"})
return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"})
}),
).
Complete(r)
Expand Down
2 changes: 1 addition & 1 deletion controllers/tlspolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (r *TLSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
Watches(
&gatewayapiv1.Gateway{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return gatewayEventMapper.MapToPolicy(object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1alpha1", Kind: "TLSPolicy"})
return gatewayEventMapper.MapToPolicy(ctx, object, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1alpha1", Kind: "TLSPolicy"})
}),
).
Complete(r)
Expand Down
7 changes: 4 additions & 3 deletions pkg/library/kuadrant/referrer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package kuadrant

import (
"encoding/json"
"fmt"
"strings"

"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -35,16 +36,16 @@ func BackReferencesFromObject(obj client.Object, referrer Referrer) []client.Obj
return refs
}

func DirectReferencesFromObject(obj client.Object, referrer Referrer) client.ObjectKey {
func DirectReferencesFromObject(obj client.Object, referrer Referrer) (client.ObjectKey, error) {
annotations := utils.ReadAnnotationsFromObject(obj)
key := referrer.DirectReferenceAnnotationName()
directRefs, found := annotations[key]
if !found {
return client.ObjectKey{}
return client.ObjectKey{}, fmt.Errorf("annotation %s not found", key)
}

parts := strings.Split(directRefs, "/")
ref := client.ObjectKey{Namespace: parts[0], Name: parts[1]}

return ref
return ref, nil
}
7 changes: 4 additions & 3 deletions pkg/library/mappers/event_mapper.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
package mappers

import (
"context"

"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
)

type EventMapper interface {
MapToPolicy(client.Object, kuadrant.Referrer) []reconcile.Request
MapToPolicy(context.Context, client.Object, schema.GroupVersionKind) []reconcile.Request
}

// options
Expand Down
14 changes: 4 additions & 10 deletions pkg/library/mappers/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,18 @@ import (
"github.com/kuadrant/kuadrant-operator/pkg/library/utils"
)

// TODO: Clean this up
type EventMapperTwo interface {
MapToPolicy(client.Object, schema.GroupVersionKind) []reconcile.Request
}

func NewGatewayEventMapper(o ...MapperOption) EventMapperTwo {
func NewGatewayEventMapper(o ...MapperOption) EventMapper {
return &gatewayEventMapper{opts: Apply(o...)}
}

var _ EventMapperTwo = &gatewayEventMapper{}
var _ EventMapper = &gatewayEventMapper{}

type gatewayEventMapper struct {
opts MapperOptions
}

func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request {
func (m *gatewayEventMapper) MapToPolicy(ctx context.Context, obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request {
logger := m.opts.Logger.WithValues("gateway", client.ObjectKeyFromObject(obj))
ctx := context.Background()
gateway, ok := obj.(*gatewayapiv1.Gateway)
if !ok {
logger.V(1).Info(fmt.Sprintf("%T is not type gateway, unable to map policies to gateway", obj))
Expand All @@ -53,7 +47,7 @@ func (m *gatewayEventMapper) MapToPolicy(obj client.Object, policyGVK schema.Gro
policyList.SetAPIVersion(policyGVK.Version)
policyList.SetKind(policyGVK.Kind)
if err := m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil {
logger.V(1).Info("unable to list UnstructuredList of policies, %T", policyGVK)
logger.V(1).Info("unable to list UnstructuredList of policies, %T", "policyGVl", policyGVK)
return []reconcile.Request{}
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/library/mappers/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package mappers

import (
"context"
"testing"

"gotest.tools/assert"
Expand Down Expand Up @@ -68,12 +69,12 @@ func TestNewGatewayEventMapper(t *testing.T) {
em := NewGatewayEventMapper(WithLogger(log.NewLogger()), WithClient(cl))

t.Run("not gateway related event", func(subT *testing.T) {
requests := em.MapToPolicy(&gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"})
requests := em.MapToPolicy(context.Background(), &gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"})
assert.DeepEqual(subT, []reconcile.Request{}, requests)
})

t.Run("gateway related event - no requests", func(subT *testing.T) {
requests := em.MapToPolicy(&gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"})
requests := em.MapToPolicy(context.Background(), &gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "RateLimitPolicy"})
assert.DeepEqual(subT, []reconcile.Request{}, requests)
})

Expand All @@ -89,7 +90,7 @@ func TestNewGatewayEventMapper(t *testing.T) {
},
},
}
requests := em.MapToPolicy(gateway, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
requests := em.MapToPolicy(context.Background(), gateway, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}, {NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-2"}}}
assert.DeepEqual(subT, expected, requests)
})
Expand Down
26 changes: 15 additions & 11 deletions pkg/library/mappers/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ package mappers
import (
"context"
"fmt"
"github.com/kuadrant/kuadrant-operator/api/v1alpha1"
api "github.com/kuadrant/kuadrant-operator/api/v1beta2"
kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
"github.com/kuadrant/kuadrant-operator/pkg/library/utils"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -16,21 +12,26 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/kuadrant/kuadrant-operator/api/v1alpha1"
api "github.com/kuadrant/kuadrant-operator/api/v1beta2"
kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
"github.com/kuadrant/kuadrant-operator/pkg/library/utils"
)

func NewHTTPRouteEventMapper(o ...MapperOption) EventMapperTwo {
func NewHTTPRouteEventMapper(o ...MapperOption) EventMapper {
return &httpRouteEventMapper{opts: Apply(o...)}
}

var _ EventMapperTwo = &httpRouteEventMapper{}
var _ EventMapper = &httpRouteEventMapper{}

type httpRouteEventMapper struct {
opts MapperOptions
}

func (m *httpRouteEventMapper) MapToPolicy(obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request {
func (m *httpRouteEventMapper) MapToPolicy(ctx context.Context, obj client.Object, policyGVK schema.GroupVersionKind) []reconcile.Request {
logger := m.opts.Logger.WithValues("httproute", client.ObjectKeyFromObject(obj))
ctx := context.Background()
requests := make([]reconcile.Request, 0)
httpRoute, ok := obj.(*gatewayapiv1.HTTPRoute)
if !ok {
Expand Down Expand Up @@ -58,7 +59,7 @@ func (m *httpRouteEventMapper) MapToPolicy(obj client.Object, policyGVK schema.G
policyList.SetAPIVersion(policyGVK.Version)
policyList.SetKind(policyGVK.Kind)
if err = m.opts.Client.List(ctx, policyList, client.InNamespace(obj.GetNamespace())); err != nil {
logger.V(1).Info("unable to list UnstructuredList of policies, %T", policyGVK)
logger.V(1).Info("unable to list UnstructuredList of policies, %T", "policyGVK", policyGVK)
continue
}

Expand Down Expand Up @@ -147,7 +148,10 @@ func (m *httpRouteEventMapper) MapToPolicy(obj client.Object, policyGVK schema.G
default:
return requests
}
policyKey := kuadrant.DirectReferencesFromObject(httpRoute, policy)
policyKey, err := kuadrant.DirectReferencesFromObject(httpRoute, policy)
if err != nil {
return requests
}
requests = append(requests, reconcile.Request{NamespacedName: policyKey})
return requests
}
117 changes: 108 additions & 9 deletions pkg/library/mappers/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,134 @@
package mappers

import (
"context"
"testing"

"github.com/kuadrant/kuadrant-operator/pkg/library/utils"
"github.com/kuadrant/kuadrant-operator/pkg/log"
"gotest.tools/assert"
"istio.io/client-go/pkg/clientset/versioned/scheme"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
"github.com/kuadrant/kuadrant-operator/pkg/log"
kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2"
kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi"
)

func TestNewHTTPRouteEventMapper(t *testing.T) {
em := NewHTTPRouteEventMapper(WithLogger(log.NewLogger()))
err := appsv1.AddToScheme(scheme.Scheme)
if err != nil {
t.Fatal(err)
}
err = gatewayapiv1.AddToScheme(scheme.Scheme)
if err != nil {
t.Fatal(err)
}
err = kuadrantv1beta2.AddToScheme(scheme.Scheme)
if err != nil {
t.Fatal(err)
}

spec := kuadrantv1beta2.AuthPolicySpec{
TargetRef: gatewayapiv1alpha2.PolicyTargetReference{
Group: "gateway.networking.k8s.io",
Kind: "HTTPRoute",
Name: "test-route",
},
}
routeList := &gatewayapiv1.HTTPRouteList{Items: make([]gatewayapiv1.HTTPRoute, 0)}
authPolicyList := &kuadrantv1beta2.AuthPolicyList{Items: []kuadrantv1beta2.AuthPolicy{
{
ObjectMeta: metav1.ObjectMeta{
Name: "policy-1",
Namespace: "app-ns",
},
Spec: spec,
},
}}
gateway := &gatewayapiv1.Gateway{
ObjectMeta: metav1.ObjectMeta{Name: "test-gw", Namespace: "app-ns"},
Status: gatewayapiv1alpha2.GatewayStatus{
Conditions: []metav1.Condition{
{
Type: "Programmed",
Status: "True",
},
},
},
}
objs := []runtime.Object{routeList, authPolicyList, gateway}
cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, kuadrantgatewayapi.HTTPRouteGatewayParentField, func(rawObj client.Object) []string {
return nil
}).Build()
em := NewHTTPRouteEventMapper(WithLogger(log.NewLogger()), WithClient(cl))

t.Run("not http route related event", func(subT *testing.T) {
requests := em.MapToPolicy(&gatewayapiv1.Gateway{}, &kuadrant.PolicyKindStub{})
requests := em.MapToPolicy(context.Background(), &gatewayapiv1.Gateway{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
assert.DeepEqual(subT, []reconcile.Request{}, requests)
})

t.Run("http route related event - no requests", func(subT *testing.T) {
requests := em.MapToPolicy(&gatewayapiv1.HTTPRoute{}, &kuadrant.PolicyKindStub{})
requests := em.MapToPolicy(context.Background(), &gatewayapiv1.HTTPRoute{}, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
assert.DeepEqual(subT, []reconcile.Request{}, requests)
})

t.Run("http related event - requests", func(subT *testing.T) {
httpRoute := &gatewayapiv1.HTTPRoute{}
httpRoute.SetAnnotations(map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`})
requests := em.MapToPolicy(httpRoute, &kuadrant.PolicyKindStub{})
expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}, {NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-2"}}}
httpRoute := &gatewayapiv1.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "test-route",
Namespace: "app-ns",
Annotations: map[string]string{"kuadrant.io/testpolicies": `[{"Namespace":"app-ns","Name":"policy-1"},{"Namespace":"app-ns","Name":"policy-2"}]`},
},
Spec: gatewayapiv1.HTTPRouteSpec{
CommonRouteSpec: gatewayapiv1.CommonRouteSpec{
ParentRefs: []gatewayapiv1.ParentReference{{Namespace: ptr.To(gatewayapiv1.Namespace("app-ns")), Name: "test-gw"}},
},
},

Status: gatewayapiv1.HTTPRouteStatus{
RouteStatus: gatewayapiv1.RouteStatus{
Parents: []gatewayapiv1.RouteParentStatus{
{
ParentRef: gatewayapiv1.ParentReference{
Name: "test-gw",
Namespace: ptr.To(gatewayapiv1.Namespace("app-ns")),
},
Conditions: []metav1.Condition{
{
Type: "Accepted",
Status: metav1.ConditionTrue,
},
},
},
},
},
},
}

objs = []runtime.Object{routeList, authPolicyList, gateway, httpRoute}
cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(objs...).WithIndex(&gatewayapiv1.HTTPRoute{}, kuadrantgatewayapi.HTTPRouteGatewayParentField, func(rawObj client.Object) []string {
route, assertionOk := rawObj.(*gatewayapiv1.HTTPRoute)
if !assertionOk {
return nil
}

return utils.Map(kuadrantgatewayapi.GetRouteAcceptedGatewayParentKeys(route), func(key client.ObjectKey) string {
return key.String()
})
}).Build()
em = NewHTTPRouteEventMapper(WithLogger(log.NewLogger()), WithClient(cl))
requests := em.MapToPolicy(context.Background(), httpRoute, schema.GroupVersionKind{Group: "kuadrant.io", Version: "kuadrant.io/v1beta2", Kind: "AuthPolicy"})
expected := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "app-ns", Name: "policy-1"}}}
assert.DeepEqual(subT, expected, requests)
})
}

0 comments on commit 71a3057

Please sign in to comment.