Skip to content

Commit

Permalink
feat: rlp conflicted reason
Browse files Browse the repository at this point in the history
  • Loading branch information
KevFan committed Dec 5, 2023
1 parent 2811e55 commit fe46db0
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 34 deletions.
4 changes: 4 additions & 0 deletions api/v1beta2/authpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ func (ap *AuthPolicy) GetRulesHostnames() (ruleHosts []string) {
return
}

func (ap *AuthPolicy) Kind() string {
return ap.TypeMeta.Kind
}

//+kubebuilder:object:root=true

// AuthPolicyList contains a list of AuthPolicy
Expand Down
4 changes: 4 additions & 0 deletions api/v1beta2/ratelimitpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ func (r *RateLimitPolicy) GetRulesHostnames() (ruleHosts []string) {
return
}

func (r *RateLimitPolicy) Kind() string {
return r.TypeMeta.Kind
}

func init() {
SchemeBuilder.Register(&RateLimitPolicy{}, &RateLimitPolicyList{})
}
4 changes: 2 additions & 2 deletions controllers/authpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ func (r *AuthPolicyReconciler) deleteResources(ctx context.Context, ap *api.Auth
}

// Ensures only one RLP targets the network resource
func (r *AuthPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error {
return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(ap), targetNetworkObject, common.AuthPolicyBackRefAnnotation)
func (r *AuthPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx context.Context, ap common.KuadrantPolicy, targetNetworkObject client.Object) error {
return r.ReconcileTargetBackReference(ctx, ap, targetNetworkObject, common.AuthPolicyBackRefAnnotation)
}

func (r *AuthPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, targetNetworkObject client.Object) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ var _ = Describe("Limitador Cluster EnvoyFilter controller", func() {
Expect(err).ToNot(HaveOccurred())
// Check RLP status is available
rlpKey := client.ObjectKey{Name: rlpName, Namespace: testNamespace}
Eventually(testRLPIsAvailable(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())
Eventually(testRLPIsAccepted(rlpKey), time.Minute, 5*time.Second).Should(BeTrue())

// Check envoy filter
Eventually(func() bool {
Expand Down
8 changes: 4 additions & 4 deletions controllers/ratelimitpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl
if delResErr == nil {
delResErr = err
}
return r.reconcileStatus(ctx, rlp, common.ErrTargetNotFound{Kind: rlp.Kind, TargetRef: rlp.GetTargetRef()})
return r.reconcileStatus(ctx, rlp, common.ErrTargetNotFound{Kind: rlp.Kind(), TargetRef: rlp.GetTargetRef()})
}
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -154,12 +154,12 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp
// TODO - Validation Error
err := rlp.Validate()
if err != nil {
return common.ErrInvalid{Kind: rlp.Kind, Err: err}
return common.ErrInvalid{Kind: rlp.Kind(), Err: err}
}

err = common.ValidateHierarchicalRules(rlp, targetNetworkObject)
if err != nil {
return common.ErrInvalid{Kind: rlp.Kind, Err: err}
return common.ErrInvalid{Kind: rlp.Kind(), Err: err}
}

// reconcile based on gateway diffs
Expand Down Expand Up @@ -213,7 +213,7 @@ func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *ku

// Ensures only one RLP targets the network resource
func (r *RateLimitPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx context.Context, policy common.KuadrantPolicy, targetNetworkObject client.Object) error {
return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(policy), targetNetworkObject, common.RateLimitPolicyBackRefAnnotation)
return r.ReconcileTargetBackReference(ctx, policy, targetNetworkObject, common.RateLimitPolicyBackRefAnnotation)
}

func (r *RateLimitPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, targetNetworkObject client.Object) error {
Expand Down
6 changes: 3 additions & 3 deletions controllers/ratelimitpolicy_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ func (r *RateLimitPolicyReconciler) calculateStatus(_ context.Context, rlp *kuad
return newStatus
}

// TODO - Accepted Condition
func (r *RateLimitPolicyReconciler) acceptedCondition(specErr error) *metav1.Condition {
// Accepted
cond := &metav1.Condition{
Expand All @@ -89,12 +88,13 @@ func (r *RateLimitPolicyReconciler) acceptedCondition(specErr error) *metav1.Con
// Invalid
case errors.As(specErr, &common.ErrInvalid{}):
cond.Reason = string(gatewayapiv1alpha2.PolicyReasonInvalid)
// Conflicted
case errors.As(specErr, &common.ErrConflict{}):
cond.Reason = string(gatewayapiv1alpha2.PolicyReasonConflicted)
default:
cond.Reason = "ReconciliationError"
}
}

// TODO - Conflicted Reason

return cond
}
20 changes: 0 additions & 20 deletions pkg/common/apimachinery_status_conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,12 @@ package common

import (
"encoding/json"
"fmt"
"sort"

"golang.org/x/exp/slices"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
)

type ErrTargetNotFound struct {
Kind string
TargetRef gatewayapiv1alpha2.PolicyTargetReference
}

func (e ErrTargetNotFound) Error() string {
return fmt.Sprintf("%s target %s was not found", e.Kind, e.TargetRef.Name)
}

type ErrInvalid struct {
Kind string
Err error
}

func (e ErrInvalid) Error() string {
return fmt.Sprintf("%s target is invalid: %s", e.Kind, e.Err.Error())
}

// ConditionMarshal marshals the set of conditions as a JSON array, sorted by condition type.
func ConditionMarshal(conditions []metav1.Condition) ([]byte, error) {
condCopy := slices.Clone(conditions)
Expand Down
1 change: 1 addition & 0 deletions pkg/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type KuadrantPolicy interface {
GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference
GetWrappedNamespace() gatewayapiv1.Namespace
GetRulesHostnames() []string
Kind() string
}

type KuadrantPolicyList interface {
Expand Down
35 changes: 35 additions & 0 deletions pkg/common/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package common

import (
"fmt"

gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
)

type ErrTargetNotFound struct {
Kind string
TargetRef gatewayapiv1alpha2.PolicyTargetReference
}

func (e ErrTargetNotFound) Error() string {
return fmt.Sprintf("%s target %s was not found", e.Kind, e.TargetRef.Name)
}

type ErrInvalid struct {
Kind string
Err error
}

func (e ErrInvalid) Error() string {
return fmt.Sprintf("%s target is invalid: %s", e.Kind, e.Err.Error())
}

type ErrConflict struct {
Kind string
NameNamespace string
Err error
}

func (e ErrConflict) Error() string {
return fmt.Sprintf("%s is conflicted by %s: %s", e.Kind, e.NameNamespace, e.Err.Error())
}
4 changes: 4 additions & 0 deletions pkg/common/gatewayapi_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1320,6 +1320,10 @@ func (p *FakePolicy) GetRulesHostnames() []string {
return p.Hosts
}

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

func TestValidateHierarchicalRules(t *testing.T) {
hostname := gatewayapiv1.Hostname("*.example.com")
gateway := &gatewayapiv1.Gateway{
Expand Down
5 changes: 3 additions & 2 deletions pkg/reconcilers/targetref_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,10 @@ func (r *TargetRefReconciler) TargetedGatewayKeys(_ context.Context, targetNetwo
}

// ReconcileTargetBackReference adds policy key in annotations of the target object
func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, policyKey client.ObjectKey, targetNetworkObject client.Object, annotationName string) error {
func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, policy common.KuadrantPolicy, targetNetworkObject client.Object, annotationName string) error {
logger, _ := logr.FromContext(ctx)

policyKey := client.ObjectKeyFromObject(policy)
targetNetworkObjectKey := client.ObjectKeyFromObject(targetNetworkObject)
targetNetworkObjectKind := targetNetworkObject.GetObjectKind().GroupVersionKind()

Expand All @@ -165,7 +166,7 @@ func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context,

if val, ok := objAnnotations[annotationName]; ok {
if val != policyKey.String() {
return fmt.Errorf("the %s target %s is already referenced by policy %s", targetNetworkObjectKind, targetNetworkObjectKey, policyKey.String())
return common.ErrConflict{Kind: policy.Kind(), NameNamespace: val, Err: fmt.Errorf("the %s target %s is already referenced by policy %s", targetNetworkObjectKind, targetNetworkObjectKey, val)}
}
} else {
objAnnotations[annotationName] = policyKey.String()
Expand Down
6 changes: 4 additions & 2 deletions pkg/reconcilers/targetref_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"testing"

"github.com/go-logr/logr"
"github.com/kuadrant/kuadrant-operator/api/v1beta2"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -302,6 +303,7 @@ func TestReconcileTargetBackReference(t *testing.T) {
t.Fatal(err)
}

policy := &v1beta2.RateLimitPolicy{ObjectMeta: metav1.ObjectMeta{Name: "someName", Namespace: "someNamespace"}}
policyKey := client.ObjectKey{Name: "someName", Namespace: "someNamespace"}

existingRoute := &gatewayapiv1.HTTPRoute{
Expand Down Expand Up @@ -354,7 +356,7 @@ func TestReconcileTargetBackReference(t *testing.T) {
BaseReconciler: baseReconciler,
}

err = targetRefReconciler.ReconcileTargetBackReference(ctx, policyKey, existingRoute, annotationName)
err = targetRefReconciler.ReconcileTargetBackReference(ctx, policy, existingRoute, annotationName)
if err != nil {
t.Fatal(err)
}
Expand All @@ -378,7 +380,7 @@ func TestReconcileTargetBackReference(t *testing.T) {
t.Fatal("expected annotation not found")
}

if val != policyKey.String() {
if val != client.ObjectKeyFromObject(policy).String() {
t.Fatalf("annotation value (%s) does not match expected (%s)", val, policyKey.String())
}
}
Expand Down

0 comments on commit fe46db0

Please sign in to comment.