diff --git a/cmd/readmevalidation/coderresources.go b/cmd/readmevalidation/coderresources.go index 32b123e8..7cf9259a 100644 --- a/cmd/readmevalidation/coderresources.go +++ b/cmd/readmevalidation/coderresources.go @@ -17,6 +17,7 @@ import ( var ( supportedResourceTypes = []string{"modules", "templates"} + operatingSystems = []string{"windows", "macos", "linux"} // 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 @@ -25,11 +26,21 @@ var ( ) type coderResourceFrontmatter struct { - Description string `yaml:"description"` - IconURL string `yaml:"icon"` - DisplayName *string `yaml:"display_name"` - Verified *bool `yaml:"verified"` - Tags []string `yaml:"tags"` + Description string `yaml:"description"` + IconURL string `yaml:"icon"` + DisplayName *string `yaml:"display_name"` + Verified *bool `yaml:"verified"` + Tags []string `yaml:"tags"` + OperatingSystems []string `yaml:"supported_os"` +} + +// A slice version of the struct tags from coderResourceFrontmatter. Might be worth using reflection to generate this +// list at runtime in the future, but this should be okay for now +var supportedCoderResourceStructKeys = []string{ + "description", "icon", "display_name", "verified", "tags", "supported_os", + // TODO: This is an old, officially deprecated key from the archived coder/modules repo. We can remove this once we + // make sure that the Registry Server is no longer checking this field. + "maintainer_github", } // coderResourceReadme represents a README describing a Terraform resource used @@ -42,6 +53,17 @@ type coderResourceReadme struct { frontmatter coderResourceFrontmatter } +func validateSupportedOperatingSystems(systems []string) []error { + var errs []error + for _, s := range systems { + if slices.Contains(operatingSystems, s) { + continue + } + errs = append(errs, xerrors.Errorf("detected unknown operating system %q", s)) + } + return errs +} + func validateCoderResourceDisplayName(displayName *string) error { if displayName != nil && *displayName == "" { return xerrors.New("if defined, display_name must not be empty string") @@ -211,19 +233,31 @@ func validateCoderResourceReadme(rm coderResourceReadme) []error { for _, err := range validateCoderResourceIconURL(rm.frontmatter.IconURL) { errs = append(errs, addFilePathToError(rm.filePath, err)) } + for _, err := range validateSupportedOperatingSystems(rm.frontmatter.OperatingSystems) { + errs = append(errs, addFilePathToError(rm.filePath, err)) + } return errs } -func parseCoderResourceReadme(resourceType string, rm readme) (coderResourceReadme, error) { +func parseCoderResourceReadme(resourceType string, rm readme) (coderResourceReadme, []error) { fm, body, err := separateFrontmatter(rm.rawText) if err != nil { - return coderResourceReadme{}, xerrors.Errorf("%q: failed to parse frontmatter: %v", rm.filePath, err) + return coderResourceReadme{}, []error{xerrors.Errorf("%q: failed to parse frontmatter: %v", rm.filePath, err)} + } + + keyErrs := validateFrontmatterYamlKeys(fm, supportedCoderResourceStructKeys) + if len(keyErrs) != 0 { + remapped := []error{} + for _, e := range keyErrs { + remapped = append(remapped, addFilePathToError(rm.filePath, e)) + } + return coderResourceReadme{}, remapped } yml := coderResourceFrontmatter{} if err := yaml.Unmarshal([]byte(fm), &yml); err != nil { - return coderResourceReadme{}, xerrors.Errorf("%q: failed to parse: %v", rm.filePath, err) + return coderResourceReadme{}, []error{xerrors.Errorf("%q: failed to parse: %v", rm.filePath, err)} } return coderResourceReadme{ @@ -238,9 +272,9 @@ func parseCoderResourceReadmeFiles(resourceType string, rms []readme) (map[strin resources := map[string]coderResourceReadme{} var yamlParsingErrs []error for _, rm := range rms { - p, err := parseCoderResourceReadme(resourceType, rm) - if err != nil { - yamlParsingErrs = append(yamlParsingErrs, err) + p, errs := parseCoderResourceReadme(resourceType, rm) + if len(errs) != 0 { + yamlParsingErrs = append(yamlParsingErrs, errs...) continue } diff --git a/cmd/readmevalidation/contributors.go b/cmd/readmevalidation/contributors.go index aeab7ffa..c2e95e8a 100644 --- a/cmd/readmevalidation/contributors.go +++ b/cmd/readmevalidation/contributors.go @@ -19,11 +19,16 @@ type contributorProfileFrontmatter struct { Bio string `yaml:"bio"` ContributorStatus string `yaml:"status"` AvatarURL *string `yaml:"avatar"` + GithubUsername *string `yaml:"github"` LinkedinURL *string `yaml:"linkedin"` WebsiteURL *string `yaml:"website"` SupportEmail *string `yaml:"support_email"` } +// A slice version of the struct tags from contributorProfileFrontmatter. Might be worth using reflection to generate +// this list at runtime in the future, but this should be okay for now +var supportedContributorProfileStructKeys = []string{"display_name", "bio", "status", "avatar", "linkedin", "github", "website", "support_email"} + type contributorProfileReadme struct { frontmatter contributorProfileFrontmatter namespace string @@ -50,6 +55,22 @@ func validateContributorLinkedinURL(linkedinURL *string) error { return nil } +func validateGithubUsername(username *string) error { + if username == nil { + return nil + } + + name := *username + trimmed := strings.TrimSpace(name) + if trimmed == "" { + return xerrors.New("username must have non-whitespace characters") + } + if name != trimmed { + return xerrors.Errorf("username %q has extra whitespace", trimmed) + } + return nil +} + // validateContributorSupportEmail does best effort validation of a contributors email address. We can't 100% validate // that this is correct without actually sending an email, especially because some contributors are individual developers // and we don't want to do that on every single run of the CI pipeline. The best we can do is verify the general structure. @@ -153,6 +174,9 @@ func validateContributorReadme(rm contributorProfileReadme) []error { if err := validateContributorLinkedinURL(rm.frontmatter.LinkedinURL); err != nil { allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } + if err := validateGithubUsername(rm.frontmatter.GithubUsername); err != nil { + allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) + } if err := validateContributorWebsite(rm.frontmatter.WebsiteURL); err != nil { allErrs = append(allErrs, addFilePathToError(rm.filePath, err)) } @@ -170,15 +194,24 @@ func validateContributorReadme(rm contributorProfileReadme) []error { return allErrs } -func parseContributorProfile(rm readme) (contributorProfileReadme, error) { +func parseContributorProfile(rm readme) (contributorProfileReadme, []error) { fm, _, err := separateFrontmatter(rm.rawText) if err != nil { - return contributorProfileReadme{}, xerrors.Errorf("%q: failed to parse frontmatter: %v", rm.filePath, err) + return contributorProfileReadme{}, []error{xerrors.Errorf("%q: failed to parse frontmatter: %v", rm.filePath, err)} + } + + keyErrs := validateFrontmatterYamlKeys(fm, supportedContributorProfileStructKeys) + if len(keyErrs) != 0 { + remapped := []error{} + for _, e := range keyErrs { + remapped = append(remapped, addFilePathToError(rm.filePath, e)) + } + return contributorProfileReadme{}, remapped } yml := contributorProfileFrontmatter{} if err := yaml.Unmarshal([]byte(fm), &yml); err != nil { - return contributorProfileReadme{}, xerrors.Errorf("%q: failed to parse: %v", rm.filePath, err) + return contributorProfileReadme{}, []error{xerrors.Errorf("%q: failed to parse: %v", rm.filePath, err)} } return contributorProfileReadme{ @@ -192,9 +225,9 @@ func parseContributorFiles(readmeEntries []readme) (map[string]contributorProfil profilesByNamespace := map[string]contributorProfileReadme{} yamlParsingErrors := []error{} for _, rm := range readmeEntries { - p, err := parseContributorProfile(rm) - if err != nil { - yamlParsingErrors = append(yamlParsingErrors, err) + p, errs := parseContributorProfile(rm) + if len(errs) != 0 { + yamlParsingErrors = append(yamlParsingErrors, errs...) continue } diff --git a/cmd/readmevalidation/readmefiles.go b/cmd/readmevalidation/readmefiles.go index c55806a8..70580a77 100644 --- a/cmd/readmevalidation/readmefiles.go +++ b/cmd/readmevalidation/readmefiles.go @@ -4,6 +4,7 @@ import ( "bufio" "fmt" "regexp" + "slices" "strings" "golang.org/x/xerrors" @@ -39,7 +40,9 @@ const ( var ( supportedAvatarFileFormats = []string{".png", ".jpeg", ".jpg", ".gif", ".svg"} - // Matches markdown headers, must be at the beginning of a line, such as "# " or "### ". + // Matches markdown headers placed at the beginning of a line (e.g., "# " or "### "). To make the logic for + // validateReadmeBody easier, this pattern deliberately matches on invalid headers (header levels must be in the + // range 1–6 to be valid). The function has checks to see if the level is correct. readmeHeaderRe = regexp.MustCompile(`^(#+)(\s*)`) ) @@ -168,3 +171,25 @@ func validateReadmeBody(body string) []error { return errs } + +func validateFrontmatterYamlKeys(frontmatter string, allowedKeys []string) []error { + if len(allowedKeys) == 0 { + return []error{xerrors.New("Set of allowed keys is empty")} + } + + var key string + var cutOk bool + var line string + + var errs []error + lineScanner := bufio.NewScanner(strings.NewReader(frontmatter)) + for lineScanner.Scan() { + line = lineScanner.Text() + key, _, cutOk = strings.Cut(line, ":") + if !cutOk || slices.Contains(allowedKeys, key) { + continue + } + errs = append(errs, xerrors.Errorf("detected unknown key %q", key)) + } + return errs +} diff --git a/cmd/readmevalidation/repostructure.go b/cmd/readmevalidation/repostructure.go index 1841d79b..7a529c01 100644 --- a/cmd/readmevalidation/repostructure.go +++ b/cmd/readmevalidation/repostructure.go @@ -10,18 +10,21 @@ import ( "golang.org/x/xerrors" ) -var supportedUserNameSpaceDirectories = append(supportedResourceTypes, ".icons", ".images") +var supportedUserNameSpaceDirectories = append(supportedResourceTypes, ".images") +// validateCoderResourceSubdirectory validates that the structure of a module or template within a namespace follows all +// expected file conventions func validateCoderResourceSubdirectory(dirPath string) []error { - subDir, err := os.Stat(dirPath) + resourceDir, 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. + // 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) { return []error{addFilePathToError(dirPath, err)} } } - if !subDir.IsDir() { + if !resourceDir.IsDir() { return []error{xerrors.Errorf("%q: path is not a directory", dirPath)} } @@ -30,10 +33,11 @@ func validateCoderResourceSubdirectory(dirPath string) []error { return []error{addFilePathToError(dirPath, err)} } - errs := []error{} + var errs []error for _, f := range files { - // 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. + // The .coder subdirectories are sometimes generated as part of our 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 } @@ -59,49 +63,53 @@ func validateCoderResourceSubdirectory(dirPath string) []error { return errs } +// validateRegistryDirectory validates that the contents of `/registry` follow all expected file conventions. This +// includes the top-level structure of the individual namespace directories. func validateRegistryDirectory() []error { - userDirs, err := os.ReadDir(rootRegistryPath) + namespaceDirs, err := os.ReadDir(rootRegistryPath) if err != nil { return []error{err} } - allErrs := []error{} - for _, d := range userDirs { - dirPath := path.Join(rootRegistryPath, d.Name()) - if !d.IsDir() { - allErrs = append(allErrs, xerrors.Errorf("detected non-directory file %q at base of main Registry directory", dirPath)) + var allErrs []error + for _, nDir := range namespaceDirs { + namespacePath := path.Join(rootRegistryPath, nDir.Name()) + if !nDir.IsDir() { + allErrs = append(allErrs, xerrors.Errorf("detected non-directory file %q at base of main Registry directory", namespacePath)) continue } - contributorReadmePath := path.Join(dirPath, "README.md") + contributorReadmePath := path.Join(namespacePath, "README.md") if _, err := os.Stat(contributorReadmePath); err != nil { allErrs = append(allErrs, err) } - files, err := os.ReadDir(dirPath) + files, err := os.ReadDir(namespacePath) 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. + // TODO: Decide if there's anything more formal that we want to ensure about non-directories at the top + // level of each user namespace. if !f.IsDir() { continue } segment := f.Name() - filePath := path.Join(dirPath, segment) + filePath := path.Join(namespacePath, segment) if !slices.Contains(supportedUserNameSpaceDirectories, segment) { allErrs = append(allErrs, xerrors.Errorf("%q: only these sub-directories are allowed at top of user namespace: [%s]", filePath, strings.Join(supportedUserNameSpaceDirectories, ", "))) continue } + if !slices.Contains(supportedResourceTypes, segment) { + continue + } - if slices.Contains(supportedResourceTypes, segment) { - if errs := validateCoderResourceSubdirectory(filePath); len(errs) != 0 { - allErrs = append(allErrs, errs...) - } + if errs := validateCoderResourceSubdirectory(filePath); len(errs) != 0 { + allErrs = append(allErrs, errs...) } } } @@ -109,6 +117,9 @@ func validateRegistryDirectory() []error { return allErrs } +// validateRepoStructure validates that the structure of the repo is "correct enough" to do all necessary validation +// checks. It is NOT an exhaustive validation of the entire repo structure – it only checks the parts of the repo that +// are relevant for the main validation steps func validateRepoStructure() error { var errs []error if vrdErrs := validateRegistryDirectory(); len(vrdErrs) != 0 {