From 14e9179c6a6a9bf727f785fb12d7e122543aca4c Mon Sep 17 00:00:00 2001
From: Nadia Pinaeva <npinaeva@redhat.com>
Date: Mon, 20 Nov 2023 10:14:45 +0100
Subject: [PATCH] Add Tenancy API design suggestions.

Signed-off-by: Nadia Pinaeva <npinaeva@redhat.com>
---
 npeps/npep-122.md | 144 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 142 insertions(+), 2 deletions(-)

diff --git a/npeps/npep-122.md b/npeps/npep-122.md
index ab67829a..5445b664 100644
--- a/npeps/npep-122.md
+++ b/npeps/npep-122.md
@@ -1,7 +1,7 @@
 # NPEP-122: Tenancy API
 
 * Issue: [#122](https://github.com/kubernetes-sigs/network-policy-api/issues/122)
-* Status: Provisional
+* Status: Implementable
 
 ## TLDR
 
@@ -158,7 +158,147 @@ Fourth, the ANP subject allows using pod selectors, while tenancy use cases only
 
 ## API
 
-TBD
+### Tenant definition
+
+We define Tenant as a set of namespaces.
+All Namespaces affected by Tenancy will be split into Tenants based on their label(s), we call these labels `tenancyLabels`. 
+A Tenant is identified by values of `tenancyLabels`, which are shared by all namespaces in the Tenant.
+
+There are 2 ways to select which namespaces should be affected by Tenancy:
+
+1. Separate from `tenancyLabels`. We have a separate label selector to define which namespaces should be affected by Tenancy.
+Then selected namespaces are split into Tenants.
+
+    **Cons**: selected namespace may not have some of the `tenancyLabels`, which will likely result in introducing "None" value
+    for `tenancyLabels`. To only select namespaces with existing labels extra `key: X, operator: Exists` condition should
+    be added to the selector.
+
+2. Joined with `tenancyLabels`. We select all namespace that all `tenancyLabels` present.
+
+    **Cons**: applying all labels from `tenancyLabels` to a namespaces automatically makes it affected by tenancy.
+    No way to make sure ALL namespaces in a cluster are a part of some tenant, e.g. newly created namespace will not
+    be limited/protected by Tenancy rules until it gets `tenancyLabels` assigned, may potentially be a security problem.
+    
+    Example: by setting deny-from-other-tenants I want to ensure only namespaces with the same label `security-zone` value
+    can send incoming connections. That policy will be violated if a new tenant is created without `security-zone` label,
+    meaning it is not a part of any Tenant. This may potentially be solved by using allow-from-same-tenant + deny ALL
+    ANP, but it is stricter, since it affects all incoming connections, not just from other pods.
+
+### Peers and actions
+
+Based on the existing User Stories, Tenancy only needs Allow and Deny actions, and the only 2 types of peers are
+`SameTenant` and `NotSameTenant`.
+
+Therefore, Tenancy rules will look something like:
+
+```yaml
+kind: NetworkTenancy
+spec:
+  tenancyLabels: ['label1']
+  ingress:
+    - actions: Deny
+      from: NotSameTenant
+    - actions: Allow
+      from: SameTenant
+  egress:
+    - actions: Deny
+      to: NotSameTenant
+```
+
+### Priorities
+
+Based on User Story 4.4, we need to have Tenancy in the same priority range as ANP and BANP. There are multiple ways to do so:
+
+1. Reuse ANP and BANP to define priority, link Tenancy API
+
+```yaml
+kind: AdminNetworkPolicy
+spec:
+  priority: 10
+  # EITHER
+  subject: <...>
+  ingress: <...>
+  egress: <...>
+  # OR
+  tenant: <tenant reference>
+```
+
+```yaml
+kind: BaselineAdminNetworkPolicy
+spec:
+  # EITHER
+  subject: <...>
+  ingress: <...>
+  egress: <...>
+  # OR
+  tenant: <tenant reference>
+```
+
+We may want to allow either `subject`+`ingress`/`egress` or `tenant`, since having both at the same priority
+is confusing (`tenancyLabels` and `subject` may select unrelated sets of pods) and may result in conflicting rules.
+
+2. Create 2 objects with ANP and BANP priorities (let's say Tenancy and BaselineTenancy)
+
+```yaml
+kind: NetworkTenancy
+spec:
+  priority: 10
+  ingress:
+    - actions: Deny
+      from: NotSameTenant
+    - actions: Allow
+      from: SameTenant
+  egress:
+    - actions: Deny
+      to: NotSameTenant
+```
+
+```yaml
+kind: BaselineNetworkTenancy
+spec:
+  ingress:
+    - actions: Deny
+      from: NotSameTenant
+    - actions: Allow
+      from: SameTenant
+  egress:
+    - actions: Deny
+      to: NotSameTenant
+```
+
+While multiple ANPs with the same priority are allowed, we probably can allow multiple Tenancies or Tenancy and ANP
+with the same priority, but if we decide to only allow ANP per priority, Tenancy needs to be accounted for in the same range.
+
+3. Create 1 object with extra `baselinePolicy` field or IntOrString value for priority.
+
+```yaml
+kind: NetworkTenancy
+spec:
+  baselinePolicy: false
+  priority: 10
+  ingress:
+    - actions: Deny
+      from: NotSameTenant
+    - actions: Allow
+      from: SameTenant
+  egress:
+    - actions: Deny
+      to: NotSameTenant
+```
+
+```yaml
+kind: NetworkTenancy
+spec:
+  priority: baseline
+  ingress:
+    - actions: Deny
+      from: NotSameTenant
+    - actions: Allow
+      from: SameTenant
+  egress:
+    - actions: Deny
+      to: NotSameTenant
+```
 
 ## Conformance Details