-
Notifications
You must be signed in to change notification settings - Fork 59
chore: add validation for module source URLs #406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
72d7ee4
5b6d878
3b6b1ba
7e94395
5419676
9c00c57
9cc3d5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,6 @@ on: | |
push: | ||
branches: | ||
- main | ||
- master | ||
pull_request: | ||
|
||
permissions: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,74 @@ 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 + `/([^/]+)/([^/]+)/coder"`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to swap the |
||
) | ||
|
||
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 | ||
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 { | ||
break | ||
} | ||
Comment on lines
+41
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just making sure we're okay with this: this still has the behavior of breaking out of the loop the moment we finish processing the first block |
||
continue | ||
} | ||
|
||
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 | ||
} | ||
} | ||
|
||
if !foundCorrectSource { | ||
errs = append(errs, xerrors.Errorf("did not find correct source URL %q in first Terraform code block", expectedSource)) | ||
} | ||
|
||
return errs | ||
} | ||
|
||
func validateCoderModuleReadmeBody(body string) []error { | ||
var errs []error | ||
|
||
|
@@ -94,6 +157,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 +209,4 @@ func validateAllCoderModules() error { | |
} | ||
logger.Info(context.Background(), "all relative URLs for READMEs are valid", "resource_type", resourceType) | ||
return nil | ||
} | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests generated by gemini CLI There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assuming we still want to process all the blocks in a file, I think it'd be really good to have some test cases that include 2+ blocks |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
module coder.com/coder-registry | ||
|
||
go 1.23.2 | ||
go 1.24 | ||
|
||
require ( | ||
cdr.dev/slog v1.6.1 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was having weird issues with running Golang CI locally, and had to match the Go version.