Skip to content

Commit 11fcc79

Browse files
authored
all: Fix various bugs related to resource type names that match the provider type name (i.e. no resource type suffix) (#421)
* remove launch.json file * bug: skip static files that already exist rather than returning an error * validate: Fixed bug where valid resource type names with the same name as the provider were flagged as invalid * generate: Fixed bug where resource type with same name as provider overwrites all other documentation files * added changelogs * add note to readme
1 parent 4209153 commit 11fcc79

9 files changed

+102
-28
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: BUG FIXES
2+
body: 'validate: Fixed a bug that caused false positive validation errors for resource
3+
types that have the same name as the provider.'
4+
time: 2024-11-18T12:50:01.114497-05:00
5+
custom:
6+
Issue: "419"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
kind: BUG FIXES
2+
body: 'generate: Fixed a bug that caused all generated resource documentation to have
3+
the same content when the provider has a resource type with the same name as the
4+
provider.'
5+
time: 2024-11-18T12:51:00.736136-05:00
6+
custom:
7+
Issue: "419"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: BUG FIXES
2+
body: 'generate: Fixed a bug that would return an error when a static file exists
3+
in both `templates` and `docs`, which will now be ignored.'
4+
time: 2024-11-18T12:52:04.748022-05:00
5+
custom:
6+
Issue: "421"

.gitignore

+3
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,6 @@ tfproviderdocsgen
1919
# JetBrains IDEs files
2020
.idea/
2121
*.iws
22+
23+
# VSCode files
24+
.vscode

.vscode/launch.json

-17
This file was deleted.

README.md

+5
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ When you run `tfplugindocs`, by default from the root directory of a provider co
114114
* Generate function template files, if missing (Requires Terraform v1.8.0+)
115115
* Generate ephemeral resource template files, if missing (Requires Terraform v1.10.0+)
116116
* Copy all non-template files to the output website directory
117+
118+
> [!NOTE]
119+
>
120+
> Non-template files that already exist in the output website directory will not be overwritten.
121+
117122
* Process all the remaining templates to generate files for the output website directory
118123
119124
For inspiration, you can look at the templates and output of the

internal/check/file_mismatch.go

+20-5
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func (check *FileMismatchCheck) Run() error {
8484
return result
8585
}
8686

87-
// ResourceFileMismatchCheck checks for mismatched files, either missing or extraneous, against the resource/datasouce schema
87+
// ResourceFileMismatchCheck checks for mismatched files, either missing or extraneous, against the resource/datasource schema
8888
func (check *FileMismatchCheck) ResourceFileMismatchCheck(files []os.DirEntry, resourceType string, schemas map[string]*tfjson.Schema) error {
8989
if len(files) == 0 {
9090
log.Printf("[DEBUG] Skipping %s file mismatch checks due to missing file list", resourceType)
@@ -200,7 +200,11 @@ func (check *FileMismatchCheck) FunctionFileMismatchCheck(files []os.DirEntry, f
200200

201201
func (check *FileMismatchCheck) IgnoreFileMismatch(file string) bool {
202202
for _, ignoreResourceName := range check.Options.IgnoreFileMismatch {
203-
if ignoreResourceName == fileResourceName(check.Options.ProviderShortName, file) {
203+
if ignoreResourceName == fileResourceNameWithProvider(check.Options.ProviderShortName, file) {
204+
return true
205+
} else if ignoreResourceName == TrimFileExtension(file) {
206+
// While uncommon, it is valid for a resource type to be named the same as the provider itself.
207+
// https://github.com/hashicorp/terraform-plugin-docs/issues/419
204208
return true
205209
}
206210
}
@@ -219,7 +223,13 @@ func (check *FileMismatchCheck) IgnoreFileMissing(resourceName string) bool {
219223
}
220224

221225
func fileHasResource(schemaResources map[string]*tfjson.Schema, providerName, file string) bool {
222-
if _, ok := schemaResources[fileResourceName(providerName, file)]; ok {
226+
if _, ok := schemaResources[fileResourceNameWithProvider(providerName, file)]; ok {
227+
return true
228+
}
229+
230+
// While uncommon, it is valid for a resource type to be named the same as the provider itself.
231+
// https://github.com/hashicorp/terraform-plugin-docs/issues/419
232+
if _, ok := schemaResources[TrimFileExtension(file)]; ok {
223233
return true
224234
}
225235

@@ -234,7 +244,7 @@ func fileHasFunction(functions map[string]*tfjson.FunctionSignature, file string
234244
return false
235245
}
236246

237-
func fileResourceName(providerName, fileName string) string {
247+
func fileResourceNameWithProvider(providerName, fileName string) string {
238248
resourceSuffix := TrimFileExtension(fileName)
239249

240250
return fmt.Sprintf("%s_%s", providerName, resourceSuffix)
@@ -244,7 +254,12 @@ func resourceHasFile(files []os.DirEntry, providerName, resourceName string) boo
244254
var found bool
245255

246256
for _, file := range files {
247-
if fileResourceName(providerName, file.Name()) == resourceName {
257+
if fileResourceNameWithProvider(providerName, file.Name()) == resourceName {
258+
found = true
259+
break
260+
} else if TrimFileExtension(file.Name()) == resourceName {
261+
// While uncommon, it is valid for a resource type to be named the same as the provider itself.
262+
// https://github.com/hashicorp/terraform-plugin-docs/issues/419
248263
found = true
249264
break
250265
}

internal/check/file_mismatch_test.go

+46-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func TestFileResourceName(t *testing.T) {
8282
testCase := testCase
8383
t.Run(name, func(t *testing.T) {
8484
t.Parallel()
85-
got := fileResourceName("test", testCase.File)
85+
got := fileResourceNameWithProvider("test", testCase.File)
8686
want := testCase.Expect
8787

8888
if got != want {
@@ -115,6 +115,19 @@ func TestFileMismatchCheck(t *testing.T) {
115115
},
116116
},
117117
},
118+
"all found - resource with no suffix": {
119+
ResourceFiles: fstest.MapFS{
120+
"test.md": {},
121+
},
122+
Options: &FileMismatchOptions{
123+
ProviderShortName: "test",
124+
Schema: &tfjson.ProviderSchema{
125+
ResourceSchemas: map[string]*tfjson.Schema{
126+
"test": {},
127+
},
128+
},
129+
},
130+
},
118131
"all found - function": {
119132
FunctionFiles: fstest.MapFS{
120133
"function1.md": {},
@@ -181,6 +194,23 @@ func TestFileMismatchCheck(t *testing.T) {
181194
},
182195
},
183196
},
197+
"ignore extra file - resource with no suffix": {
198+
ResourceFiles: fstest.MapFS{
199+
"resource1.md": {},
200+
"resource2.md": {},
201+
"test.md": {},
202+
},
203+
Options: &FileMismatchOptions{
204+
IgnoreFileMismatch: []string{"test"},
205+
ProviderShortName: "test",
206+
Schema: &tfjson.ProviderSchema{
207+
ResourceSchemas: map[string]*tfjson.Schema{
208+
"test_resource1": {},
209+
"test_resource2": {},
210+
},
211+
},
212+
},
213+
},
184214
"ignore extra file - function": {
185215
FunctionFiles: fstest.MapFS{
186216
"function1.md": {},
@@ -214,6 +244,21 @@ func TestFileMismatchCheck(t *testing.T) {
214244
},
215245
ExpectError: true,
216246
},
247+
"missing file - resource with no suffix": {
248+
ResourceFiles: fstest.MapFS{
249+
"resource1.md": {},
250+
},
251+
Options: &FileMismatchOptions{
252+
ProviderShortName: "test",
253+
Schema: &tfjson.ProviderSchema{
254+
ResourceSchemas: map[string]*tfjson.Schema{
255+
"test_resource1": {},
256+
"test": {},
257+
},
258+
},
259+
},
260+
ExpectError: true,
261+
},
217262
"missing file - function": {
218263
FunctionFiles: fstest.MapFS{
219264
"function1.md": {},

internal/provider/util.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package provider
55

66
import (
7+
"errors"
78
"fmt"
89
"io"
910
"log"
@@ -45,6 +46,10 @@ func copyFile(srcPath, dstPath string, mode os.FileMode) error {
4546
// If the destination file already exists, we shouldn't blow it away
4647
dstFile, err := os.OpenFile(dstPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode)
4748
if err != nil {
49+
// If the file already exists, we can skip it without returning an error.
50+
if errors.Is(err, os.ErrExist) {
51+
return nil
52+
}
4853
return err
4954
}
5055
defer dstFile.Close()
@@ -71,16 +76,15 @@ func removeAllExt(file string) string {
7176
// has either the providerShortName or the providerShortName concatenated with the
7277
// templateFileName (stripped of file extension.
7378
func resourceSchema(schemas map[string]*tfjson.Schema, providerShortName, templateFileName string) (*tfjson.Schema, string) {
74-
if schema, ok := schemas[providerShortName]; ok {
75-
return schema, providerShortName
76-
}
77-
7879
resName := providerShortName + "_" + removeAllExt(templateFileName)
79-
8080
if schema, ok := schemas[resName]; ok {
8181
return schema, resName
8282
}
8383

84+
if schema, ok := schemas[providerShortName]; ok {
85+
return schema, providerShortName
86+
}
87+
8488
return nil, resName
8589
}
8690

0 commit comments

Comments
 (0)