Skip to content

Commit

Permalink
Merge pull request #2090 from rikatz/port-range
Browse files Browse the repository at this point in the history 
KEP 2079 - Add PortRange KEP
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Dec 9, 2020
2 parents 2df8338 + d180bf7 commit f40c24a
Show file tree
Hide file tree
Showing 2 changed files with 422 additions and 0 deletions.
386 changes: 386 additions & 0 deletions keps/sig-network/2079-network-policy-port-range/README.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,386 @@
# KEP-2079: Network Policy to support Port Ranges

<!-- toc -->
- [Release Signoff Checklist](#release-signoff-checklist)
- [Summary](#summary)
- [Motivation](#motivation)
- [Goals](#goals)
- [Non-Goals](#non-goals)
- [Proposal](#proposal)
- [User Stories](#user-stories)
- [Story 1 - Opening communication to NodePorts of other cluster](#story-1---opening-communication-to-nodeports-of-other-cluster)
- [Story 2 - Blocking the egress for not allowed insecure ports](#story-2---blocking-the-egress-for-not-allowed-insecure-ports)
- [Story 3 - Containerized Passive FTP Server](#story-3---containerized-passive-ftp-server)
- [Notes/Constraints/Caveats](#notesconstraintscaveats)
- [Risks and Mitigations](#risks-and-mitigations)
- [Design Details](#design-details)
- [Validations](#validations)
- [Test Plan](#test-plan)
- [Graduation Criteria](#graduation-criteria)
- [Alpha](#alpha)
- [Beta](#beta)
- [GA Graduation](#ga-graduation)
- [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
- [Feature Enablement and Rollback](#feature-enablement-and-rollback)
- [Monitoring Requirements](#monitoring-requirements)
- [Dependencies](#dependencies)
- [Scalability](#scalability)
- [Troubleshooting](#troubleshooting)
- [Implementation History](#implementation-history)
- [Drawbacks](#drawbacks)
- [Alternatives](#alternatives)
<!-- /toc -->

## Release Signoff Checklist

Items marked with (R) are required *prior to targeting to a milestone / release*.

- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
- [ ] (R) KEP approvers have approved the KEP status as `implementable`
- [ ] (R) Design details are appropriately documented
- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
- [ ] (R) Graduation criteria is in place
- [ ] (R) Production readiness review completed
- [ ] Production readiness review approved
- [ ] "Implementation History" section is up-to-date for milestone
- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes


## Summary

Today the `ports` field in ingress and egress network policies is an array
that needs a declaration of each single port to be contemplated. This KEP
proposes to add a new field that allows a declaration of a port range,
simplifying the creation of rules with multiple ports.

## Motivation

NetworkPolicy object is a complex object, that allows a developer to specify
what's the traffic behavior expected of the application and allow/deny
undesired traffic.

There are a number of user issues like [kubernetes #67526](https://github.com/kubernetes/kubernetes/issues/67526)
and [kubernetes #93111](https://github.com/kubernetes/kubernetes/issues/93111)
where users expose the need to create a policy that allow a range of ports but some
specific port, or also cases that a user wants to create a policy that allows
the egress to other cluster to the NodePort range (eg 32000-32768) and in this case,
the rule should be created specifying each port separately, as:

```
spec:
egress:
- ports:
- protocol: TCP
port: 32000
- protocol: TCP
port: 32001
- protocol: TCP
port: 32002
- protocol: TCP
port: 32003
[...]
- protocol: TCP
port: 32768
```

So for the user:
* To allow a range of ports, each of them must be declared as an item from
`ports` array
* To make an exception needs a declaration of all ports but the exception

Adding a new `endPort` field inside the `ports` will allow a simpler
creation of NetworkPolicy to the user.

### Goals

Add an endPort field in `NetworkPolicyPort`

### Non-Goals

* Support specific `Exception` field.
* Support `endPort` when the starting `port` is a named port.

## Proposal

In NetworkPolicy specification, inside `NetworkPolicyPort` specify a new
`endPort` field composed of a numbered port that defines if this is a range
and when it ends.

### User Stories

#### Story 1 - Opening communication to NodePorts of other cluster

I have an application that communicates with NodePorts of a different cluster
and I want to allow the egress of the traffic only the NodePort range
(eg. 30000-32767) as I don't know which port is going to be allocated on the
other side, but don't want to create a rule for each of them.

#### Story 2 - Blocking the egress for not allowed insecure ports
As a developer, I need to create an application that scrapes informations from
multiple sources, being those sources databases running in random ports, web
applications and other sources. But the security policy of my company asks me
to block communication with well known ports, like 111 and 445, so I need to create
a network policy that allows me to communicate with any port except those two and so
I can be compliant with the company's policy.

#### Story 3 - Containerized Passive FTP Server
As a Kubernetes User, I've received a demand from my boss to run our FTP server in an
existing Kubernetes cluster, to support some of my legacy applications.
This FTP Server must be acessible from inside the cluster and outside the cluster,
but I still need to keep the basic security policies from my company, that demands
the existence of a default deny rule for all workloads and allowing only specific ports.

Because this FTP Server runs in PASV mode, I need to open the Network Policy to ports 21
and also to the range 49152-65535 without allowing any other ports.


### Notes/Constraints/Caveats

* The technology used by the CNI provider might not support port range in a
trivial way as described in [#drawbacks]

### Risks and Mitigations

CNIs will need to support the new field in their controllers. For this case
we'll try to make broader communication with the main CNIs so they can be aware
of the new field.

## Design Details

API changes to NetworkPolicy:
* Add a new field called `EndPort` inside `NetworkPolicyPort` as the following:
```
// NetworkPolicyPort describes a port to allow traffic on
type NetworkPolicyPort struct {
// The protocol (TCP, UDP, or SCTP) which traffic must match. If not specified, this
// field defaults to TCP.
// +optional
Protocol *v1.Protocol `json:"protocol,omitempty" protobuf:"bytes,1,opt,name=protocol,casttype=k8s.io/api/core/v1.Protocol"`
// The port on the given protocol. This can either be a numerical or named
// port on a pod. If this field is not provided, this matches all port names and
// numbers, whether an endPort is defined or not.
// +optional
Port *intstr.IntOrString `json:"port,omitempty" protobuf:"bytes,2,opt,name=port"`
// EndPort defines the last port included in the port range.
// Example:
// endPort: 12345
// +optional
EndPort int32 `json:"port,omitempty" protobuf:"bytes,2,opt,name=endPort"`
}
```

### Validations
The `NetworkPolicyPort` will need to be validated, with the following scenarios:
* If an `EndPort` is specified a `Port` must also be specified
* If `Port` is a string (named port) `EndPort` cannot be specified
* `EndPort` must be equal or bigger than `Port`

### Test Plan

Unit tests:
* test API validation logic
* test API strategy to ensure disabled fields

E2E tests:
* Add e2e tests exercising only the API operations for port ranges. Data-path
validation should be done by CNIs.


### Graduation Criteria

#### Alpha
- Add a feature gated new field to NetworkPolicy
- Communicate CNI providers about the new field
- Add validation tests in API

#### Beta
- `EndPort` has been supported for at least 1 minor release
- Four commonly used NetworkPolicy (or CNI providers) implement the new field,
with generally positive feedback on its usage.
- Feature Gate is enabled by Default.

#### GA Graduation

- At least **four** NetworkPolicy providers (or CNI providers) support the `EndPort` field
- `EndPort` has been enabled by default for at least 1 minor release

### Upgrade / Downgrade Strategy

If upgraded no impact should happen as this is a new field.

If downgraded the CNI wont be able to look into the new field, as this does not
exists and network policies using this field will stop working correctly and
start working incorrectly. This is a fail-closed failure, so it is acceptable.

## Production Readiness Review Questionnaire

### Feature Enablement and Rollback

_This section must be completed when targeting alpha to a release._

* **How can this feature be enabled / disabled in a live cluster?**
- [X] Feature gate (also fill in values in `kep.yaml`)
- Feature gate name: NetworkPolicyEndPort
- Components depending on the feature gate: Kubernetes API Server

* **Does enabling the feature change any default behavior?**
No

* **Can the feature be disabled once it has been enabled (i.e. can we roll back
the enablement)?**
Yes, but CNIs relying on the new field wont recognize it anymore

* **What happens if we reenable the feature if it was previously rolled back?**
Nothing. Just need to check if the data is persisted in `etcd` after the
feature is disabled and reenabled or if the data is missed

* **Are there any tests for feature enablement/disablement?**

TBD

### Monitoring Requirements

_This section must be completed when targeting beta graduation to a release._

* **How can an operator determine if the feature is in use by workloads?**

Operators can determine if NetworkPolicies are making use of EndPort creating
an object specifying the range and validating if the traffic is allowed within
the specified range

* **What are the SLIs (Service Level Indicators) an operator can use to determine
the health of the service?**
Operators would need to monitor the traffic of the Pods to verify if a
specified port range is applied and allowed in their workloads

* **What are the reasonable SLOs (Service Level Objectives) for the above SLIs?**
N/A

* **Are there any missing metrics that would be useful to have to improve observability
of this feature?**
N/A


### Dependencies

* **Does this feature depend on any specific services running in the cluster?**
No


### Scalability

_For alpha, this section is encouraged: reviewers should consider these questions
and attempt to answer them._

_For beta, this section is required: reviewers must answer these questions._

_For GA, this section is required: approvers should be able to confirm the
previous answers based on experience in the field._

* **Will enabling / using this feature result in any new API calls?**
TBD

* **Will enabling / using this feature result in introducing new API types?**
No, unless the new `EndPort` is considered a new API type

* **Will enabling / using this feature result in any new calls to the cloud
provider?**
No

* **Will enabling / using this feature result in increasing size or count of
the existing API objects?**

- API type(s): NetworkPolicyPorts
- Estimated increase in size: 2 bytes for each new `EndPort` specified
- Estimated amount of new objects: N/A

* **Will enabling / using this feature result in increasing time taken by any
operations covered by [existing SLIs/SLOs]?**
N/A

* **Will enabling / using this feature result in non-negligible increase of
resource usage (CPU, RAM, disk, IO, ...) in any components?**
It might get some increase of resource usage by the CNI while parsing the
new field.

### Troubleshooting

_This section must be completed when targeting beta graduation to a release._

* **How does this feature react if the API server and/or etcd is unavailable?**
As this feature is mainly used by CNI providers, the reaction with API server
and/or etcd being unavailable will be the same as before.

* **What are other known failure modes?**
N/A

* **What steps should be taken if SLOs are not being met to determine the problem?**
N/A

## Implementation History
- 2020-10-08 Initial [KEP PR](https://github.com/kubernetes/enhancements/pull/2079)

## Drawbacks

* The technology used by the CNI provider might not support port range in a
trivial way. As an example, OpenFlow did not supported to specify port range
for a while as commented in [kubernetes #67526](https://github.com/kubernetes/kubernetes/issues/67526#issuecomment-415170435).
While this has changed in Open vSwitch v1.6, this still might be a caveat
for other CNIs, like eBPF based CNIs will need to populate their maps in a
different way.

For this cases, CNIs will have to iteract through the Port Range and
populate their packet filtering tables with each port.

## Alternatives

During the development of this KEP there was an alternative implementation
of the `NetworkPolicyPortRange` field inside the `NetworkPolicyPort` as the following:

```
// NetworkPolicyPort describes a port or a range of ports to allow traffic on
type NetworkPolicyPort struct {
// The protocol (TCP, UDP, or SCTP) which traffic must match. If not specified, this
// field defaults to TCP.
// +optional
Protocol *api.Protocol
// The port on the given protocol. This can either be a numerical or named
// port on a pod. If this field is not provided but a Range is
// provided, this field is ignored. Otherwise this matches all port names and
// numbers.
// +optional
Port *intstr.IntOrString
// A range of ports on a given protocol and the exceptions. If this field
// is not provided, this doesn't matches anything
// +optional
Range *NetworkPolicyPortRange
}
```

But the main design suggested in this Kep seems more clear, so this alternative
has been discarded.

Also it has been proposed that the implementation contains an `Except` array and a new
struct to be used in Ingress/Egress rules, but because it would bring much more complexity
than desired the proposal has been dropped right now:

```
// NetworkPolicyPortRange describes the range of ports to be used in a
// NetworkPolicyPort struct
type NetworkPolicyPortRange struct {
// From defines the start of the port range
From uint16
// To defines the end of the port range, being the end included within the
// range
To uint16
// Except defines all the exceptions in the port range
+optional
Except []uint16
```
Loading

0 comments on commit f40c24a

Please sign in to comment.