servicebinding / spec

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

Native bindings #92

Closed arthurdm closed 3 years ago

arthurdm commented 3 years ago

One scenario that comes up a lot are services that have their bindings as a k8s Secret, but don't have a duck typed CR nor the extension annotations.

This issue proposes that we allow ServiceBinding.spec.service to point to a regular k8s Secret and behave as if it was defined via a duck type. This would enable ServiceBinding authors to connect to various existing services without the need to wrap it with other CRs (such as the ProvisionedService concept).

Taking this further, and inspired by Service Binding Operator's detection support, we could even allow for Ingress / Route resources to be natively bindable - for example, if a microservice just needs to know the URL to connect to.

scothis commented 3 years ago

This is a concept I've been torn by in the past. The implementation is not hard, but to what end?

A bare secret is unlikely to contain the metadata necessary to actually consume a service unless it was created with that intent in mind. The ProvisionedService resource is a crutch, but creating it is meaningful because it is blessing a Secret as containing that information, or at least that intent.

In order to provide the best user experience, our goal is for service providers to implement the provisioned service duck type and not say that users referencing a Secret directly is good enough.

Taking this further, and inspired by Service Binding Operator's detection support, we could even allow for Ingress / Route resources to be natively bindable

Not sure what Route you're talking about (Knative?), but the Ingress resource supports virtual hosts with multiple hostnames and backend services, so I'm not sure how a ServiceBinding could make sense of it without already knowing what it's looking for.

I'm in favor of allowing implementations to understand service references beyond the provisioned service duck type, and likewise for applications, but I'm unsettled on adding that to the spec.

arthurdm commented 3 years ago

Ya - I guess it's a balance in terms of which part of the spec we favour adoption: allowing Secrets to be referenced directly doesn't provide motivation for service providers to adopt the spec (they can say "hey, I am already exposing the information via a Secret, leave me alone" 😄 ), but, it does allow spec adoption for ServiceBinding authors that want to consume that service.

I think there are vastly more ServiceBinding authors than there are service providers.

scothis commented 3 years ago

We can see some of the side effects of thinking of arbitrary Secrets as binding material here. https://github.com/k8s-service-bindings/spec/issues/96#issuecomment-689611046

I fully agree that we need to be realistic about interop with the existing world, but we should avoid encouraging behavior that will lead to confusion or downstream violations of the spec (like a projected volume not including a type, or worse including a misleading type)

arthurdm commented 3 years ago

I think the concept of the duck type can apply here: if a Secret conforms to the spec's view of a bindable Secret, it can be used a spec Secret.

In other words, if it has things like type, etc, then why not allow that to be bindable?

arthurdm commented 3 years ago

Arthur to create a PR and continue discussions there

baijum commented 3 years ago

@arthurdm After thinking about the "native binding" I wrote this. Let me know if the extension described below captures your proposal. Let me know if you want me to send this as a PR.

## Direct Secret Projection

There are scenarios where Provisioned Service is not available.  But there is a
`Secret` resource available for binding.  A Custom Projection with an additional
annotation **MAY** be used for directly binding this secret into the
application.

The author **MUST** set the `projection.service.binding/source` annotation to
`Secret`.

The `Secret` **SHOULD** contain a `type` entry with a value that identifies the
abstract classification of the binding.  It is **RECOMMENDED** that the `Secret`
also contain a `provider` entry with a value that identifies the provider of the
binding.  The `Secret` **MAY** contain any other entry.

The `.spec.service.kind` attribute **MUST** be `Secret` and the
`.spec.service.apiVersion` **MUST** be `v1`.  The `spec.service.name` attribute
**MUST** be a `LocalObjectReference`-able to a `Secret`.

Since the Direct Secret Binding Projection is a Custom Projection, the behavior
**MUST** be the same as that of [Custom Projection][cp].

[cp]: #custom-projection

### Direct Secret Projection Example Resource

