From 32c483fa20a5839b88ed68c41cdcbc8b8ab4cd6c Mon Sep 17 00:00:00 2001
From: Lars Kellogg-Stedman <lars@redhat.com>
Date: Thu, 2 Nov 2023 15:48:02 -0400
Subject: [PATCH] Add openapi schema for proper patching of CRDs

Due to issues with the way Kustomize handles patching lists in custom
resource types, we need to provide a complete OpenAPI schema that
includes schema definitions for any CRDs we wish to patch.

See `cluster-scope/base/openapi/README.md` for more information.
---
 .pre-commit-config.yaml                       |  1 +
 cluster-scope/base/openapi/README.md          | 97 +++++++++++++++++++
 cluster-scope/base/openapi/kustomization.yaml |  5 +
 .../overlays/common/kustomization.yaml        |  2 +
 4 files changed, 105 insertions(+)
 create mode 100644 cluster-scope/base/openapi/README.md
 create mode 100644 cluster-scope/base/openapi/kustomization.yaml

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index d8b64555..06559204 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -12,6 +12,7 @@ repos:
       - id: check-merge-conflict
       - id: end-of-file-fixer
       - id: check-added-large-files
+        exclude: ^cluster-scope/base/openapi/
       - id: check-case-conflict
       - id: check-json
       - id: check-symlinks
diff --git a/cluster-scope/base/openapi/README.md b/cluster-scope/base/openapi/README.md
new file mode 100644
index 00000000..7c3399f4
--- /dev/null
+++ b/cluster-scope/base/openapi/README.md
@@ -0,0 +1,97 @@
+# Why is this OpenAPI schema here?
+
+Many kubernetes resources include lists of objects, such the
+`spec.containers` field of Pods:
+
+    spec:
+      containers:
+        - name: container1
+          image: example
+        - name: container2
+          image: example
+
+Kustomize is able to apply a strategic merge patch to specific
+elements in this list by matching the patch against a "merge key".
+That is, if we want to add an `env` attribute to `container2`, we can
+create a patch like this:
+
+    apiVersion: v1
+    kind: Pod
+    metadata:
+      name: example
+    spec:
+      containers:
+        - name: container2
+          env:
+            - MYVAR=somevalue
+
+Kustomize "knows" to patch the list item with `name` equal to
+`container2` because it has a compiled in schema for native kubernetes
+resources that identifies the appropriate merge keys (the `name`
+field). We end up with:
+
+    spec:
+      containers:
+        - name: container1
+          image: example
+        - name: container2
+          image: example
+          env:
+            - MYVAR=somevalue
+
+This won't work by default for custom resource types. For example, if
+we have a resource like this:
+
+    apiVersion: multicluster.openshift.io/v1
+    kind: MultiClusterEngine
+    metadata:
+      name: multiclusterengine
+    spec:
+      overrides:
+        components:
+        - enabled: true
+          name: local-cluster
+        - enabled: true
+          name: assisted-service
+        - enabled: false
+          name: hypershift-preview
+
+And we try to modify it with a patch like this:
+
+    apiVersion: multicluster.openshift.io/v1
+    kind: MultiClusterEngine
+    metadata:
+      name: multiclusterengine
+    spec:
+      overrides:
+        components:
+        - enabled: true
+          name: hypershift-preview
+
+We will see that rather than making the expected change (setting
+`enabled` to `true` for the list item with `name:
+hypershift-preview`), this will simply *replace* the entire content of
+the `components` key.
+
+It is possible to provide Kustomize with a custom openapi schema that
+informs of it of the appropriate merge keys for this sort of custom
+resource. There is documentation and an example at [1], but
+unfortunately this example is misleading. After reading that document
+you may believe that you can provide a schema specific to the custom
+resources you want to patch, but that is not the case: a custom schema
+*completely replaces* the compiled-in schema, so for example with the
+configuration shown in that example, Kustomize would no longer be able
+to correctly patch containers in Pods and Deployments. This problem is
+more fully discussed in [2], and there is some discussion in
+kubernetes/kubernetes#82942 and kubernetes-sigs/kustomize#4613.
+
+[1]: https://github.com/kubernetes-sigs/kustomize/blob/master/examples/customOpenAPIschema.md
+[2]: https://blog.argoproj.io/argo-crds-and-kustomize-the-problem-of-patching-lists-5cfc43da288c
+
+Strategic merge patches are the only way to patch lists by
+object id, rather than by index, so it's functionality we need. To
+support this, that means we need to dump the complete openapi schema
+from our clusters to a file and make that available to Kustomize. This
+commit introduces an initial version of this schema, but we will need
+to update this in the future whenever we want to patch CRDs that
+weren't available at the time we last generated this schema dump.
diff --git a/cluster-scope/base/openapi/kustomization.yaml b/cluster-scope/base/openapi/kustomization.yaml
new file mode 100644
index 00000000..80ed70e3
--- /dev/null
+++ b/cluster-scope/base/openapi/kustomization.yaml
@@ -0,0 +1,5 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+
+openapi:
+  path: https://github.com/nerc-project/openshift-schemas/raw/main/openshift-api-schema.json
diff --git a/cluster-scope/overlays/common/kustomization.yaml b/cluster-scope/overlays/common/kustomization.yaml
index b6351712..5b14b69c 100644
--- a/cluster-scope/overlays/common/kustomization.yaml
+++ b/cluster-scope/overlays/common/kustomization.yaml
@@ -1,6 +1,8 @@
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
+
 resources:
+- ../../base/openapi
 - ../../base/config.openshift.io/oauths/cluster
 - ../../base/rbac.authorization.k8s.io/clusterroles/node-labeler/
 - ../../base/rbac.authorization.k8s.io/clusterrolebindings/self-provisioners