Add NPEP for new CIDRGroup object peer
#183
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| # NPEP-182: Add new CIDR object peer for northbound traffic | ||
|
|
||
| * Issue: [#182](https://github.com/kubernetes-sigs/network-policy-api/issues/182) | ||
| * Status: Provisional | ||
|
|
||
| ## Co-Authors | ||
| @joestringer and @networkop for raising relevant user stories | ||
|
|
||
| ## TLDR | ||
|
|
||
| This NPEP proposes adding support for a new CIDRGroup object peer type for | ||
| cluster egress (northbound) traffic control that can be referred in the | ||
| `AdminNetworkPolicy` and `BaselineAdminNetworkPolicy` API objects using selectors. | ||
| [NPEP-126](https://network-policy-api.sigs.k8s.io/npeps/npep-126-egress-traffic-control/#implementing-egress-traffic-control-towards-cidrs) | ||
| already adds support for inline CIDR peer type directly on the | ||
| `AdminNetworkPolicy` and `BaselineAdminNetworkPolicy` API objects. This NPEP proposes | ||
| adding more extensibility by introducing a new CIDRGroup object in addition to the | ||
| inline CIDR peers so that users can choose either of these methods based on their needs. | ||
|
|
||
| ## Goals | ||
|
|
||
| * Provide users with a way to group their CIDRs in a meaningful | ||
| manner which can then be referred to from ANP and BANP objects. | ||
|
|
||
| ## Non-Goals | ||
|
|
||
| ## Introduction | ||
|
|
||
| The current approach of defining inline CIDR peers works well | ||
| if the number of CIDR blocks involved in defining policies are | ||
| less in number and mostly static in nature. However in environments | ||
| where we could have a more dynamic setup, the management of inline CIDR | ||
| peers gets more tricker an cumbersome. In such cases having a way to | ||
| group CIDR blocks together to represent an entity or a group of | ||
| entities which the policy can refer to as a network peer can be useful. | ||
| This also ensures reference of same CIDR group peer from ANP and BANP | ||
| stays consistent and any changes to the list of CIDR blocks only involves | ||
| editing the object itself and not the rules in the policy. | ||
|
|
||
| ### User Stories for CIDRGrouping | ||
|
|
||
| * As a cluster admin I want to be able to create admin network policies that | ||
| match a dynamic set of external IPs (e.g. set of VMs or set of directly reachable | ||
| Pods in another cluster). I may not be able to use FQDN rules for that due to | ||
| TTL being too long or simply lack of DNS service discovery in an external system. | ||
| As a cluster admin, I would create CIDR group resource and a BGP controller that | ||
| would manage it. The mapping between BGP communities and CIDR group resource names | ||
| is a BGP controller configuration (e.g. annotation on the CIDR group resource). | ||
| The speed of IP churn is bounded by the BGP advertisement interval and can be | ||
| further reduced by the BGP controller implementation. | ||
|
|
||
| * As a cluster administrator I want to to ensure that pods can reach | ||
| commonly-used databases under my control but outside Kubernetes. Many but | ||
| not all applications in my environment rely on these databases. I want to | ||
| delegate writing network policy for this traffic to namespace owners. | ||
|
|
||
| Example: As a cluster administrator I define a CIDR group that defines | ||
| a set of RDS instances that is used across multiple apps. The owners of | ||
| namespaceA and namespaceB can then define policies that allow traffic to | ||
| this group of RDS instances, and they reference the instances by CIDR group. | ||
| As a cluster administrator I can migrate the database infrastructure and | ||
| update the CIDR group independently of the namespace owners. The applications | ||
| in namespaceC do not use this infrastructure, so the cluster administrator | ||
| and the owners of namespaceC do not need to think about network policy | ||
| for apps in namespaceC. | ||
|
|
||
| NOTE: Second use case is not possible today using NetworkPolicy resource | ||
| since we only have `ipBlocks` as a peer however this is definitely a useful | ||
| case to keep in mind for having a CIDR Group. | ||
|
|
||
|
There was a problem hiding this comment. Calico has a two similar resources:
The feature is popular. It's much nicer to write policies as I think the namespaced version of the Calico resource came about to allow tenants/teams to define these resources within their own namespace. Often a particular team has some particular external servers that they need to access and it become unmanageable to go through the platform owner to manage those network sets. Arguably, a mistake we made in our model was to put NetowrkSets in the same selector scope as Pods in the namespace. This proposal makes it explicit what you're matching, which I think is better. Folks expect "allow from all within namespace" to mean "allow all my workloads", not "allow all my workloads and network sets". |
||
| ## API | ||
|
|
||
| This NPEP Proposes to add a new `CIDRGroup` object: | ||
|
|
||
| ``` | ||
| // CIDRGroup defines a group of CIDR blocks that can be referred to from | ||
| // AdminNetworkPolicy and BaselineAdminNetworkPolicy resources. | ||
| // +kubebuilder:validation:MaxProperties=1 | ||
| // +kubebuilder:validation:MinProperties=1 | ||
| type CIDRGroup struct { | ||
| // cidrs is the list of network cidrs that can be used to define destinations. | ||
| // A total of 25 CIDRs will be allowed in each CIDRGroup instance. | ||
| // ANP & BANP APIs may use the .spec.egress.to.networks.cidrGroups selector | ||
| // to select a set of cidrGroups. | ||
| // | ||
| // +optional | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=25 | ||
| cidrs []CIDR `json:"cidrs,omitempty" | ||
| } | ||
| ``` | ||
|
|
||
| In order to ensure it is coexisting with inline CIDR peers without confusion, | ||
| we propose to change the type of `networks` peer from `string` to a struct of type | ||
| `NetworkPeer`: | ||
|
Comment on lines
+93
to
+95
There was a problem hiding this comment. Given the API is headed for beta, would this breaking change be OK? I'd have thought we want to add a new top-level peer type to make it easier to adopt in non-breaking fashion. |
||
|
|
||
| ``` | ||
| // +kubebuilder:validation:MaxProperties=1 | ||
| // +kubebuilder:validation:MinProperties=1 | ||
| type NetworkPeer struct { | ||
| // cidrs represents a list of CIDR blocks | ||
| // | ||
| // +optional | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=25 | ||
| CIDRs []CIDR `json:"cidrs,omitempty" | ||
| // cidrGroups defines a way to select cidrGroup objects | ||
| // that consist of network CIDRs as a peer. | ||
| // This field follows standard label selector semantics; if present | ||
| // but empty, it selects all cidrGroups defined in the cluster. | ||
| // | ||
| // +optional | ||
| CIDRGroups *metav1.LabelSelector `json:"cidrGroups,omitempty" | ||
| } | ||
| ``` | ||
|
|
||
| and this is referenced from an ANP or BANP Egress Peer in the following | ||
| manner: | ||
|
|
||
| ``` | ||
| type AdminNetworkPolicyEgressPeer struct { | ||
| <snipped> | ||
| // Networks defines a way to select peers via CIDR blocks. This is | ||
| // intended for representing entities that live outside the cluster, | ||
| // which can't be selected by pods and namespaces peers, but note | ||
| // that cluster-internal traffic will be checked against the rule as | ||
| // well, so if you Allow or Deny traffic to `"0.0.0.0/0"`, that will allow | ||
| // or deny all IPv4 pod-to-pod traffic as well. If you don't want that, | ||
| // add a rule that Passes all pod traffic before the Networks rule. | ||
| // | ||
| // Support: Core | ||
| // | ||
| // +optional | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=100 | ||
| Networks []NetworkPeer `json:"networks,omitempty" | ||
| } | ||
| ``` | ||
|
|
||
| Define a `CIDRGroup` object, example: | ||
|
|
||
| ``` | ||
| apiVersion: policy.networking.k8s.io/v1alpha1 | ||
| kind: CIDRGroup | ||
| metadata: | ||
| name: cluster-wide-cidr-cloud-1 | ||
| labels: | ||
| env: cloud-1 | ||
| annotations: | ||
| "bgp.cidrmanager.k8s.io/is-managed": "true" | ||
| "bgp.cidrmanager.k8s.io/32bit-community": "2147483647" | ||
| spec: | ||
| cidrs: | ||
| - 192.0.2.0/24 | ||
| - 203.0.113.0/24 | ||
| - 198.51.100.0/24 | ||
| status: | ||
| conditions: | ||
| - lastTransitionTime: "2022-12-29T14:53:50Z" | ||
| status: "True" | ||
| type: Reconciled | ||
| ``` | ||
|
|
||
| Then refer to this object from an ANP: | ||
|
|
||
| ``` | ||
| apiVersion: policy.networking.k8s.io/v1alpha1 | ||
| kind: AdminNetworkPolicy | ||
| metadata: | ||
| name: networks-peer-example | ||
| spec: | ||
| priority: 30 | ||
| subject: | ||
| namespaces: {} | ||
| egress: | ||
| - action: Allow | ||
| to: | ||
| - networks: | ||
| cidrGroups: | ||
| matchLabels: | ||
| env: cloud-1 | ||
| - action: Deny | ||
| to: | ||
| - networks: | ||
| cidrs: | ||
| - 0.0.0.0/0 | ||
| ``` | ||
|
|
||
| ## Alternatives | ||
|
|
||
| N/A | ||
|
|
||
| ## References | ||
|
|
||
| See https://github.com/kubernetes-sigs/network-policy-api/pull/144#discussion_r1408175206 for details | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| # NPEP-182: Add new CIDR object peer for northbound traffic | ||
|
|
||
| * Issue: [#182](https://github.com/kubernetes-sigs/network-policy-api/issues/182) | ||
| * Status: Provisional | ||
|
|
||
| ## Co-Authors | ||
| @joestringer and @networkop for raising relevant user stories | ||
|
|
||
| ## TLDR | ||
|
|
||
| This NPEP proposes adding support for a new CIDRGroup object peer type for | ||
| cluster egress (northbound) traffic control that can be referred in the | ||
| `AdminNetworkPolicy` and `BaselineAdminNetworkPolicy` API objects using selectors. | ||
| [NPEP-126](https://network-policy-api.sigs.k8s.io/npeps/npep-126-egress-traffic-control/#implementing-egress-traffic-control-towards-cidrs) | ||
| already adds support for inline CIDR peer type directly on the | ||
| `AdminNetworkPolicy` and `BaselineAdminNetworkPolicy` API objects. This NPEP proposes | ||
| adding more extensibility by introducing a new CIDRGroup object in addition to the | ||
| inline CIDR peers so that users can choose either of these methods based on their needs. | ||
|
|
||
| ## Goals | ||
|
|
||
| * Provide users with a way to group their CIDRs in a meaningful | ||
| manner which can then be referred to from ANP and BANP objects. | ||
|
|
||
| ## Non-Goals | ||
|
|
||
| ## Introduction | ||
|
|
||
| The current approach of defining inline CIDR peers works well | ||
| if the number of CIDR blocks involved in defining policies are | ||
| less in number and mostly static in nature. However in environments | ||
| where we could have a more dynamic setup, the management of inline CIDR | ||
| peers gets more tricker an cumbersome. In such cases having a way to | ||
| group CIDR blocks together to represent an entity or a group of | ||
| entities which the policy can refer to as a network peer can be useful. | ||
| This also ensures reference of same CIDR group peer from ANP and BANP | ||
| stays consistent and any changes to the list of CIDR blocks only involves | ||
| editing the object itself and not the rules in the policy. | ||
|
|
||
| ### User Stories for CIDRGrouping | ||
|
|
||
| * As a cluster admin I want to be able to create admin network policies that | ||
| match a dynamic set of external IPs (e.g. set of VMs or set of directly reachable | ||
| Pods in another cluster). I may not be able to use FQDN rules for that due to | ||
| TTL being too long or simply lack of DNS service discovery in an external system. | ||
| As a cluster admin, I would create CIDR group resource and a BGP controller that | ||
| would manage it. The mapping between BGP communities and CIDR group resource names | ||
| is a BGP controller configuration (e.g. annotation on the CIDR group resource). | ||
| The speed of IP churn is bounded by the BGP advertisement interval and can be | ||
| further reduced by the BGP controller implementation. | ||
|
|
||
| * As a cluster administrator I want to to ensure that pods can reach | ||
| commonly-used databases under my control but outside Kubernetes. Many but | ||
| not all applications in my environment rely on these databases. I want to | ||
| delegate writing network policy for this traffic to namespace owners. | ||
|
|
||
| Example: As a cluster administrator I define a CIDR group that defines | ||
| a set of RDS instances that is used across multiple apps. The owners of | ||
| namespaceA and namespaceB can then define policies that allow traffic to | ||
| this group of RDS instances, and they reference the instances by CIDR group. | ||
| As a cluster administrator I can migrate the database infrastructure and | ||
| update the CIDR group independently of the namespace owners. The applications | ||
| in namespaceC do not use this infrastructure, so the cluster administrator | ||
| and the owners of namespaceC do not need to think about network policy | ||
| for apps in namespaceC. | ||
|
|
||
| NOTE: Second use case is not possible today using NetworkPolicy resource | ||
| since we only have `ipBlocks` as a peer however this is definitely a useful | ||
| case to keep in mind for having a CIDR Group. | ||
|
|
||
| ## API | ||
|
|
||
| This NPEP Proposes to add a new `CIDRGroup` object: | ||
|
|
||
| ``` | ||
| // CIDRGroup defines a group of CIDR blocks that can be referred to from | ||
| // AdminNetworkPolicy and BaselineAdminNetworkPolicy resources. | ||
| // +kubebuilder:validation:MaxProperties=1 | ||
| // +kubebuilder:validation:MinProperties=1 | ||
| type CIDRGroup struct { | ||
| // cidrs is the list of network cidrs that can be used to define destinations. | ||
| // A total of 25 CIDRs will be allowed in each CIDRGroup instance. | ||
| // ANP & BANP APIs may use the .spec.egress.to.networks.cidrGroups selector | ||
| // to select a set of cidrGroups. | ||
| // | ||
| // +optional | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=25 | ||
| cidrs []CIDR `json:"cidrs,omitempty" | ||
| } | ||
| ``` | ||
|
|
||
| In order to ensure it is coexisting with inline CIDR peers without confusion, | ||
| we propose to change the type of `networks` peer from `string` to a struct of type | ||
| `NetworkPeer`: | ||
|
|
||
| ``` | ||
| // +kubebuilder:validation:MaxProperties=1 | ||
| // +kubebuilder:validation:MinProperties=1 | ||
| type NetworkPeer struct { | ||
| // cidrs represents a list of CIDR blocks | ||
| // | ||
| // +optional | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=25 | ||
| CIDRs []CIDR `json:"cidrs,omitempty" | ||
| // cidrGroups defines a way to select cidrGroup objects | ||
| // that consist of network CIDRs as a peer. | ||
| // This field follows standard label selector semantics; if present | ||
| // but empty, it selects all cidrGroups defined in the cluster. | ||
| // | ||
| // +optional | ||
| CIDRGroups *metav1.LabelSelector `json:"cidrGroups,omitempty" | ||
| } | ||
| ``` | ||
|
|
||
| and this is referenced from an ANP or BANP Egress Peer in the following | ||
| manner: | ||
|
|
||
| ``` | ||
| type AdminNetworkPolicyEgressPeer struct { | ||
| <snipped> | ||
| // Networks defines a way to select peers via CIDR blocks. This is | ||
| // intended for representing entities that live outside the cluster, | ||
| // which can't be selected by pods and namespaces peers, but note | ||
| // that cluster-internal traffic will be checked against the rule as | ||
| // well, so if you Allow or Deny traffic to `"0.0.0.0/0"`, that will allow | ||
| // or deny all IPv4 pod-to-pod traffic as well. If you don't want that, | ||
| // add a rule that Passes all pod traffic before the Networks rule. | ||
| // | ||
| // Support: Core | ||
| // | ||
| // +optional | ||
| // +kubebuilder:validation:MinItems=1 | ||
| // +kubebuilder:validation:MaxItems=100 | ||
| Networks []NetworkPeer `json:"networks,omitempty" | ||
| } | ||
| ``` | ||
|
|
||
| Define a `CIDRGroup` object, example: | ||
|
|
||
| ``` | ||
| apiVersion: policy.networking.k8s.io/v1alpha1 | ||
| kind: CIDRGroup | ||
| metadata: | ||
| name: cluster-wide-cidr-cloud-1 | ||
| labels: | ||
| env: cloud-1 | ||
| annotations: | ||
| "bgp.cidrmanager.k8s.io/is-managed": "true" | ||
| "bgp.cidrmanager.k8s.io/32bit-community": "2147483647" | ||
| spec: | ||
| cidrs: | ||
| - 192.0.2.0/24 | ||
| - 203.0.113.0/24 | ||
| - 198.51.100.0/24 | ||
| status: | ||
| conditions: | ||
| - lastTransitionTime: "2022-12-29T14:53:50Z" | ||
| status: "True" | ||
| type: Reconciled | ||
| ``` | ||
|
|
||
| Then refer to this object from an ANP: | ||
|
|
||
| ``` | ||
| apiVersion: policy.networking.k8s.io/v1alpha1 | ||
| kind: AdminNetworkPolicy | ||
| metadata: | ||
| name: networks-peer-example | ||
| spec: | ||
| priority: 30 | ||
| subject: | ||
| namespaces: {} | ||
| egress: | ||
| - action: Allow | ||
| to: | ||
| - networks: | ||
| cidrGroups: | ||
| matchLabels: | ||
| env: cloud-1 | ||
| - action: Deny | ||
| to: | ||
| - networks: | ||
| cidrs: | ||
| - 0.0.0.0/0 | ||
| ``` | ||
|
|
||
| ## Alternatives | ||
|
|
||
| N/A | ||
|
|
||
| ## References | ||
|
|
||
| See https://github.com/kubernetes-sigs/network-policy-api/pull/144#discussion_r1408175206 for details |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just playing devil's advocate with maybe a really bad idea:
What if we try to use the EndpointSlice API to solve this set of problems?
Pros:
objectsCRDSCons:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rahulkjoshi can you specify IP prefixes or only IP addresses in EndpointSlice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😦 yeah you're right only individual addresses in EndpointSlice
My underlying thought with this situation was whether there's overlap with a more general service discovery problem. My thinking was that whatever application the rule protects also needs to discover the IPs it's talking to.
I was hoping that plugging into that system might be more ergonomic? But it's more likely to just be incredibly complicated and not easily generalizable.