Skip to content

Commit

Permalink
PolicyAffected condition for AuthPolicy and RateLimitPolicy (#536)
Browse files Browse the repository at this point in the history
* PolicyAffected status condition added to Gateway and HTTPRoute objects for AuthPolicy and RateLimitPolicy

* refactor: fetch routes accepted by a given gateway without specifying the gateway controller name

Removes the need for authorizing reading gateway classes

* Trigger target status reconciliation for policies when the policy status changed only

* refactor: trigger target status reconciliation for policies when the policy status changed only based on simple predicate function instead of custom handler

* tests: fixed integration test for policyaffected condition inherited by the routes
  • Loading branch information
guicassolato authored Apr 23, 2024
1 parent 82aaff3 commit bed695f
Show file tree
Hide file tree
Showing 32 changed files with 1,224 additions and 295 deletions.
24 changes: 24 additions & 0 deletions api/v1alpha1/dnspolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import (
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

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"
)

type RoutingStrategy string
Expand Down Expand Up @@ -128,6 +130,10 @@ type DNSPolicyStatus struct {
HealthCheck *HealthCheckStatus `json:"healthCheck,omitempty"`
}

func (s *DNSPolicyStatus) GetConditions() []metav1.Condition {
return s.Conditions
}

var _ kuadrant.Policy = &DNSPolicy{}
var _ kuadrant.Referrer = &DNSPolicy{}

Expand Down Expand Up @@ -160,8 +166,16 @@ func (p *DNSPolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference {
return p.Spec.TargetRef
}

func (p *DNSPolicy) GetStatus() kuadrantgatewayapi.PolicyStatus {
return &p.Status
}

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

func (p *DNSPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass {
return kuadrantgatewayapi.DirectPolicy
}

func (p *DNSPolicy) BackReferenceAnnotationName() string {
return DNSPolicyBackReferenceAnnotationName
}
Expand Down Expand Up @@ -209,6 +223,12 @@ type DNSPolicyList struct {
Items []DNSPolicy `json:"items"`
}

func (l *DNSPolicyList) GetItems() []kuadrant.Policy {
return utils.Map(l.Items, func(item DNSPolicy) kuadrant.Policy {
return &item
})
}

// HealthCheckSpec configures health checks in the DNS provider.
// By default, this health check will be applied to each unique DNS A Record for
// the listeners assigned to the target gateway
Expand Down Expand Up @@ -267,6 +287,10 @@ func init() {

func NewDNSPolicy(name, ns string) *DNSPolicy {
return &DNSPolicy{
TypeMeta: metav1.TypeMeta{
Kind: "DNSPolicy",
APIVersion: GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: ns,
Expand Down
24 changes: 24 additions & 0 deletions api/v1alpha1/tlspolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import (
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

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"
)

const (
Expand Down Expand Up @@ -116,6 +118,10 @@ type TLSPolicyStatus struct {
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

func (s *TLSPolicyStatus) GetConditions() []metav1.Condition {
return s.Conditions
}

var _ kuadrant.Policy = &TLSPolicy{}
var _ kuadrant.Referrer = &TLSPolicy{}

Expand All @@ -138,6 +144,10 @@ type TLSPolicy struct {

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

func (p *TLSPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass {
return kuadrantgatewayapi.DirectPolicy
}

func (p *TLSPolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
return gatewayapiv1.Namespace(p.Namespace)
}
Expand All @@ -150,6 +160,10 @@ func (p *TLSPolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference {
return p.Spec.TargetRef
}

func (p *TLSPolicy) GetStatus() kuadrantgatewayapi.PolicyStatus {
return &p.Status
}

func (p *TLSPolicy) BackReferenceAnnotationName() string {
return TLSPolicyBackReferenceAnnotationName
}
Expand Down Expand Up @@ -183,6 +197,12 @@ type TLSPolicyList struct {
Items []TLSPolicy `json:"items"`
}

func (l *TLSPolicyList) GetItems() []kuadrant.Policy {
return utils.Map(l.Items, func(item TLSPolicy) kuadrant.Policy {
return &item
})
}

func init() {
SchemeBuilder.Register(&TLSPolicy{}, &TLSPolicyList{})
}
Expand All @@ -191,6 +211,10 @@ func init() {

func NewTLSPolicy(policyName, ns string) *TLSPolicy {
return &TLSPolicy{
TypeMeta: metav1.TypeMeta{
Kind: "TLSPolicy",
APIVersion: GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: policyName,
Namespace: ns,
Expand Down
26 changes: 13 additions & 13 deletions api/v1beta2/authpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (
"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"

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"
)
Expand Down Expand Up @@ -242,6 +242,10 @@ func (s *AuthPolicyStatus) Equals(other *AuthPolicyStatus, logger logr.Logger) b
return true
}

func (s *AuthPolicyStatus) GetConditions() []metav1.Condition {
return s.Conditions
}

var _ kuadrant.Policy = &AuthPolicy{}
var _ kuadrant.Referrer = &AuthPolicy{}

Expand All @@ -267,18 +271,6 @@ func (ap *AuthPolicy) IsAtomicOverride() bool {
return ap.Spec.Overrides != nil
}

func (ap *AuthPolicy) TargetKey() client.ObjectKey {
ns := ap.Namespace
if ap.Spec.TargetRef.Namespace != nil {
ns = string(*ap.Spec.TargetRef.Namespace)
}

return client.ObjectKey{
Name: string(ap.Spec.TargetRef.Name),
Namespace: ns,
}
}

func (ap *AuthPolicy) Validate() error {
if ap.Spec.TargetRef.Namespace != nil && string(*ap.Spec.TargetRef.Namespace) != ap.Namespace {
return fmt.Errorf("invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", *ap.Spec.TargetRef.Namespace)
Expand All @@ -291,6 +283,10 @@ func (ap *AuthPolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference {
return ap.Spec.TargetRef
}

func (ap *AuthPolicy) GetStatus() kuadrantgatewayapi.PolicyStatus {
return &ap.Status
}

func (ap *AuthPolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
return gatewayapiv1.Namespace(ap.Namespace)
}
Expand Down Expand Up @@ -342,6 +338,10 @@ func (ap *AuthPolicy) Kind() string {
return ap.TypeMeta.Kind
}

func (ap *AuthPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass {
return kuadrantgatewayapi.InheritedPolicy
}

func (ap *AuthPolicy) BackReferenceAnnotationName() string {
return AuthPolicyBackReferenceAnnotationName
}
Expand Down
28 changes: 0 additions & 28 deletions api/v1beta2/authpolicy_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,34 +47,6 @@ func TestAuthPolicySpecGetRouteSelectors(t *testing.T) {
}
}

func TestAuthPolicyTargetKey(t *testing.T) {
policy := &AuthPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "my-policy",
Namespace: "my-namespace",
},
Spec: AuthPolicySpec{
TargetRef: gatewayapiv1alpha2.PolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: "HTTPRoute",
Name: "my-route",
},
},
}
// targetRef missing namespace
expected := "my-namespace/my-route"
if result := policy.TargetKey().String(); result != expected {
t.Errorf("Expected target key %s, got %s", expected, result)
}

// targetRef with namespace
policy.Spec.TargetRef.Namespace = ptr.To(gatewayapiv1.Namespace("route-namespace"))
expected = "route-namespace/my-route"
if result := policy.TargetKey().String(); result != expected {
t.Errorf("Expected target key %s, got %s", expected, result)
}
}

func TestAuthPolicyListGetItems(t *testing.T) {
list := &AuthPolicyList{}
if len(list.GetItems()) != 0 {
Expand Down
25 changes: 12 additions & 13 deletions api/v1beta2/ratelimitpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"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 @@ -190,6 +189,10 @@ func (s *RateLimitPolicyStatus) Equals(other *RateLimitPolicyStatus, logger logr
return true
}

func (s *RateLimitPolicyStatus) GetConditions() []metav1.Condition {
return s.Conditions
}

var _ kuadrant.Policy = &RateLimitPolicy{}
var _ kuadrant.Referrer = &RateLimitPolicy{}

Expand Down Expand Up @@ -221,18 +224,6 @@ func (r *RateLimitPolicy) Validate() error {
return nil
}

func (r *RateLimitPolicy) TargetKey() client.ObjectKey {
tmpNS := r.Namespace
if r.Spec.TargetRef.Namespace != nil {
tmpNS = string(*r.Spec.TargetRef.Namespace)
}

return client.ObjectKey{
Name: string(r.Spec.TargetRef.Name),
Namespace: tmpNS,
}
}

//+kubebuilder:object:root=true

// RateLimitPolicyList contains a list of RateLimitPolicy
Expand All @@ -252,6 +243,10 @@ func (r *RateLimitPolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReferenc
return r.Spec.TargetRef
}

func (r *RateLimitPolicy) GetStatus() kuadrantgatewayapi.PolicyStatus {
return &r.Status
}

func (r *RateLimitPolicy) GetWrappedNamespace() gatewayapiv1.Namespace {
return gatewayapiv1.Namespace(r.Namespace)
}
Expand All @@ -277,6 +272,10 @@ func (r *RateLimitPolicy) Kind() string {
return r.TypeMeta.Kind
}

func (r *RateLimitPolicy) PolicyClass() kuadrantgatewayapi.PolicyClass {
return kuadrantgatewayapi.InheritedPolicy
}

func (r *RateLimitPolicy) BackReferenceAnnotationName() string {
return RateLimitPolicyBackReferenceAnnotationName
}
Expand Down
10 changes: 9 additions & 1 deletion bundle/manifests/kuadrant-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ metadata:
capabilities: Basic Install
categories: Integration & Delivery
containerImage: quay.io/kuadrant/kuadrant-operator:latest
createdAt: "2024-04-22T07:53:19Z"
createdAt: "2024-04-22T18:17:29Z"
operators.operatorframework.io/builder: operator-sdk-v1.32.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
repository: https://github.com/Kuadrant/kuadrant-operator
Expand Down Expand Up @@ -318,6 +318,14 @@ spec:
- patch
- update
- watch
- apiGroups:
- gateway.networking.k8s.io
resources:
- httproutes/status
verbs:
- get
- patch
- update
- apiGroups:
- install.istio.io
resources:
Expand Down
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,14 @@ rules:
- patch
- update
- watch
- apiGroups:
- gateway.networking.k8s.io
resources:
- httproutes/status
verbs:
- get
- patch
- update
- apiGroups:
- install.istio.io
resources:
Expand Down
2 changes: 1 addition & 1 deletion controllers/authpolicy_authconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au
hosts = utils.HostnamesToStrings(gwHostnames)

rules := make([]gatewayapiv1.HTTPRouteRule, 0)
routes := r.TargetRefReconciler.FetchAcceptedGatewayHTTPRoutes(ctx, ap.TargetKey())
routes := r.TargetRefReconciler.FetchAcceptedGatewayHTTPRoutes(ctx, obj)
for idx := range routes {
route := routes[idx]
// skip routes that have an authpolicy of its own and Gateway authpolicy does not define atomic overrides
Expand Down
16 changes: 10 additions & 6 deletions controllers/authpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package controllers
import (
"context"
"encoding/json"
"fmt"

"github.com/go-logr/logr"
authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
Expand Down Expand Up @@ -160,11 +161,11 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A
}

if err := r.reconcileIstioAuthorizationPolicies(ctx, ap, targetNetworkObject, gatewayDiffObj); err != nil {
return err
return fmt.Errorf("reconcile AuthorizationPolicy error %w", err)
}

if err := r.reconcileAuthConfigs(ctx, ap, targetNetworkObject); err != nil {
return err
return fmt.Errorf("reconcile AuthConfig error %w", err)
}

// if the AuthPolicy(ap) targets a Gateway then all policies attached to that Gateway need to be checked.
Expand All @@ -184,8 +185,7 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A
return err
}

refNetworkObject := &gatewayapiv1.HTTPRoute{}
err = r.Client().Get(ctx, ref.TargetKey(), refNetworkObject)
refNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), ref.GetTargetRef(), ref.Namespace)
if err != nil {
return err
}
Expand All @@ -198,11 +198,15 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A

// set direct back ref - i.e. claim the target network object as taken asap
if err := r.reconcileNetworkResourceDirectBackReference(ctx, ap, targetNetworkObject); err != nil {
return err
return fmt.Errorf("reconcile TargetBackReference error %w", err)
}

// set annotation of policies affecting the gateway - should be the last step, only when all the reconciliation steps succeed
return r.TargetRefReconciler.ReconcileGatewayPolicyReferences(ctx, ap, gatewayDiffObj)
if err := r.TargetRefReconciler.ReconcileGatewayPolicyReferences(ctx, ap, gatewayDiffObj); err != nil {
return fmt.Errorf("ReconcileGatewayPolicyReferences error %w", err)
}

return nil
}

func (r *AuthPolicyReconciler) deleteResources(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error {
Expand Down
Loading

0 comments on commit bed695f

Please sign in to comment.