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

Implements GEP 709 - ReferencePolicy #741

Merged
merged 7 commits into from
Aug 9, 2021
Merged

Implements GEP 709 - ReferencePolicy #741

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3362e3d
Implement GEP-709, ReferencePolicy
Jul 29, 2021
0aee0fc
Complete documentation and add Gateway status info
Aug 2, 2021
50a148b
Update after PR review
Aug 6, 2021
0ad53af
Refactor after GEP-718 changes
Aug 9, 2021
e4bfd3a
Fix last round of PR comments
Aug 9, 2021
4038057
Fix another round of PR comments
Aug 9, 2021
677a3c0
Add admonitions to mkdocs and note to cross-namespace-references.md
Aug 9, 2021
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
7 changes: 7 additions & 0 deletions apis/v1alpha2/gateway_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,7 @@ const (
// * "DegradedRoutes"
// * "InvalidCertificateRef"
// * "InvalidRoutesRef"
// * "RefNotPermitted"
//
// Controllers may raise this condition with other reasons,
// but should prefer to use the reasons listed above to improve
Expand Down Expand Up @@ -839,6 +840,12 @@ const (
// not resolve any Routes, and the "ResolvedRefs" status condition
// should not be raised in that case.
ListenerReasonInvalidRoutesRef ListenerConditionReason = "InvalidRoutesRef"

// This reason is used with the "ResolvedRefs" condition when
// one of the Listener's Routes has a BackendRef to an object in
// another namespace, where the object in the other namespace does
// not have a ReferencePolicy explicitly allowing the reference.
ListenerReasonRefNotPermitted ListenerConditionReason = "RefNotPermitted"
youngnick marked this conversation as resolved.
Show resolved Hide resolved
)

const (
Expand Down
43 changes: 33 additions & 10 deletions apis/v1alpha2/httproute_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,10 +723,20 @@ type HTTPRequestRedirect struct {
type HTTPRequestMirrorFilter struct {
// BackendRef references a resource where mirrored requests are sent.
//
// If the referent cannot be found, the rule is not included in the route.
// The controller should raise the "ResolvedRefs" condition on the Gateway
// with the "DegradedRoutes" reason. The gateway status for this route should
// be updated with a condition that describes the error more specifically.
// If the referent cannot be found, this HTTPBackendRef is invalid
// and must be dropped from the Gateway. The controller must ensure the
// "ResolvedRefs" condition on the Gateway is set to `status: true`
// with the "DegradedRoutes" reason, and not configure this backend in the
// underlying implemenation.
//
// If there is a cross-namespace reference to an *existing* object
// that is not covered by a ReferencePolicy, the controller must ensure the
youngnick marked this conversation as resolved.
Show resolved Hide resolved
// "ResolvedRefs" condition on the Gateway is set to `status: true`,
// with the "RefNotPermitted" reason and not configure this backend in the
// underlying implementation.
//
// In either error case, the Message of the `ResolvedRefs` Condition
// should be used to provide more detail about the problem.
//
// Support: Extended for Kubernetes Service
// Support: Custom for any other resource
Expand All @@ -737,13 +747,26 @@ type HTTPRequestMirrorFilter struct {

// HTTPBackendRef defines how a HTTPRoute should forward an HTTP request.
type HTTPBackendRef struct {
// BackendRef defines how a Route should forward a request to a Kubernetes
// resource.
// BackendRef is a reference to a backend to forward matched requests to.
//
// If the referent cannot be found, this HTTPBackendRef is invalid
// and must be dropped from the Gateway. The controller must ensure the
// "ResolvedRefs" condition on the Gateway is set to `status: true`
// with the "DegradedRoutes" reason, and not configure this backend in the
// underlying implemenation.
//
// If there is a cross-namespace reference to an *existing* object
// that is not covered by a ReferencePolicy, the controller must ensure the
// "ResolvedRefs" condition on the Gateway is set to `status: true`,
// with the "RefNotPermitted" reason and not configure this backend in the
// underlying implementation.
//
// If the referent cannot be found, the rule is not included in the route.
// The controller should raise the "ResolvedRefs" condition on the Gateway
// with the "DegradedRoutes" reason. The gateway status for this route should
// be updated with a condition that describes the error more specifically.
// In either error case, the Message of the `ResolvedRefs` Condition
// should be used to provide more detail about the problem.
//
// Support: Custom
//
// +optional
BackendRef `json:",inline"`

// Filters defined at this-level should be executed if and only if the
Expand Down
38 changes: 0 additions & 38 deletions apis/v1alpha2/local_object_reference_types.go
View file
Open in desktop

This file was deleted.

130 changes: 130 additions & 0 deletions apis/v1alpha2/object_reference_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
/*
Copyright 2020 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha2

// LocalObjectReference identifies an API object within the namespace of the
// referrer.
type LocalObjectReference struct {
// Group is the group of the referent.
//
// +kubebuilder:validation:MaxLength=253
Group string `json:"group"`

// Kind is kind of the referent.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Kind string `json:"kind"`

// Name is the name of the referent.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Name string `json:"name"`
}

// ObjectReference identifies an API object including its namespace.
type ObjectReference struct {
// Group is the group of the referent.
youngnick marked this conversation as resolved.
Show resolved Hide resolved
// When unspecified (empty string), core API group is inferred.
//
// +optional
// +kubebuilder:default=""
// +kubebuilder:validation:MaxLength=253
Group *string `json:"group"`

// Kind is kind of the referent.
//
// +optional
// +kubebuilder:default=Service
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Kind *string `json:"kind"`

// Name is the name of the referent.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Name string `json:"name"`

// Namespace is the namespace of the backend. When unspecified, the local
hbagdi marked this conversation as resolved.
Show resolved Hide resolved
// namespace is inferred.
//
// Note that when a namespace is specified, a ReferencePolicy object
// is required in the referent namespace to allow that namespace's
// owner to accept the reference. See the ReferencePolicy object for details.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// owner to accept the reference. See the ReferencePolicy object for details.
// owner to accept the reference. See the ReferencePolicy documentation for details.

//
// Support: Core
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +optional
Namespace *string `json:"namespace,omitempty"`
}

// BackendObjectReference defines how an ObjectReference that is
// specific to BackendRef. It includes a few additional fields and features
// than a regular ObjectReference.
//
// Note that when a namespace is specified, a ReferencePolicy object
// is required in the referent namespace to allow that namespace's
// owner to accept the reference. See the ReferencePolicy object for details.
type BackendObjectReference struct {
// Group is the group of the referent.
// When unspecified (empty string), core API group is inferred.
//
// +optional
// +kubebuilder:default=""
// +kubebuilder:validation:MaxLength=253
Group *string `json:"group"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Group *string `json:"group"`
Group *string `json:"group,omitempty"`


// Kind is kind of the referent.
//
// +optional
// +kubebuilder:default=Service
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Kind *string `json:"kind"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Kind *string `json:"kind"`
Kind *string `json:"kind,omitempty"`


// Name is the name of the referent.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Name string `json:"name"`

// Namespace is the namespace of the backend. When unspecified, the local
// namespace is inferred.
//
// Note that when a namespace is specified, a ReferencePolicy object
// is required in the referent namespace to allow that namespace's
// owner to accept the reference. See the ReferencePolicy object for details.
//
// Support: Core
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
// +optional
Namespace *string `json:"namespace,omitempty"`

// Port specifies the destination port number to use for this resource.
// Port is required when the referent is a Kubernetes Service.
// For other resources, destination port can be derived from the referent
// resource or this field.
//
// +optional
Port *PortNumber `json:"port,omitempty"`
}
138 changes: 138 additions & 0 deletions apis/v1alpha2/referencepolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*
Copyright 2021 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha2

import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

// +genclient
// +kubebuilder:object:root=true
// +kubebuilder:resource:categories=gateway-api,shortName=refpol
// +kubebuilder:storageversion
// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
Copy link
Contributor

Choose a reason for hiding this comment

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

We can revisit this, but the age doesn't seem particularly useful?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's a pretty standard column for k8s resources. I think it could be helpful to know when a resource was created.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, should be on all resources.

hbagdi marked this conversation as resolved.
Show resolved Hide resolved

// ReferencePolicy identifies kinds of resources in other namespaces that are
// trusted to reference the specified kinds of resources in the same namespace
// as the policy.
//
// Each ReferencePolicy can be used to represent a unique trust relationship.
// Additional Reference Policies can be used to add to the set of trusted
// sources of inbound references for the namespace they are defined within.
youngnick marked this conversation as resolved.
Show resolved Hide resolved
//
// All cross-namespace references in Gateway API (with the exception of cross-namespace
// Gateway-route attachment) require a ReferencePolicy.
youngnick marked this conversation as resolved.
Show resolved Hide resolved
//
// Support: Core
//
type ReferencePolicy struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// Spec defines the desired state of ReferencePolicy.
Spec ReferencePolicySpec `json:"spec,omitempty"`

// Note that `Status` sub-resource has been excluded at the
// moment as it was difficult to work out the design.
// `Status` sub-resource may be added in future.
}

// +kubebuilder:object:root=true
// ReferencePolicyList contains a list of ReferencePolicy.
type ReferencePolicyList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []ReferencePolicy `json:"items"`
}

// ReferencePolicySpec identifies a cross namespace relationship that is trusted
// for Gateway API.
type ReferencePolicySpec struct {
// From describes the trusted namespaces and kinds that can reference the
// resources described in "To". Each entry in this list must be considered
// to be an additional place that references can be valid from, or to put
// this another way, entries must be combined using OR.
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I personally got more confused. Feel free to reject.

Suggested change
// to be an additional place that references can be valid from, or to put
// this another way, entries must be combined using OR.
// to be an additional place that references can be valid from.

Copy link
Member

Choose a reason for hiding this comment

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

I've gone back and forth on this, agree it's a bit confusing, but think the specific "combined using OR" may end up being helpful. Either way, we can clean this up later if it ends up being a point of confusion.

//
// Support: Core
//
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=16
From []ReferencePolicyFrom `json:"from"`
youngnick marked this conversation as resolved.
Show resolved Hide resolved

// To describes the resources that may be referenced by the resources
// described in "From". Each entry in this list must be considered to be an
// additional place that references can be valid to, or to put this another
// way, entries must be combined using OR.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment.

//
// Support: Core
//
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=16
To []ReferencePolicyTo `json:"to"`
}

// ReferencePolicyFrom describes trusted namespaces and kinds.
type ReferencePolicyFrom struct {
// Group is the group of the referent.
// When empty, the "core" API group is inferred.
//
// Support: Core
//
// +kubebuilder:validation:MaxLength=253
Group string `json:"group"`

// Kind is the kind of the referent. Although implementations may support
// additional resources, the following Route types are part of the "Core"
// support level for this field:
//
// * HTTPRoute
// * TCPRoute
// * TLSRoute
// * UDPRoute
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Kind string `json:"kind"`

// Namespace is the namespace of the referent.
//
// Support: Core
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Namespace string `json:"namespace,omitempty"`
}

// ReferencePolicyTo describes what Kinds are allowed as targets of the
// references.
type ReferencePolicyTo struct {
// Group is the group of the referent.
// When empty, the "core" API group is inferred.
//
// Support: Core
//
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Please mention empty group implies core.
  2. Any meaningful defaulting we can provide here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Agreed, done.
  2. I think that the extra explicitness we get from not having a default is better UX. Happy to discuss more though.

Copy link
Member

Choose a reason for hiding this comment

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

Agree that we should avoid defaulting here.

// +kubebuilder:validation:MaxLength=253
Group string `json:"group"`

// Kind is the kind of the referent. Although implementations may support
// additional resources, the following types are part of the "Core"
// support level for this field:
//
// * Service
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Kind string `json:"kind"`
}
Loading