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

CRD API groups should be a domain name #113

Closed
scothis opened this issue Sep 28, 2020 · 28 comments · Fixed by #185
Closed

CRD API groups should be a domain name #113

scothis opened this issue Sep 28, 2020 · 28 comments · Fixed by #185

Comments

@scothis
Copy link
Contributor

scothis commented Sep 28, 2020

SIG Architecture API Conventions:

When choosing a group name, we recommend selecting a subdomain your group or organization owns, such as "widget.mycompany.com".

service.binding is not a domain, and it's certainly not a domain that we control.

Changing the API Version is a major breaking change that while not impossible, is difficult to manage gracefully. We should address this concern before 1.0.

Buying a domain opens up questions around who actually own the domain, and implicitly the project. Getting adopted by a K8s SIG would alleviate the need to buy a domain, as we could roll up under *.k8s.io.

@navidsh navidsh added the RC3 label Sep 29, 2020
@sbose78
Copy link
Contributor

sbose78 commented Sep 30, 2020

Good point @scothis , I now remember I did get this feedback at Red Hat when we were planning to move to service.binding, which is why we didn't move to this.

@sbose78
Copy link
Contributor

sbose78 commented Oct 1, 2020

@arthurdm @scothis I feel this implies that the ServiceBinding CRD portability isn't truly possible unless the API gets adopted by a SIG, given that we shouldn't be shipping an implementation with the API group service.binding now ? Thoughts?

We've, at least for now, pivoted to a consistent Kind but a different group :

apiVersion: operators.coreos.com/v1alpha1
kind: ServiceBinding

You know what I'm getting at.. ;)

@arthurdm
Copy link
Member

arthurdm commented Oct 1, 2020

hey @sbose78 - if the spec proposed a duck type for ServiceBinding but allowed implementations to use its own apiVersion, would that help with SBO's adoption?

In other words, a CR would be deemed spec compliant if it supported the exact syntax (not equivalent syntax) proposed by the spec's ServiceBinding - but it was also free to support additional things.

For example, scope the spec's duck type to be something like [1]. The spec, status, etc, of the implementation could support additional things, but as long as it supported exactly [1] it would be compliant.

@scothis - this idea would mean the spec is then free from the requirement of not having a domain apiVerison.

[1]

apiVersion: <implementation_detail>
kind: ServiceBinding
metadata:
  name:                 # string
  generation:           # int64, defined by the Kubernetes control plane
  ...
spec:
  name:                 # string, optional, default: .metadata.name
  type:                 # string, optional
  provider:             # string, optional

  application:          # PodSpec-able resource ObjectReference-like
    apiVersion:         # string
    kind:               # string
    name:               # string, mutually exclusive with selector
    selector:           # metav1.LabelSelector, mutually exclusive with name
    containers:         # []intstr.IntOrString, optional

  service:              # Provisioned Service resource ObjectReference-like
    apiVersion:         # string
    kind:               # string
    name:               # string

  mappings:             # []Mapping, optional
  - name:               # string
    value:              # string

  env:                  # []EnvVar, optional
  - name:               # string
    key:                # string

status:
  binding:              # LocalObjectReference, optional
    name:               # string
  conditions:           # []Condition containing at least one entry for `Ready`
  - type:               # string
    status:             # string
    lastTransitionTime: # Time
    reason:             # string
    message:            # string
  observedGeneration:   # int64

@scothis
Copy link
Contributor Author

scothis commented Oct 1, 2020

I feel this implies that the ServiceBinding CRD portability isn't truly possible unless the API gets adopted by a SIG

There are a lot of valid domain names. k8s.io is just one of them. The advantage of folding under an existing community is that there is an obvious place to hold the domain registration or would have an appropriate existing domain.

@scothis
Copy link
Contributor Author

scothis commented Oct 1, 2020

if the spec proposed a duck type for ServiceBinding but allowed implementations to use its own apiVersion

In essence, this suggestion would gut any notion of portability. Duck types are interfaces. You can read and update a resource based on an interface, but you cannot create an instance directly from an interface. A concrete resource inherently requires more information than an interface provides.

