Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconcile out of order gateway resource creation. #638

Merged
merged 17 commits into from
Jun 26, 2024
+416 −72
Merged

Reconcile out of order gateway resource creation. #638

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6369543
WIP: On gateway creation if no backrefs are found reconcile AuthPolic…
Boomatang May 10, 2024
e7c5a43
WIP: using dag for reconcile trigger.
Boomatang May 14, 2024
477f83b
fix unit tests
Boomatang May 16, 2024
1caf44d
Tidy up of some WIP code
Boomatang May 16, 2024
4468dbd
Fix the out-of-order creation issue for authPolices.
Boomatang May 30, 2024
c191d07
Fix issues raised from the CI tests
Boomatang May 30, 2024
bcdfa3c
Rebase updates
Boomatang Jun 4, 2024
f3a509b
Fix integration test, sleep required for some reason.
Boomatang Jun 4, 2024
3f49467
Refactor to remove the need for switches.
Boomatang Jun 12, 2024
7c3111a
Update unit tests to match refactor
Boomatang Jun 12, 2024
e0c4a9c
Remove deprecated `FromInt`
Boomatang Jun 12, 2024
acd0246
Updates after rebase
Boomatang Jun 12, 2024
1e14e54
small update to make lists.
Boomatang Jun 13, 2024
ca84e63
rebase format fix
Boomatang Jun 17, 2024
3250185
Revert time.sleep
Boomatang Jun 17, 2024
2901d51
Add logging for error message
Boomatang Jun 18, 2024
2b37b85
Address comment request
Boomatang Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions api/v1alpha1/dnspolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ limitations under the License.
package v1alpha1

