fix: update repo structure checks

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"))
+		errs = append(errs, errors.New("company_github field is defined but has empty value"))
-		return problems
+		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))
+		errs = append(errs, fmt.Errorf("email address %q is missing @ symbol", *email))
-		return problems
+		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))
+		errs = append(errs, fmt.Errorf("email address %q is missing period for server segment", *email))
-		return problems
+		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"))
+		errs = append(errs, errors.New("avatar URL must be omitted or non-empty string"))
-		return problems
+		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,
 	}
 }
 

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)
 }

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{}
+	allErrs := []error{}
-	for _, e := range dirEntries {
+	for _, d := range userDirs {
-		dirPath := path.Join(rootRegistryPath, e.Name())
+		dirPath := path.Join(rootRegistryPath, d.Name())
-		if !e.IsDir() {
+		if !d.IsDir() {
-			problems = append(problems, fmt.Errorf("detected non-directory file %q at base of main Registry directory", dirPath))
+			allErrs = append(allErrs, fmt.Errorf("detected non-directory file %q at base of main Registry directory", dirPath))
 			continue
 		}
 
-		readmePath := path.Join(dirPath, "README.md")
+		contributorReadmePath := path.Join(dirPath, "README.md")
-		_, err := os.Stat(readmePath)
+		_, 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 {
+			errs := validateCoderResourceSubdirectory(resourcePath)
-				problems = append(problems, errs...)
+			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,