Skip to content

korg: add userinfo command for org and OWNERS lookup#6398

Open
palnabarun wants to merge 1 commit into
kubernetes:mainfrom
palnabarun:korg/memberinfo
Open

korg: add userinfo command for org and OWNERS lookup#6398
palnabarun wants to merge 1 commit into
kubernetes:mainfrom
palnabarun:korg/memberinfo

Conversation

@palnabarun

Copy link
Copy Markdown
Member

Adds korg userinfo <user>... to answer the recurring question this
repo exists for: which Kubernetes orgs is a user in, what role, and
where do they appear in OWNERS files.

Sources:

  • GitHub user API for company / profile
  • config//org.yaml (Members / Admins) for k8s org membership
  • cs.k8s.io (hound) for OWNERS file references

Behavior:

  • Honors GITHUB_TOKEN / GH_TOKEN to avoid the 60/hr unauth limit
  • Per-request context with a 30s timeout; SIGINT cancels in flight
  • Multi-user batches run concurrently (errgroup, bounded to 4) with output preserved in input order
  • One bad username does not kill the batch; per-user errors render and the process exits non-zero
  • Hound failures degrade to a warning instead of dropping org info
  • OWNERS results render as an aligned REPO / PATH / URL table; URL uses /blob/HEAD/ so the link resolves to the default branch
  • --output json for scripting

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: palnabarun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added area/github-management Issues or PRs related to GitHub Management subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 24, 2026
@palnabarun

Copy link
Copy Markdown
Member Author

/assign @kubernetes/owners

  Adds `korg userinfo <user>...` to answer the recurring question this
  repo exists for: which Kubernetes orgs is a user in, what role, and
  where do they appear in OWNERS files.

  Sources:
  - GitHub user API for company / profile
  - config/<org>/org.yaml (Members / Admins) for k8s org membership
  - cs.k8s.io (hound) for OWNERS file references

  Behavior:
  - Honors GITHUB_TOKEN / GH_TOKEN to avoid the 60/hr unauth limit
  - Per-request context with a 30s timeout; SIGINT cancels in flight
  - Multi-user batches run concurrently (errgroup, bounded to 4) with
    output preserved in input order
  - One bad username does not kill the batch; per-user errors render
    and the process exits non-zero
  - Hound failures degrade to a warning instead of dropping org info
  - OWNERS results render as an aligned REPO / PATH / URL table; URL
    uses /blob/HEAD/ so the link resolves to the default branch
  - `--output json` for scripting

  Dependencies:
  - google/go-github v17 (untagged) -> v88
  - spf13/cobra v0.0.5 -> v1.10.2 (for ExecuteContext)
  - adds golang.org/x/sync for errgroup
  - go directive bumped to 1.22+

Signed-off-by: Nabarun Pal <pal.nabarun95@gmail.com>

@Priyankasaggu11929 Priyankasaggu11929 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @palnabarun!

left a few inline comments.

