Skip to content

Commit

Permalink
Final changes for RC2 based on sig-net review feedback
Browse files Browse the repository at this point in the history
There are three changes here:

1. The "Prefix" path match type has been renamed "PathPrefix" to leave
room for a future "StringPrefix" match.

2. The "ClassName" field in PolicyTargetReference has been removed as it
is not clearly necessary at this point.

3. A new optional "Name" field has been added to ReferencePolicyTo,
enabling more fine grained control over which resource(s) may be
referenced.
  • Loading branch information
robscott committed Oct 11, 2021
1 parent 382aba4 commit 2b0cc91
Show file tree
Hide file tree
Showing 14 changed files with 57 additions and 54 deletions.
21 changes: 12 additions & 9 deletions apis/v1alpha2/httproute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ type HTTPRouteSpec struct {
//
// +optional
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:default={{matches: {{path: {type: "Prefix", value: "/"}}}}}
// +kubebuilder:default={{matches: {{path: {type: "PathPrefix", value: "/"}}}}}
Rules []HTTPRouteRule `json:"rules,omitempty"`
}

Expand Down Expand Up @@ -160,7 +160,7 @@ type HTTPRouteRule struct {
//
// +optional
// +kubebuilder:validation:MaxItems=8
// +kubebuilder:default={{path:{ type: "Prefix", value: "/"}}}
// +kubebuilder:default={{path:{ type: "PathPrefix", value: "/"}}}
Matches []HTTPRouteMatch `json:"matches,omitempty"`

// Filters define the filters that are applied to requests that match
Expand Down Expand Up @@ -209,15 +209,15 @@ type HTTPRouteRule struct {
// Valid PathMatchType values are:
//
// * "Exact"
// * "Prefix"
// * "PathPrefix"
// * "RegularExpression"
//
// Prefix and Exact paths must be syntactically valid:
// PathPrefix and Exact paths must be syntactically valid:
//
// - Must begin with the `/` character
// - Must not contain consecutive `/` characters (e.g. `/foo///`, `//`).
//
// +kubebuilder:validation:Enum=Exact;Prefix;RegularExpression
// +kubebuilder:validation:Enum=Exact;PathPrefix;RegularExpression
type PathMatchType string

const (
Expand All @@ -232,7 +232,10 @@ const (
//
// For example, `/abc`, `/abc/` and `/abc/def` match the prefix
// `/abc`, but `/abcd` does not.
PathMatchPrefix PathMatchType = "Prefix"
//
// "PathPrefix" is semantically equivalent to the "Prefix" path type in the
// Kubernetes Ingress API.
PathMatchPathPrefix PathMatchType = "PathPrefix"

// Matches if the URL path matches the given regular expression with
// case sensitivity.
Expand All @@ -248,12 +251,12 @@ const (
type HTTPPathMatch struct {
// Type specifies how to match against the path Value.
//
// Support: Core (Exact, Prefix)
// Support: Core (Exact, PathPrefix)
//
// Support: Custom (RegularExpression)
//
// +optional
// +kubebuilder:default=Prefix
// +kubebuilder:default=PathPrefix
Type *PathMatchType `json:"type,omitempty"`

// Value of the HTTP path to match against.
Expand Down Expand Up @@ -426,7 +429,7 @@ type HTTPRouteMatch struct {
// specified, a default prefix match on the "/" path is provided.
//
// +optional
// +kubebuilder:default={type: "Prefix", value: "/"}
// +kubebuilder:default={type: "PathPrefix", value: "/"}
Path *HTTPPathMatch `json:"path,omitempty"`

// Headers specifies HTTP request header matchers. Multiple match values are
Expand Down
8 changes: 0 additions & 8 deletions apis/v1alpha2/policy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,4 @@ type PolicyTargetReference struct {
//
// +optional
Namespace *Namespace `json:"namespace,omitempty"`

// ClassName is the name of the class this policy should apply to. When
// unspecified, the policy will apply to all classes that support it.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +optional
ClassName *string `json:"className,omitempty"`
}
7 changes: 7 additions & 0 deletions apis/v1alpha2/referencepolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,11 @@ type ReferencePolicyTo struct {
//
// * Service
Kind Kind `json:"kind"`

// Name is the name of the referent. When unspecified or empty, this policy
// refers to all resources of the specified Group and Kind in the local
// namespace.
//
// +optional
Name *ObjectName `json:"name,omitempty"`
}
4 changes: 2 additions & 2 deletions apis/v1alpha2/validation/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func validateHTTPPathMatch(path *gatewayv1a2.HTTPPathMatch, fldPath *field.Path)
}

switch *path.Type {
case gatewayv1a2.PathMatchExact, gatewayv1a2.PathMatchPrefix:
case gatewayv1a2.PathMatchExact, gatewayv1a2.PathMatchPathPrefix:
if !strings.HasPrefix(*path.Value, "/") {
allErrs = append(allErrs, field.Invalid(fldPath.Child("path"), path, "must be an absolute path"))
}
Expand All @@ -132,7 +132,7 @@ func validateHTTPPathMatch(path *gatewayv1a2.HTTPPathMatch, fldPath *field.Path)
}
case gatewayv1a2.PathMatchRegularExpression:
default:
pathTypes := []string{string(gatewayv1a2.PathMatchExact), string(gatewayv1a2.PathMatchPrefix), string(gatewayv1a2.PathMatchRegularExpression)}
pathTypes := []string{string(gatewayv1a2.PathMatchExact), string(gatewayv1a2.PathMatchPathPrefix), string(gatewayv1a2.PathMatchRegularExpression)}
allErrs = append(allErrs, field.NotSupported(fldPath.Child("pathType"), *path.Type, pathTypes))
}
return allErrs
Expand Down
16 changes: 8 additions & 8 deletions apis/v1alpha2/validation/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pkgutils.PathMatchTypePtr("Prefix"),
Type: pkgutils.PathMatchTypePtr("PathPrefix"),
Value: utilpointer.String("/"),
},
},
Expand All @@ -69,7 +69,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pkgutils.PathMatchTypePtr("Prefix"),
Type: pkgutils.PathMatchTypePtr("PathPrefix"),
Value: utilpointer.String("/"),
},
},
Expand All @@ -96,7 +96,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pkgutils.PathMatchTypePtr("Prefix"),
Type: pkgutils.PathMatchTypePtr("PathPrefix"),
Value: utilpointer.String("/"),
},
},
Expand Down Expand Up @@ -132,7 +132,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pkgutils.PathMatchTypePtr("Prefix"),
Type: pkgutils.PathMatchTypePtr("PathPrefix"),
Value: utilpointer.String("/"),
},
},
Expand Down Expand Up @@ -181,7 +181,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pkgutils.PathMatchTypePtr("Prefix"),
Type: pkgutils.PathMatchTypePtr("PathPrefix"),
Value: utilpointer.String("/"),
},
},
Expand Down Expand Up @@ -248,7 +248,7 @@ func TestValidateHTTPRoute(t *testing.T) {
Matches: []gatewayv1a2.HTTPRouteMatch{
{
Path: &gatewayv1a2.HTTPPathMatch{
Type: pkgutils.PathMatchTypePtr("Prefix"),
Type: pkgutils.PathMatchTypePtr("PathPrefix"),
Value: utilpointer.String("/"),
},
},
Expand Down Expand Up @@ -400,7 +400,7 @@ func TestValidateHTTPPathMatch(t *testing.T) {
{
name: "invalid httpRoute prefix",
path: &gatewayv1a2.HTTPPathMatch{
Type: pkgutils.PathMatchTypePtr("Prefix"),
Type: pkgutils.PathMatchTypePtr("PathPrefix"),
Value: utilpointer.String("/."),
},
errCount: 1,
Expand All @@ -416,7 +416,7 @@ func TestValidateHTTPPathMatch(t *testing.T) {
{
name: "invalid httpRoute prefix",
path: &gatewayv1a2.HTTPPathMatch{
Type: pkgutils.PathMatchTypePtr("Prefix"),
Type: pkgutils.PathMatchTypePtr("PathPrefix"),
Value: utilpointer.String("/"),
},
errCount: 0,
Expand Down
14 changes: 8 additions & 6 deletions apis/v1alpha2/zz_generated.deepcopy.go

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

12 changes: 6 additions & 6 deletions config/crd/v1alpha2/gateway.networking.k8s.io_httproutes.yaml

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

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

4 changes: 2 additions & 2 deletions examples/v1alpha2/basic-http.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ spec:
rules:
- matches:
- path:
type: Prefix
type: PathPrefix
value: /bar
backendRefs:
- name: my-service1
Expand All @@ -47,7 +47,7 @@ spec:
name: great
value: example
path:
type: Prefix
type: PathPrefix
value: /some/thing
method: GET
backendRefs:
Expand Down
2 changes: 1 addition & 1 deletion examples/v1alpha2/default-match-http.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ spec:
port: 80
---
# This HTTPRoute demonstrates patch match defaulting. If no path match is
# specified, CRD defaults adds a default prefix match on the path "/". This
# specified, CRD defaults adds a default PathPrefix match on the path "/". This
# matches every HTTP request and ensures that route rules always have at
# least one valid match.
apiVersion: gateway.networking.k8s.io/v1alpha2
Expand Down
2 changes: 1 addition & 1 deletion examples/v1alpha2/http-redirect.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ spec:
rules:
- matches:
- path:
type: Prefix
type: PathPrefix
value: /
backendRefs:
- name: my-filter-svc1
Expand Down
2 changes: 1 addition & 1 deletion examples/v1alpha2/http-routing/foo-httproute.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ spec:
rules:
- matches:
- path:
type: Prefix
type: PathPrefix
value: /login
backendRefs:
- name: foo-svc
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ func Test_PathMatchTypePtr(t *testing.T) {

{
name: "valid path prefix match",
pathType: "Prefix",
expectedPath: gatewayv1a2.PathMatchPrefix,
pathType: "PathPrefix",
expectedPath: gatewayv1a2.PathMatchPathPrefix,
},
{
name: "valid path regular expression match",
Expand Down
8 changes: 0 additions & 8 deletions site-src/geps/gep-713.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,6 @@ type PolicyTargetReference struct {
// +kubebuilder:validation:MaxLength=253
// +optional
Namespace string `json:"namespace,omitempty"`

// ClassName is the name of the class this policy should apply to. When
// unspecified, the policy will apply to all classes that support it.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +optional
ClassName string `json:"className,omitempty"`
}
```

Expand Down

0 comments on commit 2b0cc91

Please sign in to comment.