Compare commits

...

10 Commits

Author SHA1 Message Date
DevCats
d8c9ad122c
Merge branch 'main' into atif/validate-readme-source 2025-10-06 08:46:35 -05:00
DevCats
494e4deb72
Merge branch 'main' into atif/validate-readme-source 2025-10-01 16:56:38 -05:00
blink-so[bot]
343a2f2bb8 refactor: improve module source URL validation
- Use more specific regex pattern [a-zA-Z0-9-]+ instead of [^/]+ for namespace/module names
- Process all Terraform blocks instead of just the first one
- Report correct source if found in any block, only report incorrect sources if no correct source exists
- Add comprehensive test cases for multiple Terraform blocks

Co-authored-by: matifali <10648092+matifali@users.noreply.github.com>
2025-09-18 16:40:43 +00:00
Atif Ali
9cc3d5a776
Merge branch 'main' into atif/validate-readme-source 2025-09-13 08:13:29 +05:00
blink-so[bot]
9c00c576a3 Address PR review comments
- Make regex more specific for registry.coder.com patterns only
- Refactor to add namespace and resourceName fields to coderResourceReadme struct
- Inline path parsing logic into parseCoderResourceReadme
- Update validateModuleSourceURL to use struct fields instead of filePath parameter
- Simplify Terraform block detection logic
- Reduce nesting with early continue statements
- Add comment explaining regex pattern
- Extract registry.coder.com into a constant
- Improve test readability with extracted variables
- Remove redundant checks in tests
- Replace custom contains function with strings.Contains

Co-authored-by: matifali <matifali@users.noreply.github.com>
2025-09-04 07:00:14 +00:00
Atif Ali
5419676276
Merge branch 'main' into atif/validate-readme-source 2025-09-03 09:42:07 +05:00
Muhammad Atif Ali
7e94395bd1 Remove master branch from lint trigger 2025-09-01 19:53:35 +05:00
Muhammad Atif Ali
3b6b1ba4b9 Update README validation and Go version
- Downgrade Go version in CI to 1.24 for consistency.
- Fix naming and path issues in `readmevalidation` code.
- Improve regex validation for module and namespace names.
- Correct typos and improve comments for clarity.
2025-09-01 19:52:57 +05:00
Atif Ali
5b6d878fd7
Merge branch 'main' into atif/validate-readme-source 2025-09-01 18:21:59 +05:00
Muhammad Atif Ali
72d7ee418b Add validation for Terraform module source URLs
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index
53b912b..eb3cf8b 100644 --- a/.github/workflows/ci.yaml +++
b/.github/workflows/ci.yaml @@ -63,8 +63,8 @@ jobs: - name: Set up Go
uses: actions/setup-go@v5 with: - go-version: "1.23.2" - - name:
Validate contributors + go-version: "1.25.0" + - name: Validate Reademde
run: go build ./cmd/readmevalidation && ./readmevalidation - name:
Remove build file artifact run: rm ./readmevalidation

Signed-off-by: Muhammad Atif Ali <me@matifali.dev>
2025-09-01 18:21:00 +05:00
9 changed files with 278 additions and 19 deletions

View File

@ -63,8 +63,8 @@ jobs:
- name: Set up Go - name: Set up Go
uses: actions/setup-go@v6 uses: actions/setup-go@v6
with: with:
go-version: "1.23.2" go-version: "1.24"
- name: Validate contributors - name: Validate README
run: go build ./cmd/readmevalidation && ./readmevalidation run: go build ./cmd/readmevalidation && ./readmevalidation
- name: Remove build file artifact - name: Remove build file artifact
run: rm ./readmevalidation run: rm ./readmevalidation

View File

@ -3,7 +3,6 @@ on:
push: push:
branches: branches:
- main - main
- master
pull_request: pull_request:
permissions: permissions:

View File

