Adoption of annotations from specification

[arthurdm]

The section of the service binding specification that describes the x-descriptors and annotations that a service should use to be bindable uses slightly different names than what SBO supports today.

A proposal is:

• start supporting the annotations from the spec
• document that SBO is deprecating the usage of its old annotations described here, but continue to use it for a release or two to give time for migration of any existing services.

This is an important item to get adoption of the spec, since we're starting to reach out to services (Strimzi, Postgresql, etc) to make them more bindable.

[sbose78]

Thanks, let me follow-up with a "before" and "after" of the annotations.

[sbose78]

@Avni-Sharma Could you please review the spec and provide an example of how the new annotations would look like for https://github.com/operator-backing-service-samples/postgresql-operator ?

2. Non-OLM Operator: - An annotation in the Operator's CRD to mark which `status` properties reference the [binding data](#service-binding-schema).  The value of this annotation can be specified in either [JSONPath](https://kubernetes.io/docs/reference/kubectl/jsonpath/) or [GO templates](https://golang.org/pkg/text/template/):
      * ConfigMap:
        * servicebinding/configMap: {.status.bindable.ConfigMap}
      * Secret:
        * servicebinding/secret: {.status.bindable.Secret}
      * Individual binding items:
        * servicebinding/secret/host: {.status.address}
        * servicebinding/secret/`<binding_property>`: {.status.`<status_property>}` (where `<binding_property>` is any property from the binding schema, and `<status_property>` refers to the path to the correspoding `status` property)

[Avni-Sharma]

Currently, the annotations are like

    servicebindingoperator.redhat.io/spec.dbName: 'binding:env:attribute'
    servicebindingoperator.redhat.io/status.dbCredentials-user: 'binding:env:object:secret'
    servicebindingoperator.redhat.io/status.dbCredentials-password: 'binding:env:object:secret'
    servicebindingoperator.redhat.io/status.dbConnectionIP: 'binding:env:attribute'
    servicebindingoperator.redhat.io/status.dbConnectionPort: 'binding:env:attribute'
    servicebindingoperator.redhat.io/status.dbName: 'binding:env:attribute'
    servicebindingoperator.redhat.io/status.dbConfigMap-db.host: 'binding:env:object:configmap'
    servicebindingoperator.redhat.io/status.dbConfigMap-db.port: 'binding:env:object:configmap'
    servicebindingoperator.redhat.io/status.dbConfigMap-db.name: 'binding:env:object:configmap'
    servicebindingoperator.redhat.io/status.dbConfigMap-db.user: 'binding:env:object:configmap'
    servicebindingoperator.redhat.io/status.dbConfigMap-db.password: 'binding:env:object:configmap'

In the current scenario we have the env option when we need to inject the bindable data as an environment variable. That can also be changed to volumemount incase we need to have data as a volumemount. In the spec [https://github.com/application-stacks/service-binding-specification/tree/updateSchema#pointer-to-binding-data] we have not used it. Are we planning to drop the env/volumemount option altogether?

[arthurdm]

hi @Avni-Sharma - I have opened this issue in the spec side to also support environment variables. I was convinced that in cases of non-sensitive data it's still a very practical and used method. =)

[Avni-Sharma]

Reference - servicebinding/spec#25

[otaviof]

Community meeting notes:

• Pressuring point to avoid supporting two different annotations sets
• Relates to PR Streamline data retrieval based on annotations #407

[arthurdm]

@isutton / @sbose78 - we could use this issue to lay out the plan and divide-and-conquer in terms of resources?

[isutton]

@Avni-Sharma will be implementing this with my collaboration.

The plan so far is, with the changes introduced in #407 :

• Modify annotations.BuildHandler to support the new annotations;
• Implement a predicate similar to IsConfigMap to build the right annotation handler; and
• Implement a type consuming annotations.Handler, which will be returned by annotations.BuildHandler. The implementation will provide the Handle method which is responsible to create a annotation.Result value (this is the type that should be adapted to provide the new annotation semantics.

I currently don't believe it is necessary to have more hands on the deck for this.

What do y'all think?

[isutton]

/assign @Avni-Sharma

[Avni-Sharma]

Support added in #628