Skip to content

Commit

Permalink
Use LocalPolicyTargetReference
Browse files Browse the repository at this point in the history
  • Loading branch information
adam-cattermole committed Jun 26, 2024
1 parent 0f687c8 commit 2472341
Show file tree
Hide file tree
Showing 47 changed files with 143 additions and 401 deletions.
20 changes: 7 additions & 13 deletions api/v1alpha1/dnspolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type DNSPolicySpec struct {
// TargetRef identifies an API object to apply policy to.
// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'",message="Invalid targetRef.group. The only supported value is 'gateway.networking.k8s.io'"
// +kubebuilder:validation:XValidation:rule="self.kind == 'Gateway'",message="Invalid targetRef.kind. The only supported values are 'Gateway'"
TargetRef gatewayapiv1alpha2.NamespacedPolicyTargetReference `json:"targetRef"`
TargetRef gatewayapiv1alpha2.LocalPolicyTargetReference `json:"targetRef"`

// +optional
HealthCheck *HealthCheckSpec `json:"healthCheck,omitempty"`
Expand Down Expand Up @@ -170,7 +170,7 @@ func (p *DNSPolicy) GetRulesHostnames() []string {
return make([]string, 0)
}

func (p *DNSPolicy) GetTargetRef() gatewayapiv1alpha2.NamespacedPolicyTargetReference {
func (p *DNSPolicy) GetTargetRef() gatewayapiv1alpha2.LocalPolicyTargetReference {

Check warning on line 173 in api/v1alpha1/dnspolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1alpha1/dnspolicy_types.go#L173

Added line #L173 was not covered by tests
return p.Spec.TargetRef
}

Expand Down Expand Up @@ -218,10 +218,6 @@ func (p *DNSPolicy) Validate() error {
return fmt.Errorf("invalid targetRef.Kind %s. The only supported kind is Gateway", p.Spec.TargetRef.Kind)
}

if p.Spec.TargetRef.Namespace != nil && string(*p.Spec.TargetRef.Namespace) != p.Namespace {
return fmt.Errorf("invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", *p.Spec.TargetRef.Namespace)
}

return nil
}

Expand Down Expand Up @@ -284,7 +280,7 @@ func NewDNSPolicy(name, ns string) *DNSPolicy {
}
}

