From 812dd8faaf4ac3cbc69e9010c8b57e7cd101a7e1 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 18 Apr 2025 21:14:37 +0000 Subject: [PATCH] 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 fd7b30b3..36b7bb95 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 7f275c5f..2c0f452c 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 ed73910e..995d077f 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,