@ -3,11 +3,84 @@ package main
import ( import (
"bufio" "bufio"
"context" "context"
"regexp"
"strings" "strings"
"golang.org/x/xerrors" "golang.org/x/xerrors"
) )
var (
// Matches Terraform source lines with registry.coder.com URLs
// Pattern: source = "registry.coder.com/namespace/module/coder"
terraformSourceRe = regexp.MustCompile(`^\s*source\s*=\s*"` + registryDomain + `/([a-zA-Z0-9-]+)/([a-zA-Z0-9-]+)/coder"`)
)
func validateModuleSourceURL(rm coderResourceReadme) []error {
var errs []error
// Skip validation if we couldn't parse namespace/resourceName from path
if rm.namespace == "" || rm.resourceName == "" {
return []error{xerrors.Errorf("invalid module path format: %s", rm.filePath)}
}
expectedSource := registryDomain + "/" + rm.namespace + "/" + rm.resourceName + "/coder"
trimmed := strings.TrimSpace(rm.body)
foundCorrectSource := false
var incorrectSources []string
isInsideTerraform := false
lineScanner := bufio.NewScanner(strings.NewReader(trimmed))
for lineScanner.Scan() {
nextLine := lineScanner.Text()
if strings.HasPrefix(nextLine, "```") {
if strings.HasPrefix(nextLine, "```tf") {
isInsideTerraform = true
continue
}
if isInsideTerraform {
// End of current terraform block, continue to look for more
isInsideTerraform = false
}
continue
}
if !isInsideTerraform {
continue
}
// Check for source line in terraform blocks
if matches := terraformSourceRe.FindStringSubmatch(nextLine); matches != nil {
actualNamespace := matches[1]
actualModule := matches[2]
actualSource := registryDomain + "/" + actualNamespace + "/" + actualModule + "/coder"
if actualSource == expectedSource {
foundCorrectSource = true
} else {
// Collect incorrect sources but don't return immediately
incorrectSources = append(incorrectSources, actualSource)
}
}
}
// If we found the correct source, ignore any incorrect ones
if foundCorrectSource {
return nil
}
// If we found incorrect sources but no correct one, report the first incorrect source
if len(incorrectSources) > 0 {
errs = append(errs, xerrors.Errorf("incorrect source URL format: found %q, expected %q", incorrectSources[0], expectedSource))
return errs
}
// If we found no sources at all
errs = append(errs, xerrors.Errorf("did not find correct source URL %q in any Terraform code block", expectedSource))
return errs
}
func validateCoderModuleReadmeBody(body string) []error { func validateCoderModuleReadmeBody(body string) []error {
var errs []error var errs []error
@ -94,6 +167,9 @@ func validateCoderModuleReadme(rm coderResourceReadme) []error {
for _, err := range validateCoderModuleReadmeBody(rm.body) { for _, err := range validateCoderModuleReadmeBody(rm.body) {
errs = append(errs, addFilePathToError(rm.filePath, err)) errs = append(errs, addFilePathToError(rm.filePath, err))
} }
for _, err := range validateModuleSourceURL(rm) {
errs = append(errs, addFilePathToError(rm.filePath, err))
}
for _, err := range validateResourceGfmAlerts(rm.body) { for _, err := range validateResourceGfmAlerts(rm.body) {
errs = append(errs, addFilePathToError(rm.filePath, err)) errs = append(errs, addFilePathToError(rm.filePath, err))
} }
@ -143,4 +219,4 @@ func validateAllCoderModules() error {
} }
logger.Info(context.Background(), "all relative URLs for READMEs are valid", "resource_type", resourceType) logger.Info(context.Background(), "all relative URLs for READMEs are valid", "resource_type", resourceType)
return nil return nil
} }

View File

