Allow service brokers to specify labels that are applied to the binding secret

[maleck13]

Use case:

I have an application that wants to use the secrets created from certain services in order to generate a configuration object / file that can be consumed by external clients for those services, for example a mobile application. Currently in order to do this, the application needs to maintain a list of distinct services that it knows about. It would be preferable if I could have the secrets created from the broker have a label, something like "mobile-enabled" that it could use to discover the secrets it could work with.

[jhalliday]

It would additionally be useful if the created bind secret could inherit the "template" and "template.openshift.io/template-instance" labels which the platform adds to the Secret/Service/DeploymentConfig that are created when the template is instantiated. Perhaps also ownerReferences.

[duglin]

in your workflow, after you create the binding, could you not add the labels to the secrets yourself?

[jhalliday]

I'm not the one creating the binding. I have a process that watches deployment changes and reacts to them, but can't influence them.

[pmorie]

There are two different flavors of this that we run a risk of conflating. I believe they are:

1. Having the ServiceInstanceCredential have API surface that let the user state the labels that the secret should have
2. Allowing the broker to return information about the k8s labels to apply to the secret to create

I believe that @maleck13 can use (1), but @jhalliday needs (2).

[maleck13]

@pmorie @duglin
Thinking more about this, I think my general use case is closer to 2. I think it would be useful to allow brokers to return a set of labels (maybe as part of the catalog response that they want applying to objects created based on that ClusterServiceClass.

Example:
The broker adds a set of labels as part of the catalog response.
These labels are then applied to any ServiceInstance, ServiceBinding and Secret created by the catalog based on that ServiceClass.
This would be broker specific meta data, that would be consumed by a UI client for example or a CLI client.
This information could be added from within the broker, but it means the broker must be aware of and be able to interact with the k8s API
It could also be added by the client, but then it means you need to ensure that each client adds the same metadata.
Implementing something like this in the catalog would mean that no matter which client created the ServiceInstance etc the labels would be present.

If the use case makes sense. I am happy to start putting a proposal together but want to be sure this is the right place for it.

[maleck13]

@pmorie @duglin Had no response on this for sometime so just following up after thinking more on it.
Would a reasonable approach here be to allow a set of labels to be specified as part of the ServiceBinding object with the default being none. The name of the secret is allowed to be specified https://github.com/kubernetes-incubator/service-catalog/blob/master/pkg/apis/servicecatalog/v1beta1/types.go#L1056 so perhaps this would be a reasonable place to allow specifying metadata to be applied to that secret?
This is probably the simplest approach, but it means you have to be in control of the client creating the binding and as @duglin points out, if you are in control of that then you could watch for the secret being created and add the labels manually. Although it seems like extra work for the client and is something that the catalog could support.
This however doesn't solve the case where you are not in control of the binding resource that is being created (IE you are a broker author only). In this case it would seem like it would need to be some metadata supplied from the broker's catalog endpoint and stored as part of the ClusterServiceClass definition. Perhaps using ExternalMetadata would be an approach here? If not it seems it might mean a change to the broker spec?

Currently we have to create a second secret with the same credentials in it with the labels we need which seems counter productive.
As mentioned I am happy to put together a proposal and follow up with implementation if accepted.

[n3wscott]

drive-by-comment:

the broker already knows it is talking to a platform on a binding request, perhaps we add a kubernetes section to the payload where we can store info like this that is intended for k8s to act on? like:

{
  "credentials": {
    "uri": "mysql://mysqluser:pass@mysqlhost:3306/dbname",
    "username": "mysqluser",
    "password": "pass",
    "host": "mysqlhost",
    "port": 3306,
    "database": "dbname"
  },
  "kubernetes": {
     "secrettags": ["prod", "mysql"]
  }
}

It blurs the line a bit around the broker needing to know it is talking to k8s, but it does that already I would say. Or you pass those tags into the original request parameters, and they come out plus some.

[maleck13]

This would be an approach that works, perhaps it would just be

"platform":{
      "labels":{
              "mylabel":"myval"
      }
  }

And then the platform could decide what should be done with that information? In k8s case it would add those labels to the secret.
Let me know if I am taking that up wrong.
This would be a change to the OSB API spec right so I would need to open up an issue there?

[carolynvs]

@maleck13 Can you help me understand why you prefer that the broker add those labels instead of specifying the label say, on the binding?

I'm curious because I am working on a proposal to support adding defaults for service classes and plans, and thought that default labels could be useful as well. So that you could configure a single time at the cluster level "when resources are created for this plan, add this label".

I'm just wondering if you picked option 2 (broker supplied labels) because that seemed like less repetition or because the broker is actually in a better position to know what labels are appropriate.

Thanks!

[maleck13]

I'm curious because I am working on a proposal to support adding defaults for service classes and plans, and thought that default labels could be useful as well. So that you could configure a single time at the cluster level "when resources are created for this plan, add this label".

I don't have strong preference here. I was looking for something that would gain traction as I have been struggling with getting engagement on the issue. So delighted to hear I am not alone and you are working towards something similar.
I actually really like the idea of being able to specify some meta data (annotations and labels ) on the ClusterServiceClass that are then added to any child resources created from the ClusterServiceClass. I have a particular neeed for service instances, service bindings and the credentials secret.
If you are thinking of incorporating this kind of change into your proposal I would love to take a look once you have it ready!

[carolynvs]

@maleck13 It seems quite straightforward and fits into the intent of that proposal. I'll ping back here once it's updated.

[maleck13]

@carolynvs how is the proposal coming along?

[carolynvs]

@maleck13 With KubeCon and then falling sick, I completely dropped the ball on this. I've added it to my list but I'm out sick so it won't be updated this week. Thanks for the reminder!

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.