From a77e4532df87951934ad7af03240b1a0103adb46 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Tue, 6 Aug 2024 17:38:24 +0200 Subject: [PATCH] fix: nil schema check for additional properties (#94) * fix: nil schema check for additional properties * fix linting --- .golangci.yaml | 89 +++++++----------- Makefile | 2 +- pkg/generate.go | 2 +- pkg/generate_test.go | 20 ++++ ...sample_crd_with_additional_properties.yaml | 91 +++++++++++++++++++ ...crd_with_additional_properties_golden.yaml | 15 +++ 6 files changed, 163 insertions(+), 56 deletions(-) create mode 100644 pkg/testdata/sample_crd_with_additional_properties.yaml create mode 100644 pkg/testdata/sample_crd_with_additional_properties_golden.yaml diff --git a/.golangci.yaml b/.golangci.yaml index 2296116..2e5c6ae 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -8,68 +8,52 @@ run: linters: enable-all: true disable: - # We are working on it - - wrapcheck + - bodyclose + - containedctx # Struct should not contain context, action does. + - contextcheck + - cyclop # Complex functions are not good. + - deadcode - depguard - # Logical next step - - forcetypeassert # Priority: that can lead to serious crashes. + - dogsled + - dupl # Check code duplications. + - execinquery + - exhaustive # Doesn't really make sense. + - exhaustivestruct + - exhaustruct # Doesn't really make sense. - exportloopref - - goerr113 - - varnamelen # m, d, p < These are not so meaningful variables. - - testpackage # Blackbox testing is preffered. + - forcetypeassert # Priority: that can lead to serious crashes. - funlen # Break long functions. + - gci + - gochecknoglobals + - gochecknoinits # Init functions cause an import to have side effects, + - goerr113 + - goimports # acts weirdly, dci handles imports anyway + - golint - gomnd # Give constant values a name with constants. + - ifshort + - interfacebloat + - interfacer - ireturn # Accept interface, return concrate. + - lll + - loggercheck # Doesn't really make sense. + - maligned - nestif # Some nexted if statements are 8 or 9 deep. - - dupl # Check code duplications. - - cyclop # Complex functions are not good. - - gochecknoinits # Init functions cause an import to have side effects, - # and side effects are hard to test, - # reduce readability and increase the complexity of code. - - containedctx # Struct should not contain context, action does. - nilnil # A function should return either something valuable - # or an error, but both value and error as nil is - # useless. Like when I call it, why is it nil? Tell me - # in an error why. - - bodyclose - - unparam - nonamedreturns # Either named return, or use simply `return`. - - # Opinionated (we may want to keep it disabled) - - gochecknoglobals - - lll + - nosnakecase - paralleltest - - tagliatelle - - wsl - - interfacebloat - - - # Disabled with reason - - dogsled - - exhaustruct # Doesn't really make sense. - - exhaustive # Doesn't really make sense. - - loggercheck # Doesn't really make sense. - - goimports # acts weirdly, dci handles imports anyway - - # Disabled because of generics in go 1.18 - - contextcheck - rowserrcheck - - sqlclosecheck - - wastedassign - - # Deprecated - - execinquery - - deadcode - - exhaustivestruct - - golint - - ifshort - - interfacer - - maligned - scopelint + - sqlclosecheck - structcheck + - tagliatelle + - testpackage # Blackbox testing is preffered. + - unparam - varcheck - - gci - - nosnakecase + - varnamelen # m, d, p < These are not so meaningful variables. + - wastedassign + - wrapcheck + - wsl linters-settings: gci: @@ -84,12 +68,12 @@ linters-settings: lines: 110 statements: 60 cyclop: - max-complexity: 45 + max-complexity: 46 skip-tests: true gocognit: # Minimal code complexity to report. # Default: 30 (but we recommend 10-20) - min-complexity: 45 + min-complexity: 46 nolintlint: allow-unused: false require-explanation: true @@ -111,9 +95,6 @@ issues: - path: cmds/ linters: - forbidigo - - text: "should not use dot imports|don't use an underscore in package name" - linters: - - golint - source: "https://" linters: - lll diff --git a/Makefile b/Makefile index 95c2cbc..d49749b 100644 --- a/Makefile +++ b/Makefile @@ -54,7 +54,7 @@ clean: ## Runs go clean go clean -i GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint -GOLANGCI_LINT_VERSION ?= v1.56.1 +GOLANGCI_LINT_VERSION ?= v1.57.2 golangci-lint: $(GOLANGCI_LINT) $(GOLANGCI_LINT): $(LOCALBIN) diff --git a/pkg/generate.go b/pkg/generate.go index 953640c..8df57cc 100644 --- a/pkg/generate.go +++ b/pkg/generate.go @@ -140,7 +140,7 @@ func (p *Parser) ParseProperties(version string, file io.Writer, properties map[ } p.indent -= 2 case properties[k].AdditionalProperties != nil: - if len(properties[k].AdditionalProperties.Schema.Properties) == 0 { + if properties[k].AdditionalProperties.Schema == nil || len(properties[k].AdditionalProperties.Schema.Properties) == 0 { w.write(file, " {}\n") } else { w.write(file, "\n") diff --git a/pkg/generate_test.go b/pkg/generate_test.go index cb6a69e..aa91cca 100644 --- a/pkg/generate_test.go +++ b/pkg/generate_test.go @@ -111,3 +111,23 @@ func TestGenerateMinimalWithExample(t *testing.T) { assert.Equal(t, golden, buffer.Bytes()) } + +func TestGenerateWithAdditionalProperties(t *testing.T) { + content, err := os.ReadFile(filepath.Join("testdata", "sample_crd_with_additional_properties.yaml")) + require.NoError(t, err) + + crd := &v1beta1.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, false) + version := crd.Spec.Versions[0] + require.NoError(t, parser.ParseProperties(version.Name, buffer, version.Schema.OpenAPIV3Schema.Properties, RootRequiredFields)) + + golden, err := os.ReadFile(filepath.Join("testdata", "sample_crd_with_additional_properties_golden.yaml")) + require.NoError(t, err) + + assert.Equal(t, golden, buffer.Bytes()) +} diff --git a/pkg/testdata/sample_crd_with_additional_properties.yaml b/pkg/testdata/sample_crd_with_additional_properties.yaml new file mode 100644 index 0000000..aacee09 --- /dev/null +++ b/pkg/testdata/sample_crd_with_additional_properties.yaml @@ -0,0 +1,91 @@ +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: + annotations: + additionalProperties: true + description: Annotations set to all Kubernetes components + type: object + 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 + image: + description: 'Image defines the image name and tag of the command + example: krok-hook/slack-notification:v0.0.1' + type: string + 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 + required: + - image + type: object + status: + description: KrokCommandStatus defines the observed state of KrokCommand + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/pkg/testdata/sample_crd_with_additional_properties_golden.yaml b/pkg/testdata/sample_crd_with_additional_properties_golden.yaml new file mode 100644 index 0000000..f93e716 --- /dev/null +++ b/pkg/testdata/sample_crd_with_additional_properties_golden.yaml @@ -0,0 +1,15 @@ +apiVersion: delivery.krok.app/v1alpha1 +kind: KrokCommand +metadata: {} +spec: + annotations: {} + commandHasOutputToWrite: true + dependencies: ["string"] + enabled: true + image: string + platforms: ["string"] + readInputFromSecret: + name: string + namespace: string + schedule: string +status: {}