From 68a359276f5ab214bde4d1221ab1eac6c59ba836 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 9 Jun 2022 18:00:47 +0000
Subject: [PATCH 01/22] update kep.yaml

---
 keps/sig-api-machinery/1027-api-unions/kep.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/kep.yaml b/keps/sig-api-machinery/1027-api-unions/kep.yaml
index 6f74bebec83..86597fa8f4e 100644
--- a/keps/sig-api-machinery/1027-api-unions/kep.yaml
+++ b/keps/sig-api-machinery/1027-api-unions/kep.yaml
@@ -2,6 +2,7 @@ title: Union types
 kep-number: 1027
 authors:
   - "@apelisse"
+  - "@kevindelgado"
 owning-sig: sig-api-machinery
 participating-sigs:
 reviewers:
@@ -13,7 +14,7 @@ approvers:
   - "@lavalamp"
 editor: TBD
 creation-date: 2019-03-25
-last-updated: 2019-03-25
+last-updated: 2022-06-09
 status: implementable
 see-also:
   - "/keps/sig-api-machinery/0006-apply.md"
@@ -21,5 +22,5 @@ replaces:
   - "https://docs.google.com/document/d/1lrV-P25ZTWukixE9ZWyvchfFR0NE2eCHlObiCUgNQGQ"
 superseded-by:
 
-latest-milestone: "1.15"
+latest-milestone: "1.25"
 stage: "alpha"

From cd96e98a4edef66ca9ca33c3e47a4803d10feca0 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 9 Jun 2022 18:18:23 +0000
Subject: [PATCH 02/22] update kep readme to match current template

---
 .../1027-api-unions/README.md                 | 530 +++++++++++++++++-
 1 file changed, 522 insertions(+), 8 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 68d82ac53af..9736f7395fd 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -1,4 +1,4 @@
-# Union types
+# KEP-1027: Union types
 
 ## Table of Contents
 
