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

should we be using CRD openapi v3 schema validation for the status section #6

Closed
huizengaJoe opened this issue Mar 11, 2020 · 12 comments · Fixed by #64
Closed

should we be using CRD openapi v3 schema validation for the status section #6

huizengaJoe opened this issue Mar 11, 2020 · 12 comments · Fixed by #64
Milestone

Comments

@huizengaJoe
Copy link

Does it make sense to use CRD openapi v3 schema validation for the status section as a way to enforce consistency in naming the binding items?

It would be great if we had a way to:

  1. Know/Discover that a service supported binding
  2. Add some tooling like a new operator-sdk scorecard test/linting rules that could check for consistent binding item names.... for those services that support binding

Thoughts?

@arthurdm
Copy link
Member

hey @huizengaJoe - I really like the idea of having an operator-sdk scorecard test that would check for spec compliancy. Is that something we could contribute somewhere?

I am not sure if that necessarily means something in the OAS3 schema, since from what I have seen that's mostly to validate spec values that users would input in the CR. It's not to say it's not possible, I just don't know if it fits the common usage of OAS3 validation.

@arthurdm
Copy link
Member

hey @huizengaJoe - for your first point, I believe that will get addressed by PR #9 - please feel free to add comments there.

@nebhale
Copy link
Member

nebhale commented Jun 27, 2020

@arthurdm Is this issue still valid for the current state of the specification?

@scothis
Copy link
Contributor

scothis commented Jun 29, 2020

@nebhale it's stale since the CRD instance (which contains the schema) needs to be provided by the implementation.

@arthurdm
Copy link
Member

I guess the valid question is: should the spec provide a CRD for ServiceBinding, so that every implementation does not have to create one?

@nebhale
Copy link
Member

nebhale commented Jun 29, 2020

As in a reference implementation Go library?

@arthurdm
Copy link
Member

I was thinking just the CRD itself, since we may have non-Go implementations.

@nebhale
Copy link
Member

nebhale commented Jun 29, 2020

What concrete artifacts do you think this should be then? Just an OpenAPI schema definition?

@arthurdm
Copy link
Member

I was thinking the full CRD, including the OAS3 portion. Before the restructure of the spec we were pointing to this CRD, which was nice in terms of having a concrete CRD that implementations could share. Was looking for something equivalent.

@scothis
Copy link
Contributor

scothis commented Jun 29, 2020

I guess the valid question is: should the spec provide a CRD for ServiceBinding, so that every implementation does not have to create one?

The CRD definition is inextricably linked to the implementation since the versions severed and storage flags as well as the conversion webhook are defined on the CRD resource. The most the spec can do is provide an exemplar CRD resource, but it should not be directly used by an implementation.

@arthurdm
Copy link
Member

ya, if >90% of the CRD will be the same, would be nice to provide it in the spec.

@nebhale
Copy link
Member

nebhale commented Jun 29, 2020

OK, I'll start work on a PR for an exemplar CRD.

@arthurdm arthurdm added the RC2 label Jun 30, 2020
@nebhale nebhale added this to the RC2 milestone Jun 30, 2020
nebhale added a commit that referenced this issue Jun 30, 2020
This change contributes an exemplar CRD for the ServiceBinding as it stands in
the specification today.  This document should be kept up to date as the spec
is updated.

[resolves #6]

Signed-off-by: Ben Hale <bhale@vmware.com>
@nebhale nebhale mentioned this issue Jun 30, 2020
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.

4 participants