@ -2,12 +2,70 @@ package main
import ( import (
_ "embed" _ "embed"
"strings"
"testing" "testing"
) )
//go:embed testSamples/sampleReadmeBody.md //go:embed testSamples/sampleReadmeBody.md
var testBody string var testBody string
// Test bodies extracted for better readability
var (
validModuleBody = `# Test Module
` + "```tf\n" + `module "test-module" {
source = "registry.coder.com/test-namespace/test-module/coder"
version = "1.0.0"
agent_id = coder_agent.example.id
}
` + "```\n"
wrongNamespaceBody = `# Test Module
` + "```tf\n" + `module "test-module" {
source = "registry.coder.com/wrong-namespace/test-module/coder"
version = "1.0.0"
agent_id = coder_agent.example.id
}
` + "```\n"
missingSourceBody = `# Test Module
` + "```tf\n" + `module "test-module" {
version = "1.0.0"
agent_id = coder_agent.example.id
}
` + "```\n"
multipleBlocksValidBody = `# Test Module
` + "```tf\n" + `module "other-module" {
source = "registry.coder.com/other/module/coder"
version = "1.0.0"
}
` + "```\n" + `
` + "```tf\n" + `module "test-module" {
source = "registry.coder.com/test-namespace/test-module/coder"
version = "1.0.0"
agent_id = coder_agent.example.id
}
` + "```\n"
multipleBlocksInvalidBody = `# Test Module
` + "```tf\n" + `module "test-module" {
source = "registry.coder.com/wrong-namespace/test-module/coder"
version = "1.0.0"
}
` + "```\n" + `
` + "```tf\n" + `module "other-module" {
source = "registry.coder.com/another-wrong/test-module/coder"
version = "1.0.0"
agent_id = coder_agent.example.id
}
` + "```\n"
)
func TestValidateCoderResourceReadmeBody(t *testing.T) { func TestValidateCoderResourceReadmeBody(t *testing.T) {
t.Parallel() t.Parallel()
@ -20,3 +78,115 @@ func TestValidateCoderResourceReadmeBody(t *testing.T) {
} }
}) })
} }
func TestValidateModuleSourceURL(t *testing.T) {
t.Parallel()
t.Run("Valid source URL format", func(t *testing.T) {
t.Parallel()
rm := coderResourceReadme{
resourceType: "modules",
filePath: "registry/test-namespace/modules/test-module/README.md",
namespace: "test-namespace",
resourceName: "test-module",
body: validModuleBody,
}
errs := validateModuleSourceURL(rm)
if len(errs) != 0 {
t.Errorf("Expected no errors, got: %v", errs)
}
})
t.Run("Invalid source URL format - wrong namespace", func(t *testing.T) {
t.Parallel()
rm := coderResourceReadme{
resourceType: "modules",
filePath: "registry/test-namespace/modules/test-module/README.md",
namespace: "test-namespace",
resourceName: "test-module",
body: wrongNamespaceBody,
}
errs := validateModuleSourceURL(rm)
if len(errs) != 1 {
t.Errorf("Expected 1 error, got %d: %v", len(errs), errs)
}
if !strings.Contains(errs[0].Error(), "incorrect source URL format") {
t.Errorf("Expected source URL format error, got: %s", errs[0].Error())
}
})
t.Run("Missing source URL", func(t *testing.T) {
t.Parallel()
rm := coderResourceReadme{
resourceType: "modules",
filePath: "registry/test-namespace/modules/test-module/README.md",
namespace: "test-namespace",
resourceName: "test-module",
body: missingSourceBody,
}
errs := validateModuleSourceURL(rm)
if len(errs) != 1 {
t.Errorf("Expected 1 error, got %d: %v", len(errs), errs)
}
if !strings.Contains(errs[0].Error(), "did not find correct source URL") {
t.Errorf("Expected missing source URL error, got: %s", errs[0].Error())
}
})
t.Run("Invalid file path format", func(t *testing.T) {
t.Parallel()
rm := coderResourceReadme{
resourceType: "modules",
filePath: "invalid/path/format",
namespace: "", // Empty because path parsing failed
resourceName: "", // Empty because path parsing failed
body: "# Test Module",
}
errs := validateModuleSourceURL(rm)
if len(errs) != 1 {
t.Errorf("Expected 1 error, got %d: %v", len(errs), errs)
}
if !strings.Contains(errs[0].Error(), "invalid module path format") {
t.Errorf("Expected path format error, got: %s", errs[0].Error())
}
})
t.Run("Multiple blocks with valid source in second block", func(t *testing.T) {
t.Parallel()
rm := coderResourceReadme{
resourceType: "modules",
filePath: "registry/test-namespace/modules/test-module/README.md",
namespace: "test-namespace",
resourceName: "test-module",
body: multipleBlocksValidBody,
}
errs := validateModuleSourceURL(rm)
if len(errs) != 0 {
t.Errorf("Expected no errors, got: %v", errs)
}
})
t.Run("Multiple blocks with incorrect source in second block", func(t *testing.T) {
t.Parallel()
rm := coderResourceReadme{
resourceType: "modules",
filePath: "registry/test-namespace/modules/test-module/README.md",
namespace: "test-namespace",
resourceName: "test-module",
body: multipleBlocksInvalidBody,
}
errs := validateModuleSourceURL(rm)
if len(errs) != 1 {
t.Errorf("Expected 1 error, got %d: %v", len(errs), errs)
}
if !strings.Contains(errs[0].Error(), "incorrect source URL format") {
t.Errorf("Expected source URL format error, got: %s", errs[0].Error())
}
})
}

View File

@ -18,6 +18,7 @@ var (
supportedResourceTypes = []string{"modules", "templates"} supportedResourceTypes = []string{"modules", "templates"}
operatingSystems = []string{"windows", "macos", "linux"} operatingSystems = []string{"windows", "macos", "linux"}
gfmAlertTypes = []string{"NOTE", "IMPORTANT", "CAUTION", "WARNING", "TIP"} gfmAlertTypes = []string{"NOTE", "IMPORTANT", "CAUTION", "WARNING", "TIP"}
registryDomain = "registry.coder.com"
// TODO: This is a holdover from the validation logic used by the Coder Modules repo. It gives us some assurance, but // 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 // realistically, we probably want to parse any Terraform code snippets, and make some deeper guarantees about how it's
@ -25,7 +26,7 @@ var (
terraformVersionRe = regexp.MustCompile(`^\s*\bversion\s+=`) terraformVersionRe = regexp.MustCompile(`^\s*\bversion\s+=`)
// Matches the format "> [!INFO]". Deliberately using a broad pattern to catch formatting issues that can mess up // Matches the format "> [!INFO]". Deliberately using a broad pattern to catch formatting issues that can mess up
// the renderer for the Registry website // the renderer for the Registry website.
gfmAlertRegex = regexp.MustCompile(`^>(\s*)\[!(\w+)\](\s*)(.*)`) gfmAlertRegex = regexp.MustCompile(`^>(\s*)\[!(\w+)\](\s*)(.*)`)
) )
@ -39,7 +40,7 @@ type coderResourceFrontmatter struct {
} }
// A slice version of the struct tags from coderResourceFrontmatter. Might be worth using reflection to generate this // 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 // list at runtime in the future, but this should be okay for now.
var supportedCoderResourceStructKeys = []string{ var supportedCoderResourceStructKeys = []string{
"description", "icon", "display_name", "verified", "tags", "supported_os", "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 // TODO: This is an old, officially deprecated key from the archived coder/modules repo. We can remove this once we
@ -53,6 +54,8 @@ var supportedCoderResourceStructKeys = []string{
type coderResourceReadme struct { type coderResourceReadme struct {
resourceType string resourceType string
filePath string filePath string
namespace string
resourceName string
body string body string
frontmatter coderResourceFrontmatter frontmatter coderResourceFrontmatter
} }
@ -183,9 +186,20 @@ func parseCoderResourceReadme(resourceType string, rm readme) (coderResourceRead
return coderResourceReadme{}, []error{xerrors.Errorf("%q: failed to parse: %v", rm.filePath, err)} return coderResourceReadme{}, []error{xerrors.Errorf("%q: failed to parse: %v", rm.filePath, err)}
} }
// Extract namespace and resource name from file path
// Expected path format: registry/<namespace>/<resourceType>/<resource-name>/README.md
var namespace, resourceName string
parts := strings.Split(path.Clean(rm.filePath), "/")
if len(parts) >= 5 && parts[0] == "registry" && parts[2] == resourceType && parts[4] == "README.md" {
namespace = parts[1]
resourceName = parts[3]
}
return coderResourceReadme{ return coderResourceReadme{
resourceType: resourceType, resourceType: resourceType,
filePath: rm.filePath, filePath: rm.filePath,
namespace: namespace,
resourceName: resourceName,
body: body, body: body,
frontmatter: yml, frontmatter: yml,
}, nil }, nil
@ -315,15 +329,15 @@ func validateResourceGfmAlerts(readmeBody string) []error {
} }
// Nested GFM alerts is such a weird mistake that it's probably not really safe to keep trying to process the // Nested GFM alerts is such a weird mistake that it's probably not really safe to keep trying to process the
// rest of the content, so this will prevent any other validations from happening for the given line // rest of the content, so this will prevent any other validations from happening for the given line.
if isInsideGfmQuotes { if isInsideGfmQuotes {
errs = append(errs, errors.New("registry does not support nested GFM alerts")) errs = append(errs, xerrors.New("registry does not support nested GFM alerts"))
continue continue
} }
leadingWhitespace := currentMatch[1] leadingWhitespace := currentMatch[1]
if len(leadingWhitespace) != 1 { if len(leadingWhitespace) != 1 {
errs = append(errs, errors.New("GFM alerts must have one space between the '>' and the start of the GFM brackets")) errs = append(errs, xerrors.New("GFM alerts must have one space between the '>' and the start of the GFM brackets"))
} }
isInsideGfmQuotes = true isInsideGfmQuotes = true
@ -347,7 +361,7 @@ func validateResourceGfmAlerts(readmeBody string) []error {
} }
} }
if gfmAlertRegex.Match([]byte(sourceLine)) { if gfmAlertRegex.MatchString(sourceLine) {
errs = append(errs, xerrors.Errorf("README has an incomplete GFM alert at the end of the file")) errs = append(errs, xerrors.Errorf("README has an incomplete GFM alert at the end of the file"))
} }

