Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate: refactor internal methods to use fs.FS interface for file handling #413

Merged
merged 14 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/unreleased/BUG FIXES-20241022-163805.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: BUG FIXES
body: 'validate: File extension check now runs on `index.*` files instead of just `index.md` files.'
time: 2024-10-22T16:38:05.530944-04:00
custom:
Issue: "413"
5 changes: 5 additions & 0 deletions .changes/unreleased/BUG FIXES-20241022-164013.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: BUG FIXES
body: 'validate: File extension check now specifies the correct valid extensions in the error message.'
time: 2024-10-22T16:40:13.832638-04:00
custom:
Issue: "413"
5 changes: 5 additions & 0 deletions .changes/unreleased/BUG FIXES-20241022-164107.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: BUG FIXES
body: 'validate: Front matter check now runs with the correct options on legacy index files.'
time: 2024-10-22T16:41:07.726856-04:00
custom:
Issue: "413"
21 changes: 11 additions & 10 deletions internal/check/directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package check
import (
"fmt"
"log"
"path"
"path/filepath"
)

Expand Down Expand Up @@ -82,7 +83,7 @@ func InvalidDirectoriesCheck(dirPath string) error {
return nil
}

return fmt.Errorf("invalid Terraform Provider documentation directory found: %s", dirPath)
return fmt.Errorf("invalid Terraform Provider documentation directory found: %s", filepath.FromSlash(dirPath))

}

