-
Notifications
You must be signed in to change notification settings - Fork 68
🌱 new tag symmetry and required validations #2358
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
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 |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ import ( | |
| "log" | ||
| "os" | ||
| "regexp" | ||
| "slices" | ||
| "strings" | ||
|
|
||
| apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" | ||
|
|
@@ -136,7 +137,7 @@ func runGenerator(args ...string) { | |
| if channel == StandardChannel && strings.Contains(version.Name, "alpha") { | ||
| channelCrd.Spec.Versions[i].Served = false | ||
| } | ||
| version.Schema.OpenAPIV3Schema.Properties = opconTweaksMap(channel, version.Schema.OpenAPIV3Schema.Properties) | ||
| version.Schema.OpenAPIV3Schema.Properties, version.Schema.OpenAPIV3Schema.Required = opconTweaksMap(channel, version.Schema.OpenAPIV3Schema.Properties, version.Schema.OpenAPIV3Schema.Required) | ||
| } | ||
|
|
||
| conv, err := crd.AsVersion(*channelCrd, apiextensionsv1.SchemeGroupVersion) | ||
|
|
@@ -179,25 +180,51 @@ func runGenerator(args ...string) { | |
| } | ||
| } | ||
|
|
||
| func opconTweaksMap(channel string, props map[string]apiextensionsv1.JSONSchemaProps) map[string]apiextensionsv1.JSONSchemaProps { | ||
| // Apply Opcon specific tweaks to all properties in a map, and return a list of required fields according to opcon tags. | ||
| // For opcon validation optional/required tags, the required list is updated accordingly. | ||
| func opconTweaksMap(channel string, props map[string]apiextensionsv1.JSONSchemaProps, existingRequired []string) (map[string]apiextensionsv1.JSONSchemaProps, []string) { | ||
| newRequired := slices.Clone(existingRequired) | ||
|
|
||
| for name := range props { | ||
| jsonProps := props[name] | ||
| p := opconTweaks(channel, name, jsonProps) | ||
| p, reqStatus := opconTweaks(channel, name, jsonProps) | ||
| if p == nil { | ||
| delete(props, name) | ||
| } else { | ||
| props[name] = *p | ||
| // Update required list based on tag | ||
| switch reqStatus { | ||
| case statusRequired: | ||
| if !slices.Contains(newRequired, name) { | ||
| newRequired = append(newRequired, name) | ||
| } | ||
| case statusOptional: | ||
| newRequired = slices.DeleteFunc(newRequired, func(s string) bool { return s == name }) | ||
| default: | ||
| // "" (unspecified) means keep existing status | ||
| } | ||
| } | ||
| } | ||
| return props | ||
| return props, newRequired | ||
| } | ||
|
|
||
| // Custom Opcon API Tweaks for tags prefixed with `<opcon:` that get past | ||
| // the limitations of Kubebuilder annotations. | ||
| func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSchemaProps) *apiextensionsv1.JSONSchemaProps { | ||
| // Returns the modified schema and a string indicating required status where indicated by opcon tags: | ||
| // "required", "optional", or "" (no decision -- preserve any non-opcon required status). | ||
|
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. instead of strings, would it make sense to use a
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.
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'm generally aligning with CRD guidance that bools end up becoming enums/strings/complex types, and just trying to plumb through a large enough opening to allow expansion, if other decisions need to be added.
Comment on lines
+213
to
+214
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. this is the documentatino for |
||
|
|
||
| const ( | ||
| statusRequired = "required" | ||
| statusOptional = "optional" | ||
| statusNoOpinion = "" | ||
| ) | ||
|
|
||
| func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSchemaProps) (*apiextensionsv1.JSONSchemaProps, string) { | ||
| requiredStatus := statusNoOpinion | ||
|
|
||
| if channel == StandardChannel { | ||
| if strings.Contains(jsonProps.Description, "<opcon:experimental>") { | ||
| return nil | ||
| return nil, statusNoOpinion | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -219,7 +246,7 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche | |
|
|
||
| numValid++ | ||
| jsonProps.Enum = []apiextensionsv1.JSON{} | ||
| for _, val := range strings.Split(enumMatch[1], ";") { | ||
| for val := range strings.SplitSeq(enumMatch[1], ";") { | ||
| jsonProps.Enum = append(jsonProps.Enum, apiextensionsv1.JSON{Raw: []byte("\"" + val + "\"")}) | ||
| } | ||
| } | ||
|
|
@@ -237,6 +264,21 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche | |
| Rule: celMatch[2], | ||
| }) | ||
| } | ||
| optReqRe := regexp.MustCompile(validationPrefix + "(Optional|Required)>") | ||
| optReqMatches := optReqRe.FindAllStringSubmatch(jsonProps.Description, 64) | ||
| for _, optReqMatch := range optReqMatches { | ||
| if len(optReqMatch) != 2 { | ||
| log.Fatalf("Invalid %s Optional/Required tag for %s", validationPrefix, name) | ||
| } | ||
|
|
||
| numValid++ | ||
| switch optReqMatch[1] { | ||
| case "Optional": | ||
| requiredStatus = statusOptional | ||
| case "Required": | ||
| requiredStatus = statusRequired | ||
| } | ||
| } | ||
|
Comment on lines
+267
to
+281
|
||
| } | ||
|
|
||
| if numValid < numExpressions { | ||
|
|
@@ -246,34 +288,42 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche | |
| jsonProps.Description = formatDescription(jsonProps.Description, channel, name) | ||
|
|
||
| if len(jsonProps.Properties) > 0 { | ||
| jsonProps.Properties = opconTweaksMap(channel, jsonProps.Properties) | ||
| jsonProps.Properties, jsonProps.Required = opconTweaksMap(channel, jsonProps.Properties, jsonProps.Required) | ||
| } else if jsonProps.Items != nil && jsonProps.Items.Schema != nil { | ||
| jsonProps.Items.Schema = opconTweaks(channel, name, *jsonProps.Items.Schema) | ||
| jsonProps.Items.Schema, _ = opconTweaks(channel, name, *jsonProps.Items.Schema) | ||
| } | ||
|
|
||
| return &jsonProps | ||
| return &jsonProps, requiredStatus | ||
| } | ||
|
|
||
| func formatDescription(description string, channel string, name string) string { | ||
| startTag := "<opcon:experimental:description>" | ||
| endTag := "</opcon:experimental:description>" | ||
| if channel == StandardChannel && strings.Contains(description, startTag) { | ||
| regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*` | ||
| re := regexp.MustCompile(regexPattern) | ||
| match := re.FindStringSubmatch(description) | ||
| if len(match) != 2 { | ||
| log.Fatalf("Invalid <opcon:experimental:description> tag for %s", name) | ||
| tagset := []struct { | ||
| channel string | ||
| start string | ||
| end string | ||
| }{ | ||
| {channel: ExperimentalChannel, start: "<opcon:standard:description>", end: "</opcon:standard:description>"}, | ||
grokspawn marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| {channel: StandardChannel, start: "<opcon:experimental:description>", end: "</opcon:experimental:description>"}, | ||
| } | ||
| for _, ts := range tagset { | ||
| if channel == ts.channel && strings.Contains(description, ts.start) { | ||
| regexPattern := `\n*` + regexp.QuoteMeta(ts.start) + `(?s:(.*?))` + regexp.QuoteMeta(ts.end) + `\n*` | ||
| re := regexp.MustCompile(regexPattern) | ||
| match := re.FindStringSubmatch(description) | ||
| if len(match) != 2 { | ||
| log.Fatalf("Invalid <opcon:experimental:description> tag for %s", name) | ||
grokspawn marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| description = re.ReplaceAllString(description, "\n\n") | ||
| } else { | ||
| description = strings.ReplaceAll(description, ts.start, "") | ||
| description = strings.ReplaceAll(description, ts.end, "") | ||
| } | ||
| description = re.ReplaceAllString(description, "\n\n") | ||
| } else { | ||
| description = strings.ReplaceAll(description, startTag, "") | ||
| description = strings.ReplaceAll(description, endTag, "") | ||
| } | ||
|
|
||
| // Comments within "opcon:util:excludeFromCRD" tag are not included in the generated CRD and all trailing \n operators before | ||
| // and after the tags are removed and replaced with three \n operators. | ||
| startTag = "<opcon:util:excludeFromCRD>" | ||
| endTag = "</opcon:util:excludeFromCRD>" | ||
| startTag := "<opcon:util:excludeFromCRD>" | ||
| endTag := "</opcon:util:excludeFromCRD>" | ||
| if strings.Contains(description, startTag) { | ||
| regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*` | ||
| re := regexp.MustCompile(regexPattern) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.