Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Tenancy API design suggestions.
Browse files Browse the repository at this point in the history
Signed-off-by: Nadia Pinaeva <npinaeva@redhat.com>
npinaeva committed Mar 26, 2024

Verified

This commit was signed with the committer’s verified signature.
Lord-Kamina Gregorio Litenstein
1 parent f6c1cf2 commit 5721fa3
Showing 1 changed file with 389 additions and 7 deletions.
396 changes: 389 additions & 7 deletions npeps/npep-122.md
Original file line number Diff line number Diff line change
@@ -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,380 @@ 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 "skip same tenant" and "deny not same tenant".
Using both actions at the same time doesn't make a lot of sense, since "skip 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: SkipSameTenant/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 NetworkTenancy and BaselineNetworkTenancy)

```yaml
kind: NetworkTenancy
spec:
priority: 10
tenancyLabels: ["user"]
action: SkipSameTenant
---
kind: BaselineNetworkTenancy
spec:
priority: 10
tenancyLabels: ["user"]
action: SkipSameTenant
```

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: NetworkTenancy
spec:
tenancyLabels: ["user"]
precedence: ANP/BANP
action: SkipSameTenant
```
**CONS** Priorities are implicit and need to be added as extra layers between ANP/NP/BANP.
**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


#### Final suggestion

Considering we agree with option 4 from the previous section on priorities specification, we can outline further details here.

For
```yaml
kind: NetworkTenancy
spec:
tenancyLabels: ["user"]
precedence: ANP/BANP
action: SkipSameTenant/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.

<details>
<summary>Full yaml examples (with the initial fields, will be updates as we agree on the final CRD format)</summary>

* 4.1 "overridable isolation"
```yaml
kind: NetworkTenancy
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: NetworkTenancy
spec:
tenancyLabels:
- "user"
precedence: BANP
action: SkipSameTenant
---
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: NetworkTenancy
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: NetworkTenancy
spec:
tenancyLabels:
- "user"
precedence: ANP
action: SkipSameTenant
---
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: NetworkTenancy
spec:
tenancyLabels:
- "user"
precedence: BANP
action: SkipSameTenant
---
kind: BaselineAdminNetworkPolicy
spec:
subject:
namespaces:
matchExpression:
- key: user
operator: Exists
ingress:
- action: Deny
from:
- namespaces: {}
egress:
- action: Deny
to:
- namespaces: {}
```

ANP-level
```yaml
kind: NetworkTenancy
spec:
tenancyLabels:
- "user"
precedence: ANP
action: SkipSameTenant
---
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: NetworkTenancy
spec:
tenancyLabels:
- "user"
precedence: ANP
action: SkipSameTenant
---
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: NetworkTenancy
spec:
tenancyLabels:
- "user"
precedence: BANP
action: SkipSameTenant
---
kind: BaselineAdminNetworkPolicy
spec:
subject:
namespaces:
matchExpression:
- key: user
operator: Exists
ingress:
- action: Deny
from:
- namespaces: {}
```
</details>

## Conformance Details

0 comments on commit 5721fa3

Please sign in to comment.