servicebinding / spec

Specification for binding services to k8s workloads
https://servicebinding.io
Apache License 2.0
92 stars 35 forks source link

Revisit workload and service labels #184

Closed arthurdm closed 2 years ago

arthurdm commented 3 years ago

We need to revisit how to enable the use of labels to reference either workloads and/or services. We have previously tried to include as label selector as a mutually exclusive field with name but we hit the obstacle of having more than 1 matching resources - and that matching set being dynamic.

The proposed way is to think about a higher level CR that would create ServiceBinding CRs for every match (keeping ServiceBinding always 1-to-1)

arthurdm commented 3 years ago

I think the easiest way to keep ServiceBinding CRs always 1-to-1 between a workload and a service is to create a higher level resource such as this:

apiVersion: service.binding/v1alpha2
kind: ServiceBindingSelector
metadata:
  name:                 # string
  generation:           # int64, defined by the Kubernetes control plane
  ...
spec:
  name:                 # string, optional, default: .metadata.name
  type:                 # string, optional
  provider:             # string, optional

  workload:             # ObjectReference-like
    apiVersion:         # string
    kind:               # string
    selector:           # metav1.LabelSelector
    containers:         # []string, optional

  service:              # Provisioned Service resource ObjectReference-like
    apiVersion:         # string
    kind:               # string
    selector:           # metav1.LabelSelector

  env:                  # []EnvMapping, optional
  - name:               # string
    key:                # string

And for each matching label resource the controller creates a new ServiceBinding CR.

Some details that come to mind:

scothis commented 3 years ago

The use case that is possible today, but is not possible with this proposal is to bind into immutable resources matched by the workload selector. For example, If you had a resource like:

apiVersion: service.binding/v1alpha2
kind: ServiceBindingSelector
metadata:
  name: my-binding
spec:
  service:
    apiVersion: example.com/v1
    kind: Database
    name: my-database
  workload:
    apiVersion: batch/v1
    kind: Job
    selector:
      matchingLabels:
        app: my-task

---
apiVersion: batch/v1
kind: Job
metadata:
  name: my-job
  labels:
    app: my-task
spec: 
  ...

Should trigger the creation of:

apiVersion: service.binding/v1alpha2
kind: ServiceBinding
metadata:
  name: my-binding-0
spec:
  service:
    apiVersion: example.com/v1
    kind: Database
    name: my-database
  workload:
    apiVersion: batch/v1
    kind: Job
    name: my-job

It will match a Job and create a ServiceBinding targeting that Job. However, the ServiceBinding will not be able to inject into the Job because the job is already exists and is immutable.

This behavior works today because the ServiceBinding resource can exist before the Job is created, and intercept the Job to inject the binding as it is created. With the proposed change, the ServiceBinding resource would never be defined before the Job.

arthurdm commented 3 years ago

Thanks for illustrating with an example Scott, this helps. 👍

The main issue with the sample is the fact that inject during create mutation hook behaviour is not mandated by the spec, so while some implementations may do this, it's not guaranteed.

So I don't think it's a valid use case to counter this proposal, as we may have spec implementations that do not support this (even if our reference implementation does).

It's possible to imagine an implementation of ServiceBindingSelector that also injects resources during the create mutation hook - creating the ServiceBinding after the creation for completeness since it also owns it (with a status that it has already been processed). Maybe unconventional, but possible. 😄

arthurdm commented 2 years ago

Closing this issue - will revisit in the future as part of the core spec.