cloudfoundry / eirini

Pluggable container orchestration for Cloud Foundry, and a Kubernetes backend
Apache License 2.0
115 stars 30 forks source link

Whitelisted annotations #82

Closed heycait closed 3 years ago

heycait commented 4 years ago

Description

We'd like to add Prometheus scrape annotations to pods in k8s based on CAPI annotations in order to identity and scrape apps with metrics endpoints.

Suggestion

Map whitelisted/white-patterns Cloud Controller annotations sets directly to k8s annotations.

Our use case whitelist:

This will allow app devs to set an annotation on their app, and if allowed, it will get set on the k8s pod. These annotations are used for by prometheus-compatible scrapers for service discovery in k8s clusters.

Additional information (optional)

https://pivotal.slack.com/archives/CPPEXT81M/p1574270166358800

cf-gitbot commented 4 years ago

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

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

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

cwlbraa commented 4 years ago

On the CC I think we'd need operator-configurable whitelists for metadata passthrough and/or a global blacklist for things like *.cloudfoundry.org/* and whatever else eirini might set or read under the hood...

emalm commented 4 years ago

Since the main use-case for passing through this CC metadata is to supply configuration for Prometheus scraping, I wonder whether CC could instead provide a higher-level API to support this configuration space instead of requiring app developers to know these Prometheus-specific details. /cc @ssisil @selzoc

rosenhouse commented 4 years ago

Ok, straw-dog API. In CAPI, there's config like:

passthrough_labels:
- app/*
- prometheus.io/*
- favoriteBreakfast

Eirini doesn't do its own filtering, because that's really a policy thing that CAPI should understand and control.

cc @zrob @cwlbraa

selzoc commented 4 years ago

should the whitelist be configurable by an operator?

rosenhouse commented 4 years ago

Yes? Maybe with guardrails?

cwlbraa commented 4 years ago

could we do without globbing?

selzoc commented 4 years ago

i think so - just prefixes would be fine

gberche-orange commented 4 years ago

@rosenhouse in response to https://github.com/cloudfoundry-incubator/eirini/issues/82#issuecomment-590482782

Such CAPI white list seems similar/related to CloudController annotation propagation to service brokers, planned in OSB 2.16, see https://github.com/openservicebrokerapi/servicebroker/pull/658 and https://www.pivotaltracker.com/n/projects/2105761/stories/166935990

The rule of thumb in the case of annotation propagation to service brokers was to only propagate annotations with FQDN-like prefix, and keep unprefixed annotations private as mentionned in

https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set

If specified, the prefix must be a DNS subdomain: a series of DNS labels separated by dots (.), not longer than 253 characters in total, followed by a slash (/). If the prefix is omitted, the annotation Key is presumed to be private to the user. Automated system components (e.g. kube-scheduler, kube-controller-manager, kube-apiserver, kubectl, or other third-party automation) which add annotations to end-user objects must specify a prefix.

Would it make sense in the future to have a single white list for propagation to both K8S and service brokers, or two distinct white lists sharing the same logic ?

passthrough_labels:

  • app/*
  • prometheus.io/*
  • favoriteBreakfast

In terms of configuration naming, should it read "annotations" instead of labels, or more generally "medata" if both annotation and labels are meant to be propagated ?

It might also be useful to detect shadowing of existing annotation/labels that can be caused by user-provided metadata in CF, and potentially be seen as injection of improperly validated data, causing security issues.

cwlbraa commented 4 years ago

@gberche-orange I love watching the k8s annotation documentation expand and improve over time- We read and re-read that piece of linked annotations doc as we were developing the CF metadata API.

Would it make sense in the future to have a single white list for propagation to both K8S and service brokers, or two distinct white lists sharing the same logic ?

This is an interesting question, and I don't have an answer. My intuition is that coupling them together might be fraught, but I don't have any specific examples to back that up.

In terms of configuration naming, should it read "annotations" instead of labels, or more generally "medata" if both annotation and labels are meant to be propagated ?

You're right that we keep saying "annotations" and "labels" when we mean metadata.

It might also be useful to detect shadowing of existing annotation/labels that can be caused by user-provided metadata in CF, and potentially be seen as injection of improperly validated data, causing security issues.

I'm not sure how we might differentiate between "existing" and user provided metadata.

gberche-orange commented 4 years ago

@cwlbraa

I love watching the k8s annotation documentation expand and improve over time- We read and re-read that piece of linked annotations doc as we were developing the CF metadata API.

The CF metadata is also documenting prefix support, but is currently less opinionated about private vs public metadata depending on prefix presence.

https://docs.cloudfoundry.org/adminguide/metadata.html#prefix

You can ensure a label or annotation key is easily differentiated from other keys by using a prefix. A prefix is a namespacing pattern that helps you more clearly identify resources. Prefixes are in DNS subdomain format. For example, prefix.example.com.