servicebinding / spec

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

Inconsistent information about `type` entry in binding secret #155

Closed DhritiShikhar closed 3 years ago

DhritiShikhar commented 3 years ago

The Provisioned Service section of the README says:

The Secret data SHOULD contain a type entry with a value that identifies the abstract classification of the binding. 

The Application Projection section of the README says:

The Secret data MUST contain a type entry with a value that identifies the abstract classification of the binding.

The difference is between SHOULD and MUST convention.

baijum commented 3 years ago

With the current spec (before merging PR #154), it is possible to add a type entry in the mapping and create an intermediate Secret resource to satisfy the mandatory clause about type entry specified in the Application Projection section.

Probably PR #154 should address this issue. cc. @scothis

DhritiShikhar commented 3 years ago

If the behaviour is to add a type key with a random value just for the sake of satisfying a mandatory clause, then it will not be useful.

May be the type key should NOT be mandatory.

baijum commented 3 years ago

with a random value

It's not an arbitrary value. The mapping is provided by the user who knows about the Provisioned Service. When mapping is getting removed, I think we can make type a mandatory entry in the Provisioned Service. Once we have the proposed extensions (See #145), the value for type could be satisfied through those extensions.

baijum commented 3 years ago

I missed .spec.type attribute. Since .spec.type can override the original value. Both statements holds true even after PR #154 merged.

scothis commented 3 years ago

Thanks for the eagle eyes @DhritiShikhar

The difference between the provisioned service and application projection requirements was intentional. The type field needs to be defined before it is injected into an application, but we left wiggle room for a provisioned service to not specify the field since a mapping could set the value.

In the context of #154 that distinction is a bit less meaningful, but I'm not sure it's worth making the SHOULD for the provisioned service into a MUST. If we do want to leave the requirement as SHOULD, we should add a blurb that the type field must be set before it is consumed by an application workload.