Please add annotation to KafkaAccess CRD for Service Binding Operator discoverability

[paul-snively]

Hi folks!

First of all, thanks for developing this.

In the course of attempting to use it, I discovered that the KafkaAccess I created isn't discoverable by the Service Binding Operator, by which I mean the resource did not appear in the list of "bindable services" shown by odo. The reason for this is the lack of any appropriate metadata to expose binding information on the CRD.

It seems I can add the annotation to the CustomResource rather than having it on the CustomResourceDefinition, so I'm about to try that. But it would make the operator even more attractive, IMO, if it would simply do this "out of the box."

[scholzj]

CC @katheris

[paul-snively]

Update: Unfortunately, annotating the CustomResource itself does not make the service discoverable by odo list services -A as I had hoped. So it seems adding the annotation(s) to the CRD is our only recourse.

[katheris]

Hi @paul-snively, thanks for raising this. It looks like from the spec it is recommended to have a label in the CRD that is servicebinding.io/provisioned-service: "true" Would you be able to update your KafkaAccess CRD to add that label and verify if that makes it discoverable? If that works then I will look to add this annotation to the KafkaAccess CRD as part of the build.

[paul-snively]

@katheris, thanks for looking into this!

Unfortunately, given:

`paulsnively@Paul-Snively's-MacBook-Pro:~/moonraker/moonraker-notifications|main⚡ ⇒ kubectl describe -n strimzi-access-operator crd/kafkaaccesses.access.strimzi.io

Name:         kafkaaccesses.access.strimzi.io
Namespace:    
Labels:       servicebinding.io/provisioned-service=true
Annotations:  <none>
API Version:  apiextensions.k8s.io/v1
Kind:         CustomResourceDefinition
Metadata:
  Creation Timestamp:  2022-10-03T13:24:59Z
  Generation:          1
  Managed Fields:
    API Version:  apiextensions.k8s.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:status:
        f:acceptedNames:
          f:kind:
          f:listKind:
          f:plural:
          f:shortNames:
          f:singular:
        f:conditions:
          k:{"type":"Established"}:
            .:
            f:lastTransitionTime:
            f:message:
            f:reason:
            f:status:
            f:type:
          k:{"type":"NamesAccepted"}:
            .:
            f:lastTransitionTime:
            f:message:
            f:reason:
            f:status:
            f:type:
    Manager:      Go-http-client
    Operation:    Update
    Subresource:  status
    Time:         2022-10-03T13:24:59Z
    API Version:  apiextensions.k8s.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .:
          f:kubectl.kubernetes.io/last-applied-configuration:
        f:labels:
          .:
          f:servicebinding.io/provisioned-service:
      f:spec:
        f:conversion:
          .:
          f:strategy:
        f:group:
        f:names:
          f:kind:
          f:listKind:
          f:plural:
          f:shortNames:
          f:singular:
        f:scope:
        f:versions:
    Manager:         kubectl-client-side-apply
    Operation:       Update
    Time:            2022-10-03T13:24:59Z
  Resource Version:  167578
  UID:               3aff9e86-1f32-4588-85b9-968e64c6a109
Spec:
  Conversion:
    Strategy:  None
  Group:       access.strimzi.io
  Names:
    Kind:       KafkaAccess
    List Kind:  KafkaAccessList
    Plural:     kafkaaccesses
    Short Names:
      ka
    Singular:  kafkaaccess
  Scope:       Namespaced
  Versions:
    Name:  v1alpha1
    Schema:
      openAPIV3Schema:
        Properties:
          Spec:
            Properties:
              Kafka:
                Properties:
                  Listener:
                    Type:  string
                  Name:
                    Type:  string
                  Namespace:
                    Type:  string
                Required:
                  name
                Type:  object
              User:
                Properties:
                  API Group:
                    Type:  string
                  Kind:
                    Type:  string
                  Name:
                    Type:  string
                  Namespace:
                    Type:  string
                Required:
                  apiGroup
                  kind
                  name
                Type:  object
            Required:
              kafka
            Type:  object
          Status:
            Properties:
              Binding:
                Properties:
                  Name:
                    Type:  string
                Type:      object
              Observed Generation:
                Type:  integer
            Type:      object
        Type:          object
    Served:            true
    Storage:           true
    Subresources:
      Status:
Status:
  Accepted Names:
    Kind:       KafkaAccess
    List Kind:  KafkaAccessList
    Plural:     kafkaaccesses
    Short Names:
      ka
    Singular:  kafkaaccess
  Conditions:
    Last Transition Time:  2022-10-03T13:24:59Z
    Message:               no conflicts found
    Reason:                NoConflicts
    Status:                True
    Type:                  NamesAccepted
    Last Transition Time:  2022-10-03T13:24:59Z
    Message:               the initial names have been accepted
    Reason:                InitialNamesAccepted
    Status:                True
    Type:                  Established
  Stored Versions:
    v1alpha1
Events:  <none>

I still get:

paulsnively@Paul-Snively's-MacBook-Pro:~/moonraker/moonraker-notifications|main⚡ ⇒  odo list services -A
 ✓  Listing bindable services from all namespaces [68ms]

 NAME                                                          NAMESPACE 
 postgresql/PostgresCluster.postgres-operator.crunchydata.com  moonraker 

It seems the annotations described in the first link, which are also used by the Crunchy Data Postgres operator, are necessary for the Service Binding Operator to discover the "bindability" of an operator. I suppose it makes some sense: the annotations specify not only that the service is bindable (true or false), but specify an expression providing the resource the service provides for binding—typically a secret.

Thanks again!

[paul-snively]

When I revise the CRD post make build like so:

# Generated by Fabric8 CRDGenerator, manual edits might get overwritten!
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: kafkaaccesses.access.strimzi.io
  annotations:
    service.binding: 'path={.metadata.name},objectType=Secret'
spec:
...

and re-install, and create a KafkaAccess, I then get:

paulsnively@Paul-Snively's-MacBook-Pro:~/moonraker/moonraker-notifications|main⚡ ⇒  odo list services -A
 ✓  Listing bindable services from all namespaces [70ms]

 NAME                                                          NAMESPACE 
 postgresql/PostgresCluster.postgres-operator.crunchydata.com  moonraker 
 kafka-access/KafkaAccess.access.strimzi.io                    moonraker 

as expected. I believe this path is correct—that is, I believe the operator does, in fact, create a Secret with the same name as the KafkaAccess.

[katheris]

@paul-snively thanks for doing these tests. That is different to the behaviour I expected, since the spec indicates that the location of the resource to bind (the secret in this case), should be referenced in the status of the CR. It doesn't mention the need for the annotation. This is in line with this section of the Service Binding Operator docs: https://docs.openshift.com/container-platform/4.9/applications/connecting_applications_to_services/exposing-binding-data-from-a-service.html#provisioned-service

I've added a question to the Service Binding Operator channel in slack to ask for clarification as I don't think we should need to provide this information twice.

[paul-snively]

OK, with quite a bit of digging into source code I shouldn't have needed to, I determined that a label won't do, but:

  annotations:
    "servicebinding.io/provisioned-service": "true"

on the KafkaAccess CRD does. I'm not sure how to add this to the code generation process for the CRD, or I'd just offer a PR. Sorry about that, and thanks for the help!

[katheris]

Ah, so the spec and the service binding operator implementation are not lined up! Is it the source code for odo or service binding operator that showed it was looking for annotation?

The spec clearly states label: https://github.com/servicebinding/spec#provisioned-service

To facilitate discoverability, it is RECOMMENDED that a CustomResourceDefinition exposing a Provisioned Service add servicebinding.io/provisioned-service: "true" as a label.

Before we add either a label or annotation I think we need to confirm where the mix up has come from.

I've separately raised an issue to see if adding labels (but the same would apply to annotations) can be done as part of the code generation process (https://github.com/fabric8io/kubernetes-client/discussions/4475). So once we've determined the one we need for now we would add it as part of the Makefile. I already have a branch with the change ready, so I can create a PR for that once I've confirmed which the spec expects.

[katheris]

Hi @paul-snively, I have spoken to the Service Binding Operator contributors and they have said it is a bug that it is searching for an annotation instead of a label. odo is using SBO code to search for bindable services. I've raised a PR here to add the label that the spec expects: https://github.com/strimzi/kafka-access-operator/pull/13

Until SBO fixes their codebase the workaround for you will be to add the annotation to the KafkaAccess CRD before applying it to your cluster as below:

annotations:
    "servicebinding.io/provisioned-service": "true"

Once the SBO bug has been fixed and my PR has been merged then KafkaAccess should be discoverable correctly by odo.

[paul-snively]

@katheris, thanks so much for your diligence in following up on this. It's disheartening to hear, considering that the Service Binding Operator is currently at version 1.2.0, raising questions about both the testing of the operator and its adoption in the field, when I'm trying to promote the use of operators generally, the Service Binding Operator specifically, and odo at work.

Still, better that we uncover these things now rather than be bitten by them later.

Thanks again!

[katheris]

@paul-snively just to update you on this, the Service Binding Operator team have made a change (linked above) so that now it will identify a service is bindable when it has the binding section in the status field. This means it should detect KafkaAccess CRs as bindable without any changes to the Kafka Access operator. The change didn't go into version 1.3.2 of the Service Binding Operator, but I believe should be included in the next version and is part of the current main branch.

[katheris]

@paul-snively I have verified this with v1.3.4 of the Service Binding Operator and can now see the Kafka Access CR when I run odo list services -A so I am closing this issue. Let me know if you feel this issue isn't resolved.