From 480f791ca34d81f52aefe7ac161849ff1e52cbfd Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 08:27:20 -0400 Subject: [PATCH 01/14] Allow certain kube markers to be ignored --- integration_test.go | 23 ++++++++ main.go | 6 ++ openapiGenerator.go | 5 +- pkg/markers/constraints.go | 4 ++ pkg/markers/register.go | 16 +++++- pkg/markers/validation.go | 22 +++++++ testdata/golden/test8/openapiv3.yaml | 47 +++++++++++++++ testdata/test8/markers.proto | 85 ++++++++++++++++++++++++++++ 8 files changed, 204 insertions(+), 4 deletions(-) create mode 100644 testdata/golden/test8/openapiv3.yaml create mode 100644 testdata/test8/markers.proto diff --git a/integration_test.go b/integration_test.go index c6dccc0..5e16731 100644 --- a/integration_test.go +++ b/integration_test.go @@ -21,11 +21,14 @@ import ( "path/filepath" "strings" "testing" + "regexp" ) const goldenDir = "testdata/golden/" func TestOpenAPIGeneration(t *testing.T) { + regexp.MustCompile("(:?kubebuilder:altName)") + testcases := []struct { name string id string @@ -109,6 +112,26 @@ func TestOpenAPIGeneration(t *testing.T) { }, wantFiles: []string{"test7/openapiv3.yaml"}, }, + { + name: "Test no markers are ignored when ignored_kube_markers is zero length", + id: "test7", + perPackage: false, + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=true,ignored_kube_markers=", + inputFiles: map[string][]string{ + "test7": {"./testdata/test7/markers.proto"}, + }, + wantFiles: []string{"test7/openapiv3.yaml"}, + }, + { + name: "Test ignored_kube_markers option ignores specific markers", + id: "test8", + perPackage: false, + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=true,ignored_kube_markers=Required", + inputFiles: map[string][]string{ + "test8": {"./testdata/test8/markers.proto"}, + }, + wantFiles: []string{"test8/openapiv3.yaml"}, + }, } for _, tc := range testcases { diff --git a/main.go b/main.go index aefe52a..86382a4 100644 --- a/main.go +++ b/main.go @@ -56,6 +56,7 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes disableKubeMarkers := false var messagesWithEmptySchema []string + var ignoredKubeMarkers []string p := extractParams(request.GetParameter()) for k, v := range p { @@ -147,6 +148,10 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes default: return nil, fmt.Errorf("unknown value '%s' for disable_kube_markers", v) } + } else if k == "ignored_kube_markers" { + if len(v) > 0 { + ignoredKubeMarkers = strings.Split(v, "+") + } } else { return nil, fmt.Errorf("unknown argument '%s' specified", k) } @@ -184,6 +189,7 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes protoOneof, intNative, disableKubeMarkers, + ignoredKubeMarkers, ) return g.generateOutput(filesToGen) } diff --git a/openapiGenerator.go b/openapiGenerator.go index 3dca79c..0018d69 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -121,6 +121,8 @@ type openapiGenerator struct { // If set to true, kubebuilder markers and validations such as PreserveUnknownFields, MinItems, default, and all CEL rules will be omitted from the OpenAPI schema. // The Type and Required markers will be maintained. disableKubeMarkers bool + + ignoredKubeMarkers []string } type DescriptionConfiguration struct { @@ -143,8 +145,9 @@ func newOpenAPIGenerator( protoOneof bool, intNative bool, disableKubeMarkers bool, + ignoredKubeMarkers []string, ) *openapiGenerator { - mRegistry, err := markers.NewRegistry() + mRegistry, err := markers.NewRegistry(ignoredKubeMarkers) if err != nil { log.Panicf("error initializing marker registry: %v", err) } diff --git a/pkg/markers/constraints.go b/pkg/markers/constraints.go index b933ae8..5a605d0 100644 --- a/pkg/markers/constraints.go +++ b/pkg/markers/constraints.go @@ -311,6 +311,10 @@ func (m Example) ApplyToSchema(o *openapi3.Schema) { o.Example = m.Value } +type AltName string + +func (a AltName) ApplyToSchema(o *openapi3.Schema) {} + // Schemaless marks a field as being a schemaless object. // // Schemaless objects are not introspected, so you must provide diff --git a/pkg/markers/register.go b/pkg/markers/register.go index 46d4bbf..505e0a3 100644 --- a/pkg/markers/register.go +++ b/pkg/markers/register.go @@ -4,6 +4,9 @@ import ( "reflect" "sigs.k8s.io/controller-tools/pkg/markers" + "regexp" + "fmt" + "strings" ) const ( @@ -17,13 +20,20 @@ type definitionWithHelp struct { } type Registry struct { - mRegistry *markers.Registry + mRegistry *markers.Registry + ignoredKubeMarkersRegex *regexp.Regexp } -func NewRegistry() (*Registry, error) { +func NewRegistry(ignoredKubeMarkers []string) (*Registry, error) { + var ignoredKubeMarkersRegexp *regexp.Regexp + if len(ignoredKubeMarkers) > 0 { + toIgnore := strings.Join(ignoredKubeMarkers, "|") + ignoredKubeMarkersRegexp = regexp.MustCompile(fmt.Sprintf("(?:%s)", toIgnore)) + } mReg := &markers.Registry{} r := &Registry{ - mRegistry: mReg, + mRegistry: mReg, + ignoredKubeMarkersRegex: ignoredKubeMarkersRegexp, } return r, Register(mReg) } diff --git a/pkg/markers/validation.go b/pkg/markers/validation.go index 05aadc3..b272fca 100644 --- a/pkg/markers/validation.go +++ b/pkg/markers/validation.go @@ -141,7 +141,12 @@ func (r *Registry) ApplyRulesToSchema( o *openapi3.Schema, target markers.TargetType, ) error { + for _, rule := range rules { + if r.isIgnoredKubeMarker(rule) { + continue + } + defn := r.mRegistry.Lookup(rule, target) if defn == nil { return fmt.Errorf("no definition found for rule: %s", rule) @@ -164,6 +169,10 @@ func (r *Registry) GetSchemaType( target markers.TargetType, ) Type { for _, rule := range rules { + if r.isIgnoredKubeMarker(rule) { + continue + } + defn := r.mRegistry.Lookup(rule, target) if defn == nil { log.Panicf("no definition found for rule: %s", rule) @@ -183,6 +192,10 @@ func (r *Registry) IsRequired( rules []string, ) bool { for _, rule := range rules { + if r.isIgnoredKubeMarker(rule) { + continue + } + defn := r.mRegistry.Lookup(rule, markers.DescribesField) if defn == nil { log.Panicf("no definition found for rule: %s", rule) @@ -191,5 +204,14 @@ func (r *Registry) IsRequired( return true } } + return false } + +func (r *Registry) isIgnoredKubeMarker(rule string) bool { + if r.ignoredKubeMarkersRegex == nil { + return false + } + + return r.ignoredKubeMarkersRegex.MatchString(rule) +} diff --git a/testdata/golden/test8/openapiv3.yaml b/testdata/golden/test8/openapiv3.yaml new file mode 100644 index 0000000..96a3fa6 --- /dev/null +++ b/testdata/golden/test8/openapiv3.yaml @@ -0,0 +1,47 @@ +components: + schemas: + test7.Msg: + description: This is a top-level message. + properties: + a: + format: int32 + type: integer + blist: + items: + type: string + type: array + nested: + properties: + a: + type: string + b: + type: string + c: + type: string + d: + type: string + defaultValue: + type: string + embedded: + type: string + intOrString: + type: string + schemaless: + description: Schemaless field + type: string + type: object + object: + description: Should maintain valid Type marker and not enumerate subfields. + type: object + x-kubernetes-preserve-unknown-fields: true + recursive: + type: object + x-kubernetes-preserve-unknown-fields: true + val: + x-kubernetes-preserve-unknown-fields: true + type: object +info: + title: OpenAPI Spec for Solo APIs. + version: "" +openapi: 3.0.1 +paths: null diff --git a/testdata/test8/markers.proto b/testdata/test8/markers.proto new file mode 100644 index 0000000..fe271f0 --- /dev/null +++ b/testdata/test8/markers.proto @@ -0,0 +1,85 @@ +syntax = "proto3"; + +package test7; + +import "struct.proto"; + +// This is a top-level message. +// +// +kubebuilder:pruning:PreserveUnknownFields +message Msg { + // +kubebuilder:pruning:PreserveUnknownFields + Nested nested = 1; + + // +kubebuilder:validation:Maximum=100 + // +kubebuilder:validation:Minimum=5 + // +kubebuilder:validation:ExclusiveMaximum=true + // +kubebuilder:validation:ExclusiveMinimum=true + // +kubebuilder:validation:MultipleOf=2 + // +kubebuilder:validation:XValidation:rule="self != 27",message="must not equal 27" + int32 a = 2; + + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=5 + // +kubebuilder:validation:UniqueItems=true + repeated string blist = 3; + + // +kubebuilder:validation:Type=value + google.protobuf.Value val = 4; + + // Should maintain valid Type marker and not enumerate subfields. + // + // +kubebuilder:validation:Type=object + Nested2 object = 5; + + // +kubebuilder:validation:Type=object + Recursive recursive = 6; + + // This is a nested message. + // + // +kubebuilder:validation:MinProperties=1 + // +kubebuilder:validation:MaxProperties=2 + message Nested { + // +kubebuilder:validation:Pattern="^[a-zA-Z0-9_]*$" + // +kubebuilder:validation:Required + string a = 1; + + // +kubebuilder:validation:Enum=Allow;Forbid;Replace + // +kubebuilder:validation:Required + string b = 2; + + // +kubebuilder:validation:MaxLength=100 + // +kubebuilder:validation:MinLength=1 + string c = 3; + + // +kubebuilder:validation:Format=date-time + string d = 4; + + // +kubebuilder:validation:XIntOrString + string int_or_string = 5; + + // +kubebuilder:default=forty-two + // +kubebuilder:example=forty-two + string default_value = 6; + + // Schemaless field + // + // +kubebuilder:validation:Schemaless + string schemaless = 7; + + // +kubebuilder:validation:EmbeddedResource + // +kubebuilder:validation:Nullable + string embedded = 8; + } + + message Nested2 { + string a = 1; + string b = 2; + int32 c = 3; + } + + message Recursive { + Recursive r = 1; + } +} + From f923f6122e3ce3873fdb872c4c2bba1439f197a1 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 08:27:42 -0400 Subject: [PATCH 02/14] Revert "Allow certain kube markers to be ignored" This reverts commit 480f791ca34d81f52aefe7ac161849ff1e52cbfd. --- integration_test.go | 23 -------- main.go | 6 -- openapiGenerator.go | 5 +- pkg/markers/constraints.go | 4 -- pkg/markers/register.go | 16 +----- pkg/markers/validation.go | 22 ------- testdata/golden/test8/openapiv3.yaml | 47 --------------- testdata/test8/markers.proto | 85 ---------------------------- 8 files changed, 4 insertions(+), 204 deletions(-) delete mode 100644 testdata/golden/test8/openapiv3.yaml delete mode 100644 testdata/test8/markers.proto diff --git a/integration_test.go b/integration_test.go index 5e16731..c6dccc0 100644 --- a/integration_test.go +++ b/integration_test.go @@ -21,14 +21,11 @@ import ( "path/filepath" "strings" "testing" - "regexp" ) const goldenDir = "testdata/golden/" func TestOpenAPIGeneration(t *testing.T) { - regexp.MustCompile("(:?kubebuilder:altName)") - testcases := []struct { name string id string @@ -112,26 +109,6 @@ func TestOpenAPIGeneration(t *testing.T) { }, wantFiles: []string{"test7/openapiv3.yaml"}, }, - { - name: "Test no markers are ignored when ignored_kube_markers is zero length", - id: "test7", - perPackage: false, - genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=true,ignored_kube_markers=", - inputFiles: map[string][]string{ - "test7": {"./testdata/test7/markers.proto"}, - }, - wantFiles: []string{"test7/openapiv3.yaml"}, - }, - { - name: "Test ignored_kube_markers option ignores specific markers", - id: "test8", - perPackage: false, - genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=true,ignored_kube_markers=Required", - inputFiles: map[string][]string{ - "test8": {"./testdata/test8/markers.proto"}, - }, - wantFiles: []string{"test8/openapiv3.yaml"}, - }, } for _, tc := range testcases { diff --git a/main.go b/main.go index 86382a4..aefe52a 100644 --- a/main.go +++ b/main.go @@ -56,7 +56,6 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes disableKubeMarkers := false var messagesWithEmptySchema []string - var ignoredKubeMarkers []string p := extractParams(request.GetParameter()) for k, v := range p { @@ -148,10 +147,6 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes default: return nil, fmt.Errorf("unknown value '%s' for disable_kube_markers", v) } - } else if k == "ignored_kube_markers" { - if len(v) > 0 { - ignoredKubeMarkers = strings.Split(v, "+") - } } else { return nil, fmt.Errorf("unknown argument '%s' specified", k) } @@ -189,7 +184,6 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes protoOneof, intNative, disableKubeMarkers, - ignoredKubeMarkers, ) return g.generateOutput(filesToGen) } diff --git a/openapiGenerator.go b/openapiGenerator.go index 0018d69..3dca79c 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -121,8 +121,6 @@ type openapiGenerator struct { // If set to true, kubebuilder markers and validations such as PreserveUnknownFields, MinItems, default, and all CEL rules will be omitted from the OpenAPI schema. // The Type and Required markers will be maintained. disableKubeMarkers bool - - ignoredKubeMarkers []string } type DescriptionConfiguration struct { @@ -145,9 +143,8 @@ func newOpenAPIGenerator( protoOneof bool, intNative bool, disableKubeMarkers bool, - ignoredKubeMarkers []string, ) *openapiGenerator { - mRegistry, err := markers.NewRegistry(ignoredKubeMarkers) + mRegistry, err := markers.NewRegistry() if err != nil { log.Panicf("error initializing marker registry: %v", err) } diff --git a/pkg/markers/constraints.go b/pkg/markers/constraints.go index 5a605d0..b933ae8 100644 --- a/pkg/markers/constraints.go +++ b/pkg/markers/constraints.go @@ -311,10 +311,6 @@ func (m Example) ApplyToSchema(o *openapi3.Schema) { o.Example = m.Value } -type AltName string - -func (a AltName) ApplyToSchema(o *openapi3.Schema) {} - // Schemaless marks a field as being a schemaless object. // // Schemaless objects are not introspected, so you must provide diff --git a/pkg/markers/register.go b/pkg/markers/register.go index 505e0a3..46d4bbf 100644 --- a/pkg/markers/register.go +++ b/pkg/markers/register.go @@ -4,9 +4,6 @@ import ( "reflect" "sigs.k8s.io/controller-tools/pkg/markers" - "regexp" - "fmt" - "strings" ) const ( @@ -20,20 +17,13 @@ type definitionWithHelp struct { } type Registry struct { - mRegistry *markers.Registry - ignoredKubeMarkersRegex *regexp.Regexp + mRegistry *markers.Registry } -func NewRegistry(ignoredKubeMarkers []string) (*Registry, error) { - var ignoredKubeMarkersRegexp *regexp.Regexp - if len(ignoredKubeMarkers) > 0 { - toIgnore := strings.Join(ignoredKubeMarkers, "|") - ignoredKubeMarkersRegexp = regexp.MustCompile(fmt.Sprintf("(?:%s)", toIgnore)) - } +func NewRegistry() (*Registry, error) { mReg := &markers.Registry{} r := &Registry{ - mRegistry: mReg, - ignoredKubeMarkersRegex: ignoredKubeMarkersRegexp, + mRegistry: mReg, } return r, Register(mReg) } diff --git a/pkg/markers/validation.go b/pkg/markers/validation.go index b272fca..05aadc3 100644 --- a/pkg/markers/validation.go +++ b/pkg/markers/validation.go @@ -141,12 +141,7 @@ func (r *Registry) ApplyRulesToSchema( o *openapi3.Schema, target markers.TargetType, ) error { - for _, rule := range rules { - if r.isIgnoredKubeMarker(rule) { - continue - } - defn := r.mRegistry.Lookup(rule, target) if defn == nil { return fmt.Errorf("no definition found for rule: %s", rule) @@ -169,10 +164,6 @@ func (r *Registry) GetSchemaType( target markers.TargetType, ) Type { for _, rule := range rules { - if r.isIgnoredKubeMarker(rule) { - continue - } - defn := r.mRegistry.Lookup(rule, target) if defn == nil { log.Panicf("no definition found for rule: %s", rule) @@ -192,10 +183,6 @@ func (r *Registry) IsRequired( rules []string, ) bool { for _, rule := range rules { - if r.isIgnoredKubeMarker(rule) { - continue - } - defn := r.mRegistry.Lookup(rule, markers.DescribesField) if defn == nil { log.Panicf("no definition found for rule: %s", rule) @@ -204,14 +191,5 @@ func (r *Registry) IsRequired( return true } } - return false } - -func (r *Registry) isIgnoredKubeMarker(rule string) bool { - if r.ignoredKubeMarkersRegex == nil { - return false - } - - return r.ignoredKubeMarkersRegex.MatchString(rule) -} diff --git a/testdata/golden/test8/openapiv3.yaml b/testdata/golden/test8/openapiv3.yaml deleted file mode 100644 index 96a3fa6..0000000 --- a/testdata/golden/test8/openapiv3.yaml +++ /dev/null @@ -1,47 +0,0 @@ -components: - schemas: - test7.Msg: - description: This is a top-level message. - properties: - a: - format: int32 - type: integer - blist: - items: - type: string - type: array - nested: - properties: - a: - type: string - b: - type: string - c: - type: string - d: - type: string - defaultValue: - type: string - embedded: - type: string - intOrString: - type: string - schemaless: - description: Schemaless field - type: string - type: object - object: - description: Should maintain valid Type marker and not enumerate subfields. - type: object - x-kubernetes-preserve-unknown-fields: true - recursive: - type: object - x-kubernetes-preserve-unknown-fields: true - val: - x-kubernetes-preserve-unknown-fields: true - type: object -info: - title: OpenAPI Spec for Solo APIs. - version: "" -openapi: 3.0.1 -paths: null diff --git a/testdata/test8/markers.proto b/testdata/test8/markers.proto deleted file mode 100644 index fe271f0..0000000 --- a/testdata/test8/markers.proto +++ /dev/null @@ -1,85 +0,0 @@ -syntax = "proto3"; - -package test7; - -import "struct.proto"; - -// This is a top-level message. -// -// +kubebuilder:pruning:PreserveUnknownFields -message Msg { - // +kubebuilder:pruning:PreserveUnknownFields - Nested nested = 1; - - // +kubebuilder:validation:Maximum=100 - // +kubebuilder:validation:Minimum=5 - // +kubebuilder:validation:ExclusiveMaximum=true - // +kubebuilder:validation:ExclusiveMinimum=true - // +kubebuilder:validation:MultipleOf=2 - // +kubebuilder:validation:XValidation:rule="self != 27",message="must not equal 27" - int32 a = 2; - - // +kubebuilder:validation:MinItems=1 - // +kubebuilder:validation:MaxItems=5 - // +kubebuilder:validation:UniqueItems=true - repeated string blist = 3; - - // +kubebuilder:validation:Type=value - google.protobuf.Value val = 4; - - // Should maintain valid Type marker and not enumerate subfields. - // - // +kubebuilder:validation:Type=object - Nested2 object = 5; - - // +kubebuilder:validation:Type=object - Recursive recursive = 6; - - // This is a nested message. - // - // +kubebuilder:validation:MinProperties=1 - // +kubebuilder:validation:MaxProperties=2 - message Nested { - // +kubebuilder:validation:Pattern="^[a-zA-Z0-9_]*$" - // +kubebuilder:validation:Required - string a = 1; - - // +kubebuilder:validation:Enum=Allow;Forbid;Replace - // +kubebuilder:validation:Required - string b = 2; - - // +kubebuilder:validation:MaxLength=100 - // +kubebuilder:validation:MinLength=1 - string c = 3; - - // +kubebuilder:validation:Format=date-time - string d = 4; - - // +kubebuilder:validation:XIntOrString - string int_or_string = 5; - - // +kubebuilder:default=forty-two - // +kubebuilder:example=forty-two - string default_value = 6; - - // Schemaless field - // - // +kubebuilder:validation:Schemaless - string schemaless = 7; - - // +kubebuilder:validation:EmbeddedResource - // +kubebuilder:validation:Nullable - string embedded = 8; - } - - message Nested2 { - string a = 1; - string b = 2; - int32 c = 3; - } - - message Recursive { - Recursive r = 1; - } -} - From 5fb14fa7f501d23fad3c523e13e9f387beeae354 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 08:27:20 -0400 Subject: [PATCH 03/14] Allow certain kube markers to be ignored --- integration_test.go | 23 ++++++++ main.go | 6 ++ openapiGenerator.go | 5 +- pkg/markers/constraints.go | 4 ++ pkg/markers/register.go | 16 +++++- pkg/markers/validation.go | 22 +++++++ testdata/golden/test8/openapiv3.yaml | 47 +++++++++++++++ testdata/test8/markers.proto | 85 ++++++++++++++++++++++++++++ 8 files changed, 204 insertions(+), 4 deletions(-) create mode 100644 testdata/golden/test8/openapiv3.yaml create mode 100644 testdata/test8/markers.proto diff --git a/integration_test.go b/integration_test.go index c6dccc0..5e16731 100644 --- a/integration_test.go +++ b/integration_test.go @@ -21,11 +21,14 @@ import ( "path/filepath" "strings" "testing" + "regexp" ) const goldenDir = "testdata/golden/" func TestOpenAPIGeneration(t *testing.T) { + regexp.MustCompile("(:?kubebuilder:altName)") + testcases := []struct { name string id string @@ -109,6 +112,26 @@ func TestOpenAPIGeneration(t *testing.T) { }, wantFiles: []string{"test7/openapiv3.yaml"}, }, + { + name: "Test no markers are ignored when ignored_kube_markers is zero length", + id: "test7", + perPackage: false, + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=true,ignored_kube_markers=", + inputFiles: map[string][]string{ + "test7": {"./testdata/test7/markers.proto"}, + }, + wantFiles: []string{"test7/openapiv3.yaml"}, + }, + { + name: "Test ignored_kube_markers option ignores specific markers", + id: "test8", + perPackage: false, + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=true,ignored_kube_markers=Required", + inputFiles: map[string][]string{ + "test8": {"./testdata/test8/markers.proto"}, + }, + wantFiles: []string{"test8/openapiv3.yaml"}, + }, } for _, tc := range testcases { diff --git a/main.go b/main.go index aefe52a..86382a4 100644 --- a/main.go +++ b/main.go @@ -56,6 +56,7 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes disableKubeMarkers := false var messagesWithEmptySchema []string + var ignoredKubeMarkers []string p := extractParams(request.GetParameter()) for k, v := range p { @@ -147,6 +148,10 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes default: return nil, fmt.Errorf("unknown value '%s' for disable_kube_markers", v) } + } else if k == "ignored_kube_markers" { + if len(v) > 0 { + ignoredKubeMarkers = strings.Split(v, "+") + } } else { return nil, fmt.Errorf("unknown argument '%s' specified", k) } @@ -184,6 +189,7 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes protoOneof, intNative, disableKubeMarkers, + ignoredKubeMarkers, ) return g.generateOutput(filesToGen) } diff --git a/openapiGenerator.go b/openapiGenerator.go index 3dca79c..0018d69 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -121,6 +121,8 @@ type openapiGenerator struct { // If set to true, kubebuilder markers and validations such as PreserveUnknownFields, MinItems, default, and all CEL rules will be omitted from the OpenAPI schema. // The Type and Required markers will be maintained. disableKubeMarkers bool + + ignoredKubeMarkers []string } type DescriptionConfiguration struct { @@ -143,8 +145,9 @@ func newOpenAPIGenerator( protoOneof bool, intNative bool, disableKubeMarkers bool, + ignoredKubeMarkers []string, ) *openapiGenerator { - mRegistry, err := markers.NewRegistry() + mRegistry, err := markers.NewRegistry(ignoredKubeMarkers) if err != nil { log.Panicf("error initializing marker registry: %v", err) } diff --git a/pkg/markers/constraints.go b/pkg/markers/constraints.go index b933ae8..5a605d0 100644 --- a/pkg/markers/constraints.go +++ b/pkg/markers/constraints.go @@ -311,6 +311,10 @@ func (m Example) ApplyToSchema(o *openapi3.Schema) { o.Example = m.Value } +type AltName string + +func (a AltName) ApplyToSchema(o *openapi3.Schema) {} + // Schemaless marks a field as being a schemaless object. // // Schemaless objects are not introspected, so you must provide diff --git a/pkg/markers/register.go b/pkg/markers/register.go index 46d4bbf..505e0a3 100644 --- a/pkg/markers/register.go +++ b/pkg/markers/register.go @@ -4,6 +4,9 @@ import ( "reflect" "sigs.k8s.io/controller-tools/pkg/markers" + "regexp" + "fmt" + "strings" ) const ( @@ -17,13 +20,20 @@ type definitionWithHelp struct { } type Registry struct { - mRegistry *markers.Registry + mRegistry *markers.Registry + ignoredKubeMarkersRegex *regexp.Regexp } -func NewRegistry() (*Registry, error) { +func NewRegistry(ignoredKubeMarkers []string) (*Registry, error) { + var ignoredKubeMarkersRegexp *regexp.Regexp + if len(ignoredKubeMarkers) > 0 { + toIgnore := strings.Join(ignoredKubeMarkers, "|") + ignoredKubeMarkersRegexp = regexp.MustCompile(fmt.Sprintf("(?:%s)", toIgnore)) + } mReg := &markers.Registry{} r := &Registry{ - mRegistry: mReg, + mRegistry: mReg, + ignoredKubeMarkersRegex: ignoredKubeMarkersRegexp, } return r, Register(mReg) } diff --git a/pkg/markers/validation.go b/pkg/markers/validation.go index 05aadc3..b272fca 100644 --- a/pkg/markers/validation.go +++ b/pkg/markers/validation.go @@ -141,7 +141,12 @@ func (r *Registry) ApplyRulesToSchema( o *openapi3.Schema, target markers.TargetType, ) error { + for _, rule := range rules { + if r.isIgnoredKubeMarker(rule) { + continue + } + defn := r.mRegistry.Lookup(rule, target) if defn == nil { return fmt.Errorf("no definition found for rule: %s", rule) @@ -164,6 +169,10 @@ func (r *Registry) GetSchemaType( target markers.TargetType, ) Type { for _, rule := range rules { + if r.isIgnoredKubeMarker(rule) { + continue + } + defn := r.mRegistry.Lookup(rule, target) if defn == nil { log.Panicf("no definition found for rule: %s", rule) @@ -183,6 +192,10 @@ func (r *Registry) IsRequired( rules []string, ) bool { for _, rule := range rules { + if r.isIgnoredKubeMarker(rule) { + continue + } + defn := r.mRegistry.Lookup(rule, markers.DescribesField) if defn == nil { log.Panicf("no definition found for rule: %s", rule) @@ -191,5 +204,14 @@ func (r *Registry) IsRequired( return true } } + return false } + +func (r *Registry) isIgnoredKubeMarker(rule string) bool { + if r.ignoredKubeMarkersRegex == nil { + return false + } + + return r.ignoredKubeMarkersRegex.MatchString(rule) +} diff --git a/testdata/golden/test8/openapiv3.yaml b/testdata/golden/test8/openapiv3.yaml new file mode 100644 index 0000000..96a3fa6 --- /dev/null +++ b/testdata/golden/test8/openapiv3.yaml @@ -0,0 +1,47 @@ +components: + schemas: + test7.Msg: + description: This is a top-level message. + properties: + a: + format: int32 + type: integer + blist: + items: + type: string + type: array + nested: + properties: + a: + type: string + b: + type: string + c: + type: string + d: + type: string + defaultValue: + type: string + embedded: + type: string + intOrString: + type: string + schemaless: + description: Schemaless field + type: string + type: object + object: + description: Should maintain valid Type marker and not enumerate subfields. + type: object + x-kubernetes-preserve-unknown-fields: true + recursive: + type: object + x-kubernetes-preserve-unknown-fields: true + val: + x-kubernetes-preserve-unknown-fields: true + type: object +info: + title: OpenAPI Spec for Solo APIs. + version: "" +openapi: 3.0.1 +paths: null diff --git a/testdata/test8/markers.proto b/testdata/test8/markers.proto new file mode 100644 index 0000000..fe271f0 --- /dev/null +++ b/testdata/test8/markers.proto @@ -0,0 +1,85 @@ +syntax = "proto3"; + +package test7; + +import "struct.proto"; + +// This is a top-level message. +// +// +kubebuilder:pruning:PreserveUnknownFields +message Msg { + // +kubebuilder:pruning:PreserveUnknownFields + Nested nested = 1; + + // +kubebuilder:validation:Maximum=100 + // +kubebuilder:validation:Minimum=5 + // +kubebuilder:validation:ExclusiveMaximum=true + // +kubebuilder:validation:ExclusiveMinimum=true + // +kubebuilder:validation:MultipleOf=2 + // +kubebuilder:validation:XValidation:rule="self != 27",message="must not equal 27" + int32 a = 2; + + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=5 + // +kubebuilder:validation:UniqueItems=true + repeated string blist = 3; + + // +kubebuilder:validation:Type=value + google.protobuf.Value val = 4; + + // Should maintain valid Type marker and not enumerate subfields. + // + // +kubebuilder:validation:Type=object + Nested2 object = 5; + + // +kubebuilder:validation:Type=object + Recursive recursive = 6; + + // This is a nested message. + // + // +kubebuilder:validation:MinProperties=1 + // +kubebuilder:validation:MaxProperties=2 + message Nested { + // +kubebuilder:validation:Pattern="^[a-zA-Z0-9_]*$" + // +kubebuilder:validation:Required + string a = 1; + + // +kubebuilder:validation:Enum=Allow;Forbid;Replace + // +kubebuilder:validation:Required + string b = 2; + + // +kubebuilder:validation:MaxLength=100 + // +kubebuilder:validation:MinLength=1 + string c = 3; + + // +kubebuilder:validation:Format=date-time + string d = 4; + + // +kubebuilder:validation:XIntOrString + string int_or_string = 5; + + // +kubebuilder:default=forty-two + // +kubebuilder:example=forty-two + string default_value = 6; + + // Schemaless field + // + // +kubebuilder:validation:Schemaless + string schemaless = 7; + + // +kubebuilder:validation:EmbeddedResource + // +kubebuilder:validation:Nullable + string embedded = 8; + } + + message Nested2 { + string a = 1; + string b = 2; + int32 c = 3; + } + + message Recursive { + Recursive r = 1; + } +} + From d59ef63489878bcb27626d3ffc737f4c2dacbb86 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 08:33:56 -0400 Subject: [PATCH 04/14] Add required docs --- README.md | 2 ++ changelog/v0.2.5/define_ignored_kube_markers.yaml | 7 +++++++ openapiGenerator.go | 1 + 3 files changed, 10 insertions(+) create mode 100644 changelog/v0.2.5/define_ignored_kube_markers.yaml diff --git a/README.md b/README.md index 9c4f004..5e6d8de 100644 --- a/README.md +++ b/README.md @@ -76,3 +76,5 @@ Other supported options are: * when set to `true`, the native openapi schemas will be used for Integer types instead of Solo wrappers that add Kubernetes extension headers to the schema to treat int as strings. * `disable_kube_markers` * when set to `true`, kubebuilder markers and validations such as PreserveUnknownFields, MinItems, default, and all CEL rules will be omitted from the OpenAPI schema. The Type and Required markers will be maintained. +* `ignored_kube_markers` + * when set, ignores the contained kubebuilder markers and validations, and prevents them from being applied to the OpenAPI schema. diff --git a/changelog/v0.2.5/define_ignored_kube_markers.yaml b/changelog/v0.2.5/define_ignored_kube_markers.yaml new file mode 100644 index 0000000..b7f6621 --- /dev/null +++ b/changelog/v0.2.5/define_ignored_kube_markers.yaml @@ -0,0 +1,7 @@ +changelog: + - type: NEW_FEATURE + issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/18119 + resolvesIssue: false + description: | + Allows the user to define one or more kube markers to ignore. This is useful when using protos that contain + unsupported kubebuilder decorators. \ No newline at end of file diff --git a/openapiGenerator.go b/openapiGenerator.go index 0018d69..0a17aa1 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -122,6 +122,7 @@ type openapiGenerator struct { // The Type and Required markers will be maintained. disableKubeMarkers bool + // If provided, ignores the contained kubebuilder markers and validations, and prevents them from being applied to the OpenAPI schema. ignoredKubeMarkers []string } From 82f42ed8ef949399bd812e8bf60ea86d2cf53f80 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 08:35:03 -0400 Subject: [PATCH 05/14] Untouch --- pkg/markers/constraints.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/markers/constraints.go b/pkg/markers/constraints.go index 5a605d0..b933ae8 100644 --- a/pkg/markers/constraints.go +++ b/pkg/markers/constraints.go @@ -311,10 +311,6 @@ func (m Example) ApplyToSchema(o *openapi3.Schema) { o.Example = m.Value } -type AltName string - -func (a AltName) ApplyToSchema(o *openapi3.Schema) {} - // Schemaless marks a field as being a schemaless object. // // Schemaless objects are not introspected, so you must provide From b3ea9cc5499eb656b39faf44ce8a4bcdebee5472 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 09:08:20 -0400 Subject: [PATCH 06/14] Rename --- ..._ignored_kube_markers.yaml => allow_ignored_kube_markers.yaml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/v0.2.5/{define_ignored_kube_markers.yaml => allow_ignored_kube_markers.yaml} (100%) diff --git a/changelog/v0.2.5/define_ignored_kube_markers.yaml b/changelog/v0.2.5/allow_ignored_kube_markers.yaml similarity index 100% rename from changelog/v0.2.5/define_ignored_kube_markers.yaml rename to changelog/v0.2.5/allow_ignored_kube_markers.yaml From 0b408ca0edf33c83c1c9904c9e4cd8762b4c52cc Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 09:11:02 -0400 Subject: [PATCH 07/14] Clarify doc comments --- openapiGenerator.go | 1 + 1 file changed, 1 insertion(+) diff --git a/openapiGenerator.go b/openapiGenerator.go index 0a17aa1..4fc89ba 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -123,6 +123,7 @@ type openapiGenerator struct { disableKubeMarkers bool // If provided, ignores the contained kubebuilder markers and validations, and prevents them from being applied to the OpenAPI schema. + // If none are provided, no markers will be ignored. ignoredKubeMarkers []string } From 92a5f305b07df0ab07b22348beef98273110232d Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 10:42:37 -0400 Subject: [PATCH 08/14] Prevent rules from being added to the registry if they're ignored --- openapiGenerator.go | 8 ++++++-- pkg/markers/validation.go | 22 ---------------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/openapiGenerator.go b/openapiGenerator.go index 4fc89ba..96b056f 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -33,6 +33,7 @@ import ( "github.com/solo-io/protoc-gen-openapi/pkg/markers" "github.com/solo-io/protoc-gen-openapi/pkg/protomodel" + "regexp" ) var descriptionExclusionMarkers = []string{"$hide_from_docs", "$hide", "@exclude"} @@ -153,7 +154,6 @@ func newOpenAPIGenerator( if err != nil { log.Panicf("error initializing marker registry: %v", err) } - return &openapiGenerator{ model: model, perFile: perFile, @@ -668,6 +668,9 @@ func (g *openapiGenerator) parseComments(desc protomodel.CoreDesc) (comments str c := strings.TrimSpace(desc.Location().GetLeadingComments()) blocks := strings.Split(c, "\n\n") + toIgnore := strings.Join(g.ignoredKubeMarkers, "|") + ignoredKubeMarkersRegexp := regexp.MustCompile(fmt.Sprintf("(?:%s)", toIgnore)) + var sb strings.Builder for i, block := range blocks { if shouldNotRenderDesc(strings.TrimSpace(block)) { @@ -686,7 +689,8 @@ func (g *openapiGenerator) parseComments(desc protomodel.CoreDesc) (comments str if shouldNotRenderDesc(l) { continue } - if strings.HasPrefix(l, markers.Kubebuilder) { + // If this is a kube builder marker and not an ignored marker, add it to the list of validation rules + if strings.HasPrefix(l, markers.Kubebuilder) && !ignoredKubeMarkersRegexp.MatchString(l) { validationRules = append(validationRules, l) continue } diff --git a/pkg/markers/validation.go b/pkg/markers/validation.go index b272fca..05aadc3 100644 --- a/pkg/markers/validation.go +++ b/pkg/markers/validation.go @@ -141,12 +141,7 @@ func (r *Registry) ApplyRulesToSchema( o *openapi3.Schema, target markers.TargetType, ) error { - for _, rule := range rules { - if r.isIgnoredKubeMarker(rule) { - continue - } - defn := r.mRegistry.Lookup(rule, target) if defn == nil { return fmt.Errorf("no definition found for rule: %s", rule) @@ -169,10 +164,6 @@ func (r *Registry) GetSchemaType( target markers.TargetType, ) Type { for _, rule := range rules { - if r.isIgnoredKubeMarker(rule) { - continue - } - defn := r.mRegistry.Lookup(rule, target) if defn == nil { log.Panicf("no definition found for rule: %s", rule) @@ -192,10 +183,6 @@ func (r *Registry) IsRequired( rules []string, ) bool { for _, rule := range rules { - if r.isIgnoredKubeMarker(rule) { - continue - } - defn := r.mRegistry.Lookup(rule, markers.DescribesField) if defn == nil { log.Panicf("no definition found for rule: %s", rule) @@ -204,14 +191,5 @@ func (r *Registry) IsRequired( return true } } - return false } - -func (r *Registry) isIgnoredKubeMarker(rule string) bool { - if r.ignoredKubeMarkersRegex == nil { - return false - } - - return r.ignoredKubeMarkersRegex.MatchString(rule) -} From 12a09a179592066f96b9d4a0f7e669ae5db386f1 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 11:00:27 -0400 Subject: [PATCH 09/14] Prevent marker from being rendered in description of openapi output as well --- openapiGenerator.go | 28 +++++++++++++++++++++++----- pkg/markers/register.go | 16 +++------------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/openapiGenerator.go b/openapiGenerator.go index 96b056f..518c755 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -150,7 +150,7 @@ func newOpenAPIGenerator( disableKubeMarkers bool, ignoredKubeMarkers []string, ) *openapiGenerator { - mRegistry, err := markers.NewRegistry(ignoredKubeMarkers) + mRegistry, err := markers.NewRegistry() if err != nil { log.Panicf("error initializing marker registry: %v", err) } @@ -167,6 +167,7 @@ func newOpenAPIGenerator( intNative: intNative, markerRegistry: mRegistry, disableKubeMarkers: disableKubeMarkers, + ignoredKubeMarkers: ignoredKubeMarkers, } } @@ -668,8 +669,12 @@ func (g *openapiGenerator) parseComments(desc protomodel.CoreDesc) (comments str c := strings.TrimSpace(desc.Location().GetLeadingComments()) blocks := strings.Split(c, "\n\n") - toIgnore := strings.Join(g.ignoredKubeMarkers, "|") - ignoredKubeMarkersRegexp := regexp.MustCompile(fmt.Sprintf("(?:%s)", toIgnore)) + var ignoredKubeMarkersRegexp *regexp.Regexp + if len(g.ignoredKubeMarkers) > 0 { + ignoredKubeMarkersRegexp = regexp.MustCompile( + fmt.Sprintf("(?:%s)", strings.Join(g.ignoredKubeMarkers, "|")), + ) + } var sb strings.Builder for i, block := range blocks { @@ -689,8 +694,13 @@ func (g *openapiGenerator) parseComments(desc protomodel.CoreDesc) (comments str if shouldNotRenderDesc(l) { continue } - // If this is a kube builder marker and not an ignored marker, add it to the list of validation rules - if strings.HasPrefix(l, markers.Kubebuilder) && !ignoredKubeMarkersRegexp.MatchString(l) { + + // If this is an ignored kube marker, we'll skip this line, preventing validation and rendering marker in output + if strings.HasPrefix(l, markers.Kubebuilder) && isIgnoredKubeMarker(ignoredKubeMarkersRegexp, l) { + continue + } + + if strings.HasPrefix(l, markers.Kubebuilder) { validationRules = append(validationRules, l) continue } @@ -822,3 +832,11 @@ func (g *openapiGenerator) relativeName(desc protomodel.CoreDesc) string { return desc.PackageDesc().Name + "." + typeName } + +func isIgnoredKubeMarker(regexp *regexp.Regexp, l string) bool { + if regexp == nil { + return false + } + + return regexp.MatchString(l) +} diff --git a/pkg/markers/register.go b/pkg/markers/register.go index 505e0a3..46d4bbf 100644 --- a/pkg/markers/register.go +++ b/pkg/markers/register.go @@ -4,9 +4,6 @@ import ( "reflect" "sigs.k8s.io/controller-tools/pkg/markers" - "regexp" - "fmt" - "strings" ) const ( @@ -20,20 +17,13 @@ type definitionWithHelp struct { } type Registry struct { - mRegistry *markers.Registry - ignoredKubeMarkersRegex *regexp.Regexp + mRegistry *markers.Registry } -func NewRegistry(ignoredKubeMarkers []string) (*Registry, error) { - var ignoredKubeMarkersRegexp *regexp.Regexp - if len(ignoredKubeMarkers) > 0 { - toIgnore := strings.Join(ignoredKubeMarkers, "|") - ignoredKubeMarkersRegexp = regexp.MustCompile(fmt.Sprintf("(?:%s)", toIgnore)) - } +func NewRegistry() (*Registry, error) { mReg := &markers.Registry{} r := &Registry{ - mRegistry: mReg, - ignoredKubeMarkersRegex: ignoredKubeMarkersRegexp, + mRegistry: mReg, } return r, Register(mReg) } From bf468e92711068d552893d427ed4c4df9023af02 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 12:18:41 -0400 Subject: [PATCH 10/14] Negative testing for test cases (no values supplied) and positive tests cases (one or more) --- integration_test.go | 28 ++++++--- openapiGenerator.go | 9 ++- testdata/golden/test10/openapiv3.yaml | 73 +++++++++++++++++++++++ testdata/golden/test8/openapiv3.yaml | 34 ++++++++++- testdata/golden/test9/openapiv3.yaml | 74 +++++++++++++++++++++++ testdata/test10/markers.proto | 85 +++++++++++++++++++++++++++ testdata/test8/markers.proto | 2 +- testdata/test9/markers.proto | 85 +++++++++++++++++++++++++++ 8 files changed, 373 insertions(+), 17 deletions(-) create mode 100644 testdata/golden/test10/openapiv3.yaml create mode 100644 testdata/golden/test9/openapiv3.yaml create mode 100644 testdata/test10/markers.proto create mode 100644 testdata/test9/markers.proto diff --git a/integration_test.go b/integration_test.go index 5e16731..519c06e 100644 --- a/integration_test.go +++ b/integration_test.go @@ -114,23 +114,33 @@ func TestOpenAPIGeneration(t *testing.T) { }, { name: "Test no markers are ignored when ignored_kube_markers is zero length", - id: "test7", + id: "test8", perPackage: false, - genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=true,ignored_kube_markers=", + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_markers=", inputFiles: map[string][]string{ - "test7": {"./testdata/test7/markers.proto"}, + "test8": {"./testdata/test8/markers.proto"}, }, - wantFiles: []string{"test7/openapiv3.yaml"}, + wantFiles: []string{"test8/openapiv3.yaml"}, }, { - name: "Test ignored_kube_markers option ignores specific markers", - id: "test8", + name: "Test ignored_kube_markers option ignores a single marker", + id: "test9", perPackage: false, - genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=true,ignored_kube_markers=Required", + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_markers=Required", inputFiles: map[string][]string{ - "test8": {"./testdata/test8/markers.proto"}, + "test9": {"./testdata/test9/markers.proto"}, }, - wantFiles: []string{"test8/openapiv3.yaml"}, + wantFiles: []string{"test9/openapiv3.yaml"}, + }, + { + name: "Test ignored_kube_markers option ignores multiple markers", + id: "test10", + perPackage: false, + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_markers=Required+example", + inputFiles: map[string][]string{ + "test10": {"./testdata/test10/markers.proto"}, + }, + wantFiles: []string{"test10/openapiv3.yaml"}, }, } diff --git a/openapiGenerator.go b/openapiGenerator.go index 518c755..8a971cd 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -695,12 +695,11 @@ func (g *openapiGenerator) parseComments(desc protomodel.CoreDesc) (comments str continue } - // If this is an ignored kube marker, we'll skip this line, preventing validation and rendering marker in output - if strings.HasPrefix(l, markers.Kubebuilder) && isIgnoredKubeMarker(ignoredKubeMarkersRegexp, l) { - continue - } - if strings.HasPrefix(l, markers.Kubebuilder) { + if isIgnoredKubeMarker(ignoredKubeMarkersRegexp, l) { + continue + } + validationRules = append(validationRules, l) continue } diff --git a/testdata/golden/test10/openapiv3.yaml b/testdata/golden/test10/openapiv3.yaml new file mode 100644 index 0000000..cc6146a --- /dev/null +++ b/testdata/golden/test10/openapiv3.yaml @@ -0,0 +1,73 @@ +components: + schemas: + test10.Msg: + description: This is a top-level message. + properties: + a: + exclusiveMaximum: true + exclusiveMinimum: true + format: int32 + maximum: 100 + minimum: 5 + multipleOf: 2 + type: integer + x-kubernetes-validations: + - message: must not equal 27 + rule: self != 27 + blist: + items: + type: string + maxItems: 5 + minItems: 1 + type: array + uniqueItems: true + nested: + maxProperties: 2 + minProperties: 1 + properties: + a: + pattern: ^[a-zA-Z0-9_]*$ + type: string + b: + enum: + - Allow + - Forbid + - Replace + type: string + c: + maxLength: 100 + minLength: 1 + type: string + d: + format: date-time + type: string + defaultValue: + default: forty-two + type: string + embedded: + nullable: true + type: string + x-kubernetes-embedded-resource: true + intOrString: + type: string + x-kubernetes-int-or-string: true + schemaless: + description: Schemaless field + type: object + x-kubernetes-preserve-unknown-fields: true + object: + description: Should maintain valid Type marker and not enumerate subfields. + type: object + x-kubernetes-preserve-unknown-fields: true + recursive: + type: object + x-kubernetes-preserve-unknown-fields: true + val: + x-kubernetes-preserve-unknown-fields: true + type: object + x-kubernetes-preserve-unknown-fields: true +info: + title: OpenAPI Spec for Solo APIs. + version: "" +openapi: 3.0.1 +paths: null diff --git a/testdata/golden/test8/openapiv3.yaml b/testdata/golden/test8/openapiv3.yaml index 96a3fa6..a072b99 100644 --- a/testdata/golden/test8/openapiv3.yaml +++ b/testdata/golden/test8/openapiv3.yaml @@ -1,35 +1,64 @@ components: schemas: - test7.Msg: + test8.Msg: description: This is a top-level message. properties: a: + exclusiveMaximum: true + exclusiveMinimum: true format: int32 + maximum: 100 + minimum: 5 + multipleOf: 2 type: integer + x-kubernetes-validations: + - message: must not equal 27 + rule: self != 27 blist: items: type: string + maxItems: 5 + minItems: 1 type: array + uniqueItems: true nested: + maxProperties: 2 + minProperties: 1 properties: a: + pattern: ^[a-zA-Z0-9_]*$ type: string b: + enum: + - Allow + - Forbid + - Replace type: string c: + maxLength: 100 + minLength: 1 type: string d: + format: date-time type: string defaultValue: + default: forty-two + example: forty-two type: string embedded: + nullable: true type: string + x-kubernetes-embedded-resource: true intOrString: type: string + x-kubernetes-int-or-string: true schemaless: description: Schemaless field - type: string + required: + - a + - b type: object + x-kubernetes-preserve-unknown-fields: true object: description: Should maintain valid Type marker and not enumerate subfields. type: object @@ -40,6 +69,7 @@ components: val: x-kubernetes-preserve-unknown-fields: true type: object + x-kubernetes-preserve-unknown-fields: true info: title: OpenAPI Spec for Solo APIs. version: "" diff --git a/testdata/golden/test9/openapiv3.yaml b/testdata/golden/test9/openapiv3.yaml new file mode 100644 index 0000000..9d5f2d1 --- /dev/null +++ b/testdata/golden/test9/openapiv3.yaml @@ -0,0 +1,74 @@ +components: + schemas: + test9.Msg: + description: This is a top-level message. + properties: + a: + exclusiveMaximum: true + exclusiveMinimum: true + format: int32 + maximum: 100 + minimum: 5 + multipleOf: 2 + type: integer + x-kubernetes-validations: + - message: must not equal 27 + rule: self != 27 + blist: + items: + type: string + maxItems: 5 + minItems: 1 + type: array + uniqueItems: true + nested: + maxProperties: 2 + minProperties: 1 + properties: + a: + pattern: ^[a-zA-Z0-9_]*$ + type: string + b: + enum: + - Allow + - Forbid + - Replace + type: string + c: + maxLength: 100 + minLength: 1 + type: string + d: + format: date-time + type: string + defaultValue: + default: forty-two + example: forty-two + type: string + embedded: + nullable: true + type: string + x-kubernetes-embedded-resource: true + intOrString: + type: string + x-kubernetes-int-or-string: true + schemaless: + description: Schemaless field + type: object + x-kubernetes-preserve-unknown-fields: true + object: + description: Should maintain valid Type marker and not enumerate subfields. + type: object + x-kubernetes-preserve-unknown-fields: true + recursive: + type: object + x-kubernetes-preserve-unknown-fields: true + val: + x-kubernetes-preserve-unknown-fields: true + type: object + x-kubernetes-preserve-unknown-fields: true +info: + title: OpenAPI Spec for Solo APIs. + version: "" +openapi: 3.0.1 +paths: null diff --git a/testdata/test10/markers.proto b/testdata/test10/markers.proto new file mode 100644 index 0000000..353a9fb --- /dev/null +++ b/testdata/test10/markers.proto @@ -0,0 +1,85 @@ +syntax = "proto3"; + +package test10; + +import "struct.proto"; + +// This is a top-level message. +// +// +kubebuilder:pruning:PreserveUnknownFields +message Msg { + // +kubebuilder:pruning:PreserveUnknownFields + Nested nested = 1; + + // +kubebuilder:validation:Maximum=100 + // +kubebuilder:validation:Minimum=5 + // +kubebuilder:validation:ExclusiveMaximum=true + // +kubebuilder:validation:ExclusiveMinimum=true + // +kubebuilder:validation:MultipleOf=2 + // +kubebuilder:validation:XValidation:rule="self != 27",message="must not equal 27" + int32 a = 2; + + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=5 + // +kubebuilder:validation:UniqueItems=true + repeated string blist = 3; + + // +kubebuilder:validation:Type=value + google.protobuf.Value val = 4; + + // Should maintain valid Type marker and not enumerate subfields. + // + // +kubebuilder:validation:Type=object + Nested2 object = 5; + + // +kubebuilder:validation:Type=object + Recursive recursive = 6; + + // This is a nested message. + // + // +kubebuilder:validation:MinProperties=1 + // +kubebuilder:validation:MaxProperties=2 + message Nested { + // +kubebuilder:validation:Pattern="^[a-zA-Z0-9_]*$" + // +kubebuilder:validation:Required + string a = 1; + + // +kubebuilder:validation:Enum=Allow;Forbid;Replace + // +kubebuilder:validation:Required + string b = 2; + + // +kubebuilder:validation:MaxLength=100 + // +kubebuilder:validation:MinLength=1 + string c = 3; + + // +kubebuilder:validation:Format=date-time + string d = 4; + + // +kubebuilder:validation:XIntOrString + string int_or_string = 5; + + // +kubebuilder:default=forty-two + // +kubebuilder:example=forty-two + string default_value = 6; + + // Schemaless field + // + // +kubebuilder:validation:Schemaless + string schemaless = 7; + + // +kubebuilder:validation:EmbeddedResource + // +kubebuilder:validation:Nullable + string embedded = 8; + } + + message Nested2 { + string a = 1; + string b = 2; + int32 c = 3; + } + + message Recursive { + Recursive r = 1; + } +} + diff --git a/testdata/test8/markers.proto b/testdata/test8/markers.proto index fe271f0..9053939 100644 --- a/testdata/test8/markers.proto +++ b/testdata/test8/markers.proto @@ -1,6 +1,6 @@ syntax = "proto3"; -package test7; +package test8; import "struct.proto"; diff --git a/testdata/test9/markers.proto b/testdata/test9/markers.proto new file mode 100644 index 0000000..3855ad1 --- /dev/null +++ b/testdata/test9/markers.proto @@ -0,0 +1,85 @@ +syntax = "proto3"; + +package test9; + +import "struct.proto"; + +// This is a top-level message. +// +// +kubebuilder:pruning:PreserveUnknownFields +message Msg { + // +kubebuilder:pruning:PreserveUnknownFields + Nested nested = 1; + + // +kubebuilder:validation:Maximum=100 + // +kubebuilder:validation:Minimum=5 + // +kubebuilder:validation:ExclusiveMaximum=true + // +kubebuilder:validation:ExclusiveMinimum=true + // +kubebuilder:validation:MultipleOf=2 + // +kubebuilder:validation:XValidation:rule="self != 27",message="must not equal 27" + int32 a = 2; + + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=5 + // +kubebuilder:validation:UniqueItems=true + repeated string blist = 3; + + // +kubebuilder:validation:Type=value + google.protobuf.Value val = 4; + + // Should maintain valid Type marker and not enumerate subfields. + // + // +kubebuilder:validation:Type=object + Nested2 object = 5; + + // +kubebuilder:validation:Type=object + Recursive recursive = 6; + + // This is a nested message. + // + // +kubebuilder:validation:MinProperties=1 + // +kubebuilder:validation:MaxProperties=2 + message Nested { + // +kubebuilder:validation:Pattern="^[a-zA-Z0-9_]*$" + // +kubebuilder:validation:Required + string a = 1; + + // +kubebuilder:validation:Enum=Allow;Forbid;Replace + // +kubebuilder:validation:Required + string b = 2; + + // +kubebuilder:validation:MaxLength=100 + // +kubebuilder:validation:MinLength=1 + string c = 3; + + // +kubebuilder:validation:Format=date-time + string d = 4; + + // +kubebuilder:validation:XIntOrString + string int_or_string = 5; + + // +kubebuilder:default=forty-two + // +kubebuilder:example=forty-two + string default_value = 6; + + // Schemaless field + // + // +kubebuilder:validation:Schemaless + string schemaless = 7; + + // +kubebuilder:validation:EmbeddedResource + // +kubebuilder:validation:Nullable + string embedded = 8; + } + + message Nested2 { + string a = 1; + string b = 2; + int32 c = 3; + } + + message Recursive { + Recursive r = 1; + } +} + From 6ab0f42e42014d3584ee0cdcd422fa4c18900117 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 12:20:28 -0400 Subject: [PATCH 11/14] Improve doc comment --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5e6d8de..a957622 100644 --- a/README.md +++ b/README.md @@ -77,4 +77,4 @@ Other supported options are: * `disable_kube_markers` * when set to `true`, kubebuilder markers and validations such as PreserveUnknownFields, MinItems, default, and all CEL rules will be omitted from the OpenAPI schema. The Type and Required markers will be maintained. * `ignored_kube_markers` - * when set, ignores the contained kubebuilder markers and validations, and prevents them from being applied to the OpenAPI schema. + * when set, ignores the contained kubebuilder markers and validations, and prevents them from being applied to the OpenAPI schema. These values will be assembled into a regex that will perform a substring match on rules. From f17338e0445ab5982f903285c3cbc7c7fd77830b Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 12:21:39 -0400 Subject: [PATCH 12/14] Improve doc comment --- openapiGenerator.go | 1 + 1 file changed, 1 insertion(+) diff --git a/openapiGenerator.go b/openapiGenerator.go index 8a971cd..0ab8306 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -125,6 +125,7 @@ type openapiGenerator struct { // If provided, ignores the contained kubebuilder markers and validations, and prevents them from being applied to the OpenAPI schema. // If none are provided, no markers will be ignored. + // These values will be assembled into a regex that performs a logical OR substring match ignoredKubeMarkers []string } From 8982a3e42e663778d5af5f008393aa44c6ff57f8 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 12:28:09 -0400 Subject: [PATCH 13/14] Rename, and ensure comment is consistent in API and across repos --- README.md | 3 ++- integration_test.go | 6 +++--- main.go | 8 ++++---- openapiGenerator.go | 37 ++++++++++++++++++------------------- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index a957622..cd01b8a 100644 --- a/README.md +++ b/README.md @@ -77,4 +77,5 @@ Other supported options are: * `disable_kube_markers` * when set to `true`, kubebuilder markers and validations such as PreserveUnknownFields, MinItems, default, and all CEL rules will be omitted from the OpenAPI schema. The Type and Required markers will be maintained. * `ignored_kube_markers` - * when set, ignores the contained kubebuilder markers and validations, and prevents them from being applied to the OpenAPI schema. These values will be assembled into a regex that will perform a substring match on rules. + * when set, this list of substrings will be used to identify kubebuilder markers to ignore. When multiple are + supplied, this will function as a logical OR i.e. any rule which contains a provided substring will be ignored \ No newline at end of file diff --git a/integration_test.go b/integration_test.go index 519c06e..7f8a37c 100644 --- a/integration_test.go +++ b/integration_test.go @@ -116,7 +116,7 @@ func TestOpenAPIGeneration(t *testing.T) { name: "Test no markers are ignored when ignored_kube_markers is zero length", id: "test8", perPackage: false, - genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_markers=", + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_marker_substrings=", inputFiles: map[string][]string{ "test8": {"./testdata/test8/markers.proto"}, }, @@ -126,7 +126,7 @@ func TestOpenAPIGeneration(t *testing.T) { name: "Test ignored_kube_markers option ignores a single marker", id: "test9", perPackage: false, - genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_markers=Required", + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_marker_substrings=Required", inputFiles: map[string][]string{ "test9": {"./testdata/test9/markers.proto"}, }, @@ -136,7 +136,7 @@ func TestOpenAPIGeneration(t *testing.T) { name: "Test ignored_kube_markers option ignores multiple markers", id: "test10", perPackage: false, - genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_markers=Required+example", + genOpts: "yaml=true,single_file=true,proto_oneof=true,int_native=true,multiline_description=true,disable_kube_markers=false,ignored_kube_marker_substrings=Required+example", inputFiles: map[string][]string{ "test10": {"./testdata/test10/markers.proto"}, }, diff --git a/main.go b/main.go index 86382a4..fed1349 100644 --- a/main.go +++ b/main.go @@ -56,7 +56,7 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes disableKubeMarkers := false var messagesWithEmptySchema []string - var ignoredKubeMarkers []string + var ignoredKubeMarkerSubstrings []string p := extractParams(request.GetParameter()) for k, v := range p { @@ -148,9 +148,9 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes default: return nil, fmt.Errorf("unknown value '%s' for disable_kube_markers", v) } - } else if k == "ignored_kube_markers" { + } else if k == "ignored_kube_marker_substrings" { if len(v) > 0 { - ignoredKubeMarkers = strings.Split(v, "+") + ignoredKubeMarkerSubstrings = strings.Split(v, "+") } } else { return nil, fmt.Errorf("unknown argument '%s' specified", k) @@ -189,7 +189,7 @@ func generate(request pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorRes protoOneof, intNative, disableKubeMarkers, - ignoredKubeMarkers, + ignoredKubeMarkerSubstrings, ) return g.generateOutput(filesToGen) } diff --git a/openapiGenerator.go b/openapiGenerator.go index 0ab8306..697bd15 100644 --- a/openapiGenerator.go +++ b/openapiGenerator.go @@ -123,10 +123,9 @@ type openapiGenerator struct { // The Type and Required markers will be maintained. disableKubeMarkers bool - // If provided, ignores the contained kubebuilder markers and validations, and prevents them from being applied to the OpenAPI schema. - // If none are provided, no markers will be ignored. - // These values will be assembled into a regex that performs a logical OR substring match - ignoredKubeMarkers []string + // when set, this list of substrings will be used to identify kubebuilder markers to ignore. When multiple are + // supplied, this will function as a logical OR i.e. any rule which contains a provided substring will be ignored + ignoredKubeMarkerSubstrings []string } type DescriptionConfiguration struct { @@ -156,19 +155,19 @@ func newOpenAPIGenerator( log.Panicf("error initializing marker registry: %v", err) } return &openapiGenerator{ - model: model, - perFile: perFile, - singleFile: singleFile, - yaml: yaml, - useRef: useRef, - descriptionConfiguration: descriptionConfiguration, - enumAsIntOrString: enumAsIntOrString, - customSchemasByMessageName: buildCustomSchemasByMessageName(messagesWithEmptySchema), - protoOneof: protoOneof, - intNative: intNative, - markerRegistry: mRegistry, - disableKubeMarkers: disableKubeMarkers, - ignoredKubeMarkers: ignoredKubeMarkers, + model: model, + perFile: perFile, + singleFile: singleFile, + yaml: yaml, + useRef: useRef, + descriptionConfiguration: descriptionConfiguration, + enumAsIntOrString: enumAsIntOrString, + customSchemasByMessageName: buildCustomSchemasByMessageName(messagesWithEmptySchema), + protoOneof: protoOneof, + intNative: intNative, + markerRegistry: mRegistry, + disableKubeMarkers: disableKubeMarkers, + ignoredKubeMarkerSubstrings: ignoredKubeMarkers, } } @@ -671,9 +670,9 @@ func (g *openapiGenerator) parseComments(desc protomodel.CoreDesc) (comments str blocks := strings.Split(c, "\n\n") var ignoredKubeMarkersRegexp *regexp.Regexp - if len(g.ignoredKubeMarkers) > 0 { + if len(g.ignoredKubeMarkerSubstrings) > 0 { ignoredKubeMarkersRegexp = regexp.MustCompile( - fmt.Sprintf("(?:%s)", strings.Join(g.ignoredKubeMarkers, "|")), + fmt.Sprintf("(?:%s)", strings.Join(g.ignoredKubeMarkerSubstrings, "|")), ) } From bd88584c93462f22ad0ef28551c6ebd45b8b7ee8 Mon Sep 17 00:00:00 2001 From: Jonathan Jamroga Date: Wed, 7 Aug 2024 13:54:37 -0400 Subject: [PATCH 14/14] Cleaning --- README.md | 2 +- integration_test.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/README.md b/README.md index cd01b8a..1e76738 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,6 @@ Other supported options are: * when set to `true`, the native openapi schemas will be used for Integer types instead of Solo wrappers that add Kubernetes extension headers to the schema to treat int as strings. * `disable_kube_markers` * when set to `true`, kubebuilder markers and validations such as PreserveUnknownFields, MinItems, default, and all CEL rules will be omitted from the OpenAPI schema. The Type and Required markers will be maintained. -* `ignored_kube_markers` +* `ignored_kube_marker_substrings` * when set, this list of substrings will be used to identify kubebuilder markers to ignore. When multiple are supplied, this will function as a logical OR i.e. any rule which contains a provided substring will be ignored \ No newline at end of file diff --git a/integration_test.go b/integration_test.go index 7f8a37c..6c9e637 100644 --- a/integration_test.go +++ b/integration_test.go @@ -21,14 +21,11 @@ import ( "path/filepath" "strings" "testing" - "regexp" ) const goldenDir = "testdata/golden/" func TestOpenAPIGeneration(t *testing.T) { - regexp.MustCompile("(:?kubebuilder:altName)") - testcases := []struct { name string id string