Page MenuHomePhabricator

Create a networkpolicy template allowing charts to define a Calico Network policy to external services
Closed, ResolvedPublic

Description

When https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/984819 is deployed, we will get a kubernetes Service and Endpoints resource for each external service "cluster" we need to configure egress to.
The next step becomes making it easy to define a Calico NetworkPolicy with egress rules pointing at such Service resources, to authorize egress traffic to the associated IPs.

We currently achieve with the following data structure:

# Add here the list of kafka-clusters (by name) that the service will need to reach.
kafka:
  allowed_clusters: []

# Add here the list of zookeeper-clusters (by name) that the service will need to reach.
zookeeper:
  allowed_clusters: []

An example of this would be

kafka:
  allowed_clusters:
    - logging-eqiad
    - logging-codfw

This data would then generate the following resource:

apiVersion: crd.projectcalico.org/v1
kind: NetworkPolicy
metadata:
  name: {{ template "base.meta.name" . }}-egress-external-services
  namespace: {{ .Release.Namespace }}
spec:
  selector: name == '{{ template "base.meta.name" . }}'
  types:
  - Egress
  egress:
    - action: Allow
      destination:
        services:
          name: logging-eqiad
          namespace: kafka
    - action: Allow
      destination:
        services:
          name: logging-codfw
          namespace: kafka

As the kafka namespace already contains the logging-eqiad/codfw Services, managed by the external-services chart, the generation of such template would be pretty trivial.

The question becomes: how can we guarantee a smooth migration for all apps?

My thinking is: we don't want to deprecate the current networkpolicy.yaml.skel file, as it is useful to be able to generate ingress policies, as well as custom egress policies as well. We'd probably want to remove this block from it:

  egress:
    {{- include "base.networkpolicy.egress-basic" .Values }}
-   {{- include "base.networkpolicy.egress.kafka" . | indent 4 }}
-   {{- include "base.networkpolicy.egress.mariadb" . | indent 4 }}
-   {{- include "base.networkpolicy.egress.zookeeper" . | indent 4 }}

and define a base.networkpolicy.egress-external-services template in modules/base/networkpolicy_1.3.0.tpl, that would loop over all allowed egresses and create the associated Calico NetworkPolicy resources.

Note: I wonder if we should refactor the

kafka:
  allowed_clusters: []

zookeeper:
  allowed_clusters: []

structure into something like:

allowed_clusters:
  kafka: []
  zookeeper: []

to make the definition of allowed egresses more centralized?

Event Timeline

Thanks for the writeup! I pretty much agree with your plan/idea although I would suggest to add a separate module (modules/base/external-services-networkplicy_1.0.0.yaml or alike) to we can create new versions independently from the base/networkpolicy module.

I also (obviously) like the idea of refactoring the .Values interface into something generic which might even allow us to not rely on a static list of sections (https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/984819/18/charts/external-services/templates/service.yaml#10). But rather map those directly to keys in the globl_config yaml.

Thanks for the feedback! My initial thought was to make values.yaml unaware of the change, so that

kafka:
  allowed_clusters: [...]
zookeeper:
  allowed_clusters: [...]

would work in both cases (vanilla Network Policies and Calico Network policies). However, if we're going to create a separate base module for external services related networkpolicies, then we could actually break with the actual convention and introduce another value structure, such as

external_services:
  egress:
    kafka:
    - logging-eqiad
    zookeeper:
    - flink-eqiad
    presto:
    - analytics

or maybe just

external_services:
  kafka:
  - logging-eqiad
  zookeeper:
  - flink-eqiad
  presto:
  - analytics

This way, we make it explicit that we're using the external-services chart, and we break away from the current data structure that would have been awkward to add new types of services to.

As you alluded to in our IRC conversation, this also protects us from values shadowing (eg: someone introduces a kerberos value at the root of values.yaml unaware that it would break the

kerberos:
  allowed_clusters: [...]

data structure that is currently the norm)

Yes, exactly. I think we would benefit from breaking with the current structure and instead create a generic one for helm chart values files as well as for the values.yaml we generate via global_config.pp. We can keep the old global_config.pp structure (AIUI that's just for kafka and zookeeper) around until we've migrated all relevant charts which should not take that long.

Great, I'm glad we agree on this. I'll start working on a prototype, so we can adjust as needed.

Change 1009279 had a related patch set uploaded (by Brouberol; author: Brouberol):

[operations/deployment-charts@master] Add template rendering external services egress NetworkPolicy resources

https://gerrit.wikimedia.org/r/1009279

Gehel triaged this task as High priority.Mar 8 2024, 10:09 AM

Change #1009279 merged by Brouberol:

[operations/deployment-charts@master] Add template rendering external services egress NetworkPolicy resources

https://gerrit.wikimedia.org/r/1009279