bottlerocket-update-operator icon indicating copy to clipboard operation
bottlerocket-update-operator copied to clipboard

schemars `v0.8.11` broke the brupop CRD

Open jpmcb opened this issue 1 year ago • 3 comments

Problem

Something in the new schemars v0.8.11 library broke in regards to how our openapiv3 schema is consumed and how enum types are generated. It appears they are now being serialized as oneOf: https://github.com/GREsau/schemars/releases/tag/v0.8.11

If using this newer version off develop, I am unable to apply our custom resource that is generated in the yaml:

❯ k apply -f ./bottlerocket-update-operator.yaml

...

Error from server (Invalid): error when creating "./bottlerocket-update-operator.yaml": CustomResourceDefinition.apiextensions.k8s.io "bottlerocketshadows.brupop.bottlerocket.aws" is invalid: [spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[0].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[0].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[1].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[1].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[2].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[2].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[3].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[3].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[4].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[4].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[spec].properties[state].type: Required value: must not be empty for specified object fields, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[0].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[0].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[1].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[1].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[2].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[2].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[3].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[3].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[4].description: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[4].type: Forbidden: must be empty to be structural, spec.versions[0].schema.openAPIV3Schema.properties[status].properties[current_state].type: Required value: must not be empty for specified object fields, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[0].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[0].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[1].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[1].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[2].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[2].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[3].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[3].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[4].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].oneOf[4].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[spec].properties[state].type: Required value: must not be empty for specified object fields, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[0].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[0].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[1].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[1].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[2].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[2].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[3].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[3].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[4].description: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].oneOf[4].type: Forbidden: must be empty to be structural, spec.versions[1].schema.openAPIV3Schema.properties[status].properties[current_state].type: Required value: must not be empty for specified object fields]

My assumption is this is coming from this chunk where we define the JsonSchema on our custom resources state:

https://github.com/bottlerocket-os/bottlerocket-update-operator/blob/4fb5849557378d1bfa28f20986802c2f47e5687f/models/src/node/crd/v2.rs#L20-L38

and is somehow incompatible with what is generated for the oneOf key: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#specifying-a-structural-schema

Here's the main difference in my git diff:

diff --git a/yamlgen/deploy/bottlerocket-update-operator.yaml b/yamlgen/deploy/bottlerocket-update-operator.yaml
index c6e4dc7..4bb071c 100644
--- a/yamlgen/deploy/bottlerocket-update-operator.yaml
+++ b/yamlgen/deploy/bottlerocket-update-operator.yaml
@@ -54,13 +54,27 @@ spec:
               properties:
                 state:
                   description: "Records the desired state of the `BottlerocketShadow`"
-                  enum:
-                    - Idle
-                    - StagedAndPerformedUpdate
-                    - RebootedIntoUpdate
-                    - MonitoringUpdate
-                    - ErrorReset
-                  type: string
+                  oneOf:
+                    - description: "Nodes in this state are waiting for new updates to become available. This is both the starting, terminal and recovery state in the update process."
+                      enum:
+                        - Idle
+                      type: string
+                    - description: "Nodes in this state have staged a new update image, have installed the new image, and have updated the partition table to mark it as the new active image."
+                      enum:
+                        - StagedAndPerformedUpdate
+                      type: string
+                    - description: "Nodes in this state have used the kubernetes cordon and drain APIs to remove running pods, have un-cordoned the node to allow work to be scheduled, and have rebooted after performing an update."
+                      enum:
+                        - RebootedIntoUpdate
+                      type: string
+                    - description: Nodes in this state are monitoring to ensure that the node seems healthy before marking the update as complete.
+                      enum:
+                        - MonitoringUpdate
+                      type: string
+                    - description: Nodes in this state have crashed due to Bottlerocket Update API call failure.
+                      enum:
+                        - ErrorReset
+                      type: string
                 state_transition_timestamp:
                   description: The time at which the most recent state was set as the desired state.
                   nullable: true

This was also seen in testsys: https://github.com/bottlerocket-os/bottlerocket-test-system/pull/593

TLDR: This needs it's own investigation

Intermittent solution

Quickfix incoming to lock to v0.8.10 but we'll want to figure out what we need to change to continue to work with

jpmcb avatar Oct 07 '22 22:10 jpmcb

Cc @webern - maybe you have an idea of what's going on here?

jpmcb avatar Oct 10 '22 17:10 jpmcb

Cc @webern - maybe you have an idea of what's going on here?

Not at first glance. Looks like this will require a deep dive.

webern avatar Oct 10 '22 18:10 webern

Opened an issue in kube with investigation, small example, and quickfix: https://github.com/kube-rs/kube/issues/1047

jpmcb avatar Oct 10 '22 20:10 jpmcb