From a66fdfd54589254e60cbb0ad5e326f4d2e5f69bb Mon Sep 17 00:00:00 2001 From: AislingHPE <143514178+AislingHPE@users.noreply.github.com> Date: Tue, 27 Aug 2024 11:42:32 +0100 Subject: [PATCH] Allow setting the default value of nullable (#17) * Allow setting the default value of nullable .. to true. This is to make the schema more scrictly align with what Terraform actually permits when `nullable` is not set in a variable definition. See the updated README for details. * Document an example for nullable --- README.md | 75 +++++++++++++++++++++++++++++- cmd/cmd.go | 6 +++ pkg/jsonschema/json-schema.go | 11 ++++- pkg/jsonschema/json-schema_test.go | 1 + 4 files changed, 90 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index fb22b3a..86e4ee2 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,8 @@ running Terraform. - `--disallow-additional-properties`: Set additionalProperties to false in the root object and nested objects (see [JSON Schema definition](https://json-schema.org/understanding-json-schema/reference/object#additionalproperties)). +- `--nullable-all`: Change the default value for `nullable` in a Variable block to 'true'. This is to make the behaviour more closely reflect Terraform's own validation. See 'Nullable Variables' below. + - `--overwrite`: Allow overwriting an existing file at the output location. - `--debug`: Print debug logs for variable retrieval and errors related to custom validation rules. @@ -295,7 +297,78 @@ If `nullable` is true in the `variable` block, then the JSON Schema will be modi }, ``` -This is actually a slight behaviour change from the validator used by terraform. If `nullable` is unset, then terraform treats them as `nullable` by default. I chose not to implement that behaviour here and instead am making the Terraform module author specify `nullable = true`. This is because otherwise schema definitions for simple programs would have to become a lot more verbose just to handle this case. +This is actually a slight behaviour change from the validator used by terraform. If `nullable` is unset, then terraform treats them as `nullable` by default. I chose not to implement that default behaviour here and instead am making the Terraform module author specify `nullable = true`. This is because otherwise schema definitions for simple programs would have to become a lot more verbose just to handle this case. + +For behaviour more consistent with Terraform, the flag `--nullable-all` can be used to reset the default value for nullable to be true. Note: this rule only applies to variables which have not explicitly set the value of nullable themselves. See [Terraform documentation on nullable](https://developer.hashicorp.com/terraform/language/values/variables#disallowing-null-input-values +). + +As an example, here is a Terraform configuration file which does not specify `nullable`: + +```hcl +variable "name" { + type = string + nullable = false +} + +variable "age" { + type = number + default = 10 +} +``` + +Without `--nullable-all`, this would result in the following JSON Schema file: + +```json +{ + "additionalProperties": true, + "properties": { + "age": { + "default": 10, + "type": "number" + }, + "name": { + "type": "string" + } + }, + "required": [ + "age", + "name" + ] +} +``` + +And if `--nullable-all` is set to true, then the 'default' value for nullable will be true, so the schema will change to reflect this: + +```json +{ + "additionalProperties": true, + "properties": { + "age": { + "anyOf": [ + { + "title": "null", + "type": "null" + }, + { + "title": "number", + "type": "number" + } + ], + "default": 10, + "title": "age: Select a type" + }, + "name": { + "type": "string" + } + }, + "required": [ + "age", + "name" + ] +} +``` + +`name` is not affected here since it has `nullable = false` in its HCL definition. ### Default Handling diff --git a/cmd/cmd.go b/cmd/cmd.go index 7a0d5d8..ff95f41 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -19,6 +19,7 @@ var ( allowEmpty bool requireAll bool outputStdOut bool + nullableAll bool inputPath string outputPath string debugOut bool @@ -88,6 +89,10 @@ func init() { "validation rules. Does not work with --stdout", ) + rootCmd.Flags().BoolVar(&nullableAll, "nullable-all", false, + "make all variables nullable unless nullable set to false explicitly, to make behavior consistent with Terraform", + ) + rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { _ = rootCmd.Usage() @@ -160,6 +165,7 @@ func runCommand(cmd *cobra.Command, args []string) error { AllowEmpty: allowEmpty, DebugOut: debugOut && !outputStdOut, SuppressLogging: outputStdOut, + NullableAll: nullableAll, }) if err != nil { return fmt.Errorf("error creating schema: %w", err) diff --git a/pkg/jsonschema/json-schema.go b/pkg/jsonschema/json-schema.go index c718b78..77ba5c6 100644 --- a/pkg/jsonschema/json-schema.go +++ b/pkg/jsonschema/json-schema.go @@ -16,6 +16,7 @@ type CreateSchemaOptions struct { AllowEmpty bool DebugOut bool SuppressLogging bool + NullableAll bool } func CreateSchema(path string, options CreateSchemaOptions) (map[string]any, error) { @@ -69,8 +70,14 @@ func createNode(name string, v model.TranslatedVariable, options CreateSchemaOpt return nil, fmt.Errorf("getting type constraint for %q: %w", name, err) } - nullableIsTrue := v.Variable.Nullable != nil && *v.Variable.Nullable - node, err := getNodeFromType(name, tc, nullableIsTrue, options) + // The default value for nullable is the value of NullableAll. For the purpose of keeping the JSON Schema relatively + // clean, this is normally set to false. Setting the default value to true is consistent with Terraform behavior. + nullableTranslatedValue := options.NullableAll + if v.Variable.Nullable != nil { + nullableTranslatedValue = *v.Variable.Nullable + } + + node, err := getNodeFromType(name, tc, nullableTranslatedValue, options) if err != nil { return nil, fmt.Errorf("%q: %w", name, err) } diff --git a/pkg/jsonschema/json-schema_test.go b/pkg/jsonschema/json-schema_test.go index 0684777..2f7b00e 100644 --- a/pkg/jsonschema/json-schema_test.go +++ b/pkg/jsonschema/json-schema_test.go @@ -36,6 +36,7 @@ func TestCreateSchema(t *testing.T) { RequireAll: false, AllowAdditionalProperties: true, AllowEmpty: true, + NullableAll: false, }) require.NoError(t, err)