Skip to content

Commit

Permalink
fix: properly render required fields and empty object bodies
Browse files Browse the repository at this point in the history
  • Loading branch information
Skarlso committed Sep 12, 2024
1 parent 947519f commit a57c665
Show file tree
Hide file tree
Showing 19 changed files with 212 additions and 69 deletions.
6 changes: 2 additions & 4 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,10 @@ linters-settings:
lines: 110
statements: 60
cyclop:
max-complexity: 50
max-complexity: 60
skip-tests: true
gocognit:
# Minimal code complexity to report.
# Default: 30 (but we recommend 10-20)
min-complexity: 50
min-complexity: 60
nolintlint:
allow-unused: false
require-explanation: true
Expand Down
2 changes: 1 addition & 1 deletion pkg/create_html_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func RenderContent(w io.WriteCloser, crd *v1.CustomResourceDefinition, comments,
}
var buffer []byte
buf := bytes.NewBuffer(buffer)
if err := parser.ParseProperties(version.Name, buf, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields); err != nil {
if err := parser.ParseProperties(version.Name, buf, version.Schema.OpenAPIV3Schema.Properties); err != nil {
return fmt.Errorf("failed to generate yaml sample: %w", err)
}
versions = append(versions, Version{
Expand Down
60 changes: 42 additions & 18 deletions pkg/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func Generate(crd *v1.CustomResourceDefinition, w io.WriteCloser, enableComments

parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, enableComments, minimal, skipRandom)
for i, version := range crd.Spec.Versions {
if err := parser.ParseProperties(version.Name, w, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields); err != nil {
if err := parser.ParseProperties(version.Name, w, version.Schema.OpenAPIV3Schema.Properties); err != nil {
return fmt.Errorf("failed to parse properties: %w", err)
}

Expand Down Expand Up @@ -77,7 +77,7 @@ func NewParser(group, kind string, comments, requiredOnly, skipRandom bool) *Par
// ParseProperties takes a writer and puts out any information / properties it encounters during the runs.
// It will recursively parse every "properties:" and "additionalProperties:". Using the types, it will also output
// some sample data based on those types.
func (p *Parser) ParseProperties(version string, file io.Writer, properties map[string]v1.JSONSchemaProps, requiredFields []string) error {
func (p *Parser) ParseProperties(version string, file io.Writer, properties map[string]v1.JSONSchemaProps) error {
sortedKeys := make([]string, 0, len(properties))
for k := range properties {
sortedKeys = append(sortedKeys, k)
Expand All @@ -86,13 +86,6 @@ func (p *Parser) ParseProperties(version string, file io.Writer, properties map[

w := &writer{}
for _, k := range sortedKeys {
// if field is not required, skip the entire flow.
if p.onlyRequired {
if !slices.Contains(requiredFields, k) {
continue
}
}

if p.inArray {
w.write(file, k+":")
p.inArray = false
Expand Down Expand Up @@ -124,24 +117,38 @@ func (p *Parser) ParseProperties(version string, file io.Writer, properties map[
}
// If we are dealing with an array, and we have properties to parse
// we need to reparse all of them again.
var result string
if properties[k].Type == array && properties[k].Items.Schema != nil && len(properties[k].Items.Schema.Properties) > 0 {
w.write(file, fmt.Sprintf("\n%s- ", strings.Repeat(" ", p.indent)))
p.indent += 2
p.inArray = true
if err := p.ParseProperties(version, file, properties[k].Items.Schema.Properties, properties[k].Items.Schema.Required); err != nil {

if p.onlyRequired && p.emptyAfterTrimRequired(properties[k].Items.Schema.Properties, properties[k].Items.Schema.Required) {
p.indent -= 2
w.write(file, " {}\n")
p.inArray = false // no longer in an array...

continue
}

if err := p.ParseProperties(version, file, properties[k].Items.Schema.Properties); err != nil {
return err
}
p.indent -= 2
} else {
result = outputValueType(properties[k], p.skipRandom)
w.write(file, fmt.Sprintf(" %s\n", result))
w.write(file, fmt.Sprintf(" %s\n", outputValueType(properties[k], p.skipRandom)))
}
case len(properties[k].Properties) > 0:
w.write(file, "\n")
// recursively parse all sub-properties
p.indent += 2
if err := p.ParseProperties(version, file, properties[k].Properties, properties[k].Required); err != nil {
if p.onlyRequired && p.emptyAfterTrimRequired(properties[k].Properties, properties[k].Required) {
p.indent -= 2
w.write(file, " {}\n")

continue
}

w.write(file, "\n")
if err := p.ParseProperties(version, file, properties[k].Properties); err != nil {
return err
}
p.indent -= 2
Expand All @@ -152,14 +159,21 @@ func (p *Parser) ParseProperties(version string, file io.Writer, properties map[
(properties[k].AdditionalProperties.Schema == nil || len(properties[k].AdditionalProperties.Schema.Properties) == 0) {
w.write(file, " {}\n")
} else {
w.write(file, "\n")

p.indent += 2
if p.onlyRequired && p.emptyAfterTrimRequired(
properties[k].AdditionalProperties.Schema.Properties,
properties[k].AdditionalProperties.Schema.Required) {
p.indent -= 2
w.write(file, " {}\n")

continue
}

w.write(file, "\n")
if err := p.ParseProperties(
version,
file,
properties[k].AdditionalProperties.Schema.Properties,
properties[k].AdditionalProperties.Schema.Required,
); err != nil {
return err
}
Expand All @@ -175,6 +189,16 @@ func (p *Parser) ParseProperties(version string, file io.Writer, properties map[
return nil
}

func (p *Parser) emptyAfterTrimRequired(properties map[string]v1.JSONSchemaProps, required []string) bool {
for k := range properties {
if !slices.Contains(required, k) {
delete(properties, k)
}
}

return len(properties) == 0
}

// outputValueType generate an output value based on the given type.
func outputValueType(v v1.JSONSchemaProps, skipRandom bool) string {
if v.Default != nil {
Expand Down
46 changes: 33 additions & 13 deletions pkg/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestGenerate(t *testing.T) {

version := crd.Spec.Versions[0]
parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, false, false, true)
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields))
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties))

golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_golden.yaml"))
require.NoError(t, err)
Expand All @@ -44,12 +44,12 @@ func TestGenerateWithTemplateDelimiter(t *testing.T) {

version := crd.Spec.Versions[0]
parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, false, false, true)
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields))
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties))

golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_with_template_start_character_default_value_golden.yaml"))
require.NoError(t, err)

assert.Equal(t, golden, buffer.Bytes())
assert.Equal(t, string(golden), buffer.String())
}

func TestGenerateWithExample(t *testing.T) {
Expand All @@ -64,12 +64,12 @@ func TestGenerateWithExample(t *testing.T) {

parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, false, false, true)
version := crd.Spec.Versions[0]
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields))
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties))

golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_with_example_golden.yaml"))
require.NoError(t, err)

assert.Equal(t, golden, buffer.Bytes())
assert.Equal(t, string(golden), buffer.String())
}

func TestGenerateWithComments(t *testing.T) {
Expand All @@ -84,12 +84,12 @@ func TestGenerateWithComments(t *testing.T) {

parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, true, false, true)
version := crd.Spec.Versions[0]
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields))
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties))

golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_with_comments_golden.yaml"))
require.NoError(t, err)

assert.Equal(t, golden, buffer.Bytes())
assert.Equal(t, string(golden), buffer.String())
}

func TestGenerateMinimal(t *testing.T) {
Expand All @@ -104,12 +104,12 @@ func TestGenerateMinimal(t *testing.T) {

parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, false, true, true)
version := crd.Spec.Versions[0]
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields))
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties))

golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_with_minimal_example_golden.yaml"))
require.NoError(t, err)

assert.Equal(t, golden, buffer.Bytes())
assert.Equal(t, string(golden), buffer.String())
}

