STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #8899 from babakks/8679-include-num-selected-repos

Browse source 
Include `numSelectedRepos` in JSON output of `gh secret list`
This commit is contained in:
Andy Feller 2024-04-09 13:18:50 -04:00 • committed by GitHub
commit 6a55528882
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 278 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

17
pkg/cmd/secret/list/list.go
View file

@ -3,6 +3,7 @@ package list
import (
"fmt"
"net/http"
"slices"
"strings"
"time"
@ -39,6 +40,8 @@ var secretFields = []string{
"numSelectedRepos",
}
const fieldNumSelectedRepos = "numSelectedRepos"
func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command {
opts := &ListOptions{
IO: f.IOStreams,
@ -114,9 +117,21 @@ func listRun(opts *ListOptions) error {
return fmt.Errorf("%s secrets are not supported for %s", secretEntity, secretApp)
}
var secrets []Secret
// Since populating the `NumSelectedRepos` field costs further API requests
// (one per secret), it's important to avoid extra calls when the output will
// not present the field's value. So, we should only populate this field in
// these cases:
// 1. The command is run in the TTY mode without the `--json <fields>` option.
// 2. The command is run with `--json <fields>` option, and `numSelectedRepos`
// is among the selected fields. In this case, TTY mode is irrelevant.
showSelectedRepoInfo := opts.IO.IsStdoutTTY()
if opts.Exporter != nil {
// Note that if there's an exporter set, then we don't mind the TTY mode
// because we just have to populate the requested fields.
showSelectedRepoInfo = slices.Contains(opts.Exporter.Fields(), fieldNumSelectedRepos)
}
var secrets []Secret
switch secretEntity {
case shared.Repository:
secrets, err = getRepoSecrets(client, baseRepo, secretApp)

280
pkg/cmd/secret/list/list_test.go
View file

@ -104,6 +104,7 @@ func Test_listRun(t *testing.T) {
tests := []struct {
name string
tty bool
json bool
opts *ListOptions
wantOut []string
}{
@ -153,6 +154,30 @@ func Test_listRun(t *testing.T) {
"SECRET_THREE\t1975-11-30T00:00:00Z\tSELECTED",
},
},
{
name: "org tty, json",
tty: true,
json: true,
opts: &ListOptions{
OrgName: "UmbrellaCorporation",
},
wantOut: []string{
// Note the `"numSelectedRepos":2` pair in the last entry.
`[{"name":"SECRET_ONE","numSelectedRepos":0,"selectedReposURL":"","updatedAt":"1988-10-11T00:00:00Z","visibility":"all"},{"name":"SECRET_TWO","numSelectedRepos":0,"selectedReposURL":"","updatedAt":"2020-12-04T00:00:00Z","visibility":"private"},{"name":"SECRET_THREE","numSelectedRepos":2,"selectedReposURL":"https://api.github.com/orgs/UmbrellaCorporation/actions/secrets/SECRET_THREE/repositories","updatedAt":"1975-11-30T00:00:00Z","visibility":"selected"}]`,
},
},
{
name: "org not tty, json",
tty: false,
json: true,
opts: &ListOptions{
OrgName: "UmbrellaCorporation",
},
wantOut: []string{
// Note the `"numSelectedRepos":2` pair in the last entry.
`[{"name":"SECRET_ONE","numSelectedRepos":0,"selectedReposURL":"","updatedAt":"1988-10-11T00:00:00Z","visibility":"all"},{"name":"SECRET_TWO","numSelectedRepos":0,"selectedReposURL":"","updatedAt":"2020-12-04T00:00:00Z","visibility":"private"},{"name":"SECRET_THREE","numSelectedRepos":2,"selectedReposURL":"https://api.github.com/orgs/UmbrellaCorporation/actions/secrets/SECRET_THREE/repositories","updatedAt":"1975-11-30T00:00:00Z","visibility":"selected"}]`,
},
},
{
name: "env tty",
tty: true,
@ -203,6 +228,30 @@ func Test_listRun(t *testing.T) {
"SECRET_THREE\t1975-11-30T00:00:00Z\tSELECTED",
},
},
{
name: "user tty, json",
tty: true,
json: true,
opts: &ListOptions{
UserSecrets: true,
},
wantOut: []string{
// Note that `numSelectedRepos` fields are not set to default (zero).
`[{"name":"SECRET_ONE","numSelectedRepos":1,"selectedReposURL":"https://api.github.com/user/codespaces/secrets/SECRET_ONE/repositories","updatedAt":"1988-10-11T00:00:00Z","visibility":"selected"},{"name":"SECRET_TWO","numSelectedRepos":2,"selectedReposURL":"https://api.github.com/user/codespaces/secrets/SECRET_TWO/repositories","updatedAt":"2020-12-04T00:00:00Z","visibility":"selected"},{"name":"SECRET_THREE","numSelectedRepos":3,"selectedReposURL":"https://api.github.com/user/codespaces/secrets/SECRET_THREE/repositories","updatedAt":"1975-11-30T00:00:00Z","visibility":"selected"}]`,
},
},
{
name: "user not tty, json",
tty: false,
json: true,
opts: &ListOptions{
UserSecrets: true,
},
wantOut: []string{
// Note that `numSelectedRepos` fields are not set to default (zero).
`[{"name":"SECRET_ONE","numSelectedRepos":1,"selectedReposURL":"https://api.github.com/user/codespaces/secrets/SECRET_ONE/repositories","updatedAt":"1988-10-11T00:00:00Z","visibility":"selected"},{"name":"SECRET_TWO","numSelectedRepos":2,"selectedReposURL":"https://api.github.com/user/codespaces/secrets/SECRET_TWO/repositories","updatedAt":"2020-12-04T00:00:00Z","visibility":"selected"},{"name":"SECRET_THREE","numSelectedRepos":3,"selectedReposURL":"https://api.github.com/user/codespaces/secrets/SECRET_THREE/repositories","updatedAt":"1975-11-30T00:00:00Z","visibility":"selected"}]`,
},
},
{
name: "Dependabot repo tty",
tty: true,
@ -310,13 +359,11 @@ func Test_listRun(t *testing.T) {
},
}
if tt.tty {
reg.Register(
httpmock.REST("GET", fmt.Sprintf("orgs/%s/actions/secrets/SECRET_THREE/repositories", tt.opts.OrgName)),
httpmock.JSONResponse(struct {
TotalCount int `json:"total_count"`
}{2}))
}
reg.Register(
httpmock.REST("GET", fmt.Sprintf("orgs/%s/actions/secrets/SECRET_THREE/repositories", tt.opts.OrgName)),
httpmock.JSONResponse(struct {
TotalCount int `json:"total_count"`
}{2}))
}
if tt.opts.UserSecrets {
@ -342,17 +389,15 @@ func Test_listRun(t *testing.T) {
}
path = "user/codespaces/secrets"
if tt.tty {
for i, secret := range payload.Secrets {
hostLen := len("https://api.github.com/")
path := secret.SelectedReposURL[hostLen:len(secret.SelectedReposURL)]
repositoryCount := i + 1
reg.Register(
httpmock.REST("GET", path),
httpmock.JSONResponse(struct {
TotalCount int `json:"total_count"`
}{repositoryCount}))
}
for i, secret := range payload.Secrets {
hostLen := len("https://api.github.com/")
path := secret.SelectedReposURL[hostLen:len(secret.SelectedReposURL)]
repositoryCount := i + 1
reg.Register(
httpmock.REST("GET", path),
httpmock.JSONResponse(struct {
TotalCount int `json:"total_count"`
}{repositoryCount}))
}
}
@ -381,6 +426,12 @@ func Test_listRun(t *testing.T) {
return t
}
if tt.json {
exporter := cmdutil.NewJSONExporter()
exporter.SetFields(secretFields)
tt.opts.Exporter = exporter
}
err := listRun(tt.opts)
assert.NoError(t, err)
@ -390,6 +441,199 @@ func Test_listRun(t *testing.T) {
}
}
// Test_listRun_populatesNumSelectedReposIfRequired asserts that NumSelectedRepos
// field is populated **only** when it's going to be presented in the output. Since
// populating this field costs further API requests (one per secret), it's important
// to avoid extra calls when the output will not present the field's value. Note
// that NumSelectedRepos is only meant for user or organization secrets.
//
// We should only populate the NumSelectedRepos field in these cases:
// 1. The command is run in the TTY mode without the `--json <fields>` option.
// 2. The command is run with `--json <fields>` option, and `numSelectedRepos`
// is among the selected fields. In this case, TTY mode is irrelevant.
func Test_listRun_populatesNumSelectedReposIfRequired(t *testing.T) {
type secretKind string
const secretKindUser secretKind = "user"
const secretKindOrg secretKind = "org"
tests := []struct {
name string
kind secretKind
tty bool
jsonFields []string
wantPopulated bool
}{
{
name: "org tty",
kind: secretKindOrg,
tty: true,
wantPopulated: true,
},
{
name: "org tty, json with numSelectedRepos",
kind: secretKindOrg,
tty: true,
jsonFields: []string{"numSelectedRepos"},
wantPopulated: true,
},
{
name: "org tty, json without numSelectedRepos",
kind: secretKindOrg,
tty: true,
jsonFields: []string{"name"},
wantPopulated: false,
},
{
name: "org not tty",
kind: secretKindOrg,
tty: false,
wantPopulated: false,
},
{
name: "org not tty, json with numSelectedRepos",
kind: secretKindOrg,
tty: false,
jsonFields: []string{"numSelectedRepos"},
wantPopulated: true,
},
{
name: "org not tty, json without numSelectedRepos",
kind: secretKindOrg,
tty: false,
jsonFields: []string{"name"},
wantPopulated: false,
},
{
name: "user tty",
kind: secretKindUser,
tty: true,
wantPopulated: true,
},
{
name: "user tty, json with numSelectedRepos",
kind: secretKindUser,
tty: true,
jsonFields: []string{"numSelectedRepos"},
wantPopulated: true,
},
{
name: "user tty, json without numSelectedRepos",
kind: secretKindUser,
tty: true,
jsonFields: []string{"name"},
wantPopulated: false,
},
{
name: "user not tty",
kind: secretKindUser,
tty: false,
wantPopulated: false,
},
{
name: "user not tty, json with numSelectedRepos",
kind: secretKindUser,
tty: false,
jsonFields: []string{"numSelectedRepos"},
wantPopulated: true,
},
{
name: "user not tty, json without numSelectedRepos",
kind: secretKindUser,
tty: false,
jsonFields: []string{"name"},
wantPopulated: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
reg.Verify(t)
t0, _ := time.Parse("2006-01-02", "1988-10-11")
opts := &ListOptions{}
if tt.kind == secretKindOrg {
opts.OrgName = "umbrellaOrganization"
reg.Register(
httpmock.REST("GET", "orgs/umbrellaOrganization/actions/secrets"),
httpmock.JSONResponse(struct{ Secrets []Secret }{
[]Secret{
{
Name: "SECRET",
UpdatedAt: t0,
Visibility: shared.Selected,
SelectedReposURL: "https://api.github.com/orgs/umbrellaOrganization/actions/secrets/SECRET/repositories",
},
},
}))
reg.Register(
httpmock.REST("GET", "orgs/umbrellaOrganization/actions/secrets/SECRET/repositories"),
httpmock.JSONResponse(struct {
TotalCount int `json:"total_count"`
}{999}))
}
if tt.kind == secretKindUser {
opts.UserSecrets = true
reg.Register(
httpmock.REST("GET", "user/codespaces/secrets"),
httpmock.JSONResponse(struct{ Secrets []Secret }{
[]Secret{
{
Name: "SECRET",
UpdatedAt: t0,
Visibility: shared.Selected,
SelectedReposURL: "https://api.github.com/user/codespaces/secrets/SECRET/repositories",
},
},
}))
reg.Register(
httpmock.REST("GET", "user/codespaces/secrets/SECRET/repositories"),
httpmock.JSONResponse(struct {
TotalCount int `json:"total_count"`
}{999}))
}
if tt.jsonFields != nil {
exporter := cmdutil.NewJSONExporter()
exporter.SetFields(tt.jsonFields)
opts.Exporter = exporter
}
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(tt.tty)
opts.IO = ios
opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
}
opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
opts.Config = func() (config.Config, error) {
return config.NewBlankConfig(), nil
}
opts.Now = func() time.Time {
t, _ := time.Parse(time.RFC822, "4 Apr 24 00:00 UTC")
return t
}
err := listRun(opts)
assert.NoError(t, err)
if tt.wantPopulated {
// There should be 2 requests; one to get the secrets list and
// another to populate the numSelectedRepos field.
assert.Len(t, reg.Requests, 2)
} else {
// Only one requests to get the secrets list.
assert.Len(t, reg.Requests, 1)
}
})
}
}
func Test_getSecrets_pagination(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)