```yaml
apiVersion: service.binding/v1alpha2
kind: ServiceBinding
metadata:
  name: account-service
  annotations:
    projection.service.binding/type: "Custom"
    projection.service.binding/source: "Secret"
spec:
  application:
    apiVersion: apps/v1
    kind:       Deployment
    name:       online-banking

  service:
    apiVersion: v1
    kind:       Secret
    name:       prod-account-service-secret

status:
  binding:
    name: prod-account-service-projection
  conditions:
  - type:   Ready
    status: 'True'
  - type:   ProjectionReady
    status: 'True'

Edit 1: Change annotation from "method" to "source" Edit 2: Emphasize this is a Custom Projection

baijum commented 3 years ago

Probably the annotation could be:

projection.service.binding/source: "Secret"
scothis commented 3 years ago

is the projection.service.binding/source: "Secret" annotation redundant with the service reference:

  service:
    apiVersion: v1
    kind:       Secret

If they are redundant, we should drop the annotation, if they are not we should explain how they are different.

scothis commented 3 years ago

Edit 2: Emphasize this is a Custom Projection

Why limit this to Custom Projections? The source of the service secret is orthogonal to how it is projected into an application.

baijum commented 3 years ago

is the projection.service.binding/source: "Secret" annotation redundant with the service reference:

  service:
    apiVersion: v1
    kind:       Secret

If they are redundant, we should drop the annotation, if they are not we should explain how they are different.

Yes, the annotation is redundant. I am fine with dropping it.

baijum commented 3 years ago

Edit 2: Emphasize this is a Custom Projection

Why limit this to Custom Projections? The source of the service secret is orthogonal to how it is projected into an application.

Yes, there is no need to restrict this into Custom Projection. But this feature should be still an extension for the sake of portability, right?

baijum commented 3 years ago

I have changed the extention proposal without using Custom Projection.

## Direct Secret Projection

There are scenarios where Provisioned Service is not available.  But there is a
`Secret` resource available for binding.  This extension, named Direct Secret
Projection, allows Service Binding resource with the service directly pointing
to a secret resource.

The `Secret` **SHOULD** contain a `type` entry with a value that identifies the
abstract classification of the binding.  It is **RECOMMENDED** that the `Secret`
also contain a `provider` entry with a value that identifies the provider of the
binding.  The `Secret` **MAY** contain any other entry.

The `.spec.service.kind` attribute **MUST** be `Secret` and the
`.spec.service.apiVersion` **MUST** be `v1`.  The `spec.service.name` attribute
**MUST** be a `LocalObjectReference`-able to a `Secret`.

### Direct Secret Projection Example Resource

```yaml
apiVersion: service.binding/v1alpha2
kind: ServiceBinding
metadata:
  name: account-service

spec:
  application:
    apiVersion: apps/v1
    kind:       Deployment
    name:       online-banking

  service:
    apiVersion: v1
    kind:       Secret
    name:       prod-account-service-secret

status:
  binding:
    name: prod-account-service-projection
  conditions:
  - type:   Ready
    status: 'True'
arthurdm commented 3 years ago

hey @baijum - sorry for the delay in responding to this. I like the latest proposal a lot better 👍 I definitely agree this is orthogonal to projection - this is about the "other" side, the provisioned service just having a single Secret. The projected app should see no differences.

An extension sounds like the right way to go, since we already have annotations / x-descriptors as extensions.

Basically the options for a service provider would be: (a) status.binding duck type (core spec, recommended) (b) Secret as-is (extension) (c) annotations / x-descriptors (extension)

If you wanted to make a PR with your latest post, that would be awesome @baijum! I appreciate that.

My suggestion is to call it something like Direct Secret Reference (dropping the word Projection to avoid confusion).

baijum commented 3 years ago

Thanks for the feedback @arthurdm I have sent a PR: #104

arthurdm commented 3 years ago

Closing as completed