@arthurdm
Copy link
Member

arthurdm commented Oct 2, 2020

A concrete resource inherently requires more information than an interface provides.

I think that's only true if you want to go beyond what the duck type interface provides. As a user, I just need to know what apiVersion I should put into my ServiceBinding CR, and I can safely use any of the elements defined in the interface. If I stay within the spec then my CR can be ported by simply changing the apiVersion according to the new target framework.

Example 1

apiVersion: abc.io/v1alpha2
kind: ServiceBinding
metadata:
  name: account-service
spec:
  application:
    apiVersion: apps/v1
    kind:       Deployment
    name:       online-banking

  service:
    apiVersion: com.example/v1alpha1
    kind:       AccountService
    name:       prod-account-service

Example 2 (ported by just changing the apiVersion)

apiVersion: xyz.io/v1alpha2
kind: ServiceBinding
metadata:
  name: account-service
spec:
  application:
    apiVersion: apps/v1
    kind:       Deployment
    name:       online-banking

  service:
    apiVersion: com.example/v1alpha1
    kind:       AccountService
    name:       prod-account-service

@baijum
Copy link
Contributor

baijum commented Oct 2, 2020

I have created a separate issue to track SIG proposal #116

@scothis
Copy link
Contributor Author

scothis commented Oct 2, 2020

@arthurdm If I'm shipping a component that needs to create a ServiceBinding resource, which API group do I create the resource within? How can I discover what options are available? Who sets up the RBAC permissions for these resources?

@arthurdm
Copy link
Member

arthurdm commented Oct 2, 2020

I was thinking that a query for ServiceBinding CRDs would expose the api groups available to use, which also addresses #114 as a bonus =)

@sbose78
Copy link
Contributor

sbose78 commented Oct 2, 2020

I see what you mean, Scott 👍

@scothis
Copy link
Contributor Author

scothis commented Oct 2, 2020

I was thinking that a query for ServiceBinding CRDs would expose the api groups available to use

An intelligent client/controller could do that, but it's a high burden to place on users. Applying static yaml wouldn't work.

@arthurdm
Copy link
Member

arthurdm commented Oct 5, 2020

ya - I definitely agree that having a static yaml that we can just apply to any spec compliant framework offers the easiest user experience.

