Skip to content

Aligns API Fields with K8s Conventions #538

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

Merged
merged 1 commit into from
Feb 11, 2021
Merged

Aligns API Fields with K8s Conventions #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 16 additions & 6 deletions apis/v1alpha1/backendpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@ type BackendPolicy struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec BackendPolicySpec `json:"spec,omitempty"`
// Spec defines the desired state of BackendPolicy.
Spec BackendPolicySpec `json:"spec,omitempty"`

// Status defines the current state of BackendPolicy.
Status BackendPolicyStatus `json:"status,omitempty"`
}

// +kubebuilder:object:root=true

// BackendPolicyList contains a list of BackendPolicy
// BackendPolicyList contains a list of BackendPolicy.
type BackendPolicyList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Expand All @@ -55,33 +58,39 @@ type BackendPolicySpec struct {
// the oldest BackendPolicy.
//
// Support: Core
//
// +kubebuilder:validation:MaxItems=16
BackendRefs []BackendRef `json:"backendRefs"`

// TLS is the TLS configuration for these backends.
//
// Support: Extended
//
// +optional
TLS *BackendTLSConfig `json:"tls,omitempty"`
}

// BackendRef identifies an API object within a known namespace that defaults
// group to core and resource to services if unspecified.
// BackendRef identifies an API object within the same namespace
// as the BackendPolicy.
type BackendRef struct {
// Group is the group of the referent.
//
// +kubebuilder:validation:MaxLength=253
Group string `json:"group"`

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

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

// Port is the port of the referent. If unspecified, this policy applies to
// all ports on the backend.
//
// +optional
Port *PortNumber `json:"port,omitempty"`
}
Expand All @@ -108,12 +117,13 @@ type BackendTLSConfig struct {
// provider.
//
// Support: Implementation-specific.
//
// +optional
Options map[string]string `json:"options,omitempty"`
}

// BackendPolicyStatus defines the observed state of BackendPolicy. Conditions
// that are related to a specific Route or Gateway should be placed on the
// that are related to a specific Route or Gateway must be placed on the
// Route(s) using backends configured by this BackendPolicy.
type BackendPolicyStatus struct {
// Conditions describe the current conditions of the BackendPolicy.
Expand Down
31 changes: 23 additions & 8 deletions apis/v1alpha1/gateway_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,18 @@ type Gateway struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// Spec defines the desired state of Gateway.
Spec GatewaySpec `json:"spec,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be the other way around where we remove the omitempty tag. An object without a spec is going to confuse users and even maintainers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we OK diverging from how upstream defines spec/status/metadata optionality?

xref: https://github.com/kubernetes/api/blob/master/core/v1/types.go

Copy link
Member

Choose a reason for hiding this comment

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

As strange as it is, I think it likely is appropriate to follow what upstream is doing here.


// Status defines the current state of Gateway.
//
// +kubebuilder:default={conditions: {{type: "Scheduled", status: "False", reason:"NotReconciled", message:"Waiting for controller", lastTransitionTime: "1970-01-01T00:00:00Z"}}}
Status GatewayStatus `json:"status,omitempty"`
}

// +kubebuilder:object:root=true

// GatewayList contains a list of Gateway
// GatewayList contains a list of Gateway.
type GatewayList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Expand All @@ -61,6 +64,7 @@ type GatewayList struct {
type GatewaySpec struct {
// GatewayClassName used for this Gateway. This is the name of a
// GatewayClass resource.
//
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
GatewayClassName string `json:"gatewayClassName"`
Expand Down Expand Up @@ -293,8 +297,9 @@ type TLSOverridePolicy struct {
//
// Support: Core
//
// +optional
// +kubebuilder:default=Deny
Certificate TLSRouteOverrideType `json:"certificate"`
Certificate TLSRouteOverrideType `json:"certificate,omitempty"`
Comment on lines +300 to +302
Copy link
Contributor Author

Choose a reason for hiding this comment

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

+optional/omitempty was added since this field is being defaulted.

}

// GatewayTLSConfig describes a TLS configuration.
Expand All @@ -319,6 +324,7 @@ type GatewayTLSConfig struct {
//
// Support: Core
//
// +optional
// +kubebuilder:default=Terminate
Mode TLSModeType `json:"mode,omitempty"`

Expand All @@ -338,7 +344,7 @@ type GatewayTLSConfig struct {
// Support: Implementation-specific (Other resource types)
//
// +optional
CertificateRef LocalObjectReference `json:"certificateRef,omitempty"`
CertificateRef *LocalObjectReference `json:"certificateRef,omitempty"`

// RouteOverride dictates if TLS settings can be configured
// via Routes or not.
Expand All @@ -349,6 +355,7 @@ type GatewayTLSConfig struct {
//
// Support: Core
//
// +optional
// +kubebuilder:default={certificate:Deny}
RouteOverride TLSOverridePolicy `json:"routeOverride,omitempty"`

Expand All @@ -363,7 +370,7 @@ type GatewayTLSConfig struct {
// Support: Implementation-specific.
//
// +optional
Options map[string]string `json:"options"`
Options map[string]string `json:"options,omitempty"`
}

// TLSModeType type defines behavior of gateway with TLS protocol.
Expand Down Expand Up @@ -391,8 +398,10 @@ type RouteBindingSelector struct {
// default.
//
// Support: Core
// +kubebuilder:default={from: "Same"}
Namespaces *RouteNamespaces `json:"namespaces,omitempty"`
//
// +optional
// +kubebuilder:default={from: Same}
Namespaces RouteNamespaces `json:"namespaces,omitempty"`
// Selector specifies a set of route labels used for selecting
// routes to associate with the Gateway. If this Selector is defined,
// only routes matching the Selector are associated with the Gateway.
Expand All @@ -418,6 +427,7 @@ type RouteBindingSelector struct {
//
// Support: Core
//
// +optional
// +kubebuilder:default=networking.x-k8s.io
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
Expand All @@ -436,8 +446,8 @@ type RouteBindingSelector struct {
}

// RouteSelectType specifies where Routes should be selected by a Gateway.
//
// +kubebuilder:validation:Enum=All;Selector;Same
// +kubebuilder:default=Same
type RouteSelectType string

const (
Expand All @@ -462,6 +472,9 @@ type RouteNamespaces struct {
// * Same: Only Routes in the same namespace may be used by this Gateway.
//
// Support: Core
//
// +optional
// +kubebuilder:default=Same
From RouteSelectType `json:"from,omitempty"`

// Selector must be specified when From is set to "Selector". In that case,
Expand All @@ -480,6 +493,7 @@ type GatewayAddress struct {
//
// Support: Extended
//
// +optional
// +kubebuilder:default=IPAddress
Type AddressType `json:"type,omitempty"`

Expand Down Expand Up @@ -540,7 +554,7 @@ type GatewayStatus struct {
//
// +optional
// +kubebuilder:validation:MaxItems=16
Addresses []GatewayAddress `json:"addresses"`
Addresses []GatewayAddress `json:"addresses,omitempty"`

// Conditions describe the current conditions of the Gateway.
//
Expand All @@ -554,6 +568,7 @@ type GatewayStatus struct {
// * "Scheduled"
// * "Ready"
//
// +optional
// +listType=map
// +listMapKey=type
// +kubebuilder:validation:MaxItems=8
Expand Down
9 changes: 5 additions & 4 deletions apis/v1alpha1/gatewayclass_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ import (
// for creating Gateway resources.
//
// GatewayClass is a Cluster level resource.
//
// Support: Core.
Comment on lines -34 to -35
Copy link
Contributor Author

@danehans danehans Feb 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Fields should indicate the support-level and not the top-level resource.

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 there may be a place for this, especially for route types, but I'm not exactly sure what that should be. I think we want a world where both L4 and L7 implementations can support different parts of this API. I don't mind removing this here though since it doesn't seem like we've added it for other resources.

type GatewayClass struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// Spec for this GatewayClass.
// Spec defines the desired state of GatewayClass.
Spec GatewayClassSpec `json:"spec,omitempty"`
// Status of the GatewayClass.

// Status defines the current state of GatewayClass.
//
// +kubebuilder:default={conditions: {{type: "Admitted", status: "False", message: "Waiting for controller", reason: "Waiting", lastTransitionTime: "1970-01-01T00:00:00Z"}}}
Status GatewayClassStatus `json:"status,omitempty"`
}
Expand Down Expand Up @@ -113,6 +113,7 @@ type GatewayClassStatus struct {
// Conditions is the current status from the controller for
// this GatewayClass.
//
// +optional
// +listType=map
// +listMapKey=type
// +kubebuilder:validation:MaxItems=8
Expand Down
Loading