@@ -9,6 +9,12 @@
   - [Goals](#goals)
   - [Non-Goals](#non-goals)
 - [Proposal](#proposal)
+  - [User Stories (Optional)](#user-stories-optional)
+    - [Story 1](#story-1)
+    - [Story 2](#story-2)
+  - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional)
+  - [Risks and Mitigations](#risks-and-mitigations)
+- [Design Details](#design-details)
   - [Go tags](#go-tags)
   - [OpenAPI](#openapi)
   - [Discriminator](#discriminator)
@@ -18,22 +24,54 @@
   - [Backward compatibilities properties](#backward-compatibilities-properties)
   - [Validation](#validation)
   - [Test Plan](#test-plan)
+      - [Prerequisite testing updates](#prerequisite-testing-updates)
+      - [Unit tests](#unit-tests)
+      - [Integration tests](#integration-tests)
+      - [e2e tests](#e2e-tests)
   - [Graduation Criteria](#graduation-criteria)
     - [Beta -&gt; GA Graduation](#beta---ga-graduation)
+  - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
+  - [Version Skew Strategy](#version-skew-strategy)
+- [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)
+  - [Troubleshooting](#troubleshooting)
 - [Future Work](#future-work)
 - [Implementation History](#implementation-history)
+- [Drawbacks](#drawbacks)
+- [Alternatives](#alternatives)
 <!-- /toc -->
 
 ## Release Signoff Checklist
 
-- [x] kubernetes/enhancements issue in release milestone, which links to KEP (this should be a link to the KEP location in kubernetes/enhancements, not the initial KEP PR)
-- [x] KEP approvers have set the KEP status to `implementable`
-- [x] Design details are appropriately documented
-- [x] Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
-- [x] Graduation criteria is in place
-- [x] "Implementation History" section is up-to-date for milestone
+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 (including test refactors)
+  - [ ] e2e Tests for all Beta API Operations (endpoints)
+  - [ ] (R) Ensure GA e2e tests for meet requirements for [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) 
+  - [ ] (R) Minimum Two Week Window for GA e2e tests to prove flake free
+- [ ] (R) Graduation criteria is in place
+  - [ ] (R) [all GA Endpoints](https://github.com/kubernetes/community/pull/1806) must be hit by [Conformance Tests](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/conformance-tests.md) 
+- [ ] (R) Production readiness review completed
+- [ ] (R) 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
+- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes
+
+<!--
+**Note:** This checklist is iterative and should be reviewed and updated every time this enhancement is being considered for a milestone.
+-->
+
+[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
 
@@ -102,6 +140,44 @@ proposing the following changes.
 Note that this proposes unions to be "at most one of". Whether exactly one is
 supported or not should be implemented by the validation logic.
 
+### User Stories (Optional)
+
+<!--
+Detail the things that people will be able to do if this KEP is implemented.
+Include as much detail as possible so that people can understand the "how" of
+the system. The goal here is to make this feel real for users without getting
+bogged down.
+-->
+
+#### Story 1
+
+#### Story 2
+
+### Notes/Constraints/Caveats (Optional)
+
+<!--
+What are the caveats to the proposal?
+What are some important details that didn't come across above?
+Go in to as much detail as necessary here.
+This might be a good place to talk about core concepts and how they relate.
+-->
+
+### Risks and Mitigations
+
+<!--
+What are the risks of this proposal, and how do we mitigate? Think broadly.
+For example, consider both security and how this will impact the larger
+Kubernetes ecosystem.
+
+How will security be reviewed, and by whom?
+
+How will UX be reviewed, and by whom?
+
+Consider including folks who also work outside the SIG or subproject.
+-->
+
+## Design Details
+
 ### Go tags
 
 We're proposing a new type of tags for go types (in-tree types, and also
@@ -276,6 +352,66 @@ There are mostly 3 aspects to this plan:
 - [x] Functionality as part of server-side apply: How human and robot interactions work: https://github.com/kubernetes-sigs/structured-merge-diff/blob/master/typed/union_test.go
 - [x] Integration in kubernetes: https://github.com/kubernetes/kubernetes/pull/77370/files#diff-4ac5831d494b1b52c7c7be81e552a458
 
+[ ] I/we understand the owners of the involved components may require updates to
+existing tests to make this code solid enough prior to committing the changes necessary
+to implement this enhancement.
+
+##### Prerequisite testing updates
+
+<!--
+Based on reviewers feedback describe what additional tests need to be added prior
+implementing this enhancement to ensure the enhancements have also solid foundations.
+-->
+
+##### Unit tests
+
+<!--
+In principle every added code should have complete unit test coverage, so providing
+the exact set of tests will not bring additional value.
+However, if complete unit test coverage is not possible, explain the reason of it
+together with explanation why this is acceptable.
+-->
+
+<!--
+Additionally, for Alpha try to enumerate the core package you will be touching
+to implement this enhancement and provide the current unit coverage for those
+in the form of:
+- <package>: <date> - <current test coverage>
+The data can be easily read from:
+https://testgrid.k8s.io/sig-testing-canaries#ci-kubernetes-coverage-unit
+
+This can inform certain test coverage improvements that we want to do before
+extending the production code to implement this enhancement.
+-->
+
+- `<package>`: `<date>` - `<test coverage>`
+
+##### Integration tests
+
+<!--
+This question should be filled when targeting a release.
+For Alpha, describe what tests will be added to ensure proper quality of the enhancement.
+
+For Beta and GA, add links to added tests together with links to k8s-triage for those tests:
+https://storage.googleapis.com/k8s-triage/index.html
+-->
+
+- <test>: <link to test coverage>
+
+##### e2e tests
+
+<!--
+This question should be filled when targeting a release.
+For Alpha, describe what tests will be added to ensure proper quality of the enhancement.
+
+For Beta and GA, add links to added tests together with links to k8s-triage for those tests:
+https://storage.googleapis.com/k8s-triage/index.html
+
+We expect no non-infra related flakes in the last month as a GA graduation criteria.
+-->
+
+- <test>: <link to test coverage>
+
 ### Graduation Criteria
 
 Since this project is a sub-project of Server-side apply, it will be introduced
@@ -288,6 +424,367 @@ criteria below.
 - Core-types all implement the semantics properly
 - Stable and bug-free for two releases
 
+### Upgrade / Downgrade Strategy
+
+<!--
+If applicable, how will the component be upgraded and downgraded? Make sure
+this is in the test plan.
+
+Consider the following in developing an upgrade/downgrade strategy for this
+enhancement:
+- What changes (in invocations, configurations, API use, etc.) is an existing
+  cluster required to make on upgrade, in order to maintain previous behavior?
+- What changes (in invocations, configurations, API use, etc.) is an existing
+  cluster required to make on upgrade, in order to make use of the enhancement?
+-->
+
+### Version Skew Strategy
+
+<!--
+If applicable, how will the component handle version skew with other
+components? What are the guarantees? Make sure this is in the test plan.
+
+Consider the following in developing a version skew strategy for this
+enhancement:
+- Does this enhancement involve coordinating behavior in the control plane and
+  in the kubelet? How does an n-2 kubelet without this feature available behave
+  when this feature is used?
+- Will any other components on the node change? For example, changes to CSI,
+  CRI or CNI may require updating that component before the kubelet.
+-->
+
+## Production Readiness Review Questionnaire
+
+<!--
+
+Production readiness reviews are intended to ensure that features merging into
+Kubernetes are observable, scalable and supportable; can be safely operated in
+production environments, and can be disabled or rolled back in the event they
+cause increased failures in production. See more in the PRR KEP at
+https://git.k8s.io/enhancements/keps/sig-architecture/1194-prod-readiness.
+
+The production readiness review questionnaire must be completed and approved
+for the KEP to move to `implementable` status and be included in the release.
+
+In some cases, the questions below should also have answers in `kep.yaml`. This
+is to enable automation to verify the presence of the review, and to reduce review
+burden and latency.
+
+The KEP must have a approver from the
+[`prod-readiness-approvers`](http://git.k8s.io/enhancements/OWNERS_ALIASES)
+team. Please reach out on the
+[#prod-readiness](https://kubernetes.slack.com/archives/CPNHUMN74) channel if
+you need any help or guidance.
+-->
+
+### Feature Enablement and Rollback
+
+<!--
+This section must be completed when targeting alpha to a release.
+-->
+
+###### How can this feature be enabled / disabled in a live cluster?
+
+<!--
+Pick one of these and delete the rest.
+
+Documentation is available on [feature gate lifecycle] and expectations, as
+well as the [existing list] of feature gates.
+
+[feature gate lifecycle]: https://git.k8s.io/community/contributors/devel/sig-architecture/feature-gates.md
+[existing list]: https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/
+-->
+
+- [ ] Feature gate (also fill in values in `kep.yaml`)
+  - Feature gate name:
+  - Components depending on the feature gate:
+- [ ] Other
+  - Describe the mechanism:
+  - Will enabling / disabling the feature require downtime of the control
+    plane?
+  - Will enabling / disabling the feature require downtime or reprovisioning
+    of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled).
+
+###### Does enabling the feature change any default behavior?
+
+<!--
+Any change of default behavior may be surprising to users or break existing
+automations, so be extremely careful here.
+-->
+
+###### Can the feature be disabled once it has been enabled (i.e. can we roll back the enablement)?
+
+<!--
+Describe the consequences on existing workloads (e.g., if this is a runtime
+feature, can it break the existing applications?).
+
+Feature gates are typically disabled by setting the flag to `false` and
+restarting the component. No other changes should be necessary to disable the
+feature.
+
+NOTE: Also set `disable-supported` to `true` or `false` in `kep.yaml`.
+-->
+
+###### What happens if we reenable the feature if it was previously rolled back?
+
+###### Are there any tests for feature enablement/disablement?
+
+<!--
+The e2e framework does not currently support enabling or disabling feature
+gates. However, unit tests in each component dealing with managing data, created
+with and without the feature, are necessary. At the very least, think about
+conversion tests if API types are being modified.
+
+Additionally, for features that are introducing a new API field, unit tests that
+are exercising the `switch` of feature gate itself (what happens if I disable a
+feature gate after having objects written with the new field) are also critical.
+You can take a look at one potential example of such test in:
+https://github.com/kubernetes/kubernetes/pull/97058/files#diff-7826f7adbc1996a05ab52e3f5f02429e94b68ce6bce0dc534d1be636154fded3R246-R282
+-->
+
+### Rollout, Upgrade and Rollback Planning
+
+<!--
+This section must be completed when targeting beta to a release.
+-->
+
+###### How can a rollout or rollback fail? Can it impact already running workloads?
+
+<!--
+Try to be as paranoid as possible - e.g., what if some components will restart
+mid-rollout?
+
+Be sure to consider highly-available clusters, where, for example,
+feature flags will be enabled on some API servers and not others during the
+rollout. Similarly, consider large clusters and how enablement/disablement
+will rollout across nodes.
+-->
+
+###### What specific metrics should inform a rollback?
+
+<!--
+What signals should users be paying attention to when the feature is young
+that might indicate a serious problem?
+-->
+
+###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?
+
+<!--
+Describe manual testing that was done and the outcomes.
+Longer term, we may want to require automated upgrade/rollback tests, but we
+are missing a bunch of machinery and tooling and can't do that now.
+-->
+
+###### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.?
+
+<!--
+Even if applying deprecation policies, they may still surprise some users.
+-->
+
+### Monitoring Requirements
+
+<!--
+This section must be completed when targeting beta to a release.
+
+For GA, this section is required: approvers should be able to confirm the
+previous answers based on experience in the field.
+-->
+
+###### How can an operator determine if the feature is in use by workloads?
+
+<!--
+Ideally, this should be a metric. Operations against the Kubernetes API (e.g.,
+checking if there are objects with field X set) may be a last resort. Avoid
+logs or events for this purpose.
+-->
+
+###### How can someone using this feature know that it is working for their instance?
+
+<!--
+For instance, if this is a pod-related feature, it should be possible to determine if the feature is functioning properly
+for each individual pod.
+Pick one more of these and delete the rest.
+Please describe all items visible to end users below with sufficient detail so that they can verify correct enablement
+and operation of this feature.
+Recall that end users cannot usually observe component logs or access metrics.
+-->
+
+- [ ] Events
+  - Event Reason: 
+- [ ] API .status
+  - Condition name: 
+  - Other field: 
+- [ ] Other (treat as last resort)
+  - Details:
+
+###### What are the reasonable SLOs (Service Level Objectives) for the enhancement?
+
+<!--
+This is your opportunity to define what "normal" quality of service looks like
+for a feature.
+
+It's impossible to provide comprehensive guidance, but at the very
+high level (needs more precise definitions) those may be things like:
+  - per-day percentage of API calls finishing with 5XX errors <= 1%
+  - 99% percentile over day of absolute value from (job creation time minus expected
+    job creation time) for cron job <= 10%
+  - 99.9% of /health requests per day finish with 200 code
+
+These goals will help you determine what you need to measure (SLIs) in the next
+question.
+-->
+
+###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service?
+
+<!--
+Pick one more of these and delete the rest.
+-->
+
+- [ ] Metrics
+  - Metric name:
+  - [Optional] Aggregation method:
+  - Components exposing the metric:
+- [ ] Other (treat as last resort)
+  - Details:
+
+###### Are there any missing metrics that would be useful to have to improve observability of this feature?
+
+<!--
+Describe the metrics themselves and the reasons why they weren't added (e.g., cost,
+implementation difficulties, etc.).
+-->
+
+### Dependencies
+
+<!--
+This section must be completed when targeting beta to a release.
+-->
+
+###### Does this feature depend on any specific services running in the cluster?
+
+<!--
+Think about both cluster-level services (e.g. metrics-server) as well
+as node-level agents (e.g. specific version of CRI). Focus on external or
+optional services that are needed. For example, if this feature depends on
+a cloud provider API, or upon an external software-defined storage or network
+control plane.
+
+For each of these, fill in the following—thinking about running existing user workloads
+and creating new ones, as well as about cluster-level services (e.g. DNS):
+  - [Dependency name]
+    - Usage description:
+      - Impact of its outage on the feature:
+      - Impact of its degraded performance or high-error rates on the feature:
+-->
+
+### Scalability
+
+<!--
+For alpha, this section is encouraged: reviewers should consider these questions
+and attempt to answer them.
+
+For beta, this section is required: reviewers must answer these questions.
+
+For GA, this section is required: approvers should be able to confirm the
+previous answers based on experience in the field.
+-->
+
+###### Will enabling / using this feature result in any new API calls?
+
+<!--
+Describe them, providing:
+  - API call type (e.g. PATCH pods)
+  - estimated throughput
+  - originating component(s) (e.g. Kubelet, Feature-X-controller)
+Focusing mostly on:
+  - components listing and/or watching resources they didn't before
+  - API calls that may be triggered by changes of some Kubernetes resources
+    (e.g. update of object X triggers new updates of object Y)
+  - periodic API calls to reconcile state (e.g. periodic fetching state,
+    heartbeats, leader election, etc.)
+-->
+
+###### Will enabling / using this feature result in introducing new API types?
+
+<!--
+Describe them, providing:
+  - API type
+  - Supported number of objects per cluster
+  - Supported number of objects per namespace (for namespace-scoped objects)
+-->
+
+###### Will enabling / using this feature result in any new calls to the cloud provider?
+
+<!--
+Describe them, providing:
+  - Which API(s):
+  - Estimated increase:
+-->
+
+###### Will enabling / using this feature result in increasing size or count of the existing API objects?
+
+<!--
+Describe them, providing:
+  - API type(s):
+  - Estimated increase in size: (e.g., new annotation of size 32B)
+  - Estimated amount of new objects: (e.g., new Object X for every existing Pod)
+-->
+
+###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?
+
+<!--
+Look at the [existing SLIs/SLOs].
+
+Think about adding additional work or introducing new steps in between
+(e.g. need to do X to start a container), etc. Please describe the details.
+
+[existing SLIs/SLOs]: https://git.k8s.io/community/sig-scalability/slos/slos.md#kubernetes-slisslos
+-->
+
+###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?
+
+<!--
+Things to keep in mind include: additional in-memory state, additional
+non-trivial computations, excessive access to disks (including increased log
+volume), significant amount of data sent and/or received over network, etc.
+This through this both in small and large cases, again with respect to the
+[supported limits].
+
+[supported limits]: https://git.k8s.io/community//sig-scalability/configs-and-limits/thresholds.md
+-->
+
+### Troubleshooting
+
+<!--
+This section must be completed when targeting beta to a release.
+
+For GA, this section is required: approvers should be able to confirm the
+previous answers based on experience in the field.
+
+The Troubleshooting section currently serves the `Playbook` role. We may consider
+splitting it into a dedicated `Playbook` document (potentially with some monitoring
+details). For now, we leave it here.
+-->
+
+###### How does this feature react if the API server and/or etcd is unavailable?
+
+###### What are other known failure modes?
+
+<!--
+For each of them, fill in the following information by copying the below template:
+  - [Failure mode brief description]
+    - Detection: How can it be detected via metrics? Stated another way:
+      how can an operator troubleshoot without logging into a master or worker node?
+    - Mitigations: What can be done to stop the bleeding, especially for already
+      running user workloads?
+    - Diagnostics: What are the useful log messages and their required logging
+      levels that could help debug the issue?
+      Not required until feature graduated to beta.
+    - Testing: Are there any tests for failure mode? If not, describe why.
+-->
+
+###### What steps should be taken if SLOs are not being met to determine the problem?
+
 ## Future Work
 
 Since the proposal only normalizes the object after the patch has been applied,
@@ -313,3 +810,20 @@ Here are the major milestones for this KEP:
   - conversion between schema and openapi definition are in k8s.io/kube-openapi
   - core types have been modified in k8s.io/kubernetes
 - Feature is ready to be released in Beta in kubernetes 1.15
+
+## Drawbacks
+
+<!--
+Why should this KEP _not_ be implemented?
+-->
+* Stutter with discriminator
+* Inconsistent for existing types here
+
+## Alternatives
+* Non-Discrminated
+
+<!--
+What other approaches did you consider, and why did you rule them out? These do
+not need to be as detailed as the proposal, but should include enough
+information to express the idea and why it was not acceptable.
+-->

From 5100aa8438e1f887c620bfa64b1ad32078a46358 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 9 Jun 2022 21:21:36 +0000
Subject: [PATCH 03/22] Update unions KEP for v1.25

---
 .../1027-api-unions/README.md                 | 245 ++++++++++--------
 1 file changed, 134 insertions(+), 111 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 9736f7395fd..f756070b1fb 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -125,12 +125,14 @@ become possible.
 ### Goals
 
 The goal is to enable a union or "oneof" semantics in Kubernetes types, both for
-in-tree types and for CRDs.
+in-tree types and for CRDs. Validation of these unions should be centralized
+rather than being written on a per resource basis in validation functions.
 
 ### Non-Goals
 
-We're not planning to use this KEP to release the feature, but mostly as a way
-to document what we're doing.
+A priority will be minimum disruption to existing types rather than ensuring all
+existing types conform to a single union paradigm (i.e. unions without a
+discriminator should continue to not need a discriminator)
 
 ## Proposal
 
@@ -147,15 +149,27 @@ Detail the things that people will be able to do if this KEP is implemented.
 Include as much detail as possible so that people can understand the "how" of
 the system. The goal here is to make this feel real for users without getting
 bogged down.
+
+TODO: ask antoine if these stories are complete/sufficient
 -->
 
+
 #### Story 1
 
+As a CRD owner, I should be able to author my CRD such that the openapi schema
+indicates which fields are members of this oneOf, so that when users crate
+custom resources, the oneOf fields will be automatically validated, without me
+having to write custom validation logic.
+
 #### Story 2
 
+As a maintainer of existing builtin APIs, I should be able to add new fields to
+a union and have it behave as expected.
+
 ### Notes/Constraints/Caveats (Optional)
 
 <!--
+TODO: ? 
 What are the caveats to the proposal?
 What are some important details that didn't come across above?
 Go in to as much detail as necessary here.
@@ -163,6 +177,12 @@ This might be a good place to talk about core concepts and how they relate.
 -->
 
 ### Risks and Mitigations
+- We need to ensure we do not break existing union types. This can be done by
+  not forcing existing unions to conform to the newly proposed union semantics.
+  Integration testing with older types should give us the confidence to be sure
+  we have done so.
+- There is a lot of risk for errors when there exists skew between clients and
+  server. In the section on normalization, we discuss mitigating these risks.
 
 <!--
 What are the risks of this proposal, and how do we mitigate? Think broadly.
@@ -183,21 +203,16 @@ Consider including folks who also work outside the SIG or subproject.
 We're proposing a new type of tags for go types (in-tree types, and also
 kubebuilder types):
 
-- `// +union` before a structure means that the structure is a union. All the
-  fields must be optional (beside the discriminator) and will be included as
-  members of the union. That structure CAN be embedded in another structure.
-- `// +unionDeprecated` before a field means that it is part of the
-  union. Multiple fields can have this prefix. These fields MUST BE optional,
-  omitempty and pointers. The field is named deprecated because we don't want
-  people to embed their unions directly in structures, and only exist because of
-  some existing core types (e.g. `Value` and `ValueFrom` in
-  [EnvVar](https://github.com/kubernetes/kubernetes/blob/3ebb8ddd8a21b/staging/src/k8s.io/api/core/v1/types.go#L1817-L1836)).
 - `// +unionDiscriminator` before a field means that this field is the
-  discriminator for the union. Only one field per structure can have this
-  prefix. This field HAS TO be a string, and CAN be optional.
-
-Multiple unions can exist per structure, but unions can't span across multiple
-go structures (all the fields that are part of a union has to be together in the
+  discriminator for the union. This field MUST be a string. Can be optional or
+  required. If required, the union must have exactly one member set, if
+  optional, the union may have at most one member set.
+- `// +unionMember=<discriminatorName>` before a field means that this
+  field is a member of a union. The `<discriminatorName>` is optional if there
+  is only union in a struct and required if there are multiple unions per
+  struct. 
+
+Note unions can't span across multiple go structures (all the fields that are part of a union has to be together in the
 same structure), examples of what is allowed:
 
 ```
@@ -209,27 +224,28 @@ type TopLevelUnion struct {
 }
 
 // This will generate one union, with two fields and a discriminator.
-// +union
 type Union struct {
 	// +unionDiscriminator
 	// +optional
 	UnionType string `json:"unionType"`
 
 	// +optional
+        // +unionMember
 	FieldA int `json:"fieldA"`
-    // +optional
+        // +optional
 	FieldB int `json:"fieldB"`
 }
 
-// This also generates one union, with two fields and on discriminator.
+// This generates two unions, each with two fields and a discriminator. The 
 type Union2 struct {
 	// +unionDiscriminator
+        // +required
 	Type string `json:"type"`
-	// +unionDeprecated
-    // +optional
+	// +unionMember=type
+        // +optional
 	Alpha int `json:"alpha"`
-	// +unionDeprecated
-    // +optional
+	// +unionMember=type
+        // +optional
 	Beta int `json:"beta"`
 }
 
@@ -238,14 +254,19 @@ type Union2 struct {
 type InlinedUnion struct {
 	Name string `json:"name"`
 
-	// +unionDeprecated
+        // +unionDiscriminator
+        // +optional
+        FieldType string `json:"fieldType"`
+	// +unionMember=fieldType
 	// +optional
 	Field1 *int `json:"field1,omitempty"`
-	// +unionDeprecated
+	// +unionMember=fieldType
 	// +optional
 	Field2 *int `json:"field2,omitempty"`
 
-	Union  `json:",inline"`
+        // Union does not label its members, so it
+        cannot be inlined
+	union Union  `json:"union"`
 	Union2 `json:",inline"`
 }
 ```
@@ -271,6 +292,7 @@ Conversion between OpenAPI v2 and OpenAPI v3 will preserve these fields.
 
 ### Discriminator
 
+// TODO: ask Antoine
 For backward compatibility reasons, discriminators should be added to existing
 union structures as an optional string. This has a nice property that it's going
 to allow conflict detection when the selected union field is changed.
@@ -287,63 +309,51 @@ When the value of the discriminator is explicitly changed by the client, it
 will be interpreted as an intention to clear all the other fields. See section
 below.
 
-### Normalizing on updates
-
-A "normalization process" will run automatically for all creations and
-modifications (either with update or patch). It will happen automatically in order
-to clear fields and update discriminators. This process will run for both
-core-types and CRDs. It will take place before validation. The sent object
-doesn't have to be valid, but fields will be cleared in order to make it valid.
-This process will also happen before fields tracking (server-side apply), so
-changes in discriminator, even if implicit, will be owned by the client making
-the update (and may result in conflicts).
-
-This process works as follows:
-- If there is a discriminator, and its value has changed, clear all fields but
-  the one specified by the discriminator,
-- If there is no discriminator, or if its value hasn't changed,
-  - if there is exactly one field, set the discriminator when there is one
-    to that value. Otherwise,
-  - compare the fields set before and after. If there is exactly one field
-    added, set the discriminator (if present) to that value, and remove all
-    other fields. if more than one field has been added, leave the process so
-    that validation will fail.
-
 #### "At most one" versus "exactly one"
 
-The goal of this proposal is not to change the validation, but to help clients
-to clear other fields in the union. Validation should be implemented for in-tree
-types as it is today, or through "oneOf" properties in CRDs.
+// TODO: check this
+We propose supporting both "at most one" and "exactly one" semantics. 
+
+To distinguish between the two, API authors should tag the discriminator as
+"optional" or "required" based on the desired behavior. We the discriminator is
+set to an empty string (or missing from the object), this will indicate to the
+server to clear all fields of the union (or fail the request if the
+discriminator is required).
 
-In other word, this is proposing to implement "at most one", and the exactly one
-should be provided through another layer of validation (separated).
+### Normalization and Validation
 
-#### Clearing all the fields
+Normalization refers to the process by which the API server attempts to understand
+and correct clients which may provide the server with conflicting or incomplete
+information about a union.
 
-Since the system is trying to do the right thing, it can be hard to "clear
-everything". In that case, each API could decide to have their own "Nothing"
-value in the discriminator, which will automatically trigger a clearing of all
-fields beside "Nothing".
+Issues primarily arise here because of version skew between a client and server,
+such as when a client is unaware of new fields added to a union and thus doesn't
+know how to clear these new fields when trying to set a different.
 
-### Backward compatibilities properties
+For unions with a discriminator (all newly created unions and some existing
+unions), normalization is simple: the server should always respect the
+discriminator.
 
-This normalization process has a few nice properties, especially for dumb
-clients, when it comes to backward compatibility:
-- A dumb client that doesn't know which fields belong to the union can just set
-  a new field and get all the others cleared automatically
-- A dumb client that doesn't know about the discriminator is going to change a
-  field, leave the discriminator as it is, and should still expect the fields to
-  be cleared accordingly
-- A dumb client that knows about the discriminator can change the discriminator
-  without knowing which fields to clear, they will get cleared automatically
+This means two things:
+1. when the server receives a request with a discriminator set to a
+given field, it should clear out any other fields that are set.
+2. when the server receives a request with a discriminator set to a given field,
+   but that given field is empty, it should maintain the value of the field that
+   currently exists.
 
+For union types without a discriminator (only existing unions), normalization is a little more
+complicated and incomplete. If multiple union fields are set:
+1. If greater than two of the set union fields were not previously set, error
+2. If one of the set union fields was previously set, and one is newly set,
+   respect the newly set field and clear the other.
 
-### Validation
+For situations where a typed client is unaware of a new field (that is currently
+set) and drops the set field such that no fields are now set, the server will
+clear all the fields of the union. This is unavoidable for unions without a
+discriminator and a major reason why we recommend all new unions have a
+discriminator.
 
-Objects have to be validated AFTER the normalization process, which is going to
-leave multiple fields of the union if it can't normalize. As discussed in
-drawbacks below, it can also be useful to validate apply requests before
-applying them.
+Objects must be validated AFTER the normalization process.
 
 ### Test Plan
 
@@ -382,21 +392,46 @@ https://testgrid.k8s.io/sig-testing-canaries#ci-kubernetes-coverage-unit
 
 This can inform certain test coverage improvements that we want to do before
 extending the production code to implement this enhancement.
--->
 
 - `<package>`: `<date>` - `<test coverage>`
 
+-->
+Core functionality will be extensively unit tested in the SMD typed package
+(union_test.go). 
+
+Parts of the kubernetes endpoints handlers package that are modified to call
+into the SMD code will also be unit tested as appropriate.
+
+
+
+
+
 ##### Integration tests
 
+We will have extensive integration testing of the union code in the
+`test/integration/apiserver` package.
+
+We will be testing along the dimensions of:
+* Which fields of the union get modified (none, existing fields, newly updated
+  fields)
+* Type of union (discriminated vs non-discriminated)
+* Whether the client is aware of all the fields
+* Whether the server is aware of all fields
+* Whether the union is optional or required
+
+A fully documented test matrix exists in a [google
+spreadsheet](https://docs.google.com/spreadsheets/d/1dIOOqgrgvMI9b2412eVuRSydEaOxhYOoqk6bUjZOY9c/edit?resourcekey=0-wlOfJTC_EX-qpU680STHMA#gid=3601413) along with a
+[guide
+doc](https://docs.google.com/document/d/1Wruosjo0ELLl1yxauzpsUjgH2fK9KdgXDmOdJ5sG7Kg/edit?resourcekey=0-8Pwzx6EvsFR7VQoXzCTY4Q) on how to read and understand the test matrix.
 <!--
 This question should be filled when targeting a release.
 For Alpha, describe what tests will be added to ensure proper quality of the enhancement.
 
 For Beta and GA, add links to added tests together with links to k8s-triage for those tests:
 https://storage.googleapis.com/k8s-triage/index.html
--->
 
 - <test>: <link to test coverage>
+-->
 
 ##### e2e tests
 
@@ -408,21 +443,21 @@ For Beta and GA, add links to added tests together with links to k8s-triage for
 https://storage.googleapis.com/k8s-triage/index.html
 
 We expect no non-infra related flakes in the last month as a GA graduation criteria.
--->
 
 - <test>: <link to test coverage>
+-->
+ We are considering adding kubectl e2e tests to mimic kubectl users performing
+ various operations on objects with union fields.
 
 ### Graduation Criteria
 
-Since this project is a sub-project of Server-side apply, it will be introduced
-directly as Beta, and will graduate to GA in a later release, according to the
-criteria below.
+#### Alpha Graduation
 
-#### Beta -> GA Graduation
-
-- CRD support has been proven successful
-- Core-types all implement the semantics properly
-- Stable and bug-free for two releases
+- CRDs can be created with union fields and are properly validated when
+  created/updated.
+- Existing unions that have discriminators, are properly tagged and validated via
+  union validation
+- Existing unions that don't have discriminators do not break when upgraded.
 
 ### Upgrade / Downgrade Strategy
 
@@ -785,42 +820,30 @@ For each of them, fill in the following information by copying the below templat
 
 ###### What steps should be taken if SLOs are not being met to determine the problem?
 
-## Future Work
-
-Since the proposal only normalizes the object after the patch has been applied,
-it is hard to fail if the patch is invalid. There are scenarios where the patch
-is invalid but it results in an unpredictable object. For example, if a patch
-sends both a discriminator and a field that is not the discriminated field, it
-will either clear the value sent if the discriminator changes, or it will change
-the value of the sent discriminator.
-
-Validating patches is not a problem that we want to tackle now, but we can
-validate "Apply" objects to make sure that they do not define such broken
-semantics.
-
 ## Implementation History
-
-Here are the major milestones for this KEP:
-- Initial discussion happened a year before the creation of this kep:
-  https://docs.google.com/document/d/1lrV-P25ZTWukixE9ZWyvchfFR0NE2eCHlObiCUgNQGQ/edit#heading=h.w5eqnf1f76x5
-- Points made in the initial document have been improved and put into this kep,
-  which has approved by sig-api-machinery tech-leads
-- KEP has been implemented:
-  - logic mostly lives in sigs.k8s.io/structured-merge-diff
-  - conversion between schema and openapi definition are in k8s.io/kube-openapi
-  - core types have been modified in k8s.io/kubernetes
-- Feature is ready to be released in Beta in kubernetes 1.15
+- Unions implemented, but disabled in SMD.
 
 ## Drawbacks
 
 <!--
 Why should this KEP _not_ be implemented?
 -->
-* Stutter with discriminator
-* Inconsistent for existing types here
+An issue that one might have with requiring a discriminator is that it might
+seem redundant to have to set a field _and_ set another field indicating to the
+server to use the set field. The reasons for doing so are discussed above in the
+normalization section.
+
+One other drawback is that our approach does not standardize all existing unions
+into a single format. We don't see a way to do so without drastically changing
+existing APIs and breaking backwards compatibility
 
 ## Alternatives
 * Non-Discrminated
+The primary alternative discussed is to not have a discriminator for new union
+types. As discussed in the normalization section, requiring a discriminator
+allows the server to better understand the intentions of clients that do not
+have knowledge of all the fields in a union if newer verisons of the server add
+new fields to the union.
 
 <!--
 What other approaches did you consider, and why did you rule them out? These do

From d882c04064c19c63bcb48c1dd85e45704210cec5 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Fri, 10 Jun 2022 08:34:47 +0000
Subject: [PATCH 04/22] address apelisse feedback

---
 .../1027-api-unions/README.md                 | 92 ++++++++-----------
 1 file changed, 38 insertions(+), 54 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index f756070b1fb..224003cdd04 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -124,9 +124,14 @@ become possible.
 
 ### Goals
 
-The goal is to enable a union or "oneof" semantics in Kubernetes types, both for
-in-tree types and for CRDs. Validation of these unions should be centralized
-rather than being written on a per resource basis in validation functions.
+Provide a simple mechanism for API authors to label fields of a resource as
+members of a oneOf, in order to receive standardized:
+
+* Validation - ensuring only one member field is set (or at most one if
+  desired).
+* Normalization - ensuring the API server can modify the fields of the oneOf to
+  best match the intent of the client, despite the client potentially having
+  incomplete information
 
 ### Non-Goals
 
@@ -150,26 +155,29 @@ Include as much detail as possible so that people can understand the "how" of
 the system. The goal here is to make this feel real for users without getting
 bogged down.
 
-TODO: ask antoine if these stories are complete/sufficient
 -->
 
 
 #### Story 1
 
-As a CRD owner, I should be able to author my CRD such that the openapi schema
-indicates which fields are members of this oneOf, so that when users crate
-custom resources, the oneOf fields will be automatically validated, without me
-having to write custom validation logic.
+As a CRD owner, I can use simple semantics (such as go tags), to express the
+desired validation of a oneOf (at most one or exactly one field may be set), and
+the API server will perform this validation automatically.
 
 #### Story 2
 
+As a client, I can read, modify, and update the union fields of an object, even
+if I am not aware of all of the possible fields, and the server will properly
+interpret my intent.
+
+#### Story 3
+
 As a maintainer of existing builtin APIs, I should be able to add new fields to
 a union and have it behave as expected.
 
 ### Notes/Constraints/Caveats (Optional)
 
 <!--
-TODO: ? 
 What are the caveats to the proposal?
 What are some important details that didn't come across above?
 Go in to as much detail as necessary here.
@@ -203,14 +211,19 @@ Consider including folks who also work outside the SIG or subproject.
 We're proposing a new type of tags for go types (in-tree types, and also
 kubebuilder types):
 
+
 - `// +unionDiscriminator` before a field means that this field is the
-  discriminator for the union. This field MUST be a string. Can be optional or
-  required. If required, the union must have exactly one member set, if
-  optional, the union may have at most one member set.
+  discriminator for the union. This field MUST be a string. This field MUST be
+  required.
+- `// +unionAllowEmpty` before a discriminator field means that by setting the
+  discriminator to an empty string, none of the union fields are persisted
+  (meaning at most one member of the union must be set). If not present on the
+  discriminator, exactly one of the union members must be set in order to be
+  valid.
 - `// +unionMember=<discriminatorName>` before a field means that this
   field is a member of a union. The `<discriminatorName>` is optional if there
   is only union in a struct and required if there are multiple unions per
-  struct. 
+  struct.
 
 Note unions can't span across multiple go structures (all the fields that are part of a union has to be together in the
 same structure), examples of what is allowed:
@@ -226,12 +239,14 @@ type TopLevelUnion struct {
 // This will generate one union, with two fields and a discriminator.
 type Union struct {
 	// +unionDiscriminator
-	// +optional
+        // +unionAllowEmpty
+        // +required
 	UnionType string `json:"unionType"`
 
-	// +optional
         // +unionMember
+	// +optional
 	FieldA int `json:"fieldA"`
+        // +unionMember
         // +optional
 	FieldB int `json:"fieldB"`
 }
@@ -255,7 +270,8 @@ type InlinedUnion struct {
 	Name string `json:"name"`
 
         // +unionDiscriminator
-        // +optional
+        // +unionAllowEmpty
+        // +required
         FieldType string `json:"fieldType"`
 	// +unionMember=fieldType
 	// +optional
@@ -290,45 +306,15 @@ each list item is made of:
 
 Conversion between OpenAPI v2 and OpenAPI v3 will preserve these fields.
 
-### Discriminator
-
-// TODO: ask Antoine
-For backward compatibility reasons, discriminators should be added to existing
-union structures as an optional string. This has a nice property that it's going
-to allow conflict detection when the selected union field is changed.
-
-We also do strongly recommend new APIs to be written with a discriminator, and
-tools (kube-builder) should probably enforce the presence of a discriminator in
-CRDs.
-
-The value of the discriminator is going to be set automatically by the apiserver
-when a new field is changed in the union. It will be set to the value of the
-`fields-to-discriminateBy` for that specific field.
-
-When the value of the discriminator is explicitly changed by the client, it
-will be interpreted as an intention to clear all the other fields. See section
-below.
-
-#### "At most one" versus "exactly one"
-
-// TODO: check this
-We propose supporting both "at most one" and "exactly one" semantics. 
-
-To distinguish between the two, API authors should tag the discriminator as
-"optional" or "required" based on the desired behavior. We the discriminator is
-set to an empty string (or missing from the object), this will indicate to the
-server to clear all fields of the union (or fail the request if the
-discriminator is required).
-
 ### Normalization and Validation
 
 Normalization refers to the process by which the API server attempts to understand
 and correct clients which may provide the server with conflicting or incomplete
 information about a union.
 
-Issues primarily arise here because of version skew between a client and server,
+Issues primarily arise here because of version skew between a client and a server,
 such as when a client is unaware of new fields added to a union and thus doesn't
-know how to clear these new fields when trying to set a different.
+know how to clear these new fields when trying to set a different field.
 
 For unions with a discriminator (all newly created unions and some existing
 unions), normalization is simple: the server should always respect the
@@ -341,18 +327,16 @@ given field, it should clear out any other fields that are set.
    but that given field is empty, it should maintain the value of the field that
    currently exists.
 
-For union types without a discriminator (only existing unions), normalization is a little more
-complicated and incomplete. If multiple union fields are set:
-1. If greater than two of the set union fields were not previously set, error
-2. If one of the set union fields was previously set, and one is newly set,
-   respect the newly set field and clear the other.
-
 For situations where a typed client is unaware of a new field (that is currently
 set) and drops the set field such that no fields are now set, the server will
 clear all the fields of the union. This is unavoidable for unions without a
 discriminator and a major reason why we recommend all new unions have a
 discriminator.
 
+Union types without a discriminator must continue to perform normalization and
+validation manually (i.e. per resource in the validation functions), as is
+currently done for existing union types.
+
 Objects must be validated AFTER the normalization process.
 
 ### Test Plan

From e31cd196a4a16ff1e04a51f3e76b97dccba74d5d Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Mon, 13 Jun 2022 10:01:03 +0000
Subject: [PATCH 05/22] Add test case summary and open questions

---
 .../1027-api-unions/README.md                 | 79 +++++++++++++++----
 1 file changed, 63 insertions(+), 16 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 224003cdd04..55c64621b3e 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -12,24 +12,22 @@
   - [User Stories (Optional)](#user-stories-optional)
     - [Story 1](#story-1)
     - [Story 2](#story-2)
+    - [Story 3](#story-3)
   - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional)
   - [Risks and Mitigations](#risks-and-mitigations)
 - [Design Details](#design-details)
+  - [Discriminator Field](#discriminator-field)
   - [Go tags](#go-tags)
   - [OpenAPI](#openapi)
-  - [Discriminator](#discriminator)
-  - [Normalizing on updates](#normalizing-on-updates)
-    - [&quot;At most one&quot; versus &quot;exactly one&quot;](#at-most-one-versus-exactly-one)
-    - [Clearing all the fields](#clearing-all-the-fields)
-  - [Backward compatibilities properties](#backward-compatibilities-properties)
-  - [Validation](#validation)
+  - [Normalization and Validation](#normalization-and-validation)
+  - [Open Questions](#open-questions)
   - [Test Plan](#test-plan)
       - [Prerequisite testing updates](#prerequisite-testing-updates)
       - [Unit tests](#unit-tests)
       - [Integration tests](#integration-tests)
       - [e2e tests](#e2e-tests)
   - [Graduation Criteria](#graduation-criteria)
-    - [Beta -&gt; GA Graduation](#beta---ga-graduation)
+    - [Alpha Graduation](#alpha-graduation)
   - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
   - [Version Skew Strategy](#version-skew-strategy)
 - [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
@@ -39,7 +37,6 @@
   - [Dependencies](#dependencies)
   - [Scalability](#scalability)
   - [Troubleshooting](#troubleshooting)
-- [Future Work](#future-work)
 - [Implementation History](#implementation-history)
 - [Drawbacks](#drawbacks)
 - [Alternatives](#alternatives)
@@ -144,8 +141,6 @@ discriminator should continue to not need a discriminator)
 In order to support unions in a backward compatible way in kubernetes, we're
 proposing the following changes.
 
-Note that this proposes unions to be "at most one of". Whether exactly one is
-supported or not should be implemented by the validation logic.
 
 ### User Stories (Optional)
 
@@ -206,6 +201,41 @@ Consider including folks who also work outside the SIG or subproject.
 
 ## Design Details
 
+### Discriminator Field
+
+We propose that all new unions maintain a "discriminator". This is a field that
+points to which of the other "union member" fields is to be respected as the
+truly desired union field in the case that there are any conflicts.
+
+In order to demonstrate the need for the discriminator, we developed an
+extensive [test matrix](https://docs.google.com/spreadsheets/d/1dIOOqgrgvMI9b2412eVuRSydEaOxhYOoqk6bUjZOY9c/edit?resourcekey=0-wlOfJTC_EX-qpU680STHMA#gid=3601413) that looks at various configurations of the performing
+REST operations on a union where the client or server is unaware of a newly
+added field to the union (due to version skew).
+
+We present a [guide doc](https://docs.google.com/document/d/1Wruosjo0ELLl1yxauzpsUjgH2fK9KdgXDmOdJ5sG7Kg/edit?resourcekey=0-8Pwzx6EvsFR7VQoXzCTY4Q) on how to interpret the test matrix, but the major
+conclusions are as follows (along with the test case number from the test matrix):
+
+* (Case #22 and #27) If an unstructured client is unaware of field on the union, but wants to clear
+  the union entirely (assuming the union is optional), it will have no way of doing
+  so without a discriminator. With a discriminator, the client can express its
+  intention by setting the discriminator to the empty value and the server can
+  respects it intentions and clear any fields the client is unaware of.
+* (Case #12 and #16) If a structured client is unaware of a field in the union that is set and it
+  just wants to echo back the union it received in a get request (such as when
+  updating other parts of the object), a client without a discriminator will
+  silently drop the currently set field, while a client with the discriminator
+  will not change the discriminator value, indicating to the client that no
+  changes are desired in the union.
+* (Case #34 and #39) If a client sets a union field that the server is not aware of, the server
+  will silently drop it and attempt to clear the object of the union field. With
+  a discriminator, the server will see the unrecognized discriminator value and
+  can fail loudly.
+* (Case #23 and #28) When a client goes to set a field it knows of, but a separate field it doesn't
+  know about is currently set, the server can simply know to always respect the
+  discriminator. Without a discriminator, the server will have to do convoluted
+  logic to detect that the previously set field has not been modified and that
+  only one of the other union fields has been.
+
 ### Go tags
 
 We're proposing a new type of tags for go types (in-tree types, and also
@@ -339,14 +369,24 @@ currently done for existing union types.
 
 Objects must be validated AFTER the normalization process.
 
-### Test Plan
+### Open Questions
 
-There are mostly 3 aspects to this plan:
-- [x] Core functionality, isolated from all other components: https://github.com/kubernetes-sigs/structured-merge-diff/blob/master/typed/union_test.go
-- [x] Functionality as part of server-side apply: How human and robot interactions work: https://github.com/kubernetes-sigs/structured-merge-diff/blob/master/typed/union_test.go
-- [x] Integration in kubernetes: https://github.com/kubernetes/kubernetes/pull/77370/files#diff-4ac5831d494b1b52c7c7be81e552a458
+A few open questions exist with the design that need to be resolved with SIG API
+Machinery before moving forward.
 
-[ ] I/we understand the owners of the involved components may require updates to
+1. When a server receives an update request with empty data in the member fields
+   but a validly set discriminator pointing to the object's union member field
+   that was previously set, should the server error and inform the client that
+   the field pointed to by the discriminator is currently empty, or should the
+   server retain the previous value it knows was set to the field being pointed
+   to by the discriminator?
+2. What should the value of the discriminator be when no field in the union is
+   to be set. A couple options inclued an empty string, an field common to all
+   union (e.g. "NONE"), or a field specified on a per union basis.
+
+### Test Plan
+
+[x] I/we understand the owners of the involved components may require updates to
 existing tests to make this code solid enough prior to committing the changes necessary
 to implement this enhancement.
 
@@ -457,6 +497,9 @@ enhancement:
   cluster required to make on upgrade, in order to make use of the enhancement?
 -->
 
+At first, Only new union types will follow the prescribed guinance, so no upgrades/downgrades are possible for types
+that don't exist yet.
+
 ### Version Skew Strategy
 
 <!--
@@ -472,6 +515,10 @@ enhancement:
   CRI or CNI may require updating that component before the kubelet.
 -->
 
+See test matrix and commentary about discriminators. It clearly documents how
+the server will use the discriminator to understand the client's intention even
+if the client is not aware of all union fields because of version skew.
+
 ## Production Readiness Review Questionnaire
 
 <!--

From 85c40c091078fc5a0dbd23de7f9e5661adff036b Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Mon, 13 Jun 2022 21:41:02 +0000
Subject: [PATCH 06/22] update kep

---
 .../sig-api-machinery/1027.yaml               |  3 +
 .../1027-api-unions/README.md                 | 81 +++++++++++++------
 .../1027-api-unions/kep.yaml                  | 18 +++--
 3 files changed, 70 insertions(+), 32 deletions(-)
 create mode 100644 keps/prod-readiness/sig-api-machinery/1027.yaml

diff --git a/keps/prod-readiness/sig-api-machinery/1027.yaml b/keps/prod-readiness/sig-api-machinery/1027.yaml
new file mode 100644
index 00000000000..865ab5fc50d
--- /dev/null
+++ b/keps/prod-readiness/sig-api-machinery/1027.yaml
@@ -0,0 +1,3 @@
+kep-number: 1027
+alpha:
+        approver: "@deads2k"
diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 55c64621b3e..ad0c013a5e7 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -12,7 +12,6 @@
   - [User Stories (Optional)](#user-stories-optional)
     - [Story 1](#story-1)
     - [Story 2](#story-2)
-    - [Story 3](#story-3)
   - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional)
   - [Risks and Mitigations](#risks-and-mitigations)
 - [Design Details](#design-details)
@@ -132,16 +131,13 @@ members of a oneOf, in order to receive standardized:
 
 ### Non-Goals
 
-A priority will be minimum disruption to existing types rather than ensuring all
-existing types conform to a single union paradigm (i.e. unions without a
-discriminator should continue to not need a discriminator)
+The focus of this KEP is on providing unified validation and normalization of
+new union types. Migrating existing unions away from their bespoke validation
+logic (e.g validation functions), is an explicit non-goal and will be pursued in
+a separate KEP or later release.
 
 ## Proposal
 
-In order to support unions in a backward compatible way in kubernetes, we're
-proposing the following changes.
-
-
 ### User Stories (Optional)
 
 <!--
@@ -165,11 +161,6 @@ As a client, I can read, modify, and update the union fields of an object, even
 if I am not aware of all of the possible fields, and the server will properly
 interpret my intent.
 
-#### Story 3
-
-As a maintainer of existing builtin APIs, I should be able to add new fields to
-a union and have it behave as expected.
-
 ### Notes/Constraints/Caveats (Optional)
 
 <!--
@@ -383,6 +374,10 @@ Machinery before moving forward.
 2. What should the value of the discriminator be when no field in the union is
    to be set. A couple options inclued an empty string, an field common to all
    union (e.g. "NONE"), or a field specified on a per union basis.
+3. How should a server behave when it receives a union with multiple fields set
+   and the discriminator pointing to one of them? Reject the request, accept the
+   request but don't clear the fields, or automatically clear the fields not
+   specified by the discriminator.
 
 ### Test Plan
 
@@ -561,18 +556,19 @@ well as the [existing list] of feature gates.
 [existing list]: https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/
 -->
 
-- [ ] Feature gate (also fill in values in `kep.yaml`)
-  - Feature gate name:
-  - Components depending on the feature gate:
-- [ ] Other
-  - Describe the mechanism:
-  - Will enabling / disabling the feature require downtime of the control
-    plane?
-  - Will enabling / disabling the feature require downtime or reprovisioning
-    of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled).
+- [x] Feature gate (also fill in values in `kep.yaml`)
+  - Feature gate name: APIUnions
+  - Components depending on the feature gate: kube-apiserver
+  
+Request handlers in the api server will call into union validation and
+normalization function from the structured-merge-diff repo when feature is
+enabled.
+
 
 ###### Does enabling the feature change any default behavior?
 
+No
+
 <!--
 Any change of default behavior may be surprising to users or break existing
 automations, so be extremely careful here.
@@ -591,8 +587,12 @@ feature.
 NOTE: Also set `disable-supported` to `true` or `false` in `kep.yaml`.
 -->
 
+Yes, requests will simply skip union validation and normalization.
+
 ###### What happens if we reenable the feature if it was previously rolled back?
 
+Requests will resume perform union validation and normalization
+
 ###### Are there any tests for feature enablement/disablement?
 
 <!--
@@ -616,6 +616,8 @@ This section must be completed when targeting beta to a release.
 
 ###### How can a rollout or rollback fail? Can it impact already running workloads?
 
+N/A
+
 <!--
 Try to be as paranoid as possible - e.g., what if some components will restart
 mid-rollout?
@@ -628,6 +630,7 @@ will rollout across nodes.
 
 ###### What specific metrics should inform a rollback?
 
+N/A
 <!--
 What signals should users be paying attention to when the feature is young
 that might indicate a serious problem?
@@ -635,6 +638,7 @@ that might indicate a serious problem?
 
 ###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?
 
+N/A
 <!--
 Describe manual testing that was done and the outcomes.
 Longer term, we may want to require automated upgrade/rollback tests, but we
@@ -643,6 +647,7 @@ are missing a bunch of machinery and tooling and can't do that now.
 
 ###### Is the rollout accompanied by any deprecations and/or removals of features, APIs, fields of API types, flags, etc.?
 
+N/A
 <!--
 Even if applying deprecation policies, they may still surprise some users.
 -->
@@ -658,6 +663,7 @@ previous answers based on experience in the field.
 
 ###### How can an operator determine if the feature is in use by workloads?
 
+Simply creating and updating objects with union fields.
 <!--
 Ideally, this should be a metric. Operations against the Kubernetes API (e.g.,
 checking if there are objects with field X set) may be a last resort. Avoid
@@ -666,6 +672,10 @@ logs or events for this purpose.
 
 ###### How can someone using this feature know that it is working for their instance?
 
+1. Create a new CRD with a union field
+2. Apply the CRD
+3. Create a CR with an invalid union (multiple fields set, no discriminator
+   set), see if the CR is rejected via union validation
 <!--
 For instance, if this is a pod-related feature, it should be possible to determine if the feature is functioning properly
 for each individual pod.
@@ -673,7 +683,6 @@ Pick one more of these and delete the rest.
 Please describe all items visible to end users below with sufficient detail so that they can verify correct enablement
 and operation of this feature.
 Recall that end users cannot usually observe component logs or access metrics.
--->
 
 - [ ] Events
   - Event Reason: 
@@ -682,9 +691,12 @@ Recall that end users cannot usually observe component logs or access metrics.
   - Other field: 
 - [ ] Other (treat as last resort)
   - Details:
+-->
 
 ###### What are the reasonable SLOs (Service Level Objectives) for the enhancement?
 
+N/A
+
 <!--
 This is your opportunity to define what "normal" quality of service looks like
 for a feature.
@@ -702,9 +714,9 @@ question.
 
 ###### What are the SLIs (Service Level Indicators) an operator can use to determine the health of the service?
 
+N/A
 <!--
 Pick one more of these and delete the rest.
--->
 
 - [ ] Metrics
   - Metric name:
@@ -712,9 +724,11 @@ Pick one more of these and delete the rest.
   - Components exposing the metric:
 - [ ] Other (treat as last resort)
   - Details:
+-->
 
 ###### Are there any missing metrics that would be useful to have to improve observability of this feature?
 
+N/A
 <!--
 Describe the metrics themselves and the reasons why they weren't added (e.g., cost,
 implementation difficulties, etc.).
@@ -722,12 +736,14 @@ implementation difficulties, etc.).
 
 ### Dependencies
 
+
 <!--
 This section must be completed when targeting beta to a release.
 -->
 
 ###### Does this feature depend on any specific services running in the cluster?
 
+N/A
 <!--
 Think about both cluster-level services (e.g. metrics-server) as well
 as node-level agents (e.g. specific version of CRI). Focus on external or
@@ -745,6 +761,7 @@ and creating new ones, as well as about cluster-level services (e.g. DNS):
 
 ### Scalability
 
+N/A
 <!--
 For alpha, this section is encouraged: reviewers should consider these questions
 and attempt to answer them.
@@ -757,6 +774,8 @@ previous answers based on experience in the field.
 
 ###### Will enabling / using this feature result in any new API calls?
 
+No
+
 <!--
 Describe them, providing:
   - API call type (e.g. PATCH pods)
@@ -772,6 +791,8 @@ Focusing mostly on:
 
 ###### Will enabling / using this feature result in introducing new API types?
 
+No
+
 <!--
 Describe them, providing:
   - API type
@@ -781,6 +802,7 @@ Describe them, providing:
 
 ###### Will enabling / using this feature result in any new calls to the cloud provider?
 
+No
 <!--
 Describe them, providing:
   - Which API(s):
@@ -789,6 +811,7 @@ Describe them, providing:
 
 ###### Will enabling / using this feature result in increasing size or count of the existing API objects?
 
+No
 <!--
 Describe them, providing:
   - API type(s):
@@ -798,6 +821,7 @@ Describe them, providing:
 
 ###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?
 
+No
 <!--
 Look at the [existing SLIs/SLOs].
 
@@ -809,6 +833,7 @@ Think about adding additional work or introducing new steps in between
 
 ###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?
 
+No
 <!--
 Things to keep in mind include: additional in-memory state, additional
 non-trivial computations, excessive access to disks (including increased log
@@ -834,6 +859,8 @@ details). For now, we leave it here.
 
 ###### How does this feature react if the API server and/or etcd is unavailable?
 
+N/A objects are not reachable
+
 ###### What are other known failure modes?
 
 <!--
@@ -869,11 +896,13 @@ into a single format. We don't see a way to do so without drastically changing
 existing APIs and breaking backwards compatibility
 
 ## Alternatives
-* Non-Discrminated
+
+###### Non-Discrminated
+
 The primary alternative discussed is to not have a discriminator for new union
 types. As discussed in the normalization section, requiring a discriminator
 allows the server to better understand the intentions of clients that do not
-have knowledge of all the fields in a union if newer verisons of the server add
+have knowledge of all the fields in a union if newer versions of the server add
 new fields to the union.
 
 <!--
diff --git a/keps/sig-api-machinery/1027-api-unions/kep.yaml b/keps/sig-api-machinery/1027-api-unions/kep.yaml
index 86597fa8f4e..af083e055cc 100644
--- a/keps/sig-api-machinery/1027-api-unions/kep.yaml
+++ b/keps/sig-api-machinery/1027-api-unions/kep.yaml
@@ -5,6 +5,9 @@ authors:
   - "@kevindelgado"
 owning-sig: sig-api-machinery
 participating-sigs:
+status: implementable
+creation-date: 2019-03-25
+last-updated: 2022-06-13
 reviewers:
   - "@sttts"
   - "@lavalamp"
@@ -12,15 +15,18 @@ reviewers:
   - "@DirectXMan12"
 approvers:
   - "@lavalamp"
-editor: TBD
-creation-date: 2019-03-25
-last-updated: 2022-06-09
-status: implementable
+  - "@deads2k"
+
 see-also:
   - "/keps/sig-api-machinery/0006-apply.md"
 replaces:
   - "https://docs.google.com/document/d/1lrV-P25ZTWukixE9ZWyvchfFR0NE2eCHlObiCUgNQGQ"
-superseded-by:
 
-latest-milestone: "1.25"
 stage: "alpha"
+latest-milestone: "1.25"
+
+feature-gates:
+  - name: APIUnions
+    components:
+      - kube-apiserver
+disable-supported: true

From dee6e696ef50a4bff9aa268ba1a99815c68df5f1 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Tue, 14 Jun 2022 03:08:29 +0000
Subject: [PATCH 07/22] Address lavalamp feedback

---
 .../1027-api-unions/README.md                 | 44 +++++++++++--------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index ad0c013a5e7..0d371246fc8 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -121,7 +121,10 @@ become possible.
 ### Goals
 
 Provide a simple mechanism for API authors to label fields of a resource as
-members of a oneOf, in order to receive standardized:
+members of a oneOf, in order to receive standardized validation and
+normalization, rather than having to author it themeselves per
+resource as currently done as a workaround in various validation
+functions (e.g. `pkg/apis/<group>/validation/validation.go`).
 
 * Validation - ensuring only one member field is set (or at most one if
   desired).
@@ -151,7 +154,7 @@ bogged down.
 
 #### Story 1
 
-As a CRD owner, I can use simple semantics (such as go tags), to express the
+As a CRD owner, I can use simple semantics (such as openapi tags/go markers), to express the
 desired validation of a oneOf (at most one or exactly one field may be set), and
 the API server will perform this validation automatically.
 
@@ -206,16 +209,17 @@ added field to the union (due to version skew).
 We present a [guide doc](https://docs.google.com/document/d/1Wruosjo0ELLl1yxauzpsUjgH2fK9KdgXDmOdJ5sG7Kg/edit?resourcekey=0-8Pwzx6EvsFR7VQoXzCTY4Q) on how to interpret the test matrix, but the major
 conclusions are as follows (along with the test case number from the test matrix):
 
-* (Case #22 and #27) If an unstructured client is unaware of field on the union, but wants to clear
+* (Case #22 and #27) If an unstructured client (i.e. a client that represents data as raw json maps with no knowledge of the schema)
+  is unaware of field on the union, but wants to clear
   the union entirely (assuming the union is optional), it will have no way of doing
   so without a discriminator. With a discriminator, the client can express its
   intention by setting the discriminator to the empty value and the server can
-  respects it intentions and clear any fields the client is unaware of.
+  respect its intentions and clear any fields the client is unaware of.
 * (Case #12 and #16) If a structured client is unaware of a field in the union that is set and it
   just wants to echo back the union it received in a get request (such as when
   updating other parts of the object), a client without a discriminator will
   silently drop the currently set field, while a client with the discriminator
-  will not change the discriminator value, indicating to the client that no
+  will not change the discriminator value, indicating to the server that no
   changes are desired in the union.
 * (Case #34 and #39) If a client sets a union field that the server is not aware of, the server
   will silently drop it and attempt to clear the object of the union field. With
@@ -227,7 +231,7 @@ conclusions are as follows (along with the test case number from the test matrix
   logic to detect that the previously set field has not been modified and that
   only one of the other union fields has been.
 
-### Go tags
+### Go Markers
 
 We're proposing a new type of tags for go types (in-tree types, and also
 kubebuilder types):
@@ -236,15 +240,16 @@ kubebuilder types):
 - `// +unionDiscriminator` before a field means that this field is the
   discriminator for the union. This field MUST be a string. This field MUST be
   required.
-- `// +unionAllowEmpty` before a discriminator field means that by setting the
+- `// +unionOptional` before a discriminator field means that by setting the
   discriminator to an empty string, none of the union fields are persisted
   (meaning at most one member of the union must be set). If not present on the
   discriminator, exactly one of the union members must be set in order to be
   valid.
-- `// +unionMember=<discriminatorName>` before a field means that this
-  field is a member of a union. The `<discriminatorName>` is optional if there
+- `// +unionMember=<memberName>,discriminatedBy=<discriminatorName>` before a field means that this
+  field is a member of a union. The `<memberName>` is the name of the field that must be set as the discriminator value. It defaults to the json representation of the field name if not specified. The `<discriminatorName>` is optional if there
   is only union in a struct and required if there are multiple unions per
-  struct.
+  struct. It should be the json representation of the field tagged with
+  `unionDiscriminator`.
 
 Note unions can't span across multiple go structures (all the fields that are part of a union has to be together in the
 same structure), examples of what is allowed:
@@ -272,15 +277,17 @@ type Union struct {
 	FieldB int `json:"fieldB"`
 }
 
-// This generates two unions, each with two fields and a discriminator. The 
+// This will generate one union that can be embedded because the members explicitly define their discriminator.
+// Also, the unionMember markers here demonstrate how to customize the names used for
+each field in the discriminator.
 type Union2 struct {
 	// +unionDiscriminator
         // +required
 	Type string `json:"type"`
-	// +unionMember=type
+	// +unionMember=ALPHA,discriminatedBy=type
         // +optional
 	Alpha int `json:"alpha"`
-	// +unionMember=type
+	// +unionMember=BETA,discriminatedBy=type
         // +optional
 	Beta int `json:"beta"`
 }
@@ -294,10 +301,10 @@ type InlinedUnion struct {
         // +unionAllowEmpty
         // +required
         FieldType string `json:"fieldType"`
-	// +unionMember=fieldType
+	// +unionMember,discriminatedBy=fieldType
 	// +optional
 	Field1 *int `json:"field1,omitempty"`
-	// +unionMember=fieldType
+	// +unionMember,discriminatedBy=fieldType
 	// +optional
 	Field2 *int `json:"field2,omitempty"`
 
@@ -337,16 +344,15 @@ Issues primarily arise here because of version skew between a client and a serve
 such as when a client is unaware of new fields added to a union and thus doesn't
 know how to clear these new fields when trying to set a different field.
 
-For unions with a discriminator (all newly created unions and some existing
-unions), normalization is simple: the server should always respect the
+For unions with a discriminator, normalization is simple: the server should always respect the
 discriminator.
 
 This means two things:
 1. when the server receives a request with a discriminator set to a
 given field, it should clear out any other fields that are set.
 2. when the server receives a request with a discriminator set to a given field,
-   but that given field is empty, it should maintain the value of the field that
-   currently exists.
+   but that given field is empty, the server should fail with a clear error
+   message.
 
 For situations where a typed client is unaware of a new field (that is currently
 set) and drops the set field such that no fields are now set, the server will

From c473f6dfefeaf00ce56add4d434a82b886cb09de Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Tue, 14 Jun 2022 20:13:56 +0000
Subject: [PATCH 08/22] Address feedback around open questions and PRR

---
 .../1027-api-unions/README.md                 | 103 +++++++++++++-----
 1 file changed, 77 insertions(+), 26 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 0d371246fc8..e27cbe17e66 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -16,7 +16,7 @@
   - [Risks and Mitigations](#risks-and-mitigations)
 - [Design Details](#design-details)
   - [Discriminator Field](#discriminator-field)
-  - [Go tags](#go-tags)
+  - [Go Markers](#go-markers)
   - [OpenAPI](#openapi)
   - [Normalization and Validation](#normalization-and-validation)
   - [Open Questions](#open-questions)
@@ -132,10 +132,12 @@ functions (e.g. `pkg/apis/<group>/validation/validation.go`).
   best match the intent of the client, despite the client potentially having
   incomplete information
 
+Another goal is to migrate at least one existing union to the new semantics in
+order to verify that migration is possible.
+
 ### Non-Goals
 
-The focus of this KEP is on providing unified validation and normalization of
-new union types. Migrating existing unions away from their bespoke validation
+Migrating all existing unions away from their bespoke validation
 logic (e.g validation functions), is an explicit non-goal and will be pursued in
 a separate KEP or later release.
 
@@ -248,7 +250,7 @@ kubebuilder types):
 - `// +unionMember=<memberName>,discriminatedBy=<discriminatorName>` before a field means that this
   field is a member of a union. The `<memberName>` is the name of the field that must be set as the discriminator value. It defaults to the json representation of the field name if not specified. The `<discriminatorName>` is optional if there
   is only union in a struct and required if there are multiple unions per
-  struct. It should be the json representation of the field tagged with
+  struct. It should be the go representation of the field tagged with
   `unionDiscriminator`.
 
 Note unions can't span across multiple go structures (all the fields that are part of a union has to be together in the
@@ -349,7 +351,13 @@ discriminator.
 
 This means two things:
 1. when the server receives a request with a discriminator set to a
-given field, it should clear out any other fields that are set.
+given field, and multiple member fields are set it should:
+  a. If the discriminator has been modified from the one set in the old object,
+  the server should clear all fields except the one pointed to by the discriminator.
+  b. If the discriminator is unchanged but a new member field has been set (in
+  addition to the existing one that has not been cleared), it should error
+  loudly that the client must change the discriminator if it changes any union
+  member fields.
 2. when the server receives a request with a discriminator set to a given field,
    but that given field is empty, the server should fail with a clear error
    message.
@@ -371,19 +379,36 @@ Objects must be validated AFTER the normalization process.
 A few open questions exist with the design that need to be resolved with SIG API
 Machinery before moving forward.
 
-1. When a server receives an update request with empty data in the member fields
-   but a validly set discriminator pointing to the object's union member field
-   that was previously set, should the server error and inform the client that
-   the field pointed to by the discriminator is currently empty, or should the
-   server retain the previous value it knows was set to the field being pointed
-   to by the discriminator?
+1. What do we do when the discriminator is set (to the previously set value),
+   but all union member fields are null? This likely implies that the client cleared
+   the union member unintentionally, given that the discriminator value remains.
+   Should the server:
+   a. Error loudly and inform the client that the field pointed to by the
+   discriminator is null?
+   b. Retain the previously set member field, present on the old object, absent
+   from the new object, but pointed to by the discriminator?
 2. What should the value of the discriminator be when no field in the union is
-   to be set. A couple options inclued an empty string, an field common to all
-   union (e.g. "NONE"), or a field specified on a per union basis.
+   to be set (i.e for optional unions).
+   a. mandate that the discriminator is the empty string for all optional unions?
+   b. make the field specified on a per union basis (defined in the go markers)?
+   c. mandate that the discriminator be unset (thus make the discriminator an
+   optional field for optional unios)?
 3. How should a server behave when it receives a union with multiple fields set
-   and the discriminator pointing to one of them? Reject the request, accept the
-   request but don't clear the fields, or automatically clear the fields not
-   specified by the discriminator.
+   and the discriminator modified to point to the newly set member field?
+   a. reject the request,
+   b. accept the request but don't clear any fields
+   c. accept the request and clear all the fields except the one chosen by the
+   discriminator
+4. Given that we want to migrate at least one existing union to use the new
+   semantics, which union should we do? Must we upgrade both a discriminated and
+   non-discriminated union for alpha or is only upgrading an existing
+   discriminated union sufficient?
+5. What should we default the discriminator value of a member field to if not
+   specified in the go markers?
+   a. the json representation of the field name
+   b. the go representation of the field name
+   Should we even give users the ability to define it or just stick to whatever
+   we decide the default is?
 
 ### Test Plan
 
@@ -480,7 +505,7 @@ We expect no non-infra related flakes in the last month as a GA graduation crite
 
 - CRDs can be created with union fields and are properly validated when
   created/updated.
-- Existing unions that have discriminators, are properly tagged and validated via
+- At least one existing unions that has discriminators, is properly tagged and validated via
   union validation
 - Existing unions that don't have discriminators do not break when upgraded.
 
@@ -498,8 +523,11 @@ enhancement:
   cluster required to make on upgrade, in order to make use of the enhancement?
 -->
 
-At first, Only new union types will follow the prescribed guinance, so no upgrades/downgrades are possible for types
-that don't exist yet.
+Turning the flag on for alpha just enables different runtime codepaths (i.e.
+performing the unified union validation and normalization)
+
+Any schema markers (added by CRD authors or propagated from tags on built-in
+types) will appear in the schema, but not do anything if the flag is off.
 
 ### Version Skew Strategy
 
@@ -520,6 +548,13 @@ See test matrix and commentary about discriminators. It clearly documents how
 the server will use the discriminator to understand the client's intention even
 if the client is not aware of all union fields because of version skew.
 
+Skew with alpha flag on/off shouldn't make much of a difference.
+* Objects created with the union semantics, but applied to a cluster with the
+  alpha flag off will simply not perform union validation and normalization.
+* Objects created without union semantics will simply not trigger union
+  validation and normalization (regardless of whether the server has the alpha
+  enabled or disabled).
+
 ## Production Readiness Review Questionnaire
 
 <!--
@@ -565,7 +600,7 @@ well as the [existing list] of feature gates.
 - [x] Feature gate (also fill in values in `kep.yaml`)
   - Feature gate name: APIUnions
   - Components depending on the feature gate: kube-apiserver
-  
+
 Request handlers in the api server will call into union validation and
 normalization function from the structured-merge-diff repo when feature is
 enabled.
@@ -573,7 +608,7 @@ enabled.
 
 ###### Does enabling the feature change any default behavior?
 
-No
+No, since all built in unions are currently validated in other ways.
 
 <!--
 Any change of default behavior may be surprising to users or break existing
@@ -597,7 +632,8 @@ Yes, requests will simply skip union validation and normalization.
 
 ###### What happens if we reenable the feature if it was previously rolled back?
 
-Requests will resume perform union validation and normalization
+Requests will resume performing union validation and normalization; there is no
+persistent state behind this feature.
 
 ###### Are there any tests for feature enablement/disablement?
 
@@ -669,7 +705,13 @@ previous answers based on experience in the field.
 
 ###### How can an operator determine if the feature is in use by workloads?
 
-Simply creating and updating objects with union fields.
+For builtins in alpha, it won't be possible to break clients since turning on vs
+off will validate the same thing via different code paths.
+
+For CRDs, you can see if they have the new union markers. If the CRD has no
+other validation mechanism, turning off the flag may result in CRs accepting
+invalid input.
+
 <!--
 Ideally, this should be a metric. Operations against the Kubernetes API (e.g.,
 checking if there are objects with field X set) may be a last resort. Avoid
@@ -678,10 +720,13 @@ logs or events for this purpose.
 
 ###### How can someone using this feature know that it is working for their instance?
 
-1. Create a new CRD with a union field
+1. Create a new CRD with a union field (and no other validation mechanism)
 2. Apply the CRD
 3. Create a CR with an invalid union (multiple fields set, no discriminator
    set), see if the CR is rejected via union validation
+
+When we write the e2e test, a standard union CRD and test CR will be obtainable
+for users to test on their instance.
 <!--
 For instance, if this is a pod-related feature, it should be possible to determine if the feature is functioning properly
 for each individual pod.
@@ -827,7 +872,9 @@ Describe them, providing:
 
 ###### Will enabling / using this feature result in increasing time taken by any operations covered by existing SLIs/SLOs?
 
-No
+In GA (maybe beta), we might expect resource reduction/reliability improvement,
+since this removes a need for webhooks.
+
 <!--
 Look at the [existing SLIs/SLOs].
 
@@ -839,7 +886,11 @@ Think about adding additional work or introducing new steps in between
 
 ###### Will enabling / using this feature result in non-negligible increase of resource usage (CPU, RAM, disk, IO, ...) in any components?
 
-No
+New validation and normalization logic should be negligible given that the
+functions will be in the same SMD path currently used by SSA code.
+
+We will have benchmarking to validate this assumption.
+
 <!--
 Things to keep in mind include: additional in-memory state, additional
 non-trivial computations, excessive access to disks (including increased log

From 3e4b47674d28147fbb0eb6d8cf5ad49f865461fe Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Tue, 14 Jun 2022 22:51:17 +0000
Subject: [PATCH 09/22] propose initial migration types

---
 .../1027-api-unions/README.md                 | 23 +++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index e27cbe17e66..e617cbf8715 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -19,6 +19,7 @@
   - [Go Markers](#go-markers)
   - [OpenAPI](#openapi)
   - [Normalization and Validation](#normalization-and-validation)
+    - [Migrating existing unions](#migrating-existing-unions)
   - [Open Questions](#open-questions)
   - [Test Plan](#test-plan)
       - [Prerequisite testing updates](#prerequisite-testing-updates)
@@ -133,7 +134,8 @@ functions (e.g. `pkg/apis/<group>/validation/validation.go`).
   incomplete information
 
 Another goal is to migrate at least one existing union to the new semantics in
-order to verify that migration is possible.
+order to verify that migration is possible. Ideally migrate one of each type of
+existing union (discriminated and non-discriminated).
 
 ### Non-Goals
 
@@ -374,6 +376,22 @@ currently done for existing union types.
 
 Objects must be validated AFTER the normalization process.
 
+#### Migrating existing unions
+
+As mentioned, one of the goals is to migrate at least one existing union to
+using the new marker based union validation and normalization. While open
+questions remain around the priority and urgency of migrating existing unions,
+nonetheless we should be able to come to a consensus on which types to migrate
+first.
+
+For discriminated unions, a couple relatively straightforward discriminated types are
+`MetricSpec` and `MetricStatus`. These have clearly defined discriminator values
+that map one-to-one to a member field, which make them good candidates for
+initial migration.
+
+For non-discriminated unions, there are a few relatively straightforward types
+that make good candidates for initial migration, such as `ContainerState`
+
 ### Open Questions
 
 A few open questions exist with the design that need to be resolved with SIG API
@@ -402,7 +420,8 @@ Machinery before moving forward.
 4. Given that we want to migrate at least one existing union to use the new
    semantics, which union should we do? Must we upgrade both a discriminated and
    non-discriminated union for alpha or is only upgrading an existing
-   discriminated union sufficient?
+   discriminated union sufficient? Thoughts on my proposed types of
+   `MetricStatus` (discriminated) and `ContainerState` (non-discriminated).
 5. What should we default the discriminator value of a member field to if not
    specified in the go markers?
    a. the json representation of the field name

From 7d09cfcedc4cfbad439325ab6f8acfd2c6eaed73 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Wed, 15 Jun 2022 18:15:54 +0000
Subject: [PATCH 10/22] rearrange open questions

---
 .../1027-api-unions/README.md                 | 36 ++++++++++++-------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index e617cbf8715..2b75812307a 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -401,33 +401,43 @@ Machinery before moving forward.
    but all union member fields are null? This likely implies that the client cleared
    the union member unintentionally, given that the discriminator value remains.
    Should the server:
+   
    a. Error loudly and inform the client that the field pointed to by the
    discriminator is null?
+   
    b. Retain the previously set member field, present on the old object, absent
    from the new object, but pointed to by the discriminator?
-2. What should the value of the discriminator be when no field in the union is
-   to be set (i.e for optional unions).
-   a. mandate that the discriminator is the empty string for all optional unions?
-   b. make the field specified on a per union basis (defined in the go markers)?
-   c. mandate that the discriminator be unset (thus make the discriminator an
-   optional field for optional unios)?
-3. How should a server behave when it receives a union with multiple fields set
+2. How should a server behave when it receives a union with multiple fields set
    and the discriminator modified to point to the newly set member field?
+   
    a. reject the request,
+   
    b. accept the request but don't clear any fields
+   
    c. accept the request and clear all the fields except the one chosen by the
    discriminator
-4. Given that we want to migrate at least one existing union to use the new
-   semantics, which union should we do? Must we upgrade both a discriminated and
-   non-discriminated union for alpha or is only upgrading an existing
-   discriminated union sufficient? Thoughts on my proposed types of
-   `MetricStatus` (discriminated) and `ContainerState` (non-discriminated).
-5. What should we default the discriminator value of a member field to if not
+3. What should the value of the discriminator be when no field in the union is
+   to be set (i.e for optional unions).
+   
+   a. mandate that the discriminator is the empty string for all optional unions?
+   
+   b. make the field specified on a per union basis (defined in the go markers)?
+   
+   c. mandate that the discriminator be unset (thus make the discriminator an
+   optional field for optional unions)?
+4. What should we default the discriminator value of a member field to if not
    specified in the go markers?
+   
    a. the json representation of the field name
+   
    b. the go representation of the field name
    Should we even give users the ability to define it or just stick to whatever
    we decide the default is?
+5. Given that we want to migrate at least one existing union to use the new
+   semantics, which union should we do? Must we upgrade both a discriminated and
+   non-discriminated union for alpha or is only upgrading an existing
+   discriminated union sufficient? Thoughts on my proposed types of
+   `MetricStatus` (discriminated) and `ContainerState` (non-discriminated).
 
 ### Test Plan
 

From e76861b1fda24b95558f98a63d48338509db9b04 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 16 Jun 2022 00:01:26 +0000
Subject: [PATCH 11/22] Address open questions based on meeting feedback

---
 .../1027-api-unions/README.md                 | 162 +++++++++++-------
 1 file changed, 100 insertions(+), 62 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 2b75812307a..3458300d5c8 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -17,10 +17,12 @@
 - [Design Details](#design-details)
   - [Discriminator Field](#discriminator-field)
   - [Go Markers](#go-markers)
+    - [Discriminator Values](#discriminator-values)
+      - [Empty Union Members](#empty-union-members)
+    - [Examples](#examples)
   - [OpenAPI](#openapi)
   - [Normalization and Validation](#normalization-and-validation)
     - [Migrating existing unions](#migrating-existing-unions)
-  - [Open Questions](#open-questions)
   - [Test Plan](#test-plan)
       - [Prerequisite testing updates](#prerequisite-testing-updates)
       - [Unit tests](#unit-tests)
@@ -133,10 +135,6 @@ functions (e.g. `pkg/apis/<group>/validation/validation.go`).
   best match the intent of the client, despite the client potentially having
   incomplete information
 
-Another goal is to migrate at least one existing union to the new semantics in
-order to verify that migration is possible. Ideally migrate one of each type of
-existing union (discriminated and non-discriminated).
-
 ### Non-Goals
 
 Migrating all existing unions away from their bespoke validation
@@ -242,19 +240,76 @@ kubebuilder types):
 
 
 - `// +unionDiscriminator` before a field means that this field is the
-  discriminator for the union. This field MUST be a string. This field MUST be
+  discriminator for the union. This field MUST be an enum defined as a string (see section on
+  discriminator values). This field MUST be
   required.
-- `// +unionOptional` before a discriminator field means that by setting the
-  discriminator to an empty string, none of the union fields are persisted
-  (meaning at most one member of the union must be set). If not present on the
-  discriminator, exactly one of the union members must be set in order to be
-  valid.
 - `// +unionMember=<memberName>,discriminatedBy=<discriminatorName>` before a field means that this
-  field is a member of a union. The `<memberName>` is the name of the field that must be set as the discriminator value. It defaults to the json representation of the field name if not specified. The `<discriminatorName>` is optional if there
+  field is a member of a union. The `<memberName>` is the name of the field that will be set as the discriminator value.
+  It MUST correspond to one of the valid enum values of the discriminator's enum
+  type. It defaults to the `CamelCase` representation of the field name if not specified. The `<discriminatorName>` is optional if there
   is only union in a struct and required if there are multiple unions per
-  struct. It should be the go representation of the field tagged with
+  struct. It should be the `CamelCase` representation of the field tagged with
   `unionDiscriminator`.
 
+#### Discriminator Values
+
+Here we present a description of how discriminators and their valid values
+should be defined.
+
+As described above, the discriminator field must be a string and required.
+Because, there are only a few specific values that the discriminator can be, we
+propose that all discriminators should be defined as an enum, and should be
+tagged so via the enum go marker `// +enum`.
+
+Required unions will have the number of valid discriminator values equal to the
+number of member fields (see exception below on empty union members). Optional
+unions will have the number of valid discriminator values equal to the number of
+member fields, plus one additional value for when "no member" is desired. By
+convention, this "no member" discriminator value should be the empty string.
+
+We define optional unions as union where "at most" one member field of the union
+must be non-nil (as opposed to a required union, where "exactly" one member
+field of the union must be non-nil).
+
+##### Empty Union Members
+
+In some cases there are more discriminator values than there are member fields
+defined in the struct when that specific member requires no configuration. An
+example is the `DeploymentStrategy` where it has one member field `rollingUpdate`,
+but two valid discriminator values `RollingUpdate` and `Recreate`. By using an
+enum as the discriminator value we are able to define values beyond the member
+fields in order to accommodate this pattern.
+
+#### Examples
+
+Below is an example of how to define a union based on the above design
+
+```
+// +enum
+type UnionType string
+
+const (
+       FieldA UnionType = "FieldA"
+       FieldB = "FieldC"
+       FieldC = "FieldC"
+       FieldNone = ""
+)
+
+type Union struct {
+	// +unionDiscriminator
+        // +required
+	UnionType UnionType
+
+        // +unionMember
+	// +optional
+	FieldA int
+        // +unionMember
+        // +optional
+	FieldB int
+}
+```
+
+
 Note unions can't span across multiple go structures (all the fields that are part of a union has to be together in the
 same structure), examples of what is allowed:
 
@@ -392,53 +447,6 @@ initial migration.
 For non-discriminated unions, there are a few relatively straightforward types
 that make good candidates for initial migration, such as `ContainerState`
 
-### Open Questions
-
-A few open questions exist with the design that need to be resolved with SIG API
-Machinery before moving forward.
-
-1. What do we do when the discriminator is set (to the previously set value),
-   but all union member fields are null? This likely implies that the client cleared
-   the union member unintentionally, given that the discriminator value remains.
-   Should the server:
-   
-   a. Error loudly and inform the client that the field pointed to by the
-   discriminator is null?
-   
-   b. Retain the previously set member field, present on the old object, absent
-   from the new object, but pointed to by the discriminator?
-2. How should a server behave when it receives a union with multiple fields set
-   and the discriminator modified to point to the newly set member field?
-   
-   a. reject the request,
-   
-   b. accept the request but don't clear any fields
-   
-   c. accept the request and clear all the fields except the one chosen by the
-   discriminator
-3. What should the value of the discriminator be when no field in the union is
-   to be set (i.e for optional unions).
-   
-   a. mandate that the discriminator is the empty string for all optional unions?
-   
-   b. make the field specified on a per union basis (defined in the go markers)?
-   
-   c. mandate that the discriminator be unset (thus make the discriminator an
-   optional field for optional unions)?
-4. What should we default the discriminator value of a member field to if not
-   specified in the go markers?
-   
-   a. the json representation of the field name
-   
-   b. the go representation of the field name
-   Should we even give users the ability to define it or just stick to whatever
-   we decide the default is?
-5. Given that we want to migrate at least one existing union to use the new
-   semantics, which union should we do? Must we upgrade both a discriminated and
-   non-discriminated union for alpha or is only upgrading an existing
-   discriminated union sufficient? Thoughts on my proposed types of
-   `MetricStatus` (discriminated) and `ContainerState` (non-discriminated).
-
 ### Test Plan
 
 [x] I/we understand the owners of the involved components may require updates to
@@ -502,6 +510,12 @@ A fully documented test matrix exists in a [google
 spreadsheet](https://docs.google.com/spreadsheets/d/1dIOOqgrgvMI9b2412eVuRSydEaOxhYOoqk6bUjZOY9c/edit?resourcekey=0-wlOfJTC_EX-qpU680STHMA#gid=3601413) along with a
 [guide
 doc](https://docs.google.com/document/d/1Wruosjo0ELLl1yxauzpsUjgH2fK9KdgXDmOdJ5sG7Kg/edit?resourcekey=0-8Pwzx6EvsFR7VQoXzCTY4Q) on how to read and understand the test matrix.
+
+As part of implementing the test matrix we will be able to prove the viability
+of upgrading existing unions by writing tests to mimic using the standardized
+union semantics on existing unions (even if actually upgrading these unions is
+outside the scope of alpha graduation)
+
 <!--
 This question should be filled when targeting a release.
 For Alpha, describe what tests will be added to ensure proper quality of the enhancement.
@@ -534,8 +548,8 @@ We expect no non-infra related flakes in the last month as a GA graduation crite
 
 - CRDs can be created with union fields and are properly validated when
   created/updated.
-- At least one existing unions that has discriminators, is properly tagged and validated via
-  union validation
+- Prove the viability of upgrading existing unions to the new semantics by
+  mimicking existing unions in e2e tests.
 - Existing unions that don't have discriminators do not break when upgraded.
 
 ### Upgrade / Downgrade Strategy
@@ -991,6 +1005,30 @@ allows the server to better understand the intentions of clients that do not
 have knowledge of all the fields in a union if newer versions of the server add
 new fields to the union.
 
+###### "None" Discriminator Values
+
+A number of strategies were discussed around how to represent the "none" value
+of the discriminator (see "Discriminator Values" section above).
+
+* One alternative was to mandate the "none" value always be the empty string.
+  The advantage to this is its simplicity and not creating a situation where
+  different API authors define there "none" value differently, so that anyone
+  could immediately know that a discriminator set to "" (empty string), is not
+  selecting any of the member fields. Also, it would allow us to not have to
+  define the set of enum values for each discriminator (as we could just use the
+  name of the member field). The disadvantage is that by not defining the set of
+  enum values, we make it impossible to support the "empty union members" case.
+* Another alternative was to make the discriminator a pointer to a string and
+  its value nil. The disadvantage here is that this requires more complicated
+  union validation logic (first do a nil check, then check the value) and makes
+  it harder to determine client intent on patches where the discriminator is not
+  set.
+* A third alternative is to require all unions be defined in their own separate
+  struct. This was rejected because there are many existing unions that define
+  random fields that are not members in the union within the same struct as
+  fields that do make up the union and we hope to be able to migrate at least
+  some of the existing unions to the new semantics.
+
 <!--
 What other approaches did you consider, and why did you rule them out? These do
 not need to be as detailed as the proposal, but should include enough

From 921f0a0c0a1a935078a07f7f72e8a71184d0f09e Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 16 Jun 2022 00:28:54 +0000
Subject: [PATCH 12/22] Address deads2k feedback

---
 .../1027-api-unions/README.md                 | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 3458300d5c8..0bee1c5b564 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -131,9 +131,9 @@ functions (e.g. `pkg/apis/<group>/validation/validation.go`).
 
 * Validation - ensuring only one member field is set (or at most one if
   desired).
-* Normalization - ensuring the API server can modify the fields of the oneOf to
-  best match the intent of the client, despite the client potentially having
-  incomplete information
+* Normalization - ensuring the API server can understand the intent of clients
+  that are unable to update/modify fields the clients are unaware of due to
+  version skew.
 
 ### Non-Goals
 
@@ -675,8 +675,9 @@ Yes, requests will simply skip union validation and normalization.
 
 ###### What happens if we reenable the feature if it was previously rolled back?
 
-Requests will resume performing union validation and normalization; there is no
-persistent state behind this feature.
+Custom resources that were skipping union validation when when the feature was
+rolled back may have allowed invalid data to persist. These must be updated to
+have valid data.
 
 ###### Are there any tests for feature enablement/disablement?
 
@@ -693,6 +694,10 @@ You can take a look at one potential example of such test in:
 https://github.com/kubernetes/kubernetes/pull/97058/files#diff-7826f7adbc1996a05ab52e3f5f02429e94b68ce6bce0dc534d1be636154fded3R246-R282
 -->
 
+We will have integration tests demonstrating how CRs with persisted invalid data
+will need to be corrected when the feature is re-enabled (and requires more
+strict union validation).
+
 ### Rollout, Upgrade and Rollback Planning
 
 <!--
@@ -715,7 +720,9 @@ will rollout across nodes.
 
 ###### What specific metrics should inform a rollback?
 
-N/A
+* `apiserver_request_total` could be watched to see if the number of create and
+  update requests that are failing increase substantially.
+
 <!--
 What signals should users be paying attention to when the feature is young
 that might indicate a serious problem?

From 7fa7d48961e54c9b5902f23fd1c1524cc58e00ef Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 16 Jun 2022 20:09:51 +0000
Subject: [PATCH 13/22] Address more feedback

---
 .../1027-api-unions/README.md                 | 70 ++++++++++++++-----
 1 file changed, 53 insertions(+), 17 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 0bee1c5b564..9e954b66032 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -241,15 +241,20 @@ kubebuilder types):
 
 - `// +unionDiscriminator` before a field means that this field is the
   discriminator for the union. This field MUST be an enum defined as a string (see section on
-  discriminator values). This field MUST be
-  required.
-- `// +unionMember=<memberName>,discriminatedBy=<discriminatorName>` before a field means that this
+  discriminator values). This field MUST be required if there is no default
+  option, omitempty if the default option is the empty string, or optional and
+  omitempty if a default value is specified with the `// +default` marker.
+- `// +unionMember=<memberName> before a field means that this
   field is a member of a union. The `<memberName>` is the name of the field that will be set as the discriminator value.
   It MUST correspond to one of the valid enum values of the discriminator's enum
-  type. It defaults to the `CamelCase` representation of the field name if not specified. The `<discriminatorName>` is optional if there
-  is only union in a struct and required if there are multiple unions per
-  struct. It should be the `CamelCase` representation of the field tagged with
-  `unionDiscriminator`.
+  type. It defaults to the go (i.e `CamelCase`) representation of the field name if not specified.
+  `<memberName>` should only be set if authors want to customize how the fields
+  are represented in the discriminator field.
+- `// +unionDiscriminatedBy=<discriminatorName>` before a member field identifies which
+  discriminator (and thus which union) the member field belongs to. Optional
+  unless there are multiple unions/discriminators in a single struct. If used,
+  it must be the go (i.e. `CamelCase`) representation of the field name tagged
+  with `unionDiscriminator`.
 
 #### Discriminator Values
 
@@ -290,8 +295,9 @@ type UnionType string
 
 const (
        FieldA UnionType = "FieldA"
-       FieldB = "FieldC"
+       FieldB = "FieldB"
        FieldC = "FieldC"
+       FieldD = "FieldD"
        FieldNone = ""
 )
 
@@ -321,12 +327,22 @@ type TopLevelUnion struct {
 	Union `json:",inline"`
 }
 
+// +enum
+type UnionType string
+
+const (
+       FieldA UnionType = "FieldA"
+       FieldB = "FieldB"
+       FieldC = "FieldC"
+       FieldD = "FieldD"
+       FieldNone = ""
+)
+
 // This will generate one union, with two fields and a discriminator.
 type Union struct {
 	// +unionDiscriminator
-        // +unionAllowEmpty
         // +required
-	UnionType string `json:"unionType"`
+	UnionType UnionType `json:"unionType"`
 
         // +unionMember
 	// +optional
@@ -336,34 +352,54 @@ type Union struct {
 	FieldB int `json:"fieldB"`
 }
 
+// +enum
+type Union2Type string
+
+const (
+    Alpha Union2Type = "ALPHA"
+    Beta = "BETA"
+)
+
 // This will generate one union that can be embedded because the members explicitly define their discriminator.
 // Also, the unionMember markers here demonstrate how to customize the names used for
 each field in the discriminator.
 type Union2 struct {
 	// +unionDiscriminator
         // +required
-	Type string `json:"type"`
-	// +unionMember=ALPHA,discriminatedBy=type
+	Type2 Union2Type `json:"type"`
+	// +unionMember=ALPHA,
+        // +unionDiscriminatedBy=Type2
         // +optional
 	Alpha int `json:"alpha"`
-	// +unionMember=BETA,discriminatedBy=type
+	// +unionMember=BETA
+        // +unionDiscriminatedBy=Type2
         // +optional
 	Beta int `json:"beta"`
 }
 
+// +enum
+type FieldType string
+
+const (
+    Field1 FieldType = "Field1"
+    Field2 = "Field2"
+    FieldNone = "None"
+)
+
 // This has 3 embedded unions:
 // One for the fields that are directly embedded, one for Union, and one for Union2.
 type InlinedUnion struct {
 	Name string `json:"name"`
 
         // +unionDiscriminator
-        // +unionAllowEmpty
         // +required
-        FieldType string `json:"fieldType"`
-	// +unionMember,discriminatedBy=fieldType
+        FieldType FieldType `json:"fieldType"`
+	// +unionMember
+        // +unionDiscriminatedBy=FieldType
 	// +optional
 	Field1 *int `json:"field1,omitempty"`
-	// +unionMember,discriminatedBy=fieldType
+	// +unionMember
+        // +unionDiscriminatedBy=FieldType
 	// +optional
 	Field2 *int `json:"field2,omitempty"`
 

From ae462d3ee9617e2906a7909d821cc2e3b1ad714c Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Fri, 17 Jun 2022 20:17:52 +0000
Subject: [PATCH 14/22] Describe possibility of enabling a cluster with invalid
 CRs

---
 keps/sig-api-machinery/1027-api-unions/README.md | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 9e954b66032..d0a0d0a5006 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -687,7 +687,13 @@ enabled.
 
 ###### Does enabling the feature change any default behavior?
 
-No, since all built in unions are currently validated in other ways.
+Enabling the feature could cause existing CRs to fail validation if the
+correspond CRD has union fields and the existing CRs have invalid unions that
+were unvalidated when initially created in a cluster that had the unions feature
+disabled.
+
+These CRs will need to be corrected in order to pass validation (or the feature
+disabled).
 
 <!--
 Any change of default behavior may be surprising to users or break existing

From 03cbe6cb4f6094334f50d5fac1681705907602cc Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Tue, 21 Jun 2022 20:34:36 +0000
Subject: [PATCH 15/22] ratcheting validation

---
 .../1027-api-unions/README.md                 | 36 +++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index d0a0d0a5006..e8ff912ab59 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -22,6 +22,7 @@
     - [Examples](#examples)
   - [OpenAPI](#openapi)
   - [Normalization and Validation](#normalization-and-validation)
+    - [Ratcheting Validation](#ratcheting-validation)
     - [Migrating existing unions](#migrating-existing-unions)
   - [Test Plan](#test-plan)
       - [Prerequisite testing updates](#prerequisite-testing-updates)
@@ -467,6 +468,29 @@ currently done for existing union types.
 
 Objects must be validated AFTER the normalization process.
 
+#### Ratcheting Validation
+
+When updating CRDs to support union validation, it is possible that existing CRs
+become invalid.
+
+The naive solution is to require existing CRs to be updated to a valid state
+before they can be updated again.
+
+This creates many potential landmines, and so ratcheting validation is proposed
+as an alternative. Ratcheting validation means that objects will ignore stricter
+validation rules if and only if the existing object also fails the stricter
+validation for the same reason.
+
+Ratcheting validation for custom resources is a separate effort proposed
+outside of this unions effort. For the initial alpha graduation of unions, we
+do not propose supporting ratcheting validation. We will require all invalid CRs
+to be made valid before they can be updated (the naive solution).
+
+In order to potentially support ratcheting validation in the future, we will
+ensure that all callers of union validation retain access to both old and new
+objects, so that future ratcheting validation can be implemented within the
+union validation library.
+
 #### Migrating existing unions
 
 As mentioned, one of the goals is to migrate at least one existing union to
@@ -718,8 +742,16 @@ Yes, requests will simply skip union validation and normalization.
 ###### What happens if we reenable the feature if it was previously rolled back?
 
 Custom resources that were skipping union validation when when the feature was
-rolled back may have allowed invalid data to persist. These must be updated to
-have valid data.
+rolled back may have allowed invalid data to persist.
+
+For alpha, we require that all modifying requests (update/patch) fail unless the
+data passes union validation. Retrieving newly invalid CRs should still always
+succeed.
+
+In the future, we may require looser "ratcheting validation" which would allow
+modifications to ignore union validation if the existing object fails the union
+validation for the same reason as the new object (see section on "Ratcheting
+Validation" above). This is not a priority for alpha.
 
 ###### Are there any tests for feature enablement/disablement?
 

From 908da47d80d292fcbd1d4b190f9edd67e6473c63 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Tue, 21 Jun 2022 21:51:36 +0000
Subject: [PATCH 16/22] more normalization/validation updates

---
 keps/sig-api-machinery/1027-api-unions/README.md | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index e8ff912ab59..ed8bc403627 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -22,6 +22,8 @@
     - [Examples](#examples)
   - [OpenAPI](#openapi)
   - [Normalization and Validation](#normalization-and-validation)
+    - [Normalization](#normalization)
+    - [Validation](#validation)
     - [Ratcheting Validation](#ratcheting-validation)
     - [Migrating existing unions](#migrating-existing-unions)
   - [Test Plan](#test-plan)
@@ -432,6 +434,8 @@ Conversion between OpenAPI v2 and OpenAPI v3 will preserve these fields.
 
 ### Normalization and Validation
 
+#### Normalization
+
 Normalization refers to the process by which the API server attempts to understand
 and correct clients which may provide the server with conflicting or incomplete
 information about a union.
@@ -466,8 +470,16 @@ Union types without a discriminator must continue to perform normalization and
 validation manually (i.e. per resource in the validation functions), as is
 currently done for existing union types.
 
+#### Validation
+
 Objects must be validated AFTER the normalization process.
 
+For custom resources, union validation will be done at the same point as the
+existing structural schema validation that occurs in the custom resource handler.
+This ensures that any generic validation changes made to all custom resources (such
+as the ratcheting validation discussed below), behaves appropriately with union
+validation.
+
 #### Ratcheting Validation
 
 When updating CRDs to support union validation, it is possible that existing CRs

From 0d0bcb706ac840b6f8e12526972821afdf27dfd4 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Wed, 22 Jun 2022 18:15:38 +0000
Subject: [PATCH 17/22] Address liggitt feedback

---
 .../1027-api-unions/README.md                 | 191 +++++++++---------
 1 file changed, 100 insertions(+), 91 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index ed8bc403627..8287d4060fd 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -252,7 +252,8 @@ kubebuilder types):
   It MUST correspond to one of the valid enum values of the discriminator's enum
   type. It defaults to the go (i.e `CamelCase`) representation of the field name if not specified.
   `<memberName>` should only be set if authors want to customize how the fields
-  are represented in the discriminator field.
+  are represented in the discriminator field. `<memberName>` should match the
+  serialized JSON name of the field case-insensitively.
 - `// +unionDiscriminatedBy=<discriminatorName>` before a member field identifies which
   discriminator (and thus which union) the member field belongs to. Optional
   unless there are multiple unions/discriminators in a single struct. If used,
@@ -297,24 +298,24 @@ Below is an example of how to define a union based on the above design
 type UnionType string
 
 const (
-       FieldA UnionType = "FieldA"
-       FieldB = "FieldB"
-       FieldC = "FieldC"
-       FieldD = "FieldD"
-       FieldNone = ""
+  FieldA UnionType = "FieldA"
+  FieldB UnionType = "FieldB"
+  FieldC UnionType = "FieldC"
+  FieldD UnionType = "FieldD"
+  FieldNone UnionType = ""
 )
 
 type Union struct {
-	// +unionDiscriminator
-        // +required
-	UnionType UnionType
-
-        // +unionMember
-	// +optional
-	FieldA int
-        // +unionMember
-        // +optional
-	FieldB int
+  // +unionDiscriminator
+  // +required
+  UnionType UnionType
+
+  // +unionMember
+  // +optional
+  FieldA int
+  // +unionMember
+  // +optional
+  FieldB int
 }
 ```
 
@@ -325,91 +326,91 @@ same structure), examples of what is allowed:
 ```
 // This will have one embedded union.
 type TopLevelUnion struct {
-	Name string `json:"name"`
+  Name string `json:"name"`
 
-	Union `json:",inline"`
+  Union `json:",inline"`
 }
 
 // +enum
 type UnionType string
 
 const (
-       FieldA UnionType = "FieldA"
-       FieldB = "FieldB"
-       FieldC = "FieldC"
-       FieldD = "FieldD"
-       FieldNone = ""
+  FieldA UnionType = "FieldA"
+  FieldB UnionType = "FieldB"
+  FieldC UnionType = "FieldC"
+  FieldD UnionType = "FieldD"
+  FieldNone UnionType = ""
 )
 
 // This will generate one union, with two fields and a discriminator.
 type Union struct {
-	// +unionDiscriminator
-        // +required
-	UnionType UnionType `json:"unionType"`
-
-        // +unionMember
-	// +optional
-	FieldA int `json:"fieldA"`
-        // +unionMember
-        // +optional
-	FieldB int `json:"fieldB"`
+  // +unionDiscriminator
+  // +required
+  UnionType UnionType `json:"unionType"`
+
+  // +unionMember
+  // +optional
+  FieldA int `json:"fieldA"`
+  // +unionMember
+  // +optional
+  FieldB int `json:"fieldB"`
 }
 
 // +enum
 type Union2Type string
 
 const (
-    Alpha Union2Type = "ALPHA"
-    Beta = "BETA"
+  Alpha Union2Type = "ALPHA"
+  Beta = "BETA"
 )
 
 // This will generate one union that can be embedded because the members explicitly define their discriminator.
 // Also, the unionMember markers here demonstrate how to customize the names used for
 each field in the discriminator.
 type Union2 struct {
-	// +unionDiscriminator
-        // +required
-	Type2 Union2Type `json:"type"`
-	// +unionMember=ALPHA,
-        // +unionDiscriminatedBy=Type2
-        // +optional
-	Alpha int `json:"alpha"`
-	// +unionMember=BETA
-        // +unionDiscriminatedBy=Type2
-        // +optional
-	Beta int `json:"beta"`
+  // +unionDiscriminator
+  // +required
+  Type2 Union2Type `json:"type"`
+  // +unionMember=ALPHA,
+  // +unionDiscriminatedBy=Type2
+  // +optional
+  Alpha int `json:"alpha"`
+  // +unionMember=BETA
+  // +unionDiscriminatedBy=Type2
+  // +optional
+  Beta int `json:"beta"`
 }
 
 // +enum
 type FieldType string
 
 const (
-    Field1 FieldType = "Field1"
-    Field2 = "Field2"
-    FieldNone = "None"
+  Field1 FieldType = "Field1"
+  Field2 = "Field2"
+  FieldNone = "None"
 )
 
 // This has 3 embedded unions:
 // One for the fields that are directly embedded, one for Union, and one for Union2.
 type InlinedUnion struct {
-	Name string `json:"name"`
-
-        // +unionDiscriminator
-        // +required
-        FieldType FieldType `json:"fieldType"`
-	// +unionMember
-        // +unionDiscriminatedBy=FieldType
-	// +optional
-	Field1 *int `json:"field1,omitempty"`
-	// +unionMember
-        // +unionDiscriminatedBy=FieldType
-	// +optional
-	Field2 *int `json:"field2,omitempty"`
-
-        // Union does not label its members, so it
-        cannot be inlined
-	union Union  `json:"union"`
-	Union2 `json:",inline"`
+  Name string `json:"name"`
+
+  // +unionDiscriminator
+  // +required
+  FieldType FieldType `json:"fieldType"`
+  // +unionMember
+  // +unionDiscriminatedBy=FieldType
+  // +optional
+  Field1 *int `json:"field1,omitempty"`
+  // +unionMember
+  // +unionDiscriminatedBy=FieldType
+  // +optional
+  Field2 *int `json:"field2,omitempty"`
+
+  // Union does not label its members, so it
+  cannot be inlined
+  union Union  `json:"union"`
+  Union2 `json:",inline"`
 }
 ```
 
@@ -438,42 +439,40 @@ Conversion between OpenAPI v2 and OpenAPI v3 will preserve these fields.
 
 Normalization refers to the process by which the API server attempts to understand
 and correct clients which may provide the server with conflicting or incomplete
-information about a union.
+information about a union in update or patch requests.
 
 Issues primarily arise here because of version skew between a client and a server,
 such as when a client is unaware of new fields added to a union and thus doesn't
 know how to clear these new fields when trying to set a different field.
 
-For unions with a discriminator, normalization is simple: the server should always respect the
+For unions that follow this design, normalization is simple: the server should always respect the
 discriminator.
 
-This means two things:
-1. when the server receives a request with a discriminator set to a
-given field, and multiple member fields are set it should:
-  a. If the discriminator has been modified from the one set in the old object,
-  the server should clear all fields except the one pointed to by the discriminator.
-  b. If the discriminator is unchanged but a new member field has been set (in
-  addition to the existing one that has not been cleared), it should error
-  loudly that the client must change the discriminator if it changes any union
-  member fields.
-2. when the server receives a request with a discriminator set to a given field,
-   but that given field is empty, the server should fail with a clear error
-   message.
-
-For situations where a typed client is unaware of a new field (that is currently
-set) and drops the set field such that no fields are now set, the server will
-clear all the fields of the union. This is unavoidable for unions without a
-discriminator and a major reason why we recommend all new unions have a
-discriminator.
+This means that when the server receives an update request with a discriminator set to a
+given field, and multiple member fields are set it should clear all fields
+except the one pointed to by the discriminator _if and only if_ the
+discriminator has been modified. Having multiple fields set, and a discriminator
+not modified is invalid and caught later by the validation step (see below).
 
-Union types without a discriminator must continue to perform normalization and
-validation manually (i.e. per resource in the validation functions), as is
-currently done for existing union types.
+For both custom resources and built-in types, we expect union normalization to be
+called by the request handlers shortly after mutating admission occurs.
 
 #### Validation
 
 Objects must be validated AFTER the normalization process.
 
+Some validation situations specific to unions are:
+1. When multiple union fields are set and the discriminator is not set we should
+   error loudly that the client must change the discriminator if it changes any
+   union member fields.
+2. When the server receiveds a request with a discriminator set to a given
+   field, but that given field is empty, the server should fail with a clear
+   error message. Note this does not apply to discriminator values that do not
+   correspond to any field (as in the "empty union members case").
+
+For both custom resources and built-in types, validation will occur as part of
+the request validation, before validating admission occurs.
+
 For custom resources, union validation will be done at the same point as the
 existing structural schema validation that occurs in the custom resource handler.
 This ensures that any generic validation changes made to all custom resources (such
@@ -493,7 +492,8 @@ as an alternative. Ratcheting validation means that objects will ignore stricter
 validation rules if and only if the existing object also fails the stricter
 validation for the same reason.
 
-Ratcheting validation for custom resources is a separate effort proposed
+Ratcheting validation for custom resources is a [separate
+effort](https://github.com/kubernetes/kubernetes/issues/94060) proposed
 outside of this unions effort. For the initial alpha graduation of unions, we
 do not propose supporting ratcheting validation. We will require all invalid CRs
 to be made valid before they can be updated (the naive solution).
@@ -519,6 +519,15 @@ initial migration.
 For non-discriminated unions, there are a few relatively straightforward types
 that make good candidates for initial migration, such as `ContainerState`
 
+Until migrated, union types without a discriminator (i.e. only existing unions that have not been migrated
+to the current desgin), cannot be tagged with the go markers described above and
+thus will not be treated as "unions" in the sense of this currently proposed
+normalization and validation logic.
+
+These legacy unions must continue to perform normalization and
+validation manually, per resource in the validation functions.
+
+
 ### Test Plan
 
 [x] I/we understand the owners of the involved components may require updates to

From a67ef11651ec4c767b7715a13681fb8b28577d64 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Wed, 22 Jun 2022 23:15:21 +0000
Subject: [PATCH 18/22] sketch open api

---
 .../1027-api-unions/README.md                 | 140 +++++++++++++++---
 1 file changed, 119 insertions(+), 21 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 8287d4060fd..5b5679870ad 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -247,13 +247,19 @@ kubebuilder types):
   discriminator values). This field MUST be required if there is no default
   option, omitempty if the default option is the empty string, or optional and
   omitempty if a default value is specified with the `// +default` marker.
-- `// +unionMember=<memberName> before a field means that this
+- `// +unionMember=<memberName>,<optional> before a field means that this
   field is a member of a union. The `<memberName>` is the name of the field that will be set as the discriminator value.
   It MUST correspond to one of the valid enum values of the discriminator's enum
   type. It defaults to the go (i.e `CamelCase`) representation of the field name if not specified.
   `<memberName>` should only be set if authors want to customize how the fields
   are represented in the discriminator field. `<memberName>` should match the
-  serialized JSON name of the field case-insensitively.
+  serialized JSON name of the field case-insensitively. The comma separated
+  optional value determines whether or not the member field must be set when the
+  discriminator selects it. Meaning, when `optional` is present on a field, the
+  discriminator can select the field even if the field is not set. If optional
+  is not present in the `unionMember` tag, then the object will fail validation
+  if the discriminator selects the field but it is nil. A field can be marked as
+  optional without specifying memberName via `// +unionMember,optional`.
 - `// +unionDiscriminatedBy=<discriminatorName>` before a member field identifies which
   discriminator (and thus which union) the member field belongs to. Optional
   unless there are multiple unions/discriminators in a single struct. If used,
@@ -270,15 +276,10 @@ Because, there are only a few specific values that the discriminator can be, we
 propose that all discriminators should be defined as an enum, and should be
 tagged so via the enum go marker `// +enum`.
 
-Required unions will have the number of valid discriminator values equal to the
-number of member fields (see exception below on empty union members). Optional
-unions will have the number of valid discriminator values equal to the number of
-member fields, plus one additional value for when "no member" is desired. By
-convention, this "no member" discriminator value should be the empty string.
-
-We define optional unions as union where "at most" one member field of the union
-must be non-nil (as opposed to a required union, where "exactly" one member
-field of the union must be non-nil).
+If no option is a valid option for a union, such an option must be defined as a
+member of the discriminator values enum. By convention, this "no member"
+discriminator should be the empty string, but there is nothing stopping API
+authors from defining their own "no option" discriminator value.
 
 ##### Empty Union Members
 
@@ -423,16 +424,113 @@ for validation, but is "on-top" of this proposal.
 A new extension is created in the openapi to describe the behavior:
 `x-kubernetes-unions`.
 
-This is a list of unions that are part of this structure/object. Here is what
-each list item is made of:
-- `discriminator: <discriminator>` is set to the name of the discriminator
-  field, if present,
-- `fields-to-discriminateBy: {"<fieldName>": "<discriminateName>"}` is a map of
-  fields that belong to the union to their discriminated names. The
-  discriminatedValue will typically be set to the name of the Go variable.
+This is a list of unions that are part of this structure/object. Each item in
+the list represents a discriminator for the union, a list of valid discriminator
+unions that do not correspond to member fields, and for each member field,
+the discriminator value of that field and whether or not that field is optional.
 
 Conversion between OpenAPI v2 and OpenAPI v3 will preserve these fields.
 
+The following is an example of what the generated OpenAPI definition will look
+like for a given go type.
+
+```
+const (
+  FieldA Union1Type = "FieldA"
+  FieldB Union1Type = "FieldB"
+  FieldC Union1Type = "FieldC"
+  FieldD Union1Type = "FieldD"
+  FieldNone Union1Type = ""
+)
+const (
+  Alpha Union2Type = "ALPHA"
+  Beta Union2Type = "BETA"
+  Gamma Union2Type = "GAMMA"
+  Null Union2Type = "NULL"
+)
+
+// This will generate one union, with two fields and a discriminator.
+type Union struct {
+  // +unionDiscriminator
+  // +required
+  Union1 Union1Type `json:"union1"`
+
+  // +unionMember
+  // +unionDiscriminatedBy=Union1
+  // +optional
+  FieldA int `json:"fieldA"`
+  // +unionMember,optional
+  // +unionDiscriminatedBy=Union1
+  // +optional
+  FieldB int `json:"fieldB"`
+
+  // +unionDiscriminator
+  // +required
+  Union2 Union2Type `json:"union2"`
+
+  // +unionMember=ALPHA,
+  // +unionDiscriminatedBy=Union2
+  // +optional
+  Alpha int `json:"alpha"`
+  // +unionMember=BETA,optional
+  // +unionDiscriminatedBy=Union2
+  // +optional
+  Beta int `json:"beta"`
+}
+```
+
+turns into:
+```
+OpenAPIDefinition{
+  Schema: spec.Schema{
+    SchemaProps: spec.SchemaProps{
+      ... // schema props omitted
+    },
+    VendorExtensible: spec.VendorExtensible{
+      Extensions: spec.Extensions{
+        "x-kubernetes-unions": []interface{}{
+          map[string]interface{}{
+            "discriminator": "Union1",
+            "emptyMembers":[]string{
+              "FieldC",
+              "FieldD",
+              "",
+            },
+            "fields-to-discriminateBy": map[string]interface{}{
+              "FieldA": map[string]interface{}{
+                "discriminatorValue": "FieldA",
+                "optional": false,
+              }
+              "FieldB": map[string]interface{}{
+                "discriminatorValue": "FieldB",
+                "optional": true,
+              }
+            }
+          },
+          map[string]interface{}{
+            "discriminator": "Union2",
+            "emptyMembers":[]string{
+              "GAMMA",
+              "NULL",
+            },
+            "fields-to-discriminateBy": map[string]interface{}{
+              "Alpha": map[string]interface{}{
+                "discriminatorValue": "ALPHA",
+                "optional": false,
+              }
+              "Beta": map[string]interface{}{
+                "discriminatorValue": "BETA",
+                "optional": true,
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
+```
+
 ### Normalization and Validation
 
 #### Normalization
@@ -462,11 +560,11 @@ called by the request handlers shortly after mutating admission occurs.
 Objects must be validated AFTER the normalization process.
 
 Some validation situations specific to unions are:
-1. When multiple union fields are set and the discriminator is not set we should
+1. When multiple union fields are set and the discriminator has not been modified we should
    error loudly that the client must change the discriminator if it changes any
    union member fields.
-2. When the server receiveds a request with a discriminator set to a given
-   field, but that given field is empty, the server should fail with a clear
+2. When the server receives a request with a discriminator set to a given
+   field, but that given field is empty and not marked as optional, the server should fail with a clear
    error message. Note this does not apply to discriminator values that do not
    correspond to any field (as in the "empty union members case").
 

From 182167ddef73553d1cb280281cbb96cb93dc11b8 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 23 Jun 2022 01:26:33 +0000
Subject: [PATCH 19/22] remove empty members from open api definition

---
 keps/sig-api-machinery/1027-api-unions/README.md | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 5b5679870ad..3735c916792 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -425,8 +425,7 @@ A new extension is created in the openapi to describe the behavior:
 `x-kubernetes-unions`.
 
 This is a list of unions that are part of this structure/object. Each item in
-the list represents a discriminator for the union, a list of valid discriminator
-unions that do not correspond to member fields, and for each member field,
+the list represents a discriminator for the union, and for each member field,
 the discriminator value of that field and whether or not that field is optional.
 
 Conversion between OpenAPI v2 and OpenAPI v3 will preserve these fields.
@@ -491,11 +490,6 @@ OpenAPIDefinition{
         "x-kubernetes-unions": []interface{}{
           map[string]interface{}{
             "discriminator": "Union1",
-            "emptyMembers":[]string{
-              "FieldC",
-              "FieldD",
-              "",
-            },
             "fields-to-discriminateBy": map[string]interface{}{
               "FieldA": map[string]interface{}{
                 "discriminatorValue": "FieldA",
@@ -509,10 +503,6 @@ OpenAPIDefinition{
           },
           map[string]interface{}{
             "discriminator": "Union2",
-            "emptyMembers":[]string{
-              "GAMMA",
-              "NULL",
-            },
             "fields-to-discriminateBy": map[string]interface{}{
               "Alpha": map[string]interface{}{
                 "discriminatorValue": "ALPHA",

From 9bcae5ad671e3a5ba69f3e4233a3d37f6bd97e9b Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 23 Jun 2022 06:27:24 +0000
Subject: [PATCH 20/22] Add open api deserialization struct

---
 .../1027-api-unions/README.md                 | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 3735c916792..9f31e8e9edf 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -520,6 +520,50 @@ OpenAPIDefinition{
   }
 }
 ```
+The OpenAPI data will then be deserialized into go structs as follows:
+```
+// XKubernetesUnions is the top level extension
+// that lists all the unions in the object.
+// Each union is determined by its identifying
+// discriminator.
+type XKubernetesUnions struct {
+  // Discriminators are the list of unions in an object.
+  // There is a 1:1 mapping between a union and its discriminator
+  // and hence we identify a union by its discriminator.
+  Discriminators []Discriminator
+}
+
+// Discriminator defines a union. It has
+// a name and a list of member fields
+type Discriminator struct {
+  // Name is the go (CamelCase) representation of the
+  // discriminator field
+  // It is the value that `// +unionDiscriminatedbBy=<discriminatorName>`
+  // should be set to on member fields.
+  Name string
+  // FieldsToDiscriminateBy are all the member fields that
+  // are in a given discriminator's union.
+  FieldsToDiscriminateBy []MemberField
+}
+
+// MemberField is a member of a union.
+// It has a go representation of the field name
+type MemberField struct {
+  // The camel case representation of the member field used to identify which
+  // field in the union struct corresponds to the member.
+  GoName string
+  // DiscriminatorName is the value that the discriminator is set to in order
+  // to identify a member field as the currently chosen one.
+  // It will only be different from the GoName if API authors set a member
+  // name value in the union member marker `// +unionMember=<MemberName>`
+  DiscriminatorName string
+  // Optional determines whether the discriminator can be set to a member field
+  // even if the member field is not present (or nil). Default is false, and is
+  // set to true by setting the union member marker as optional, i.e.
+  // `// +unionMember,optional` or `// +unionMember=<MemberName>,optional`
+  Optional bool
+}
+```
 
 ### Normalization and Validation
 

From 5e85c6049e871cd3d0fd5ae08a03d74a2fcdaf22 Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 23 Jun 2022 20:20:23 +0000
Subject: [PATCH 21/22] OpenAPI extension feedback

---
 .../sig-api-machinery/1027.yaml               |   2 +-
 .../1027-api-unions/README.md                 | 100 ++++--------------
 2 files changed, 23 insertions(+), 79 deletions(-)

diff --git a/keps/prod-readiness/sig-api-machinery/1027.yaml b/keps/prod-readiness/sig-api-machinery/1027.yaml
index 865ab5fc50d..ef0e558d303 100644
--- a/keps/prod-readiness/sig-api-machinery/1027.yaml
+++ b/keps/prod-readiness/sig-api-machinery/1027.yaml
@@ -1,3 +1,3 @@
 kep-number: 1027
 alpha:
-        approver: "@deads2k"
+        approver: "@johnbelamaric"
diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 9f31e8e9edf..1676ad2e7f4 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -247,7 +247,7 @@ kubebuilder types):
   discriminator values). This field MUST be required if there is no default
   option, omitempty if the default option is the empty string, or optional and
   omitempty if a default value is specified with the `// +default` marker.
-- `// +unionMember=<memberName>,<optional> before a field means that this
+- `// +unionMember[=<memberName>][,optional] before a field means that this
   field is a member of a union. The `<memberName>` is the name of the field that will be set as the discriminator value.
   It MUST correspond to one of the valid enum values of the discriminator's enum
   type. It defaults to the go (i.e `CamelCase`) representation of the field name if not specified.
@@ -478,90 +478,34 @@ type Union struct {
 }
 ```
 
-turns into:
-```
-OpenAPIDefinition{
-  Schema: spec.Schema{
-    SchemaProps: spec.SchemaProps{
-      ... // schema props omitted
-    },
-    VendorExtensible: spec.VendorExtensible{
-      Extensions: spec.Extensions{
-        "x-kubernetes-unions": []interface{}{
-          map[string]interface{}{
-            "discriminator": "Union1",
-            "fields-to-discriminateBy": map[string]interface{}{
-              "FieldA": map[string]interface{}{
-                "discriminatorValue": "FieldA",
-                "optional": false,
-              }
-              "FieldB": map[string]interface{}{
-                "discriminatorValue": "FieldB",
-                "optional": true,
-              }
-            }
-          },
-          map[string]interface{}{
-            "discriminator": "Union2",
-            "fields-to-discriminateBy": map[string]interface{}{
-              "Alpha": map[string]interface{}{
-                "discriminatorValue": "ALPHA",
-                "optional": false,
-              }
-              "Beta": map[string]interface{}{
-                "discriminatorValue": "BETA",
-                "optional": true,
-              }
-            }
-          }
-        }
-      }
-    }
-  }
-}
-```
-The OpenAPI data will then be deserialized into go structs as follows:
+The OpenAPI x-kubernetes-unions extension will then be attached to the discriminator's property and
+deserialized into go structs as follows:
 ```
 // XKubernetesUnions is the top level extension
-// that lists all the unions in the object.
-// Each union is determined by its identifying
-// discriminator.
 type XKubernetesUnions struct {
-  // Discriminators are the list of unions in an object.
-  // There is a 1:1 mapping between a union and its discriminator
-  // and hence we identify a union by its discriminator.
-  Discriminators []Discriminator
+  // FieldMembers are the mapping of all valid discriminator values in a
+  // union to the corresponding member field.
+  // Discriminator value is the value to which the discriminator is set to
+  // in order to indicate that a given member field is the currently set member
+  // of the union.
+  // MemberField may be nil in the case of empty union members where a valid
+  // discriminator value has no corresponding member field.
+  FieldMembers map[DiscriminatorValue]*MemberField `json:"fieldMembers"`
 }
 
-// Discriminator defines a union. It has
-// a name and a list of member fields
-type Discriminator struct {
-  // Name is the go (CamelCase) representation of the
-  // discriminator field
-  // It is the value that `// +unionDiscriminatedbBy=<discriminatorName>`
-  // should be set to on member fields.
-  Name string
-  // FieldsToDiscriminateBy are all the member fields that
-  // are in a given discriminator's union.
-  FieldsToDiscriminateBy []MemberField
-}
+// DiscriminatorValue is the value that the discriminator is set to 
+// in order to indicate the selection of a union member.
+type DiscriminatorValue string
 
-// MemberField is a member of a union.
-// It has a go representation of the field name
+// MemberField
 type MemberField struct {
-  // The camel case representation of the member field used to identify which
-  // field in the union struct corresponds to the member.
-  GoName string
-  // DiscriminatorName is the value that the discriminator is set to in order
-  // to identify a member field as the currently chosen one.
-  // It will only be different from the GoName if API authors set a member
-  // name value in the union member marker `// +unionMember=<MemberName>`
-  DiscriminatorName string
-  // Optional determines whether the discriminator can be set to a member field
-  // even if the member field is not present (or nil). Default is false, and is
-  // set to true by setting the union member marker as optional, i.e.
-  // `// +unionMember,optional` or `// +unionMember=<MemberName>,optional`
-  Optional bool
+  // Name is the name of the field corresponding to the member.
+  // It will be the json representation of the field marked with the
+  // `// +unionMember` marker in the go type.
+  Name string `json:"name"`
+  // Optional determines whether the discriminator _may_ select this member
+  // even when the member field is empty. Optional defaults to false.
+  Optional bool `json:"optional"`
 }
 ```
 

From 7da001c0cd2b7c986d1a694171d3e21cc6eaca3c Mon Sep 17 00:00:00 2001
From: Kevin Delgado <kevindelgado@google.com>
Date: Thu, 23 Jun 2022 20:28:22 +0000
Subject: [PATCH 22/22] remove multiple unions per object possibility

---
 .../1027-api-unions/README.md                 | 67 ++-----------------
 1 file changed, 5 insertions(+), 62 deletions(-)

diff --git a/keps/sig-api-machinery/1027-api-unions/README.md b/keps/sig-api-machinery/1027-api-unions/README.md
index 1676ad2e7f4..bc04280820f 100644
--- a/keps/sig-api-machinery/1027-api-unions/README.md
+++ b/keps/sig-api-machinery/1027-api-unions/README.md
@@ -260,11 +260,6 @@ kubebuilder types):
   is not present in the `unionMember` tag, then the object will fail validation
   if the discriminator selects the field but it is nil. A field can be marked as
   optional without specifying memberName via `// +unionMember,optional`.
-- `// +unionDiscriminatedBy=<discriminatorName>` before a member field identifies which
-  discriminator (and thus which union) the member field belongs to. Optional
-  unless there are multiple unions/discriminators in a single struct. If used,
-  it must be the go (i.e. `CamelCase`) representation of the field name tagged
-  with `unionDiscriminator`.
 
 #### Discriminator Values
 
@@ -352,7 +347,7 @@ type Union struct {
   // +unionMember
   // +optional
   FieldA int `json:"fieldA"`
-  // +unionMember
+  // +unionMember,optional
   // +optional
   FieldB int `json:"fieldB"`
 }
@@ -365,54 +360,20 @@ const (
   Beta = "BETA"
 )
 
-// This will generate one union that can be embedded because the members explicitly define their discriminator.
-// Also, the unionMember markers here demonstrate how to customize the names used for
-each field in the discriminator.
+// This generates a union where the unionMember markers demonstrate how to
+customize the names used for each field in the discriminator.
 type Union2 struct {
   // +unionDiscriminator
   // +required
   Type2 Union2Type `json:"type"`
-  // +unionMember=ALPHA,
-  // +unionDiscriminatedBy=Type2
+  // +unionMember=ALPHA
   // +optional
   Alpha int `json:"alpha"`
-  // +unionMember=BETA
-  // +unionDiscriminatedBy=Type2
+  // +unionMember=BETA,optional
   // +optional
   Beta int `json:"beta"`
 }
 
-// +enum
-type FieldType string
-
-const (
-  Field1 FieldType = "Field1"
-  Field2 = "Field2"
-  FieldNone = "None"
-)
-
-// This has 3 embedded unions:
-// One for the fields that are directly embedded, one for Union, and one for Union2.
-type InlinedUnion struct {
-  Name string `json:"name"`
-
-  // +unionDiscriminator
-  // +required
-  FieldType FieldType `json:"fieldType"`
-  // +unionMember
-  // +unionDiscriminatedBy=FieldType
-  // +optional
-  Field1 *int `json:"field1,omitempty"`
-  // +unionMember
-  // +unionDiscriminatedBy=FieldType
-  // +optional
-  Field2 *int `json:"field2,omitempty"`
-
-  // Union does not label its members, so it
-  cannot be inlined
-  union Union  `json:"union"`
-  Union2 `json:",inline"`
-}
 ```
 
 ### OpenAPI
@@ -441,12 +402,6 @@ const (
   FieldD Union1Type = "FieldD"
   FieldNone Union1Type = ""
 )
-const (
-  Alpha Union2Type = "ALPHA"
-  Beta Union2Type = "BETA"
-  Gamma Union2Type = "GAMMA"
-  Null Union2Type = "NULL"
-)
 
 // This will generate one union, with two fields and a discriminator.
 type Union struct {
@@ -463,18 +418,6 @@ type Union struct {
   // +optional
   FieldB int `json:"fieldB"`
 
-  // +unionDiscriminator
-  // +required
-  Union2 Union2Type `json:"union2"`
-
-  // +unionMember=ALPHA,
-  // +unionDiscriminatedBy=Union2
-  // +optional
-  Alpha int `json:"alpha"`
-  // +unionMember=BETA,optional
-  // +unionDiscriminatedBy=Union2
-  // +optional
-  Beta int `json:"beta"`
 }
 ```