yamanin/registry
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Address PR review comments

Browse Source 
- 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>
This commit is contained in:
blink-so[bot] 2025-09-04 07:00:14 +00:00
parent 5419676276
commit 9c00c576a3
3 changed files with 111 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
cmd/readmevalidation/codermodules.go
View File

@ -3,7 +3,6 @@ package main
import (
"bufio"
"context"
"path/filepath"
"regexp"
"strings"
@ -11,63 +10,57 @@ import (
)
var (
terraformSourceRe = regexp.MustCompile(`^\s*source\s*=\s*"([^"]+)"`)
// 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 + `/([^/]+)/([^/]+)/coder"`)
)
func extractNamespaceAndModuleFromPath(filePath string) (namespace string, moduleName string, err error) {
// Expected path format: registry/<namespace>/modules/<module-name>/README.md.
parts := strings.Split(filepath.Clean(filePath), string(filepath.Separator))
if len(parts) < 5 || parts[0] != "registry" || parts[2] != "modules" || parts[4] != "README.md" {
return "", "", xerrors.Errorf("invalid module path format: %s", filePath)
}
namespace = parts[1]
moduleName = parts[3]
return namespace, moduleName, nil
}
func validateModuleSourceURL(body string, filePath string) []error {
func validateModuleSourceURL(rm coderResourceReadme) []error {
var errs []error
namespace, moduleName, err := extractNamespaceAndModuleFromPath(filePath)
if err != nil {
return []error{err}
// 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 := "registry.coder.com/" + namespace + "/" + moduleName + "/coder"
expectedSource := registryDomain + "/" + rm.namespace + "/" + rm.resourceName + "/coder"
trimmed := strings.TrimSpace(body)
trimmed := strings.TrimSpace(rm.body)
foundCorrectSource := false
isInsideTerraform := false
firstTerraformBlock := true
lineScanner := bufio.NewScanner(strings.NewReader(trimmed))
for lineScanner.Scan() {
nextLine := lineScanner.Text()
if strings.HasPrefix(nextLine, "```") {
if strings.HasPrefix(nextLine, "```tf") && firstTerraformBlock {
if strings.HasPrefix(nextLine, "```tf") {
isInsideTerraform = true
firstTerraformBlock = false
} else if isInsideTerraform {
// End of first terraform block.
continue
}
if isInsideTerraform {
break
}
continue
}
if isInsideTerraform {
// Check for any source line in the first terraform block.
if matches := terraformSourceRe.FindStringSubmatch(nextLine); matches != nil {
actualSource := matches[1]
if actualSource == expectedSource {
foundCorrectSource = true
break
} else if strings.HasPrefix(actualSource, "registry.coder.com/") && strings.Contains(actualSource, "/"+moduleName+"/coder") {
// Found source for this module but with wrong namespace/format.
errs = append(errs, xerrors.Errorf("incorrect source URL format: found %q, expected %q", actualSource, expectedSource))
return errs
}
if !isInsideTerraform {
continue
}
// Check for source line in the first terraform block
if matches := terraformSourceRe.FindStringSubmatch(nextLine); matches != nil {
actualNamespace := matches[1]
actualModule := matches[2]
actualSource := registryDomain + "/" + actualNamespace + "/" + actualModule + "/coder"
if actualSource == expectedSource {
foundCorrectSource = true
break
}
// Found a registry.coder.com source but with wrong namespace/module
errs = append(errs, xerrors.Errorf("incorrect source URL format: found %q, expected %q", actualSource, expectedSource))
return errs
}
}
@ -164,7 +157,7 @@ func validateCoderModuleReadme(rm coderResourceReadme) []error {
for _, err := range validateCoderModuleReadmeBody(rm.body) {
errs = append(errs, addFilePathToError(rm.filePath, err))
}
for _, err := range validateModuleSourceURL(rm.body, rm.filePath) {
for _, err := range validateModuleSourceURL(rm) {
errs = append(errs, addFilePathToError(rm.filePath, err))
}
for _, err := range validateResourceGfmAlerts(rm.body) {
@ -216,4 +209,4 @@ func validateAllCoderModules() error {
}
logger.Info(context.Background(), "all relative URLs for READMEs are valid", "resource_type", resourceType)
return nil
}
}

108
cmd/readmevalidation/codermodules_test.go
View File

@ -2,12 +2,42 @@ 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"
)
func TestValidateCoderResourceReadmeBody(t *testing.T) {
t.Parallel()
@ -27,9 +57,14 @@ func TestValidateModuleSourceURL(t *testing.T) {
t.Run("Valid source URL format", func(t *testing.T) {
t.Parallel()
body := "# Test Module\n\n```tf\nmodule \"test-module\" {\n source = \"registry.coder.com/test-namespace/test-module/coder\"\n version = \"1.0.0\"\n agent_id = coder_agent.example.id\n}\n```\n"
filePath := "registry/test-namespace/modules/test-module/README.md"
errs := validateModuleSourceURL(body, filePath)
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)
}
@ -38,13 +73,18 @@ func TestValidateModuleSourceURL(t *testing.T) {
t.Run("Invalid source URL format - wrong namespace", func(t *testing.T) {
t.Parallel()
body := "# Test Module\n\n```tf\nmodule \"test-module\" {\n source = \"registry.coder.com/wrong-namespace/test-module/coder\"\n version = \"1.0.0\"\n agent_id = coder_agent.example.id\n}\n```\n"
filePath := "registry/test-namespace/modules/test-module/README.md"
errs := validateModuleSourceURL(body, filePath)
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 len(errs) > 0 && !contains(errs[0].Error(), "incorrect source URL format") {
if !strings.Contains(errs[0].Error(), "incorrect source URL format") {
t.Errorf("Expected source URL format error, got: %s", errs[0].Error())
}
})
@ -52,54 +92,38 @@ func TestValidateModuleSourceURL(t *testing.T) {
t.Run("Missing source URL", func(t *testing.T) {
t.Parallel()
body := "# Test Module\n\n```tf\nmodule \"other-module\" {\n source = \"registry.coder.com/other/other-module/coder\"\n version = \"1.0.0\"\n agent_id = coder_agent.example.id\n}\n```\n"
filePath := "registry/test-namespace/modules/test-module/README.md"
errs := validateModuleSourceURL(body, filePath)
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 len(errs) > 0 && !contains(errs[0].Error(), "did not find correct source URL") {
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("Module name with hyphens vs underscores", func(t *testing.T) {
t.Parallel()
body := "# Test Module\n\n```tf\nmodule \"test_module\" {\n source = \"registry.coder.com/test-namespace/test-module/coder\"\n version = \"1.0.0\"\n agent_id = coder_agent.example.id\n}\n```\n"
filePath := "registry/test-namespace/modules/test-module/README.md"
errs := validateModuleSourceURL(body, filePath)
if len(errs) != 0 {
t.Errorf("Expected no errors for hyphen/underscore variation, got: %v", errs)
}
})
t.Run("Invalid file path format", func(t *testing.T) {
t.Parallel()
body := "# Test Module"
filePath := "invalid/path/format"
errs := validateModuleSourceURL(body, filePath)
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 len(errs) > 0 && !contains(errs[0].Error(), "invalid module path format") {
if !strings.Contains(errs[0].Error(), "invalid module path format") {
t.Errorf("Expected path format error, got: %s", errs[0].Error())
}
})
}
func contains(s, substr string) bool {
return len(s) >= len(substr) && (s == substr || (len(s) > len(substr) &&
(s[:len(substr)] == substr || s[len(s)-len(substr):] == substr ||
indexOfSubstring(s, substr) >= 0)))
}
func indexOfSubstring(s, substr string) int {
for i := 0; i <= len(s)-len(substr); i++ {
if s[i:i+len(substr)] == substr {
return i
}
}
return -1
}
}

14
cmd/readmevalidation/coderresources.go
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
@ -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