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

SIG Network: add best practices guide for API extensions #8104

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

shaneutt
Copy link
Member

This PR is a follow up to a mailing list discussion about the trend of new APIs being developed as CRDs in upstream.

This adds a guide to our community pages which provides help, insights and best practices for future CRD-based API developments based on the experiences of those who have done it previously (like Network Policy and Gateway API). The intention is to help make developing these CRD-based APIs easier for developers, but (most importantly) making the experience for end-users easier, and more consistent.

Note: this is in draft as there are several sections not yet filled out. It's not ready for general review yet. If you're interested helping with this doc, please feel to message me on Slack, or put a PR into my fork it would be great to collaborate with others on it!

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/network Categorizes an issue or PR as relevant to SIG Network. labels Oct 10, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: shaneutt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 10, 2024
@shaneutt shaneutt changed the title docs: add best practices guide for API extensions SIG Network: add best practices guide for API extensions Oct 10, 2024

* A desire to be free of lockstep with the [KEP] process
* Experimentation and highly iterative development
* The feature is useful to multiple parties, but is niche
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think another big reason for CRDs is for cases when the API is entirely implemented by out-of-core components.

With networkingv1.NetworkPolicy, the API exists whether or not anyone is implementing it (and exists at a particular version even if the implementation only supports an older version), and this is confusing and bad. For Gateway and AdminNetworkPolicy, this is not a problem; if the types are installed in the cluster, then that implies there is a controller implementing them.

and manage these kinds of "official, but add-on and optional" features, which
throughout this document we'll refer to as **Official CRDs**.

> **Note**: **This is not a standard**, but **projects should try and follow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> **Note**: **This is not a standard**, but **projects should try and follow
> **Note**: **This is not a standard**, but **SIG Network projects should try and follow

and maybe a comment here or elsewhere about how these ideas may apply to some other SIGs but this document hasn't been reviewed by any.

We generally recommend this pattern unless there's a strong reason not to.

> **Note**: When starting new projects, we advise having enhancement proposals
> preceed any code whatsoever, for posterity and to help ensure inclusion.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not clear what you mean by "inclusion". I'm thinking something like "to ensure that a variety of use cases are represented in the design" or "to ensure that the needs of all stakeholders in the community are represented in the design" or something? (ie, "no steamrolling")

Traditionally Kubernetes enhancements go through the [KEP] process to progress.
While it may be OK for a sub-project to use the KEP process for an extension
API, it may also be reasonable for that project to have their own exclusive
enhancement proposal process specific to the project.
Copy link
Contributor

@danwinship danwinship Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't really talk about why/when this might make sense.

Comment on lines +81 to +83
> **Note**: Taking this approach doesn't mean complete autonomy. Any APIs
> (extension, or otherwise) **MUST** go through the API review process with the
> SIG Network leads for any content considered stable/standard/GA. Experimental
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*.k8s.io APIs must go through the (kubernetes) API review process with (kubernetes) API reviewers. It is not sufficient to just get buy-in from the SIG leads.

I'm not sure if we have any rules about non-*.k8s.io APIs.

[geps]:https://github.com/kubernetes-sigs/gateway-api/blob/main/geps/overview.md
[npeps]:https://github.com/kubernetes-sigs/network-policy-api/tree/main/npeps

#### Motivation Focused Enhancement Proposals
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to KEPs too. And "new working group" proposals...


### Conformance Levels

TODO - from Gateway API conformance levels
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So while "experimental" vs "standard" is a useful distinction, I think that "core" vs "extended" is generally an anti-pattern. Having a standard API is only useful if... you have a standard API.

I think "core" vs "extended" can only be justified if

  1. It is assumed that most implementations of the API will rely on some external "backend" that actually does the bulk of the work (eg, Gateway being built on top of nginx or GCP load balancers; or AdminNetworkPolicy being built on top of OVN ACLs or AWS security groups), AND
  2. There is a feature that everyone agrees really needs to be in the API in order for the API to be viable, AND
  3. Some "backends" can't implement that desired feature, and it is not reasonable to say "well then I guess you can't use that backend to implement our API" because either
    1. the choice of backend is inherently constrained in some environments (eg, you can't tell people on AWS to just use the GCP-based implementation of Gateway instead), OR
    2. the choice of backend was made well before the new feature was added (ie, a newly-proposed feature can't be implemented by some existing implementations of the API, and you don't want to retroactively force them into non-compliance).

But this should be thought of more as a regrettable one-off sort of thing than a normal feature of the API development process.

In particular, "core" vs "extended" is not appropriate for cases like "some people don't want to implement this feature because they don't think it's worth the effort to do so".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Note: this comment is more inspired by discussions I've seen within NPAPI WG than by Gateway, which I'm less familiar with. The only "extended" Gateway feature I know off the top of my head is regex matching, which I believe meets the criteria above.)

> **Note**: This is **not a comprehensive guide**.

[CRDs]:https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/
[in-tree]:https://github.com/kubernetes/kubernetes/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kubernetes/api may be a better link? or both?

* Conformance Tests & Reports
* CRD Lifecycle Management
* Documentation
* Community & Evangelism
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Community & Evangelism" seems more of a WG thing in general than a CRD vs non-CRD thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. sig/network Categorizes an issue or PR as relevant to SIG Network. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants