From 580009a9f750a559236a6648517d4940b261563d Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Thu, 8 Oct 2020 15:15:27 -0300 Subject: [PATCH 01/13] Add PortRange KEP Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 623 ++++++++++++++++++ .../2079-network-policy-port-range/kep.yaml | 34 + 2 files changed, 657 insertions(+) create mode 100644 keps/sig-network/2079-network-policy-port-range/README.md create mode 100644 keps/sig-network/2079-network-policy-port-range/kep.yaml diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md new file mode 100644 index 00000000000..348b4d87f8f --- /dev/null +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -0,0 +1,623 @@ + +# KEP-2079: Add PortRange field to NetworkPolicy + + + + +- [Release Signoff Checklist](#release-signoff-checklist) +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) + - [User Stories (Optional)](#user-stories-optional) + - [Story 1](#story-1) + - [Story 2](#story-2) + - [Story 3](#story-3) + - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional) + - [Risks and Mitigations](#risks-and-mitigations) +- [Design Details](#design-details) + - [Test Plan](#test-plan) + - [Graduation Criteria](#graduation-criteria) + - [Alpha](#alpha) + - [Beta](#beta) + - [GA Graduation](#ga-graduation) + - [Removing a Deprecated Flag](#removing-a-deprecated-flag) + - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) + - [Version Skew Strategy](#version-skew-strategy) +- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) + - [Feature Enablement and Rollback](#feature-enablement-and-rollback) +- [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) +- [Infrastructure Needed (Optional)](#infrastructure-needed-optional) + + +## 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 [this](https://github.com/kubernetes/kubernetes/issues/67526) and [this](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: 32004 # Exception to port 32003 +[...] +``` + +So for the user, actually: +* 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 ``PortRange`` field inside the ``ports`` will allow a simpler creation of NetworkPolicy to the user. + +### Goals + + +Add PortRange field to Ports in NetworkPolicy + +### Non-Goals + + + +TBD +## Proposal + + + +In NetworkPolicy specification, inside ``NetworkPolicyPort`` object struct specify a new ``PortRange`` field: + +``` +// NetworkPolicyPort describes a port to allow traffic on +type NetworkPolicyPort struct { + PortRange *string +} +``` + +### User Stories (Optional) + + + +#### Story 1 + +As the owner of a web application I want to allow the traffic to my Pods in different ports without having to create multiple network policies. As an example, my application is exposed in ports 80, 443 and 8443 and I want to group this in a single rule + +#### Story 2 + +I have another 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 in the other side, but don't want to create a rule for each of them. + +#### Story 3 + +As the owner of an application, I want to allow the communication with a set of ports between 6000 and 7000 except port 6667. + +### Notes/Constraints/Caveats (Optional) + + + +### 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. + +The field will need some syntax validation either, to not allow the mix of named and non-named ports inside a range (like http-9000), and also to allow only numbered ports inside uint16 range. + +## Design Details + + + +API changes to NetworkPolicy: +* Add a new field ``spec.ingress|egress.ports.PortRange +* On field creation/change a validation should happen to verify if the format is compliant with ``nnnnn,nnnn1-nnnn5,http`` where nnnnn is a number between 0 and 65535 and named ports aren't mixed with numbered ports + +### Test Plan + + + +### Graduation Criteria + + +#### Alpha +- Add a feature gated new field to NetworkPolicy +- Communicate CNI providers about the new field +- Add validation tests in API + +#### Beta +- ``PortRanges`` has been supported for at least 1 minor release +- At least one CNI provider implements the new field +- Feature Gate is enabled by Default + +#### GA Graduation + +- At least two CNIs support the ``PortRanges`` field +- ``PortRanges`` has been enabled by default for at least 1 minor release + + +**Note:** Generally we also wait at least two releases between beta and +GA/stable, because there's no opportunity for user feedback, or even bug reports, +in back-to-back releases. + +#### Removing a Deprecated Flag + +- Announce deprecation and support policy of the existing flag +- Two versions passed since introducing the functionality that deprecates the flag (to address version skew) +- Address feedback on usage/changed behavior, provided on GitHub issues +- Deprecate the flag + +**For non-optional features moving to GA, the graduation criteria must include +[conformance tests].** + +[conformance tests]: https://git.k8s.io/community/contributors/devel/sig-architecture/conformance-tests.md +--> + +### 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. + +### Version Skew Strategy + + + +## 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: NetworkPolicyPortRange + - 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 + + + +## Drawbacks + + + +## Alternatives + + + +## Infrastructure Needed (Optional) + + diff --git a/keps/sig-network/2079-network-policy-port-range/kep.yaml b/keps/sig-network/2079-network-policy-port-range/kep.yaml new file mode 100644 index 00000000000..52e825de103 --- /dev/null +++ b/keps/sig-network/2079-network-policy-port-range/kep.yaml @@ -0,0 +1,34 @@ +title: KEP Template +kep-number: NNNN +authors: + - "@rikatz" +owning-sig: sig-network +status: provisional +creation-date: "2020-10-08" +reviewers: + - TBD +approvers: + - TBD + +# The target maturity stage in the current dev cycle for this KEP. +stage: alpha + +# The most recent milestone for which work toward delivery of this KEP has been +# done. This can be the current (upcoming) milestone, if it is being actively +# worked on. +latest-milestone: "v1.21" + +# The milestone at which this feature was, or is targeted to be, at each stage. +milestone: + alpha: "v1.21" + beta: "v1.22" + stable: "v1.23" + +# The following PRR answers are required at alpha release +# List the feature gate name and the components for which it must be enabled +feature-gates: + - name: NetworkPolicyPortRange + components: + - kube-apiserver +disable-supported: true + From 5aab111d5d37ffd293af0a1f137630dd6e97d55d Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Sun, 25 Oct 2020 16:40:43 -0300 Subject: [PATCH 02/13] Add PortRange KEP Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 521 +++--------------- 1 file changed, 91 insertions(+), 430 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index 348b4d87f8f..bfa2448edd8 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -1,75 +1,3 @@ - -# KEP-2079: Add PortRange field to NetworkPolicy - - - - [Release Signoff Checklist](#release-signoff-checklist) - [Summary](#summary) @@ -79,9 +7,7 @@ tags, and then generate with `hack/update-toc.sh`. - [Proposal](#proposal) - [User Stories (Optional)](#user-stories-optional) - [Story 1](#story-1) - - [Story 2](#story-2) - - [Story 3](#story-3) - - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional) + - [Notes/Constraints/Caveats](#notesconstraintscaveats) - [Risks and Mitigations](#risks-and-mitigations) - [Design Details](#design-details) - [Test Plan](#test-plan) @@ -89,32 +15,18 @@ tags, and then generate with `hack/update-toc.sh`. - [Alpha](#alpha) - [Beta](#beta) - [GA Graduation](#ga-graduation) - - [Removing a Deprecated Flag](#removing-a-deprecated-flag) - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) - - [Version Skew Strategy](#version-skew-strategy) - [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) - [Feature Enablement and Rollback](#feature-enablement-and-rollback) -- [Drawbacks](#drawbacks) -- [Alternatives](#alternatives) -- [Infrastructure Needed (Optional)](#infrastructure-needed-optional) + - [Monitoring Requirements](#monitoring-requirements) + - [Dependencies](#dependencies) + - [Scalability](#scalability) + - [Troubleshooting](#troubleshooting) +- [Implementation History](#implementation-history) ## 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) @@ -131,41 +43,23 @@ Items marked with (R) are required *prior to targeting to a milestone / release* ## 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. +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. -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 [this](https://github.com/kubernetes/kubernetes/issues/67526) and [this](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: +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: @@ -178,159 +72,86 @@ spec: - protocol: TCP port: 32002 - protocol: TCP - port: 32004 # Exception to port 32003 + port: 32003 [...] + - protocol: TCP + port: 32768 ``` So for the user, actually: * 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 ``PortRange`` field inside the ``ports`` will allow a simpler creation of NetworkPolicy to the user. +Adding a new ``PortRange`` field inside the ``ports`` will allow a simpler +creation of NetworkPolicy to the user. ### Goals - -Add PortRange field to Ports in NetworkPolicy +Add Range field to Ports in NetworkPolicy ### Non-Goals - - TBD + ## Proposal - -In NetworkPolicy specification, inside ``NetworkPolicyPort`` object struct specify a new ``PortRange`` field: +In NetworkPolicy specification, inside ``NetworkPolicyPort`` object struct +specify a new ``Range`` field composed of the minimum and maximum ports +inside the range. -``` -// NetworkPolicyPort describes a port to allow traffic on -type NetworkPolicyPort struct { - PortRange *string -} -``` ### User Stories (Optional) - - #### Story 1 -As the owner of a web application I want to allow the traffic to my Pods in different ports without having to create multiple network policies. As an example, my application is exposed in ports 80, 443 and 8443 and I want to group this in a single rule - -#### Story 2 - -I have another 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 in the other side, but don't want to create a rule for each of them. +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 3 +### Notes/Constraints/Caveats -As the owner of an application, I want to allow the communication with a set of ports between 6000 and 7000 except port 6667. - -### Notes/Constraints/Caveats (Optional) - - +* The technology used by the CNI provider might not support port range in a +trivial way. As an example, OpenFlow does not have a way to specify port range +and this might be a problem for OVS based CNIs. The same way, eBPF based CNIs +will need to populate their maps in a different way. ### 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. - -The field will need some syntax validation either, to not allow the mix of named and non-named ports inside a range (like http-9000), and also to allow only numbered ports inside uint16 range. +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 ``spec.ingress|egress.ports.PortRange -* On field creation/change a validation should happen to verify if the format is compliant with ``nnnnn,nnnn1-nnnn5,http`` where nnnnn is a number between 0 and 65535 and named ports aren't mixed with numbered ports - -### Test Plan - - +TBD ### Graduation Criteria - #### Alpha - Add a feature gated new field to NetworkPolicy - Communicate CNI providers about the new field @@ -346,83 +167,15 @@ Below are some examples to consider, in addition to the aforementioned [maturity - At least two CNIs support the ``PortRanges`` field - ``PortRanges`` has been enabled by default for at least 1 minor release - -**Note:** Generally we also wait at least two releases between beta and -GA/stable, because there's no opportunity for user feedback, or even bug reports, -in back-to-back releases. - -#### Removing a Deprecated Flag - -- Announce deprecation and support policy of the existing flag -- Two versions passed since introducing the functionality that deprecates the flag (to address version skew) -- Address feedback on usage/changed behavior, provided on GitHub issues -- Deprecate the flag - -**For non-optional features moving to GA, the graduation criteria must include -[conformance tests].** - -[conformance tests]: https://git.k8s.io/community/contributors/devel/sig-architecture/conformance-tests.md ---> - ### 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. - -### Version Skew Strategy - - +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. ## Production Readiness Review Questionnaire - - ### Feature Enablement and Rollback _This section must be completed when targeting alpha to a release._ @@ -440,73 +193,40 @@ _This section must be completed when targeting alpha to a release._ 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 + 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 - - -## Drawbacks - - - -## Alternatives - - - -## Infrastructure Needed (Optional) - - +- 2020-10-08 Initial [KEP PR](https://github.com/kubernetes/enhancements/pull/2079) \ No newline at end of file From 5f5fbd04cd58a0c32f6710b576c033bc96e158ce Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Tue, 27 Oct 2020 09:07:39 -0300 Subject: [PATCH 03/13] Add PortRange KEP Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 18 ++++++++++-------- .../2079-network-policy-port-range/kep.yaml | 4 +++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index bfa2448edd8..892a219c113 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -114,14 +114,15 @@ other side, but don't want to create a rule for each of them. * The technology used by the CNI provider might not support port range in a trivial way. As an example, OpenFlow does not have a way to specify port range -and this might be a problem for OVS based CNIs. The same way, eBPF based CNIs -will need to populate their maps in a different way. +and this might be a problem for OVS based CNIs as commented in +[kubernetes #67526](https://github.com/kubernetes/kubernetes/issues/67526#issuecomment-415170435). +The same way, eBPF based CNIs will need to populate their maps in a different way. ### 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. +of the new field. ## Design Details @@ -129,8 +130,8 @@ API changes to NetworkPolicy: * Add a new struct called ``NetworkPolicyPortRange`` as the following: ``` type NetworkPolicyPortRange struct { - Max uint16 - Min uint16 + From uint16 + To uint16 } ``` * Add a new field ``spec.ingress|egress.ports.range`` that points to the @@ -141,9 +142,10 @@ type NetworkPolicyPort struct { Range PortRange } ``` -As a suggestion, there's already a ``HostPortRange`` defined in -the [code](https://github.com/kubernetes/kubernetes/blob/a138be8722ebd0ce281029fa747315500a99ffd5/pkg/apis/policy/types.go#L262) so maybe this could be renamed and re-used in -other parts of the code. + +The range will need to be validated, with the following scenarios: +* If there's a ``From`` or a ``To`` field defined, the other one must be defined. +* ``From`` needs to be less than or equal to ``To`` ### Test Plan diff --git a/keps/sig-network/2079-network-policy-port-range/kep.yaml b/keps/sig-network/2079-network-policy-port-range/kep.yaml index 52e825de103..11cfb03089c 100644 --- a/keps/sig-network/2079-network-policy-port-range/kep.yaml +++ b/keps/sig-network/2079-network-policy-port-range/kep.yaml @@ -6,7 +6,9 @@ owning-sig: sig-network status: provisional creation-date: "2020-10-08" reviewers: - - TBD + - "@andrewsykim" + - "@abhiraut" + - "@danwinship" approvers: - TBD From c4041bb9c0c5f89b0e2cf51a95245a8b42937f80 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Fri, 30 Oct 2020 09:26:17 -0300 Subject: [PATCH 04/13] Add PortRange KEP Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index 892a219c113..25db0400778 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -23,6 +23,7 @@ - [Scalability](#scalability) - [Troubleshooting](#troubleshooting) - [Implementation History](#implementation-history) +- [Drawbacks](#drawbacks) ## Release Signoff Checklist @@ -79,7 +80,8 @@ spec: ``` So for the user, actually: -* To allow a range of ports, each of them must be declared as an item from ``ports`` array +* 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 ``PortRange`` field inside the ``ports`` will allow a simpler @@ -91,15 +93,18 @@ Add Range field to Ports in NetworkPolicy ### Non-Goals -TBD +Specify exceptions to the range. An exception can at this time be specified +as multiple ranges not contemplating the exception. ## Proposal - In NetworkPolicy specification, inside ``NetworkPolicyPort`` object struct specify a new ``Range`` field composed of the minimum and maximum ports inside the range. +If both ``Port`` and ``Range`` are specified, they are cumulative and no further +validation might occur. It's up to the CNI to summarize both of the fields in a +single rule. ### User Stories (Optional) @@ -113,10 +118,7 @@ other side, but don't want to create a rule for each of them. ### Notes/Constraints/Caveats * The technology used by the CNI provider might not support port range in a -trivial way. As an example, OpenFlow does not have a way to specify port range -and this might be a problem for OVS based CNIs as commented in -[kubernetes #67526](https://github.com/kubernetes/kubernetes/issues/67526#issuecomment-415170435). -The same way, eBPF based CNIs will need to populate their maps in a different way. +trivial way as described in [#drawbacks] ### Risks and Mitigations @@ -139,7 +141,7 @@ new struct: ``` // NetworkPolicyPort describes a port to allow traffic on type NetworkPolicyPort struct { - Range PortRange + Range NetworkPolicyPortRange } ``` @@ -283,4 +285,12 @@ _This section must be completed when targeting beta graduation to a release._ N/A ## Implementation History -- 2020-10-08 Initial [KEP PR](https://github.com/kubernetes/enhancements/pull/2079) \ No newline at end of file +- 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 does not have a way to specify port range +and this might be a problem for OVS based CNIs as commented in +[kubernetes #67526](https://github.com/kubernetes/kubernetes/issues/67526#issuecomment-415170435). +The same way, eBPF based CNIs will need to populate their maps in a different way. From 976c55bdf1e090bb5b149930bd39cd5727b4ba4d Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Tue, 3 Nov 2020 14:27:21 -0300 Subject: [PATCH 05/13] Add PortRange KEP Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 32 ++++++++++++------- .../2079-network-policy-port-range/kep.yaml | 4 +-- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index 25db0400778..0581ff52d87 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -10,6 +10,7 @@ - [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) @@ -79,7 +80,7 @@ spec: port: 32768 ``` -So for the user, actually: +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 @@ -93,8 +94,7 @@ Add Range field to Ports in NetworkPolicy ### Non-Goals -Specify exceptions to the range. An exception can at this time be specified -as multiple ranges not contemplating the exception. +N/A ## Proposal @@ -132,8 +132,9 @@ API changes to NetworkPolicy: * Add a new struct called ``NetworkPolicyPortRange`` as the following: ``` type NetworkPolicyPortRange struct { - From uint16 - To uint16 + From uint16 + To uint16 + Exceptions: []uint16 } ``` * Add a new field ``spec.ingress|egress.ports.range`` that points to the @@ -145,10 +146,15 @@ type NetworkPolicyPort struct { } ``` +### Validations The range will need to be validated, with the following scenarios: * If there's a ``From`` or a ``To`` field defined, the other one must be defined. * ``From`` needs to be less than or equal to ``To`` - +* All the ports in the ``Exceptions`` array must be inside the defined range. +* ``Exception`` can only be defined if ``From`` and ``To`` are also defined. +* Because ``ports`` is a superset of all ports specified in ``port`` and +``range``, if a port is specified in at least one of the fields it should be +allowed. ### Test Plan @@ -258,7 +264,7 @@ the existing API objects?** - API type(s): NetworkPolicy / NetworkPolicyPorts - Estimated increase in size: New struct inside the object with two fields of - 16 bits each + 16 bits each + 16 bits for each port in the ``Exceptions`` array - Estimated amount of new objects: N/A * **Will enabling / using this feature result in increasing time taken by any @@ -290,7 +296,11 @@ _This section must be completed when targeting beta graduation to a release._ ## Drawbacks * The technology used by the CNI provider might not support port range in a -trivial way. As an example, OpenFlow does not have a way to specify port range -and this might be a problem for OVS based CNIs as commented in -[kubernetes #67526](https://github.com/kubernetes/kubernetes/issues/67526#issuecomment-415170435). -The same way, eBPF based CNIs will need to populate their maps in a different way. +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. diff --git a/keps/sig-network/2079-network-policy-port-range/kep.yaml b/keps/sig-network/2079-network-policy-port-range/kep.yaml index 11cfb03089c..1b4fc037347 100644 --- a/keps/sig-network/2079-network-policy-port-range/kep.yaml +++ b/keps/sig-network/2079-network-policy-port-range/kep.yaml @@ -1,5 +1,5 @@ -title: KEP Template -kep-number: NNNN +title: Allow a Network Policy to contemplate a set of ports in a single rule +kep-number: 2079 authors: - "@rikatz" owning-sig: sig-network From bd244b1c22c9d2a274e79946f5891d999482622e Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Tue, 3 Nov 2020 17:44:34 -0300 Subject: [PATCH 06/13] Add PortRange KEP Signed-off-by: Ricardo Pchevuzinske Katz --- keps/sig-network/2079-network-policy-port-range/kep.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keps/sig-network/2079-network-policy-port-range/kep.yaml b/keps/sig-network/2079-network-policy-port-range/kep.yaml index 1b4fc037347..cf65f8b798c 100644 --- a/keps/sig-network/2079-network-policy-port-range/kep.yaml +++ b/keps/sig-network/2079-network-policy-port-range/kep.yaml @@ -3,7 +3,7 @@ kep-number: 2079 authors: - "@rikatz" owning-sig: sig-network -status: provisional +status: implementable creation-date: "2020-10-08" reviewers: - "@andrewsykim" From 7e6146f8de9071756f9d641f6503f0c6bcbe4acf Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Thu, 5 Nov 2020 14:43:18 -0300 Subject: [PATCH 07/13] Update with full struct and test docs Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 48 +++++++++++++++---- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index 0581ff52d87..c636084a524 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -131,20 +131,42 @@ of the new field. API changes to NetworkPolicy: * Add a new struct called ``NetworkPolicyPortRange`` as the following: ``` +// 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 - Exceptions: []uint16 + + // Except defines all the exceptions in the port range + Except: []uint16 } ``` * Add a new field ``spec.ingress|egress.ports.range`` that points to the new struct: ``` -// NetworkPolicyPort describes a port to allow traffic on +// NetworkPolicyPort describes a port or a range of ports to allow traffic on type NetworkPolicyPort struct { - Range NetworkPolicyPortRange + // 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 } -``` ### Validations The range will need to be validated, with the following scenarios: @@ -155,10 +177,19 @@ The range will need to be validated, with the following scenarios: * Because ``ports`` is a superset of all ports specified in ``port`` and ``range``, if a port is specified in at least one of the fields it should be allowed. +* If ``Range`` is defined but no ``Port`` is defined, the old behavior of matching +all should be changed. ### Test Plan -TBD +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 @@ -169,12 +200,13 @@ TBD #### Beta - ``PortRanges`` has been supported for at least 1 minor release -- At least one CNI provider implements the new field -- Feature Gate is enabled by Default +- 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 two CNIs support the ``PortRanges`` field +- At least **four** NetworkPolicy providers (or CNI providers) support the ``PortRanges`` field - ``PortRanges`` has been enabled by default for at least 1 minor release ### Upgrade / Downgrade Strategy From e1809e30fddc5782853604ff3c699016e412b926 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Thu, 5 Nov 2020 14:47:03 -0300 Subject: [PATCH 08/13] Add PortRange KEP Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index c636084a524..5fd244ce892 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -150,23 +150,24 @@ new struct: ``` // 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 + // 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 + // +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 } +``` ### Validations The range will need to be validated, with the following scenarios: From 2710f62db72ebef5347490e7b0892a6de7671ae4 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Thu, 12 Nov 2020 15:10:46 -0300 Subject: [PATCH 09/13] Change portranges location and correct parts in the Kep Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 171 ++++++++++++++---- 1 file changed, 133 insertions(+), 38 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index 5fd244ce892..c05410494f4 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -1,3 +1,5 @@ +# KEP-2079: Network Policy to support Port Ranges + - [Release Signoff Checklist](#release-signoff-checklist) - [Summary](#summary) @@ -5,8 +7,10 @@ - [Goals](#goals) - [Non-Goals](#non-goals) - [Proposal](#proposal) - - [User Stories (Optional)](#user-stories-optional) - - [Story 1](#story-1) + - [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) @@ -25,6 +29,7 @@ - [Troubleshooting](#troubleshooting) - [Implementation History](#implementation-history) - [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) ## Release Signoff Checklist @@ -90,7 +95,7 @@ creation of NetworkPolicy to the user. ### Goals -Add Range field to Ports in NetworkPolicy +Add Port Range field in a NetworkPolicy ### Non-Goals @@ -98,23 +103,43 @@ N/A ## Proposal -In NetworkPolicy specification, inside ``NetworkPolicyPort`` object struct -specify a new ``Range`` field composed of the minimum and maximum ports -inside the range. +In NetworkPolicy specification, inside ``NetworkPolicyIngressRule`` and +``NetworkPolicyEgressRule`` object struct specify a new ``PortRanges`` field +composed of the minimum and maximum ports inside the range, and the exceptions +within this range. If both ``Port`` and ``Range`` are specified, they are cumulative and no further validation might occur. It's up to the CNI to summarize both of the fields in a single rule. -### User Stories (Optional) +### User Stories -#### Story 1 +#### 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. +his 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 @@ -142,44 +167,82 @@ type NetworkPolicyPortRange struct { To uint16 // Except defines all the exceptions in the port range - Except: []uint16 + +optional + Except []uint16 } ``` -* Add a new field ``spec.ingress|egress.ports.range`` that points to the -new struct: +* Add a new field ``PortRanges`` that defines an array of +``NetworkPolicyPortRange`` in both ``NetworkPolicyIngressRule`` and +``NetworkPolicyEgressRule`` + ``` -// 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 +// NetworkPolicyIngressRule describes a particular set of traffic that is allowed to the pods +// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and from. +type NetworkPolicyIngressRule struct { + // List of ports which should be made accessible on the pods selected for this + // rule. Each item in this list is combined using a logical OR. If this field is + // empty or missing, this rule matches all ports (traffic not restricted by port). + // If this field is present and contains at least one item, then this rule allows + // traffic only if the traffic matches at least one port in the list. + // +optional + Ports []NetworkPolicyPort + + // List of sources which should be able to access the pods selected for this rule. + // Items in this list are combined using a logical OR operation. If this field is + // empty or missing, this rule matches all sources (traffic not restricted by + // source). If this field is present and contains at least one item, this rule + // allows traffic only if the traffic matches at least one item in the from list. + // +optional + From []NetworkPolicyPeer + + // List of port ranges which should be made accessible on the pods selected for this + // rule. Each item in this list is combined using a logical OR. If this field is + // empty or missing, AND the Ports field is also empty or missing, + // this rule matches all ports (traffic not restricted by port). + // If this field is present and contains at least one item, then this rule allows + // traffic only if the traffic matches at least one port in the list. + // +optional + PortRanges []NetworkPolicyPortRange +} + +// NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods +// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to. +// This type is beta-level in 1.8 +type NetworkPolicyEgressRule struct { + // List of destination ports for outgoing traffic. + // Each item in this list is combined using a logical OR. If this field is + // empty or missing, this rule matches all ports (traffic not restricted by port). + // If this field is present and contains at least one item, then this rule allows + // traffic only if the traffic matches at least one port in the list. + // +optional + Ports []NetworkPolicyPort + + // List of destinations for outgoing traffic of pods selected for this rule. + // Items in this list are combined using a logical OR operation. If this field is + // empty or missing, this rule matches all destinations (traffic not restricted by + // destination). If this field is present and contains at least one item, this rule + // allows traffic only if the traffic matches at least one item in the to list. + // +optional + To []NetworkPolicyPeer + + // List of destination port ranges for outgoing traffic. + // Each item in this list is combined using a logical OR. If this field is + // empty or missing AND the Ports field is also empty or missing, + // this rule matches all ports (traffic not restricted by port). + // If this field is present and contains at least one item, then this rule allows + // traffic only if the traffic matches at least one port inside this range. + // +optional + PortRanges []NetworkPolicyPortRange - // 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 } ``` ### Validations The range will need to be validated, with the following scenarios: -* If there's a ``From`` or a ``To`` field defined, the other one must be defined. * ``From`` needs to be less than or equal to ``To`` -* All the ports in the ``Exceptions`` array must be inside the defined range. -* ``Exception`` can only be defined if ``From`` and ``To`` are also defined. -* Because ``ports`` is a superset of all ports specified in ``port`` and -``range``, if a port is specified in at least one of the fields it should be -allowed. -* If ``Range`` is defined but no ``Port`` is defined, the old behavior of matching -all should be changed. +* All the ports in the ``Except`` array must be inside the defined range. +* If ``PortRange`` is defined but no ``Ports`` is defined, the old behavior of +matching all Ports should be changed to match only PortRange ports. ### Test Plan @@ -215,7 +278,8 @@ with generally positive feedback on its usage. 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. +exists and network policies using this field will stop working correctly and +start working incorrectly. ## Production Readiness Review Questionnaire @@ -297,7 +361,7 @@ the existing API objects?** - API type(s): NetworkPolicy / NetworkPolicyPorts - Estimated increase in size: New struct inside the object with two fields of - 16 bits each + 16 bits for each port in the ``Exceptions`` array + 16 bits each + 16 bits for each port in the ``Except`` array - Estimated amount of new objects: N/A * **Will enabling / using this feature result in increasing time taken by any @@ -337,3 +401,34 @@ 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. + From 7c58e37e66e037837dfeb0963e5745920790059c Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Sun, 22 Nov 2020 16:29:59 -0300 Subject: [PATCH 10/13] Drop exception proposal and move to endport Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 151 ++++++------------ .../2079-network-policy-port-range/kep.yaml | 2 +- 2 files changed, 51 insertions(+), 102 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index c05410494f4..fff609a75eb 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -90,27 +90,23 @@ So for the user: ``ports`` array * To make an exception needs a declaration of all ports but the exception -Adding a new ``PortRange`` field inside the ``ports`` will allow a simpler +Adding a new ``endPort`` field inside the ``ports`` will allow a simpler creation of NetworkPolicy to the user. ### Goals -Add Port Range field in a NetworkPolicy +Add an endPort field in ``NetworkPolicyPort`` ### Non-Goals -N/A +* Support specific ``Exception`` field. +* Support ``endPort`` when the starting ``port`` is a named port. ## Proposal -In NetworkPolicy specification, inside ``NetworkPolicyIngressRule`` and -``NetworkPolicyEgressRule`` object struct specify a new ``PortRanges`` field -composed of the minimum and maximum ports inside the range, and the exceptions -within this range. - -If both ``Port`` and ``Range`` are specified, they are cumulative and no further -validation might occur. It's up to the CNI to summarize both of the fields in a -single rule. +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 @@ -132,7 +128,7 @@ 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. -his FTP Server must be acessible from inside the cluster and outside the cluster, +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. @@ -154,95 +150,31 @@ of the new field. ## Design Details API changes to NetworkPolicy: -* Add a new struct called ``NetworkPolicyPortRange`` as the following: +* Add a new field called ``EndPort`` inside ``NetworkPolicyPort`` as the following: ``` -// 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 -} -``` -* Add a new field ``PortRanges`` that defines an array of -``NetworkPolicyPortRange`` in both ``NetworkPolicyIngressRule`` and -``NetworkPolicyEgressRule`` - -``` -// NetworkPolicyIngressRule describes a particular set of traffic that is allowed to the pods -// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and from. -type NetworkPolicyIngressRule struct { - // List of ports which should be made accessible on the pods selected for this - // rule. Each item in this list is combined using a logical OR. If this field is - // empty or missing, this rule matches all ports (traffic not restricted by port). - // If this field is present and contains at least one item, then this rule allows - // traffic only if the traffic matches at least one port in the list. - // +optional - Ports []NetworkPolicyPort - - // List of sources which should be able to access the pods selected for this rule. - // Items in this list are combined using a logical OR operation. If this field is - // empty or missing, this rule matches all sources (traffic not restricted by - // source). If this field is present and contains at least one item, this rule - // allows traffic only if the traffic matches at least one item in the from list. - // +optional - From []NetworkPolicyPeer - - // List of port ranges which should be made accessible on the pods selected for this - // rule. Each item in this list is combined using a logical OR. If this field is - // empty or missing, AND the Ports field is also empty or missing, - // this rule matches all ports (traffic not restricted by port). - // If this field is present and contains at least one item, then this rule allows - // traffic only if the traffic matches at least one port in the list. - // +optional - PortRanges []NetworkPolicyPortRange -} - -// NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods -// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to. -// This type is beta-level in 1.8 -type NetworkPolicyEgressRule struct { - // List of destination ports for outgoing traffic. - // Each item in this list is combined using a logical OR. If this field is - // empty or missing, this rule matches all ports (traffic not restricted by port). - // If this field is present and contains at least one item, then this rule allows - // traffic only if the traffic matches at least one port in the list. +// 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 - Ports []NetworkPolicyPort + Protocol *v1.Protocol `json:"protocol,omitempty" protobuf:"bytes,1,opt,name=protocol,casttype=k8s.io/api/core/v1.Protocol"` - // List of destinations for outgoing traffic of pods selected for this rule. - // Items in this list are combined using a logical OR operation. If this field is - // empty or missing, this rule matches all destinations (traffic not restricted by - // destination). If this field is present and contains at least one item, this rule - // allows traffic only if the traffic matches at least one item in the to list. - // +optional - To []NetworkPolicyPeer - - // List of destination port ranges for outgoing traffic. - // Each item in this list is combined using a logical OR. If this field is - // empty or missing AND the Ports field is also empty or missing, - // this rule matches all ports (traffic not restricted by port). - // If this field is present and contains at least one item, then this rule allows - // traffic only if the traffic matches at least one port inside this range. + // 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. // +optional - PortRanges []NetworkPolicyPortRange + Port *intstr.IntOrString `json:"port,omitempty" protobuf:"bytes,2,opt,name=port"` + // To defines the end of the port range, being the end included within the + // range + // +optional + EndPort uint16 `json:"port,omitempty" protobuf:"bytes,2,opt,name=endPort"` } ``` ### Validations -The range will need to be validated, with the following scenarios: -* ``From`` needs to be less than or equal to ``To`` -* All the ports in the ``Except`` array must be inside the defined range. -* If ``PortRange`` is defined but no ``Ports`` is defined, the old behavior of -matching all Ports should be changed to match only PortRange ports. +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 ``EndPort`` cannot be specified ### Test Plan @@ -263,15 +195,15 @@ validation should be done by CNIs. - Add validation tests in API #### Beta -- ``PortRanges`` has been supported for at least 1 minor release +- ``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 ``PortRanges`` field -- ``PortRanges`` has been enabled by default for at least 1 minor release +- 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 @@ -289,7 +221,7 @@ _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: NetworkPolicyPortRange + - Feature gate name: NetworkPolicyEndPort - Components depending on the feature gate: Kubernetes API Server * **Does enabling the feature change any default behavior?** @@ -313,7 +245,7 @@ _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 Range creating + 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 @@ -350,7 +282,7 @@ previous answers based on experience in the field._ TBD * **Will enabling / using this feature result in introducing new API types?** - No, unless the new ``NetworkPolicyPortRange`` is considered a new API type + 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?** @@ -359,9 +291,8 @@ provider?** * **Will enabling / using this feature result in increasing size or count of the existing API objects?** - - API type(s): NetworkPolicy / NetworkPolicyPorts - - Estimated increase in size: New struct inside the object with two fields of - 16 bits each + 16 bits for each port in the ``Except`` array + - 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 @@ -432,3 +363,21 @@ type NetworkPolicyPort struct { 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 +``` diff --git a/keps/sig-network/2079-network-policy-port-range/kep.yaml b/keps/sig-network/2079-network-policy-port-range/kep.yaml index cf65f8b798c..aa74b888691 100644 --- a/keps/sig-network/2079-network-policy-port-range/kep.yaml +++ b/keps/sig-network/2079-network-policy-port-range/kep.yaml @@ -29,7 +29,7 @@ milestone: # The following PRR answers are required at alpha release # List the feature gate name and the components for which it must be enabled feature-gates: - - name: NetworkPolicyPortRange + - name: NetworkPolicyEndPort components: - kube-apiserver disable-supported: true From a4e7dae592a3d4323ca3ff028a00e68001e97696 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Mon, 23 Nov 2020 09:42:26 -0300 Subject: [PATCH 11/13] Change endport to int32 Signed-off-by: Ricardo Pchevuzinske Katz --- keps/sig-network/2079-network-policy-port-range/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index fff609a75eb..2beb34ed240 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -164,10 +164,10 @@ type NetworkPolicyPort struct { // +optional Port *intstr.IntOrString `json:"port,omitempty" protobuf:"bytes,2,opt,name=port"` - // To defines the end of the port range, being the end included within the - // range - // +optional - EndPort uint16 `json:"port,omitempty" protobuf:"bytes,2,opt,name=endPort"` + // To defines the end of the port range, being the end included within the + // range + // +optional + EndPort int32 `json:"port,omitempty" protobuf:"bytes,2,opt,name=endPort"` } ``` From fc03f7daf5e61751eb009d3eaa05664839aac416 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Mon, 30 Nov 2020 19:01:01 -0300 Subject: [PATCH 12/13] Correct some typos in Design Details Signed-off-by: Ricardo Pchevuzinske Katz --- keps/sig-network/2079-network-policy-port-range/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index 2beb34ed240..bcd12954520 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -164,10 +164,10 @@ type NetworkPolicyPort struct { // +optional Port *intstr.IntOrString `json:"port,omitempty" protobuf:"bytes,2,opt,name=port"` - // To defines the end of the port range, being the end included within the - // range - // +optional - EndPort int32 `json:"port,omitempty" protobuf:"bytes,2,opt,name=endPort"` + // EndPort defines the end of the port range, being the end included within the + // range + // +optional + EndPort int32 `json:"port,omitempty" protobuf:"bytes,2,opt,name=endPort"` } ``` From d180bf755353415b30377e8e1bb5daa4719cdfa8 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Wed, 9 Dec 2020 00:49:05 -0300 Subject: [PATCH 13/13] Correct some more parts of the Kep Signed-off-by: Ricardo Pchevuzinske Katz --- .../2079-network-policy-port-range/README.md | 53 ++++++++++--------- .../2079-network-policy-port-range/kep.yaml | 2 +- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/keps/sig-network/2079-network-policy-port-range/README.md b/keps/sig-network/2079-network-policy-port-range/README.md index bcd12954520..f7389fa0c16 100644 --- a/keps/sig-network/2079-network-policy-port-range/README.md +++ b/keps/sig-network/2079-network-policy-port-range/README.md @@ -50,7 +50,7 @@ Items marked with (R) are required *prior to targeting to a milestone / release* ## Summary -Today the ``ports``field in ingress and egress network policies is an array +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. @@ -87,25 +87,25 @@ spec: So for the user: * To allow a range of ports, each of them must be declared as an item from -``ports`` array +`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 +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`` +Add an endPort field in `NetworkPolicyPort` ### Non-Goals -* Support specific ``Exception`` field. -* Support ``endPort`` when the starting ``port`` is a named port. +* 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 +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 @@ -150,7 +150,7 @@ of the new field. ## Design Details API changes to NetworkPolicy: -* Add a new field called ``EndPort`` inside ``NetworkPolicyPort`` as the following: +* Add a new field called `EndPort` inside `NetworkPolicyPort` as the following: ``` // NetworkPolicyPort describes a port to allow traffic on type NetworkPolicyPort struct { @@ -159,22 +159,25 @@ type NetworkPolicyPort struct { // +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. + // 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 end of the port range, being the end included within the - // range + // 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 ``EndPort`` cannot be specified +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 @@ -195,15 +198,15 @@ validation should be done by CNIs. - Add validation tests in API #### Beta -- ``EndPort`` has been supported for at least 1 minor release +- `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 +- 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 @@ -211,7 +214,7 @@ 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. +start working incorrectly. This is a fail-closed failure, so it is acceptable. ## Production Readiness Review Questionnaire @@ -232,7 +235,7 @@ _This section must be completed when targeting alpha to a release._ 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 + 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?** @@ -282,7 +285,7 @@ previous answers based on experience in the field._ TBD * **Will enabling / using this feature result in introducing new API types?** - No, unless the new ``EndPort`` is considered a new API type + 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?** @@ -292,7 +295,7 @@ provider?** the existing API objects?** - API type(s): NetworkPolicyPorts - - Estimated increase in size: 2 bytes for each new ``EndPort`` specified + - 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 @@ -336,7 +339,7 @@ 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: +of the `NetworkPolicyPortRange` field inside the `NetworkPolicyPort` as the following: ``` // NetworkPolicyPort describes a port or a range of ports to allow traffic on @@ -363,7 +366,7 @@ type NetworkPolicyPort struct { 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 +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: diff --git a/keps/sig-network/2079-network-policy-port-range/kep.yaml b/keps/sig-network/2079-network-policy-port-range/kep.yaml index aa74b888691..5dfcdaaa345 100644 --- a/keps/sig-network/2079-network-policy-port-range/kep.yaml +++ b/keps/sig-network/2079-network-policy-port-range/kep.yaml @@ -10,7 +10,7 @@ reviewers: - "@abhiraut" - "@danwinship" approvers: - - TBD + - "@thockin" # The target maturity stage in the current dev cycle for this KEP. stage: alpha