Skip to content
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.

Sign up for GitHub

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

optionally include descriptions in schema #585

Closed
wants to merge 8 commits into from
Closed

optionally include descriptions in schema #585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8e242cf
optionally include descriptions in schema
puertomontt Sep 11, 2024
9c70bf0
Merge branch 'main' of ssh://github.com/solo-io/skv2 into puertomontt…
puertomontt Sep 11, 2024
a7c35d3
invert logic to maintain default=true behavior
puertomontt Sep 11, 2024
18beb23
cl
puertomontt Sep 11, 2024
367befc
determine whether to skip generating schema descriptions per group
puertomontt Sep 19, 2024
25081a8
lint
puertomontt Sep 19, 2024
ce47235
changelog
puertomontt Sep 20, 2024
e5fa8a3
clean up
puertomontt Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/v0.41.1/exclude-descriptions-in-schema.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
changelog:
- type: NEW_FEATURE
issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/18562
description: >
Add option to exclude descriptions in schema.
skipCI: "false"
2 changes: 1 addition & 1 deletion codegen/collector/compiler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (p *protoCompiler) CompileDescriptorsFromRoot(root string, skipDirs []strin
sem = make(chan struct{}, maxProtocs)
limitConcurrency = true
}
for _, dir := range append([]string{root}) {
for _, dir := range []string{root} {
absoluteDir, err := filepath.Abs(dir)
if err != nil {
return nil, err
Expand Down
7 changes: 4 additions & 3 deletions codegen/collector/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ func (d *DefaultProtocExecutor) Execute(protoFile string, toFile string, imports
type OpenApiProtocExecutor struct {
OutputDir string

// Whether to include descriptions in validation schemas
IncludeDescriptionsInSchema bool
// Whether to include descriptions in schemas
ExcludeDescriptionsInSchema bool

// Whether to assign Enum fields the `x-kubernetes-int-or-string` property
// which allows the value to either be an integer or a string
Expand Down Expand Up @@ -108,11 +108,12 @@ func (o *OpenApiProtocExecutor) Execute(protoFile string, toFile string, imports
_, fileName := filepath.Split(protoFile)
directoryName := fileName[0 : len(fileName)-len(filepath.Ext(fileName))]

baseArgument := "--openapi_out=yaml=true,single_file=false,include_description=true,multiline_description=true,proto_oneof=true,int_native=true"
baseArgument := "--openapi_out=yaml=true,single_file=false,multiline_description=true,proto_oneof=true,int_native=true"
baseArgument = fmt.Sprintf("%s,enum_as_int_or_string=%v", baseArgument, o.EnumAsIntOrString)
baseArgument = fmt.Sprintf("%s,additional_empty_schema=%v", baseArgument, strings.Join(o.MessagesWithEmptySchema, "+"))
baseArgument = fmt.Sprintf("%s,disable_kube_markers=%v", baseArgument, o.DisableKubeMarkers)
baseArgument = fmt.Sprintf("%s,ignored_kube_marker_substrings=%v", baseArgument, strings.Join(o.IgnoredKubeMarkerSubstrings, "+"))
baseArgument = fmt.Sprintf("%s,include_description=%v", baseArgument, !o.ExcludeDescriptionsInSchema)

// Create the directory
directoryPath := filepath.Join(o.OutputDir, directoryName)
Expand Down
9 changes: 6 additions & 3 deletions codegen/proto/schemagen/protoc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ import (

// Implementation of JsonSchemaGenerator that uses a plugin for the protocol buffer compiler
type protocGenerator struct {
validationSchemaOptions *ValidationSchemaOptions
validationSchemaOptions *ValidationSchemaOptions
excludeDescriptionsInSchema bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to move this field into ValidationSchemaOptions?
then NewProtocGenerator signature doesn't need to change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not related to validation schema specifically, which is why I felt it should be separate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open to suggestions on how to refactor though.
Maybe a high level options with validationSchemaOpts nested along with the excludeDescriptionsInSchema?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the skip descriptions flag does sound like it's related to the validation schema options, as they all go into the openapi schema in the CRD. but i could be mistaken as i'm not too familiar with the usage of these options

}

func NewProtocGenerator(validationSchemaOptions *ValidationSchemaOptions) *protocGenerator {
func NewProtocGenerator(validationSchemaOptions *ValidationSchemaOptions, excludeDescriptionsInSchema bool) *protocGenerator {
return &protocGenerator{
validationSchemaOptions: validationSchemaOptions,
validationSchemaOptions: validationSchemaOptions,
excludeDescriptionsInSchema: excludeDescriptionsInSchema,
}
}

Expand All @@ -44,6 +46,7 @@ func (p *protocGenerator) GetJSONSchemas(protoFiles []string, imports []string,
EnumAsIntOrString: p.validationSchemaOptions.EnumAsIntOrString,
MessagesWithEmptySchema: p.validationSchemaOptions.MessagesWithEmptySchema,
DisableKubeMarkers: p.validationSchemaOptions.DisableKubeMarkers,
ExcludeDescriptionsInSchema: p.excludeDescriptionsInSchema,
IgnoredKubeMarkerSubstrings: p.validationSchemaOptions.IgnoredKubeMarkerSubstrings,
}

Expand Down
6 changes: 3 additions & 3 deletions codegen/render/manifests_renderer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,13 @@ func (r ManifestsRenderer) RenderManifests(grps []*Group, protoOpts protoutil.Op

func generateOpenApi(grp model.Group, protoDir string, protoOpts protoutil.Options, groupOptions model.GroupOptions) (model.OpenApiSchemas, error) {
if groupOptions.SchemaGenerator == schemagen.ProtocGenOpenAPI {
return generateOpenApiFromProtocGen(grp, protoDir, protoOpts, groupOptions)
return generateOpenApiFromProtocGen(grp, groupOptions)
}
return generateOpenApiFromCue(grp, protoDir, protoOpts, groupOptions)
}

// Use protoc-gen-openapi for transpiling protobuf schemas to openapi v3 with k8s structural schema constraints.
func generateOpenApiFromProtocGen(grp model.Group, protoDir string, _ protoutil.Options, groupOptions model.GroupOptions) (model.OpenApiSchemas, error) {
func generateOpenApiFromProtocGen(grp model.Group, groupOptions model.GroupOptions) (model.OpenApiSchemas, error) {
// Collect all protobuf definitions including transitive dependencies.
var imports []string
for _, fileDescriptor := range grp.Descriptors {
Expand All @@ -152,7 +152,7 @@ func generateOpenApiFromProtocGen(grp model.Group, protoDir string, _ protoutil.
for i, fileDescriptor := range grp.Descriptors {
protoFiles[i] = fileDescriptor.ProtoFilePath
}
protoGen := schemagen.NewProtocGenerator(&groupOptions.SchemaValidationOpts)
protoGen := schemagen.NewProtocGenerator(&groupOptions.SchemaValidationOpts, grp.SkipSchemaDescriptions)
gvkSchemas, err := protoGen.GetJSONSchemas(protoFiles, imports, grp.GroupVersion)
if err != nil {
return nil, err
Expand Down
Loading