From 906d2016b4aebba39d18de7a89d682994d59bad7 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Tue, 23 Sep 2025 13:51:08 +0100 Subject: [PATCH 1/5] feat: azapi rules --- .../conventional-commit.instructions.md | 73 +++++ .github/instructions/go.instructions.md | 292 ++++++++++++++++++ .vscode/mcp.json | 28 ++ README.md | 18 ++ RULES.md | 35 +++ cmd/rulesdoc/main.go | 85 +++++ common/either.go | 35 ++- common/either_test.go | 30 +- generate.go | 7 + outputs/no_entire_resource_output.go | 176 +++++++++++ outputs/no_entire_resource_output_test.go | 157 ++++++++++ outputs/outputs.go | 1 + rules/disallowed_provider.go | 106 +++++++ rules/disallowed_provider_test.go | 73 +++++ rules/required_attribute.go | 149 +++++++++ rules/required_attribute_test.go | 68 ++++ rules/rule_register.go | 31 ++ 17 files changed, 1338 insertions(+), 26 deletions(-) create mode 100644 .github/instructions/conventional-commit.instructions.md create mode 100644 .github/instructions/go.instructions.md create mode 100644 .vscode/mcp.json create mode 100644 RULES.md create mode 100644 cmd/rulesdoc/main.go create mode 100644 generate.go create mode 100644 outputs/no_entire_resource_output.go create mode 100644 outputs/no_entire_resource_output_test.go create mode 100644 rules/disallowed_provider.go create mode 100644 rules/disallowed_provider_test.go create mode 100644 rules/required_attribute.go create mode 100644 rules/required_attribute_test.go diff --git a/.github/instructions/conventional-commit.instructions.md b/.github/instructions/conventional-commit.instructions.md new file mode 100644 index 0000000..a5e056f --- /dev/null +++ b/.github/instructions/conventional-commit.instructions.md @@ -0,0 +1,73 @@ +--- +description: 'Prompt and workflow for generating conventional commit messages using a structured XML format. Guides users to create standardized, descriptive commit messages in line with the Conventional Commits specification, including instructions, examples, and validation.' +tools: ['run_in_terminal', 'get_terminal_output'] +--- + +### Instructions + +```xml + This file contains a prompt template for generating conventional commit messages. It provides instructions, examples, and formatting guidelines to help users write standardized, descriptive commit messages in accordance with the Conventional Commits specification. + +``` + +### Workflow + +**Follow these steps:** + +1. Run `git status` to review changed files. +2. Run `git diff` or `git diff --cached` to inspect changes. +3. Stage your changes with `git add `. +4. Construct your commit message using the following XML structure. +5. After generating your commit message, Copilot will automatically run the following command in your integrated terminal (no confirmation needed): + +```bash +git commit -m "type(scope): description" +``` + +6. Just execute this prompt and Copilot will handle the commit for you in the terminal. + +### Commit Message Structure + +```xml + + feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert + () + A short, imperative summary of the change + (optional: more detailed explanation) +
(optional: e.g. BREAKING CHANGE: details, or issue references)
+ +``` + +### Examples + +```xml + + feat(parser): add ability to parse arrays + fix(ui): correct button alignment + docs: update README with usage instructions + refactor: improve performance of data processing + chore: update dependencies + feat!: send email on registration (BREAKING CHANGE: email service required) + +``` + +### Validation + +```xml + + Must be one of the allowed types. See https://www.conventionalcommits.org/en/v1.0.0/#specification + Optional, but recommended for clarity. + Required. Use the imperative mood (e.g., "add", not "added"). + Optional. Use for additional context. +
Use for breaking changes or issue references.
+ +``` + +### Final Step + +```xml + + git commit -m "type(scope): description" + Replace with your constructed message. Include body and footer if needed. + +``` diff --git a/.github/instructions/go.instructions.md b/.github/instructions/go.instructions.md new file mode 100644 index 0000000..ce4cdba --- /dev/null +++ b/.github/instructions/go.instructions.md @@ -0,0 +1,292 @@ +--- +description: 'Instructions for writing Go code following idiomatic Go practices and community standards' +applyTo: '**/*.go,**/go.mod,**/go.sum' +--- + +# Go Development Instructions + +Follow idiomatic Go practices and community standards when writing Go code. These instructions are based on [Effective Go](https://go.dev/doc/effective_go), [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments), and [Google's Go Style Guide](https://google.github.io/styleguide/go/). + +## General Instructions + +- Write simple, clear, and idiomatic Go code +- Favor clarity and simplicity over cleverness +- Follow the principle of least surprise +- Keep the happy path left-aligned (minimize indentation) +- Return early to reduce nesting +- Make the zero value useful +- Document exported types, functions, methods, and packages +- Use Go modules for dependency management + +## Naming Conventions + +### Packages + +- Use lowercase, single-word package names +- Avoid underscores, hyphens, or mixedCaps +- Choose names that describe what the package provides, not what it contains +- Avoid generic names like `util`, `common`, or `base` +- Package names should be singular, not plural + +### Variables and Functions + +- Use mixedCaps or MixedCaps (camelCase) rather than underscores +- Keep names short but descriptive +- Use single-letter variables only for very short scopes (like loop indices) +- Exported names start with a capital letter +- Unexported names start with a lowercase letter +- Avoid stuttering (e.g., avoid `http.HTTPServer`, prefer `http.Server`) + +### Interfaces + +- Name interfaces with -er suffix when possible (e.g., `Reader`, `Writer`, `Formatter`) +- Single-method interfaces should be named after the method (e.g., `Read` → `Reader`) +- Keep interfaces small and focused + +### Constants + +- Use MixedCaps for exported constants +- Use mixedCaps for unexported constants +- Group related constants using `const` blocks +- Consider using typed constants for better type safety + +## Code Style and Formatting + +### Formatting + +- Always use `gofmt` to format code +- Use `goimports` to manage imports automatically +- Keep line length reasonable (no hard limit, but consider readability) +- Add blank lines to separate logical groups of code + +### Comments + +- Write comments in complete sentences +- Start sentences with the name of the thing being described +- Package comments should start with "Package [name]" +- Use line comments (`//`) for most comments +- Use block comments (`/* */`) sparingly, mainly for package documentation +- Document why, not what, unless the what is complex + +### Error Handling + +- Check errors immediately after the function call +- Don't ignore errors using `_` unless you have a good reason (document why) +- Wrap errors with context using `fmt.Errorf` with `%w` verb +- Create custom error types when you need to check for specific errors +- Place error returns as the last return value +- Name error variables `err` +- Keep error messages lowercase and don't end with punctuation + +## Architecture and Project Structure + +### Package Organization + +- Follow standard Go project layout conventions +- Keep `main` packages in `cmd/` directory +- Put reusable packages in `pkg/` or `internal/` +- Use `internal/` for packages that shouldn't be imported by external projects +- Group related functionality into packages +- Avoid circular dependencies + +### Dependency Management + +- Use Go modules (`go.mod` and `go.sum`) +- Keep dependencies minimal +- Regularly update dependencies for security patches +- Use `go mod tidy` to clean up unused dependencies +- Vendor dependencies only when necessary + +## Type Safety and Language Features + +### Type Definitions + +- Define types to add meaning and type safety +- Use struct tags for JSON, XML, database mappings +- Prefer explicit type conversions +- Use type assertions carefully and check the second return value + +### Pointers vs Values + +- Use pointers for large structs or when you need to modify the receiver +- Use values for small structs and when immutability is desired +- Be consistent within a type's method set +- Consider the zero value when choosing pointer vs value receivers + +### Interfaces and Composition + +- Accept interfaces, return concrete types +- Keep interfaces small (1-3 methods is ideal) +- Use embedding for composition +- Define interfaces close to where they're used, not where they're implemented +- Don't export interfaces unless necessary + +## Concurrency + +### Goroutines + +- Don't create goroutines in libraries; let the caller control concurrency +- Always know how a goroutine will exit +- Use `sync.WaitGroup` or channels to wait for goroutines +- Avoid goroutine leaks by ensuring cleanup + +### Channels + +- Use channels to communicate between goroutines +- Don't communicate by sharing memory; share memory by communicating +- Close channels from the sender side, not the receiver +- Use buffered channels when you know the capacity +- Use `select` for non-blocking operations + +### Synchronization + +- Use `sync.Mutex` for protecting shared state +- Keep critical sections small +- Use `sync.RWMutex` when you have many readers +- Prefer channels over mutexes when possible +- Use `sync.Once` for one-time initialization + +## Error Handling Patterns + +### Creating Errors + +- Use `errors.New` for simple static errors +- Use `fmt.Errorf` for dynamic errors +- Create custom error types for domain-specific errors +- Export error variables for sentinel errors +- Use `errors.Is` and `errors.As` for error checking + +### Error Propagation + +- Add context when propagating errors up the stack +- Don't log and return errors (choose one) +- Handle errors at the appropriate level +- Consider using structured errors for better debugging + +## API Design + +### HTTP Handlers + +- Use `http.HandlerFunc` for simple handlers +- Implement `http.Handler` for handlers that need state +- Use middleware for cross-cutting concerns +- Set appropriate status codes and headers +- Handle errors gracefully and return appropriate error responses + +### JSON APIs + +- Use struct tags to control JSON marshaling +- Validate input data +- Use pointers for optional fields +- Consider using `json.RawMessage` for delayed parsing +- Handle JSON errors appropriately + +## Performance Optimization + +### Memory Management + +- Minimize allocations in hot paths +- Reuse objects when possible (consider `sync.Pool`) +- Use value receivers for small structs +- Preallocate slices when size is known +- Avoid unnecessary string conversions + +### Profiling + +- Use built-in profiling tools (`pprof`) +- Benchmark critical code paths +- Profile before optimizing +- Focus on algorithmic improvements first +- Consider using `testing.B` for benchmarks + +## Testing + +### Test Organization + +- Keep tests in the same package (white-box testing) +- Use `_test` package suffix for black-box testing +- Name test files with `_test.go` suffix +- Place test files next to the code they test + +### Writing Tests + +- Use table-driven tests for multiple test cases +- Name tests descriptively using `Test_functionName_scenario` +- Use subtests with `t.Run` for better organization +- Test both success and error cases +- Use `testify` or similar libraries sparingly + +### Test Helpers + +- Mark helper functions with `t.Helper()` +- Create test fixtures for complex setup +- Use `testing.TB` interface for functions used in tests and benchmarks +- Clean up resources using `t.Cleanup()` + +## Security Best Practices + +### Input Validation + +- Validate all external input +- Use strong typing to prevent invalid states +- Sanitize data before using in SQL queries +- Be careful with file paths from user input +- Validate and escape data for different contexts (HTML, SQL, shell) + +### Cryptography + +- Use standard library crypto packages +- Don't implement your own cryptography +- Use crypto/rand for random number generation +- Store passwords using bcrypt or similar +- Use TLS for network communication + +## Documentation + +### Code Documentation + +- Document all exported symbols +- Start documentation with the symbol name +- Use examples in documentation when helpful +- Keep documentation close to code +- Update documentation when code changes + +### README and Documentation Files + +- Include clear setup instructions +- Document dependencies and requirements +- Provide usage examples +- Document configuration options +- Include troubleshooting section + +## Tools and Development Workflow + +### Essential Tools + +- `go fmt`: Format code +- `go vet`: Find suspicious constructs +- `golint` or `golangci-lint`: Additional linting +- `go test`: Run tests +- `go mod`: Manage dependencies +- `go generate`: Code generation + +### Development Practices + +- Run tests before committing +- Use pre-commit hooks for formatting and linting +- Keep commits focused and atomic +- Write meaningful commit messages +- Review diffs before committing + +## Common Pitfalls to Avoid + +- Not checking errors +- Ignoring race conditions +- Creating goroutine leaks +- Not using defer for cleanup +- Modifying maps concurrently +- Not understanding nil interfaces vs nil pointers +- Forgetting to close resources (files, connections) +- Using global variables unnecessarily +- Over-using empty interfaces (`interface{}`) +- Not considering the zero value of types diff --git a/.vscode/mcp.json b/.vscode/mcp.json new file mode 100644 index 0000000..bff38cf --- /dev/null +++ b/.vscode/mcp.json @@ -0,0 +1,28 @@ +{ + "servers": { + "godoc": { + "command": "godoc-mcp", + "args": [], + "env": {}, + }, + "terraform-mcp-eva": { + "type": "stdio", + "command": "docker", + "args": [ + "run", + "-i", + "--rm", + "-v", + "${workspaceFolder}:/workspace", + "-w", + "/workspace", + "-e", + "TRANSPORT_MODE=stdio", + "-e", + "GITHUB_TOKEN", + "--pull=always", + "ghcr.io/lonegunmanb/terraform-mcp-eva" + ] + } + } +} diff --git a/README.md b/README.md index 7d8b857..cd5d6bf 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,24 @@ ALGvKXPLwggdNA86RIQc9tc3z/uJrBGSA2n6UkJbV1gFZDETjHzVtgDqqEQwap7D > *TBC* +### Regenerating the Rules Documentation + +The full list of rules is generated automatically into `RULES.md` using a small helper program. + +To regenerate after adding, removing, or modifying rules: + +```bash +go generate ./... +``` + +This runs the generator in `cmd/rulesdoc` (triggered by the `go:generate` directive in `generate.go`) which: + +1. Loads the registered rules from `rules.Rules`. +2. Sorts them by name for deterministic output. +3. Writes a markdown table to `RULES.md` at the repository root. + +Please commit the updated `RULES.md` alongside any rule changes. + ## Building the plugin Clone the repository locally and run the following command: diff --git a/RULES.md b/RULES.md new file mode 100644 index 0000000..f1f6245 --- /dev/null +++ b/RULES.md @@ -0,0 +1,35 @@ +# Rules Reference + +This document lists all rules currently registered in this ruleset. The Enabled column reflects the default state (some external rules are wrapped to be disabled by default). + +| Name | Enabled | Severity | Link | +| ---- | ------- | -------- | ---- | +| azapi_data_response_export_values | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/a...](https://azure.github.io/Azure-Verified-Modules/specs/tf/azapi/#response_export_values-required) | +| azapi_replace_triggers_refs | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/a...](https://azure.github.io/Azure-Verified-Modules/specs/tf/azapi/#replace_triggers_refs) | +| azapi_response_export_values | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/a...](https://azure.github.io/Azure-Verified-Modules/specs/tf/azapi/#response_export_values-required) | +| azurerm_resource_tag | false | NOTICE | [https://github.com/Azure/tflint-ruleset-azurerm-ext/blob/...](https://github.com/Azure/tflint-ruleset-azurerm-ext/blob/v0.6.0/docs/rules/azurerm_resource_tag.md) | +| customer_managed_key | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/shar...](https://azure.github.io/Azure-Verified-Modules/specs/shared/interfaces/#customer-managed-keys) | +| diagnostic_settings | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/i...](https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#diagnostic-settings) | +| location | true | ERROR | - | +| lock | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/i...](https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#resource-locks) | +| managed_identities | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/i...](https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#managed-identities) | +| no_entire_resource_output_tffr2 | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/r...](https://azure.github.io/Azure-Verified-Modules/specs/tf/res/#id-tffr2---category-outputs---additional-terraform-outputs) | +| private_endpoints | true | ERROR | - | +| provider_azapi_version_constraint | true | ERROR | - | +| provider_azurerm_disallowed | true | ERROR | - | +| provider_azurerm_version_constraint | true | ERROR | - | +| provider_modtm_version_constraint | true | ERROR | - | +| required_module_source_tffr1 | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/terr...](https://azure.github.io/Azure-Verified-Modules/specs/terraform/#id-tffr1---category-composition---cross-referencing-modules) | +| required_module_source_tfnfr10 | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/terr...](https://azure.github.io/Azure-Verified-Modules/specs/terraform/#id-tfnfr10---category-code-style---no-double-quotes-in-ignore_changes) | +| required_output_rmfr7 | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/shar...](https://azure.github.io/Azure-Verified-Modules/specs/shared/#id-rmfr7---category-outputs---minimum-required-outputs) | +| role_assignments | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/i...](https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#role-assignments) | +| tags | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/i...](https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#tags) | +| terraform_heredoc_usage | false | NOTICE | - | +| terraform_module_provider_declaration | false | WARNING | - | +| terraform_output_separate | false | NOTICE | [https://github.com/Azure/tflint-ruleset-basic-ext/blob/v0...](https://github.com/Azure/tflint-ruleset-basic-ext/blob/v0.6.0/docs/rules/terraform_output_separate.md) | +| terraform_required_providers_declaration | false | NOTICE | - | +| terraform_required_version_declaration | false | NOTICE | - | +| terraform_sensitive_variable_no_default | false | WARNING | - | +| terraform_variable_nullable_false | false | NOTICE | [https://github.com/Azure/tflint-ruleset-basic-ext/blob/v0...](https://github.com/Azure/tflint-ruleset-basic-ext/blob/v0.6.0/docs/rules/terraform_variable_nullable_false.md) | +| terraform_variable_separate | false | NOTICE | [https://github.com/Azure/tflint-ruleset-basic-ext/blob/v0...](https://github.com/Azure/tflint-ruleset-basic-ext/blob/v0.6.0/docs/rules/terraform_variable_separate.md) | +| tfnfr26 | false | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/terr...](https://azure.github.io/Azure-Verified-Modules/specs/terraform/#id-tfnfr26---category-code-style---providers-must-be-declared-in-the-required_providers-block-in-terraformtf-and-must-have-a-constraint-on-minimum-and-maximum-major-version) | diff --git a/cmd/rulesdoc/main.go b/cmd/rulesdoc/main.go new file mode 100644 index 0000000..8a4cc4d --- /dev/null +++ b/cmd/rulesdoc/main.go @@ -0,0 +1,85 @@ +package main + +import ( + "bufio" + "fmt" + "os" + "sort" + "strings" + + "github.com/Azure/tflint-ruleset-avm/rules" + "github.com/terraform-linters/tflint-plugin-sdk/tflint" +) + +// generator collects rule metadata from the central rule register and writes +// a markdown table to RULES.md in the repo root. Intended to be invoked via +// `go generate` (see generate.go in the root package). +func main() { + // Copy slice so we can sort without mutating original backing array (defensive) + rs := append([]tflint.Rule(nil), rules.Rules...) + + // Sort by rule.Name() ascending (stable for deterministic output) + sort.SliceStable(rs, func(i, j int) bool { return rs[i].Name() < rs[j].Name() }) + + f, err := os.Create("RULES.md") + if err != nil { + panic(err) + } + defer f.Close() + + w := bufio.NewWriter(f) + defer w.Flush() + + fmt.Fprintln(w, "# Rules Reference") + fmt.Fprintln(w) + fmt.Fprintln(w, "This document lists all rules currently registered in this ruleset. The Enabled column reflects the default state (some external rules are wrapped to be disabled by default).") + fmt.Fprintln(w) + fmt.Fprintln(w, "| Name | Enabled | Severity | Link |") + fmt.Fprintln(w, "| ---- | ------- | -------- | ---- |") + + for _, r := range rs { + name := r.Name() + enabled := "false" + if r.Enabled() { // default state + enabled = "true" + } + // Normalize severity textual representation. + severity := severityString(r.Severity()) + link := linkOrDash(r) + fmt.Fprintf(w, "| %s | %s | %s | %s |\n", name, enabled, severity, link) + } +} + +func linkOrDash(r tflint.Rule) string { + // Link() may return empty string. Some rules may not implement Link(); rely on interface existence. + // Detect empty and return '-'. Escape pipes by replacing with %7C (very unlikely). + l := r.Link() + if l == "" { + return "-" + } + // Prevent table breakage (simple sanitization) + l = strings.ReplaceAll(l, "|", "%7C") + return fmt.Sprintf("[%s](%s)", displayLinkText(l), l) +} + +func displayLinkText(l string) string { + // Show domain/path tail for brevity + if len(l) > 60 { + return l[:57] + "..." + } + return l +} + +func severityString(s tflint.Severity) string { + // Mirror tflint severities if possible; fallback to numeric. + switch s { + case tflint.ERROR: + return "ERROR" + case tflint.WARNING: + return "WARNING" + case tflint.NOTICE: + return "NOTICE" + default: + return fmt.Sprintf("%d", s) + } +} diff --git a/common/either.go b/common/either.go index 0f7c758..f6610a2 100644 --- a/common/either.go +++ b/common/either.go @@ -8,20 +8,18 @@ var _ tflint.Rule = new(EitherCheckRule) type EitherCheckRule struct { tflint.DefaultRule - primaryRule tflint.Rule - secondaryRule tflint.Rule - name string - enabled bool - severity tflint.Severity + rules []tflint.Rule + name string + enabled bool + severity tflint.Severity } -func NewEitherCheckRule(name string, enabled bool, severity tflint.Severity, primaryRule tflint.Rule, secondary tflint.Rule) *EitherCheckRule { +func NewEitherCheckRule(name string, enabled bool, severity tflint.Severity, rules ...tflint.Rule) *EitherCheckRule { return &EitherCheckRule{ - name: name, - enabled: enabled, - severity: severity, - primaryRule: primaryRule, - secondaryRule: secondary, + name: name, + enabled: enabled, + severity: severity, + rules: rules, } } @@ -40,7 +38,9 @@ func (e *EitherCheckRule) Severity() tflint.Severity { func (e *EitherCheckRule) Check(runner tflint.Runner) error { runners := map[tflint.Rule]*subRunner{} - for _, r := range []tflint.Rule{e.primaryRule, e.secondaryRule} { + issues := []issue{} + var failingRule tflint.Rule + for _, r := range e.rules { sr := &subRunner{ Runner: runner, } @@ -51,12 +51,19 @@ func (e *EitherCheckRule) Check(runner tflint.Runner) error { if len(sr.issues) == 0 { return nil } + + if len(issues) == 0 && failingRule == nil { + issues = sr.issues + failingRule = r + } } - sr := runners[e.primaryRule] + + sr := runners[e.rules[0]] for _, issue := range sr.issues { - if err := runner.EmitIssue(e.primaryRule, issue.message, issue.issueRange); err != nil { + if err := runner.EmitIssue(failingRule, issue.message, issue.issueRange); err != nil { return err } } + return nil } diff --git a/common/either_test.go b/common/either_test.go index cc48b7c..ac36114 100644 --- a/common/either_test.go +++ b/common/either_test.go @@ -1,13 +1,14 @@ package common_test import ( + "testing" + "github.com/Azure/tflint-ruleset-avm/common" "github.com/hashicorp/hcl/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/terraform-linters/tflint-plugin-sdk/helper" "github.com/terraform-linters/tflint-plugin-sdk/tflint" - "testing" ) var _ tflint.Rule = &mockRule{} @@ -43,34 +44,39 @@ func (m *mockRule) Link() string { func TestEitherPrivateEndpoints(t *testing.T) { cases := []struct { name string - rule1 tflint.Rule - rule2 tflint.Rule + rules []tflint.Rule expectedIssue bool }{ { name: "correct", - rule1: &mockRule{success: true}, - rule2: &mockRule{success: true}, + rules: []tflint.Rule{&mockRule{success: true}, &mockRule{success: true}}, expectedIssue: false, }, { name: "correct2", - rule1: &mockRule{success: true}, - rule2: &mockRule{success: false}, + rules: []tflint.Rule{&mockRule{success: true}, &mockRule{success: false}}, expectedIssue: false, }, { name: "correct3", - rule1: &mockRule{success: false}, - rule2: &mockRule{success: true}, + rules: []tflint.Rule{&mockRule{success: false}, &mockRule{success: true}}, expectedIssue: false, }, { name: "incorrect", - rule1: &mockRule{success: false}, - rule2: &mockRule{success: false}, + rules: []tflint.Rule{&mockRule{success: false}, &mockRule{success: false}}, expectedIssue: true, }, + { + name: "threeRulesCorrect", + rules: []tflint.Rule{&mockRule{success: false}, &mockRule{success: false}, &mockRule{success: false}}, + expectedIssue: true, + }, + { + name: "threeRulesIncorrect", + rules: []tflint.Rule{&mockRule{success: false}, &mockRule{success: false}, &mockRule{success: true}}, + expectedIssue: false, + }, } for _, tc := range cases { @@ -81,7 +87,7 @@ func TestEitherPrivateEndpoints(t *testing.T) { runner := helper.TestRunner(t, map[string]string{filename: ""}) - sut := common.NewEitherCheckRule("either", true, tflint.ERROR, tc.rule1, tc.rule2) + sut := common.NewEitherCheckRule("either", true, tflint.ERROR, tc.rules...) err := sut.Check(runner) require.NoError(t, err) diff --git a/generate.go b/generate.go new file mode 100644 index 0000000..968d409 --- /dev/null +++ b/generate.go @@ -0,0 +1,7 @@ +package main + +//go:generate go run ./cmd/rulesdoc + +// This file hosts the go:generate directive for producing RULES.md from the +// registered rules. Run `go generate ./...` at the repository root after +// adding or modifying rules to refresh the documentation. diff --git a/outputs/no_entire_resource_output.go b/outputs/no_entire_resource_output.go new file mode 100644 index 0000000..d13afb4 --- /dev/null +++ b/outputs/no_entire_resource_output.go @@ -0,0 +1,176 @@ +package outputs + +import ( + "fmt" + + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + "github.com/terraform-linters/tflint-plugin-sdk/tflint" +) + +// Schema to capture resource and output blocks. +var noEntireResourceOutputBodySchema = &hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { // resource "type" "name" {} + Type: "resource", + LabelNames: []string{"type", "name"}, + }, + { // output "name" { value = ... } + Type: "output", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{Attributes: []hclext.AttributeSchema{{Name: "value"}}}, + }, + }, +} + +var _ tflint.Rule = new(NoEntireResourceOutputRule) + +// NoEntireResourceOutputRule checks that an output does not expose an entire resource object. +type NoEntireResourceOutputRule struct { + tflint.DefaultRule + ruleName string + link string +} + +// NewNoEntireResourceOutputRule returns the rule. +func NewNoEntireResourceOutputRule(ruleName, link string) *NoEntireResourceOutputRule { + return &NoEntireResourceOutputRule{ruleName: ruleName, link: link} +} + +func (r *NoEntireResourceOutputRule) Name() string { return r.ruleName } +func (r *NoEntireResourceOutputRule) Link() string { return r.link } +func (r *NoEntireResourceOutputRule) Enabled() bool { return true } +func (r *NoEntireResourceOutputRule) Severity() tflint.Severity { return tflint.ERROR } + +func (r *NoEntireResourceOutputRule) Check(run tflint.Runner) error { + path, err := run.GetModulePath() + if err != nil { + return err + } + if !path.IsRoot() { // only evaluate root module like other rules + return nil + } + + content, err := run.GetModuleContent(noEntireResourceOutputBodySchema, &tflint.GetModuleContentOption{ExpandMode: tflint.ExpandModeNone}) + if err != nil { + return err + } + + // Build a set of resource identifiers: type.name + resources := map[string]struct{}{} + for _, b := range content.Blocks { + if b.Type == "resource" && len(b.Labels) == 2 { // type, name + resources[fmt.Sprintf("%s.%s", b.Labels[0], b.Labels[1])] = struct{}{} + } + } + + var errs error + for _, b := range content.Blocks { + if b.Type != "output" { + continue + } + attr, ok := b.Body.Attributes["value"] + if !ok { // nothing to check + continue + } + if isWholeResourceExpression(attr.Expr, resources) { + if e := run.EmitIssue(r, "authors SHOULD NOT output entire resource objects; expose specific computed attributes instead", attr.Range); e != nil { + errs = multierror.Append(errs, e) + } + } + } + return errs +} + +// isWholeResourceTraversal returns true if traversal represents a resource base object optionally followed by only index or splat traversers (i.e. still the entire object or collection of objects). +func isWholeResourceTraversal(traversal hcl.Traversal, resources map[string]struct{}) bool { + if len(traversal) < 2 { + return false + } + root, ok := traversal[0].(hcl.TraverseRoot) + if !ok { + return false + } + nameAttr, ok := traversal[1].(hcl.TraverseAttr) + if !ok { + return false + } + key := fmt.Sprintf("%s.%s", root.Name, nameAttr.Name) + if _, exists := resources[key]; !exists { + return false + } + // (debug tracing removed in finalized implementation) + // Remaining traversers (if any) must all be index or splat; BUT if an attribute appears after a splat/index chain we treat it as narrowed (acceptable) + encounteredAttr := false + for _, tr := range traversal[2:] { + switch tr.(type) { + case hcl.TraverseIndex, hcl.TraverseSplat: + if encounteredAttr { // once narrowed by attr, further index/splat shouldn't re-widen + return false + } + continue + case hcl.TraverseAttr: + // attribute access narrows to a specific property => not whole resource object(s) + return false + default: + return false + } + } + return true +} + +// isCompositeWholeResource unwraps index/splat expression layers until reaching a traversal and evaluates it. +func isWholeResourceExpression(expr hcl.Expression, resources map[string]struct{}) bool { + switch e := expr.(type) { + case *hclsyntax.ScopeTraversalExpr: + return isWholeResourceTraversal(e.Traversal, resources) + case *hclsyntax.RelativeTraversalExpr: + // RelativeTraversalExpr has a source expression and additional traversers appended. + if base := isWholeResourceExpression(e.Source, resources); base { + // If base already whole, any added index/splat keeps it whole; any attr makes it partial. + for _, tr := range e.Traversal { + switch tr.(type) { + case hcl.TraverseIndex, hcl.TraverseSplat: + continue + default: + return false + } + } + return true + } + // Or base not whole: attempt to treat combined traversal if source is plain + if se, ok := e.Source.(*hclsyntax.ScopeTraversalExpr); ok { + combined := append(se.Traversal, e.Traversal...) + return isWholeResourceTraversal(combined, resources) + } + return false + case *hclsyntax.IndexExpr: + return isWholeResourceExpression(e.Collection, resources) + case *hclsyntax.SplatExpr: + // Determine if this is a full splat (exposes entire resource objects) or a projection. + // Full splat: collection[*] + // Projection: collection[*].attribute => represented by Each being a RelativeTraversalExpr whose Source is the element symbol. + baseWhole := isWholeResourceExpression(e.Source, resources) + if !baseWhole { + return false + } + // Classify Each + switch each := e.Each.(type) { + case *hclsyntax.AnonSymbolExpr: + // Full splat: still whole + return true + case *hclsyntax.RelativeTraversalExpr: + if _, ok := each.Source.(*hclsyntax.AnonSymbolExpr); ok { + // Attribute (or further) traversal off element => projection (narrowed) + return false + } + return true // Unexpected shape; be conservative and treat as whole + default: + // Unexpected Each expression kind; conservatively treat as whole + return true + } + } + return false +} diff --git a/outputs/no_entire_resource_output_test.go b/outputs/no_entire_resource_output_test.go new file mode 100644 index 0000000..78e7589 --- /dev/null +++ b/outputs/no_entire_resource_output_test.go @@ -0,0 +1,157 @@ +package outputs_test + +import ( + "testing" + + "github.com/Azure/tflint-ruleset-avm/outputs" + "github.com/hashicorp/hcl/v2" + "github.com/terraform-linters/tflint-plugin-sdk/helper" +) + +func TestNoEntireResourceOutputRule(t *testing.T) { + cases := []struct { + desc string + config string + issues helper.Issues + }{ + { + desc: "output attribute only (ok)", + config: `resource "azurerm_resource_group" "rg" { + location = "westeurope" + name = "rg-test" +} + +output "rg_id" { + value = azurerm_resource_group.rg.id +}`, + issues: helper.Issues{}, + }, + { + desc: "entire resource output (not ok)", + config: `resource "azurerm_resource_group" "rg" { + location = "westeurope" + name = "rg-test" +} + +output "rg" { + value = azurerm_resource_group.rg +}`, + issues: helper.Issues{{ + Rule: outputs.NewNoEntireResourceOutputRule("no_entire_resource_output", ""), + Message: "authors SHOULD NOT output entire resource objects; expose specific computed attributes instead", + Range: hcl.Range{Filename: "variables.tf"}, + }}, + }, + { + desc: "entire resource index output (not ok)", + config: `resource "azurerm_resource_group" "rg" { + count = 1 + location = "westeurope" + name = "rg-test" +} + +output "rg" { + value = azurerm_resource_group.rg[0] +}`, + issues: helper.Issues{{ + Rule: outputs.NewNoEntireResourceOutputRule("no_entire_resource_output", ""), + Message: "authors SHOULD NOT output entire resource objects; expose specific computed attributes instead", + Range: hcl.Range{Filename: "variables.tf"}, + }}, + }, + { + desc: "for_each attribute only (ok)", + config: `resource "azurerm_resource_group" "rg" { + for_each = { a = "one", b = "two" } + location = "westeurope" + name = "rg-${each.key}" +} + +output "rg_ids" { + value = [for v in azurerm_resource_group.rg : v.id] +}`, + issues: helper.Issues{}, + }, + { + desc: "for_each whole instance (not ok)", + config: `resource "azurerm_resource_group" "rg" { + for_each = { a = "one" } + location = "westeurope" + name = "rg-${each.key}" +} + +output "rg" { + value = azurerm_resource_group.rg["a"] +}`, + issues: helper.Issues{{ + Rule: outputs.NewNoEntireResourceOutputRule("no_entire_resource_output", ""), + Message: "authors SHOULD NOT output entire resource objects; expose specific computed attributes instead", + Range: hcl.Range{Filename: "variables.tf"}, + }}, + }, + { + desc: "for_each whole collection splat (not ok)", + config: `resource "azurerm_resource_group" "rg" { + for_each = { a = "one", b = "two" } + location = "westeurope" + name = "rg-${each.key}" +} + +output "rgs" { + value = azurerm_resource_group.rg[*] +}`, + issues: helper.Issues{{ + Rule: outputs.NewNoEntireResourceOutputRule("no_entire_resource_output", ""), + Message: "authors SHOULD NOT output entire resource objects; expose specific computed attributes instead", + Range: hcl.Range{Filename: "variables.tf"}, + }}, + }, + { + desc: "for_each base map reference (not ok)", + config: `resource "azurerm_resource_group" "rg" { + for_each = { a = "one", b = "two" } + location = "westeurope" + name = "rg-${each.key}" +} + +output "rgs" { + value = azurerm_resource_group.rg +}`, + issues: helper.Issues{{ + Rule: outputs.NewNoEntireResourceOutputRule("no_entire_resource_output", ""), + Message: "authors SHOULD NOT output entire resource objects; expose specific computed attributes instead", + Range: hcl.Range{Filename: "variables.tf"}, + }}, + }, + { + desc: "for_each splat attribute access (ok)", + config: `resource "azurerm_resource_group" "rg" { + for_each = { a = "one", b = "two" } + location = "westeurope" + name = "rg-${each.key}" +} + +output "rg_ids" { + value = azurerm_resource_group.rg[*].id +}`, + issues: helper.Issues{}, + }, + } + + for _, tc := range cases { + c := tc + // nolint:scopelint // parallel subtests closure capture + t.Run(c.desc, func(t *testing.T) { + t.Parallel() + rule := outputs.NewNoEntireResourceOutputRule("no_entire_resource_output", "") + filename := "variables.tf" + runner := helper.TestRunner(t, map[string]string{filename: c.config}) + + if err := rule.Check(runner); err != nil { + t.Fatalf("Unexpected error occurred: %s", err) + } + + helper.AssertIssuesWithoutRange(t, c.issues, runner.Issues) + }) + } +} diff --git a/outputs/outputs.go b/outputs/outputs.go index f1e9018..9ca9e17 100644 --- a/outputs/outputs.go +++ b/outputs/outputs.go @@ -6,4 +6,5 @@ import "github.com/terraform-linters/tflint-plugin-sdk/tflint" var Rules = []tflint.Rule{ NewRequiredOutputRule("required_output_rmfr7", "resource_id", "https://azure.github.io/Azure-Verified-Modules/specs/shared/#id-rmfr7---category-outputs---minimum-required-outputs"), + NewNoEntireResourceOutputRule("no_entire_resource_output_tffr2", "https://azure.github.io/Azure-Verified-Modules/specs/tf/res/#id-tffr2---category-outputs---additional-terraform-outputs"), } diff --git a/rules/disallowed_provider.go b/rules/disallowed_provider.go new file mode 100644 index 0000000..d1ecf49 --- /dev/null +++ b/rules/disallowed_provider.go @@ -0,0 +1,106 @@ +package rules + +import ( + "fmt" + "strings" + + "github.com/hashicorp/go-multierror" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + "github.com/terraform-linters/tflint-plugin-sdk/tflint" + "github.com/zclconf/go-cty/cty" +) + +// DisallowedProviderRule checks that a specific provider is not used in the module. +// It performs two checks when ProviderName is "azurerm": +// 1. The `required_providers` block must not declare a provider whose source is hashicorp/azurerm. +// 2. No resource / data / ephemeral blocks whose first label starts with "azurerm_" should exist. +// +// If any are found, an issue is emitted. +// The rule is generic for potential future expansion by changing ProviderName/ProviderSource/Prefix. +// Currently only azurerm is required per user request. +var _ tflint.Rule = new(DisallowedProviderRule) + +type DisallowedProviderRule struct { + tflint.DefaultRule + ProviderName string + ProviderSource string +} + +func NewDisallowedProviderRule(providerName, providerSource string) *DisallowedProviderRule { + return &DisallowedProviderRule{ProviderName: providerName, ProviderSource: providerSource} +} + +func (r *DisallowedProviderRule) Name() string { + // naming convention: provider__disallowed + return fmt.Sprintf("provider_%s_disallowed", r.ProviderName) +} + +func (r *DisallowedProviderRule) Enabled() bool { return true } + +func (r *DisallowedProviderRule) Severity() tflint.Severity { return tflint.ERROR } + +// requiredProvidersBodySchema dynamically requests the attribute for the provider name inside required_providers. +func (r *DisallowedProviderRule) requiredProvidersBodySchema() *hclext.BodySchema { + return &hclext.BodySchema{Blocks: []hclext.BlockSchema{{Type: "terraform", Body: &hclext.BodySchema{Blocks: []hclext.BlockSchema{{Type: "required_providers", Body: &hclext.BodySchema{Attributes: []hclext.AttributeSchema{{Name: r.ProviderName}}}}}}}}} +} + +var disallowedProviderUsageSchema = &hclext.BodySchema{Blocks: []hclext.BlockSchema{ + {Type: "resource", LabelNames: []string{"type", "name"}}, + {Type: "data", LabelNames: []string{"type", "name"}}, + {Type: "ephemeral", LabelNames: []string{"type", "name"}}, +}} + +func (r *DisallowedProviderRule) Check(run tflint.Runner) error { + path, err := run.GetModulePath() + if err != nil { + return err + } + if !path.IsRoot() { // only root module enforcement for consistency with other rules + return nil + } + + // 1. Check required_providers for disallowed provider with matching source + content, err := run.GetModuleContent(r.requiredProvidersBodySchema(), &tflint.GetModuleContentOption{ExpandMode: tflint.ExpandModeNone}) + if err != nil { + return err + } + for _, tb := range content.Blocks { // terraform blocks + for _, rp := range tb.Body.Blocks { // required_providers blocks + if attr, ok := rp.Body.Attributes[r.ProviderName]; ok { + provider := struct { + Source string `cty:"source"` + }{} + wantType := cty.Object(map[string]cty.Type{"source": cty.String}) + if evalErr := run.EvaluateExpr(attr.Expr, &provider, &tflint.EvaluateExprOption{WantType: &wantType}); evalErr == nil { + if strings.EqualFold(provider.Source, r.ProviderSource) { + if issueErr := run.EmitIssue(r, fmt.Sprintf("provider '%s' (source %s) is disallowed", r.ProviderName, r.ProviderSource), attr.Range); issueErr != nil { + return issueErr + } + } + } + } + } + } + + // 2. Scan for any usage blocks with provider name prefix + usage, err := run.GetModuleContent(disallowedProviderUsageSchema, &tflint.GetModuleContentOption{ExpandMode: tflint.ExpandModeNone}) + if err != nil { + return err + } + + var errs error + for _, blk := range usage.Blocks { + if len(blk.Labels) == 0 { // defensive + continue + } + first := blk.Labels[0] + if strings.HasPrefix(first, r.ProviderName+"_") { + if e := run.EmitIssue(r, fmt.Sprintf("%s block type '%s' is disallowed (provider '%s')", blk.Type, first, r.ProviderName), blk.DefRange); e != nil { + errs = multierror.Append(errs, e) + } + } + } + return errs +} + +func (r *DisallowedProviderRule) Link() string { return "" } diff --git a/rules/disallowed_provider_test.go b/rules/disallowed_provider_test.go new file mode 100644 index 0000000..f618a6c --- /dev/null +++ b/rules/disallowed_provider_test.go @@ -0,0 +1,73 @@ +package rules_test + +import ( + "testing" + + rules "github.com/Azure/tflint-ruleset-avm/rules" + "github.com/terraform-linters/tflint-plugin-sdk/helper" +) + +func TestDisallowedProviderRule_RequiredProviders(t *testing.T) { + const content = `terraform { + required_providers { + azurerm = { + source = "hashicorp/azurerm" + version = "~> 4.0" + } + } +}` + rule := rules.NewDisallowedProviderRule("azurerm", "hashicorp/azurerm") + runner := helper.TestRunner(t, map[string]string{"main.tf": content}) + if err := rule.Check(runner); err != nil { + t.Fatalf("unexpected error: %s", err) + } + if len(runner.Issues) != 1 { + t.Fatalf("expected 1 issue, got %d", len(runner.Issues)) + } + if runner.Issues[0].Rule.Name() != rule.Name() { + t.Fatalf("unexpected rule name: %s", runner.Issues[0].Rule.Name()) + } +} + +func TestDisallowedProviderRule_ResourceUsage(t *testing.T) { + const content = `resource "random_string" "allowed" { + length = 8 +} +resource "azurerm_resource_group" "rg" { + name = "rg1" + location = "uksouth" +}` + rule := rules.NewDisallowedProviderRule("azurerm", "hashicorp/azurerm") + runner := helper.TestRunner(t, map[string]string{"main.tf": content}) + if err := rule.Check(runner); err != nil { + t.Fatalf("unexpected error: %s", err) + } + if len(runner.Issues) != 1 { + t.Fatalf("expected 1 issue, got %d", len(runner.Issues)) + } + if runner.Issues[0].Rule.Name() != rule.Name() { + t.Fatalf("unexpected rule name: %s", runner.Issues[0].Rule.Name()) + } +} + +func TestDisallowedProviderRule_NoIssues(t *testing.T) { + const content = `terraform { + required_providers { + random = { + source = "hashicorp/random" + version = "~> 3.0" + } + } +} +resource "random_string" "s" { + length = 4 +}` + rule := rules.NewDisallowedProviderRule("azurerm", "hashicorp/azurerm") + runner := helper.TestRunner(t, map[string]string{"main.tf": content}) + if err := rule.Check(runner); err != nil { + t.Fatalf("unexpected error: %s", err) + } + if len(runner.Issues) != 0 { + t.Fatalf("expected no issues, got %d", len(runner.Issues)) + } +} diff --git a/rules/required_attribute.go b/rules/required_attribute.go new file mode 100644 index 0000000..2ef0fdd --- /dev/null +++ b/rules/required_attribute.go @@ -0,0 +1,149 @@ +package rules + +import ( + "fmt" + "strings" + + "github.com/hashicorp/go-multierror" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + "github.com/terraform-linters/tflint-plugin-sdk/tflint" + "github.com/zclconf/go-cty/cty" +) + +// RequiredAttributeRule is a generic rule to enforce that a particular attribute is present +// on blocks of a given type whose first label (usually the resource type) matches an allow list. +// This is intentionally generic so future required-attribute rules can be added with a single +// line registration instead of bespoke implementations. +// +// Example enforced block shape: resource "" "name" { = <...> } +// Only blocks whose first label value appears in allowedFirstLabelValues are checked. +// When the attribute is missing an issue is emitted including a suggested default value text. +type RequiredAttributeRule struct { + tflint.DefaultRule + + // blockType is the HCL block type (e.g. "resource", "data") + blockType string + // allowedFirstLabelValues restricts enforcement to blocks whose first label value is in the set. + allowedFirstLabelValues map[string]struct{} + // attributeName is the attribute that must be present. + attributeName string + // defaultSuggestion is text describing a default value (NOT auto-fix) shown in the issue message. + defaultSuggestion string + // name is the unique rule name. + name string + // link is documentation URL. + link string + // severity of the rule (defaults to ERROR when empty). + severity tflint.Severity + // optional value validator executed when attribute is present + valueValidator func(run tflint.Runner, rule *RequiredAttributeRule, attr *hclext.Attribute) error +} + +// NewAzapiRequiredAttributeRule constructs a new required attribute rule. +type RequiredAttributeOption func(*RequiredAttributeRule) + +// WithValueValidator registers a callback to validate attribute value (only runs if attribute exists). +func WithValueValidator(fn func(run tflint.Runner, rule *RequiredAttributeRule, attr *hclext.Attribute) error) RequiredAttributeOption { + return func(r *RequiredAttributeRule) { r.valueValidator = fn } +} + +// DisallowWildcardList returns an option that forbids a single-item list containing "*" for the given attribute. +func DisallowWildcardList(attributeName string) RequiredAttributeOption { + return WithValueValidator(func(run tflint.Runner, rule *RequiredAttributeRule, attr *hclext.Attribute) error { + var values []string + wantType := cty.List(cty.String) + if err := run.EvaluateExpr(attr.Expr, &values, &tflint.EvaluateExprOption{WantType: &wantType}); err == nil { + if len(values) == 1 && values[0] == "*" { + msg := fmt.Sprintf("`%s` must not contain the wildcard \"*\"; explicitly list required fields or use empty list []", attributeName) + return run.EmitIssue(rule, msg, attr.Range) + } + } + return nil + }) +} + +func NewRequiredAttributeRule(name, link, blockType string, allowedFirstLabels []string, attributeName, defaultSuggestion string, severity tflint.Severity, opts ...RequiredAttributeOption) *RequiredAttributeRule { + set := make(map[string]struct{}, len(allowedFirstLabels)) + for _, v := range allowedFirstLabels { + set[v] = struct{}{} + } + if severity == 0 { // treat unknown as ERROR + severity = tflint.ERROR + } + r := &RequiredAttributeRule{ + blockType: blockType, + allowedFirstLabelValues: set, + attributeName: attributeName, + defaultSuggestion: defaultSuggestion, + name: name, + link: link, + severity: severity, + } + for _, o := range opts { + o(r) + } + return r +} + +func (r *RequiredAttributeRule) Name() string { return r.name } +func (r *RequiredAttributeRule) Link() string { return r.link } +func (r *RequiredAttributeRule) Enabled() bool { return true } +func (r *RequiredAttributeRule) Severity() tflint.Severity { return r.severity } + +// dynamic schema built per rule (we only care about presence, not value shape). +func (r *RequiredAttributeRule) schema() *hclext.BodySchema { + return &hclext.BodySchema{ // e.g. resource "type" "name" {} + Blocks: []hclext.BlockSchema{{ + Type: r.blockType, + LabelNames: []string{"type", "name"}, + Body: &hclext.BodySchema{Attributes: []hclext.AttributeSchema{{ + Name: r.attributeName, + }}}, + }}, + } +} + +func (r *RequiredAttributeRule) Check(run tflint.Runner) error { + path, err := run.GetModulePath() + if err != nil { + return err + } + if !path.IsRoot() { // only enforce at root, consistent with existing azapi rules + return nil + } + + content, err := run.GetModuleContent(r.schema(), &tflint.GetModuleContentOption{ExpandMode: tflint.ExpandModeNone}) + if err != nil { + return err + } + + var errs error + for _, b := range content.Blocks { + if b.Type != r.blockType || len(b.Labels) < 2 { // defensive + continue + } + first := b.Labels[0] + if _, ok := r.allowedFirstLabelValues[first]; !ok { + continue + } + attr, ok := b.Body.Attributes[r.attributeName] + if ok { + if r.valueValidator != nil { + if e := r.valueValidator(run, r, attr); e != nil { + errs = multierror.Append(errs, e) + } + } + continue + } + + suggestion := r.defaultSuggestion + if strings.TrimSpace(suggestion) != "" { + suggestion = fmt.Sprintf(" (suggested default: %s)", suggestion) + } + msg := fmt.Sprintf("`%s` attribute must be specified%s", r.attributeName, suggestion) + if e := run.EmitIssue(r, msg, b.DefRange); e != nil { + errs = multierror.Append(errs, e) + } + } + return errs +} diff --git a/rules/required_attribute_test.go b/rules/required_attribute_test.go new file mode 100644 index 0000000..4623f1b --- /dev/null +++ b/rules/required_attribute_test.go @@ -0,0 +1,68 @@ +package rules + +import ( + "testing" + + "github.com/terraform-linters/tflint-plugin-sdk/helper" + "github.com/terraform-linters/tflint-plugin-sdk/tflint" +) + +// TestAzapiRequiredAttributeRule validates the generic rule behaviour using an artificial attribute. +func TestAzapiRequiredAttributeRule(t *testing.T) { + rule := NewRequiredAttributeRule( + "azapi_resource_required_fake_attr", // name + "https://example.invalid/docs/fake", // link + "resource", // block type + []string{"azapi_resource"}, // first label allow list + "fake_required", // attribute name + "\"default\"", // suggestion text (string literal) + tflint.ERROR, + ) + + cases := []struct { + desc string + config string + expected helper.Issues + }{ + { + desc: "no matching resources => no issue", + config: `resource "random_pet" "example" {}`, + expected: helper.Issues{}, + }, + { + desc: "matching resource missing attribute => issue", + config: `resource "azapi_resource" "ex" { + type = "Microsoft.Example/examples@2023-06-01" + name = "ex1" + location = "westus2" + parent_id = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg1" +}`, + expected: helper.Issues{{ + Rule: rule, + Message: "`fake_required` attribute must be specified (suggested default: \"default\")", + }}, + }, + { + desc: "matching resource with attribute => ok", + config: `resource "azapi_resource" "ex" { + type = "Microsoft.Example/examples@2023-06-01" + name = "ex1" + location = "westus2" + parent_id = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg1" + fake_required = "custom" +}`, + expected: helper.Issues{}, + }, + } + + for _, c := range cases { + c := c + t.Run(c.desc, func(t *testing.T) { + runner := helper.TestRunner(t, map[string]string{"main.tf": c.config}) + if err := rule.Check(runner); err != nil { + t.Fatalf("unexpected error: %s", err) + } + helper.AssertIssuesWithoutRange(t, c.expected, runner.Issues) + }) + } +} diff --git a/rules/rule_register.go b/rules/rule_register.go index 6e99df1..b197905 100644 --- a/rules/rule_register.go +++ b/rules/rule_register.go @@ -25,9 +25,40 @@ var Rules = func() []tflint.Rule { NewTerraformDotTfRule(), NewModuleSourceRule(), NewNoDoubleQuotesInIgnoreChangesRule(), + NewDisallowedProviderRule("azurerm", "hashicorp/azurerm"), NewProviderVersionRule("modtm", "Azure/modtm", "0.3.0", "~> 0.3", true), NewProviderVersionRule("azapi", "Azure/azapi", "2.999.0", "~> 2.0", false), NewProviderVersionRule("azurerm", "hashicorp/azurerm", "4.999.0", "~> 4.0", false), + // Generic azapi required attribute rules replacing bespoke implementations + NewRequiredAttributeRule( + "azapi_response_export_values", + "https://azure.github.io/Azure-Verified-Modules/specs/tf/azapi/#response_export_values-required", + "resource", + []string{"azapi_resource", "azapi_resource_action", "azapi_update_resource"}, + "response_export_values", + "[]", + tflint.ERROR, + DisallowWildcardList("response_export_values"), + ), + NewRequiredAttributeRule( + "azapi_data_response_export_values", + "https://azure.github.io/Azure-Verified-Modules/specs/tf/azapi/#response_export_values-required", + "data", + []string{"azapi_resource", "azapi_resource_action", "azapi_resource_list"}, + "response_export_values", + "[]", + tflint.ERROR, + DisallowWildcardList("response_export_values"), + ), + NewRequiredAttributeRule( + "azapi_replace_triggers_refs", + "https://azure.github.io/Azure-Verified-Modules/specs/tf/azapi/#replace_triggers_refs", + "resource", + []string{"azapi_resource"}, + "replace_triggers_refs", + "[]", + tflint.ERROR, + ), }, interfaces.Rules, outputs.Rules, From 0057a04c8613633b957e32300ad933f72a574693 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Wed, 5 Nov 2025 17:12:03 +0000 Subject: [PATCH 2/5] feat: update lock with notes field --- .vscode/mcp.json | 3 ++- interfaces/lock.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.vscode/mcp.json b/.vscode/mcp.json index bff38cf..b474710 100644 --- a/.vscode/mcp.json +++ b/.vscode/mcp.json @@ -2,8 +2,9 @@ "servers": { "godoc": { "command": "godoc-mcp", + "type": "stdio", "args": [], - "env": {}, + "env": {} }, "terraform-mcp-eva": { "type": "stdio", diff --git a/interfaces/lock.go b/interfaces/lock.go index 830cfa7..33d0b1d 100644 --- a/interfaces/lock.go +++ b/interfaces/lock.go @@ -10,8 +10,9 @@ import ( // When updating the type constraint string, make sure to also update the two // private endpoint interfaces (the one with subresource and the one without). var LockTypeString = `object({ - kind = string - name = optional(string, null) + kind = string + name = optional(string, null) + notes = optional(string, null) })` var lockType = StringToTypeConstraintWithDefaults(LockTypeString) From f6f4e27855df2139717b5b82fe5158de67a078b8 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Wed, 5 Nov 2025 17:16:32 +0000 Subject: [PATCH 3/5] feat: update role assignments to include name --- interfaces/doc.go | 2 ++ interfaces/private_endpoints.go | 1 + interfaces/role_assignments.go | 1 + 3 files changed, 4 insertions(+) create mode 100644 interfaces/doc.go diff --git a/interfaces/doc.go b/interfaces/doc.go new file mode 100644 index 0000000..3fb7e35 --- /dev/null +++ b/interfaces/doc.go @@ -0,0 +1,2 @@ +// Package interfaces defines the reuable interfaces for Azure Verified Modules. +package interfaces diff --git a/interfaces/private_endpoints.go b/interfaces/private_endpoints.go index 546dad7..79bb34e 100644 --- a/interfaces/private_endpoints.go +++ b/interfaces/private_endpoints.go @@ -8,6 +8,7 @@ import ( var PrivateEndpointTypeString = `map(object({ name = optional(string, null) role_assignments = optional(map(object({ + name = optional(string, null) role_definition_id_or_name = string principal_id = string description = optional(string, null) diff --git a/interfaces/role_assignments.go b/interfaces/role_assignments.go index 6014492..4a0a58d 100644 --- a/interfaces/role_assignments.go +++ b/interfaces/role_assignments.go @@ -9,6 +9,7 @@ import ( // When updating the type constraint string, make sure to also update the two // private endpoint interfaces (the one with subresource and the one without). var RoleAssignmentsTypeString = `map(object({ + name = optional(string, null) role_definition_id_or_name = string principal_id = string description = optional(string, null) From 19113484bb0f9af9a11600ee8b32a348ce2d26ea Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Wed, 5 Nov 2025 17:25:44 +0000 Subject: [PATCH 4/5] feat: simplify private endpoints --- interfaces/interfaces.go | 7 +-- interfaces/private_endpoints.go | 1 + ...private_endpoints_with_subresource_name.go | 46 ------------------- ...te_endpoints_with_subresource_name_test.go | 43 ----------------- 4 files changed, 2 insertions(+), 95 deletions(-) delete mode 100644 interfaces/private_endpoints_with_subresource_name.go delete mode 100644 interfaces/private_endpoints_with_subresource_name_test.go diff --git a/interfaces/interfaces.go b/interfaces/interfaces.go index f1b0013..018864d 100644 --- a/interfaces/interfaces.go +++ b/interfaces/interfaces.go @@ -1,7 +1,6 @@ package interfaces import ( - "github.com/Azure/tflint-ruleset-avm/common" "github.com/terraform-linters/tflint-plugin-sdk/tflint" ) @@ -13,9 +12,5 @@ var Rules = []tflint.Rule{ NewVarCheckRuleFromAvmInterface(ManagedIdentities), NewVarCheckRuleFromAvmInterface(RoleAssignments), NewVarCheckRuleFromAvmInterface(Tags), - func() tflint.Rule { - return common.NewEitherCheckRule("private_endpoints", true, tflint.ERROR, - NewVarCheckRuleFromAvmInterface(PrivateEndpoints), - NewVarCheckRuleFromAvmInterface(PrivateEndpointsWithSubresourceName)) - }(), + NewVarCheckRuleFromAvmInterface(PrivateEndpoints), } diff --git a/interfaces/private_endpoints.go b/interfaces/private_endpoints.go index 79bb34e..710b7ba 100644 --- a/interfaces/private_endpoints.go +++ b/interfaces/private_endpoints.go @@ -23,6 +23,7 @@ var PrivateEndpointTypeString = `map(object({ name = optional(string, null) }), null) tags = optional(map(string), null) + subresource_name = optional(string, null) subnet_resource_id = string private_dns_zone_group_name = optional(string, "default") private_dns_zone_resource_ids = optional(set(string), []) diff --git a/interfaces/private_endpoints_with_subresource_name.go b/interfaces/private_endpoints_with_subresource_name.go deleted file mode 100644 index e8c2f45..0000000 --- a/interfaces/private_endpoints_with_subresource_name.go +++ /dev/null @@ -1,46 +0,0 @@ -package interfaces - -import ( - "github.com/matt-FFFFFF/tfvarcheck/varcheck" - "github.com/zclconf/go-cty/cty" -) - -var PrivateEndpointWithSubresourceNameTypeString = `map(object({ - name = optional(string, null) - role_assignments = optional(map(object({ - role_definition_id_or_name = string - principal_id = string - description = optional(string, null) - skip_service_principal_aad_check = optional(bool, false) - condition = optional(string, null) - condition_version = optional(string, null) - delegated_managed_identity_resource_id = optional(string, null) - principal_type = optional(string, null) - })), {}) - lock = optional(object({ - kind = string - name = optional(string, null) - }), null) - tags = optional(map(string), null) - subnet_resource_id = string - subresource_name = string - private_dns_zone_group_name = optional(string, "default") - private_dns_zone_resource_ids = optional(set(string), []) - application_security_group_associations = optional(map(string), {}) - private_service_connection_name = optional(string, null) - network_interface_name = optional(string, null) - location = optional(string, null) - resource_group_name = optional(string, null) - ip_configurations = optional(map(object({ - name = string - private_ip_address = string - })), {}) -}))` - -var PrivateEndpointsWithSubresourceName = AvmInterface{ - VarCheck: varcheck.NewVarCheck(StringToTypeConstraintWithDefaults(PrivateEndpointWithSubresourceNameTypeString), cty.EmptyObjectVal, false), - RuleName: "private_endpoints", - VarTypeString: PrivateEndpointWithSubresourceNameTypeString, - RuleEnabled: true, - RuleLink: "https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#private-endpoints", -} diff --git a/interfaces/private_endpoints_with_subresource_name_test.go b/interfaces/private_endpoints_with_subresource_name_test.go deleted file mode 100644 index 6c881c0..0000000 --- a/interfaces/private_endpoints_with_subresource_name_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package interfaces_test - -import ( - "github.com/Azure/tflint-ruleset-avm/interfaces" - "github.com/terraform-linters/tflint-plugin-sdk/helper" - "testing" -) - -func TestPrivateEndpointsWithSubresourceName(t *testing.T) { - cases := []struct { - Name string - Content string - JSON bool - Expected helper.Issues - }{ - { - Name: "correct", - Content: toTerraformVarType(interfaces.PrivateEndpointsWithSubresourceName), - Expected: helper.Issues{}, - }, - } - - rule := interfaces.NewVarCheckRuleFromAvmInterface(interfaces.PrivateEndpointsWithSubresourceName) - - for _, tc := range cases { - tc := tc - t.Run(tc.Name, func(t *testing.T) { - t.Parallel() - filename := "variables.tf" - if tc.JSON { - filename += ".json" - } - - runner := helper.TestRunner(t, map[string]string{filename: tc.Content}) - - if err := rule.Check(runner); err != nil { - t.Fatalf("Unexpected error occurred: %s", err) - } - - helper.AssertIssues(t, tc.Expected, runner.Issues) - }) - } -} From abe9e816c48bea5729addd0cc50c5ce445fb4655 Mon Sep 17 00:00:00 2001 From: Matt White <16320656+matt-FFFFFF@users.noreply.github.com> Date: Wed, 5 Nov 2025 17:28:19 +0000 Subject: [PATCH 5/5] docs: gen RULES.md --- RULES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RULES.md b/RULES.md index f1f6245..f26d564 100644 --- a/RULES.md +++ b/RULES.md @@ -14,7 +14,7 @@ This document lists all rules currently registered in this ruleset. The Enabled | lock | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/i...](https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#resource-locks) | | managed_identities | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/i...](https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#managed-identities) | | no_entire_resource_output_tffr2 | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/r...](https://azure.github.io/Azure-Verified-Modules/specs/tf/res/#id-tffr2---category-outputs---additional-terraform-outputs) | -| private_endpoints | true | ERROR | - | +| private_endpoints | true | ERROR | [https://azure.github.io/Azure-Verified-Modules/specs/tf/i...](https://azure.github.io/Azure-Verified-Modules/specs/tf/interfaces/#private-endpoints) | | provider_azapi_version_constraint | true | ERROR | - | | provider_azurerm_disallowed | true | ERROR | - | | provider_azurerm_version_constraint | true | ERROR | - |