import (
"context"
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

Expand Down Expand Up @@ -178,6 +180,21 @@ func (p *DNSPolicy) GetStatus() kuadrantgatewayapi.PolicyStatus {

func (p *DNSPolicy) Kind() string { return p.TypeMeta.Kind }

func (p *DNSPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not fuction instead of method?

func DNSPolicyList(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy {
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of where it is being used. The List method was added to the Policy Interface as it required in the mapping functions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bad smell is that you are not using the receiver, thus looks like a dog, walks like a dog and barks like a dog, but it is a cat. (dog => function, cat => method)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the previous iterations of this PR did not have this. Instead it listed the policies from the cluster based on the type passed in. There was a switch within the mapper function. It was suggested that this was the cleaner approach. I understand why you don't like it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method could have been name sth like ListAllOfKind.

policyList := &DNSPolicyList{}
listOptions := &client.ListOptions{Namespace: namespace}
err := c.List(ctx, policyList, listOptions)
if err != nil {
return []kuadrantgatewayapi.Policy{}
}
policies := make([]kuadrantgatewayapi.Policy, 0, len(policyList.Items))
for i := range policyList.Items {
policies = append(policies, &policyList.Items[i])
}

return policies
}

func (p *DNSPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass {
return kuadrantgatewayapi.DirectPolicy
}
Expand Down
17 changes: 17 additions & 0 deletions api/v1alpha1/tlspolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ limitations under the License.
package v1alpha1

import (
"context"
"fmt"

certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
certmanmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

Expand Down Expand Up @@ -146,6 +148,21 @@ type TLSPolicy struct {

func (p *TLSPolicy) Kind() string { return p.TypeMeta.Kind }

func (p *TLSPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy {
policyList := &TLSPolicyList{}
listOptions := &client.ListOptions{Namespace: namespace}
err := c.List(ctx, policyList, listOptions)
if err != nil {
return []kuadrantgatewayapi.Policy{}
}
policies := make([]kuadrantgatewayapi.Policy, 0, len(policyList.Items))
for i := range policyList.Items {
policies = append(policies, &policyList.Items[i])
}

return policies
}

func (p *TLSPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass {
return kuadrantgatewayapi.DirectPolicy
}
Expand Down
17 changes: 17 additions & 0 deletions api/v1beta2/authpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package v1beta2

import (
"context"
"fmt"

"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

Expand Down Expand Up @@ -338,6 +340,21 @@ func (ap *AuthPolicy) Kind() string {
return ap.TypeMeta.Kind
}

func (ap *AuthPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy {
policyList := &AuthPolicyList{}
listOptions := &client.ListOptions{Namespace: namespace}
err := c.List(ctx, policyList, listOptions)
if err != nil {
return []kuadrantgatewayapi.Policy{}
}
policies := make([]kuadrantgatewayapi.Policy, 0, len(policyList.Items))
for i := range policyList.Items {
policies = append(policies, &policyList.Items[i])
}

return policies
}

func (ap *AuthPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass {
return kuadrantgatewayapi.InheritedPolicy
}
Expand Down
17 changes: 17 additions & 0 deletions api/v1beta2/ratelimitpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ limitations under the License.
package v1beta2

import (
"context"
"fmt"

"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

Expand Down Expand Up @@ -272,6 +274,21 @@ func (r *RateLimitPolicy) Kind() string {
return r.TypeMeta.Kind
}

func (r *RateLimitPolicy) List(ctx context.Context, c client.Client, namespace string) []kuadrantgatewayapi.Policy {
policyList := &RateLimitPolicyList{}
listOptions := &client.ListOptions{Namespace: namespace}
err := c.List(ctx, policyList, listOptions)
eguzki marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return []kuadrantgatewayapi.Policy{}
}
policies := make([]kuadrantgatewayapi.Policy, 0, len(policyList.Items))
for i := range policyList.Items {
policies = append(policies, &policyList.Items[i])
}

return policies
}

func (r *RateLimitPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass {
return kuadrantgatewayapi.InheritedPolicy
}
Expand Down
8 changes: 4 additions & 4 deletions controllers/authpolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,21 +270,21 @@ func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
return nil
}

httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper")))
gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")))
httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper")), mappers.WithClient(mgr.GetClient()))
gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient()))

return ctrl.NewControllerManagedBy(mgr).
For(&api.AuthPolicy{}).
Owns(&authorinoapi.AuthConfig{}).
Watches(
&gatewayapiv1.HTTPRoute{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return httpRouteEventMapper.MapToPolicy(object, &api.AuthPolicy{})
return httpRouteEventMapper.MapToPolicy(ctx, object, &api.AuthPolicy{})
}),
).
Watches(&gatewayapiv1.Gateway{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return gatewayEventMapper.MapToPolicy(object, &api.AuthPolicy{})
return gatewayEventMapper.MapToPolicy(ctx, object, &api.AuthPolicy{})
}),
).
Complete(r)
Expand Down
4 changes: 2 additions & 2 deletions controllers/dnspolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
return nil
}

gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")))
gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient()))

r.dnsHelper = dnsHelper{Client: r.Client()}
ctrlr := ctrl.NewControllerManagedBy(mgr).
Expand All @@ -196,7 +196,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, &v1alpha1.DNSPolicy{})
return gatewayEventMapper.MapToPolicy(ctx, object, &v1alpha1.DNSPolicy{})
}),
)
return ctrlr.Complete(r)
Expand Down
5 changes: 2 additions & 3 deletions controllers/dnspolicy_status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ import (
"reflect"
"testing"

"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

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

func TestPropagateRecordConditions(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion controllers/helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ func testEndpointsTraversable(endpoints []*endpoint.Endpoint, host string, desti
// this means that at least one of the targets on the endpoint leads to the destination
allTargetsFound = allTargetsFound || testEndpointsTraversable(endpoints, target, []string{destination})
}

}
}
// we must match all destinations
Expand Down
8 changes: 4 additions & 4 deletions controllers/ratelimitpolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,23 +235,23 @@ func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
return nil
}

httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper")))
gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")))
httpRouteEventMapper := mappers.NewHTTPRouteEventMapper(mappers.WithLogger(r.Logger().WithName("httpRouteEventMapper")), mappers.WithClient(mgr.GetClient()))
gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient()))

