fix: add support for detecting missing folder in docs/#585
fix: add support for detecting missing folder in docs/#585remyleone wants to merge 2 commits intohashicorp:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to improve documentation validation by detecting cases where provider schemas define documentable entities (e.g., list resources/state stores/actions) but the corresponding documentation subfolders under docs/ (and website/docs/) are missing—so missing docs are surfaced as validation errors instead of being skipped.
Changes:
- Populate mismatch-check entry lists with empty slices when schemas exist but the docs subdirectory is missing (to force the mismatch check to run).
- Update
ResourceFileMismatchCheckto treat “no files” as “missing documentation” when schemas exist (instead of skipping).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
internal/provider/validate.go |
Pass empty directory-entry slices for select doc categories when schema exists but docs folder is missing. |
internal/check/file_mismatch.go |
Change resource mismatch behavior to error when schemas exist but no files are present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if len(v.providerSchema.ActionSchemas) > 0 { | ||
| // If actions exist in schema but directory doesn't, create empty slice to trigger validation | ||
| mismatchOpt.ActionEntries = []os.DirEntry{} |
There was a problem hiding this comment.
The new empty slice assignment is intended to "trigger validation", but FileMismatchCheck calls ActionFileMismatchCheck for actions and that function currently returns early when len(files) == 0. As a result, missing docs/actions (or an empty actions folder) will still be silently skipped even when ActionSchemas exist. Consider updating ActionFileMismatchCheck to treat an empty/non-nil files list as missing documentation (similar to ResourceFileMismatchCheck), or adjust the mismatch check logic so actions are validated when schemas exist but the directory is absent.
| } else if len(v.providerSchema.ActionSchemas) > 0 { | ||
| // If actions exist in schema but directory doesn't, create empty slice to trigger validation | ||
| mismatchOpt.ActionEntries = []os.DirEntry{} |
There was a problem hiding this comment.
Same issue as in validateStaticDocs: setting ActionEntries to an empty slice will not currently trigger a missing-docs error because ActionFileMismatchCheck short-circuits when len(files) == 0. Missing website/docs/actions (or an empty folder) will therefore still be skipped even if ActionSchemas exist.
| } else if len(v.providerSchema.ActionSchemas) > 0 { | |
| // If actions exist in schema but directory doesn't, create empty slice to trigger validation | |
| mismatchOpt.ActionEntries = []os.DirEntry{} | |
| if len(v.providerSchema.ActionSchemas) > 0 && len(actionFiles) == 0 { | |
| result = errors.Join(result, fmt.Errorf("actions documentation directory %q is missing or empty", filepath.Join(dir, "actions"))) | |
| } | |
| } else if len(v.providerSchema.ActionSchemas) > 0 { | |
| result = errors.Join(result, fmt.Errorf("actions documentation directory %q is missing or empty", filepath.Join(dir, "actions"))) |
| if len(files) == 0 { | ||
| // No files found, report all schemas as missing documentation | ||
| for _, resourceName := range resourceNames(schemas) { | ||
| if check.IgnoreFileMissing(resourceName) { | ||
| continue | ||
| } | ||
| log.Printf("[DEBUG] Missing file for %s %s", resourceType, resourceName) | ||
| return fmt.Errorf("missing documentation file for %s: %s", resourceType, resourceName) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
When len(files) == 0, this loop returns on the first non-ignored schema, so it does not actually "report all schemas" and also behaves inconsistently with the rest of the function (which aggregates multiple missing/extra items via errors.Join). Consider collecting all non-ignored schema names and returning a joined error for each missing doc (or reusing the existing missingFiles aggregation path).
| for _, resourceName := range resourceNames(schemas) { | ||
| if check.IgnoreFileMissing(resourceName) { | ||
| continue | ||
| } | ||
| log.Printf("[DEBUG] Missing file for %s %s", resourceType, resourceName) | ||
| return fmt.Errorf("missing documentation file for %s: %s", resourceType, resourceName) | ||
| } | ||
| return nil |
There was a problem hiding this comment.
The new behavior for an empty (but non-nil) files list is not covered by tests. Given this is intended to detect missing documentation directories, please add unit tests that pass an empty slice for ResourceEntries/ListResourceEntries/StateStoreEntries while schemas are present, and assert that the error includes all missing schema names (including ignored vs non-ignored behavior).
| for _, resourceName := range resourceNames(schemas) { | |
| if check.IgnoreFileMissing(resourceName) { | |
| continue | |
| } | |
| log.Printf("[DEBUG] Missing file for %s %s", resourceType, resourceName) | |
| return fmt.Errorf("missing documentation file for %s: %s", resourceType, resourceName) | |
| } | |
| return nil | |
| var result error | |
| for _, resourceName := range resourceNames(schemas) { | |
| if check.IgnoreFileMissing(resourceName) { | |
| continue | |
| } | |
| log.Printf("[DEBUG] Missing file for %s %s", resourceType, resourceName) | |
| err := fmt.Errorf("missing documentation file for %s: %s", resourceType, resourceName) | |
| result = errors.Join(result, err) | |
| } | |
| return result |
Related Issue
Fixes #584
Description
I forgot the documentation for my new list resource and I was surprised to not having an error when I tried to validate the documentation for it. I think it is because when the folder is just not present, no error is raised.
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.