diff --git a/npeps/npep-122.md b/npeps/npep-122.md index ab67829a..f9c16690 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 @@ -88,11 +88,21 @@ and be totally independent of the other tenants. We can write it down like this may own more than 1 Namespace. #### Story 4.3: Allow internal connections for tenants + + BANP + As a cluster admin, I want to setup an overridable deny-all policy to protect namespaces by default. + At the same time I want to build tenants in my cluster and allow connections inside one tenant by default. - As a cluster admin, I want to build tenants in my cluster and always allow connections inside one tenant. - At the same time I want to setup an overridable deny-all policy to protect namespaces by default. - This policy should make sure internal connectivity for a tenant is always allowed, in case there are - lower-priority deny rules. + ANP + As a cluster admin, I want to setup a deny-all policy to only allow connections that are explicitly specified. + Besides allowing required cluster services (like kube-api, dns, etc.) with ANP, I want to build tenants + and allow connections inside one tenant. + +Both user stories have zero-trust policy in mind, where every allowed connection should be explicitly specified, +and everything else is denied. It may be set up as "by default"/overridable/BANP or strict/ANP. +Allow connection inside one tenant in this context means skip deny rules for same tenant, and delegate same-tenant +policies to the namespaces NetworkPolicy. We assume that there is no reason for cluster admin to forcefully allow connections +inside one tenant instead of delegating to NetworkPolicy. #### Story 4.4: Tenants interaction with (B)ANP @@ -105,7 +115,6 @@ and be totally independent of the other tenants. We can write it down like this #### What I couldn't figure out user stories for -- Skip action - Ports *[]AdminNetworkPolicyPort ### Existing API @@ -158,7 +167,403 @@ Fourth, the ANP subject allows using pod selectors, while tenancy use cases only ## API -TBD +### Tenant definition + +For the purposes of this NPEP we define a Tenant as a set of namespaces. +`tenancyLabels` is a set of label keys, based on which all Namespaces affected by Tenancy are be split into Tenants. +A Tenant is identified by values of `tenancyLabels`, which are shared by all namespaces in a given Tenant. + +There are 2 ways to select which namespaces should be affected by Tenancy rules: + +1. Use a distinct new `namespaceSelector` field to define which namespaces should be affected by Tenancy, +while the `tenancyLabels` field can be used to define how the 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`. It brings more complexity and is not required for any of the listed use cases. + +```yaml +spec: + matchExpression: + - key: system-namespace + operator: DoesNotExist + - key: user + operator: Exists + tenancyLabels: + - user +``` + +2. Overload `tenancyLabels` and implicitly apply tenancy rules only to namespaces where `tenancyLabels` are present. + + **Cons**: the simplest option that covers all given use cases. + +```yaml +spec: + tenancyLabels: + - user +``` + +### Peers and actions + +Based on the existing User Stories, Tenancy only needs action to "pass same tenant" and "deny not same tenant". +Using both actions at the same time doesn't make a lot of sense, since "pass same tenant" action only makes sense if +there is a (B)ANP that will deny tenancy connections. Otherwise, "deny not same tenant" is sufficient. + +Tenancy rules don't need to specify separate rules for ingress and egress, because +- for `SameTenant` if ingress is denied, then egress to the same tenant may be allowed when leaving the source pod, but will + be denied by ingress rule when coming to the destination pod. The same applies to egress. +- for `NotSameTenant` if ingress is denied, then egress from `Tenant1` to `Tenant2` will be allowed by `Tenant1`, but + considered ingress from another tenant by `Tenant2`, and denied. The same applies to egress. + +It means that deny in at least one direction automatically denies the other direction. +Therefore, the only extra parameter Tenancy needs is priority/precedence, and Tenancy rules may look something like: + +```yaml +spec: + action: PassSameTenant/DenyNotSameTenant +``` + +### 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, insert Tenancy rules to `ingress` and `egress`. +Current (B)ANP semantics is: `subject` selects pod to which all `ingress` and `egress` rules are applied. +Tenancy can't be used in this semantics, because it has its own "subject" defined by the tenancy labels. +There are multiple ways to "turn on" tenancy mode in B(ANP) and allow tenancy rules. + +Exclusive fields `subject`/`tenancySubject` +```yaml +kind: AdminNetworkPolicy +spec: + # this field turns on tenancy + tenancySubject: + labels: ["user"] + ingress: + - action: Allow + from: + - SameTenant + - pods: + podSelector: {} +``` + +A similar, but slightly different option with pushing `tenantNamespace` as an alternative subject selector to existing +`pods` and `namespaces` +```yaml +kind: AdminNetworkPolicy +spec: + # this field turns on tenancy + subject: + tenantNamespaces: + labels: ["user"] + ingress: + - action: Allow + from: + - SameTenant + - pods: + podSelector: {} +``` +**CONS** +- "switch" that enables some peer types is not the best API design +- gives more flexibility than intended (multiple tenancy definitions) +- conflicts with singleton BANP, meaning that if Tenancy is defined on BANP level, general-purpose BANP selecting different +pods can't be created. + +2. Reuse ANP and BANP to define priority, insert Tenancy definition and Rules to `ingress` and `egress`. + +It splits the tenancy labels and namespace selector which defines the namespace that are affected by the tenancy. + +```yaml +kind: AdminNetworkPolicy +spec: + priority: 10 + subject: + namespaces: + matchLabels: + kubernetes.io/metadata.name: sensitive-ns + ingress: + - action: Allow + from: + - tenant: + labels: ["user"] + tenant: Same +``` + +**CONS** complicates selection or affected namespaces, allows strange configuration, where namespaces selected by +`subject` have no tenancy labels. Has the same problem as the original design. + +3. Create 2 objects with ANP and BANP priorities (let's say TenancyNetworkPolicy and BaselineTenancyNetworkPolicy) + +```yaml +kind: TenancyNetworkPolicy +spec: + priority: 10 + tenancyLabels: ["user"] + action: PassSameTenant +--- +kind: BaselineTenancyNetworkPolicy +spec: + priority: 10 + tenancyLabels: ["user"] + action: PassSameTenant +``` + +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. + +**CONS**: BANP doesn't have a priority, to use this method we would need to define a priority for BANP. + +4. Create 1 new object with implicit priorities. + +`precedence` field + reserved highest-priority rule before (B)ANP +Similar to the previous one, but a bit more flexible: +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: ["user"] + precedence: ANP/BANP + action: PassSameTenant +``` +**CONS** +- Priorities are implicit and need to be added as extra layers between ANP/NP/BANP. +- Doesn't follow current naming scheme, where ANP and BANP are separate objects. +**PROS** +- No changes to the existing ANP/BANP objects +- Limited to the use cases we designed it for (smaller chance to shoot yourself in a foot) +- Users that don't care about tenancy can just ignore this CRD +- We can throw it away if we want to change API again + +4.1 Create 2 new objects with implicit priorities. + +Same as option 4, but with separate objects for ANP and BANP. +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: ["user"] + action: PassSameTenant +--- +kind: BaselineTenancyNetworkPolicy +spec: + tenancyLabels: ["user"] + action: PassSameTenant +``` + +Shares most PROS and CONS with 4, except for +**CONS** +- Required 2 new CRDs with just 2? fields +**PROS** +- Follows existing naming scheme where ANP and BANP are separate objects + + +#### Final suggestion + +Considering we agree with option 4 from the previous section on priorities specification, we can outline further details here. + +For +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: ["user"] + precedence: ANP/BANP + action: PassSameTenant/DenyNotSameTenant +``` + +To implement user story 4.3, Tenancy rules should have higher priority than ANP/BANP. +Considering the following priority precedence: ANP Tenancy->ANP->NP->BANP Tenancy->BANP, we can express all mentioned user stories. + +
+Full yaml examples (with the initial fields, will be updates as we agree on the final CRD format) + +* 4.1 "overridable isolation" +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: + - "user" + precedence: BANP + action: DenyNotSameTenant +``` +OR (second option may be more useful if there is a deny BANP in the cluster) +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: + - "user" + precedence: BANP + action: PassSameTenant +--- +kind: BaselineAdminNetworkPolicy +spec: + subject: + namespaces: + matchExpression: + - key: user + operator: Exists + ingress: + - action: Deny + from: + - namespaces: {} + egress: + - action: Deny + to: + - namespaces: {} +``` +BANP can also be replaced with deny-all BANP +```yaml +kind: BaselineAdminNetworkPolicy +spec: + subject: + namespaces: {} + ingress: + - action: Deny + from: + - namespaces: {} + egress: + - action: Deny + to: + - namespaces: {} +``` + +* 4.2 strict isolation +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: + - "user" + precedence: ANP + action: DenyNotSameTenant +``` +OR (second option may be more useful if there is a deny ANP in the cluster) +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: + - "user" + precedence: ANP + action: PassSameTenant +--- +kind: AdminNetworkPolicy +spec: + priority: 1 + subject: + namespaces: + matchExpression: + - key: user + operator: Exists + ingress: + - action: Deny + from: + - namespaces: {} + egress: + - action: Deny + to: + - namespaces: {} +``` + +* 4.3 Allow internal connections for tenants +BANP-level +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: + - "user" + precedence: BANP + action: PassSameTenant +--- +kind: BaselineAdminNetworkPolicy +spec: + subject: + namespaces: + matchExpression: + - key: user + operator: Exists + ingress: + - action: Deny + from: + - namespaces: {} + egress: + - action: Deny + to: + - namespaces: {} +``` + +ANP-level +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: + - "user" + precedence: ANP + action: PassSameTenant +--- +kind: AdminNetworkPolicy +spec: + priority: 1 + subject: + namespaces: + matchExpression: + - key: user + operator: Exists + ingress: + - action: Deny + from: + - namespaces: {} + egress: + - action: Deny + to: + - namespaces: {} +``` + +* 4.4 Tenants interaction with (B)ANP +* 4.4.1 allow from monitoring + deny from not same tenant +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: + - "user" + precedence: ANP + action: PassSameTenant +--- +kind: AdminNetworkPolicy +spec: + priority: 1 + subject: + namespaces: + matchExpression: + - key: user + operator: Exists + ingress: + - action: Allow + from: + - namespaces: + namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: monitoring-ns + - action: Deny + from: + - namespaces: {} +``` +* 4.4.2 allow from same tenant + BANP deny all +```yaml +kind: TenancyNetworkPolicy +spec: + tenancyLabels: + - "user" + precedence: BANP + action: PassSameTenant +--- +kind: BaselineAdminNetworkPolicy +spec: + subject: + namespaces: + matchExpression: + - key: user + operator: Exists + ingress: + - action: Deny + from: + - namespaces: {} +``` +
## Conformance Details