func TestGenerateMinimalWithExample(t *testing.T) {
Expand All @@ -124,12 +124,32 @@ func TestGenerateMinimalWithExample(t *testing.T) {

parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, false, true, true)
version := crd.Spec.Versions[0]
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields))
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties))

golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_with_minimal_example_with_example_for_field_golden.yaml"))
require.NoError(t, err)

assert.Equal(t, golden, buffer.Bytes())
assert.Equal(t, string(golden), buffer.String())
}

func TestGenerateMinimalWithNoRequiredFields(t *testing.T) {
content, err := os.ReadFile(filepath.Join("testdata", "sample_crd_minimal_no_required_fields.yaml"))
require.NoError(t, err)

crd := &v1.CustomResourceDefinition{}
require.NoError(t, yaml.Unmarshal(content, crd))

var output []byte
buffer := bytes.NewBuffer(output)

parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, false, true, true)
version := crd.Spec.Versions[0]
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties))

golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_minimal_no_required_fields_golden.yaml"))
require.NoError(t, err)

assert.Equal(t, string(golden), buffer.String())
}

func TestGenerateWithAdditionalProperties(t *testing.T) {
Expand All @@ -144,10 +164,10 @@ func TestGenerateWithAdditionalProperties(t *testing.T) {

parser := NewParser(crd.Spec.Group, crd.Spec.Names.Kind, false, false, true)
version := crd.Spec.Versions[0]
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields))
require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties))

golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_with_additional_properties_golden.yaml"))
require.NoError(t, err)

assert.Equal(t, golden, buffer.Bytes())
assert.Equal(t, string(golden), buffer.String())
}
2 changes: 1 addition & 1 deletion pkg/matches/matchsnapshot/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (u *Update) Update(sourceTemplateLocation string, targetSnapshotLocation st
defer file.Close()

parser := pkg.NewParser(crd.Spec.Group, crd.Spec.Names.Kind, false, minimal, false)
if err := parser.ParseProperties(version.Name, file, version.Schema.OpenAPIV3Schema.Properties, pkg.RootRequiredFields); err != nil {
if err := parser.ParseProperties(version.Name, file, version.Schema.OpenAPIV3Schema.Properties); err != nil {
return fmt.Errorf("failed to parse properties: %w", err)
}
}
Expand Down
89 changes: 89 additions & 0 deletions pkg/testdata/sample_crd_minimal_no_required_fields.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.9.2
creationTimestamp: null
name: krokcommands.delivery.krok.app
spec:
group: delivery.krok.app
names:
kind: KrokCommand
listKind: KrokCommandList
plural: krokcommands
singular: krokcommand
scope: Namespaced
versions:
- name: v1alpha1
schema:
openAPIV3Schema:
description: KrokCommand is the Schema for the krokcommands API
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
spec:
description: KrokCommandSpec defines the desired state of KrokCommand
properties:
commandHasOutputToWrite:
description: CommandHasOutputToWrite if defined, it signals the underlying
Job, to put its output into a generated and created secret.
type: boolean
dependencies:
description: Dependencies defines a list of command names that this
command depends on.
items:
type: string
type: array
enabled:
description: Enabled defines if this command can be executed or not.
type: boolean
complex:
type: object
example: {key: "value"}
image:
description: 'Image defines the image name and tag of the command
example: krok-hook/slack-notification:v0.0.1'
type: string
example: krok-hook/slack-notification:v0.0.1
platforms:
description: Platforms holds all the platforms which this command
supports.
items:
type: string
type: array
readInputFromSecret:
description: ReadInputFromSecret if defined, the command will take
a list of key/value pairs in a secret and apply them as arguments
to the command.
properties:
name:
type: string
namespace:
type: string
required:
- name
- namespace
type: object
schedule:
description: 'Schedule of the command. example: 0 * * * * // follows
cron job syntax.'
type: string
type: object
status:
description: KrokCommandStatus defines the observed state of KrokCommand
type: object
type: object
served: true
storage: true
subresources:
status: {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
apiVersion: delivery.krok.app/v1alpha1
kind: KrokCommand
metadata: {}
spec: {}
status: {}
1 change: 1 addition & 0 deletions pkg/testdata/sample_crd_with_minimal_example_golden.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ kind: KrokCommand
metadata: {}
spec:
image: string
status: {}
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ kind: KrokCommand
metadata: {}
spec:
image: "krok-hook/slack-notification:v0.0.1"
status: {}
1 change: 1 addition & 0 deletions sample-tests/__snapshots__/bootstrap_crd-v1alpha1.min.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ kind: Bootstrap
metadata: {}
spec:
source: {}
status: {}
4 changes: 2 additions & 2 deletions sample-tests/__snapshots__/bootstrap_crd-v1alpha1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ status:
- lastTransitionTime: string
message: string
observedGeneration: 0
reason: i # ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
reason: N # ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
status: "True"
type: d # ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: 8x.46qk/L # ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
lastAppliedCRDNames: {}
lastAppliedRevision: string
lastAttemptedRevision: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
kind: AWSCluster
metadata: {}
spec: {}
status:
ready: false
Loading

0 comments on commit a57c665

Please sign in to comment.