From 404b1e0a469a527021b179e4acdaef0e28883c8f Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 19:16:35 +0000 Subject: [PATCH 01/25] refactor: update file structure to reflect new changes --- .github/workflows/ci.yaml | 4 ++-- .gitignore | 4 ++-- .../contributors => cmd/readmevalidation}/contributors.go | 0 {scripts/contributors => cmd/readmevalidation}/main.go | 0 4 files changed, 4 insertions(+), 4 deletions(-) rename {scripts/contributors => cmd/readmevalidation}/contributors.go (100%) rename {scripts/contributors => cmd/readmevalidation}/main.go (100%) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 159e8c9..34778cd 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -17,9 +17,9 @@ jobs: with: go-version: "1.23.2" - name: Validate contributors - run: go build ./scripts/contributors && ./contributors + run: go build ./cmd/readmevalidation && ./readmevalidation - name: Remove build file artifact - run: rm ./contributors + run: rm ./readmevalidation test-terraform: runs-on: ubuntu-latest steps: diff --git a/.gitignore b/.gitignore index 6ee570e..157c642 100644 --- a/.gitignore +++ b/.gitignore @@ -135,8 +135,8 @@ dist .yarn/install-state.gz .pnp.* -# Script output -/contributors +# Things needed for CI +/readmevalidation # Terraform files generated during testing .terraform* diff --git a/scripts/contributors/contributors.go b/cmd/readmevalidation/contributors.go similarity index 100% rename from scripts/contributors/contributors.go rename to cmd/readmevalidation/contributors.go diff --git a/scripts/contributors/main.go b/cmd/readmevalidation/main.go similarity index 100% rename from scripts/contributors/main.go rename to cmd/readmevalidation/main.go From 36d33895b7a425987599165e173583f70c1cc91c Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 19:27:53 +0000 Subject: [PATCH 02/25] refactor: start splitting up files --- cmd/readmevalidation/contributors.go | 84 ++------------------ cmd/readmevalidation/errors.go | 28 +++++++ cmd/readmevalidation/main.go | 2 +- cmd/readmevalidation/readmeFiles.go | 113 +++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 77 deletions(-) create mode 100644 cmd/readmevalidation/errors.go create mode 100644 cmd/readmevalidation/readmeFiles.go diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index 02823f2..4dfdfe4 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -1,7 +1,6 @@ package main import ( - "bufio" "errors" "fmt" "net/url" @@ -13,18 +12,10 @@ import ( "gopkg.in/yaml.v3" ) -const rootRegistryPath = "./registry" - var ( - validContributorStatuses = []string{"official", "partner", "community"} - supportedAvatarFileFormats = []string{".png", ".jpeg", ".jpg", ".gif", ".svg"} + validContributorStatuses = []string{"official", "partner", "community"} ) -type readme struct { - filePath string - rawText string -} - type contributorProfileFrontmatter struct { DisplayName string `yaml:"display_name"` Bio string `yaml:"bio"` @@ -44,61 +35,6 @@ type contributorProfile struct { filePath string } -var _ error = validationPhaseError{} - -type validationPhaseError struct { - phase string - errors []error -} - -func (vpe validationPhaseError) Error() string { - validationStrs := []string{} - for _, e := range vpe.errors { - validationStrs = append(validationStrs, fmt.Sprintf("- %v", e)) - } - slices.Sort(validationStrs) - - msg := fmt.Sprintf("Error during %q phase of README validation:", vpe.phase) - msg += strings.Join(validationStrs, "\n") - msg += "\n" - - return msg -} - -func extractFrontmatter(readmeText string) (string, error) { - if readmeText == "" { - return "", errors.New("README is empty") - } - - const fence = "---" - fm := "" - fenceCount := 0 - lineScanner := bufio.NewScanner( - strings.NewReader(strings.TrimSpace(readmeText)), - ) - for lineScanner.Scan() { - nextLine := lineScanner.Text() - if fenceCount == 0 && nextLine != fence { - return "", errors.New("README does not start with frontmatter fence") - } - - if nextLine != fence { - fm += nextLine + "\n" - continue - } - - fenceCount++ - if fenceCount >= 2 { - break - } - } - - if fenceCount == 1 { - return "", errors.New("README does not have two sets of frontmatter fences") - } - return fm, nil -} - func validateContributorGithubUsername(githubUsername string) error { if githubUsername == "" { return errors.New("missing GitHub username") @@ -260,10 +196,6 @@ func validateContributorAvatarURL(avatarURL *string) []error { return problems } -func addFilePathToError(filePath string, err error) error { - return fmt.Errorf("%q: %v", filePath, err) -} - func validateContributorYaml(yml contributorProfile) []error { allProblems := []error{} @@ -297,7 +229,7 @@ func validateContributorYaml(yml contributorProfile) []error { } func parseContributorProfile(rm readme) (contributorProfile, error) { - fm, err := extractFrontmatter(rm.rawText) + fm, _, err := separateFrontmatter(rm.rawText) if err != nil { return contributorProfile{}, fmt.Errorf("%q: failed to parse frontmatter: %v", rm.filePath, err) } @@ -331,7 +263,7 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil } if len(yamlParsingErrors) != 0 { return nil, validationPhaseError{ - phase: "YAML parsing", + phase: validationPhaseReadmeParsing, errors: yamlParsingErrors, } } @@ -356,11 +288,11 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil if _, found := profilesByUsername[companyName]; found { continue } - yamlValidationErrors = append(yamlValidationErrors, fmt.Errorf("company %q does not exist in %q directory but is referenced by these profiles: [%s]", companyName, rootRegistryPath, strings.Join(group, ", "))) + yamlValidationErrors = append(yamlValidationErrors, fmt.Errorf("%q: company %q does not exist but is referenced by these profiles: [%s]", rootRegistryPath, companyName, strings.Join(group, ", "))) } if len(yamlValidationErrors) != 0 { return nil, validationPhaseError{ - phase: "Raw YAML Validation", + phase: validationPhaseReadmeParsing, errors: yamlValidationErrors, } } @@ -397,7 +329,7 @@ func aggregateContributorReadmeFiles() ([]readme, error) { if len(problems) != 0 { return nil, validationPhaseError{ - phase: "FileSystem reading", + phase: validationPhaseFileLoad, errors: problems, } } @@ -405,7 +337,7 @@ func aggregateContributorReadmeFiles() ([]readme, error) { return allReadmeFiles, nil } -func validateRelativeUrls( +func validateContributorRelativeUrls( contributors map[string]contributorProfile, ) error { // This function only validates relative avatar URLs for now, but it can be @@ -440,7 +372,7 @@ func validateRelativeUrls( return nil } return validationPhaseError{ - phase: "Relative URL validation", + phase: validationPhaseAssetCrossReference, errors: problems, } } diff --git a/cmd/readmevalidation/errors.go b/cmd/readmevalidation/errors.go new file mode 100644 index 0000000..db13edc --- /dev/null +++ b/cmd/readmevalidation/errors.go @@ -0,0 +1,28 @@ +package main + +import "fmt" + +// validationPhaseError represents an error that occurred during a specific +// phase of README validation. It should be used to collect ALL validation +// errors that happened during a specific phase, rather than the first one +// encountered. +type validationPhaseError struct { + phase validationPhase + errors []error +} + +var _ error = validationPhaseError{} + +func (vpe validationPhaseError) Error() string { + msg := fmt.Sprintf("Error during %q phase of README validation:", vpe.phase.String()) + for _, e := range vpe.errors { + msg += fmt.Sprintf("\n- %v", e) + } + msg += "\n" + + return msg +} + +func addFilePathToError(filePath string, err error) error { + return fmt.Errorf("%q: %v", filePath, err) +} diff --git a/cmd/readmevalidation/main.go b/cmd/readmevalidation/main.go index 9091318..bc8209f 100644 --- a/cmd/readmevalidation/main.go +++ b/cmd/readmevalidation/main.go @@ -26,7 +26,7 @@ func main() { log.Panic(err) } - err = validateRelativeUrls(contributors) + err = validateContributorRelativeUrls(contributors) if err != nil { log.Panic(err) } diff --git a/cmd/readmevalidation/readmeFiles.go b/cmd/readmevalidation/readmeFiles.go new file mode 100644 index 0000000..69ccf9f --- /dev/null +++ b/cmd/readmevalidation/readmeFiles.go @@ -0,0 +1,113 @@ +package main + +import ( + "bufio" + "errors" + "fmt" + "strings" +) + +const rootRegistryPath = "./registry" + +var supportedAvatarFileFormats = []string{".png", ".jpeg", ".jpg", ".gif", ".svg"} + +// readme represents a single README file within the repo (usually within the +// top-level "/registry" directory). +type readme struct { + filePath string + rawText string +} + +// separateFrontmatter attempts to separate a README file's frontmatter content +// from the main README body, returning both values in that order. It does not +// validate whether the structure of the frontmatter is valid (i.e., that it's +// structured as YAML). +func separateFrontmatter(readmeText string) (string, string, error) { + if readmeText == "" { + return "", "", errors.New("README is empty") + } + + const fence = "---" + fm := "" + body := "" + fenceCount := 0 + lineScanner := bufio.NewScanner( + strings.NewReader(strings.TrimSpace(readmeText)), + ) + for lineScanner.Scan() { + nextLine := lineScanner.Text() + if fenceCount < 2 && nextLine == fence { + fenceCount++ + continue + } + // Break early if the very first line wasn't a fence, because then we + // know for certain that the README has problems + if fenceCount == 0 { + break + } + + // It should be safe to trim each line of the frontmatter on a per-line + // basis, because there shouldn't be any extra meaning attached to the + // indentation. The same does NOT apply to the README; best we can do is + // gather all the lines, and then trim around it + if inReadmeBody := fenceCount >= 2; inReadmeBody { + body += nextLine + "\n" + } else { + fm += strings.TrimSpace(nextLine) + "\n" + } + } + if fenceCount < 2 { + return "", "", errors.New("README does not have two sets of frontmatter fences") + } + if fm == "" { + return "", "", errors.New("readme has frontmatter fences but no frontmatter content") + } + + return fm, strings.TrimSpace(body), nil +} + +// validationPhase represents a specific phase during README validation. It is +// expected that each phase is discrete, and errors during one will prevent a +// future phase from starting. +type validationPhase int + +const ( + // validationPhaseFileStructureValidation indicates when the entire Registry + // directory is being verified for having all files be placed in the file + // system as expected. + validationPhaseFileStructureValidation validationPhase = iota + + // validationPhaseFileLoad indicates when README files are being read from + // the file system + validationPhaseFileLoad + + // validationPhaseReadmeParsing indicates when a README's frontmatter is + // being parsed as YAML. This phase does not include YAML validation. + validationPhaseReadmeParsing + + // validationPhaseReadmeValidation indicates when a README's frontmatter is + // being validated as proper YAML with expected keys. + validationPhaseReadmeValidation + + // validationPhaseAssetCrossReference indicates when a README's frontmatter + // is having all its relative URLs be validated for whether they point to + // valid resources. + validationPhaseAssetCrossReference +) + +func (p validationPhase) String() string { + switch p { + case validationPhaseFileStructureValidation: + return "File structure validation" + case validationPhaseFileLoad: + return "Filesystem reading" + case validationPhaseReadmeParsing: + return "README parsing" + case validationPhaseReadmeValidation: + return "README validation" + case validationPhaseAssetCrossReference: + return "Cross-referencing relative asset URLs" + default: + return fmt.Sprintf("Unknown validation phase: %d", p) + } +} From 1a5c102c2f49bd5cb92115213aef95e1fea7cf86 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 20:49:04 +0000 Subject: [PATCH 03/25] refactor: more domain splitting --- cmd/readmevalidation/contributors.go | 37 +++++--- cmd/readmevalidation/main.go | 23 +---- cmd/readmevalidation/repoStructure.go | 116 ++++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 32 deletions(-) create mode 100644 cmd/readmevalidation/repoStructure.go diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index 4dfdfe4..ad30d08 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -3,6 +3,7 @@ package main import ( "errors" "fmt" + "log" "net/url" "os" "path" @@ -12,9 +13,7 @@ import ( "gopkg.in/yaml.v3" ) -var ( - validContributorStatuses = []string{"official", "partner", "community"} -) +var validContributorStatuses = []string{"official", "partner", "community"} type contributorProfileFrontmatter struct { DisplayName string `yaml:"display_name"` @@ -48,10 +47,7 @@ func validateContributorGithubUsername(githubUsername string) error { return nil } -func validateContributorEmployerGithubUsername( - employerGithubUsername *string, - githubUsername string, -) []error { +func validateContributorEmployerGithubUsername(employerGithubUsername *string, githubUsername string) []error { if employerGithubUsername == nil { return nil } @@ -337,9 +333,7 @@ func aggregateContributorReadmeFiles() ([]readme, error) { return allReadmeFiles, nil } -func validateContributorRelativeUrls( - contributors map[string]contributorProfile, -) error { +func validateContributorRelativeUrls(contributors map[string]contributorProfile) error { // This function only validates relative avatar URLs for now, but it can be // beefed up to validate more in the future problems := []error{} @@ -376,3 +370,26 @@ func validateContributorRelativeUrls( errors: problems, } } + +func validateAllContributorFiles() error { + allReadmeFiles, err := aggregateContributorReadmeFiles() + if err != nil { + return err + } + + log.Printf("Processing %d README files\n", len(allReadmeFiles)) + contributors, err := parseContributorFiles(allReadmeFiles) + if err != nil { + return err + } + log.Printf("Processed %d README files as valid contributor profiles", len(contributors)) + + err = validateContributorRelativeUrls(contributors) + if err != nil { + return err + } + log.Println("All relative URLs for READMEs are valid") + + log.Printf("Processed all READMEs in the %q directory\n", rootRegistryPath) + return nil +} diff --git a/cmd/readmevalidation/main.go b/cmd/readmevalidation/main.go index bc8209f..7f275c5 100644 --- a/cmd/readmevalidation/main.go +++ b/cmd/readmevalidation/main.go @@ -11,29 +11,8 @@ import ( func main() { log.Println("Starting README validation") - allReadmeFiles, err := aggregateContributorReadmeFiles() + err := validateAllContributorFiles() if err != nil { log.Panic(err) } - - log.Printf("Processing %d README files\n", len(allReadmeFiles)) - contributors, err := parseContributorFiles(allReadmeFiles) - log.Printf( - "Processed %d README files as valid contributor profiles", - len(contributors), - ) - if err != nil { - log.Panic(err) - } - - err = validateContributorRelativeUrls(contributors) - if err != nil { - log.Panic(err) - } - log.Println("All relative URLs for READMEs are valid") - - log.Printf( - "Processed all READMEs in the %q directory\n", - rootRegistryPath, - ) } diff --git a/cmd/readmevalidation/repoStructure.go b/cmd/readmevalidation/repoStructure.go new file mode 100644 index 0000000..ed73910 --- /dev/null +++ b/cmd/readmevalidation/repoStructure.go @@ -0,0 +1,116 @@ +package main + +import ( + "errors" + "fmt" + "os" + "path" +) + +var supportedResourceTypes = []string{"modules", "templates"} + +func validateCoderResourceSubdirectory(dirPath string) []error { + errs := []error{} + + dir, err := os.Stat(dirPath) + if err != nil { + // It's valid for a specific resource directory not to exist. It's just + // that if it does exist, it must follow specific rules + if !errors.Is(err, os.ErrNotExist) { + errs = append(errs, addFilePathToError(dirPath, err)) + } + return errs + } + + if !dir.IsDir() { + errs = append(errs, fmt.Errorf("%q: path is not a directory", dirPath)) + return errs + } + + files, err := os.ReadDir(dirPath) + if err != nil { + errs = append(errs, fmt.Errorf("%q: %v", dirPath, err)) + return errs + } + for _, f := range files { + if !f.IsDir() { + continue + } + + resourceReadmePath := path.Join(dirPath, f.Name(), "README.md") + _, err := os.Stat(resourceReadmePath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + errs = append(errs, fmt.Errorf("%q: 'README.md' does not exist", resourceReadmePath)) + } else { + errs = append(errs, addFilePathToError(resourceReadmePath, err)) + } + } + + mainTerraformPath := path.Join(dirPath, f.Name(), "main.tf") + _, err = os.Stat(mainTerraformPath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + errs = append(errs, fmt.Errorf("%q: 'main.tf' file does not exist", mainTerraformPath)) + } else { + errs = append(errs, addFilePathToError(mainTerraformPath, err)) + } + } + + } + + return errs +} + +func validateRegistryDirectory() []error { + dirEntries, err := os.ReadDir(rootRegistryPath) + if err != nil { + return []error{err} + } + + problems := []error{} + for _, e := range dirEntries { + dirPath := path.Join(rootRegistryPath, e.Name()) + if !e.IsDir() { + problems = append(problems, fmt.Errorf("detected non-directory file %q at base of main Registry directory", dirPath)) + continue + } + + readmePath := path.Join(dirPath, "README.md") + _, err := os.Stat(readmePath) + if err != nil { + problems = append(problems, err) + } + + for _, rType := range supportedResourceTypes { + resourcePath := path.Join(dirPath, rType) + if errs := validateCoderResourceSubdirectory(resourcePath); len(errs) != 0 { + problems = append(problems, errs...) + } + } + } + + return problems +} + +func validateRepoStructure() error { + var problems []error + if errs := validateRegistryDirectory(); len(errs) != 0 { + problems = append(problems, errs...) + } + + _, err := os.Stat("./.icons") + if err != nil { + problems = append(problems, err) + } + + // Todo: figure out what other directories we want to make guarantees for + // and add them to this function + if len(problems) != 0 { + return validationPhaseError{ + phase: validationPhaseFileStructureValidation, + errors: problems, + } + } + return nil +} From 52c1f45da6821821bb6db0efd333f1cceb291323 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 20:51:18 +0000 Subject: [PATCH 04/25] refactor: remove directory validation from contributors file --- cmd/readmevalidation/contributors.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index ad30d08..fd7b30b 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -307,7 +307,6 @@ func aggregateContributorReadmeFiles() ([]readme, error) { for _, e := range dirEntries { dirPath := path.Join(rootRegistryPath, e.Name()) if !e.IsDir() { - problems = append(problems, fmt.Errorf("detected non-directory file %q at base of main Registry directory", dirPath)) continue } From 812dd8faaf4ac3cbc69e9010c8b57e7cd101a7e1 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 21:14:37 +0000 Subject: [PATCH 05/25] fix: update repo structure checks --- cmd/readmevalidation/contributors.go | 84 +++++++++++++-------------- cmd/readmevalidation/main.go | 23 +++++++- cmd/readmevalidation/repoStructure.go | 39 +++++++------ 3 files changed, 84 insertions(+), 62 deletions(-) diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index fd7b30b..36b7bb9 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -52,22 +52,22 @@ func validateContributorEmployerGithubUsername(employerGithubUsername *string, g return nil } - problems := []error{} + errs := []error{} if *employerGithubUsername == "" { - problems = append(problems, errors.New("company_github field is defined but has empty value")) - return problems + errs = append(errs, errors.New("company_github field is defined but has empty value")) + return errs } lower := strings.ToLower(*employerGithubUsername) if uriSafe := url.PathEscape(lower); uriSafe != lower { - problems = append(problems, fmt.Errorf("gitHub company username %q is not a valid URL path segment", *employerGithubUsername)) + errs = append(errs, fmt.Errorf("gitHub company username %q is not a valid URL path segment", *employerGithubUsername)) } if *employerGithubUsername == githubUsername { - problems = append(problems, fmt.Errorf("cannot list own GitHub name (%q) as employer", githubUsername)) + errs = append(errs, fmt.Errorf("cannot list own GitHub name (%q) as employer", githubUsername)) } - return problems + return errs } func validateContributorDisplayName(displayName string) error { @@ -95,7 +95,7 @@ func validateContributorSupportEmail(email *string) []error { return nil } - problems := []error{} + errs := []error{} // Can't 100% validate that this is correct without actually sending // an email, and especially with some contributors being individual @@ -103,31 +103,31 @@ func validateContributorSupportEmail(email *string) []error { // pipeline. Best we can do is verify the general structure username, server, ok := strings.Cut(*email, "@") if !ok { - problems = append(problems, fmt.Errorf("email address %q is missing @ symbol", *email)) - return problems + errs = append(errs, fmt.Errorf("email address %q is missing @ symbol", *email)) + return errs } if username == "" { - problems = append(problems, fmt.Errorf("email address %q is missing username", *email)) + errs = append(errs, fmt.Errorf("email address %q is missing username", *email)) } domain, tld, ok := strings.Cut(server, ".") if !ok { - problems = append(problems, fmt.Errorf("email address %q is missing period for server segment", *email)) - return problems + errs = append(errs, fmt.Errorf("email address %q is missing period for server segment", *email)) + return errs } if domain == "" { - problems = append(problems, fmt.Errorf("email address %q is missing domain", *email)) + errs = append(errs, fmt.Errorf("email address %q is missing domain", *email)) } if tld == "" { - problems = append(problems, fmt.Errorf("email address %q is missing top-level domain", *email)) + errs = append(errs, fmt.Errorf("email address %q is missing top-level domain", *email)) } if strings.Contains(*email, "?") { - problems = append(problems, errors.New("email is not allowed to contain query parameters")) + errs = append(errs, errors.New("email is not allowed to contain query parameters")) } - return problems + return errs } func validateContributorWebsite(websiteURL *string) error { @@ -161,19 +161,19 @@ func validateContributorAvatarURL(avatarURL *string) []error { return nil } - problems := []error{} + errs := []error{} if *avatarURL == "" { - problems = append(problems, errors.New("avatar URL must be omitted or non-empty string")) - return problems + errs = append(errs, errors.New("avatar URL must be omitted or non-empty string")) + return errs } // Have to use .Parse instead of .ParseRequestURI because this is the // one field that's allowed to be a relative URL if _, err := url.Parse(*avatarURL); err != nil { - problems = append(problems, fmt.Errorf("URL %q is not a valid relative or absolute URL", *avatarURL)) + errs = append(errs, fmt.Errorf("URL %q is not a valid relative or absolute URL", *avatarURL)) } if strings.Contains(*avatarURL, "?") { - problems = append(problems, errors.New("avatar URL is not allowed to contain search parameters")) + errs = append(errs, errors.New("avatar URL is not allowed to contain search parameters")) } matched := false @@ -186,42 +186,42 @@ func validateContributorAvatarURL(avatarURL *string) []error { if !matched { segments := strings.Split(*avatarURL, ".") fileExtension := segments[len(segments)-1] - problems = append(problems, fmt.Errorf("avatar URL '.%s' does not end in a supported file format: [%s]", fileExtension, strings.Join(supportedAvatarFileFormats, ", "))) + errs = append(errs, fmt.Errorf("avatar URL '.%s' does not end in a supported file format: [%s]", fileExtension, strings.Join(supportedAvatarFileFormats, ", "))) } - return problems + return errs } func validateContributorYaml(yml contributorProfile) []error { - allProblems := []error{} + allErrs := []error{} if err := validateContributorGithubUsername(yml.frontmatter.GithubUsername); err != nil { - allProblems = append(allProblems, addFilePathToError(yml.filePath, err)) + allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } if err := validateContributorDisplayName(yml.frontmatter.DisplayName); err != nil { - allProblems = append(allProblems, addFilePathToError(yml.filePath, err)) + allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } if err := validateContributorLinkedinURL(yml.frontmatter.LinkedinURL); err != nil { - allProblems = append(allProblems, addFilePathToError(yml.filePath, err)) + allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } if err := validateContributorWebsite(yml.frontmatter.WebsiteURL); err != nil { - allProblems = append(allProblems, addFilePathToError(yml.filePath, err)) + allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } if err := validateContributorStatus(yml.frontmatter.ContributorStatus); err != nil { - allProblems = append(allProblems, addFilePathToError(yml.filePath, err)) + allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } for _, err := range validateContributorEmployerGithubUsername(yml.frontmatter.EmployerGithubUsername, yml.frontmatter.GithubUsername) { - allProblems = append(allProblems, addFilePathToError(yml.filePath, err)) + allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } for _, err := range validateContributorSupportEmail(yml.frontmatter.SupportEmail) { - allProblems = append(allProblems, addFilePathToError(yml.filePath, err)) + allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } for _, err := range validateContributorAvatarURL(yml.frontmatter.AvatarURL) { - allProblems = append(allProblems, addFilePathToError(yml.filePath, err)) + allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } - return allProblems + return allErrs } func parseContributorProfile(rm readme) (contributorProfile, error) { @@ -303,7 +303,7 @@ func aggregateContributorReadmeFiles() ([]readme, error) { } allReadmeFiles := []readme{} - problems := []error{} + errs := []error{} for _, e := range dirEntries { dirPath := path.Join(rootRegistryPath, e.Name()) if !e.IsDir() { @@ -313,7 +313,7 @@ func aggregateContributorReadmeFiles() ([]readme, error) { readmePath := path.Join(dirPath, "README.md") rmBytes, err := os.ReadFile(readmePath) if err != nil { - problems = append(problems, err) + errs = append(errs, err) continue } allReadmeFiles = append(allReadmeFiles, readme{ @@ -322,10 +322,10 @@ func aggregateContributorReadmeFiles() ([]readme, error) { }) } - if len(problems) != 0 { + if len(errs) != 0 { return nil, validationPhaseError{ phase: validationPhaseFileLoad, - errors: problems, + errors: errs, } } @@ -335,7 +335,7 @@ func aggregateContributorReadmeFiles() ([]readme, error) { func validateContributorRelativeUrls(contributors map[string]contributorProfile) error { // This function only validates relative avatar URLs for now, but it can be // beefed up to validate more in the future - problems := []error{} + errs := []error{} for _, con := range contributors { // If the avatar URL is missing, we'll just assume that the Registry @@ -349,7 +349,7 @@ func validateContributorRelativeUrls(contributors map[string]contributorProfile) } if strings.HasPrefix(*con.frontmatter.AvatarURL, "..") { - problems = append(problems, fmt.Errorf("%q: relative avatar URLs cannot be placed outside a user's namespaced directory", con.filePath)) + errs = append(errs, fmt.Errorf("%q: relative avatar URLs cannot be placed outside a user's namespaced directory", con.filePath)) continue } @@ -357,16 +357,16 @@ func validateContributorRelativeUrls(contributors map[string]contributorProfile) *con.frontmatter.AvatarURL _, err := os.ReadFile(absolutePath) if err != nil { - problems = append(problems, fmt.Errorf("%q: relative avatar path %q does not point to image in file system", con.filePath, *con.frontmatter.AvatarURL)) + errs = append(errs, fmt.Errorf("%q: relative avatar path %q does not point to image in file system", con.filePath, *con.frontmatter.AvatarURL)) } } - if len(problems) == 0 { + if len(errs) == 0 { return nil } return validationPhaseError{ phase: validationPhaseAssetCrossReference, - errors: problems, + errors: errs, } } diff --git a/cmd/readmevalidation/main.go b/cmd/readmevalidation/main.go index 7f275c5..2c0f452 100644 --- a/cmd/readmevalidation/main.go +++ b/cmd/readmevalidation/main.go @@ -6,13 +6,34 @@ package main import ( + "fmt" "log" + "os" ) func main() { log.Println("Starting README validation") + + // If there are fundamental problems with how the repo is structured, we + // can't make any guarantees that any further validations will be relevant + // or accurate + repoErr := validateRepoStructure() + if repoErr != nil { + log.Println(repoErr) + os.Exit(1) + } + + errs := []error{} err := validateAllContributorFiles() if err != nil { - log.Panic(err) + errs = append(errs, err) + } + + if len(errs) == 0 { + os.Exit(0) + } + for _, err := range errs { + fmt.Println(err) } + os.Exit(1) } diff --git a/cmd/readmevalidation/repoStructure.go b/cmd/readmevalidation/repoStructure.go index ed73910..995d077 100644 --- a/cmd/readmevalidation/repoStructure.go +++ b/cmd/readmevalidation/repoStructure.go @@ -12,7 +12,7 @@ var supportedResourceTypes = []string{"modules", "templates"} func validateCoderResourceSubdirectory(dirPath string) []error { errs := []error{} - dir, err := os.Stat(dirPath) + subDir, err := os.Stat(dirPath) if err != nil { // It's valid for a specific resource directory not to exist. It's just // that if it does exist, it must follow specific rules @@ -22,18 +22,20 @@ func validateCoderResourceSubdirectory(dirPath string) []error { return errs } - if !dir.IsDir() { + if !subDir.IsDir() { errs = append(errs, fmt.Errorf("%q: path is not a directory", dirPath)) return errs } files, err := os.ReadDir(dirPath) if err != nil { - errs = append(errs, fmt.Errorf("%q: %v", dirPath, err)) + errs = append(errs, addFilePathToError(dirPath, err)) return errs } for _, f := range files { - if !f.IsDir() { + // The .coder file is sometimes generated as part of Bun tests. These + // directories will never be committed to + if !f.IsDir() || f.Name() == ".coder" { continue } @@ -63,34 +65,35 @@ func validateCoderResourceSubdirectory(dirPath string) []error { } func validateRegistryDirectory() []error { - dirEntries, err := os.ReadDir(rootRegistryPath) + userDirs, err := os.ReadDir(rootRegistryPath) if err != nil { return []error{err} } - problems := []error{} - for _, e := range dirEntries { - dirPath := path.Join(rootRegistryPath, e.Name()) - if !e.IsDir() { - problems = append(problems, fmt.Errorf("detected non-directory file %q at base of main Registry directory", dirPath)) + allErrs := []error{} + for _, d := range userDirs { + dirPath := path.Join(rootRegistryPath, d.Name()) + if !d.IsDir() { + allErrs = append(allErrs, fmt.Errorf("detected non-directory file %q at base of main Registry directory", dirPath)) continue } - readmePath := path.Join(dirPath, "README.md") - _, err := os.Stat(readmePath) + contributorReadmePath := path.Join(dirPath, "README.md") + _, err := os.Stat(contributorReadmePath) if err != nil { - problems = append(problems, err) + allErrs = append(allErrs, err) } for _, rType := range supportedResourceTypes { resourcePath := path.Join(dirPath, rType) - if errs := validateCoderResourceSubdirectory(resourcePath); len(errs) != 0 { - problems = append(problems, errs...) + errs := validateCoderResourceSubdirectory(resourcePath) + if len(errs) != 0 { + allErrs = append(allErrs, errs...) } } } - return problems + return allErrs } func validateRepoStructure() error { @@ -101,11 +104,9 @@ func validateRepoStructure() error { _, err := os.Stat("./.icons") if err != nil { - problems = append(problems, err) + problems = append(problems, errors.New("missing top-level .icons directory (used for storing reusable Coder resource icons)")) } - // Todo: figure out what other directories we want to make guarantees for - // and add them to this function if len(problems) != 0 { return validationPhaseError{ phase: validationPhaseFileStructureValidation, From 1dcc645b14f8e309e4e662701591de855c1f1622 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 21:26:26 +0000 Subject: [PATCH 06/25] fix: improve check for user namespace subdirectories --- cmd/readmevalidation/repoStructure.go | 38 ++++++++++++++++--- .../{ => modules}/apache-airflow/README.md | 0 .../{ => modules}/apache-airflow/main.tf | 0 .../{ => modules}/apache-airflow/run.sh | 0 4 files changed, 32 insertions(+), 6 deletions(-) rename registry/nataindata/{ => modules}/apache-airflow/README.md (100%) rename registry/nataindata/{ => modules}/apache-airflow/main.tf (100%) rename registry/nataindata/{ => modules}/apache-airflow/run.sh (100%) diff --git a/cmd/readmevalidation/repoStructure.go b/cmd/readmevalidation/repoStructure.go index 995d077..f504f08 100644 --- a/cmd/readmevalidation/repoStructure.go +++ b/cmd/readmevalidation/repoStructure.go @@ -5,9 +5,14 @@ import ( "fmt" "os" "path" + "slices" + "strings" ) -var supportedResourceTypes = []string{"modules", "templates"} +var ( + supportedResourceTypes = []string{"modules", "templates"} + supportedUserNameSpaceDirectories = append(supportedResourceTypes[:], ".icons", ".images") +) func validateCoderResourceSubdirectory(dirPath string) []error { errs := []error{} @@ -84,11 +89,32 @@ func validateRegistryDirectory() []error { allErrs = append(allErrs, err) } - for _, rType := range supportedResourceTypes { - resourcePath := path.Join(dirPath, rType) - errs := validateCoderResourceSubdirectory(resourcePath) - if len(errs) != 0 { - allErrs = append(allErrs, errs...) + files, err := os.ReadDir(dirPath) + if err != nil { + allErrs = append(allErrs, err) + continue + } + + for _, f := range files { + // Todo: Decide if there's anything more formal that we want to + // ensure about non-directories scoped to user namespaces + if !f.IsDir() { + continue + } + + segment := f.Name() + filePath := path.Join(dirPath, segment) + + if !slices.Contains(supportedUserNameSpaceDirectories, segment) { + allErrs = append(allErrs, fmt.Errorf("%q: only these sub-directories are allowed at top of user namespace: [%s]", filePath, strings.Join(supportedUserNameSpaceDirectories, ", "))) + continue + } + + if slices.Contains(supportedResourceTypes, segment) { + errs := validateCoderResourceSubdirectory(filePath) + if len(errs) != 0 { + allErrs = append(allErrs, errs...) + } } } } diff --git a/registry/nataindata/apache-airflow/README.md b/registry/nataindata/modules/apache-airflow/README.md similarity index 100% rename from registry/nataindata/apache-airflow/README.md rename to registry/nataindata/modules/apache-airflow/README.md diff --git a/registry/nataindata/apache-airflow/main.tf b/registry/nataindata/modules/apache-airflow/main.tf similarity index 100% rename from registry/nataindata/apache-airflow/main.tf rename to registry/nataindata/modules/apache-airflow/main.tf diff --git a/registry/nataindata/apache-airflow/run.sh b/registry/nataindata/modules/apache-airflow/run.sh similarity index 100% rename from registry/nataindata/apache-airflow/run.sh rename to registry/nataindata/modules/apache-airflow/run.sh From eea1e059f0c5d5590fb08e28e555e9c2e3a44f69 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 21:29:21 +0000 Subject: [PATCH 07/25] docs: add missing words to comment --- cmd/readmevalidation/repoStructure.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/readmevalidation/repoStructure.go b/cmd/readmevalidation/repoStructure.go index f504f08..6e68f4a 100644 --- a/cmd/readmevalidation/repoStructure.go +++ b/cmd/readmevalidation/repoStructure.go @@ -39,7 +39,9 @@ func validateCoderResourceSubdirectory(dirPath string) []error { } for _, f := range files { // The .coder file is sometimes generated as part of Bun tests. These - // directories will never be committed to + // directories will never be committed to the repo, but in the off + // chance that they don't get cleaned up properly, we want to skip over + // them if !f.IsDir() || f.Name() == ".coder" { continue } From fdc0f98c0f379c23a996480dc33db04cd69be626 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 21:29:42 +0000 Subject: [PATCH 08/25] docs: update typo --- cmd/readmevalidation/repoStructure.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/readmevalidation/repoStructure.go b/cmd/readmevalidation/repoStructure.go index 6e68f4a..164547f 100644 --- a/cmd/readmevalidation/repoStructure.go +++ b/cmd/readmevalidation/repoStructure.go @@ -38,10 +38,10 @@ func validateCoderResourceSubdirectory(dirPath string) []error { return errs } for _, f := range files { - // The .coder file is sometimes generated as part of Bun tests. These - // directories will never be committed to the repo, but in the off - // chance that they don't get cleaned up properly, we want to skip over - // them + // The .coder subdirectories are sometimes generated as part of Bun + // tests. These subdirectories will never be committed to the repo, but + // in the off chance that they don't get cleaned up properly, we want to + // skip over them if !f.IsDir() || f.Name() == ".coder" { continue } From 31c69e28a8dc45b0a43543b254c68df483a79fe2 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 21:43:52 +0000 Subject: [PATCH 09/25] refactor: make code easier to read --- cmd/readmevalidation/contributors.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index 36b7bb9..85a95f0 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -343,8 +343,10 @@ func validateContributorRelativeUrls(contributors map[string]contributorProfile) if con.frontmatter.AvatarURL == nil { continue } - if isRelativeURL := strings.HasPrefix(*con.frontmatter.AvatarURL, ".") || - strings.HasPrefix(*con.frontmatter.AvatarURL, "/"); !isRelativeURL { + + isRelativeURL := strings.HasPrefix(*con.frontmatter.AvatarURL, ".") || + strings.HasPrefix(*con.frontmatter.AvatarURL, "/") + if !isRelativeURL { continue } From 1634c40b2782f3ef92a6749eea3222a4d8b007e9 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 22:05:49 +0000 Subject: [PATCH 10/25] chore: add simplified version of module validation logic --- cmd/readmevalidation/coderResources.go | 119 +++++++++++++++++++++++++ cmd/readmevalidation/contributors.go | 52 +++++------ cmd/readmevalidation/readmeFiles.go | 8 ++ cmd/readmevalidation/repoStructure.go | 5 +- 4 files changed, 154 insertions(+), 30 deletions(-) create mode 100644 cmd/readmevalidation/coderResources.go diff --git a/cmd/readmevalidation/coderResources.go b/cmd/readmevalidation/coderResources.go new file mode 100644 index 0000000..51ef9ad --- /dev/null +++ b/cmd/readmevalidation/coderResources.go @@ -0,0 +1,119 @@ +package main + +import ( + "errors" + "fmt" + "net/url" + "strings" +) + +var supportedResourceTypes = []string{"modules", "templates"} + +type coderResourceFrontmatter struct { + Description string `yaml:"description"` + IconURL string `yaml:"icon"` + DisplayName *string `yaml:"display_name"` + Verified *bool `yaml:"verified"` + Tags []string `yaml:"tags"` +} + +// coderResourceReadme represents a README describing a Terraform resource used +// to help create Coder workspaces. As of 2025-04-15, this encapsulates both +// Coder Modules and Coder Templates +type coderResourceReadme struct { + resourceType string + filePath string + body string + frontmatter coderResourceFrontmatter +} + +func validateCoderResourceDisplayName(displayName *string) error { + if displayName != nil && *displayName == "" { + return errors.New("if defined, display_name must not be empty string") + } + return nil +} + +func validateCoderResourceDescription(description string) error { + if description == "" { + return errors.New("frontmatter description cannot be empty") + } + return nil +} + +func validateCoderResourceIconURL(iconURL string) []error { + problems := []error{} + + if iconURL == "" { + problems = append(problems, errors.New("icon URL cannot be empty")) + return problems + } + + isAbsoluteURL := !strings.HasPrefix(iconURL, ".") && !strings.HasPrefix(iconURL, "/") + if isAbsoluteURL { + if _, err := url.ParseRequestURI(iconURL); err != nil { + problems = append(problems, errors.New("absolute icon URL is not correctly formatted")) + } + if strings.Contains(iconURL, "?") { + problems = append(problems, errors.New("icon URLs cannot contain query parameters")) + } + return problems + } + + // Would normally be skittish about having relative paths like this, but it + // should be safe because we have guarantees about the structure of the + // repo, and where this logic will run + isPermittedRelativeURL := strings.HasPrefix(iconURL, "./") || + strings.HasPrefix(iconURL, "/") || + strings.HasPrefix(iconURL, "../../../../.icons") + if !isPermittedRelativeURL { + problems = append(problems, fmt.Errorf("relative icon URL %q must either be scoped to that module's directory, or the top-level /.icons directory (this can usually be done by starting the path with \"../../../.icons\")", iconURL)) + } + + return problems +} + +func validateCoderResourceTags(tags []string) error { + if len(tags) == 0 { + return nil + } + + // All of these tags are used for the module/template filter controls in the + // Registry site. Need to make sure they can all be placed in the browser + // URL without issue + invalidTags := []string{} + for _, t := range tags { + if t != url.QueryEscape(t) { + invalidTags = append(invalidTags, t) + } + } + + if len(invalidTags) != 0 { + return fmt.Errorf("found invalid tags (tags that cannot be used for filter state in the Registry website): [%s]", strings.Join(invalidTags, ", ")) + } + return nil +} + +func validateCoderResourceChanges(resource coderResourceReadme) []error { + var errs []error + + if err := validateReadmeBody(resource.body); err != nil { + errs = append(errs, addFilePathToError(resource.filePath, err)) + } + + if err := validateCoderResourceDisplayName(resource.frontmatter.DisplayName); err != nil { + errs = append(errs, addFilePathToError(resource.filePath, err)) + } + if err := validateCoderResourceDescription(resource.frontmatter.Description); err != nil { + errs = append(errs, addFilePathToError(resource.filePath, err)) + } + if err := validateCoderResourceTags(resource.frontmatter.Tags); err != nil { + errs = append(errs, addFilePathToError(resource.filePath, err)) + } + + for _, err := range validateCoderResourceIconURL(resource.frontmatter.IconURL) { + errs = append(errs, addFilePathToError(resource.filePath, err)) + } + + return errs +} diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index 85a95f0..10f2544 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -29,7 +29,7 @@ type contributorProfileFrontmatter struct { ContributorStatus *string `yaml:"status"` } -type contributorProfile struct { +type contributorProfileReadme struct { frontmatter contributorProfileFrontmatter filePath string } @@ -192,57 +192,57 @@ func validateContributorAvatarURL(avatarURL *string) []error { return errs } -func validateContributorYaml(yml contributorProfile) []error { +func validateContributorReadme(rm contributorProfileReadme) []error { allErrs := []error{} - if err := validateContributorGithubUsername(yml.frontmatter.GithubUsername); err != nil { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) + if err := validateContributorGithubUsername(rm.frontmatter.GithubUsername); err != nil { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } - if err := validateContributorDisplayName(yml.frontmatter.DisplayName); err != nil { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) + if err := validateContributorDisplayName(rm.frontmatter.DisplayName); err != nil { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } - if err := validateContributorLinkedinURL(yml.frontmatter.LinkedinURL); err != nil { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) + if err := validateContributorLinkedinURL(rm.frontmatter.LinkedinURL); err != nil { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } - if err := validateContributorWebsite(yml.frontmatter.WebsiteURL); err != nil { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) + if err := validateContributorWebsite(rm.frontmatter.WebsiteURL); err != nil { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } - if err := validateContributorStatus(yml.frontmatter.ContributorStatus); err != nil { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) + if err := validateContributorStatus(rm.frontmatter.ContributorStatus); err != nil { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } - for _, err := range validateContributorEmployerGithubUsername(yml.frontmatter.EmployerGithubUsername, yml.frontmatter.GithubUsername) { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) + for _, err := range validateContributorEmployerGithubUsername(rm.frontmatter.EmployerGithubUsername, rm.frontmatter.GithubUsername) { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } - for _, err := range validateContributorSupportEmail(yml.frontmatter.SupportEmail) { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) + for _, err := range validateContributorSupportEmail(rm.frontmatter.SupportEmail) { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } - for _, err := range validateContributorAvatarURL(yml.frontmatter.AvatarURL) { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) + for _, err := range validateContributorAvatarURL(rm.frontmatter.AvatarURL) { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } return allErrs } -func parseContributorProfile(rm readme) (contributorProfile, error) { +func parseContributorProfile(rm readme) (contributorProfileReadme, error) { fm, _, err := separateFrontmatter(rm.rawText) if err != nil { - return contributorProfile{}, fmt.Errorf("%q: failed to parse frontmatter: %v", rm.filePath, err) + return contributorProfileReadme{}, fmt.Errorf("%q: failed to parse frontmatter: %v", rm.filePath, err) } yml := contributorProfileFrontmatter{} if err := yaml.Unmarshal([]byte(fm), &yml); err != nil { - return contributorProfile{}, fmt.Errorf("%q: failed to parse: %v", rm.filePath, err) + return contributorProfileReadme{}, fmt.Errorf("%q: failed to parse: %v", rm.filePath, err) } - return contributorProfile{ + return contributorProfileReadme{ filePath: rm.filePath, frontmatter: yml, }, nil } -func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfile, error) { - profilesByUsername := map[string]contributorProfile{} +func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfileReadme, error) { + profilesByUsername := map[string]contributorProfileReadme{} yamlParsingErrors := []error{} for _, rm := range readmeEntries { p, err := parseContributorProfile(rm) @@ -267,7 +267,7 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil employeeGithubGroups := map[string][]string{} yamlValidationErrors := []error{} for _, p := range profilesByUsername { - errors := validateContributorYaml(p) + errors := validateContributorReadme(p) if len(errors) > 0 { yamlValidationErrors = append(yamlValidationErrors, errors...) continue @@ -332,7 +332,7 @@ func aggregateContributorReadmeFiles() ([]readme, error) { return allReadmeFiles, nil } -func validateContributorRelativeUrls(contributors map[string]contributorProfile) error { +func validateContributorRelativeUrls(contributors map[string]contributorProfileReadme) error { // This function only validates relative avatar URLs for now, but it can be // beefed up to validate more in the future errs := []error{} diff --git a/cmd/readmevalidation/readmeFiles.go b/cmd/readmevalidation/readmeFiles.go index 69ccf9f..a0387c4 100644 --- a/cmd/readmevalidation/readmeFiles.go +++ b/cmd/readmevalidation/readmeFiles.go @@ -66,6 +66,14 @@ func separateFrontmatter(readmeText string) (string, string, error) { return fm, strings.TrimSpace(body), nil } +func validateReadmeBody(body string) error { + trimmed := strings.TrimSpace(body) + if !strings.HasPrefix(trimmed, "# ") { + return errors.New("README body must start with ATX-style h1 header (i.e., \"# \")") + } + return nil +} + // validationPhase represents a specific phase during README validation. It is // expected that each phase is discrete, and errors during one will prevent a // future phase from starting. diff --git a/cmd/readmevalidation/repoStructure.go b/cmd/readmevalidation/repoStructure.go index 164547f..11bd920 100644 --- a/cmd/readmevalidation/repoStructure.go +++ b/cmd/readmevalidation/repoStructure.go @@ -9,10 +9,7 @@ import ( "strings" ) -var ( - supportedResourceTypes = []string{"modules", "templates"} - supportedUserNameSpaceDirectories = append(supportedResourceTypes[:], ".icons", ".images") -) +var supportedUserNameSpaceDirectories = append(supportedResourceTypes[:], ".icons", ".images") func validateCoderResourceSubdirectory(dirPath string) []error { errs := []error{} From 05f58e31a7641acb04a5abe100fcad3ab010b5f9 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Sat, 19 Apr 2025 00:13:10 +0000 Subject: [PATCH 11/25] wip: get initial version of h1 validation done --- cmd/readmevalidation/coderResources.go | 125 ++++++++++++++++++++++--- cmd/readmevalidation/readmeFiles.go | 73 +++++++++++++-- 2 files changed, 181 insertions(+), 17 deletions(-) diff --git a/cmd/readmevalidation/coderResources.go b/cmd/readmevalidation/coderResources.go index 51ef9ad..498618e 100644 --- a/cmd/readmevalidation/coderResources.go +++ b/cmd/readmevalidation/coderResources.go @@ -1,9 +1,11 @@ package main import ( + "bufio" "errors" "fmt" "net/url" + "regexp" "strings" ) @@ -94,25 +96,126 @@ func validateCoderResourceTags(tags []string) error { return nil } -func validateCoderResourceChanges(resource coderResourceReadme) []error { +// Todo: This is a holdover from the validation logic used by the Coder Modules +// repo. It gives us some assurance, but realistically, we probably want to +// parse any Terraform code snippets, and make some deeper guarantees about how +// it's structured. Just validating whether it *can* be parsed as Terraform +// would be a big improvement. +var terraformVersionRe = regexp.MustCompile("^\\bversion\\s+=") + +// This validation function definitely has risks of false positives right now, +// but realistically, it's not going to cause problems for launch. The most +// foolproof way to rebuild this would be to parse each README into an AST, and +// then parse each Terraform code block as Terraform +func validateCoderResourceReadmeBody(body string) []error { + trimmed := strings.TrimSpace(body) + var errs []error + errs = append(errs, validateReadmeBody(trimmed)...) + + foundParagraph := false + terraformCodeBlockCount := 0 + foundTerraformVersionRef := false + + lineNum := 0 + isInsideCodeBlock := false + isInsideTerraform := false + + lineScanner := bufio.NewScanner(strings.NewReader(trimmed)) + for lineScanner.Scan() { + lineNum++ + nextLine := lineScanner.Text() + + // Code assumes that invalid headers would've already been handled by + // the base validation function, so we don't need to check deeper if the + // first line isn't an h1 + if lineNum == 1 && !strings.HasPrefix(nextLine, "# ") { + break + } + + if nextLine == "```" { + if !isInsideCodeBlock { + errs = append(errs, fmt.Errorf("line %d: found stray ``` (either an extra code block terminator, or a code block header without a specified language)", lineNum)) + break + } + + isInsideCodeBlock = false + isInsideTerraform = false + continue + } + + if isInsideTerraform { + foundTerraformVersionRef = foundTerraformVersionRef || terraformVersionRe.MatchString(nextLine) + continue + } + if isInsideCodeBlock { + continue + } + + // Code assumes that we can treat this case as the end of the "h1 + // section" and don't need to process any further lines + if strings.HasPrefix(nextLine, "#") { + break + } + + // This is meant to catch cases like ```tf, ```hcl, and ```js + if strings.HasPrefix(nextLine, "```") { + isInsideCodeBlock = true + isInsideTerraform = strings.HasPrefix(nextLine, "```tf") + if isInsideTerraform { + terraformCodeBlockCount++ + } + + if strings.HasPrefix(nextLine, "```hcl") { + errs = append(errs, fmt.Errorf("line %d: all .hcl language references must be converted to .tf", lineNum)) + } + + continue + } + + // Code assumes that if we've reached this point, the only other options + // are: (1) empty spaces, (2) paragraphs, (3) HTML, and (4) asset + // references made via [] syntax + trimmedLine := strings.TrimSpace(nextLine) + isParagraph := trimmedLine != "" && !strings.HasPrefix(trimmedLine, "[") && !strings.HasPrefix(trimmedLine, "<") + foundParagraph = foundParagraph || isParagraph + } + + if terraformCodeBlockCount == 0 { + errs = append(errs, errors.New("did not find Terraform code block within h1 section")) + } else { + if terraformCodeBlockCount > 1 { + errs = append(errs, errors.New("cannot have more than one Terraform code block in h1 section")) + } + if !foundTerraformVersionRef { + errs = append(errs, errors.New("did not find Terraform code block that specifies 'version' field")) + } + } + if !foundParagraph { + errs = append(errs, errors.New("did not find paragraph within h1 section")) + } + + return errs +} + +func validateCoderResourceReadme(rm coderResourceReadme) []error { var errs []error - if err := validateReadmeBody(resource.body); err != nil { - errs = append(errs, addFilePathToError(resource.filePath, err)) + for _, err := range validateCoderResourceReadmeBody(rm.body) { + errs = append(errs, addFilePathToError(rm.filePath, err)) } - if err := validateCoderResourceDisplayName(resource.frontmatter.DisplayName); err != nil { - errs = append(errs, addFilePathToError(resource.filePath, err)) + if err := validateCoderResourceDisplayName(rm.frontmatter.DisplayName); err != nil { + errs = append(errs, addFilePathToError(rm.filePath, err)) } - if err := validateCoderResourceDescription(resource.frontmatter.Description); err != nil { - errs = append(errs, addFilePathToError(resource.filePath, err)) + if err := validateCoderResourceDescription(rm.frontmatter.Description); err != nil { + errs = append(errs, addFilePathToError(rm.filePath, err)) } - if err := validateCoderResourceTags(resource.frontmatter.Tags); err != nil { - errs = append(errs, addFilePathToError(resource.filePath, err)) + if err := validateCoderResourceTags(rm.frontmatter.Tags); err != nil { + errs = append(errs, addFilePathToError(rm.filePath, err)) } - for _, err := range validateCoderResourceIconURL(resource.frontmatter.IconURL) { - errs = append(errs, addFilePathToError(resource.filePath, err)) + for _, err := range validateCoderResourceIconURL(rm.frontmatter.IconURL) { + errs = append(errs, addFilePathToError(rm.filePath, err)) } return errs diff --git a/cmd/readmevalidation/readmeFiles.go b/cmd/readmevalidation/readmeFiles.go index a0387c4..c505dc6 100644 --- a/cmd/readmevalidation/readmeFiles.go +++ b/cmd/readmevalidation/readmeFiles.go @@ -4,6 +4,7 @@ import ( "bufio" "errors" "fmt" + "regexp" "strings" ) @@ -31,9 +32,8 @@ func separateFrontmatter(readmeText string) (string, string, error) { fm := "" body := "" fenceCount := 0 - lineScanner := bufio.NewScanner( - strings.NewReader(strings.TrimSpace(readmeText)), - ) + + lineScanner := bufio.NewScanner(strings.NewReader(strings.TrimSpace(readmeText))) for lineScanner.Scan() { nextLine := lineScanner.Text() if fenceCount < 2 && nextLine == fence { @@ -66,12 +66,73 @@ func separateFrontmatter(readmeText string) (string, string, error) { return fm, strings.TrimSpace(body), nil } -func validateReadmeBody(body string) error { +var readmeHeaderRe = regexp.MustCompile("^(#{1,})(\\s*)") + +// Todo: This is a little chaotic, and might have some risks of false positives. +// Might be better to bring in an +func validateReadmeBody(body string) []error { trimmed := strings.TrimSpace(body) + var errs []error + + // If the very first line of the README, there's a risk that the rest of the + // validation logic will break, since we don't have many guarantees about + // how the README is actually structured if !strings.HasPrefix(trimmed, "# ") { - return errors.New("README body must start with ATX-style h1 header (i.e., \"# \")") + errs = append(errs, errors.New("README body must start with ATX-style h1 header (i.e., \"# \")")) + return errs + } + + lineNum := 0 + lastHeaderLevel := 0 + foundFirstH1 := false + + lineScanner := bufio.NewScanner(strings.NewReader(trimmed)) + for lineScanner.Scan() { + lineNum++ + nextLine := lineScanner.Text() + + headerGroups := readmeHeaderRe.FindStringSubmatch(nextLine) + if headerGroups == nil { + continue + } + + spaceAfterHeader := headerGroups[2] + if spaceAfterHeader == "" { + errs = append(errs, fmt.Errorf("line %d: header does not have space between header characters and main header text", lineNum)) + } + + nextHeaderLevel := len(headerGroups[1]) + if nextHeaderLevel == 1 && !foundFirstH1 { + foundFirstH1 = true + lastHeaderLevel = 1 + continue + } + + // If we have obviously invalid headers, it's not really safe to keep + // proceeding with the rest of the content + if nextHeaderLevel == 1 { + errs = append(errs, errors.New("READMEs cannot contain more than h1 header")) + break + } + if nextHeaderLevel > 6 { + errs = append(errs, fmt.Errorf("line %d: README/HTML files cannot have headers exceed level 6 (found level %d)", lineNum, nextHeaderLevel)) + break + } + + // This is something we need to enforce for accessibility, not just for + // the Registry website, but also when users are viewing the README + // files in the GitHub web view + if nextHeaderLevel > lastHeaderLevel && nextHeaderLevel != (lastHeaderLevel+1) { + errs = append(errs, fmt.Errorf("line %d: headers are not allowed to increase more than 1 level at a time", lineNum)) + continue + } + + // As long as the above condition passes, there's no problems with + // going up a header level or going down 1+ header levels + lastHeaderLevel = nextHeaderLevel } - return nil + + return errs } // validationPhase represents a specific phase during README validation. It is From 492684953ece9b3eb2911920e5cb54574edd1344 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Sat, 19 Apr 2025 01:13:25 +0000 Subject: [PATCH 12/25] wip: commit progress on module validation --- cmd/readmevalidation/coderResources.go | 149 +++++++++++++++++++++++++ cmd/readmevalidation/main.go | 7 +- 2 files changed, 155 insertions(+), 1 deletion(-) diff --git a/cmd/readmevalidation/coderResources.go b/cmd/readmevalidation/coderResources.go index 498618e..cd811c3 100644 --- a/cmd/readmevalidation/coderResources.go +++ b/cmd/readmevalidation/coderResources.go @@ -4,9 +4,15 @@ import ( "bufio" "errors" "fmt" + "log" "net/url" + "os" + "path" "regexp" + "slices" "strings" + + "gopkg.in/yaml.v3" ) var supportedResourceTypes = []string{"modules", "templates"} @@ -29,6 +35,13 @@ type coderResourceReadme struct { frontmatter coderResourceFrontmatter } +type coderResourceReadmes map[string]coderResourceReadme + +func (crr coderResourceReadmes) Get(filePath string) (coderResourceReadme, bool) { + rm, ok := crr[filePath] + return rm, ok +} + func validateCoderResourceDisplayName(displayName *string) error { if displayName != nil && *displayName == "" { return errors.New("if defined, display_name must not be empty string") @@ -220,3 +233,139 @@ func validateCoderResourceReadme(rm coderResourceReadme) []error { return errs } + +func parseCoderResourceReadme(resourceType string, rm readme) (coderResourceReadme, error) { + fm, body, err := separateFrontmatter(rm.rawText) + if err != nil { + return coderResourceReadme{}, fmt.Errorf("%q: failed to parse frontmatter: %v", rm.filePath, err) + } + + yml := coderResourceFrontmatter{} + if err := yaml.Unmarshal([]byte(fm), &yml); err != nil { + return coderResourceReadme{}, fmt.Errorf("%q: failed to parse: %v", rm.filePath, err) + } + + return coderResourceReadme{ + resourceType: resourceType, + filePath: rm.filePath, + body: body, + frontmatter: yml, + }, nil +} + +func parseCoderResourceReadmeFiles(resourceType string, rms []readme) (coderResourceReadmes, error) { + resources := coderResourceReadmes(map[string]coderResourceReadme{}) + var yamlParsingErrs []error + for _, rm := range rms { + p, err := parseCoderResourceReadme(resourceType, rm) + if err != nil { + yamlParsingErrs = append(yamlParsingErrs, err) + continue + } + + resources[p.filePath] = p + } + if len(yamlParsingErrs) != 0 { + return nil, validationPhaseError{ + phase: validationPhaseReadmeParsing, + errors: yamlParsingErrs, + } + } + + yamlValidationErrors := []error{} + for _, readme := range resources { + errors := validateCoderResourceReadme(readme) + if len(errors) > 0 { + yamlValidationErrors = append(yamlValidationErrors, errors...) + } + } + if len(yamlValidationErrors) != 0 { + return nil, validationPhaseError{ + phase: validationPhaseReadmeParsing, + errors: yamlValidationErrors, + } + } + + return resources, nil +} + +// Todo: Need to beef up this function by grabbing each image/video URL from +// the body's AST +func validateCoderResourceRelativeUrls(resources coderResourceReadmes) error { + return nil +} + +func aggregateCoderResourceReadmeFiles(resourceType string) ([]readme, error) { + registryFiles, err := os.ReadDir(rootRegistryPath) + if err != nil { + return nil, err + } + + var allReadmeFiles []readme + var errs []error + for _, rf := range registryFiles { + if !rf.IsDir() { + continue + } + + resourceRootPath := path.Join(rootRegistryPath, rf.Name(), resourceType) + resourceDirs, err := os.ReadDir(resourceRootPath) + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + errs = append(errs, err) + } + continue + } + + for _, rd := range resourceDirs { + if !rd.IsDir() || rd.Name() == ".coder" { + continue + } + + resourceReadmePath := path.Join(resourceRootPath, rd.Name(), "README.md") + rm, err := os.ReadFile(resourceReadmePath) + if err != nil { + errs = append(errs, err) + continue + } + + allReadmeFiles = append(allReadmeFiles, readme{ + filePath: resourceReadmePath, + rawText: string(rm), + }) + } + } + + if len(errs) != 0 { + return nil, validationPhaseError{ + phase: validationPhaseFileLoad, + errors: errs, + } + } + return allReadmeFiles, nil +} + +func validateAllCoderResourceFilesOfType(resourceType string) error { + if !slices.Contains(supportedResourceTypes, resourceType) { + return fmt.Errorf("resource type %q is not part of supported list [%s]", resourceType, strings.Join(supportedResourceTypes, ", ")) + } + + allReadmeFiles, err := aggregateCoderResourceReadmeFiles(resourceType) + if err != nil { + return err + } + + log.Printf("Processing %d README files\n", len(allReadmeFiles)) + resources, err := parseCoderResourceReadmeFiles(resourceType, allReadmeFiles) + if err != nil { + return err + } + log.Printf("Processed %d README files as valid Coder resources with type %q", len(resources), resourceType) + + err = validateCoderResourceRelativeUrls(resources) + if err != nil { + return err + } + log.Printf("All relative URLs for %s READMEs are valid\n", resourceType) + return nil +} diff --git a/cmd/readmevalidation/main.go b/cmd/readmevalidation/main.go index 2c0f452..6f33f74 100644 --- a/cmd/readmevalidation/main.go +++ b/cmd/readmevalidation/main.go @@ -23,13 +23,18 @@ func main() { os.Exit(1) } - errs := []error{} + var errs []error err := validateAllContributorFiles() if err != nil { errs = append(errs, err) } + err = validateAllCoderResourceFilesOfType("modules") + if err != nil { + errs = append(errs, err) + } if len(errs) == 0 { + log.Printf("Processed all READMEs in the %q directory\n", rootRegistryPath) os.Exit(0) } for _, err := range errs { From e350be43ab5c8d1efefca0b800a3b05014442d96 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Sat, 19 Apr 2025 01:57:47 +0000 Subject: [PATCH 13/25] fix: update base README logic to ignore code blocks --- cmd/readmevalidation/coderResources.go | 8 ++--- cmd/readmevalidation/readmeFiles.go | 32 ++++++++++++------- registry/coder/modules/claude-code/README.md | 2 +- .../github-upload-public-key/README.md | 4 +-- registry/coder/modules/goose/README.md | 2 +- 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/cmd/readmevalidation/coderResources.go b/cmd/readmevalidation/coderResources.go index cd811c3..106d664 100644 --- a/cmd/readmevalidation/coderResources.go +++ b/cmd/readmevalidation/coderResources.go @@ -116,15 +116,15 @@ func validateCoderResourceTags(tags []string) error { // would be a big improvement. var terraformVersionRe = regexp.MustCompile("^\\bversion\\s+=") -// This validation function definitely has risks of false positives right now, -// but realistically, it's not going to cause problems for launch. The most -// foolproof way to rebuild this would be to parse each README into an AST, and -// then parse each Terraform code block as Terraform func validateCoderResourceReadmeBody(body string) []error { trimmed := strings.TrimSpace(body) var errs []error errs = append(errs, validateReadmeBody(trimmed)...) + if true { + return errs + } + foundParagraph := false terraformCodeBlockCount := 0 foundTerraformVersionRef := false diff --git a/cmd/readmevalidation/readmeFiles.go b/cmd/readmevalidation/readmeFiles.go index c505dc6..47dde67 100644 --- a/cmd/readmevalidation/readmeFiles.go +++ b/cmd/readmevalidation/readmeFiles.go @@ -68,8 +68,8 @@ func separateFrontmatter(readmeText string) (string, string, error) { var readmeHeaderRe = regexp.MustCompile("^(#{1,})(\\s*)") -// Todo: This is a little chaotic, and might have some risks of false positives. -// Might be better to bring in an +// Todo: This seems to work okay for now, but the really proper way of doing +// this is by parsing this as an AST, and then checking the resulting nodes func validateReadmeBody(body string) []error { trimmed := strings.TrimSpace(body) var errs []error @@ -82,15 +82,25 @@ func validateReadmeBody(body string) []error { return errs } - lineNum := 0 - lastHeaderLevel := 0 + latestHeaderLevel := 0 foundFirstH1 := false + isInCodeBlock := false lineScanner := bufio.NewScanner(strings.NewReader(trimmed)) for lineScanner.Scan() { - lineNum++ nextLine := lineScanner.Text() + // Have to check this because a lot of programming languages support # + // comments (including Terraform), and without any context, there's no + // way to tell the difference between a markdown header and code comment + if strings.HasPrefix(nextLine, "```") { + isInCodeBlock = !isInCodeBlock + continue + } + if isInCodeBlock { + continue + } + headerGroups := readmeHeaderRe.FindStringSubmatch(nextLine) if headerGroups == nil { continue @@ -98,13 +108,13 @@ func validateReadmeBody(body string) []error { spaceAfterHeader := headerGroups[2] if spaceAfterHeader == "" { - errs = append(errs, fmt.Errorf("line %d: header does not have space between header characters and main header text", lineNum)) + errs = append(errs, errors.New("header does not have space between header characters and main header text")) } nextHeaderLevel := len(headerGroups[1]) if nextHeaderLevel == 1 && !foundFirstH1 { foundFirstH1 = true - lastHeaderLevel = 1 + latestHeaderLevel = 1 continue } @@ -115,21 +125,21 @@ func validateReadmeBody(body string) []error { break } if nextHeaderLevel > 6 { - errs = append(errs, fmt.Errorf("line %d: README/HTML files cannot have headers exceed level 6 (found level %d)", lineNum, nextHeaderLevel)) + errs = append(errs, fmt.Errorf("README/HTML files cannot have headers exceed level 6 (found level %d)", nextHeaderLevel)) break } // This is something we need to enforce for accessibility, not just for // the Registry website, but also when users are viewing the README // files in the GitHub web view - if nextHeaderLevel > lastHeaderLevel && nextHeaderLevel != (lastHeaderLevel+1) { - errs = append(errs, fmt.Errorf("line %d: headers are not allowed to increase more than 1 level at a time", lineNum)) + if nextHeaderLevel > latestHeaderLevel && nextHeaderLevel != (latestHeaderLevel+1) { + errs = append(errs, fmt.Errorf("headers are not allowed to increase more than 1 level at a time")) continue } // As long as the above condition passes, there's no problems with // going up a header level or going down 1+ header levels - lastHeaderLevel = nextHeaderLevel + latestHeaderLevel = nextHeaderLevel } return errs diff --git a/registry/coder/modules/claude-code/README.md b/registry/coder/modules/claude-code/README.md index b693440..38c8b31 100644 --- a/registry/coder/modules/claude-code/README.md +++ b/registry/coder/modules/claude-code/README.md @@ -22,7 +22,7 @@ module "claude-code" { } ``` -### Prerequisites +## Prerequisites - Node.js and npm must be installed in your workspace to install Claude Code - `screen` must be installed in your workspace to run Claude Code in the background diff --git a/registry/coder/modules/github-upload-public-key/README.md b/registry/coder/modules/github-upload-public-key/README.md index 3659ade..46220f0 100644 --- a/registry/coder/modules/github-upload-public-key/README.md +++ b/registry/coder/modules/github-upload-public-key/README.md @@ -20,13 +20,13 @@ module "github-upload-public-key" { } ``` -# Requirements +## Requirements This module requires `curl` and `jq` to be installed inside your workspace. Github External Auth must be enabled in the workspace for this module to work. The Github app that is configured for external auth must have both read and write permissions to "Git SSH keys" in order to upload the public key. Additionally, a Coder admin must also have the `admin:public_key` scope added to the external auth configuration of the Coder deployment. For example: -``` +```txt CODER_EXTERNAL_AUTH_0_ID="USER_DEFINED_ID" CODER_EXTERNAL_AUTH_0_TYPE=github CODER_EXTERNAL_AUTH_0_CLIENT_ID=xxxxxx diff --git a/registry/coder/modules/goose/README.md b/registry/coder/modules/goose/README.md index 5c1dcb8..e8b844c 100644 --- a/registry/coder/modules/goose/README.md +++ b/registry/coder/modules/goose/README.md @@ -22,7 +22,7 @@ module "goose" { } ``` -### Prerequisites +## Prerequisites - `screen` must be installed in your workspace to run Goose in the background - You must add the [Coder Login](https://registry.coder.com/modules/coder-login) module to your template From 53c3efa5825bf5a8b85a4c0e599da56caad180da Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Sat, 19 Apr 2025 01:59:49 +0000 Subject: [PATCH 14/25] fix: update README again --- registry/coder/modules/github-upload-public-key/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/coder/modules/github-upload-public-key/README.md b/registry/coder/modules/github-upload-public-key/README.md index 46220f0..779d419 100644 --- a/registry/coder/modules/github-upload-public-key/README.md +++ b/registry/coder/modules/github-upload-public-key/README.md @@ -36,7 +36,7 @@ CODER_EXTERNAL_AUTH_0_SCOPES="repo,workflow,admin:public_key" Note that the default scopes if not provided are `repo,workflow`. If the module is failing to complete after updating the external auth configuration, instruct users of the module to "Unlink" and "Link" their Github account in the External Auth user settings page to get the new scopes. -# Example +## Example Using a coder github external auth with a non-default id: (default is `github`) From ba062337f538b68b40e6a8b32cd2c1a9fe32d866 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Sat, 19 Apr 2025 02:04:47 +0000 Subject: [PATCH 15/25] fix: remove line numbers in error messages --- cmd/readmevalidation/coderResources.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/cmd/readmevalidation/coderResources.go b/cmd/readmevalidation/coderResources.go index 106d664..ee9e318 100644 --- a/cmd/readmevalidation/coderResources.go +++ b/cmd/readmevalidation/coderResources.go @@ -121,10 +121,6 @@ func validateCoderResourceReadmeBody(body string) []error { var errs []error errs = append(errs, validateReadmeBody(trimmed)...) - if true { - return errs - } - foundParagraph := false terraformCodeBlockCount := 0 foundTerraformVersionRef := false @@ -147,7 +143,7 @@ func validateCoderResourceReadmeBody(body string) []error { if nextLine == "```" { if !isInsideCodeBlock { - errs = append(errs, fmt.Errorf("line %d: found stray ``` (either an extra code block terminator, or a code block header without a specified language)", lineNum)) + errs = append(errs, errors.New("found stray ``` (either an extra code block terminator, or a code block header without a specified language)")) break } @@ -179,7 +175,7 @@ func validateCoderResourceReadmeBody(body string) []error { } if strings.HasPrefix(nextLine, "```hcl") { - errs = append(errs, fmt.Errorf("line %d: all .hcl language references must be converted to .tf", lineNum)) + errs = append(errs, errors.New("all .hcl language references must be converted to .tf")) } continue @@ -189,7 +185,7 @@ func validateCoderResourceReadmeBody(body string) []error { // are: (1) empty spaces, (2) paragraphs, (3) HTML, and (4) asset // references made via [] syntax trimmedLine := strings.TrimSpace(nextLine) - isParagraph := trimmedLine != "" && !strings.HasPrefix(trimmedLine, "[") && !strings.HasPrefix(trimmedLine, "<") + isParagraph := trimmedLine != "" && !strings.HasPrefix(trimmedLine, "![") && !strings.HasPrefix(trimmedLine, "<") foundParagraph = foundParagraph || isParagraph } From db2fd0e4b8edf71a626a43df70924669e322472c Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Sat, 19 Apr 2025 02:39:47 +0000 Subject: [PATCH 16/25] fix: update README validation to block empty strings --- cmd/readmevalidation/readmeFiles.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd/readmevalidation/readmeFiles.go b/cmd/readmevalidation/readmeFiles.go index 47dde67..58b32c2 100644 --- a/cmd/readmevalidation/readmeFiles.go +++ b/cmd/readmevalidation/readmeFiles.go @@ -72,16 +72,19 @@ var readmeHeaderRe = regexp.MustCompile("^(#{1,})(\\s*)") // this is by parsing this as an AST, and then checking the resulting nodes func validateReadmeBody(body string) []error { trimmed := strings.TrimSpace(body) - var errs []error + + if trimmed == "" { + return []error{errors.New("README body is empty")} + } // If the very first line of the README, there's a risk that the rest of the // validation logic will break, since we don't have many guarantees about // how the README is actually structured if !strings.HasPrefix(trimmed, "# ") { - errs = append(errs, errors.New("README body must start with ATX-style h1 header (i.e., \"# \")")) - return errs + return []error{errors.New("README body must start with ATX-style h1 header (i.e., \"# \")")} } + var errs []error latestHeaderLevel := 0 foundFirstH1 := false isInCodeBlock := false From 1d3c04f15c3d4b735b10851f7d4478d0640394d9 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Sat, 19 Apr 2025 02:50:54 +0000 Subject: [PATCH 17/25] get initial version of parsing working --- cmd/readmevalidation/coderResources.go | 52 ++++---- cmd/readmevalidation/coderResources_test.go | 22 ++++ .../testSamples/sampleReadmeBody.md | 121 ++++++++++++++++++ registry/coder/modules/slackme/README.md | 20 +-- 4 files changed, 175 insertions(+), 40 deletions(-) create mode 100644 cmd/readmevalidation/coderResources_test.go create mode 100644 cmd/readmevalidation/testSamples/sampleReadmeBody.md diff --git a/cmd/readmevalidation/coderResources.go b/cmd/readmevalidation/coderResources.go index ee9e318..7088785 100644 --- a/cmd/readmevalidation/coderResources.go +++ b/cmd/readmevalidation/coderResources.go @@ -114,7 +114,7 @@ func validateCoderResourceTags(tags []string) error { // parse any Terraform code snippets, and make some deeper guarantees about how // it's structured. Just validating whether it *can* be parsed as Terraform // would be a big improvement. -var terraformVersionRe = regexp.MustCompile("^\\bversion\\s+=") +var terraformVersionRe = regexp.MustCompile("^\\s*\\bversion\\s+=") func validateCoderResourceReadmeBody(body string) []error { trimmed := strings.TrimSpace(body) @@ -137,50 +137,39 @@ func validateCoderResourceReadmeBody(body string) []error { // Code assumes that invalid headers would've already been handled by // the base validation function, so we don't need to check deeper if the // first line isn't an h1 - if lineNum == 1 && !strings.HasPrefix(nextLine, "# ") { - break - } - - if nextLine == "```" { - if !isInsideCodeBlock { - errs = append(errs, errors.New("found stray ``` (either an extra code block terminator, or a code block header without a specified language)")) + if lineNum == 1 { + if !strings.HasPrefix(nextLine, "# ") { break + } else { + continue } - - isInsideCodeBlock = false - isInsideTerraform = false - continue } - if isInsideTerraform { - foundTerraformVersionRef = foundTerraformVersionRef || terraformVersionRe.MatchString(nextLine) - continue - } - if isInsideCodeBlock { - continue - } - - // Code assumes that we can treat this case as the end of the "h1 - // section" and don't need to process any further lines - if strings.HasPrefix(nextLine, "#") { - break - } - - // This is meant to catch cases like ```tf, ```hcl, and ```js if strings.HasPrefix(nextLine, "```") { - isInsideCodeBlock = true - isInsideTerraform = strings.HasPrefix(nextLine, "```tf") + isInsideCodeBlock = !isInsideCodeBlock + isInsideTerraform = isInsideCodeBlock && strings.HasPrefix(nextLine, "```tf") if isInsideTerraform { terraformCodeBlockCount++ } - if strings.HasPrefix(nextLine, "```hcl") { errs = append(errs, errors.New("all .hcl language references must be converted to .tf")) } + continue + } + if isInsideCodeBlock { + if isInsideTerraform { + foundTerraformVersionRef = foundTerraformVersionRef || terraformVersionRe.MatchString(nextLine) + } continue } + // Code assumes that we can treat this case as the end of the "h1 + // section" and don't need to process any further lines + if lineNum > 1 && strings.HasPrefix(nextLine, "#") { + break + } + // Code assumes that if we've reached this point, the only other options // are: (1) empty spaces, (2) paragraphs, (3) HTML, and (4) asset // references made via [] syntax @@ -202,6 +191,9 @@ func validateCoderResourceReadmeBody(body string) []error { if !foundParagraph { errs = append(errs, errors.New("did not find paragraph within h1 section")) } + if isInsideCodeBlock { + errs = append(errs, errors.New("code blocks inside h1 section do not all terminate before end of file")) + } return errs } diff --git a/cmd/readmevalidation/coderResources_test.go b/cmd/readmevalidation/coderResources_test.go new file mode 100644 index 0000000..71ec75f --- /dev/null +++ b/cmd/readmevalidation/coderResources_test.go @@ -0,0 +1,22 @@ +package main + +import ( + _ "embed" + "testing" +) + +//go:embed testSamples/sampleReadmeBody.md +var testBody string + +func TestValidateCoderResourceReadmeBody(t *testing.T) { + t.Parallel() + + t.Run("Parses a valid README body with zero issues", func(t *testing.T) { + t.Parallel() + + errs := validateCoderResourceReadmeBody(testBody) + for _, e := range errs { + t.Error(e) + } + }) +} diff --git a/cmd/readmevalidation/testSamples/sampleReadmeBody.md b/cmd/readmevalidation/testSamples/sampleReadmeBody.md new file mode 100644 index 0000000..958fe21 --- /dev/null +++ b/cmd/readmevalidation/testSamples/sampleReadmeBody.md @@ -0,0 +1,121 @@ +# Goose + +Run the [Goose](https://block.github.io/goose/) agent in your workspace to generate code and perform tasks. + +```tf +module "goose" { + source = "registry.coder.com/modules/goose/coder" + version = "1.0.31" + agent_id = coder_agent.example.id + folder = "/home/coder" + install_goose = true + goose_version = "v1.0.16" +} +``` + +## Prerequisites + +- `screen` must be installed in your workspace to run Goose in the background +- You must add the [Coder Login](https://registry.coder.com/modules/coder-login) module to your template + +The `codercom/oss-dogfood:latest` container image can be used for testing on container-based workspaces. + +## Examples + +Your workspace must have `screen` installed to use this. + +### Run in the background and report tasks (Experimental) + +> This functionality is in early access as of Coder v2.21 and is still evolving. +> For now, we recommend testing it in a demo or staging environment, +> rather than deploying to production +> +> Learn more in [the Coder documentation](https://coder.com/docs/tutorials/ai-agents) +> +> Join our [Discord channel](https://discord.gg/coder) or +> [contact us](https://coder.com/contact) to get help or share feedback. + +```tf +module "coder-login" { + count = data.coder_workspace.me.start_count + source = "registry.coder.com/modules/coder-login/coder" + version = "1.0.15" + agent_id = coder_agent.example.id +} + +variable "anthropic_api_key" { + type = string + description = "The Anthropic API key" + sensitive = true +} + +data "coder_parameter" "ai_prompt" { + type = "string" + name = "AI Prompt" + default = "" + description = "Write a prompt for Goose" + mutable = true +} + +# Set the prompt and system prompt for Goose via environment variables +resource "coder_agent" "main" { + # ... + env = { + GOOSE_SYSTEM_PROMPT = <<-EOT + You are a helpful assistant that can help write code. + + Run all long running tasks (e.g. npm run dev) in the background and not in the foreground. + + Periodically check in on background tasks. + + Notify Coder of the status of the task before and after your steps. + EOT + GOOSE_TASK_PROMPT = data.coder_parameter.ai_prompt.value + + # An API key is required for experiment_auto_configure + # See https://block.github.io/goose/docs/getting-started/providers + ANTHROPIC_API_KEY = var.anthropic_api_key # or use a coder_parameter + } +} + +module "goose" { + count = data.coder_workspace.me.start_count + source = "registry.coder.com/modules/goose/coder" + version = "1.0.31" + agent_id = coder_agent.example.id + folder = "/home/coder" + install_goose = true + goose_version = "v1.0.16" + + # Enable experimental features + experiment_report_tasks = true + + # Run Goose in the background + experiment_use_screen = true + + # Avoid configuring Goose manually + experiment_auto_configure = true + + # Required for experiment_auto_configure + experiment_goose_provider = "anthropic" + experiment_goose_model = "claude-3-5-sonnet-latest" +} +``` + +## Run standalone + +Run Goose as a standalone app in your workspace. This will install Goose and run it directly without using screen or any task reporting to the Coder UI. + +```tf +module "goose" { + source = "registry.coder.com/modules/goose/coder" + version = "1.0.31" + agent_id = coder_agent.example.id + folder = "/home/coder" + install_goose = true + goose_version = "v1.0.16" + + # Icon is not available in Coder v2.20 and below, so we'll use a custom icon URL + icon = "https://raw.githubusercontent.com/block/goose/refs/heads/main/ui/desktop/src/images/icon.svg" +} +``` diff --git a/registry/coder/modules/slackme/README.md b/registry/coder/modules/slackme/README.md index d28862c..bc2bf2a 100644 --- a/registry/coder/modules/slackme/README.md +++ b/registry/coder/modules/slackme/README.md @@ -11,6 +11,16 @@ tags: [helper] Add the `slackme` command to your workspace that DMs you on Slack when your command finishes running. +```tf +module "slackme" { + count = data.coder_workspace.me.start_count + source = "registry.coder.com/modules/slackme/coder" + version = "1.0.2" + agent_id = coder_agent.example.id + auth_provider_id = "slack" +} +``` + ```bash slackme npm run long-build ``` @@ -54,16 +64,6 @@ slackme npm run long-build 3. Restart your Coder deployment. Any Template can now import the Slack Me module, and `slackme` will be available on the `$PATH`: - ```tf - module "slackme" { - count = data.coder_workspace.me.start_count - source = "registry.coder.com/modules/slackme/coder" - version = "1.0.2" - agent_id = coder_agent.example.id - auth_provider_id = "slack" - } - ``` - ## Examples ### Custom Slack Message From 45640689e495e5148b2ed559ce36e28132606251 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Apr 2025 19:22:06 +0000 Subject: [PATCH 18/25] fix: update README files --- registry/hashicorp/README.md | 8 -------- registry/jfrog/README.md | 8 -------- registry/nataindata/README.md | 2 +- 3 files changed, 1 insertion(+), 17 deletions(-) delete mode 100644 registry/hashicorp/README.md delete mode 100644 registry/jfrog/README.md diff --git a/registry/hashicorp/README.md b/registry/hashicorp/README.md deleted file mode 100644 index 59bbe8a..0000000 --- a/registry/hashicorp/README.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -display_name: HashiCorp -bio: HashiCorp, an IBM company, empowers organizations to automate and secure multi-cloud and hybrid environments with The Infrastructure Cloud™. Our suite of Infrastructure Lifecycle Management and Security Lifecycle Management solutions are built on projects with source code freely available at their core. The HashiCorp suite underpins the world's most critical applications, helping enterprises achieve efficiency, security, and scalability at any stage of their cloud journey. -github: hashicorp -linkedin: https://www.linkedin.com/company/hashicorp -website: https://www.hashicorp.com/ -status: partner ---- diff --git a/registry/jfrog/README.md b/registry/jfrog/README.md deleted file mode 100644 index 8dea670..0000000 --- a/registry/jfrog/README.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -display_name: Jfrog -bio: At JFrog, we are making endless software versions a thing of the past, with liquid software that flows continuously and automatically from build all the way through to production. -github: jfrog -linkedin: https://www.linkedin.com/company/jfrog-ltd -website: https://jfrog.com/ -status: partner ---- diff --git a/registry/nataindata/README.md b/registry/nataindata/README.md index ddc5095..5f29181 100644 --- a/registry/nataindata/README.md +++ b/registry/nataindata/README.md @@ -3,5 +3,5 @@ display_name: Nataindata bio: Data engineer github: nataindata website: https://www.nataindata.com -status: community +status: partner --- From c731a5c64ebd28c5e034a79a65032a01ed8c29e7 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Apr 2025 19:24:34 +0000 Subject: [PATCH 19/25] fix: remove employer field entirely --- cmd/readmevalidation/contributors.go | 51 +++------------------------- 1 file changed, 5 insertions(+), 46 deletions(-) diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index 85a95f0..b27e1a8 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -21,12 +21,11 @@ type contributorProfileFrontmatter struct { GithubUsername string `yaml:"github"` // Script assumes that if value is nil, the Registry site build step will // backfill the value with the user's GitHub avatar URL - AvatarURL *string `yaml:"avatar"` - LinkedinURL *string `yaml:"linkedin"` - WebsiteURL *string `yaml:"website"` - SupportEmail *string `yaml:"support_email"` - EmployerGithubUsername *string `yaml:"employer_github"` - ContributorStatus *string `yaml:"status"` + AvatarURL *string `yaml:"avatar"` + LinkedinURL *string `yaml:"linkedin"` + WebsiteURL *string `yaml:"website"` + SupportEmail *string `yaml:"support_email"` + ContributorStatus *string `yaml:"status"` } type contributorProfile struct { @@ -47,29 +46,6 @@ func validateContributorGithubUsername(githubUsername string) error { return nil } -func validateContributorEmployerGithubUsername(employerGithubUsername *string, githubUsername string) []error { - if employerGithubUsername == nil { - return nil - } - - errs := []error{} - if *employerGithubUsername == "" { - errs = append(errs, errors.New("company_github field is defined but has empty value")) - return errs - } - - lower := strings.ToLower(*employerGithubUsername) - if uriSafe := url.PathEscape(lower); uriSafe != lower { - errs = append(errs, fmt.Errorf("gitHub company username %q is not a valid URL path segment", *employerGithubUsername)) - } - - if *employerGithubUsername == githubUsername { - errs = append(errs, fmt.Errorf("cannot list own GitHub name (%q) as employer", githubUsername)) - } - - return errs -} - func validateContributorDisplayName(displayName string) error { if displayName == "" { return fmt.Errorf("missing display_name") @@ -211,9 +187,6 @@ func validateContributorYaml(yml contributorProfile) []error { allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } - for _, err := range validateContributorEmployerGithubUsername(yml.frontmatter.EmployerGithubUsername, yml.frontmatter.GithubUsername) { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) - } for _, err := range validateContributorSupportEmail(yml.frontmatter.SupportEmail) { allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } @@ -264,7 +237,6 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil } } - employeeGithubGroups := map[string][]string{} yamlValidationErrors := []error{} for _, p := range profilesByUsername { errors := validateContributorYaml(p) @@ -272,19 +244,6 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil yamlValidationErrors = append(yamlValidationErrors, errors...) continue } - - if p.frontmatter.EmployerGithubUsername != nil { - employeeGithubGroups[*p.frontmatter.EmployerGithubUsername] = append( - employeeGithubGroups[*p.frontmatter.EmployerGithubUsername], - p.frontmatter.GithubUsername, - ) - } - } - for companyName, group := range employeeGithubGroups { - if _, found := profilesByUsername[companyName]; found { - continue - } - yamlValidationErrors = append(yamlValidationErrors, fmt.Errorf("%q: company %q does not exist but is referenced by these profiles: [%s]", rootRegistryPath, companyName, strings.Join(group, ", "))) } if len(yamlValidationErrors) != 0 { return nil, validationPhaseError{ From 7e8d4b7f27249570573ba7699f38989c9514eec5 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Apr 2025 19:37:06 +0000 Subject: [PATCH 20/25] fix: make Github field optional --- cmd/readmevalidation/contributors.go | 35 ++++++++-------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index b27e1a8..89a2b1a 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -16,9 +16,8 @@ import ( var validContributorStatuses = []string{"official", "partner", "community"} type contributorProfileFrontmatter struct { - DisplayName string `yaml:"display_name"` - Bio string `yaml:"bio"` - GithubUsername string `yaml:"github"` + DisplayName string `yaml:"display_name"` + Bio string `yaml:"bio"` // Script assumes that if value is nil, the Registry site build step will // backfill the value with the user's GitHub avatar URL AvatarURL *string `yaml:"avatar"` @@ -30,22 +29,10 @@ type contributorProfileFrontmatter struct { type contributorProfile struct { frontmatter contributorProfileFrontmatter + namespace string filePath string } -func validateContributorGithubUsername(githubUsername string) error { - if githubUsername == "" { - return errors.New("missing GitHub username") - } - - lower := strings.ToLower(githubUsername) - if uriSafe := url.PathEscape(lower); uriSafe != lower { - return fmt.Errorf("gitHub username %q is not a valid URL path segment", githubUsername) - } - - return nil -} - func validateContributorDisplayName(displayName string) error { if displayName == "" { return fmt.Errorf("missing display_name") @@ -171,9 +158,6 @@ func validateContributorAvatarURL(avatarURL *string) []error { func validateContributorYaml(yml contributorProfile) []error { allErrs := []error{} - if err := validateContributorGithubUsername(yml.frontmatter.GithubUsername); err != nil { - allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) - } if err := validateContributorDisplayName(yml.frontmatter.DisplayName); err != nil { allErrs = append(allErrs, addFilePathToError(yml.filePath, err)) } @@ -211,11 +195,12 @@ func parseContributorProfile(rm readme) (contributorProfile, error) { return contributorProfile{ filePath: rm.filePath, frontmatter: yml, + namespace: strings.TrimSuffix(strings.TrimPrefix(rm.filePath, "registry/"), "/README.md"), }, nil } func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfile, error) { - profilesByUsername := map[string]contributorProfile{} + profilesByNamespace := map[string]contributorProfile{} yamlParsingErrors := []error{} for _, rm := range readmeEntries { p, err := parseContributorProfile(rm) @@ -224,11 +209,11 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil continue } - if prev, alreadyExists := profilesByUsername[p.frontmatter.GithubUsername]; alreadyExists { - yamlParsingErrors = append(yamlParsingErrors, fmt.Errorf("%q: GitHub name %s conflicts with field defined in %q", p.filePath, p.frontmatter.GithubUsername, prev.filePath)) + if prev, alreadyExists := profilesByNamespace[p.namespace]; alreadyExists { + yamlParsingErrors = append(yamlParsingErrors, fmt.Errorf("%q: namespace %q conflicts with namespace from %q", p.filePath, p.namespace, prev.filePath)) continue } - profilesByUsername[p.frontmatter.GithubUsername] = p + profilesByNamespace[p.namespace] = p } if len(yamlParsingErrors) != 0 { return nil, validationPhaseError{ @@ -238,7 +223,7 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil } yamlValidationErrors := []error{} - for _, p := range profilesByUsername { + for _, p := range profilesByNamespace { errors := validateContributorYaml(p) if len(errors) > 0 { yamlValidationErrors = append(yamlValidationErrors, errors...) @@ -252,7 +237,7 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil } } - return profilesByUsername, nil + return profilesByNamespace, nil } func aggregateContributorReadmeFiles() ([]readme, error) { From 2e5f3bfea104dcce3b6bb583e86f27c385c7b7c7 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 2 May 2025 15:20:57 +0000 Subject: [PATCH 21/25] refactor: rename files --- cmd/readmevalidation/{readmeFiles.go => readmefiles.go} | 0 cmd/readmevalidation/{repoStructure.go => repostructure.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename cmd/readmevalidation/{readmeFiles.go => readmefiles.go} (100%) rename cmd/readmevalidation/{repoStructure.go => repostructure.go} (100%) diff --git a/cmd/readmevalidation/readmeFiles.go b/cmd/readmevalidation/readmefiles.go similarity index 100% rename from cmd/readmevalidation/readmeFiles.go rename to cmd/readmevalidation/readmefiles.go diff --git a/cmd/readmevalidation/repoStructure.go b/cmd/readmevalidation/repostructure.go similarity index 100% rename from cmd/readmevalidation/repoStructure.go rename to cmd/readmevalidation/repostructure.go From 4151e01faad62dc87ba07a128d8c9b0f37d08e66 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 2 May 2025 15:31:14 +0000 Subject: [PATCH 22/25] fix: add nil check --- cmd/readmevalidation/coderResources.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/readmevalidation/coderResources.go b/cmd/readmevalidation/coderResources.go index 7088785..863102d 100644 --- a/cmd/readmevalidation/coderResources.go +++ b/cmd/readmevalidation/coderResources.go @@ -89,6 +89,9 @@ func validateCoderResourceIconURL(iconURL string) []error { } func validateCoderResourceTags(tags []string) error { + if tags == nil { + return errors.New("provided tags array is nil") + } if len(tags) == 0 { return nil } @@ -140,9 +143,8 @@ func validateCoderResourceReadmeBody(body string) []error { if lineNum == 1 { if !strings.HasPrefix(nextLine, "# ") { break - } else { - continue } + continue } if strings.HasPrefix(nextLine, "```") { From 12205f5f148a49d57266d207789a3c97a4cfbcb1 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 2 May 2025 15:31:41 +0000 Subject: [PATCH 23/25] fix: rename files --- cmd/readmevalidation/{coderResources.go => coderresources.go} | 0 .../{coderResources_test.go => coderresources_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename cmd/readmevalidation/{coderResources.go => coderresources.go} (100%) rename cmd/readmevalidation/{coderResources_test.go => coderresources_test.go} (100%) diff --git a/cmd/readmevalidation/coderResources.go b/cmd/readmevalidation/coderresources.go similarity index 100% rename from cmd/readmevalidation/coderResources.go rename to cmd/readmevalidation/coderresources.go diff --git a/cmd/readmevalidation/coderResources_test.go b/cmd/readmevalidation/coderresources_test.go similarity index 100% rename from cmd/readmevalidation/coderResources_test.go rename to cmd/readmevalidation/coderresources_test.go From dda33d7dde9d966408918272bdee4d86bd47758b Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 2 May 2025 16:27:21 +0000 Subject: [PATCH 24/25] fix: remove unnecessary type --- cmd/readmevalidation/coderresources.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/cmd/readmevalidation/coderresources.go b/cmd/readmevalidation/coderresources.go index 863102d..98a953c 100644 --- a/cmd/readmevalidation/coderresources.go +++ b/cmd/readmevalidation/coderresources.go @@ -35,13 +35,6 @@ type coderResourceReadme struct { frontmatter coderResourceFrontmatter } -type coderResourceReadmes map[string]coderResourceReadme - -func (crr coderResourceReadmes) Get(filePath string) (coderResourceReadme, bool) { - rm, ok := crr[filePath] - return rm, ok -} - func validateCoderResourceDisplayName(displayName *string) error { if displayName != nil && *displayName == "" { return errors.New("if defined, display_name must not be empty string") @@ -243,8 +236,8 @@ func parseCoderResourceReadme(resourceType string, rm readme) (coderResourceRead }, nil } -func parseCoderResourceReadmeFiles(resourceType string, rms []readme) (coderResourceReadmes, error) { - resources := coderResourceReadmes(map[string]coderResourceReadme{}) +func parseCoderResourceReadmeFiles(resourceType string, rms []readme) (map[string]coderResourceReadme, error) { + resources := map[string]coderResourceReadme{} var yamlParsingErrs []error for _, rm := range rms { p, err := parseCoderResourceReadme(resourceType, rm) @@ -281,7 +274,7 @@ func parseCoderResourceReadmeFiles(resourceType string, rms []readme) (coderReso // Todo: Need to beef up this function by grabbing each image/video URL from // the body's AST -func validateCoderResourceRelativeUrls(resources coderResourceReadmes) error { +func validateCoderResourceRelativeUrls(resources map[string]coderResourceReadme) error { return nil } From 2180f4bab3c1e55d3f41c370d36109a62c7b2cc1 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Fri, 2 May 2025 16:26:19 -0400 Subject: [PATCH 25/25] fix: update type for validation phase --- cmd/readmevalidation/errors.go | 2 +- cmd/readmevalidation/readmefiles.go | 29 ++++++----------------------- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/cmd/readmevalidation/errors.go b/cmd/readmevalidation/errors.go index db13edc..d9dbb17 100644 --- a/cmd/readmevalidation/errors.go +++ b/cmd/readmevalidation/errors.go @@ -14,7 +14,7 @@ type validationPhaseError struct { var _ error = validationPhaseError{} func (vpe validationPhaseError) Error() string { - msg := fmt.Sprintf("Error during %q phase of README validation:", vpe.phase.String()) + msg := fmt.Sprintf("Error during %q phase of README validation:", vpe.phase) for _, e := range vpe.errors { msg += fmt.Sprintf("\n- %v", e) } diff --git a/cmd/readmevalidation/readmefiles.go b/cmd/readmevalidation/readmefiles.go index 58b32c2..2967652 100644 --- a/cmd/readmevalidation/readmefiles.go +++ b/cmd/readmevalidation/readmefiles.go @@ -151,45 +151,28 @@ func validateReadmeBody(body string) []error { // validationPhase represents a specific phase during README validation. It is // expected that each phase is discrete, and errors during one will prevent a // future phase from starting. -type validationPhase int +type validationPhase string const ( // validationPhaseFileStructureValidation indicates when the entire Registry // directory is being verified for having all files be placed in the file // system as expected. - validationPhaseFileStructureValidation validationPhase = iota + validationPhaseFileStructureValidation validationPhase = "File structure validation" // validationPhaseFileLoad indicates when README files are being read from // the file system - validationPhaseFileLoad + validationPhaseFileLoad = "Filesystem reading" // validationPhaseReadmeParsing indicates when a README's frontmatter is // being parsed as YAML. This phase does not include YAML validation. - validationPhaseReadmeParsing + validationPhaseReadmeParsing = "README parsing" // validationPhaseReadmeValidation indicates when a README's frontmatter is // being validated as proper YAML with expected keys. - validationPhaseReadmeValidation + validationPhaseReadmeValidation = "README validation" // validationPhaseAssetCrossReference indicates when a README's frontmatter // is having all its relative URLs be validated for whether they point to // valid resources. - validationPhaseAssetCrossReference + validationPhaseAssetCrossReference = "Cross-referencing relative asset URLs" ) - -func (p validationPhase) String() string { - switch p { - case validationPhaseFileStructureValidation: - return "File structure validation" - case validationPhaseFileLoad: - return "Filesystem reading" - case validationPhaseReadmeParsing: - return "README parsing" - case validationPhaseReadmeValidation: - return "README validation" - case validationPhaseAssetCrossReference: - return "Cross-referencing relative asset URLs" - default: - return fmt.Sprintf("Unknown validation phase: %d", p) - } -}