diff --git a/.github/workflows/psltool_check.yml b/.github/workflows/psltool_pr_check.yml similarity index 50% rename from .github/workflows/psltool_check.yml rename to .github/workflows/psltool_pr_check.yml index dbbd4d818..276e3a047 100644 --- a/.github/workflows/psltool_check.yml +++ b/.github/workflows/psltool_pr_check.yml @@ -1,8 +1,7 @@ -name: psltool +name: psltool PR check on: pull_request: - workflow_dispatch: permissions: {} @@ -18,5 +17,4 @@ jobs: - name: run validations run: | cd tools - go run ./psltool fmt -d ../public_suffix_list.dat - go run ./psltool validate ../public_suffix_list.dat + go run ./psltool fmt -d ../public_suffix_list.dat && go run ./psltool check-pr --gh-owner ${{ github.event.repository.owner.login }} --gh-repo ${{ github.event.repository.name }} --online-checks ${{ github.event.pull_request.number }} diff --git a/tools/internal/github/pr.go b/tools/internal/github/pr.go index 77f38cac0..69e683d89 100644 --- a/tools/internal/github/pr.go +++ b/tools/internal/github/pr.go @@ -15,7 +15,7 @@ import ( // Client is a GitHub API client that performs PSL-specific // operations. The zero value is a client that interacts with the // official publicsuffix/list repository. -type Client struct { +type Repo struct { // Owner is the github account of the repository to query. If // empty, defaults to "publicsuffix". Owner string @@ -25,21 +25,21 @@ type Client struct { client *github.Client } -func (c *Client) owner() string { +func (c *Repo) owner() string { if c.Owner != "" { return c.Owner } return "publicsuffix" } -func (c *Client) repo() string { +func (c *Repo) repo() string { if c.Repo != "" { return c.Repo } return "list" } -func (c *Client) apiClient() *github.Client { +func (c *Repo) apiClient() *github.Client { if c.client == nil { c.client = github.NewClient(nil) if token := os.Getenv("GITHUB_TOKEN"); token != "" { @@ -52,7 +52,7 @@ func (c *Client) apiClient() *github.Client { // PSLForPullRequest fetches the PSL files needed to validate the // given pull request. Returns the PSL file for the target branch, and // the same but with the PR's changes applied. -func (c *Client) PSLForPullRequest(ctx context.Context, prNum int) (withoutPR, withPR []byte, err error) { +func (c *Repo) PSLForPullRequest(ctx context.Context, prNum int) (withoutPR, withPR []byte, err error) { // Github sometimes needs a little time to think to update the PR // state, so we might need to sleep and retry a few times. Usually // the status updates in <5s, but just for safety, give it a more @@ -111,7 +111,7 @@ var errMergeInfoNotReady = errors.New("PR mergeability information not available // an open PR exists, but github needs a bit more time to update the // trial merge commit. The caller is expected to retry with // appropriate backoff. -func (c *Client) getPRCommitInfo(ctx context.Context, prNum int) (withoutPRCommit, withPRCommit string, err error) { +func (c *Repo) getPRCommitInfo(ctx context.Context, prNum int) (withoutPRCommit, withPRCommit string, err error) { ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() @@ -168,7 +168,7 @@ func (c *Client) getPRCommitInfo(ctx context.Context, prNum int) (withoutPRCommi } // PSLForHash returns the PSL file at the given git commit hash. -func (c *Client) PSLForHash(ctx context.Context, hash string) ([]byte, error) { +func (c *Repo) PSLForHash(ctx context.Context, hash string) ([]byte, error) { ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() diff --git a/tools/internal/parser/validate.go b/tools/internal/parser/validate.go index 8661128a8..ba061b79d 100644 --- a/tools/internal/parser/validate.go +++ b/tools/internal/parser/validate.go @@ -124,10 +124,10 @@ func validateSuffixUniqueness(block Block) (errs []error) { // validations are slower than offline validation, especially when // checking the entire PSL. All online validations respect // cancellation on the given context. -func ValidateOnline(ctx context.Context, l *List) (errs []error) { +func ValidateOnline(ctx context.Context, l *List, client *github.Repo) (errs []error) { for _, section := range BlocksOfType[*Section](l) { if section.Name == "PRIVATE DOMAINS" { - errs = append(errs, validateTXTRecords(ctx, section)...) + errs = append(errs, validateTXTRecords(ctx, section, client)...) break } } @@ -170,7 +170,7 @@ type txtRecordChecker struct { ctx context.Context // gh is the Github API client used to look up PRs and commits. - gh github.Client + gh *github.Repo // resolver is the DNS resolver used to do TXT lookups. resolver net.Resolver @@ -188,10 +188,11 @@ type txtRecordChecker struct { // validateTXTRecords checks the TXT records of all Suffix and // Wildcard blocks found under b. -func validateTXTRecords(ctx context.Context, b Block) (errs []error) { +func validateTXTRecords(ctx context.Context, b Block, client *github.Repo) (errs []error) { checker := txtRecordChecker{ ctx: ctx, prExpected: map[int]*prExpected{}, + gh: client, } // TXT checking happens in two phases: first, look up all TXT diff --git a/tools/psltool/psltool.go b/tools/psltool/psltool.go index 987072795..ae816833b 100644 --- a/tools/psltool/psltool.go +++ b/tools/psltool/psltool.go @@ -134,6 +134,8 @@ func runFmt(env *command.Env, path string) error { } var validateArgs struct { + Owner string `flag:"gh-owner,default=publicsuffix,Owner of the github repository to check"` + Repo string `flag:"gh-repo,default=list,Github repository to check"` Online bool `flag:"online-checks,Run validations that require querying third-party servers"` } @@ -149,12 +151,17 @@ func isHex(s string) bool { func runValidate(env *command.Env, pathOrHash string) error { var bs []byte var err error + + client := github.Repo{ + Owner: checkPRArgs.Owner, + Repo: checkPRArgs.Repo, + } + if _, err = os.Stat(pathOrHash); err == nil { // input is a local file bs, err = os.ReadFile(pathOrHash) } else if isHex(pathOrHash) { // input looks like a git hash - client := github.Client{} bs, err = client.PSLForHash(context.Background(), pathOrHash) } else { return fmt.Errorf("Failed to read PSL file %q, not a local file or a git commit hash", pathOrHash) @@ -172,7 +179,7 @@ func runValidate(env *command.Env, pathOrHash string) error { } else { ctx, cancel := context.WithTimeout(env.Context(), 1200*time.Second) defer cancel() - errs = append(errs, parser.ValidateOnline(ctx, psl)...) + errs = append(errs, parser.ValidateOnline(ctx, psl, &client)...) } } @@ -207,7 +214,7 @@ func runCheckPR(env *command.Env, prStr string) error { return fmt.Errorf("invalid PR number %q: %w", prStr, err) } - client := github.Client{ + client := github.Repo{ Owner: checkPRArgs.Owner, Repo: checkPRArgs.Repo, } @@ -224,7 +231,7 @@ func runCheckPR(env *command.Env, prStr string) error { if checkPRArgs.Online { ctx, cancel := context.WithTimeout(env.Context(), 300*time.Second) defer cancel() - errs = append(errs, parser.ValidateOnline(ctx, after)...) + errs = append(errs, parser.ValidateOnline(ctx, after, &client)...) } clean := after.MarshalPSL()