From 21c3a1c3f56bedcc5f6350eb7e104a0d7c35a535 Mon Sep 17 00:00:00 2001 From: Ruben Hoenle Date: Wed, 12 Aug 2026 18:38:22 +0200 Subject: [PATCH 1/2] feat(ci): add modify plan linter ensures all resources which have a region field implement the ModifyPlan method relates to #1684 --- .custom-gcl.yml | 5 + golang-ci.yaml | 4 + tools/linters/tfmodifyplan/tfmodifyplan.go | 121 +++++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 tools/linters/tfmodifyplan/tfmodifyplan.go diff --git a/.custom-gcl.yml b/.custom-gcl.yml index 8865366dd..e4baa11ad 100644 --- a/.custom-gcl.yml +++ b/.custom-gcl.yml @@ -12,3 +12,8 @@ plugins: - module: 'github.com/stackitcloud/terraform-provider-stackit/tools' import: 'github.com/stackitcloud/terraform-provider-stackit/tools/linters/tfwriteid' path: ./tools + - module: 'github.com/stackitcloud/terraform-provider-stackit/tools' + import: 'github.com/stackitcloud/terraform-provider-stackit/tools/linters/tfmodifyplan' + path: ./tools + # WARNING: when working with custom linting rules make sure to clear your cache when linting: + # golangci-lint cache clean && make lint diff --git a/golang-ci.yaml b/golang-ci.yaml index d65089321..1bb8b3d9d 100644 --- a/golang-ci.yaml +++ b/golang-ci.yaml @@ -13,6 +13,7 @@ linters: - tfctxinit # custom local linter - tflogresponse # custom local linter - tfwriteid # custom local linter + - tfmodifyplan # custom local linter - bodyclose - depguard - errorlint @@ -42,6 +43,9 @@ linters: tfwriteid: type: module description: "A custom local linter" + tfmodifyplan: + type: module + description: "A custom local linter" depguard: rules: main: diff --git a/tools/linters/tfmodifyplan/tfmodifyplan.go b/tools/linters/tfmodifyplan/tfmodifyplan.go new file mode 100644 index 000000000..cf48bf57c --- /dev/null +++ b/tools/linters/tfmodifyplan/tfmodifyplan.go @@ -0,0 +1,121 @@ +package tfmodifyplan + +import ( + "go/ast" + "go/token" + "go/types" + + "github.com/golangci/plugin-module-register/register" + "golang.org/x/tools/go/analysis" +) + +var Analyzer = &analysis.Analyzer{ + Name: "tfmodifyplan", + Doc: "Ensures every resource with a region field implements the ModifyPlan method.", + Run: run, +} + +func run(pass *analysis.Pass) (any, error) { + // Iterate over all parsed Go files in the package + for _, file := range pass.Files { + ast.Inspect(file, func(node ast.Node) bool { + // 1. Find all method declarations named "Schema" + fn, ok := node.(*ast.FuncDecl) + if !ok || fn.Name.Name != "Schema" || fn.Recv == nil || len(fn.Recv.List) == 0 { + return true + } + + // 2. Search the AST body of the Schema method for a "region" attribute key + hasRegionAttr := false + ast.Inspect(fn.Body, func(innerNode ast.Node) bool { + kv, ok := innerNode.(*ast.KeyValueExpr) + if !ok { + return true + } + + keyLit, ok := kv.Key.(*ast.BasicLit) + if ok && keyLit.Kind == token.STRING { + // String literals in the AST include their quotes, so we check both styles + if keyLit.Value == `"region"` || keyLit.Value == "`region`" { + hasRegionAttr = true + return false // Stop traversing this subtree, we found what we need + } + } + return true + }) + + if !hasRegionAttr { + return true + } + + // 3. Extract the receiver's underlying type name (e.g., `*MyResource` -> `MyResource`) + recvExpr := fn.Recv.List[0].Type + if star, ok := recvExpr.(*ast.StarExpr); ok { + recvExpr = star.X + } + + ident, ok := recvExpr.(*ast.Ident) + if !ok { + return true + } + + // 4. Resolve the AST identifier to its actual type representation via TypesInfo + obj := pass.TypesInfo.ObjectOf(ident) + if obj == nil { + return true + } + + named, ok := obj.Type().(*types.Named) + if !ok { + return true + } + + ptrType := types.NewPointer(named) + + // 5. Exclude Data Sources by ensuring the type implements 'Create'. + // (Resources have Create, Update, Delete. Data Sources only have Read). + if !hasMethod(named, "Create") && !hasMethod(ptrType, "Create") { + return true + } + + // 6. Check if the ModifyPlan method is present on the value or pointer receiver + if !hasMethod(named, "ModifyPlan") && !hasMethod(ptrType, "ModifyPlan") { + pass.Reportf(ident.Pos(), "'%s' defines a 'region' attribute in its Schema but does not implement the ModifyPlan method", ident.Name) + } + + return true + }) + } + + return nil, nil +} + +// hasMethod checks if a given type's method set contains a specific method name. +func hasMethod(t types.Type, methodName string) bool { + mset := types.NewMethodSet(t) + for method := range mset.Methods() { + if method.Obj().Name() == methodName { + return true + } + } + return false +} + +func init() { + register.Plugin("tfmodifyplan", New) +} + +func New(settings any) (register.LinterPlugin, error) { + return &plugin{}, nil +} + +type plugin struct{} + +func (p *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { + return []*analysis.Analyzer{Analyzer}, nil +} + +func (p *plugin) GetLoadMode() string { + // LoadModeSyntax is required because we need to inspect the AST (Syntax trees) + return register.LoadModeSyntax +} From f2fa31d512acfc052a556c5961a0972374514649 Mon Sep 17 00:00:00 2001 From: Ruben Hoenle Date: Thu, 13 Aug 2026 18:08:41 +0200 Subject: [PATCH 2/2] ensure utils.AdaptRegion is called in ModifyPlan implementations --- tools/linters/tfmodifyplan/tfmodifyplan.go | 192 +++++++++++++-------- 1 file changed, 123 insertions(+), 69 deletions(-) diff --git a/tools/linters/tfmodifyplan/tfmodifyplan.go b/tools/linters/tfmodifyplan/tfmodifyplan.go index cf48bf57c..7cfce5a4d 100644 --- a/tools/linters/tfmodifyplan/tfmodifyplan.go +++ b/tools/linters/tfmodifyplan/tfmodifyplan.go @@ -5,100 +5,154 @@ import ( "go/token" "go/types" - "github.com/golangci/plugin-module-register/register" "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + + "github.com/golangci/plugin-module-register/register" ) var Analyzer = &analysis.Analyzer{ - Name: "tfmodifyplan", - Doc: "Ensures every resource with a region field implements the ModifyPlan method.", - Run: run, + Name: "tfmodifyplan", + Doc: "Ensures every resource with a region field implements the ModifyPlan method.", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, } -func run(pass *analysis.Pass) (any, error) { - // Iterate over all parsed Go files in the package - for _, file := range pass.Files { - ast.Inspect(file, func(node ast.Node) bool { - // 1. Find all method declarations named "Schema" - fn, ok := node.(*ast.FuncDecl) - if !ok || fn.Name.Name != "Schema" || fn.Recv == nil || len(fn.Recv.List) == 0 { - return true - } +const ( + targetPkgName = "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" + targetFuncName = "AdaptRegion" +) - // 2. Search the AST body of the Schema method for a "region" attribute key - hasRegionAttr := false - ast.Inspect(fn.Body, func(innerNode ast.Node) bool { - kv, ok := innerNode.(*ast.KeyValueExpr) - if !ok { - return true - } +func run(pass *analysis.Pass) (interface{}, error) { + ins := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) - keyLit, ok := kv.Key.(*ast.BasicLit) - if ok && keyLit.Kind == token.STRING { - // String literals in the AST include their quotes, so we check both styles - if keyLit.Value == `"region"` || keyLit.Value == "`region`" { - hasRegionAttr = true - return false // Stop traversing this subtree, we found what we need - } - } - return true - }) + // structData keeps track of methods attached to a struct receiver + type structData struct { + schemaMethod *ast.FuncDecl + modifyPlanMethod *ast.FuncDecl + isResource bool // True if it has Create, Update, or Delete methods + } - if !hasRegionAttr { - return true - } + resources := make(map[string]*structData) - // 3. Extract the receiver's underlying type name (e.g., `*MyResource` -> `MyResource`) - recvExpr := fn.Recv.List[0].Type - if star, ok := recvExpr.(*ast.StarExpr); ok { - recvExpr = star.X - } + // Filter down to only function declarations + nodeFilter := []ast.Node{ + (*ast.FuncDecl)(nil), + } - ident, ok := recvExpr.(*ast.Ident) - if !ok { - return true - } + // Pass 1: Collect and group all methods by their receiver struct + ins.Preorder(nodeFilter, func(n ast.Node) { + fn := n.(*ast.FuncDecl) - // 4. Resolve the AST identifier to its actual type representation via TypesInfo - obj := pass.TypesInfo.ObjectOf(ident) - if obj == nil { - return true - } + // Skip if it doesn't have a receiver (not a struct method) + if fn.Recv == nil || len(fn.Recv.List) == 0 { + return + } - named, ok := obj.Type().(*types.Named) - if !ok { - return true + // Extract receiver struct name + var recvName string + switch t := fn.Recv.List[0].Type.(type) { + case *ast.StarExpr: // Pointer receiver: *MyResource + if ident, ok := t.X.(*ast.Ident); ok { + recvName = ident.Name } + case *ast.Ident: // Value receiver: MyResource + recvName = t.Name + } - ptrType := types.NewPointer(named) + if recvName == "" { + return + } - // 5. Exclude Data Sources by ensuring the type implements 'Create'. - // (Resources have Create, Update, Delete. Data Sources only have Read). - if !hasMethod(named, "Create") && !hasMethod(ptrType, "Create") { - return true - } + if resources[recvName] == nil { + resources[recvName] = &structData{} + } - // 6. Check if the ModifyPlan method is present on the value or pointer receiver - if !hasMethod(named, "ModifyPlan") && !hasMethod(ptrType, "ModifyPlan") { - pass.Reportf(ident.Pos(), "'%s' defines a 'region' attribute in its Schema but does not implement the ModifyPlan method", ident.Name) - } + // Identify the role of the method + switch fn.Name.Name { + case "Schema": + resources[recvName].schemaMethod = fn + case "ModifyPlan": + resources[recvName].modifyPlanMethod = fn + case "Create", "Update", "Delete": + // Data sources do not have Create/Update/Delete in the TF Plugin Framework. + // This heuristic guarantees we are looking at a Resource. + resources[recvName].isResource = true + } + }) + // Pass 2: Analyze the collected data against your business logic rules + for recvName, data := range resources { + // Only analyze valid TF Resources that have a Schema method + if !data.isResource || data.schemaMethod == nil { + continue + } + + // Check if the string "region" is defined anywhere in the Schema method + hasRegion := false + ast.Inspect(data.schemaMethod, func(n ast.Node) bool { + if lit, ok := n.(*ast.BasicLit); ok { + if lit.Kind == token.STRING && lit.Value == `"region"` { + hasRegion = true + return false // Found it, stop walking this branch + } + } return true }) - } - return nil, nil -} + // If it doesn't have a region attribute, skip it. + if !hasRegion { + continue + } -// hasMethod checks if a given type's method set contains a specific method name. -func hasMethod(t types.Type, methodName string) bool { - mset := types.NewMethodSet(t) - for method := range mset.Methods() { - if method.Obj().Name() == methodName { + // Has region, but no ModifyPlan method + if data.modifyPlanMethod == nil { + pass.Reportf( + data.schemaMethod.Pos(), + "Terraform resource '%s' defines a 'region' field but does not implement the ModifyPlan method.", + recvName, + ) + continue + } + + // Check if the specific function is called inside ModifyPlan + hasRequiredFuncCall := false + ast.Inspect(data.modifyPlanMethod, func(n ast.Node) bool { + if call, ok := n.(*ast.CallExpr); ok { + if sel, ok := call.Fun.(*ast.SelectorExpr); ok { + // Check if the method being called matches our target function name + if sel.Sel.Name == targetFuncName { + if ident, ok := sel.X.(*ast.Ident); ok { + // Use type info to resolve the identifier to its actual package + obj := pass.TypesInfo.Uses[ident] + if pkgName, ok := obj.(*types.PkgName); ok { + // Compare the actual import path + if pkgName.Imported().Path() == targetPkgName { + hasRequiredFuncCall = true + return false // Found it, stop walking this branch + } + } + } + } + } + } return true + }) + + // Has ModifyPlan, but missing the required package/function call + if !hasRequiredFuncCall { + pass.Reportf( + data.modifyPlanMethod.Pos(), + "Terraform resource '%s' ModifyPlan method must call %s.%s().", + recvName, + targetPkgName, + targetFuncName, + ) } } - return false + + return nil, nil } func init() {