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

Inconsistent information about type entry in binding secret #155

Closed
DhritiShikhar opened this issue May 20, 2021 · 5 comments · Fixed by #154
Closed

Inconsistent information about type entry in binding secret #155

DhritiShikhar opened this issue May 20, 2021 · 5 comments · Fixed by #154

Comments

@DhritiShikhar
Copy link

DhritiShikhar commented May 20, 2021

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
Copy link
Contributor

baijum commented May 20, 2021

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
Copy link
Author

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
Copy link
Contributor

baijum commented May 20, 2021

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
Copy link
Contributor

baijum commented May 20, 2021

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

@scothis
Copy link
Contributor

scothis commented May 20, 2021

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.

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

Successfully merging a pull request may close this issue.

3 participants