stackabletech / listener-operator

Stackable Operator for network listeners and load balancers
https://docs.stackable.tech/listener-operator/stable/
Other
6 stars 0 forks source link

Use addresses provided by Listeners for provisioning Secrets #95

Closed siegfriedweber closed 11 months ago

siegfriedweber commented 1 year ago

As a user I want that the TLS certificates and Kerberos keytabs contain the addresses which I use to connect, otherwise a valid connection would be denied. Therefore the secret-operator must provision the certificates and keytabs with the addresses provided by the according Listeners.

Motivation

The secret-operator currently provides the following back-ends:

autoTls uses scopes to populate the claims (such as subjectAlternateName) of the provisioned certificates. If a Listener is used to expose a service then the claims should contain the addresses provided by the Listener. Otherwise, if the Listener advertises an address which is not contained in the subjectAlternateName then the certificate would be considered as invalid if this address is used.

k8sSearch selects a Secret based on the scopes. A matching Secret is then labeled with secrets.stackable.tech/listener: <listener-name>. If a Listener is given by name in the Volume specification of the Pod (listeners.stackable.tech/listener-name: <listener-name>) then this is a valid approach. But if a Listener class is given (listeners.stackable.tech/listener-class: <listener-class-name>) then the Listener name is generated by concatenating the Pod name and the Volume name specified in the Pod. In this case, selecting by a Listener scope should be avoided.

kerberosKeytab uses scopes to set the service principals in a keytab. Like TLS certificates, the service principals must contain the correct addresses.

sbernauer commented 1 year ago

Just wanted to check if the following scenario is considered and supported: kafka-op creates a Listener kafka-bootstrap directly (not using a pvc), which has a selector to all available kafka brokers, so that kafka clients have a single endpoint to bootstrap from. Let's say the kafka-bootstrap listener is reachable at 1.2.3.4:8082, the certificate of every broker needs to include that address as well as his broker-specific address. We might need something like secrets.stackable.tech/scope: listener,listener=kafka-bootstrap on the secret-op pvcs. Would this work with the current proposal?

nightkr commented 1 year ago

A listener PVC can also name a specific precreated Listener, instead of a ListenerClass. See https://github.com/stackabletech/listener-operator/blob/f9307d0332ca1c7f602421d5ba4559e7adad1caf/examples/nginx-preprovisioned-lb.yaml. Technically speaking, adding the correct labels to join a Listener's Service without the corresponding PVC may work, but that is not a supported use case.

nightkr commented 1 year ago

Actually, https://github.com/stackabletech/listener-operator/blob/f9307d0332ca1c7f602421d5ba4559e7adad1caf/rust/operator-binary/src/csi_server/node.rs#L249-L252 also provides more details on why we can't rely on Listener.status.ingressAddresses at all. We'll need to rethink this a bit...

sbernauer commented 1 year ago

A listener PVC can also name a specific precreated Listener, instead of a ListenerClass

Ah that's good to know! So in the case of Kafka brokers, we pre-create a bootstrap Listener and than have two csi volumes, one for each listener. The bootstrap listener will load-balance over all available brokers, is that correct?

nightkr commented 1 year ago

@sbernauer Yes, exactly.

nightkr commented 1 year ago

So I've been going back and forth a bunch wrt listener secrets..

  1. ruled out secret-op reading Listener objects, because there end up being too many races and stuff before we can resolve addresses correctly, + it's kind of a pain to map a Pod's volume to the correct Listener (many different paths to rome, and we need to be able to parse them all)
  2. ruled out adding a direct API between list-op and sec-op, because it's still kind of a pain for list-op to do this resolution too
  3. list-op adding an annotation for the mounted volume to the pod containing this data is Promising:tm:, but can be easily spoofed by users (bad, would let them effectively forge TLS certs if they can create pods)
  4. current proposal: list-op defines a new PodListeners CRD that maps 1:1 to Pod objects and contains this info, sec-op reads this when provisioning secrets with the listener scope
nightkr commented 1 year ago

Preliminarily, 4 seems to be a viable path forward, but we still need to work out how this interacts with k8sSearch.

The naíve approach would be to just add another scope label to the secrets, but this doesn't work well for NodePort services. That would require the services to also include the node scope, which goes against the whole point of abstracting this away from the individual product operators.

Instead, I see two ways forward for this:

  1. Resolve secret scopes to addresses and use those as labels instead.
  2. Let listener-operator set a flag on the listener that also implies a node scope.

1 seems cleaner at first blush, but would be a major compatibility break for any existing k8sSearch users. It would also introduce an awkward scheduling cycle between listener-op and secret-op (we want to let secret-op limit scheduling to nodes with provisioned secrets, but we can't know that before listener-op is ready). Therefore, I'd propose that 2 is the way to go.