From 9f035798d13e0c394a4ece66e1990c3012425237 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Tue, 15 Apr 2025 14:32:38 +0000 Subject: [PATCH] wip: commit progress --- .env.example | 23 ++++ .env_example | 5 - cmd/github/github.go | 220 ++++++++++++++++++++--------------- cmd/readmevalidation/main.go | 30 ++++- 4 files changed, 175 insertions(+), 103 deletions(-) create mode 100644 .env.example delete mode 100644 .env_example diff --git a/.env.example b/.env.example new file mode 100644 index 00000000..6bf034b5 --- /dev/null +++ b/.env.example @@ -0,0 +1,23 @@ +# This should be the value of the GitHub Actions actor who triggered a run. The +# CI script will inject this value from the GitHub Actions context to verify +# whether changing certain README fields is allowed. In local development, you +# can set this to your GitHub username. +CI_ACTOR= + +# This is the Git ref that you want to merge into the main branch. In local +# development, this should be set to the value of the branch you're working from +CI_BASE_REF= + +# This is the configurable base URL for accessing the GitHub REST API. This +# value will be injected by the CI script's Actions context, but if the value is +# not defined (either in CI or when running locally), "https://api.github.com/" +# will be used as a fallback. +GITHUB_API_URL= + +# This is the API token for the user that will be used to authenticate calls to +# the GitHub API. In CI, the value will be loaded with a token belonging to a +# Coder Registry admin to verify whether modifying certain README fields is +# allowed. In local development, you can set a token with the read:org +# permission. If the loaded token does not belong to a Coder employee, certain +# README verification steps will be skipped. +GITHUB_API_TOKEN= diff --git a/.env_example b/.env_example deleted file mode 100644 index 267b2039..00000000 --- a/.env_example +++ /dev/null @@ -1,5 +0,0 @@ -ACTOR= -BASE_REF= -HEAD_REF= -GITHUB_API_URL= -GITHUB_API_TOKEN= \ No newline at end of file diff --git a/cmd/github/github.go b/cmd/github/github.go index d5f67842..68b40f14 100644 --- a/cmd/github/github.go +++ b/cmd/github/github.go @@ -10,16 +10,14 @@ import ( "log" "net/http" "os" - "strings" "time" ) -const defaultGithubAPIRoute = "https://api.github.com/" +const defaultGithubAPIBaseRoute = "https://api.github.com/" const ( - actionsActorKey = "ACTOR" - actionsBaseRefKey = "BASE_REF" - actionsHeadRefKey = "HEAD_REF" + actionsActorKey = "CI_ACTOR" + actionsBaseRefKey = "CI_BASE_REF" ) const ( @@ -38,119 +36,157 @@ func ActionsActor() (string, error) { return username, nil } -// ActionsRefs returns the name of the head ref and the base ref for current CI -// run, in that order. Both values must be loaded into the env as part of the -// GitHub Actions YAML file, or else the function fails. -func ActionsRefs() (string, string, error) { +// BaseRef returns the name of the base ref for the Git branch that will be +// merged into the main branch. +func BaseRef() (string, error) { baseRef := os.Getenv(actionsBaseRefKey) - headRef := os.Getenv(actionsHeadRefKey) - - if baseRef == "" && headRef == "" { - return "", "", fmt.Errorf("values for %q and %q are not in env. If running from CI, please add values via ci.yaml file", actionsHeadRefKey, actionsBaseRefKey) - } else if headRef == "" { - return "", "", fmt.Errorf("value for %q is not in env. If running from CI, please add value via ci.yaml file", actionsHeadRefKey) - } else if baseRef == "" { - return "", "", fmt.Errorf("value for %q is not in env. If running from CI, please add value via ci.yaml file", actionsBaseRefKey) + if baseRef == "" { + return "", fmt.Errorf("value for %q is not in env. If running from CI, please add value via ci.yaml file", actionsBaseRefKey) } - return headRef, baseRef, nil + return baseRef, nil } -// CoderEmployees represents all members of the Coder GitHub organization. This -// value should not be instantiated from outside the package, and should instead -// be created via one of the package's exported functions. -type CoderEmployees struct { - // Have map defined as private field to make sure that it can't ever be - // mutated from an outside package - _employees map[string]struct{} +// Client is a reusable REST client for making requests to the GitHub API. +// It should be instantiated via NewGithubClient +type Client struct { + baseURL string + token string + httpClient http.Client } -// IsEmployee takes a GitHub username and indicates whether the matching user is -// a member of the Coder organization -func (ce *CoderEmployees) IsEmployee(username string) bool { - if ce._employees == nil { - return false +// NewClient instantiates a GitHub client +func NewClient() (*Client, error) { + // Considered letting the user continue on with no token and more aggressive + // rate-limiting, but from experimentation, the non-authenticated experience + // hit the rate limits really quickly, and had a lot of restrictions + apiToken := os.Getenv(githubAPITokenKey) + if apiToken == "" { + return nil, fmt.Errorf("missing env variable %q", githubAPITokenKey) } - _, ok := ce._employees[username] - return ok + baseURL := os.Getenv(githubAPIURLKey) + if baseURL == "" { + log.Printf("env variable %q is not defined. Falling back to %q\n", githubAPIURLKey, defaultGithubAPIBaseRoute) + baseURL = defaultGithubAPIBaseRoute + } + + return &Client{ + baseURL: baseURL, + token: apiToken, + httpClient: http.Client{Timeout: 10 * time.Second}, + }, nil } -// TotalEmployees returns the number of members in the Coder organization -func (ce *CoderEmployees) TotalEmployees() int { - return len(ce._employees) -} - -type ghOrganizationMember struct { +// User represents a truncated version of the API response from Github's /user +// endpoint. +type User struct { Login string `json:"login"` } -type ghRateLimitedRes struct { - Message string `json:"message"` -} - -func parseResponse[V any](b []byte) (V, error) { - var want V - var rateLimitedRes ghRateLimitedRes - - if err := json.Unmarshal(b, &rateLimitedRes); err != nil { - return want, err - } - if isRateLimited := strings.Contains(rateLimitedRes.Message, "API rate limit exceeded for "); isRateLimited { - return want, errors.New("request was rate-limited") - } - if err := json.Unmarshal(b, &want); err != nil { - return want, err - } - - return want, nil -} - -// CoderEmployeeUsernames requests from the GitHub API the list of all usernames -// of people who are employees of Coder. -func CoderEmployeeUsernames() (CoderEmployees, error) { - apiURL := os.Getenv(githubAPIURLKey) - if apiURL == "" { - log.Printf("API URL not set via env key %q. Defaulting to %q\n", githubAPIURLKey, defaultGithubAPIRoute) - apiURL = defaultGithubAPIRoute - } - token := os.Getenv(githubAPITokenKey) - if token == "" { - log.Printf("API token not set via env key %q. All requests will be non-authenticated and subject to more aggressive rate limiting", githubAPITokenKey) - } - - req, err := http.NewRequest("GET", apiURL+"/orgs/coder/members", nil) +// GetUserFromToken returns the user associated with the loaded API token +func (gc *Client) GetUserFromToken() (User, error) { + req, err := http.NewRequest("GET", gc.baseURL+"user", nil) if err != nil { - return CoderEmployees{}, fmt.Errorf("coder employee names: %v", err) + return User{}, err } - if token != "" { - req.Header.Add("Authorization", "Bearer "+token) + if gc.token != "" { + req.Header.Add("Authorization", "Bearer "+gc.token) } - client := http.Client{Timeout: 5 * time.Second} - res, err := client.Do(req) + res, err := gc.httpClient.Do(req) if err != nil { - return CoderEmployees{}, fmt.Errorf("coder employee names: %v", err) + return User{}, err } defer res.Body.Close() - if res.StatusCode != http.StatusOK { - return CoderEmployees{}, fmt.Errorf("coder employee names: got back status code %d", res.StatusCode) + + if res.StatusCode == http.StatusUnauthorized { + return User{}, errors.New("request is not authorized") + } + if res.StatusCode == http.StatusForbidden { + return User{}, errors.New("request is forbidden") } b, err := io.ReadAll(res.Body) if err != nil { - return CoderEmployees{}, fmt.Errorf("coder employee names: %v", err) - } - rawMembers, err := parseResponse[[]ghOrganizationMember](b) - if err != nil { - return CoderEmployees{}, fmt.Errorf("coder employee names: %v", err) + return User{}, err } - employeesSet := map[string]struct{}{} - for _, m := range rawMembers { - employeesSet[m.Login] = struct{}{} + user := User{} + if err := json.Unmarshal(b, &user); err != nil { + return User{}, err + } + return user, nil +} + +// OrgStatus indicates whether a GitHub user is a member of a given organization +type OrgStatus int + +var _ fmt.Stringer = OrgStatus(0) + +const ( + // OrgStatusIndeterminate indicates when a user's organization status + // could not be determined. It is the zero value of the OrgStatus type, and + // any users with this value should be treated as completely untrusted + OrgStatusIndeterminate = iota + // OrgStatusNonMember indicates when a user is definitely NOT part of an + // organization + OrgStatusNonMember + // OrgStatusMember indicates when a user is a member of a Github + // organization + OrgStatusMember +) + +func (s OrgStatus) String() string { + switch s { + case OrgStatusMember: + return "Member" + case OrgStatusNonMember: + return "Non-member" + default: + return "Indeterminate" + } +} + +// GetUserOrgStatus takes a GitHub username, and checks the GitHub API to see +// whether that member is part of the Coder organization +func (gc *Client) GetUserOrgStatus(org string, username string) (OrgStatus, error) { + // This API endpoint is really annoying, because it's able to produce false + // negatives. Any user can be a public member of Coder, a private member of + // Coder, or a non-member. + // + // So if the function returns status 200, you can always trust that. But if + // it returns any 400 code, that could indicate a few things: + // 1. The user being checked is not part of the organization, but the user + // associated with the token is. + // 2. The user being checked is a member of the organization, but their + // status is private, and the token being used to check belongs to a user + // who is not part of the Coder organization. + // 3. Neither the user being checked nor the user associated with the token + // are members of the organization + // + // The best option is to make sure that the token being used belongs to a + // member of the Coder organization + req, err := http.NewRequest("GET", fmt.Sprintf("%sorgs/%s/%s", gc.baseURL, org, username), nil) + if err != nil { + return OrgStatusIndeterminate, err + } + if gc.token != "" { + req.Header.Add("Authorization", "Bearer "+gc.token) + } + + res, err := gc.httpClient.Do(req) + if err != nil { + return OrgStatusIndeterminate, err + } + defer res.Body.Close() + + switch res.StatusCode { + case http.StatusNoContent: + return OrgStatusMember, nil + case http.StatusNotFound: + return OrgStatusNonMember, nil + default: + return OrgStatusIndeterminate, nil } - return CoderEmployees{ - _employees: employeesSet, - }, nil } diff --git a/cmd/readmevalidation/main.go b/cmd/readmevalidation/main.go index 8fd657c2..9a5aaf97 100644 --- a/cmd/readmevalidation/main.go +++ b/cmd/readmevalidation/main.go @@ -7,6 +7,7 @@ package main import ( + "fmt" "log" "coder.com/coder-registry/cmd/github" @@ -14,26 +15,43 @@ import ( ) func main() { + log.Println("Beginning README file validation") err := godotenv.Load() if err != nil { log.Panic(err) } - username, err := github.ActionsActor() + actorUsername, err := github.ActionsActor() if err != nil { log.Panic(err) } - log.Printf("Running validation for user %q", username) - headRef, baseRef, err := github.ActionsRefs() + baseRef, err := github.BaseRef() if err != nil { log.Panic(err) } - log.Printf("Using branches %q and %q for validation comparison", headRef, baseRef) + log.Printf("Using branch %q for validation comparison", baseRef) - employees, err := github.CoderEmployeeUsernames() + log.Printf("Using GitHub API to determine what fields can be set by user %q\n", actorUsername) + client, err := github.NewClient() if err != nil { log.Panic(err) } - log.Printf("got back %d employees\n", employees.TotalEmployees()) + tokenUser, err := client.GetUserFromToken() + if err != nil { + log.Panic(err) + } + tokenUserStatus, err := client.GetUserOrgStatus("coder", tokenUser.Login) + if err != nil { + log.Panic(err) + } + var actorOrgStatus github.OrgStatus + if tokenUserStatus == github.OrgStatusMember { + actorOrgStatus, err = client.GetUserOrgStatus("coder", actorUsername) + if err != nil { + log.Panic(err) + } + } + + fmt.Printf("actor %q is %s\n", actorUsername, actorOrgStatus.String()) log.Println("Starting README validation") allReadmeFiles, err := aggregateContributorReadmeFiles()