cloudfoundry / cf-for-k8s

The open source deployment manifest for Cloud Foundry on Kubernetes
Apache License 2.0
300 stars 115 forks source link

Manifest annotations and labels should be applied to cf-workload pods #579

Open braunsonm opened 3 years ago

braunsonm commented 3 years ago

Describe the bug

CF Manifests support annotations and labels. These should be translated to labels and annotations on the pods to allow greater customization.

For instance in our usecase we want to apply a certain policy using a podSelector. If we could deploy our apps with a label using the manifest, we could match on that label.

To Reproduce*

Steps to reproduce the behavior:

  1. Create a test app with this manifest:
    ---
    applications:
    - name: go-webserver
    health-check-type: process
    instances: 1
    metadata:
      annotations:
        another: "hey"
      labels:
        test: "hello"
  2. cf push
  3. Notice that neither the annotation nor labels gets applied on the pod
cf-gitbot commented 3 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/175823164

The labels on this github issue will be updated when the story is started.

Birdrock commented 3 years ago

@piyalibanerjee @matt-royal What do y'all think of this request?

braunsonm commented 3 years ago

For a use-case, since cf-for-k8s doesn't support internal domains or C2C communication, we were going to have apps tag themselves as internal with a label. Then disable ingress access using a network policy matching that label.

---
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
metadata:
  name: deny-app-ingress-internal-services
  namespace: cf-workloads
spec:
  podSelector:
    matchLabels:
      mydomain/internal: true
  policyTypes:
  - Ingress

This coupled with a Sidecar rule to allow C2C is currently the only way for CF apps to talk to eachother without going through the internet. We have some apps which do not have authentication on them and are meant for internal usage only.

matt-royal commented 3 years ago

I just did a dive through the cloud_controller_ng and eirini code bases to understand what this would take. Adding support for annotations requires a small change to CCNG, but is already fully supported in eirini. Labels, on the other hand, would require changes to both components.

The technical work aside, this feature request does seem like it could have security implications. @tcdowney, @MasslessParticle, and I discussed this offline and here's a summary of the concerns and mitigations we came up with:

loewenstein commented 3 years ago

I do understand the use-case, but I also share the security concerns. I would recommend that this feature is guarded by explicit configuration, e.g. allow_application_annotations and allow_application_labels. The most convenient config API might be to allow a list of allowed keys (or key prefixes) and support * as an all-in option.