return ctrl.NewControllerManagedBy(mgr).
For(&kuadrantv1beta2.RateLimitPolicy{}).
Watches(
&gatewayapiv1.HTTPRoute{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return httpRouteEventMapper.MapToPolicy(object, &kuadrantv1beta2.RateLimitPolicy{})
return httpRouteEventMapper.MapToPolicy(ctx, object, &kuadrantv1beta2.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, &kuadrantv1beta2.RateLimitPolicy{})
return gatewayEventMapper.MapToPolicy(ctx, object, &kuadrantv1beta2.RateLimitPolicy{})
}),
).
Complete(r)
Expand Down
4 changes: 2 additions & 2 deletions controllers/tlspolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,14 @@ func (r *TLSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error {
return nil
}

gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")))
gatewayEventMapper := mappers.NewGatewayEventMapper(mappers.WithLogger(r.Logger().WithName("gatewayEventMapper")), mappers.WithClient(mgr.GetClient()))

return ctrl.NewControllerManagedBy(mgr).
For(&v1alpha1.TLSPolicy{}).
Watches(
&gatewayapiv1.Gateway{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request {
return gatewayEventMapper.MapToPolicy(object, &v1alpha1.TLSPolicy{})
return gatewayEventMapper.MapToPolicy(ctx, object, &v1alpha1.TLSPolicy{})
}),
).
Complete(r)
Expand Down
8 changes: 4 additions & 4 deletions pkg/common/yaml_decoder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import (
"testing"

"github.com/go-logr/logr"
"github.com/kuadrant/kuadrant-operator/pkg/log"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"

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

type testCase struct {
Expand Down
5 changes: 3 additions & 2 deletions pkg/istio/mesh_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ package istio
import (
"testing"

maistrav1 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v1"
maistrav2 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v2"
"google.golang.org/protobuf/types/known/structpb"
"gotest.tools/assert"
istiomeshv1alpha1 "istio.io/api/mesh/v1alpha1"
Expand All @@ -16,6 +14,9 @@ import (
istiov1alpha1 "maistra.io/istio-operator/api/v1alpha1"
"maistra.io/istio-operator/pkg/helm"
"sigs.k8s.io/controller-runtime/pkg/client"

maistrav1 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v1"
maistrav2 "github.com/kuadrant/kuadrant-operator/api/external/maistra/v2"
)

type stubbedConfigWrapper struct {
Expand Down
6 changes: 3 additions & 3 deletions pkg/kuadranttools/limitador_tools_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import (
"strings"
"testing"

"k8s.io/utils/ptr"

"github.com/kuadrant/kuadrant-operator/api/v1beta1"
limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kuadrant/kuadrant-operator/api/v1beta1"
)

func TestLimitadorMutator(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion pkg/library/gatewayapi/helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
package gatewayapi

import (
"github.com/kuadrant/kuadrant-operator/pkg/library/utils"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

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

const (
Expand Down
6 changes: 6 additions & 0 deletions pkg/library/gatewayapi/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package gatewayapi

import (
"context"

"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
Expand All @@ -20,6 +22,10 @@ type Policy interface {
PolicyClass() PolicyClass
GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference
GetStatus() PolicyStatus
List(context.Context, client.Client, string) []Policy
Kind() string
BackReferenceAnnotationName() string
DirectReferenceAnnotationName() string
}

type PolicyStatus interface {
Expand Down
19 changes: 19 additions & 0 deletions pkg/library/gatewayapi/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
package gatewayapi

import (
"context"
"reflect"
"sort"
"testing"
"time"

"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/google/go-cmp/cmp"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -29,6 +32,22 @@ type TestPolicy struct {
Status FakePolicyStatus `json:"status"`
}

func (p *TestPolicy) Kind() string {
return "FakePolicy"
}

func (p *TestPolicy) List(ctx context.Context, c client.Client, namespace string) []Policy {
return nil
}

func (p *TestPolicy) BackReferenceAnnotationName() string {
return ""
}

func (p *TestPolicy) DirectReferenceAnnotationName() string {
return ""
}

func (p *TestPolicy) PolicyClass() PolicyClass {
return DirectPolicy
}
Expand Down
Loading
Loading