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
+2 -2
View File
@@ -63,8 +63,8 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v6
with:
go-version: "1.23.2"
- name: Validate contributors
go-version: "1.24"
- name: Validate README
run: go build ./cmd/readmevalidation && ./readmevalidation
- name: Remove build file artifact
run: rm ./readmevalidation
-1
View File
@@ -3,7 +3,6 @@ on:
push:
branches:
- main
- master
pull_request:
permissions:
+77 -1
View File
@@ -3,11 +3,84 @@ package main
import (
"bufio"
"context"
"regexp"
"strings"
"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 {
var errs []error
@@ -94,6 +167,9 @@ func validateCoderModuleReadme(rm coderResourceReadme) []error {
for _, err := range validateCoderModuleReadmeBody(rm.body) {
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) {
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)
return nil
}
}
+170
View File
@@ -2,12 +2,70 @@ package main
import (
_ "embed"
"strings"
"testing"
)
//go:embed testSamples/sampleReadmeBody.md
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) {
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())
}
})
}
+20 -6
View File
@@ -18,6 +18,7 @@ var (
supportedResourceTypes = []string{"modules", "templates"}
operatingSystems = []string{"windows", "macos", "linux"}
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
// 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+=`)
// 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*)(.*)`)
)
@@ -39,7 +40,7 @@ type coderResourceFrontmatter struct {
}
// 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{
"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
@@ -53,6 +54,8 @@ var supportedCoderResourceStructKeys = []string{
type coderResourceReadme struct {
resourceType string
filePath string
namespace string
resourceName string
body string
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)}
}
// 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{
resourceType: resourceType,
filePath: rm.filePath,
namespace: namespace,
resourceName: resourceName,
body: body,
frontmatter: yml,
}, 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
// 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 {
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
}
leadingWhitespace := currentMatch[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
@@ -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"))
}
+1 -1
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
// 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"}
type contributorProfileReadme struct {
+5 -6
View File
@@ -13,12 +13,11 @@ import (
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])?$`)
// 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 {
resourceDir, err := os.Stat(dirPath)
if err != nil {
@@ -47,7 +46,7 @@ func validateCoderResourceSubdirectory(dirPath string) []error {
continue
}
// Validate module/template name
// Validate module/template 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())))
continue
@@ -90,7 +89,7 @@ func validateRegistryDirectory() []error {
continue
}
// Validate namespace name
// Validate namespace 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))
continue
@@ -136,7 +135,7 @@ func validateRegistryDirectory() []error {
// 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
// are relevant for the main validation steps.
func validateRepoStructure() error {
var errs []error
if vrdErrs := validateRegistryDirectory(); len(vrdErrs) != 0 {
+1 -1
View File
@@ -1,6 +1,6 @@
module coder.com/coder-registry
go 1.23.2
go 1.24
require (
cdr.dev/slog v1.6.1
+2 -1
View File
@@ -5,7 +5,8 @@
"fmt:ci": "bun x prettier --check . && terraform fmt -check -recursive -diff",
"terraform-validate": "./scripts/terraform_validate.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": {
"@types/bun": "^1.2.21",