func (p *DNSPolicy) WithTargetRef(targetRef gatewayapiv1alpha2.NamespacedPolicyTargetReference) *DNSPolicy {
func (p *DNSPolicy) WithTargetRef(targetRef gatewayapiv1alpha2.LocalPolicyTargetReference) *DNSPolicy {

Check warning on line 283 in api/v1alpha1/dnspolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1alpha1/dnspolicy_types.go#L283

Added line #L283 was not covered by tests
p.Spec.TargetRef = targetRef
return p
}
Expand All @@ -307,12 +303,10 @@ func (p *DNSPolicy) WithRoutingStrategy(strategy RoutingStrategy) *DNSPolicy {
//TargetRef

func (p *DNSPolicy) WithTargetGateway(gwName string) *DNSPolicy {
typedNamespace := gatewayapiv1.Namespace(p.GetNamespace())
return p.WithTargetRef(gatewayapiv1alpha2.NamespacedPolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: "Gateway",
Name: gatewayapiv1.ObjectName(gwName),
Namespace: &typedNamespace,
return p.WithTargetRef(gatewayapiv1alpha2.LocalPolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: "Gateway",
Name: gatewayapiv1.ObjectName(gwName),

Check warning on line 309 in api/v1alpha1/dnspolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1alpha1/dnspolicy_types.go#L306-L309

Added lines #L306 - L309 were not covered by tests
})
}

Expand Down
18 changes: 6 additions & 12 deletions api/v1alpha1/tlspolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type TLSPolicySpec struct {
// TargetRef identifies an API object to apply policy to.
// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'",message="Invalid targetRef.group. The only supported value is 'gateway.networking.k8s.io'"
// +kubebuilder:validation:XValidation:rule="self.kind == 'Gateway'",message="Invalid targetRef.kind. The only supported values are 'Gateway'"
TargetRef gatewayapiv1alpha2.NamespacedPolicyTargetReference `json:"targetRef"`
TargetRef gatewayapiv1alpha2.LocalPolicyTargetReference `json:"targetRef"`

CertificateSpec `json:",inline"`
}
Expand Down Expand Up @@ -175,7 +175,7 @@ func (p *TLSPolicy) GetRulesHostnames() []string {
return make([]string, 0)
}

func (p *TLSPolicy) GetTargetRef() gatewayapiv1alpha2.NamespacedPolicyTargetReference {
func (p *TLSPolicy) GetTargetRef() gatewayapiv1alpha2.LocalPolicyTargetReference {

Check warning on line 178 in api/v1alpha1/tlspolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1alpha1/tlspolicy_types.go#L178

Added line #L178 was not covered by tests
return p.Spec.TargetRef
}

Expand All @@ -200,10 +200,6 @@ func (p *TLSPolicy) Validate() error {
return fmt.Errorf("invalid targetRef.Kind %s. The only supported kind is Gateway", p.Spec.TargetRef.Kind)
}

if p.Spec.TargetRef.Namespace != nil && string(*p.Spec.TargetRef.Namespace) != p.Namespace {
return fmt.Errorf("invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", *p.Spec.TargetRef.Namespace)
}

return nil
}

Expand Down Expand Up @@ -243,12 +239,10 @@ func NewTLSPolicy(policyName, ns string) *TLSPolicy {
}

func (p *TLSPolicy) WithTargetGateway(gwName string) *TLSPolicy {
typedNamespace := gatewayapiv1.Namespace(p.GetNamespace())
p.Spec.TargetRef = gatewayapiv1alpha2.NamespacedPolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: "Gateway",
Name: gatewayapiv1.ObjectName(gwName),
Namespace: &typedNamespace,
p.Spec.TargetRef = gatewayapiv1alpha2.LocalPolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: "Gateway",
Name: gatewayapiv1.ObjectName(gwName),

Check warning on line 245 in api/v1alpha1/tlspolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1alpha1/tlspolicy_types.go#L242-L245

Added lines #L242 - L245 were not covered by tests
}
return p
}
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 2 additions & 11 deletions api/v1beta2/authpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package v1beta2

import (
"context"
"fmt"

"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -163,7 +162,7 @@ type AuthPolicySpec struct {
// TargetRef identifies an API object to apply policy to.
// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'",message="Invalid targetRef.group. The only supported value is 'gateway.networking.k8s.io'"
// +kubebuilder:validation:XValidation:rule="self.kind == 'HTTPRoute' || self.kind == 'Gateway'",message="Invalid targetRef.kind. The only supported values are 'HTTPRoute' and 'Gateway'"
TargetRef gatewayapiv1alpha2.NamespacedPolicyTargetReference `json:"targetRef"`
TargetRef gatewayapiv1alpha2.LocalPolicyTargetReference `json:"targetRef"`

// Defaults define explicit default values for this policy and for policies inheriting this policy.
// Defaults are mutually exclusive with implicit defaults defined by AuthPolicyCommonSpec.
Expand Down Expand Up @@ -273,15 +272,7 @@ func (ap *AuthPolicy) IsAtomicOverride() bool {
return ap.Spec.Overrides != nil
}

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

return nil
}

func (ap *AuthPolicy) GetTargetRef() gatewayapiv1alpha2.NamespacedPolicyTargetReference {
func (ap *AuthPolicy) GetTargetRef() gatewayapiv1alpha2.LocalPolicyTargetReference {

Check warning on line 275 in api/v1beta2/authpolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta2/authpolicy_types.go#L275

Added line #L275 was not covered by tests
return ap.Spec.TargetRef
}

Expand Down
69 changes: 1 addition & 68 deletions api/v1beta2/authpolicy_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"reflect"
"testing"

authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
Expand Down Expand Up @@ -71,7 +70,7 @@ func TestAuthPolicyGetRulesHostnames(t *testing.T) {
Namespace: "my-namespace",
},
Spec: AuthPolicySpec{
TargetRef: gatewayapiv1alpha2.NamespacedPolicyTargetReference{
TargetRef: gatewayapiv1alpha2.LocalPolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: "HTTPRoute",
Name: "my-route",
Expand Down Expand Up @@ -213,72 +212,6 @@ func TestAuthPolicyGetRulesHostnames(t *testing.T) {
}
}

func TestAuthPolicyValidate(t *testing.T) {
testCases := []struct {
name string
policy *AuthPolicy
valid bool
message string
}{
{
name: "invalid targetRef namespace",
policy: &AuthPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "my-policy",
Namespace: "my-namespace",
},
Spec: AuthPolicySpec{
TargetRef: gatewayapiv1alpha2.NamespacedPolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: "HTTPRoute",
Name: "my-route",
Namespace: ptr.To(gatewayapiv1.Namespace("other-namespace")),
},
AuthPolicyCommonSpec: AuthPolicyCommonSpec{
AuthScheme: &AuthSchemeSpec{
Authentication: map[string]AuthenticationSpec{
"my-rule": {
AuthenticationSpec: authorinoapi.AuthenticationSpec{
AuthenticationMethodSpec: authorinoapi.AuthenticationMethodSpec{
AnonymousAccess: &authorinoapi.AnonymousAccessSpec{},
},
},
CommonAuthRuleSpec: CommonAuthRuleSpec{
RouteSelectors: []RouteSelector{
{
Hostnames: []gatewayapiv1.Hostname{"*.foo.io"},
Matches: []gatewayapiv1.HTTPRouteMatch{
{
Path: &gatewayapiv1.HTTPPathMatch{
Value: ptr.To("/foo"),
},
},
},
},
},
},
},
},
},
},
},
},
message: "invalid targetRef.Namespace other-namespace. Currently only supporting references to the same namespace",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result := tc.policy.Validate()
if tc.valid && result != nil {
t.Errorf("Expected policy to be valid, got %t", result)
}
if !tc.valid && result == nil {
t.Error("Expected policy to be invalid, got no validation error")
}
})
}
}

