-
Notifications
You must be signed in to change notification settings - Fork 585
OCPEDGE-2084: chore(tools): bump controller-tools with XValidation ordering fix #2658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,7 +125,8 @@ type ConsoleSampleSpec struct { | |
|
|
||
| // ConsoleSampleSourceType is an enumeration of the supported sample types. | ||
| // Unsupported samples types will be ignored in the web console. | ||
| // +kubebuilder:validation:Enum:=GitImport;ContainerImport | ||
| // +kubebuilder:validation:Enum:="GitImport";"ContainerImport" | ||
| // +enum | ||
|
Comment on lines
+128
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Search for enum values in the generated OpenAPI file
fd -e go -path "*generated*openapi*" | head -5Repository: openshift/api Length of output: 225 🏁 Script executed: # Check what enum marker syntax is used in the codebase
rg '\+kubebuilder:validation:Enum' --type go -B 2 -A 2 | head -60Repository: openshift/api Length of output: 4190 🌐 Web query:
💡 Result: Short answer — use the +kubebuilder:validation:Enum marker with either semicolon-separated values or a quoted Go-syntax slice. Examples:
Reference: Kubebuilder marker docs — CRD validation and marker-syntax. [1][2] Sources Fix the non-standard The correct kubebuilder marker syntax uses to: 🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this feedback points to the correct/preferred semantic, the existing tag is actually ok and works just fine, we don't need to call out these differences There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Learnings added: Applicable to openshift/api |
||
| type ConsoleSampleSourceType string | ||
|
|
||
| const ( | ||
|
|
@@ -144,7 +145,6 @@ type ConsoleSampleSource struct { | |
| // type of the sample, currently supported: "GitImport";"ContainerImport" | ||
| // +unionDiscriminator | ||
| // +required | ||
| // +kubebuilder:validation:Enum:="GitImport";"ContainerImport" | ||
| Type ConsoleSampleSourceType `json:"type"` | ||
|
|
||
| // gitImport allows the user to import code from a git repository. | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14421,9 +14421,13 @@ | |
| "$ref": "#/definitions/com.github.openshift.api.console.v1.ConsoleSampleGitImportSource" | ||
| }, | ||
| "type": { | ||
| "description": "type of the sample, currently supported: \"GitImport\";\"ContainerImport\"", | ||
| "description": "type of the sample, currently supported: \"GitImport\";\"ContainerImport\"\n\nPossible enum values:\n - `\"ContainerImport\"` A sample that let the user import a container image.\n - `\"GitImport\"` A sample that let the user import code from a git repository.", | ||
| "type": "string", | ||
| "default": "" | ||
| "default": "", | ||
| "enum": [ | ||
| "ContainerImport", | ||
| "GitImport" | ||
| ] | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| }, | ||
| "x-kubernetes-unions": [ | ||
|
|
@@ -24706,6 +24710,10 @@ | |
| "format": "int32", | ||
| "default": 0 | ||
| }, | ||
| "synchronizedAPI": { | ||
| "description": "synchronizedAPI holds the last stable value of authoritativeAPI. It is used to detect migration cancellation requests and to restore the resource to its previous state. Valid values are \"MachineAPI\" and \"ClusterAPI\". When omitted, the resource has not yet been reconciled by the migration controller.", | ||
| "type": "string" | ||
| }, | ||
|
Comment on lines
+24713
to
+24716
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Schema doesn’t enforce the stated valid values. 🔧 Suggested schema tightening (apply to both locations) "synchronizedAPI": {
"description": "synchronizedAPI holds the last stable value of authoritativeAPI. It is used to detect migration cancellation requests and to restore the resource to its previous state. Valid values are \"MachineAPI\" and \"ClusterAPI\". When omitted, the resource has not yet been reconciled by the migration controller.",
- "type": "string"
+ "type": "string",
+ "enum": [
+ "MachineAPI",
+ "ClusterAPI"
+ ]
},Also applies to: 24820-24823 🤖 Prompt for AI Agents |
||
| "synchronizedGeneration": { | ||
| "description": "synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with. This field is set when the authoritative resource is updated and the sync controller has updated the non-authoritative resource to match.", | ||
| "type": "integer", | ||
|
|
@@ -24809,6 +24817,10 @@ | |
| "description": "providerStatus details a Provider-specific status. It is recommended that providers maintain their own versioned API types that should be serialized/deserialized from this field.", | ||
| "$ref": "#/definitions/io.k8s.apimachinery.pkg.runtime.RawExtension" | ||
| }, | ||
| "synchronizedAPI": { | ||
| "description": "synchronizedAPI holds the last stable value of authoritativeAPI. It is used to detect migration cancellation requests and to restore the resource to its previous state. Valid values are \"MachineAPI\" and \"ClusterAPI\". When omitted, the resource has not yet been reconciled by the migration controller.", | ||
| "type": "string" | ||
| }, | ||
| "synchronizedGeneration": { | ||
| "description": "synchronizedGeneration is the generation of the authoritative resource that the non-authoritative resource is synchronised with. This field is set when the authoritative resource is updated and the sync controller has updated the non-authoritative resource to match.", | ||
| "type": "integer", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -115,7 +115,7 @@ $(OUTPUT_DIR)/kube-api-linter.so: $(OUTPUT_DIR)/vendor-version | |
| ln -fs $(OUTPUT_DIR)/kube-api-linter.so $(TOOLS_DIR)/_output/bin/kube-api-linter.so | ||
|
|
||
| $(OUTPUT_DIR)/openapi-gen: $(OUTPUT_DIR)/vendor-version | ||
| go build -mod=vendor -o $(OUTPUT_DIR)/openapi-gen ./vendor/k8s.io/code-generator/cmd/openapi-gen | ||
| go build -mod=vendor -o $(OUTPUT_DIR)/openapi-gen ./vendor/k8s.io/kube-openapi/cmd/openapi-gen | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How come this is changing? 🤔 Appears not to be affecting anything, was the previous location deprecated?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't build without it. I'm not sure why. I assumed this was changed upstream?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Announcement: https://groups.google.com/a/kubernetes.io/g/dev/c/Ix-ACY9DhEs |
||
|
|
||
| $(OUTPUT_DIR)/protoc-gen-gogo: $(OUTPUT_DIR)/vendor-version | ||
| go build -mod=vendor -o $(OUTPUT_DIR)/protoc-gen-gogo ./vendor/k8s.io/code-generator/cmd/go-to-protobuf/protoc-gen-gogo | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: openshift/api
Length of output: 93
🌐 Web query:
controller-tools kubebuilder +enum marker validation:Enum syntax documentation💡 Result:
Short summary — syntax for the +kubebuilder:validation:Enum marker
Basic form: place the marker on the field comment:
// +kubebuilder:validation:Enum=. [1][2]
Value lists can be written two ways:
// +kubebuilder:validation:Enum=Wallace;Gromit;Chicken. [1][2]
// +kubebuilder:validation:Enum={"crackers, Gromit, we forgot the crackers!","not even wensleydale?"}. [1][2]
Notes:
Sources:
[1] Kubebuilder markers reference (controller-gen marker syntax & examples).
[2] Kubebuilder CRD validation docs (Enum examples).
[3] sigs.k8s.io/controller-tools/pkg/crd/markers (pkg docs).
[4] Practical pitfalls (example of misspelling/case-sensitivity).
🌐 Web query:
kubebuilder validation Enum quoted values semicolon separator controller-gen💡 Result:
Short answer: controller-gen accepts two forms for +kubebuilder:validation:Enum.
Simple single-word values (no spaces/punctuation): semicolon-separated, quotes optional.
Example: // +kubebuilder:validation:Enum=Wallace;Gromit;Chicken. [1][2]
Values that need quotes (spaces, punctuation, or contain semicolons): use a brace-enclosed, comma-separated quoted list (JSON-like).
Example: // +kubebuilder:validation:Enum={"crackers, Gromit, we forgot the crackers!","not even wensleydale?"}. [1][2]
Notes:
Sources:
[1] Kubebuilder markers docs (Enum examples).
[2] Kubebuilder book v3 markers page (semicolon and quoted-list examples).
🏁 Script executed:
Repository: openshift/api
Length of output: 801
🌐 Web query:
kubebuilder validation Enum marker "Enum:=" syntax colon equals💡 Result:
Short answer: there is no special "colon‑equals" operator — the correct marker syntax uses =. What you may be seeing is the marker name including a colon (e.g. items:Enum) followed by =, which can look like ":=" when written without spacing. Example forms from the Kubebuilder docs:
So "Enum:=" is just the marker name with a colon plus the normal "=" assignment; use +kubebuilder:validation:Enum= or +kubebuilder:validation:items:Enum= as shown in the docs. [1][2]
Sources:
[1] Kubebuilder markers reference — Marker Syntax & Enum examples. [2] Kubebuilder CRD validation reference.
🏁 Script executed:
Repository: openshift/api
Length of output: 793
🌐 Web query:
kubebuilder +enum marker controller-gen what does it do💡 Result:
Short answer: the +kubebuilder:validation:Enum marker tells controller-gen to emit an OpenAPI v3 "enum" validation for that Go field into the generated CRD. controller-gen reads the marker comment on your type/field when you run make manifests (or controller-gen crd ...), and writes the enum list into spec.versions[*].schema.openAPIV3Schema so the API server will validate values. [1][2][3][4]
Notes/usage:
Sources: Kubebuilder/controller-gen docs and examples. [1][2][3][4][5]
🏁 Script executed:
Repository: openshift/api
Length of output: 287
🏁 Script executed:
Repository: openshift/api
Length of output: 34841
🌐 Web query:
kubebuilder validation Enum colon-equals syntax ":=" marker💡 Result:
Kubebuilder/controller-gen markers use = (equals) for arguments (e.g. // +kubebuilder:validation:Enum=One;Two;Three). There is no documented ":=" (colon-equals) marker syntax in the kubebuilder/controller-tools marker docs — if you’re seeing ":=" it’s likely a typo, a different tool’s output, or comes from some other layer. See the marker syntax and Enum examples in the Kubebuilder docs. [1][2]
If you can paste the exact comment or generated CRD showing ":=", I’ll inspect it and explain where it came from and how to fix it.
Sources:
Fix enum marker syntax: remove colon from
Enum:=.The marker uses
Enum:=which is not standard kubebuilder/controller-tools syntax. The correct form isEnum=with an equals sign only. Update line 128 to:The colon character will cause controller-gen to misparse or ignore the marker, potentially failing to emit enum validation into the generated CRD.
🤖 Prompt for AI Agents