Create KEP for built-in Defaulting

keps/sig-api-machinery/1929-built-in-default/README.md

@@ -0,0 +1,316 @@
+<!--
+**Note:** When your KEP is complete, all of these comment blocks should be removed.
+
+To get started with this template:
+
+- [x] **Pick a hosting SIG.**
+  Make sure that the problem space is something the SIG is interested in taking
+  up. KEPs should not be checked in without a sponsoring SIG.
+- [ ] **Create an issue in kubernetes/enhancements**
+  When filing an enhancement tracking issue, please make sure to complete all
+  fields in that template. One of the fields asks for a link to the KEP. You
+  can leave that blank until this KEP is filed, and then go back to the
+  enhancement and add the link.
+- [x] **Make a copy of this template directory.**
+  Copy this template into the owning SIG's directory and name it
+  `NNNN-short-descriptive-title`, where `NNNN` is the issue number (with no
+  leading-zero padding) assigned to your enhancement above.
+- [x] **Fill out as much of the kep.yaml file as you can.**
+  At minimum, you should fill in the "Title", "Authors", "Owning-sig",
+  "Status", and date-related fields.
+- [ ] **Fill out this file as best you can.**
+  At minimum, you should fill in the "Summary" and "Motivation" sections.
+  These should be easy if you've preflighted the idea of the KEP with the
+  appropriate SIG(s).
+- [x] **Create a PR for this KEP.**
+  Assign it to people in the SIG who are sponsoring this process.
+- [ ] **Merge early and iterate.**
+  Avoid getting hung up on specific details and instead aim to get the goals of
+  the KEP clarified and merged quickly. The best way to do this is to just
+  start with the high-level sections and fill out details incrementally in
+  subsequent PRs.
+
+Just because a KEP is merged does not mean it is complete or approved. Any KEP
+marked as `provisional` is a working document and subject to change. You can
+denote sections that are under active debate as follows:
+
+```
+<<[UNRESOLVED optional short context or usernames ]>>
+Stuff that is being argued.
+<<[/UNRESOLVED]>>
+```
+
+When editing KEPS, aim for tightly-scoped, single-topic PRs to keep discussions
+focused. If you disagree with what is already in a document, open a new PR
+with suggested changes.
+
+One KEP corresponds to one "feature" or "enhancement" for its whole lifecycle.
+You do not need a new KEP to move from beta to GA, for example. If
+new details emerge that belong in the KEP, edit the KEP. Once a feature has become
+"implemented", major changes should get new KEPs.
+
+The canonical place for the latest set of instructions (and the likely source
+of this file) is [here](/keps/NNNN-kep-template/README.md).
+
+**Note:** Any PRs to move a KEP to `implementable`, or significant changes once
+it is marked `implementable`, must be approved by each of the KEP approvers.
+If none of those approvers are still appropriate, then changes to that list
+should be approved by the remaining approvers and/or the owning SIG (or
+SIG Architecture for cross-cutting KEPs).
+-->
+# KEP-NNNN: Built-in declarative defaults
+
+<!-- toc -->
+- [Release Signoff Checklist](#release-signoff-checklist)
+- [Summary](#summary)
+- [Motivation](#motivation)
+  - [Goals](#goals)
+  - [Non-Goals](#non-goals)
+- [Proposal](#proposal)
+  - [Risks and Mitigations](#risks-and-mitigations)
+- [Design Details](#design-details)
+  - [Marker format](#marker-format)
+  - [Test Plan](#test-plan)
+  - [Graduation Criteria](#graduation-criteria)
+    - [Beta -&gt; GA Graduation](#beta---ga-graduation)
+- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
+  - [Feature Enablement and Rollback](#feature-enablement-and-rollback)
+  - [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning)
+  - [Monitoring Requirements](#monitoring-requirements)
+  - [Dependencies](#dependencies)
+  - [Scalability](#scalability)
+- [Implementation History](#implementation-history)
+<!-- /toc -->
+
+## Release Signoff Checklist
+
+<!--
+**ACTION REQUIRED:** In order to merge code into a release, there must be an
+issue in [kubernetes/enhancements] referencing this KEP and targeting a release
+milestone **before the [Enhancement Freeze](https://git.k8s.io/sig-release/releases)
+of the targeted release**.
+
+For enhancements that make changes to code or processes/procedures in core
+Kubernetes—i.e., [kubernetes/kubernetes], we require the following Release
+Signoff checklist to be completed.
+
+Check these off as they are completed for the Release Team to track. These
+checklist items _must_ be updated for the enhancement to be released.
+-->
+
+Items marked with (R) are required *prior to targeting to a milestone / release*.
+
+- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR)
+- [ ] (R) KEP approvers have approved the KEP status as `implementable`
+- [ ] (R) Design details are appropriately documented
+- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
+- [ ] (R) Graduation criteria is in place
+- [ ] (R) Production readiness review completed
+- [ ] Production readiness review approved
+- [ ] "Implementation History" section is up-to-date for milestone
+- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
+- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes
+
+[kubernetes.io]: https://kubernetes.io/
+[kubernetes/enhancements]: https://git.k8s.io/enhancements
+[kubernetes/kubernetes]: https://git.k8s.io/kubernetes
+[kubernetes/website]: https://git.k8s.io/website
+
+## Summary
+
+Currently, built-in API types are written as Go structures annotated
+with markers to add additional semantics. This method has inspired
+controller-tools who has re-used and improved on that technique for
+building CRDs. We're proposing using the `// +default` marker to
+declaratively define defaults (and have kube-builder use that as well)
+since that mechanism still doesn't exist for built-in. This proposal
+tries to solve that gap.
+
+## Motivation
+
+Having a new `// +default` marker for built-in types has several
+important benefits:
+
+- It's more convenient for types authors.
+- The types and their behaviors are easier to understand since they are
+described together.
+- They are less error-prone, and prevent people from using anti-patterns
+like "cross-field dependency defaults" (default depends on another
+field), non-deterministic or complicated defaults. These patterns
+might be useful in limited circumstances, but it should be easier to do
+the right thing in most cases.
+- Removes the inconsistency with CRDs that already support this
+mechanism.
+- Solves bug where associative keys need to either be required or
+  defaulted.
+
+### Goals
+
+The goal is to add a new `// +default` tag to our current built-in Go
+IDL. That tag will be transformed into the OpenAPI `default` tag and
+then routed to defaulting functions so that defaulting can be done
+declaratively.
+
+### Non-Goals
+
+While this proposal is probably paving the way for additional OpenAPI
+based validation, validation is entirely out-of-scope for this project.
+
+This proposal also doesn't intend to remove the existing defaulting
+mechanism since some defaulting still requires non-static values that
+need to be computed and can't be defined declaratively. We could replace
+some defaulting that doesn't require to be done as code though.
+
+## Proposal
+
+Since the goal of this proposal is to increase consistency with the CRD
+defaulting, we propose to use the same mechanism (both in OpenAPI and
+similar to what kubebuilder does). This defaulting will happen BEFORE
+the existing defaulting functions are called (existing functions can be
+updated to panic so that we can see if defaults are properly applied).
+
+One very important nuance with CRDs is that CRDs are deserialized in
+unstructured objects that can distinguish between specified and
+unspecified fields. That is not the case for built-in types that are
+directly deserialized into Go-struct, where the unspecified fields are
+zero-valued. It's important to understand that only zero-value
+omit-empty fields will be defaulted using that mechanism.
+
+Computed defaults will not be supported as a declarative marker and will
+continue being supported through the existing defaulting functions.
+
+### Risks and Mitigations
+
+The main risk is that we could end-up with different defaults for
+existing types which would be extremely confusing for our users.
+
+## Design Details
+
+There are multiple challenges to running defaults declaratively and
+changing the current mechanisms:
+
+- We don't use OpenAPI to drive validation nor defaulting
+currently. There is no routing of the OpenAPI to most operations in
+kubernetes (the only thing right now is server-side apply).
+- OpenAPI fields don't strictly map to Go types fields
+(e.g. capitalization, embedding)
+- Reflection can't access comments, only structTags (which is why we have
+both structTags AND comments for SMP, but that's ugly and suboptimal).
+
+One possible way to address that second point is to serialize the object
+back to json, apply the default, and re-deserialize back into a
+go-struct, but that's memory intensive and Kubernetes doesn't need that.
+
+We're proposing to use the same mechanisms that we've put in place for
+server-side apply, implemented in
+"sigs.k8s.io/structured-merge-diff". We currently have a way for
+server-side apply to convert Go types into "json"-like values without
+serializing/deserializing and allocating memory by using reflection. We
+also have mechanisms in place to parse, process and use the OpenAPI.
+
+In other words, the plan would be the following:
+1. Update kube-openapi to parse the `+default` marker and include it in
+the OpenAPI definition
+2. Add additional "default" field to sigs.k8s.io/structured-merge-diff
+internal schema, and parse that field from the OpenAPI.
+3. Create a new "AddDefault" method in sigs.k8s.io/structured-merge-diff
+that receives a Value (value is an interface that can be backed by a
+reflection based Go-type, associated with the corresponding
+OpenAPI). That method would add the defaults in-place.
+4. Add and wire things so that this function can be called prior to
+running the existing defaulting code.
+
+### Marker format
+
+The marker will have the following format:
+```
+// +default=<any>
+```
+which describes the value that the server will use if the field has a
+zero-value.
+
+`<any>` has to be in one-line json format, representing the actual value
+to use by default.
+
+### Test Plan
+
+As soon as this is implemented and working, we can start removing a lot
+of the existing defaulting code and replace it with this values, and we
+can re-use the existing defaulting tests, make sure that the behavior
+doesn't change when moving from code-based defaults to declarative
+default. Also panic in specific places of the defaulters to guarantee
+that the defaulting have happened before.
+
+### Graduation Criteria
+
+The feature will be released as Beta and enabled by default. This is to
+give a chance to disable the feature if something goes wrong, but we're
+expecting GA quality from the start, with no changes.
+
+#### Beta -> GA Graduation
+
+Feature has been stable and worked as expected for a release.
+
+## Production Readiness Review Questionnaire
+
+### Feature Enablement and Rollback
+
+* **How can this feature be enabled / disabled in a live cluster?**
+  - [x] Feature gate (also fill in values in `kep.yaml`)
+    - Feature gate name: DeclarativeDefaults
+
+* **Does enabling the feature change any default behavior?** No
+
+* **Can the feature be disabled once it has been enabled (i.e. can we roll back
+  the enablement)?** Yes
+
+* **What happens if we reenable the feature if it was previously rolled back?** Nothing
+
+* **Are there any tests for feature enablement/disablement?** No
+
+### Rollout, Upgrade and Rollback Planning
+
+* **How can a rollout fail? Can it impact already running workloads?** No
+
+* **What specific metrics should inform a rollback?** None
+
+* **Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?** No
+
+* **Is the rollout accompanied by any deprecations and/or removals of features, APIs,
+fields of API types, flags, etc.?** No
+
+### Monitoring Requirements
+
+* **How can an operator determine if the feature is in use by workloads?** N/A
+
+* **What are the SLIs (Service Level Indicators) an operator can use to determine
+the health of the service?** None
+
+* **What are the reasonable SLOs (Service Level Objectives) for the above SLIs?** N/A
+
+* **Are there any missing metrics that would be useful to have to improve observability
+of this feature?** No
+
+### Dependencies
+
+* **Does this feature depend on any specific services running in the cluster?** No
+
+### Scalability
+
+* **Will enabling / using this feature result in any new API calls?** No
+
+* **Will enabling / using this feature result in introducing new API types?** No
+
+* **Will enabling / using this feature result in any new calls to the cloud
+provider?** No
+
+* **Will enabling / using this feature result in increasing size or count of
+the existing API objects?** No
+
+* **Will enabling / using this feature result in increasing time taken by any
+operations covered by [existing SLIs/SLOs]?** No
+
+* **Will enabling / using this feature result in non-negligible increase of
+resource usage (CPU, RAM, disk, IO, ...) in any components?** No
+
+## Implementation History

keps/sig-api-machinery/1929-built-in-default/kep.yaml

@@ -0,0 +1,21 @@
+title: Built-in declarative defaults
+kep-number: 1929
+authors:
+  - apelisse
+owning-sig: sig-api-machinery
+status: provisional
+creation-date: 2020-08-04
+reviewers:
+  - jpbetz
+  - lavalamp
+  - sttts
+approvers:
+  - TBD
+prr-approvers:
+  - deads2k
+see-also:
+  - "/keps/sig-api-machinery/20190426-crd-defaulting.md"
+stage: beta
+latest-milestone: "v1.20"
+milestone:
+  stable: "v1.20"