Skip to content

Update ports to protocols #326

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update ports to protocols #326

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
77 changes: 39 additions & 38 deletions apis/v1alpha2/clusternetworkpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,16 +202,15 @@ type ClusterNetworkPolicyIngressRule struct {
// +kubebuilder:validation:MaxItems=25
From []ClusterNetworkPolicyIngressPeer `json:"from"`

// Ports allows for matching traffic based on port and protocols.
// This field is a list of ports which should be matched on
// the pods selected for this policy i.e the subject of the policy.
// So it matches on the destination port for the ingress traffic.
// If Ports is not set then the rule does not filter traffic via port.
// Protocols this rule matches. This rule matches if any of
// the elements in the list match the incoming traffic.
//
// This field must contain at least one item.
//
// +optional
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=25
Ports *[]ClusterNetworkPolicyPort `json:"ports,omitempty"`
// +kubebuilder:validation:MaxItems=100
Protocols *[]ClusterNetworkPolicyProtocol `json:"protocols,omitempty"`
}

// ClusterNetworkPolicyEgressRule describes an action to take on a particular
Expand Down Expand Up @@ -316,29 +315,49 @@ type ClusterNetworkPolicyIngressPeer struct {
Pods *NamespacedPod `json:"pods,omitempty"`
}

// ClusterNetworkPolicyPort describes how to select destination network ports.
// Exactly one field must be set.
// ClusterNetworkPolicyProtocol describes how to select traffic by
// protocol-specific attributes.
//
// +kubebuilder:validation:XValidation:rule="!(self.protocol in ['TCP', 'UDP', 'SCTP']) || has(self.port)",message="port must be specified for protocols that support ports"
type ClusterNetworkPolicyProtocol struct {
// Protocol is the network protocol (TCP, UDP, or SCTP) which
// traffic must match. If not specified, this field defaults
// to TCP.
//
// +kubebuilder:default=TCP
Protocol corev1.Protocol `json:"protocol,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm... this preserves the existing "bug" where you can specify

protocols:
  - protocol: TCP
    port:
      name: foo

where the "TCP" is either redundant or incorrect. Named ports should not have an explicitly-specified protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think (!(has(self.Port) && has(self.Port.Name)) || self.Protocol == '') would cover this case?

There is a slight problem in that we can only generate this validation for experimental fields.


// Specific port to match against.
//
// +optional
Port *ClusterNetworkPolicyPort `json:"port,omitempty"`
Comment on lines +327 to +333
Copy link
Contributor

Choose a reason for hiding this comment

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

is Protocol defaulted if Port is nil? I'm confused on how this will work since Port is optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See CEL validation : TCP => need to set port.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming the order will be default and then validate, but have to check.

}

// ClusterNetworkPolicyPort describes how to match by port. This can
// only be used with protocols that use port numbers (e.g. TCP, UDP).
//
// Exactly one of the fields in this struct must be set.
//
// +kubebuilder:validation:MaxProperties=1
// +kubebuilder:validation:MinProperties=1
type ClusterNetworkPolicyPort struct {
// Port selects a destination port based on protocol and port number.
// Port selects the port by number.
//
// +optional
PortNumber *Port `json:"portNumber,omitempty"`
Number *int32 `json:"number,omitempty"`

// PortRange selects a destination port range based on protocol and
// start and end port numbers.
// PortRange selects the port by range.
//
// +optional
PortRange *PortRange `json:"portRange,omitempty"`
Range *PortRange `json:"range,omitempty"`

// NamedPort selects a destination port on a pod based on the ContainerPort
// name. You can't use this in a rule with Nodes or Networks peers,
// because they do not have named ports.
// NamedPort selects a destination port on a pod based on the
// ContainerPort name. You can't use this in a rule with Nodes
// or Networks peers, because they do not have named ports.
//
// <network-policy-api:experimental>
// +optional
NamedPort *string `json:"namedPort,omitempty"`
Name *string `json:"name,omitempty"`
}

// ClusterNetworkPolicyEgressPeer defines a peer to allow traffic to.
Expand Down Expand Up @@ -424,39 +443,21 @@ type NamespacedPod struct {
PodSelector metav1.LabelSelector `json:"podSelector"`
}

type Port struct {
// Protocol is the network protocol (TCP, UDP, or SCTP) which traffic must
// match. If not specified, this field defaults to TCP.
// +kubebuilder:default=TCP
//
Protocol corev1.Protocol `json:"protocol"`

// Number defines a network port value.
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
//
Port int32 `json:"port"`
}

// PortRange defines an inclusive range of ports from the assigned
// Start value to End value.
// +kubebuilder:validation:XValidation:rule="self.start < self.end", message="Start port must be less than End port"
type PortRange struct {
// Protocol is the network protocol (TCP, UDP, or SCTP) which traffic must
// match. If not specified, this field defaults to TCP.
// +kubebuilder:default=TCP
//
Protocol corev1.Protocol `json:"protocol,omitempty"`

// Start defines a network port that is the start of a port range, the Start
// value must be less than End.
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
//
Start int32 `json:"start"`

// End defines a network port that is the end of a port range, the End value
// must be greater than Start.
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
//
Expand Down
57 changes: 31 additions & 26 deletions apis/v1alpha2/zz_generated.deepcopy.go
View file
Open in desktop

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

Loading
Loading