Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotation for ServiceBinding resource in the absence of an extension #109

Closed
baijum opened this issue Sep 24, 2020 · 6 comments
Closed

Comments

@baijum
Copy link
Member

baijum commented Sep 24, 2020

PR #104 is introducing the Direct Secret Reference extension. Currently, there is a Custom Projection extension. If there is an annotation that is used in the absence of any extension, it would be easy for implementors. This will also help those who are creating new extensions; the code is not going to break as soon as they introduce non-compatible changes.

I propose to add this annotation for the ServiceBinding resource in the absence of an extension:

service.binding/type: "Standard"
@scothis
Copy link
Contributor

scothis commented Sep 25, 2020

If a spec extensions changes the default behavior so significantly that it breaks the behavior of existing resources. I'd argue that any implementation of that extension would be out of conformance with the core spec.

@baijum
Copy link
Member Author

baijum commented Sep 25, 2020

If an implementor decides to implement core specification without any extension. Currently, they need to put logic in the controller to skip any incompatible ServiceBinding resources based on all the supported extensions. When the spec introduces new extensions in the upcoming versions, the code will be required to change again. I think in addition to the above-mentioned annotation, probably there should be a spec version number also. Something like this:

service.binding/spec-version: "1.0"

@scothis
Copy link
Contributor

scothis commented Sep 25, 2020

If an implementor decides to implement core specification without any extension. Currently, they need to put logic in the controller to skip any incompatible ServiceBinding resources based on all the supported extensions.

That's not how spec extensions work. The core behavior is the only portable, guaranteed behavior; extensions are extensions of the core behavior. Creators of a ServiceBinding cannot rely on extension behavior unless that have direct knowledge that the install supports that extension.

When the spec introduces new extensions in the upcoming versions, the code will be required to change again.

If we ship 1.0 of the spec, and then add a new extension in 1.1, every 1.0 conformant implementations does not suddenly become non-conformant. If so, we failed as spec authors.

I think in addition to the above-mentioned annotation, probably there should be a spec version number also. Something like this:

service.binding/spec-version: "1.0"

K8s resources already have an api version number.

apiVersion: service.binding/v1alpha2
kind: ServiceBinding
...

@navidsh
Copy link
Contributor

navidsh commented Sep 29, 2020

This was discussed during the interlock call. Continuing the discussion here...

@baijum
Copy link
Member Author

baijum commented Sep 30, 2020

From yesterday's interlock call, these are my understanding. The spec is not extensible, but it has well-defined extensions. The core spec and extensions are coherent to each other. The implementation should be aware of extensions, even if it doesn't implement it. This way, implementation can display better error messages when it doesn't support a particular extension.

So, I am closing this issue.

@baijum baijum closed this as completed Sep 30, 2020
@scothis
Copy link
Contributor

scothis commented Sep 30, 2020

The spec is not extensible, but it has well-defined extensions.

The spec very much is extensible. There are extensions that we publish along side the spec, but there's no reason why extensions need to be versioned with the core. An implementation may define its own extensions to the spec so long as the behavior remains conformant with the core spec.

The rub with extensions is that they are inherently not portable between implementations since there is no guarantee that the other implementation supports the same extensions. For portable behavior, you can only depend on the core.

The implementation should be aware of extensions, even if it doesn't implement it.

I strongly disagree. The core spec is perfectly capable without awareness of any extensions, this is a fundamental requirement for the core. Extensions are aware of the core semantics and add new capabilities in a compatible way. An extension that brings the core out of conformance would mean the implementation of that extension would no longer conform to the core spec.

This way, implementation can display better error messages when it doesn't support a particular extension.

The failure mode for a user attempting to consume an unimplemented extension is dependent on what the extended behavior is. For example, with the direct secret extension the failure mode would be that the service won't resolve to a secret. We don't need to extend the core to add that behavior, that's the default behavior of the core spec.

It's the responsibility of extension authors to create extensions that layer on top of the core behavior in a way that is compatible with the core. If the implementation chooses to be aware of common extensions it doesn't support that fine, but we cannot require that behavior since there is no way to know of all possible extensions.

The user is responsible for ensuring the system implements any extension they depend upon. It's worth discussing how we can make enabled extensions detectable for smarter clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants