From d41b90b6dd7d98c116b613dbad24d1bf4586fcd8 Mon Sep 17 00:00:00 2001 From: Chris Hill Date: Tue, 12 Aug 2025 13:20:15 -0700 Subject: [PATCH 1/9] Gracefully handle empty and 'any' types in terraform --- .../testdata/opentofu/any/variables.tf | 3 --- .../testdata/opentofu/empty/variables.tf | 1 - .../testdata/opentofu/nestedany/variables.tf | 5 ---- .../testdata/opentofu/simple/schema.json | 24 +++++++++++++++++++ .../testdata/opentofu/simple/variables.tf | 12 ++++++++++ pkg/opentofu/tofutoschema.go | 12 ++++++---- pkg/opentofu/tofutoschema_test.go | 12 ---------- pkg/schema/types.go | 2 +- 8 files changed, 44 insertions(+), 27 deletions(-) delete mode 100644 pkg/opentofu/testdata/opentofu/any/variables.tf delete mode 100644 pkg/opentofu/testdata/opentofu/empty/variables.tf delete mode 100644 pkg/opentofu/testdata/opentofu/nestedany/variables.tf diff --git a/pkg/opentofu/testdata/opentofu/any/variables.tf b/pkg/opentofu/testdata/opentofu/any/variables.tf deleted file mode 100644 index 53521e6..0000000 --- a/pkg/opentofu/testdata/opentofu/any/variables.tf +++ /dev/null @@ -1,3 +0,0 @@ -variable "foo" { - type = any -} \ No newline at end of file diff --git a/pkg/opentofu/testdata/opentofu/empty/variables.tf b/pkg/opentofu/testdata/opentofu/empty/variables.tf deleted file mode 100644 index 27d4129..0000000 --- a/pkg/opentofu/testdata/opentofu/empty/variables.tf +++ /dev/null @@ -1 +0,0 @@ -variable "foo" {} \ No newline at end of file diff --git a/pkg/opentofu/testdata/opentofu/nestedany/variables.tf b/pkg/opentofu/testdata/opentofu/nestedany/variables.tf deleted file mode 100644 index 1f099e0..0000000 --- a/pkg/opentofu/testdata/opentofu/nestedany/variables.tf +++ /dev/null @@ -1,5 +0,0 @@ -variable "foo" { - type = object({ - bar = any - }) -} \ No newline at end of file diff --git a/pkg/opentofu/testdata/opentofu/simple/schema.json b/pkg/opentofu/testdata/opentofu/simple/schema.json index d64aae2..b047603 100644 --- a/pkg/opentofu/testdata/opentofu/simple/schema.json +++ b/pkg/opentofu/testdata/opentofu/simple/schema.json @@ -1,5 +1,8 @@ { "required": [ + "any", + "empty", + "nestedany", "nodescription", "testbool", "testemptybool", @@ -197,6 +200,27 @@ "nodescription": { "title": "nodescription", "type": "string" + }, + "any": { + "title": "any", + "$comment": "Airlock warning: unconstrained type from OpenTofu/Terraform 'any'" + }, + "nestedany": { + "title": "nestedany", + "type": "object", + "properties": { + "foo": { + "title": "foo", + "$comment": "Airlock warning: unconstrained type from OpenTofu/Terraform 'any'" + } + }, + "required": [ + "foo" + ] + }, + "empty": { + "title": "empty", + "$comment": "Airlock warning: unconstrained type from OpenTofu/Terraform 'any'" } } } diff --git a/pkg/opentofu/testdata/opentofu/simple/variables.tf b/pkg/opentofu/testdata/opentofu/simple/variables.tf index 182f718..4ae54d0 100644 --- a/pkg/opentofu/testdata/opentofu/simple/variables.tf +++ b/pkg/opentofu/testdata/opentofu/simple/variables.tf @@ -89,3 +89,15 @@ variable "testmap" { variable "nodescription" { type = string } + +variable "any" { + type = any +} + +variable "nestedany" { + type = object({ + foo = any + }) +} + +variable "empty" {} diff --git a/pkg/opentofu/tofutoschema.go b/pkg/opentofu/tofutoschema.go index 2ec509c..63ec58d 100644 --- a/pkg/opentofu/tofutoschema.go +++ b/pkg/opentofu/tofutoschema.go @@ -73,10 +73,7 @@ func variableToSchema(variable *tfconfig.Variable) (*schema.Schema, error) { func variableTypeStringToCtyType(variableType string) (cty.Type, *typeexpr.Defaults, error) { if variableType == "" { - return cty.NilType, nil, errors.New("type cannot be empty") - } - if variableType == "any" { - return cty.NilType, nil, errors.New("type 'any' cannot be converted to a JSON schema type") + variableType = "any" } expr, diags := hclsyntax.ParseExpression([]byte(variableType), "", hcl.Pos{Line: 1, Column: 1}) if len(diags) != 0 { @@ -110,7 +107,7 @@ func hydrateSchemaFromNameTypeAndDefaults(sch *schema.Schema, name string, ty ct } else if ty.IsSetType() { return hydrateSetSchema(sch, name, ty, defaults) } else if ty.HasDynamicTypes() { - return fmt.Errorf("dynamic types are not supported (are you using type 'any'?)") + return hydrateAnySchema(sch) } else { return fmt.Errorf("unsupported type %q", ty.FriendlyName()) } @@ -165,6 +162,11 @@ func hydrateSetSchema(sch *schema.Schema, name string, ty cty.Type, defaults *ty return hydrateArraySchema(sch, name, ty, defaults) } +func hydrateAnySchema(sch *schema.Schema) error { + sch.Comment = "Airlock warning: unconstrained type from OpenTofu/Terraform 'any'" + return nil +} + func ctyValueToInterface(val cty.Value) interface{} { valJSON, err := ctyjson.Marshal(val, val.Type()) if err != nil { diff --git a/pkg/opentofu/tofutoschema_test.go b/pkg/opentofu/tofutoschema_test.go index 47ccae4..00fffbf 100644 --- a/pkg/opentofu/tofutoschema_test.go +++ b/pkg/opentofu/tofutoschema_test.go @@ -21,18 +21,6 @@ func TestTofuToSchema(t *testing.T) { { name: "simple", }, - { - name: "any", - err: "type 'any' cannot be converted to a JSON schema type", - }, - { - name: "nestedany", - err: "dynamic types are not supported (are you using type 'any'?)", - }, - { - name: "empty", - err: "type cannot be empty", - }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/schema/types.go b/pkg/schema/types.go index d4ceb63..9e88bc3 100644 --- a/pkg/schema/types.go +++ b/pkg/schema/types.go @@ -16,7 +16,7 @@ type Schema struct { Ref string `json:"$ref,omitempty"` // section 8.2.3.1 DynamicRef string `json:"$dynamicRef,omitempty"` // section 8.2.3.2 // Definitions Definitions `json:"$defs,omitempty"` // section 8.2.4 - Comments string `json:"$comment,omitempty"` // section 8.3 + Comment string `json:"$comment,omitempty"` // section 8.3 // RFC draft-bhutton-json-schema-00 section 10.2.1 (Sub-schemas with logic) AllOf []*Schema `json:"allOf,omitempty"` // section 10.2.1.1 AnyOf []*Schema `json:"anyOf,omitempty"` // section 10.2.1.2 From 6f133a283be5cc5213a3ac750a47a500a7522559 Mon Sep 17 00:00:00 2001 From: Chris Hill Date: Thu, 14 Aug 2025 22:09:35 -0700 Subject: [PATCH 2/9] graceful reporting of errors/warnings --- go.mod | 2 +- go.sum | 4 +- pkg/helm/helmtoschema.go | 181 ++++++++++++++++++------------ pkg/helm/helmtoschema_test.go | 46 ++++++-- pkg/opentofu/tofutoschema.go | 100 +++++++++++------ pkg/opentofu/tofutoschema_test.go | 51 +++++---- pkg/result/types.go | 27 +++++ 7 files changed, 277 insertions(+), 134 deletions(-) create mode 100644 pkg/result/types.go diff --git a/go.mod b/go.mod index 4cc8a08..7d5af25 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/hashicorp/hcl/v2 v2.22.0 github.com/massdriver-cloud/terraform-config-inspect v0.0.1 github.com/spf13/cobra v1.8.1 - github.com/stretchr/testify v1.9.0 + github.com/stretchr/testify v1.10.0 github.com/wk8/go-ordered-map/v2 v2.1.8 github.com/xeipuuv/gojsonschema v1.2.0 github.com/zclconf/go-cty v1.15.0 diff --git a/go.sum b/go.sum index ef9e2ff..a6f54c6 100644 --- a/go.sum +++ b/go.sum @@ -105,8 +105,8 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= -github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= +github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/fJgbpc= github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= diff --git a/pkg/helm/helmtoschema.go b/pkg/helm/helmtoschema.go index e69105d..12cd1d4 100644 --- a/pkg/helm/helmtoschema.go +++ b/pkg/helm/helmtoschema.go @@ -6,39 +6,56 @@ import ( "strconv" "strings" + "github.com/massdriver-cloud/airlock/pkg/result" "github.com/massdriver-cloud/airlock/pkg/schema" orderedmap "github.com/wk8/go-ordered-map/v2" yaml "gopkg.in/yaml.v3" ) -type nullError struct{} - -func (e *nullError) Error() string { - return "type is indeterminate (null)" -} - -func HelmToSchema(valuesPath string) (*schema.Schema, error) { +func HelmToSchema(valuesPath string) result.SchemaResult { valuesBytes, readErr := os.ReadFile(valuesPath) if readErr != nil { - return nil, readErr + return result.SchemaResult{ + Schema: nil, + Diags: []result.Diagnostic{ + { + Path: valuesPath, + Code: "file_read_error", + Message: fmt.Sprintf("failed to read values file: %s", readErr), + Level: result.Error, + }, + }, + } } valuesDocument := yaml.Node{} unmarshalErr := yaml.Unmarshal(valuesBytes, &valuesDocument) if unmarshalErr != nil { - return nil, unmarshalErr + return result.SchemaResult{ + Schema: nil, + Diags: []result.Diagnostic{ + { + Path: valuesPath, + Code: "yaml_unmarshal_error", + Message: fmt.Sprintf("failed to unmarshal values file: %s", unmarshalErr), + Level: result.Error, + }, + }, + } } - // the top level node is a document node. We need to go one layer - // deeper to get the actual yaml content sch := new(schema.Schema) - err := parseMapNode(sch, valuesDocument.Content[0]) - if err != nil { - return nil, err + result := result.SchemaResult{ + Schema: sch, + Diags: []result.Diagnostic{}, } - return sch, nil + // the top level node is a document node. We need to go one layer + // deeper to get the actual yaml content + result.Diags = parseMapNode(sch, valuesDocument.Content[0], result.Diags) + + return result } func parseNameNode(schema *schema.Schema, node *yaml.Node) { @@ -50,83 +67,99 @@ func parseNameNode(schema *schema.Schema, node *yaml.Node) { } } -func parseValueNode(schema *schema.Schema, node *yaml.Node) error { +func parseValueNode(schema *schema.Schema, node *yaml.Node, diags []result.Diagnostic) []result.Diagnostic { switch node.Tag { case "!!str": - return parseStringNode(schema, node) + parseStringNode(schema, node) case "!!int": - return parseIntegerNode(schema, node) + return parseIntegerNode(schema, node, diags) case "!!float": - return parseFloatNode(schema, node) + return parseFloatNode(schema, node, diags) case "!!bool": - return parseBooleanNode(schema, node) + return parseBooleanNode(schema, node, diags) case "!!map": - return parseMapNode(schema, node) + return parseMapNode(schema, node, diags) case "!!seq": - return parseArrayNode(schema, node) + return parseArrayNode(schema, node, diags) case "!!null": - return &nullError{} + schema.Comment = "Airlock Warning: unknown type from null value" + return append(diags, result.Diagnostic{ + Path: schema.Title, + Code: "unknown_type", + Message: fmt.Sprintf("type of field %s is indeterminate (null)", schema.Title), + Level: result.Warning, + }) default: - return fmt.Errorf("unrecognized tag %s", node.Tag) + schema.Comment = fmt.Sprintf("Airlock Warning: unknown type %s", node.Tag) + return append(diags, result.Diagnostic{ + Path: schema.Title, + Code: "unknown_type", + Message: fmt.Sprintf("type of field %s is unsupported (%s)", schema.Title, node.Tag), + Level: result.Warning, + }) } + return diags } -func nodeToProperty(name, value *yaml.Node) (*schema.Schema, error) { - sch := new(schema.Schema) - +func nodeToProperty(sch *schema.Schema, name, value *yaml.Node, diags []result.Diagnostic) []result.Diagnostic { parseNameNode(sch, name) - err := parseValueNode(sch, value) - if err != nil { - //nolint:errorlint - if _, ok := err.(*nullError); ok { - fmt.Printf("warning: skipping field %s\n reason: %v\n", sch.Title, err) - //nolint:nilnil - return nil, nil - } - return nil, err - } + diags = parseValueNode(sch, value, diags) - return sch, nil + return diags } -func parseStringNode(sch *schema.Schema, node *yaml.Node) error { +func parseStringNode(sch *schema.Schema, node *yaml.Node) { sch.Type = "string" sch.Default = node.Value - return nil } -func parseIntegerNode(sch *schema.Schema, node *yaml.Node) error { +func parseIntegerNode(sch *schema.Schema, node *yaml.Node, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "integer" def, err := strconv.Atoi(node.Value) if err != nil { - return err + return append(diags, result.Diagnostic{ + Path: node.Value, + Code: "invalid_value", + Message: fmt.Sprintf("failed to parse integer: %s", err), + Level: result.Error, + }) } sch.Default = def - return nil + return diags } -func parseFloatNode(sch *schema.Schema, node *yaml.Node) error { +func parseFloatNode(sch *schema.Schema, node *yaml.Node, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "number" def, err := strconv.ParseFloat(node.Value, 64) if err != nil { - return err + return append(diags, result.Diagnostic{ + Path: node.Value, + Code: "invalid_value", + Message: fmt.Sprintf("failed to parse float: %s", err), + Level: result.Error, + }) } sch.Default = def - return nil + return diags } -func parseBooleanNode(sch *schema.Schema, node *yaml.Node) error { +func parseBooleanNode(sch *schema.Schema, node *yaml.Node, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "boolean" def, err := strconv.ParseBool(node.Value) if err != nil { - return err + return append(diags, result.Diagnostic{ + Path: node.Value, + Code: "invalid_value", + Message: fmt.Sprintf("failed to parse boolean: %s", err), + Level: result.Error, + }) } sch.Default = def - return nil + return diags } -func parseMapNode(sch *schema.Schema, node *yaml.Node) error { +func parseMapNode(sch *schema.Schema, node *yaml.Node, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "object" sch.Properties = orderedmap.New[string, *schema.Schema]() @@ -135,36 +168,44 @@ func parseMapNode(sch *schema.Schema, node *yaml.Node) error { for index := 0; index < len(nodes); index += 2 { nameNode := nodes[index] valueNode := nodes[index+1] - property, err := nodeToProperty(nameNode, valueNode) - if err != nil { - return err - } - if property != nil { - sch.Properties.Set(nameNode.Value, property) - sch.Required = append(sch.Required, nameNode.Value) - } + + property := new(schema.Schema) + diags = nodeToProperty(property, nameNode, valueNode, diags) + + sch.Properties.Set(nameNode.Value, property) + sch.Required = append(sch.Required, nameNode.Value) } - return nil + return diags } -func parseArrayNode(sch *schema.Schema, node *yaml.Node) error { +func parseArrayNode(sch *schema.Schema, node *yaml.Node, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "array" - if len(node.Content) == 0 { - return &nullError{} - } sch.Items = new(schema.Schema) - err := parseValueNode(sch.Items, node.Content[0]) - if err != nil { - return err + + if len(node.Content) == 0 { + sch.Items.Comment = "Airlock Warning: unknown type from empty array" + return append(diags, result.Diagnostic{ + Path: sch.Title, + Code: "unknown_type", + Message: fmt.Sprintf("array %s is empty so it's type is unknown", sch.Title), + Level: result.Warning, + }) } + diags = parseValueNode(sch.Items, node.Content[0], diags) + // Set the default back to nil since we don't want to default all items to the first type in the list - err = node.Decode(&sch.Default) - if err != nil { - return err + decodeErr := node.Decode(&sch.Default) + if decodeErr != nil { + return append(diags, result.Diagnostic{ + Path: sch.Title, + Code: "invalid_type", + Message: fmt.Sprintf("failed to decode array default: %s", decodeErr), + Level: result.Error, + }) } - return nil + return diags } diff --git a/pkg/helm/helmtoschema_test.go b/pkg/helm/helmtoschema_test.go index a5cab1e..811cda4 100644 --- a/pkg/helm/helmtoschema_test.go +++ b/pkg/helm/helmtoschema_test.go @@ -5,19 +5,22 @@ import ( "testing" "github.com/massdriver-cloud/airlock/pkg/helm" + "github.com/massdriver-cloud/airlock/pkg/result" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestRun(t *testing.T) { type testData struct { name string - modulePath string + valuesPath string want string + diags []result.Diagnostic } tests := []testData{ { name: "simple", - modulePath: "testdata/values.yaml", + valuesPath: "testdata/values.yaml", want: ` { "required": [ @@ -25,7 +28,9 @@ func TestRun(t *testing.T) { "age", "height", "object", - "array" + "array", + "emptyArray", + "nullValue" ], "type": "object", "properties": { @@ -81,25 +86,50 @@ func TestRun(t *testing.T) { "foo", "bar" ] + }, + "emptyArray": { + "title": "emptyArray", + "type": "array", + "description": "An empty array should not cause an error", + "items": { + "$comment": "Airlock Warning: unknown type from empty array" + } + }, + "nullValue": { + "title": "nullValue", + "$comment": "Airlock Warning: unknown type from null value" } } } `, + diags: []result.Diagnostic{ + { + Path: "emptyArray", + Code: "unknown_type", + Message: "array emptyArray is empty so it's type is unknown", + Level: result.Warning, + }, + { + Path: "nullValue", + Code: "unknown_type", + Message: "type of field nullValue is indeterminate (null)", + Level: result.Warning, + }, + }, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got, err := helm.HelmToSchema(tc.modulePath) - if err != nil { - t.Fatalf("%d, unexpected error", err) - } + got := helm.HelmToSchema(tc.valuesPath) - bytes, err := json.Marshal(got) + bytes, err := json.Marshal(got.Schema) if err != nil { t.Fatalf("%d, unexpected error", err) } require.JSONEq(t, tc.want, string(bytes)) + + assert.ElementsMatch(t, tc.diags, got.Diags) }) } } diff --git a/pkg/opentofu/tofutoschema.go b/pkg/opentofu/tofutoschema.go index 63ec58d..0581850 100644 --- a/pkg/opentofu/tofutoschema.go +++ b/pkg/opentofu/tofutoschema.go @@ -9,6 +9,7 @@ import ( hcl "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/ext/typeexpr" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/massdriver-cloud/airlock/pkg/result" "github.com/massdriver-cloud/airlock/pkg/schema" "github.com/massdriver-cloud/terraform-config-inspect/tfconfig" orderedmap "github.com/wk8/go-ordered-map/v2" @@ -16,34 +17,58 @@ import ( ctyjson "github.com/zclconf/go-cty/cty/json" ) -func TofuToSchema(modulePath string) (*schema.Schema, error) { +func TofuToSchema(modulePath string) result.SchemaResult { module, err := tfconfig.LoadModule(modulePath) if err != nil { - return nil, err + return result.SchemaResult{ + Schema: nil, + Diags: []result.Diagnostic{ + { + Path: modulePath, + Code: "module_load_error", + Message: fmt.Sprintf("failed to load module: %s", err), + Level: result.Error, + }, + }, + } } sch := new(schema.Schema) sch.Properties = orderedmap.New[string, *schema.Schema]() + result := result.SchemaResult{ + Schema: sch, + Diags: []result.Diagnostic{}, + } + for _, variable := range module.Variables { - variableSchema, err := variableToSchema(variable) - if err != nil { - return nil, fmt.Errorf("failed to convert variable %q to schema: %w", variable.Name, err) + variableSchema, diags := variableToSchema(variable, result.Diags) + result.Diags = diags + + if variableSchema == nil { + continue } + sch.Properties.Set(variable.Name, variableSchema) sch.Required = append(sch.Required, variable.Name) } slices.Sort(sch.Required) - return sch, nil + return result } -func variableToSchema(variable *tfconfig.Variable) (*schema.Schema, error) { +func variableToSchema(variable *tfconfig.Variable, diags []result.Diagnostic) (*schema.Schema, []result.Diagnostic) { schema := new(schema.Schema) variableType, defaults, typeErr := variableTypeStringToCtyType(variable.Type) if typeErr != nil { - return nil, fmt.Errorf("failed to parse type %q: %w", variable.Type, typeErr) + diags = append(diags, result.Diagnostic{ + Path: variable.Name, + Code: "variable_type_error", + Message: fmt.Sprintf("failed to parse type %q: %s", variable.Type, typeErr), + Level: result.Error, + }) + return nil, diags } // To simplify the logic of recursively walking the Defaults structure in objects types, // we make the extracted Defaults a Child of a dummy "top level" node @@ -54,9 +79,7 @@ func variableToSchema(variable *tfconfig.Variable) (*schema.Schema, error) { variable.Name: defaults, } } - if hydrateErr := hydrateSchemaFromNameTypeAndDefaults(schema, variable.Name, variableType, topLevelDefault); hydrateErr != nil { - return nil, fmt.Errorf("failed to hydrate schema for variable %q: %w", variable.Name, hydrateErr) - } + diags = hydrateSchemaFromNameTypeAndDefaults(schema, variable.Name, variableType, topLevelDefault, diags) schema.Description = variable.Description @@ -68,7 +91,7 @@ func variableToSchema(variable *tfconfig.Variable) (*schema.Schema, error) { schema.Default = false } - return schema, nil + return schema, diags } func variableTypeStringToCtyType(variableType string) (cty.Type, *typeexpr.Defaults, error) { @@ -86,7 +109,7 @@ func variableTypeStringToCtyType(variableType string) (cty.Type, *typeexpr.Defau return ty, defaults, nil } -func hydrateSchemaFromNameTypeAndDefaults(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults) error { +func hydrateSchemaFromNameTypeAndDefaults(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults, diags []result.Diagnostic) []result.Diagnostic { sch.Title = name if defaults != nil { @@ -99,19 +122,25 @@ func hydrateSchemaFromNameTypeAndDefaults(sch *schema.Schema, name string, ty ct if ty.IsPrimitiveType() { hydratePrimitiveSchema(sch, ty) } else if ty.IsMapType() { - return hydrateMapSchema(sch, name, ty, defaults) + return hydrateMapSchema(sch, name, ty, defaults, diags) } else if ty.IsObjectType() { - return hydrateObjectSchema(sch, name, ty, defaults) + return hydrateObjectSchema(sch, name, ty, defaults, diags) } else if ty.IsListType() { - return hydrateArraySchema(sch, name, ty, defaults) + return hydrateArraySchema(sch, name, ty, defaults, diags) } else if ty.IsSetType() { - return hydrateSetSchema(sch, name, ty, defaults) + return hydrateSetSchema(sch, name, ty, defaults, diags) } else if ty.HasDynamicTypes() { - return hydrateAnySchema(sch) + return hydrateAnySchema(sch, diags) } else { - return fmt.Errorf("unsupported type %q", ty.FriendlyName()) - } - return nil + sch.Comment = fmt.Sprintf("unsupported OpenTofu/Terraform type '%s'", ty.FriendlyName()) + return append(diags, result.Diagnostic{ + Path: name, + Code: "unsupported_type", + Message: sch.Comment, + Level: result.Warning, + }) + } + return diags } func hydratePrimitiveSchema(sch *schema.Schema, ty cty.Type) { @@ -125,46 +154,49 @@ func hydratePrimitiveSchema(sch *schema.Schema, ty cty.Type) { } } -func hydrateObjectSchema(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults) error { +func hydrateObjectSchema(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "object" sch.Properties = orderedmap.New[string, *schema.Schema]() for attName, attType := range ty.AttributeTypes() { attributeSchema := new(schema.Schema) - if err := hydrateSchemaFromNameTypeAndDefaults(attributeSchema, attName, attType, getDefaultChildren(name, defaults)); err != nil { - return err - } + diags = hydrateSchemaFromNameTypeAndDefaults(attributeSchema, attName, attType, getDefaultChildren(name, defaults), diags) sch.Properties.Set(attName, attributeSchema) if !ty.AttributeOptional(attName) { sch.Required = append(sch.Required, attName) } } slices.Sort(sch.Required) - return nil + return diags } -func hydrateMapSchema(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults) error { +func hydrateMapSchema(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "object" sch.PropertyNames = &schema.Schema{ Pattern: "^.*$", } sch.AdditionalProperties = new(schema.Schema) - return hydrateSchemaFromNameTypeAndDefaults(sch.AdditionalProperties.(*schema.Schema), "", ty.ElementType(), getDefaultChildren(name, defaults)) + return hydrateSchemaFromNameTypeAndDefaults(sch.AdditionalProperties.(*schema.Schema), "", ty.ElementType(), getDefaultChildren(name, defaults), diags) } -func hydrateArraySchema(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults) error { +func hydrateArraySchema(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "array" sch.Items = new(schema.Schema) - return hydrateSchemaFromNameTypeAndDefaults(sch.Items, "", ty.ElementType(), getDefaultChildren(name, defaults)) + return hydrateSchemaFromNameTypeAndDefaults(sch.Items, "", ty.ElementType(), getDefaultChildren(name, defaults), diags) } -func hydrateSetSchema(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults) error { +func hydrateSetSchema(sch *schema.Schema, name string, ty cty.Type, defaults *typeexpr.Defaults, diags []result.Diagnostic) []result.Diagnostic { sch.UniqueItems = true - return hydrateArraySchema(sch, name, ty, defaults) + return hydrateArraySchema(sch, name, ty, defaults, diags) } -func hydrateAnySchema(sch *schema.Schema) error { +func hydrateAnySchema(sch *schema.Schema, diags []result.Diagnostic) []result.Diagnostic { sch.Comment = "Airlock warning: unconstrained type from OpenTofu/Terraform 'any'" - return nil + return append(diags, result.Diagnostic{ + Path: sch.Title, + Code: "unconstrained_type", + Message: "unconstrained type from OpenTofu/Terraform 'any'", + Level: result.Warning, + }) } func ctyValueToInterface(val cty.Value) interface{} { diff --git a/pkg/opentofu/tofutoschema_test.go b/pkg/opentofu/tofutoschema_test.go index 00fffbf..bfbf880 100644 --- a/pkg/opentofu/tofutoschema_test.go +++ b/pkg/opentofu/tofutoschema_test.go @@ -4,53 +4,66 @@ import ( "encoding/json" "os" "path/filepath" - "strings" "testing" "github.com/massdriver-cloud/airlock/pkg/opentofu" + "github.com/massdriver-cloud/airlock/pkg/result" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestTofuToSchema(t *testing.T) { type testData struct { - name string - err string + name string + diags []result.Diagnostic } tests := []testData{ { name: "simple", + diags: []result.Diagnostic{ + { + Path: "any", + Code: "unconstrained_type", + Message: "unconstrained type from OpenTofu/Terraform 'any'", + Level: result.Warning, + }, + { + Path: "foo", + Code: "unconstrained_type", + Message: "unconstrained type from OpenTofu/Terraform 'any'", + Level: result.Warning, + }, + { + Path: "empty", + Code: "unconstrained_type", + Message: "unconstrained type from OpenTofu/Terraform 'any'", + Level: result.Warning, + }, + }, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { modulePath := filepath.Join("testdata/opentofu", tc.name) - got, schemaErr := opentofu.TofuToSchema(modulePath) - if schemaErr != nil && tc.err == "" { - t.Fatalf("unexpected error: %s", schemaErr.Error()) - } - if tc.err != "" && schemaErr == nil { - t.Fatalf("expected error %s, got nil", tc.err) - } - if tc.err != "" && !strings.Contains(schemaErr.Error(), tc.err) { - t.Fatalf("expected error %s, got %s", tc.err, schemaErr.Error()) - } - if tc.err != "" { - return - } + got := opentofu.TofuToSchema(modulePath) - bytes, marshalErr := json.Marshal(got) + gotSchema, marshalErr := json.Marshal(got.Schema) if marshalErr != nil { t.Fatalf("unexpected error: %s", marshalErr.Error()) } - want, readErr := os.ReadFile(filepath.Join("testdata/opentofu", tc.name, "schema.json")) + wantSchema, readErr := os.ReadFile(filepath.Join("testdata/opentofu", tc.name, "schema.json")) if readErr != nil { t.Fatalf("unexpected error: %s", readErr.Error()) } - require.JSONEq(t, string(want), string(bytes)) + require.JSONEq(t, string(wantSchema), string(gotSchema)) + + gotDiags := got.Diags + + assert.ElementsMatch(t, tc.diags, gotDiags) }) } } diff --git a/pkg/result/types.go b/pkg/result/types.go new file mode 100644 index 0000000..ef900f5 --- /dev/null +++ b/pkg/result/types.go @@ -0,0 +1,27 @@ +package result + +import "github.com/massdriver-cloud/airlock/pkg/schema" + +type SchemaResult struct { + Schema *schema.Schema + Diags []Diagnostic +} + +type CodeResult struct { + Code []byte + Diags []Diagnostic +} + +type Severity string + +const ( + Warning Severity = "warning" + Error Severity = "error" +) + +type Diagnostic struct { + Path string + Code string + Message string + Level Severity +} From 70c40dbd02b3e12be7ccbd6e345507fe89d749fa Mon Sep 17 00:00:00 2001 From: Chris Hill Date: Sat, 16 Aug 2025 11:31:54 -0600 Subject: [PATCH 3/9] bicep changes mostly done --- pkg/bicep/biceptoschema.go | 166 +++++++++++++++++++++---------------- 1 file changed, 94 insertions(+), 72 deletions(-) diff --git a/pkg/bicep/biceptoschema.go b/pkg/bicep/biceptoschema.go index e220b12..f82024f 100644 --- a/pkg/bicep/biceptoschema.go +++ b/pkg/bicep/biceptoschema.go @@ -2,12 +2,12 @@ package bicep import ( "encoding/json" - "errors" "fmt" "reflect" "slices" bp "github.com/Checkmarx/kics/v2/pkg/parser/bicep" + "github.com/massdriver-cloud/airlock/pkg/result" "github.com/massdriver-cloud/airlock/pkg/schema" orderedmap "github.com/wk8/go-ordered-map/v2" @@ -29,22 +29,37 @@ type bicepParamMetadata struct { Metadata map[string]interface{} `json:"metadata"` } -func BicepToSchema(templatePath string) (*schema.Schema, error) { +func BicepToSchema(templatePath string) result.SchemaResult { // using the github.com/Checkmarx/kics parser since he already did the heavy lifting to parse a bicep template parser := bp.Parser{} - params := new(schema.Schema) - params.Type = "object" - params.Properties = orderedmap.New[string, *schema.Schema]() - params.Required = []string{} + sch := new(schema.Schema) + sch.Type = "object" + sch.Properties = orderedmap.New[string, *schema.Schema]() + sch.Required = []string{} + + doc, _, parseErr := parser.Parse(templatePath, nil) + if parseErr != nil { + return result.SchemaResult{ + Schema: nil, + Diags: []result.Diagnostic{ + { + Path: templatePath, + Code: "file_read_error", + Message: fmt.Sprintf("failed to read bicep file: %s", parseErr), + Level: result.Error, + }, + }, + } + } - doc, _, err := parser.Parse(templatePath, nil) - if err != nil { - return nil, err + result := result.SchemaResult{ + Schema: sch, + Diags: []result.Diagnostic{}, } for name, value := range doc[0]["parameters"].(map[string]interface{}) { - param := bicepParam{} + param := new(bicepParam) // marshal to json and unmarshal into custom struct to make bicep param easier to access bytes, marshalErr := json.Marshal(value) @@ -60,50 +75,60 @@ func BicepToSchema(templatePath string) (*schema.Schema, error) { property.Title = name property.Description = param.Metadata.Description - parseErr := parseBicepParam(property, param) - if parseErr != nil { - return nil, parseErr - } + result.Diags = parseBicepParam(property, param, result.Diags) - params.Properties.Set(name, property) - params.Required = append(params.Required, name) + sch.Properties.Set(name, property) + sch.Required = append(sch.Required, name) } // sorting this here just to help with testing. The order doesn't matter, but to our test suite it does. - slices.Sort(params.Required) + slices.Sort(sch.Required) - return params, nil + return result } -func parseBicepParam(sch *schema.Schema, bicepParam bicepParam) error { +func parseBicepParam(sch *schema.Schema, bicepParam *bicepParam, diags []result.Diagnostic) []result.Diagnostic { switch bicepParam.TypeString { case "int": - return parseIntParam(sch, bicepParam) + return parseIntParam(sch, bicepParam, diags) case "bool": - return parseBoolParam(sch, bicepParam) + parseBoolParam(sch, bicepParam) case "string": - return parseStringParam(sch, bicepParam, false) + return parseStringParam(sch, bicepParam, false, diags) case "secureString": - return parseStringParam(sch, bicepParam, true) + return parseStringParam(sch, bicepParam, true, diags) case "array": - return parseArrayParam(sch, bicepParam) + return parseArrayParam(sch, bicepParam, diags) case "object", "secureObject": - return parseObjectParam(sch, bicepParam) + return parseObjectParam(sch, bicepParam, diags) default: - return errors.New("unknown type: " + bicepParam.TypeString) + return append(diags, result.Diagnostic{ + Path: sch.Title, + Code: "unknown_type", + Message: fmt.Sprintf("type of field %s is unsupported (%s)", sch.Title, bicepParam.TypeString), + Level: result.Warning, + }) } + return diags } -func parseIntParam(sch *schema.Schema, bicepParam bicepParam) error { +func parseIntParam(sch *schema.Schema, bicepParam *bicepParam, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "integer" sch.Default = bicepParam.DefaultValue allowedVals := bicepParam.AllowedValues if len(allowedVals) == 1 { assertedEnum, ok := allowedVals[0].([]interface{}) - if !ok { - return fmt.Errorf("unable to cast %v to []interface{}", allowedVals) + if ok { + sch.Enum = assertedEnum + } else { + diags = append(diags, result.Diagnostic{ + Path: sch.Title, + Code: "invalid_value", + Message: fmt.Sprintf("unable to convert 'allowedValues' to enum in bicep param %s", sch.Title), + Level: result.Warning, + }) } - sch.Enum = assertedEnum + } if bicepParam.MinValue != nil { @@ -113,16 +138,15 @@ func parseIntParam(sch *schema.Schema, bicepParam bicepParam) error { sch.Maximum = json.Number(fmt.Sprintf("%d", *bicepParam.MaxValue)) } - return nil + return diags } -func parseBoolParam(sch *schema.Schema, bicepParam bicepParam) error { +func parseBoolParam(sch *schema.Schema, bicepParam *bicepParam) { sch.Type = "boolean" sch.Default = bicepParam.DefaultValue - return nil } -func parseStringParam(sch *schema.Schema, bicepParam bicepParam, secure bool) error { +func parseStringParam(sch *schema.Schema, bicepParam *bicepParam, secure bool, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "string" sch.Default = bicepParam.DefaultValue @@ -133,46 +157,46 @@ func parseStringParam(sch *schema.Schema, bicepParam bicepParam, secure bool) er allowedVals := bicepParam.AllowedValues if len(allowedVals) == 1 { assertedEnum, ok := allowedVals[0].([]interface{}) - if !ok { - return fmt.Errorf("unable to cast %v to []interface{}", allowedVals) + if ok { + sch.Enum = assertedEnum + } else { + diags = append(diags, result.Diagnostic{ + Path: sch.Title, + Code: "invalid_value", + Message: fmt.Sprintf("unable to convert 'allowedValues' to enum in bicep param %s", sch.Title), + Level: result.Warning, + }) } - sch.Enum = assertedEnum } sch.MinLength = bicepParam.MinLength sch.MaxLength = bicepParam.MaxLength - return nil + return diags } -func parseArrayParam(sch *schema.Schema, bicepParam bicepParam) error { +func parseArrayParam(sch *schema.Schema, bicepParam *bicepParam, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "array" sch.MinItems = bicepParam.MinLength sch.MaxItems = bicepParam.MaxLength if bicepParam.DefaultValue != nil && len(bicepParam.DefaultValue.([]interface{})) != 0 { - err := parseArrayType(sch, bicepParam.DefaultValue.([]interface{})) - if err != nil { - return err - } + diags = parseArrayType(sch, bicepParam.DefaultValue.([]interface{}), diags) } - return nil + return diags } -func parseObjectParam(sch *schema.Schema, bicepParam bicepParam) error { +func parseObjectParam(sch *schema.Schema, bicepParam *bicepParam, diags []result.Diagnostic) []result.Diagnostic { sch.Type = "object" if bicepParam.DefaultValue != nil && len(bicepParam.DefaultValue.(map[string]interface{})) > 1 { - err := parseObjectType(sch, bicepParam.DefaultValue.(map[string]interface{})) - if err != nil { - return err - } + diags = parseObjectType(sch, bicepParam.DefaultValue.(map[string]interface{}), diags) } - return nil + return diags } -func parseObjectType(sch *schema.Schema, objValue map[string]interface{}) error { +func parseObjectType(sch *schema.Schema, objValue map[string]interface{}, diags []result.Diagnostic) []result.Diagnostic { sch.Properties = orderedmap.New[string, *schema.Schema]() sch.Required = []string{} @@ -196,18 +220,17 @@ func parseObjectType(sch *schema.Schema, objValue map[string]interface{}) error property.Default = value case reflect.Slice: property.Type = "array" - err := parseArrayType(property, value.([]interface{})) - if err != nil { - return err - } + diags = parseArrayType(property, value.([]interface{}), diags) case reflect.Map: property.Type = "object" - err := parseObjectType(property, value.(map[string]interface{})) - if err != nil { - return err - } + diags = parseObjectType(property, value.(map[string]interface{}), diags) default: - return errors.New("unknown type: " + reflect.TypeOf(value).String()) + diags = append(diags, result.Diagnostic{ + Path: sch.Title, + Code: "unknown_type", + Message: fmt.Sprintf("type of field %s is unsupported (%s)", sch.Title, reflect.TypeOf(value).Kind()), + Level: result.Warning, + }) } sch.Properties.Set(name, property) @@ -215,10 +238,10 @@ func parseObjectType(sch *schema.Schema, objValue map[string]interface{}) error slices.Sort(sch.Required) } - return nil + return diags } -func parseArrayType(sch *schema.Schema, value []interface{}) error { +func parseArrayType(sch *schema.Schema, value []interface{}, diags []result.Diagnostic) []result.Diagnostic { if len(value) > 0 { items := new(schema.Schema) @@ -235,21 +258,20 @@ func parseArrayType(sch *schema.Schema, value []interface{}) error { sch.Default = value case reflect.Slice: items.Type = "array" - err := parseArrayType(items, elem.([]interface{})) - if err != nil { - return err - } + diags = parseArrayType(items, elem.([]interface{}), diags) case reflect.Map: items.Type = "object" - err := parseObjectType(items, elem.(map[string]interface{})) - if err != nil { - return err - } + diags = parseObjectType(items, elem.(map[string]interface{}), diags) default: - return errors.New("unknown type: " + reflect.TypeOf(elem).String()) + diags = append(diags, result.Diagnostic{ + Path: sch.Title, + Code: "unknown_type", + Message: fmt.Sprintf("type of field %s is unsupported (%s)", sch.Title, reflect.TypeOf(value).Kind()), + Level: result.Warning, + }) } sch.Items = items } - return nil + return diags } From d37f26a713e35bf860244d17b0be6bd6811ef21e Mon Sep 17 00:00:00 2001 From: chrisghill Date: Tue, 19 Aug 2025 15:30:46 -0600 Subject: [PATCH 4/9] bicep working, pretty output --- cmd/bicep.go | 13 +++--------- cmd/helm.go | 13 +++--------- cmd/opentofu.go | 13 +++--------- pkg/bicep/biceptoschema.go | 27 +++++++++++++++++++----- pkg/bicep/biceptoschema_test.go | 12 ++++++----- pkg/helm/helmtoschema_test.go | 30 +++++++++++++-------------- pkg/opentofu/tofutoschema.go | 4 ++-- pkg/opentofu/tofutoschema_test.go | 6 +++--- pkg/result/print.go | 34 +++++++++++++++++++++++++++++++ 9 files changed, 92 insertions(+), 60 deletions(-) create mode 100644 pkg/result/print.go diff --git a/cmd/bicep.go b/cmd/bicep.go index b57c0ed..7187bb6 100644 --- a/cmd/bicep.go +++ b/cmd/bicep.go @@ -1,7 +1,6 @@ package cmd import ( - "encoding/json" "fmt" "os" @@ -42,17 +41,11 @@ func NewCmdBicep() *cobra.Command { } func runBicepInput(cmd *cobra.Command, args []string) error { - schema, err := bicep.BicepToSchema(args[0]) - if err != nil { - return err - } + result := bicep.BicepToSchema(args[0]) - bytes, err := json.MarshalIndent(schema, "", " ") - if err != nil { - return err - } + fmt.Print(result.PrettyDiags()) + fmt.Print(result.PrettySchema()) - fmt.Println(string(bytes)) return nil } diff --git a/cmd/helm.go b/cmd/helm.go index 97bf9b9..5879cc4 100644 --- a/cmd/helm.go +++ b/cmd/helm.go @@ -1,7 +1,6 @@ package cmd import ( - "encoding/json" "fmt" "github.com/massdriver-cloud/airlock/docs/helpdocs" @@ -31,16 +30,10 @@ func NewCmdHelm() *cobra.Command { } func runHelmInput(cmd *cobra.Command, args []string) error { - schema, err := helm.HelmToSchema(args[0]) - if err != nil { - return err - } + result := helm.HelmToSchema(args[0]) - bytes, err := json.MarshalIndent(schema, "", " ") - if err != nil { - return err - } + fmt.Print(result.PrettyDiags()) + fmt.Print(result.PrettySchema()) - fmt.Println(string(bytes)) return nil } diff --git a/cmd/opentofu.go b/cmd/opentofu.go index 9ddb373..e8041f2 100644 --- a/cmd/opentofu.go +++ b/cmd/opentofu.go @@ -1,7 +1,6 @@ package cmd import ( - "encoding/json" "fmt" "os" @@ -43,17 +42,11 @@ func NewCmdOpenTofu() *cobra.Command { } func runOpenTofuInput(cmd *cobra.Command, args []string) error { - schema, err := opentofu.TofuToSchema(args[0]) - if err != nil { - return err - } + result := opentofu.TofuToSchema(args[0]) - bytes, err := json.MarshalIndent(schema, "", " ") - if err != nil { - return err - } + fmt.Print(result.PrettyDiags()) + fmt.Print(result.PrettySchema()) - fmt.Println(string(bytes)) return nil } diff --git a/pkg/bicep/biceptoschema.go b/pkg/bicep/biceptoschema.go index f82024f..2eb7ebd 100644 --- a/pkg/bicep/biceptoschema.go +++ b/pkg/bicep/biceptoschema.go @@ -53,7 +53,7 @@ func BicepToSchema(templatePath string) result.SchemaResult { } } - result := result.SchemaResult{ + output := result.SchemaResult{ Schema: sch, Diags: []result.Diagnostic{}, } @@ -64,18 +64,30 @@ func BicepToSchema(templatePath string) result.SchemaResult { // marshal to json and unmarshal into custom struct to make bicep param easier to access bytes, marshalErr := json.Marshal(value) if marshalErr != nil { - return nil, marshalErr + output.Diags = append(output.Diags, result.Diagnostic{ + Path: name, + Code: "invalid_value", + Message: fmt.Sprintf("failed to marshal bicep param %s: %s", name, marshalErr), + Level: result.Error, + }) + continue } unmarshalErr := json.Unmarshal(bytes, ¶m) if unmarshalErr != nil { - return nil, unmarshalErr + output.Diags = append(output.Diags, result.Diagnostic{ + Path: name, + Code: "invalid_value", + Message: fmt.Sprintf("failed to unmarshal bicep param %s: %s", name, unmarshalErr), + Level: result.Error, + }) + continue } property := new(schema.Schema) property.Title = name property.Description = param.Metadata.Description - result.Diags = parseBicepParam(property, param, result.Diags) + output.Diags = parseBicepParam(property, param, output.Diags) sch.Properties.Set(name, property) sch.Required = append(sch.Required, name) @@ -83,7 +95,7 @@ func BicepToSchema(templatePath string) result.SchemaResult { // sorting this here just to help with testing. The order doesn't matter, but to our test suite it does. slices.Sort(sch.Required) - return result + return output } func parseBicepParam(sch *schema.Schema, bicepParam *bicepParam, diags []result.Diagnostic) []result.Diagnostic { @@ -101,6 +113,7 @@ func parseBicepParam(sch *schema.Schema, bicepParam *bicepParam, diags []result. case "object", "secureObject": return parseObjectParam(sch, bicepParam, diags) default: + sch.Comment = fmt.Sprintf("Airlock Warning: unknown type from Bicep parameter (%s)", bicepParam.TypeString) return append(diags, result.Diagnostic{ Path: sch.Title, Code: "unknown_type", @@ -121,6 +134,7 @@ func parseIntParam(sch *schema.Schema, bicepParam *bicepParam, diags []result.Di if ok { sch.Enum = assertedEnum } else { + sch.Comment = "Airlock Warning: unable to convert 'allowedValues' to enum" diags = append(diags, result.Diagnostic{ Path: sch.Title, Code: "invalid_value", @@ -160,6 +174,7 @@ func parseStringParam(sch *schema.Schema, bicepParam *bicepParam, secure bool, d if ok { sch.Enum = assertedEnum } else { + sch.Comment = "Airlock Warning: unable to convert 'allowedValues' to enum" diags = append(diags, result.Diagnostic{ Path: sch.Title, Code: "invalid_value", @@ -225,6 +240,7 @@ func parseObjectType(sch *schema.Schema, objValue map[string]interface{}, diags property.Type = "object" diags = parseObjectType(property, value.(map[string]interface{}), diags) default: + sch.Comment = fmt.Sprintf("Airlock Warning: unknown type for field %s (%s)", name, reflect.TypeOf(value).Kind()) diags = append(diags, result.Diagnostic{ Path: sch.Title, Code: "unknown_type", @@ -263,6 +279,7 @@ func parseArrayType(sch *schema.Schema, value []interface{}, diags []result.Diag items.Type = "object" diags = parseObjectType(items, elem.(map[string]interface{}), diags) default: + sch.Comment = fmt.Sprintf("Airlock Warning: unknown type (%s)", reflect.TypeOf(value).Kind()) diags = append(diags, result.Diagnostic{ Path: sch.Title, Code: "unknown_type", diff --git a/pkg/bicep/biceptoschema_test.go b/pkg/bicep/biceptoschema_test.go index 0d9db18..e027256 100644 --- a/pkg/bicep/biceptoschema_test.go +++ b/pkg/bicep/biceptoschema_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/massdriver-cloud/airlock/pkg/bicep" + "github.com/massdriver-cloud/airlock/pkg/result" "github.com/stretchr/testify/assert" ) @@ -12,12 +13,14 @@ func TestBicepToSchema(t *testing.T) { type testData struct { name string bicepPath string + diags []result.Diagnostic want string } tests := []testData{ { name: "simple", bicepPath: "testdata/template.bicep", + diags: []result.Diagnostic{}, want: ` { "required": [ @@ -168,16 +171,15 @@ func TestBicepToSchema(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got, err := bicep.BicepToSchema(tc.bicepPath) - if err != nil { - t.Fatalf("%d, unexpected error", err) - } + got := bicep.BicepToSchema(tc.bicepPath) - bytes, err := json.Marshal(got) + bytes, err := json.Marshal(got.Schema) if err != nil { t.Fatalf("%d, unexpected error", err) } + assert.ElementsMatch(t, tc.diags, got.Diags) + assert.JSONEq(t, tc.want, string(bytes)) }) } diff --git a/pkg/helm/helmtoschema_test.go b/pkg/helm/helmtoschema_test.go index 811cda4..23f45c9 100644 --- a/pkg/helm/helmtoschema_test.go +++ b/pkg/helm/helmtoschema_test.go @@ -14,13 +14,27 @@ func TestRun(t *testing.T) { type testData struct { name string valuesPath string - want string diags []result.Diagnostic + want string } tests := []testData{ { name: "simple", valuesPath: "testdata/values.yaml", + diags: []result.Diagnostic{ + { + Path: "emptyArray", + Code: "unknown_type", + Message: "array emptyArray is empty so it's type is unknown", + Level: result.Warning, + }, + { + Path: "nullValue", + Code: "unknown_type", + Message: "type of field nullValue is indeterminate (null)", + Level: result.Warning, + }, + }, want: ` { "required": [ @@ -102,20 +116,6 @@ func TestRun(t *testing.T) { } } `, - diags: []result.Diagnostic{ - { - Path: "emptyArray", - Code: "unknown_type", - Message: "array emptyArray is empty so it's type is unknown", - Level: result.Warning, - }, - { - Path: "nullValue", - Code: "unknown_type", - Message: "type of field nullValue is indeterminate (null)", - Level: result.Warning, - }, - }, }, } for _, tc := range tests { diff --git a/pkg/opentofu/tofutoschema.go b/pkg/opentofu/tofutoschema.go index 0581850..b80d8ec 100644 --- a/pkg/opentofu/tofutoschema.go +++ b/pkg/opentofu/tofutoschema.go @@ -136,7 +136,7 @@ func hydrateSchemaFromNameTypeAndDefaults(sch *schema.Schema, name string, ty ct return append(diags, result.Diagnostic{ Path: name, Code: "unsupported_type", - Message: sch.Comment, + Message: fmt.Sprintf("unsupported OpenTofu/Terraform type '%s' in field '%s'", ty.FriendlyName(), name), Level: result.Warning, }) } @@ -194,7 +194,7 @@ func hydrateAnySchema(sch *schema.Schema, diags []result.Diagnostic) []result.Di return append(diags, result.Diagnostic{ Path: sch.Title, Code: "unconstrained_type", - Message: "unconstrained type from OpenTofu/Terraform 'any'", + Message: fmt.Sprintf("unconstrained type in field '%s' from OpenTofu/Terraform 'any'", sch.Title), Level: result.Warning, }) } diff --git a/pkg/opentofu/tofutoschema_test.go b/pkg/opentofu/tofutoschema_test.go index bfbf880..c19822c 100644 --- a/pkg/opentofu/tofutoschema_test.go +++ b/pkg/opentofu/tofutoschema_test.go @@ -25,19 +25,19 @@ func TestTofuToSchema(t *testing.T) { { Path: "any", Code: "unconstrained_type", - Message: "unconstrained type from OpenTofu/Terraform 'any'", + Message: "unconstrained type in field 'any' from OpenTofu/Terraform 'any'", Level: result.Warning, }, { Path: "foo", Code: "unconstrained_type", - Message: "unconstrained type from OpenTofu/Terraform 'any'", + Message: "unconstrained type in field 'foo' from OpenTofu/Terraform 'any'", Level: result.Warning, }, { Path: "empty", Code: "unconstrained_type", - Message: "unconstrained type from OpenTofu/Terraform 'any'", + Message: "unconstrained type in field 'empty' from OpenTofu/Terraform 'any'", Level: result.Warning, }, }, diff --git a/pkg/result/print.go b/pkg/result/print.go new file mode 100644 index 0000000..6af21f3 --- /dev/null +++ b/pkg/result/print.go @@ -0,0 +1,34 @@ +package result + +import ( + "encoding/json" + "fmt" + + "github.com/charmbracelet/lipgloss" +) + +func (r *SchemaResult) PrettyDiags() string { + warningString := lipgloss.NewStyle().SetString("WARNING").Foreground(lipgloss.Color("#FFA500")) + errorString := lipgloss.NewStyle().SetString("ERROR").Foreground(lipgloss.Color("#FF0000")) + + output := "" + for _, diag := range r.Diags { + levelString := warningString + if diag.Level == Error { + levelString = errorString + } + output += fmt.Sprintf("Airlock %s: %s\n", levelString, diag.Message) + } + return output +} + +func (result *SchemaResult) PrettySchema() string { + if result.Schema == nil { + return "No schema available" + } + bytes, err := json.MarshalIndent(result.Schema, "", " ") + if err != nil { + return fmt.Sprintf("Error marshaling schema: %s", err) + } + return string(bytes) +} From 7a981bce9eab3c262da30225246fb57247abf657 Mon Sep 17 00:00:00 2001 From: chrisghill Date: Tue, 19 Aug 2025 15:46:26 -0600 Subject: [PATCH 5/9] lint --- pkg/result/print.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/result/print.go b/pkg/result/print.go index 6af21f3..ab0bf77 100644 --- a/pkg/result/print.go +++ b/pkg/result/print.go @@ -7,12 +7,12 @@ import ( "github.com/charmbracelet/lipgloss" ) -func (r *SchemaResult) PrettyDiags() string { +func (result *SchemaResult) PrettyDiags() string { warningString := lipgloss.NewStyle().SetString("WARNING").Foreground(lipgloss.Color("#FFA500")) errorString := lipgloss.NewStyle().SetString("ERROR").Foreground(lipgloss.Color("#FF0000")) output := "" - for _, diag := range r.Diags { + for _, diag := range result.Diags { levelString := warningString if diag.Level == Error { levelString = errorString From 61ee1762d1db96071b8767967bedbeeb688f8077 Mon Sep 17 00:00:00 2001 From: chrisghill Date: Tue, 19 Aug 2025 15:54:06 -0600 Subject: [PATCH 6/9] lint --- .golangci.yaml | 374 +++++++++++++------------------------ pkg/bicep/biceptoschema.go | 1 - pkg/bicep/schematobicep.go | 18 +- 3 files changed, 139 insertions(+), 254 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index f2b29ab..265756f 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,249 +1,135 @@ -## Golden config for golangci-lint v1.46.2 -# -# This is the best config for golangci-lint based on my experience and opinion. -# It is very strict, but not extremely strict. -# Feel free to adopt and change it for your needs. - -run: - # Timeout for analysis, e.g. 30s, 5m. - # Default: 1m - timeout: 3m - -# This file contains only configs which differ from defaults. -# All possible options can be found here https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml -linters-settings: - cyclop: - # The maximal code complexity to report. - # Default: 10 - max-complexity: 30 - # The maximal average package complexity. - # If it's higher than 0.0 (float) the check is enabled - # Default: 0.0 - package-average: 10.0 - - errcheck: - # Report about not checking of errors in type assertions: `a := b.(MyStruct)`. - # Such cases aren't reported by default. - # Default: false - check-type-assertions: true - - funlen: - # Checks the number of lines in a function. - # If lower than 0, disable the check. - # Default: 60 - lines: 100 - # Checks the number of statements in a function. - # If lower than 0, disable the check. - # Default: 40 - statements: 51 - - gocognit: - # Minimal code complexity to report - # Default: 30 (but we recommend 10-20) - min-complexity: 20 - - gocritic: - # Settings passed to gocritic. - # The settings key is the name of a supported gocritic checker. - # The list of supported checkers can be find in https://go-critic.github.io/overview. - settings: - captLocal: - # Whether to restrict checker to params only. - # Default: true - paramsOnly: false - underef: - # Whether to skip (*x).method() calls where x is a pointer receiver. - # Default: true - skipRecvDeref: false - - gomodguard: - blocked: - # List of blocked modules. - # Default: [] - modules: - - github.com/golang/protobuf: - recommendations: - - google.golang.org/protobuf - reason: "see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules" - - github.com/satori/go.uuid: - recommendations: - - github.com/google/uuid - reason: "satori's package is not maintained" - - github.com/gofrs/uuid: - recommendations: - - github.com/google/uuid - reason: "see recommendation from dev-infra team: https://confluence.gtforge.com/x/gQI6Aw" - - govet: - # Enable all analyzers. - # Default: false - enable-all: true - # Disable analyzers by name. - # Run `go tool vet help` to see all analyzers. - # Default: [] - disable: - - fieldalignment # too strict - # Settings per analyzer. - settings: - shadow: - # Whether to be strict about shadowing; can be noisy. - # Default: false - strict: true - - nakedret: - # Make an issue if func has more lines of code than this setting, and it has naked returns. - # Default: 30 - max-func-lines: 0 - - nolintlint: - # Exclude following linters from requiring an explanation. - # Default: [] - allow-no-explanation: [funlen, gocognit, lll] - # Enable to require an explanation of nonzero length after each nolint directive. - # Default: false - require-explanation: false - # Enable to require nolint directives to mention the specific linter being suppressed. - # Default: false - require-specific: true - - rowserrcheck: - # database/sql is always checked - # Default: [] - packages: - - github.com/jmoiron/sqlx - - tenv: - # The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures. - # Otherwise, only methods that take `*testing.T`, `*testing.B`, and `testing.TB` as arguments are checked. - # Default: false - all: true - +version: "2" linters: enable: + - asciicheck + - bidichk + - bodyclose + - contextcheck + - cyclop + - dupl + - durationcheck + - errname + - errorlint + - funlen + - gocognit + - gocritic + - gocyclo + - godox - gomoddirectives - - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases - - gosimple # Linter for Go source code that specializes in simplifying a code - - govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string - - ineffassign # Detects when assignments to existing variables are not used - - staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks - - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code - - unused # Checks Go code for unused constants, variables, functions and types - - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers - - bidichk # Checks for dangerous unicode character sequences - - bodyclose # checks whether HTTP response body is closed successfully - - contextcheck # check the function whether use a non-inherited context - - cyclop # checks function and package cyclomatic complexity - - dupl # Tool for code clone detection - - durationcheck # check for two durations multiplied together - - errname # Checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error. - - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. - # - execinquery # execinquery is a linter about query string checker in Query function which reads your Go src files and warning it finds - # - exhaustive # check exhaustiveness of enum switch statements - # - exportloopref # checks for pointers to enclosing loop variables (deprecated) - # - forbidigo # Forbids identifiers - - funlen # Tool for detection of long functions - # - gochecknoglobals # check that no global variables exist - # - gochecknoinits # Checks that no init functions are present in Go code - - gocognit # Computes and checks the cognitive complexity of functions - # - goconst # Finds repeated strings that could be replaced by a constant - - gocritic # Provides diagnostics that check for bugs, performance and style issues. - - gocyclo # Computes and checks the cyclomatic complexity of functions - # - godot # Check if comments end in a period - - goimports # In addition to fixing imports, goimports also formats your code in the same style as gofmt. - # - gomnd # An analyzer to detect magic numbers. - # - gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. - - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. - - goprintffuncname # Checks that printf-like functions are named with f at the end - - gosec # Inspects source code for security problems - # - lll # Reports long lines - - makezero # Finds slice declarations with non-zero initial length - - nakedret # Finds naked returns in functions greater than a specified function length - # - nestif # Reports deeply nested if statements - - nilerr # Finds the code that returns nil even if it checks that the error is not nil. - - nilnil # Checks that there is no simultaneous return of nil error and an invalid value. - - noctx # noctx finds sending http request without context.Context - - nolintlint # Reports ill-formed or insufficient nolint directives - # - nonamedreturns # Reports all named returns - # - nosprintfhostport # Checks for misuse of Sprintf to construct a host with port in a URL. - - predeclared # find code that shadows one of Go's predeclared identifiers - - promlinter # Check Prometheus metrics naming via promlint - # - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. - - rowserrcheck # checks whether Err of rows is checked successfully - - sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. - - stylecheck # Stylecheck is a replacement for golint - - tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 - - testpackage # linter that makes you use a separate _test package - - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes - - unconvert # Remove unnecessary type conversions - - unparam # Reports unused function parameters - - wastedassign # wastedassign finds wasted assignment statements. - - whitespace # Tool for detection of leading and trailing whitespace - ## you may want to enable - #- decorder # check declaration order and count of types, constants, variables and functions - #- exhaustruct # Checks if all structure fields are initialized - #- goheader # Checks is file header matches to pattern - #- ireturn # Accept Interfaces, Return Concrete Types - #- prealloc # [premature optimization, but can be used in some cases] Finds slice declarations that could potentially be preallocated - #- varnamelen # [great idea, but too many false positives] checks that the length of a variable's name matches its scope - #- wrapcheck # Checks that errors returned from external packages are wrapped - ## disabled - #- containedctx # containedctx is a linter that detects struct contained context.Context field - #- depguard # [replaced by gomodguard] Go linter that checks if package imports are in a list of acceptable packages - #- dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) - #- errchkjson # [don't see profit + I'm against of omitting errors like in the first example https://github.com/breml/errchkjson] Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted. - #- forcetypeassert # [replaced by errcheck] finds forced type assertions - #- gci # Gci controls golang package import order and makes it always deterministic. - - godox # Tool for detection of FIXME, TODO and other comment keywords - #- goerr113 # [too strict] Golang linter to check the errors handling expressions - #- gofmt # [replaced by goimports] Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification - #- gofumpt # [replaced by goimports, gofumports is not available yet] Gofumpt checks whether code was gofumpt-ed. - #- grouper # An analyzer to analyze expression groups. - #- ifshort # Checks that your code uses short syntax for if-statements whenever possible - #- importas # Enforces consistent import aliases - #- maintidx # maintidx measures the maintainability index of each function. - #- misspell # [useless] Finds commonly misspelled English words in comments - #- nlreturn # [too strict and mostly code is not more readable] nlreturn checks for a new line before return and branch statements to increase code clarity - #- paralleltest # [too many false positives] paralleltest detects missing usage of t.Parallel() method in your Go test - #- tagliatelle # Checks the struct tags. - #- thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers - #- wsl # [too strict and mostly code is not more readable] Whitespace Linter - Forces you to use empty lines! - ## deprecated - #- exhaustivestruct # [deprecated, replaced by exhaustruct] Checks if all struct's fields are initialized - #- golint # [deprecated, replaced by revive] Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes - #- interfacer # [deprecated] Linter that suggests narrower interface types - #- maligned # [deprecated, replaced by govet fieldalignment] Tool to detect Go structs that would take less memory if their fields were sorted - #- scopelint # [deprecated, replaced by exportloopref] Scopelint checks for unpinned variables in go programs - -issues: - # Maximum count of issues with the same text. - # Set to 0 to disable. - # Default: 3 - max-same-issues: 50 - - # exclude: - - exclude-rules: - # Allow unused params at the cobra command level - # - linters: [revive] - # text: "unused-parameter: parameter ('cmd'|'args') seems to be unused, consider removing or renaming it as _" - - source: "^//\\s*go:generate\\s" - linters: [lll] - # Allow TODO for now - - source: "(noinspection|TODO)" - linters: [godox] - - source: "//noinspection" - linters: [gocritic] - - source: "^\\s+if _, ok := err\\.\\([^.]+\\.InternalError\\); ok {" - linters: [errorlint] - - path: "_test\\.go" - linters: - # - revive - - bodyclose - - dupl + - gomodguard + - goprintffuncname + - gosec + - makezero + - nakedret + - nilerr + - nilnil + - noctx + - nolintlint + - predeclared + - promlinter + - rowserrcheck + - sqlclosecheck + - staticcheck + - testpackage + - tparallel + - unconvert + - unparam + - wastedassign + - whitespace + settings: + cyclop: + max-complexity: 30 + package-average: 10 + errcheck: + check-type-assertions: true + funlen: + lines: 100 + statements: 51 + gocognit: + min-complexity: 20 + gocritic: + settings: + captLocal: + paramsOnly: false + underef: + skipRecvDeref: false + gomodguard: + blocked: + modules: + - github.com/golang/protobuf: + recommendations: + - google.golang.org/protobuf + reason: see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules + - github.com/satori/go.uuid: + recommendations: + - github.com/google/uuid + reason: satori's package is not maintained + - github.com/gofrs/uuid: + recommendations: + - github.com/google/uuid + reason: 'see recommendation from dev-infra team: https://confluence.gtforge.com/x/gQI6Aw' + govet: + disable: + - fieldalignment + enable-all: true + settings: + shadow: + strict: true + nakedret: + max-func-lines: 0 + nolintlint: + require-explanation: false + require-specific: true + allow-no-explanation: - funlen - - goconst - - gosec - - noctx - - wrapcheck - gocognit - - cyclop + - lll + rowserrcheck: + packages: + - github.com/jmoiron/sqlx + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - lll + source: ^//\s*go:generate\s + - linters: + - godox + source: (noinspection|TODO) + - linters: + - gocritic + source: //noinspection + - linters: + - errorlint + source: ^\s+if _, ok := err\.\([^.]+\.InternalError\); ok { + - linters: + - bodyclose + - cyclop + - dupl + - funlen + - gocognit + - goconst + - gosec + - noctx + - wrapcheck + path: _test\.go + paths: + - third_party$ + - builtin$ + - examples$ +issues: + max-same-issues: 50 +formatters: + enable: + - goimports + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/pkg/bicep/biceptoschema.go b/pkg/bicep/biceptoschema.go index 2eb7ebd..3084260 100644 --- a/pkg/bicep/biceptoschema.go +++ b/pkg/bicep/biceptoschema.go @@ -142,7 +142,6 @@ func parseIntParam(sch *schema.Schema, bicepParam *bicepParam, diags []result.Di Level: result.Warning, }) } - } if bicepParam.MinValue != nil { diff --git a/pkg/bicep/schematobicep.go b/pkg/bicep/schematobicep.go index 05fb231..dd1a4a6 100644 --- a/pkg/bicep/schematobicep.go +++ b/pkg/bicep/schematobicep.go @@ -69,7 +69,7 @@ func writeBicepParam(name string, sch *schema.Schema, buf *bytes.Buffer, bicepTy defVal = fmt.Sprintf(" = %s", renderedVal) } - buf.WriteString(fmt.Sprintf("param %s %s%s\n", name, bicepType, defVal)) + fmt.Fprintf(buf, "param %s %s%s\n", name, bicepType, defVal) return nil } @@ -120,7 +120,7 @@ func getBicepTypeFromSchema(schemaType string) (string, error) { func writeDescription(sch *schema.Schema, buf *bytes.Buffer) { if sch.Description != "" { // decorators are in sys namespace. to avoid potential collision with other parameters named "description", we use "sys.description" instead of just "description" https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/parameters#decorators - buf.WriteString(fmt.Sprintf("@sys.description('%s')\n", sch.Description)) + fmt.Fprintf(buf, "@sys.description('%s')\n", sch.Description) } } @@ -131,7 +131,7 @@ func writeAllowedParams(sch *schema.Schema, buf *bytes.Buffer) error { return err } - buf.WriteString(fmt.Sprintf("@allowed(%s)\n", renderedVal)) + fmt.Fprintf(buf, "@allowed(%s)\n", renderedVal) } return nil } @@ -139,13 +139,13 @@ func writeAllowedParams(sch *schema.Schema, buf *bytes.Buffer) error { func writeMinValue(sch *schema.Schema, buf *bytes.Buffer, bicepType string) { if bicepType == "int" && sch.Minimum != "" { // set this to %v because sch.Minimum uses json.Number type - buf.WriteString(fmt.Sprintf("@minValue(%v)\n", sch.Minimum)) + fmt.Fprintf(buf, "@minValue(%v)\n", sch.Minimum) } } func writeMaxValue(sch *schema.Schema, buf *bytes.Buffer, bicepType string) { if bicepType == "int" && sch.Maximum != "" { - buf.WriteString(fmt.Sprintf("@maxValue(%v)\n", sch.Maximum)) + fmt.Fprintf(buf, "@maxValue(%v)\n", sch.Maximum) } } @@ -153,11 +153,11 @@ func writeMinLength(sch *schema.Schema, buf *bytes.Buffer, bicepType string) { switch bicepType { case "array": if sch.MinItems != nil { - buf.WriteString(fmt.Sprintf("@minLength(%d)\n", *sch.MinItems)) + fmt.Fprintf(buf, "@minLength(%d)\n", *sch.MinItems) } case "string": if sch.MinLength != nil { - buf.WriteString(fmt.Sprintf("@minLength(%d)\n", *sch.MinLength)) + fmt.Fprintf(buf, "@minLength(%d)\n", *sch.MinLength) } } } @@ -166,11 +166,11 @@ func writeMaxLength(sch *schema.Schema, buf *bytes.Buffer, bicepType string) { switch bicepType { case "array": if sch.MaxItems != nil { - buf.WriteString(fmt.Sprintf("@maxLength(%d)\n", *sch.MaxItems)) + fmt.Fprintf(buf, "@maxLength(%d)\n", *sch.MaxItems) } case "string": if sch.MaxLength != nil { - buf.WriteString(fmt.Sprintf("@maxLength(%d)\n", *sch.MaxLength)) + fmt.Fprintf(buf, "@maxLength(%d)\n", *sch.MaxLength) } } } From c42e8d21eeef9c252a9825a7bc7b67648f2a266d Mon Sep 17 00:00:00 2001 From: chrisghill Date: Tue, 19 Aug 2025 15:56:39 -0600 Subject: [PATCH 7/9] revert .golangci.yaml --- .golangci.yaml | 374 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 244 insertions(+), 130 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 265756f..97e8e49 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,135 +1,249 @@ -version: "2" +## Golden config for golangci-lint v1.46.2 +# +# This is the best config for golangci-lint based on my experience and opinion. +# It is very strict, but not extremely strict. +# Feel free to adopt and change it for your needs. + +run: + # Timeout for analysis, e.g. 30s, 5m. + # Default: 1m + timeout: 3m + +# This file contains only configs which differ from defaults. +# All possible options can be found here https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml +linters-settings: + cyclop: + # The maximal code complexity to report. + # Default: 10 + max-complexity: 30 + # The maximal average package complexity. + # If it's higher than 0.0 (float) the check is enabled + # Default: 0.0 + package-average: 10.0 + + errcheck: + # Report about not checking of errors in type assertions: `a := b.(MyStruct)`. + # Such cases aren't reported by default. + # Default: false + check-type-assertions: true + + funlen: + # Checks the number of lines in a function. + # If lower than 0, disable the check. + # Default: 60 + lines: 100 + # Checks the number of statements in a function. + # If lower than 0, disable the check. + # Default: 40 + statements: 51 + + gocognit: + # Minimal code complexity to report + # Default: 30 (but we recommend 10-20) + min-complexity: 20 + + gocritic: + # Settings passed to gocritic. + # The settings key is the name of a supported gocritic checker. + # The list of supported checkers can be find in https://go-critic.github.io/overview. + settings: + captLocal: + # Whether to restrict checker to params only. + # Default: true + paramsOnly: false + underef: + # Whether to skip (*x).method() calls where x is a pointer receiver. + # Default: true + skipRecvDeref: false + + gomodguard: + blocked: + # List of blocked modules. + # Default: [] + modules: + - github.com/golang/protobuf: + recommendations: + - google.golang.org/protobuf + reason: "see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules" + - github.com/satori/go.uuid: + recommendations: + - github.com/google/uuid + reason: "satori's package is not maintained" + - github.com/gofrs/uuid: + recommendations: + - github.com/google/uuid + reason: "see recommendation from dev-infra team: https://confluence.gtforge.com/x/gQI6Aw" + + govet: + # Enable all analyzers. + # Default: false + enable-all: true + # Disable analyzers by name. + # Run `go tool vet help` to see all analyzers. + # Default: [] + disable: + - fieldalignment # too strict + # Settings per analyzer. + settings: + shadow: + # Whether to be strict about shadowing; can be noisy. + # Default: false + strict: true + + nakedret: + # Make an issue if func has more lines of code than this setting, and it has naked returns. + # Default: 30 + max-func-lines: 0 + + nolintlint: + # Exclude following linters from requiring an explanation. + # Default: [] + allow-no-explanation: [funlen, gocognit, lll] + # Enable to require an explanation of nonzero length after each nolint directive. + # Default: false + require-explanation: false + # Enable to require nolint directives to mention the specific linter being suppressed. + # Default: false + require-specific: true + + rowserrcheck: + # database/sql is always checked + # Default: [] + packages: + - github.com/jmoiron/sqlx + + tenv: + # The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures. + # Otherwise, only methods that take `*testing.T`, `*testing.B`, and `testing.TB` as arguments are checked. + # Default: false + all: true + linters: enable: - - asciicheck - - bidichk - - bodyclose - - contextcheck - - cyclop - - dupl - - durationcheck - - errname - - errorlint - - funlen - - gocognit - - gocritic - - gocyclo - - godox - gomoddirectives - - gomodguard - - goprintffuncname - - gosec - - makezero - - nakedret - - nilerr - - nilnil - - noctx - - nolintlint - - predeclared - - promlinter - - rowserrcheck - - sqlclosecheck - - staticcheck - - testpackage - - tparallel - - unconvert - - unparam - - wastedassign - - whitespace - settings: - cyclop: - max-complexity: 30 - package-average: 10 - errcheck: - check-type-assertions: true - funlen: - lines: 100 - statements: 51 - gocognit: - min-complexity: 20 - gocritic: - settings: - captLocal: - paramsOnly: false - underef: - skipRecvDeref: false - gomodguard: - blocked: - modules: - - github.com/golang/protobuf: - recommendations: - - google.golang.org/protobuf - reason: see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules - - github.com/satori/go.uuid: - recommendations: - - github.com/google/uuid - reason: satori's package is not maintained - - github.com/gofrs/uuid: - recommendations: - - github.com/google/uuid - reason: 'see recommendation from dev-infra team: https://confluence.gtforge.com/x/gQI6Aw' - govet: - disable: - - fieldalignment - enable-all: true - settings: - shadow: - strict: true - nakedret: - max-func-lines: 0 - nolintlint: - require-explanation: false - require-specific: true - allow-no-explanation: - - funlen - - gocognit - - lll - rowserrcheck: - packages: - - github.com/jmoiron/sqlx - exclusions: - generated: lax - presets: - - comments - - common-false-positives - - legacy - - std-error-handling - rules: - - linters: - - lll - source: ^//\s*go:generate\s - - linters: - - godox - source: (noinspection|TODO) - - linters: - - gocritic - source: //noinspection - - linters: - - errorlint - source: ^\s+if _, ok := err\.\([^.]+\.InternalError\); ok { - - linters: - - bodyclose - - cyclop - - dupl - - funlen - - gocognit - - goconst - - gosec - - noctx - - wrapcheck - path: _test\.go - paths: - - third_party$ - - builtin$ - - examples$ + - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases + - gosimple # Linter for Go source code that specializes in simplifying a code + - govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + - ineffassign # Detects when assignments to existing variables are not used + - staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks + - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code + - unused # Checks Go code for unused constants, variables, functions and types + - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers + - bidichk # Checks for dangerous unicode character sequences + - bodyclose # checks whether HTTP response body is closed successfully + - contextcheck # check the function whether use a non-inherited context + - cyclop # checks function and package cyclomatic complexity + - dupl # Tool for code clone detection + - durationcheck # check for two durations multiplied together + - errname # Checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error. + - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. + # - execinquery # execinquery is a linter about query string checker in Query function which reads your Go src files and warning it finds + # - exhaustive # check exhaustiveness of enum switch statements + # - exportloopref # checks for pointers to enclosing loop variables (deprecated) + # - forbidigo # Forbids identifiers + - funlen # Tool for detection of long functions + # - gochecknoglobals # check that no global variables exist + # - gochecknoinits # Checks that no init functions are present in Go code + - gocognit # Computes and checks the cognitive complexity of functions + # - goconst # Finds repeated strings that could be replaced by a constant + - gocritic # Provides diagnostics that check for bugs, performance and style issues. + - gocyclo # Computes and checks the cyclomatic complexity of functions + # - godot # Check if comments end in a period + - goimports # In addition to fixing imports, goimports also formats your code in the same style as gofmt. + # - gomnd # An analyzer to detect magic numbers. + # - gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. + - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. + - goprintffuncname # Checks that printf-like functions are named with f at the end + - gosec # Inspects source code for security problems + # - lll # Reports long lines + - makezero # Finds slice declarations with non-zero initial length + - nakedret # Finds naked returns in functions greater than a specified function length + # - nestif # Reports deeply nested if statements + - nilerr # Finds the code that returns nil even if it checks that the error is not nil. + - nilnil # Checks that there is no simultaneous return of nil error and an invalid value. + - noctx # noctx finds sending http request without context.Context + - nolintlint # Reports ill-formed or insufficient nolint directives + # - nonamedreturns # Reports all named returns + # - nosprintfhostport # Checks for misuse of Sprintf to construct a host with port in a URL. + - predeclared # find code that shadows one of Go's predeclared identifiers + - promlinter # Check Prometheus metrics naming via promlint + # - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. + - rowserrcheck # checks whether Err of rows is checked successfully + - sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. + - stylecheck # Stylecheck is a replacement for golint + - tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 + - testpackage # linter that makes you use a separate _test package + - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes + - unconvert # Remove unnecessary type conversions + - unparam # Reports unused function parameters + - wastedassign # wastedassign finds wasted assignment statements. + - whitespace # Tool for detection of leading and trailing whitespace + ## you may want to enable + #- decorder # check declaration order and count of types, constants, variables and functions + #- exhaustruct # Checks if all structure fields are initialized + #- goheader # Checks is file header matches to pattern + #- ireturn # Accept Interfaces, Return Concrete Types + #- prealloc # [premature optimization, but can be used in some cases] Finds slice declarations that could potentially be preallocated + #- varnamelen # [great idea, but too many false positives] checks that the length of a variable's name matches its scope + #- wrapcheck # Checks that errors returned from external packages are wrapped + ## disabled + #- containedctx # containedctx is a linter that detects struct contained context.Context field + #- depguard # [replaced by gomodguard] Go linter that checks if package imports are in a list of acceptable packages + #- dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) + #- errchkjson # [don't see profit + I'm against of omitting errors like in the first example https://github.com/breml/errchkjson] Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted. + #- forcetypeassert # [replaced by errcheck] finds forced type assertions + #- gci # Gci controls golang package import order and makes it always deterministic. + - godox # Tool for detection of FIXME, TODO and other comment keywords + #- goerr113 # [too strict] Golang linter to check the errors handling expressions + #- gofmt # [replaced by goimports] Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification + #- gofumpt # [replaced by goimports, gofumports is not available yet] Gofumpt checks whether code was gofumpt-ed. + #- grouper # An analyzer to analyze expression groups. + #- ifshort # Checks that your code uses short syntax for if-statements whenever possible + #- importas # Enforces consistent import aliases + #- maintidx # maintidx measures the maintainability index of each function. + #- misspell # [useless] Finds commonly misspelled English words in comments + #- nlreturn # [too strict and mostly code is not more readable] nlreturn checks for a new line before return and branch statements to increase code clarity + #- paralleltest # [too many false positives] paralleltest detects missing usage of t.Parallel() method in your Go test + #- tagliatelle # Checks the struct tags. + #- thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers + #- wsl # [too strict and mostly code is not more readable] Whitespace Linter - Forces you to use empty lines! + ## deprecated + #- exhaustivestruct # [deprecated, replaced by exhaustruct] Checks if all struct's fields are initialized + #- golint # [deprecated, replaced by revive] Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes + #- interfacer # [deprecated] Linter that suggests narrower interface types + #- maligned # [deprecated, replaced by govet fieldalignment] Tool to detect Go structs that would take less memory if their fields were sorted + #- scopelint # [deprecated, replaced by exportloopref] Scopelint checks for unpinned variables in go programs + issues: + # Maximum count of issues with the same text. + # Set to 0 to disable. + # Default: 3 max-same-issues: 50 -formatters: - enable: - - goimports - exclusions: - generated: lax - paths: - - third_party$ - - builtin$ - - examples$ + + # exclude: + + exclude-rules: + # Allow unused params at the cobra command level + # - linters: [revive] + # text: "unused-parameter: parameter ('cmd'|'args') seems to be unused, consider removing or renaming it as _" + - source: "^//\\s*go:generate\\s" + linters: [lll] + # Allow TODO for now + - source: "(noinspection|TODO)" + linters: [godox] + - source: "//noinspection" + linters: [gocritic] + - source: "^\\s+if _, ok := err\\.\\([^.]+\\.InternalError\\); ok {" + linters: [errorlint] + - path: "_test\\.go" + linters: + # - revive + - bodyclose + - dupl + - funlen + - goconst + - gosec + - noctx + - wrapcheck + - gocognit + - cyclop \ No newline at end of file From 02d33d6b05aeadd95ab248cea1404aaaa70c95d6 Mon Sep 17 00:00:00 2001 From: chrisghill Date: Wed, 20 Aug 2025 11:36:28 -0600 Subject: [PATCH 8/9] remove pre-commit workflow (dead) --- .github/workflows/pre-commit.yaml | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 .github/workflows/pre-commit.yaml diff --git a/.github/workflows/pre-commit.yaml b/.github/workflows/pre-commit.yaml deleted file mode 100644 index 2d840ea..0000000 --- a/.github/workflows/pre-commit.yaml +++ /dev/null @@ -1,19 +0,0 @@ -name: pre-commit - -on: - pull_request: - push: - branches: [main] - -jobs: - pre-commit: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v2 - - uses: pre-commit/action@v2.0.3 - env: - # this check prevents devs from commit to main. - # however, we don't want it to fail on commits to main in CI. - # we use the golangci-lint gh action in lint.yaml because it generates useful comments. - SKIP: no-commit-to-branch,golangci-lint From c7b9f5a7a031d182f4e02551c1eeb68e40b6d2e8 Mon Sep 17 00:00:00 2001 From: chrisghill Date: Wed, 20 Aug 2025 12:08:25 -0600 Subject: [PATCH 9/9] clean up pretty logs --- pkg/prettylogs/main.go | 4 ++++ pkg/result/print.go | 9 +++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/prettylogs/main.go b/pkg/prettylogs/main.go index 974cb7c..9f6fb13 100644 --- a/pkg/prettylogs/main.go +++ b/pkg/prettylogs/main.go @@ -13,3 +13,7 @@ func Green(word string) lipgloss.Style { func Orange(word string) lipgloss.Style { return lipgloss.NewStyle().SetString(word).Foreground(lipgloss.Color("#FFA500")) } + +func Red(word string) lipgloss.Style { + return lipgloss.NewStyle().SetString(word).Foreground(lipgloss.Color("#FF0000")) +} diff --git a/pkg/result/print.go b/pkg/result/print.go index ab0bf77..edd0fa9 100644 --- a/pkg/result/print.go +++ b/pkg/result/print.go @@ -4,18 +4,15 @@ import ( "encoding/json" "fmt" - "github.com/charmbracelet/lipgloss" + "github.com/massdriver-cloud/airlock/pkg/prettylogs" ) func (result *SchemaResult) PrettyDiags() string { - warningString := lipgloss.NewStyle().SetString("WARNING").Foreground(lipgloss.Color("#FFA500")) - errorString := lipgloss.NewStyle().SetString("ERROR").Foreground(lipgloss.Color("#FF0000")) - output := "" for _, diag := range result.Diags { - levelString := warningString + levelString := prettylogs.Orange("WARNING") if diag.Level == Error { - levelString = errorString + levelString = prettylogs.Red("ERROR") } output += fmt.Sprintf("Airlock %s: %s\n", levelString, diag.Message) }