Improve psltool PR check (#2218)

.github/workflows/psltool_check.yml → .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 }}

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()
 

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

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()