I think though, having the ability to duck type the ServiceBinding CR brings value (e.g. solution for #114 and adoption from existing systems like SBO), that overweighs the added step needed to insert the proper apiVersion into it.

In a lot of CI/CD pipelines there's already token substitutions (for things like image name, env vars, etc), so I think it's very reasonable to also have a token for <SERVICE_BINDING_API_VERSION> that gets replaced before kubectl apply.

@arthurdm
Copy link
Member

arthurdm commented Oct 9, 2020

overall though, I do agree with @scothis' points about the added burden this puts on the ServiceBinding authors. So maybe we can push the duck type idea to the parking lot. =)

@baijum
Copy link
Contributor

baijum commented Oct 9, 2020

I think we can use a domain managed by 2 or 3 individuals for now. If this spec becomes part of Kubernetes through some SIG, probably we can release the 2.0 version of the spec with a new API group domain.

I have registered servicebinding.org through Google Domains using my personal account.
I can add two more Google accounts to manage the domain. All these accounts will have full permission to manage this domain.
Refer: https://support.google.com/domains/answer/7179397?hl=en

My original idea for the domain was to use it as a website for the spec. I think we can use the same domain for the website and API group domain.

@arthurdm
Copy link
Member

arthurdm commented Oct 9, 2020

thanks so much @baijum! this will be a good thing to discuss in the next hangout. I also own the https://github.com/servicebinding GH org, so that could be a potential new home for the code.

One of the biggest questions is who will pay for the costs of the domain? I am interested in hearing if others know the way this has worked for other communities.

In terms of admins to manage, the current admins for this spec repo are @nebhale and myself, so that's maybe a good starting set where we can add a Red Hat representative to it.

@sbose78
Copy link
Contributor

sbose78 commented Oct 9, 2020

Don't we need service.binding ?

@nebhale
Copy link
Member

nebhale commented Oct 13, 2020

While this came up in our VMware review and is definitely something that needs to be addressed, I’m loath to make this change now, with another potential Kubernetes-domain change later (we’re already on our second one and a fourth makes me queasy). Instead, I’d like to consider this a “pre-1.0” requirement, but explore the Kubernetes-SIG angle a bit more before we make any change.

If we get to the end and don’t find a SIG home, one of our domains makes sense, but let’s not jump there immediately.

@jhvhs
Copy link

jhvhs commented May 13, 2021

@baijum
Copy link
Contributor

baijum commented Jun 1, 2021

I would propose to use the API group name as binding.x-k8s.io. This is how the CRD name and full name looks like:

  1. ServiceBinding => servicebindings.binding.x-k8s.io
  2. ClusterApplicationResourceMapping => clusterapplicationresourcemappings.binding.x-k8s.io

Later we can ask for a separate API group for the proposed mappings (#145), mapping.binding.x-k8s.io

Let me know your thoughts!

@scothis
Copy link
Contributor Author

scothis commented Jun 3, 2021

binding.x-k8s.io is a good place to start. If we take this step, then we know there will be a next step to drop the x-, forward progress is good.

@baijum
Copy link
Contributor

baijum commented Jun 4, 2021

binding.x-k8s.io is a good place to start. If we take this step, then we know there will be a next step to drop the x-, forward progress is good.

The Kubernetes API Review Process has a bullet point like this:

  • SIG sponsored CRD based APIs outside of the core that use the "*.x-k8s.io" namespace.

@jhvhs Does that mean it will always be the x-k8s.io domain?

(The discussion in the PR kubernetes/community#3875 looks like that to me)

@scothis
Copy link
Contributor Author

scothis commented Jun 4, 2021

The Kubernetes API Review Process has a bullet point like this:

Since it's under the "Voluntary" heading, I take these points to mean that *.x-k8s.io is a free for all, and it's the *.k8s.io where review is required.

@baijum
Copy link
Contributor

baijum commented Jun 5, 2021

The Kubernetes API Review Process has a bullet point like this:

Since it's under the "Voluntary" heading, I take these points to mean that *.x-k8s.io is a free for all, and it's the *.k8s.io where review is required.

I think it's free for all "SIG sponsored" projects.

@baijum
Copy link
Contributor

baijum commented Aug 25, 2021

Based on the last community call discussion, Red Hat's open-source program office has registered the servicebinding.io domain. See more discussion here:
https://kubernetes.slack.com/archives/C012F2GPMTQ/p1629396455006900

I would suggest using a subdomain (binding.servicebinding.io) instead of the naked domain (servicebinding.io) for the API group name. Using subdomains for API group names is a common practice in the Kubernetes community.

@scothis
Copy link
Contributor Author

scothis commented Aug 25, 2021

binding.servicebinding.io works for me, should we put this into a PR? Should we reset the version number to v1alpha1 since it's a "new" resource?

@baijum
Copy link
Contributor

baijum commented Aug 25, 2021

binding.servicebinding.io works for me, should we put this into a PR?

+1

Should we reset the version number to v1alpha1 since it's a "new" resource?

I would prefer to increment the version to v1alpha3 to indicate API changes. This is not a strong preference. So, let's hear from others also.

cc. @arthurdm @nebhale

@scothis
Copy link
Contributor Author

scothis commented Aug 25, 2021

#185 bumps the current version to v1alpha3 I can either enhance that PR with the new api group, or we can create a new PR.

@baijum
Copy link
Contributor

baijum commented Aug 25, 2021

#185 bumps the current version to v1alpha3 I can either enhance that PR with the new api group, or we can create a new PR.

Yeah. #185 does that already. Please take the approach that is convenient for you. Anyway, there are not many review comments in that PR, so nothing to lose.

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.

7 participants