Expand All @@ -92,7 +93,7 @@ func MixedDirectoriesCheck(docFiles []string) error {
err := fmt.Errorf("mixed Terraform Provider documentation directory layouts found, must use only legacy or registry layout")

for _, file := range docFiles {
directory := filepath.Dir(file)
directory := path.Dir(file)
log.Printf("[DEBUG] Found directory: %s", directory)

// Allow docs/ with other files
Expand Down Expand Up @@ -120,7 +121,7 @@ func MixedDirectoriesCheck(docFiles []string) error {

func IsValidLegacyDirectory(directory string) bool {
for _, validLegacyDirectory := range ValidLegacyDirectories {
if directory == filepath.FromSlash(validLegacyDirectory) {
if directory == validLegacyDirectory {
return true
}
}
Expand All @@ -130,7 +131,7 @@ func IsValidLegacyDirectory(directory string) bool {

func IsValidRegistryDirectory(directory string) bool {
for _, validRegistryDirectory := range ValidRegistryDirectories {
if directory == filepath.FromSlash(validRegistryDirectory) {
if directory == validRegistryDirectory {
return true
}
}
Expand All @@ -139,32 +140,32 @@ func IsValidRegistryDirectory(directory string) bool {
}

func IsValidCdktfDirectory(directory string) bool {
if directory == filepath.FromSlash(fmt.Sprintf("%s/%s", LegacyIndexDirectory, CdktfIndexDirectory)) {
if directory == fmt.Sprintf("%s/%s", LegacyIndexDirectory, CdktfIndexDirectory) {
return true
}

if directory == filepath.FromSlash(fmt.Sprintf("%s/%s", RegistryIndexDirectory, CdktfIndexDirectory)) {
if directory == fmt.Sprintf("%s/%s", RegistryIndexDirectory, CdktfIndexDirectory) {
return true
}

for _, validCdktfLanguage := range ValidCdktfLanguages {

if directory == filepath.FromSlash(fmt.Sprintf("%s/%s/%s", LegacyIndexDirectory, CdktfIndexDirectory, validCdktfLanguage)) {
if directory == fmt.Sprintf("%s/%s/%s", LegacyIndexDirectory, CdktfIndexDirectory, validCdktfLanguage) {
return true
}

if directory == filepath.FromSlash(fmt.Sprintf("%s/%s/%s", RegistryIndexDirectory, CdktfIndexDirectory, validCdktfLanguage)) {
if directory == fmt.Sprintf("%s/%s/%s", RegistryIndexDirectory, CdktfIndexDirectory, validCdktfLanguage) {
return true
}

for _, validLegacySubdirectory := range ValidLegacySubdirectories {
if directory == filepath.FromSlash(fmt.Sprintf("%s/%s/%s/%s", LegacyIndexDirectory, CdktfIndexDirectory, validCdktfLanguage, validLegacySubdirectory)) {
if directory == fmt.Sprintf("%s/%s/%s/%s", LegacyIndexDirectory, CdktfIndexDirectory, validCdktfLanguage, validLegacySubdirectory) {
return true
}
}

for _, validRegistrySubdirectory := range ValidRegistrySubdirectories {
if directory == filepath.FromSlash(fmt.Sprintf("%s/%s/%s/%s", RegistryIndexDirectory, CdktfIndexDirectory, validCdktfLanguage, validRegistrySubdirectory)) {
if directory == fmt.Sprintf("%s/%s/%s/%s", RegistryIndexDirectory, CdktfIndexDirectory, validCdktfLanguage, validRegistrySubdirectory) {
return true
}
}
Expand Down
86 changes: 76 additions & 10 deletions internal/check/directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,94 @@
package check

import (
"os"
"path/filepath"
"io/fs"
"testing"
"testing/fstest"

"github.com/bmatcuk/doublestar/v4"
)

var DocumentationGlobPattern = `{docs/index.md,docs/{,cdktf/}{data-sources,guides,resources,functions}/**/*,website/docs/**/*}`
var DocumentationGlobPattern = `{docs/index.*,docs/{,cdktf/}{data-sources,guides,resources,functions}/**/*,website/docs/**/*}`

func TestMixedDirectoriesCheck(t *testing.T) {
t.Parallel()
testCases := map[string]struct {
BasePath string
ProviderFS fs.FS
ExpectError bool
}{
"valid mixed directories": {
BasePath: filepath.Join("testdata", "valid-mixed-directories"),
ProviderFS: fstest.MapFS{
"docs/nonregistrydocs/thing.md": {},
"website/docs/index.md": {},
},
},
"invalid mixed directories": {
BasePath: filepath.Join("testdata", "invalid-mixed-directories"),
"valid mixed directories - cdktf": {
ProviderFS: fstest.MapFS{
"docs/cdktf/typescript/index.md": {},
"website/docs/index.md": {},
},
},
"invalid mixed directories - registry data source": {
ProviderFS: fstest.MapFS{
"docs/data-sources/invalid.md": {},
"website/docs/index.md": {},
},
ExpectError: true,
},
"invalid mixed directories - registry resource": {
ProviderFS: fstest.MapFS{
"docs/resources/invalid.md": {},
"website/docs/index.md": {},
},
ExpectError: true,
},
"invalid mixed directories - registry guide": {
ProviderFS: fstest.MapFS{
"docs/guides/invalid.md": {},
"website/docs/index.md": {},
},
ExpectError: true,
},
"invalid mixed directories - registry function": {
ProviderFS: fstest.MapFS{
"docs/functions/invalid.md": {},
"website/docs/index.md": {},
},
ExpectError: true,
},
"invalid mixed directories - legacy data source": {
ProviderFS: fstest.MapFS{
"website/docs/d/invalid.html.markdown": {},
"docs/resources/thing.md": {},
},
ExpectError: true,
},
"invalid mixed directories - legacy resource": {
ProviderFS: fstest.MapFS{
"website/docs/r/invalid.html.markdown": {},
"docs/resources/thing.md": {},
},
ExpectError: true,
},
"invalid mixed directories - legacy guide": {
ProviderFS: fstest.MapFS{
"website/docs/guides/invalid.html.markdown": {},
"docs/resources/thing.md": {},
},
ExpectError: true,
},
"invalid mixed directories - legacy function": {
ProviderFS: fstest.MapFS{
"website/docs/functions/invalid.html.markdown": {},
"docs/resources/thing.md": {},
},
ExpectError: true,
},
"invalid mixed directories - legacy index": {
ProviderFS: fstest.MapFS{
"website/docs/index.html.markdown": {},
"docs/resources/thing.md": {},
},
ExpectError: true,
},
}
Expand All @@ -34,9 +102,7 @@ func TestMixedDirectoriesCheck(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()

providerFs := os.DirFS(testCase.BasePath)

files, err := doublestar.Glob(providerFs, DocumentationGlobPattern)
files, err := doublestar.Glob(testCase.ProviderFS, DocumentationGlobPattern)
if err != nil {
t.Fatalf("error finding documentation files: %s", err)
}
Expand Down
8 changes: 4 additions & 4 deletions internal/check/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ package check

import (
"fmt"
"io/fs"
"log"
"os"
"path/filepath"
)

Expand All @@ -23,14 +23,14 @@ func (opts *FileOptions) FullPath(path string) string {
}

// FileSizeCheck verifies that documentation file is below the Terraform Registry storage limit.
func FileSizeCheck(fullpath string) error {
fi, err := os.Stat(fullpath)
func FileSizeCheck(providerFs fs.FS, path string) error {
fi, err := fs.Stat(providerFs, path)

if err != nil {
return err
}

log.Printf("[DEBUG] File %s size: %d (limit: %d)", fullpath, fi.Size(), RegistryMaximumSizeOfFile)
log.Printf("[DEBUG] File %s size: %d (limit: %d)", path, fi.Size(), RegistryMaximumSizeOfFile)
if fi.Size() >= int64(RegistryMaximumSizeOfFile) {
return fmt.Errorf("exceeded maximum (%d) size of documentation file for Terraform Registry: %d", RegistryMaximumSizeOfFile, fi.Size())
}
Expand Down
2 changes: 1 addition & 1 deletion internal/check/file_extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var ValidRegistryFileExtensions = []string{
// FileExtensionCheck checks if the file extension of the given path is valid.
func FileExtensionCheck(path string, validExtensions []string) error {
if !FilePathEndsWithExtensionFrom(path, validExtensions) {
return fmt.Errorf("file does not end with a valid extension, valid extensions: %v", ValidLegacyFileExtensions)
return fmt.Errorf("file does not end with a valid extension, valid extensions: %v", validExtensions)
}

return nil
Expand Down
32 changes: 19 additions & 13 deletions internal/check/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,40 @@
package check

import (
"os"
"io/fs"
"path/filepath"
"testing"
"testing/fstest"
)

func TestFileSizeCheck(t *testing.T) {
t.Parallel()
testCases := map[string]struct {
FileSystem fs.FS
Size int64
ExpectError bool
}{
"under limit": {
Size: RegistryMaximumSizeOfFile - 1,
FileSystem: fstest.MapFS{
"file.md": {
Data: make([]byte, RegistryMaximumSizeOfFile-1),
},
},
},
"on limit": {
Size: RegistryMaximumSizeOfFile,
FileSystem: fstest.MapFS{
"file.md": {
Data: make([]byte, RegistryMaximumSizeOfFile),
},
},
ExpectError: true,
},
"over limit": {
Size: RegistryMaximumSizeOfFile + 1,
FileSystem: fstest.MapFS{
"file.md": {
Data: make([]byte, RegistryMaximumSizeOfFile+1),
},
},
ExpectError: true,
},
}
Expand All @@ -34,15 +48,7 @@ func TestFileSizeCheck(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()

file, _ := os.CreateTemp(t.TempDir(), "TestFileSizeCheck")

defer file.Close()

if err := file.Truncate(testCase.Size); err != nil {
t.Fatalf("error writing temporary file: %s", err)
}

got := FileSizeCheck(file.Name())
got := FileSizeCheck(testCase.FileSystem, "file.md")

if got == nil && testCase.ExpectError {
t.Errorf("expected error, got no error")
Expand Down
23 changes: 13 additions & 10 deletions internal/check/provider_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ package check

import (
"fmt"
"io/fs"
"log"
"os"
"path/filepath"
)

type ProviderFileOptions struct {
Expand All @@ -17,12 +18,14 @@ type ProviderFileOptions struct {
}

type ProviderFileCheck struct {
Options *ProviderFileOptions
Options *ProviderFileOptions
ProviderFs fs.FS
}

func NewProviderFileCheck(opts *ProviderFileOptions) *ProviderFileCheck {
func NewProviderFileCheck(providerFs fs.FS, opts *ProviderFileOptions) *ProviderFileCheck {
check := &ProviderFileCheck{
Options: opts,
Options: opts,
ProviderFs: providerFs,
}

if check.Options == nil {
Expand All @@ -46,21 +49,21 @@ func (check *ProviderFileCheck) Run(path string) error {
log.Printf("[DEBUG] Checking file: %s", fullpath)

if err := FileExtensionCheck(path, check.Options.ValidExtensions); err != nil {
return fmt.Errorf("%s: error checking file extension: %w", path, err)
return fmt.Errorf("%s: error checking file extension: %w", filepath.FromSlash(path), err)
}

if err := FileSizeCheck(fullpath); err != nil {
return fmt.Errorf("%s: error checking file size: %w", path, err)
if err := FileSizeCheck(check.ProviderFs, path); err != nil {
return fmt.Errorf("%s: error checking file size: %w", filepath.FromSlash(path), err)
}

content, err := os.ReadFile(fullpath)
content, err := fs.ReadFile(check.ProviderFs, path)

if err != nil {
return fmt.Errorf("%s: error reading file: %w", path, err)
return fmt.Errorf("%s: error reading file: %w", filepath.FromSlash(path), err)
}

if err := NewFrontMatterCheck(check.Options.FrontMatter).Run(content); err != nil {
return fmt.Errorf("%s: error checking file frontmatter: %w", path, err)
return fmt.Errorf("%s: error checking file frontmatter: %w", filepath.FromSlash(path), err)
}

return nil
Expand Down

This file was deleted.

Loading