View File

@ -26,7 +26,7 @@ type contributorProfileFrontmatter struct {
} }
// A slice version of the struct tags from contributorProfileFrontmatter. Might be worth using reflection to generate // 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 // 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"} var supportedContributorProfileStructKeys = []string{"display_name", "bio", "status", "avatar", "linkedin", "github", "website", "support_email"}
type contributorProfileReadme struct { type contributorProfileReadme struct {

View File

@ -13,12 +13,11 @@ import (
var supportedUserNameSpaceDirectories = append(supportedResourceTypes, ".images") var supportedUserNameSpaceDirectories = append(supportedResourceTypes, ".images")
// validNameRe validates that names contain only alphanumeric characters and hyphens // validNameRe validates that names contain only alphanumeric characters and hyphens.
var validNameRe = regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?$`) var validNameRe = regexp.MustCompile(`^[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?$`)
// validateCoderResourceSubdirectory validates that the structure of a module or template within a namespace follows all // validateCoderResourceSubdirectory validates that the structure of a module or template within a namespace follows all
// expected file conventions // expected file conventions.
func validateCoderResourceSubdirectory(dirPath string) []error { func validateCoderResourceSubdirectory(dirPath string) []error {
resourceDir, err := os.Stat(dirPath) resourceDir, err := os.Stat(dirPath)
if err != nil { if err != nil {
@ -47,7 +46,7 @@ func validateCoderResourceSubdirectory(dirPath string) []error {
continue continue
} }
// Validate module/template name // Validate module/template name.
if !validNameRe.MatchString(f.Name()) { if !validNameRe.MatchString(f.Name()) {
errs = append(errs, xerrors.Errorf("%q: name contains invalid characters (only alphanumeric characters and hyphens are allowed)", path.Join(dirPath, f.Name()))) errs = append(errs, xerrors.Errorf("%q: name contains invalid characters (only alphanumeric characters and hyphens are allowed)", path.Join(dirPath, f.Name())))
continue continue
@ -90,7 +89,7 @@ func validateRegistryDirectory() []error {
continue continue
} }
// Validate namespace name // Validate namespace name.
if !validNameRe.MatchString(nDir.Name()) { if !validNameRe.MatchString(nDir.Name()) {
allErrs = append(allErrs, xerrors.Errorf("%q: namespace name contains invalid characters (only alphanumeric characters and hyphens are allowed)", namespacePath)) allErrs = append(allErrs, xerrors.Errorf("%q: namespace name contains invalid characters (only alphanumeric characters and hyphens are allowed)", namespacePath))
continue continue
@ -136,7 +135,7 @@ func validateRegistryDirectory() []error {
// validateRepoStructure validates that the structure of the repo is "correct enough" to do all necessary validation // 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 // 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 // are relevant for the main validation steps.
func validateRepoStructure() error { func validateRepoStructure() error {
var errs []error var errs []error
if vrdErrs := validateRegistryDirectory(); len(vrdErrs) != 0 { if vrdErrs := validateRegistryDirectory(); len(vrdErrs) != 0 {

2
go.mod
View File

@ -1,6 +1,6 @@
module coder.com/coder-registry module coder.com/coder-registry
go 1.23.2 go 1.24
require ( require (
cdr.dev/slog v1.6.1 cdr.dev/slog v1.6.1

View File

@ -5,7 +5,8 @@
"fmt:ci": "bun x prettier --check . && terraform fmt -check -recursive -diff", "fmt:ci": "bun x prettier --check . && terraform fmt -check -recursive -diff",
"terraform-validate": "./scripts/terraform_validate.sh", "terraform-validate": "./scripts/terraform_validate.sh",
"test": "./scripts/terraform_test_all.sh", "test": "./scripts/terraform_test_all.sh",
"update-version": "./update-version.sh" "update-version": "./update-version.sh",
"validate-readme": "go build ./cmd/readmevalidation && ./readmevalidation"
}, },
"devDependencies": { "devDependencies": {
"@types/bun": "^1.2.21", "@types/bun": "^1.2.21",