func testBuildRouteSelector() RouteSelector {
return RouteSelector{
Hostnames: []gatewayapiv1.Hostname{"toystore.kuadrant.io"},
Expand Down
13 changes: 2 additions & 11 deletions api/v1beta2/ratelimitpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1beta2

import (
"context"
"fmt"

"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -130,7 +129,7 @@ type RateLimitPolicySpec struct {
// TargetRef identifies an API object to apply policy to.
// +kubebuilder:validation:XValidation:rule="self.group == 'gateway.networking.k8s.io'",message="Invalid targetRef.group. The only supported value is 'gateway.networking.k8s.io'"
// +kubebuilder:validation:XValidation:rule="self.kind == 'HTTPRoute' || self.kind == 'Gateway'",message="Invalid targetRef.kind. The only supported values are 'HTTPRoute' and 'Gateway'"
TargetRef gatewayapiv1alpha2.NamespacedPolicyTargetReference `json:"targetRef"`
TargetRef gatewayapiv1alpha2.LocalPolicyTargetReference `json:"targetRef"`

// Defaults define explicit default values for this policy and for policies inheriting this policy.
// Defaults are mutually exclusive with implicit defaults defined by RateLimitPolicyCommonSpec.
Expand Down Expand Up @@ -218,14 +217,6 @@ type RateLimitPolicy struct {

var _ kuadrantgatewayapi.Policy = &RateLimitPolicy{}

func (r *RateLimitPolicy) Validate() error {
if r.Spec.TargetRef.Namespace != nil && string(*r.Spec.TargetRef.Namespace) != r.Namespace {
return fmt.Errorf("invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", *r.Spec.TargetRef.Namespace)
}

return nil
}

//+kubebuilder:object:root=true

// RateLimitPolicyList contains a list of RateLimitPolicy
Expand All @@ -241,7 +232,7 @@ func (l *RateLimitPolicyList) GetItems() []kuadrant.Policy {
})
}

func (r *RateLimitPolicy) GetTargetRef() gatewayapiv1alpha2.NamespacedPolicyTargetReference {
func (r *RateLimitPolicy) GetTargetRef() gatewayapiv1alpha2.LocalPolicyTargetReference {

Check warning on line 235 in api/v1beta2/ratelimitpolicy_types.go

View check run for this annotation

Codecov / codecov/patch

api/v1beta2/ratelimitpolicy_types.go#L235

Added line #L235 was not covered by tests
return r.Spec.TargetRef
}

Expand Down
23 changes: 1 addition & 22 deletions api/v1beta2/ratelimitpolicy_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
package v1beta2

import (
"strings"
"testing"

"gotest.tools/assert"
Expand All @@ -25,7 +24,7 @@ func testBuildBasicRLP(name string, kind gatewayapiv1.Kind, mutateFn func(*RateL
Namespace: "testNS",
},
Spec: RateLimitPolicySpec{
TargetRef: gatewayapiv1alpha2.NamespacedPolicyTargetReference{
TargetRef: gatewayapiv1alpha2.LocalPolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: kind,
Name: "some-name",
Expand All @@ -44,26 +43,6 @@ func testBuildBasicHTTPRouteRLP(name string, mutateFn func(*RateLimitPolicy)) *R
return testBuildBasicRLP(name, "HTTPRoute", mutateFn)
}

// TestRateLimitPolicyValidation calls rlp.Validate()
// for a valid return value.
func TestRateLimitPolicyValidation(t *testing.T) {
name := "httproute-a"

t.Run("Invalid - Different namespace", func(subT *testing.T) {
rlp := testBuildBasicHTTPRouteRLP(name, func(policy *RateLimitPolicy) {
otherNS := gatewayapiv1.Namespace(policy.GetNamespace() + "other")
policy.Spec.TargetRef.Namespace = &otherNS
})
err := rlp.Validate()
if err == nil {
subT.Fatal(`rlp.Validate() did not return error and should`)
}
if !strings.Contains(err.Error(), "invalid targetRef.Namespace") {
subT.Fatalf(`rlp.Validate() did not return expected error. Instead: %v`, err)
}
})
}

func TestRateLimitPolicyListGetItems(t *testing.T) {
list := &RateLimitPolicyList{}
if len(list.GetItems()) != 0 {
Expand Down
4 changes: 2 additions & 2 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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-06-11T12:12:14Z"
createdAt: "2024-06-21T10:39: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
10 changes: 0 additions & 10 deletions bundle/manifests/kuadrant.io_authpolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12929,16 +12929,6 @@ spec:
maxLength: 253
minLength: 1
type: string
namespace:
description: |-
Namespace is the namespace of the referent. When unspecified, the local
namespace is inferred. Even when policy targets a resource in a different
namespace, it MUST only apply to traffic originating from the same
namespace as the policy.
maxLength: 63
minLength: 1
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
type: string
required:
- group
- kind
Expand Down
10 changes: 0 additions & 10 deletions bundle/manifests/kuadrant.io_dnspolicies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -230,16 +230,6 @@ spec:
maxLength: 253
minLength: 1
type: string
namespace:
description: |-
Namespace is the namespace of the referent. When unspecified, the local
namespace is inferred. Even when policy targets a resource in a different
namespace, it MUST only apply to traffic originating from the same
namespace as the policy.
maxLength: 63
minLength: 1
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
type: string
required:
- group
- kind
Expand Down
Loading

0 comments on commit 2472341

Please sign in to comment.