Comment thread cmd/korg/userinfo.go
@@ -0,0 +1,306 @@
/*
Copyright 2023 The Kubernetes Authors.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Copyright 2023 The Kubernetes Authors.
Copyright The Kubernetes Authors.

per kubernetes/kubernetes@c247514

Comment thread cmd/korg/userinfo.go

func searchOwnerFiles(ctx context.Context, hc *http.Client, username string) ([]OwnerFile, error) {
q := url.Values{}
q.Set("q", username)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we do a search for "- " + username instead of just username?

since OWNERS entries are always YAML list items

Comment thread cmd/korg/userinfo.go
q.Set("q", username)
q.Set("repos", "*")
q.Set("rng", ":20")
q.Set("files", "OWNERS")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's include OWNERS_ALIASES as well

since that defines named groups (e.g. sig-foo-approvers) which are referenced in OWNERS files

Comment thread cmd/korg/userinfo.go
Comment on lines +181 to +186
for _, fm := range matches.Matches {
out = append(out, OwnerFile{
Repo: repo,
Path: fm.Filename,
URL: ownerFileURL(repo, fm.Filename),
})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a check here to skip lines starting with # in the result before appending them to out?

because commented out usernames also will show up as valid hit right now

Also -

(since we already have the repo + file path from hound)
could we fetch the actual OWNERS file from GitHub and parse which section the username falls under? because someone in emeritus_approvers/emeritus_reviewers isn't valid

it will add 1-2 extra API calls maybe but since its supposed to be used locally on a handful of users at a time, it think that should be ok

Comment thread cmd/korg/userinfo_test.go
@@ -0,0 +1,286 @@
/*
Copyright 2026 The Kubernetes Authors.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Copyright 2026 The Kubernetes Authors.
Copyright The Kubernetes Authors.

per kubernetes/kubernetes@c247514

@cblecker cblecker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition — this fills a genuine gap and the overall structure is solid. The errgroup concurrency pattern is correct, Hound failures degrade gracefully, and it's good to see tests land alongside the new code. A few things worth addressing before this merges, mostly around the JSON output dropping failed users and a double-print in main(). The go.mod changes are covered separately.

Comment thread go.mod
github.com/spf13/cobra v1.10.2
golang.org/x/sync v0.20.0
k8s.io/apimachinery v0.33.11
k8s.io/test-infra v0.0.0-20191222193732-de81526abe72

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what happened here, but you're pulling in a whole bunch of new/different stuff that seems unrelated, for example, the test-infra repo?

Comment thread go.mod
gopkg.in/warnings.v0 v0.1.2 // indirect
)

// Pin all k8s.io staging repositories to kubernetes-1.15.3.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't accurate?

Comment thread cmd/korg/korg.go
defer stop()

if err := rootCmd.ExecuteContext(ctx); err != nil {
fmt.Fprintln(os.Stderr, "error:", err)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cobra already prints command errors to stderr by default, so this line will double-print every error. Either drop it to preserve the previous behavior, or set rootCmd.SilenceErrors = true before ExecuteContext if you want to own the formatting yourself.

Comment thread cmd/korg/userinfo.go

// newGitHubClient returns a GitHub client authenticated via GITHUB_TOKEN or GH_TOKEN if set.
func newGitHubClient(ctx context.Context, httpClient *http.Client) (*github.Client, error) {
token := os.Getenv("GITHUB_TOKEN")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ctx isn't used anywhere in this function — it only reads env vars and constructs a client. Either drop it from the signature or use _ if you're planning to need it later.

Comment thread cmd/korg/userinfo.go
g.SetLimit(userinfoConcurrency)
for i, name := range usernames {
i, name := i, name
g.Go(func() error {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop-variable capture was needed before Go 1.22, but since the module is already at 1.22+ (and this PR bumps it further), loop variables are per-iteration by default. This line can be removed.

Comment thread cmd/korg/userinfo.go
})
}
_ = g.Wait()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the goroutines always return nil today, discarding the Wait() return is technically safe — but it's a maintenance trap. If someone later changes a goroutine to return a real error, it'll vanish silently. A small guard makes the invariant explicit:

if err := g.Wait(); err != nil {
    return fmt.Errorf("unexpected errgroup error: %w", err)
}

Comment thread cmd/korg/userinfo.go
_ = g.Wait()

if outputJSON {
clean := make([]*UserDetails, 0, len(results))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a user lookup fails in JSON mode, that user is silently dropped from the output array. A script parsing stdout will get fewer results than expected, with no structured way to tell which lookups failed or why. Text mode handles this well (it renders an ERROR: block inline), but JSON consumers have to correlate via exit code alone.

Worth either including an error entry in the array (e.g. {"username":"ghost","error":"..."}) or at least documenting in the help text that JSON mode omits failures and callers should check the exit code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/github-management Issues or PRs related to GitHub Management subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants