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

Tech debt cleanup for variable and secret commands (#7151)

Browse source
This commit is contained in:
Sam Coe 2023-03-15 12:03:56 +11:00 • committed by GitHub
parent 5191c502e9
commit 41a457136e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 259 additions and 432 deletions
Download patch file Download diff file Expand all files Collapse all files

3
pkg/cmd/secret/delete/delete_test.go
View file

@ -116,7 +116,6 @@ func TestNewCmdDelete(t *testing.T) {
assert.Equal(t, tt.wants.EnvName, gotOpts.EnvName)
})
}
}
func Test_removeRun_repo(t *testing.T) {
@ -264,10 +263,8 @@ func Test_removeRun_org(t *testing.T) {
assert.NoError(t, err)
reg.Verify(t)
})
}
}
func Test_removeRun_user(t *testing.T) {

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

@ -1,22 +1,19 @@
package list
import (
"encoding/json"
"fmt"
"net/http"
"regexp"
"strings"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/tableprinter"
"github.com/cli/cli/v2/pkg/cmd/secret/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/cli/cli/v2/utils"
"github.com/spf13/cobra"
)
@ -25,6 +22,7 @@ type ListOptions struct {
IO *iostreams.IOStreams
Config func() (config.Config, error)
BaseRepo func() (ghrepo.Interface, error)
Now func() time.Time
OrgName string
EnvName string
@ -37,6 +35,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
IO: f.IOStreams,
Config: f.Config,
HttpClient: f.HttpClient,
Now: time.Now,
}
cmd := &cobra.Command{
@ -106,7 +105,7 @@ func listRun(opts *ListOptions) error {
return fmt.Errorf("%s secrets are not supported for %s", secretEntity, secretApp)
}
var secrets []*Secret
var secrets []Secret
showSelectedRepoInfo := opts.IO.IsStdoutTTY()
switch secretEntity {
@ -146,26 +145,26 @@ func listRun(opts *ListOptions) error {
fmt.Fprintf(opts.IO.ErrOut, "failed to start pager: %v\n", err)
}
//nolint:staticcheck // SA1019: utils.NewTablePrinter is deprecated: use internal/tableprinter
tp := utils.NewTablePrinter(opts.IO)
table := tableprinter.New(opts.IO)
if secretEntity == shared.Organization || secretEntity == shared.User {
table.HeaderRow("Name", "Updated", "Visibility")
} else {
table.HeaderRow("Name", "Updated")
}
for _, secret := range secrets {
tp.AddField(secret.Name, nil, nil)
updatedAt := secret.UpdatedAt.Format("2006-01-02")
if opts.IO.IsStdoutTTY() {
updatedAt = fmt.Sprintf("Updated %s", updatedAt)
}
tp.AddField(updatedAt, nil, nil)
table.AddField(secret.Name)
table.AddTimeField(opts.Now(), secret.UpdatedAt, nil)
if secret.Visibility != "" {
if showSelectedRepoInfo {
tp.AddField(fmtVisibility(*secret), nil, nil)
table.AddField(fmtVisibility(secret))
} else {
tp.AddField(strings.ToUpper(string(secret.Visibility)), nil, nil)
table.AddField(strings.ToUpper(string(secret.Visibility)))
}
}
tp.EndRow()
table.EndRow()
}
err = tp.Render()
err = table.Render()
if err != nil {
return err
}
@ -197,14 +196,14 @@ func fmtVisibility(s Secret) string {
return ""
}
func getOrgSecrets(client httpClient, host, orgName string, showSelectedRepoInfo bool, app shared.App) ([]*Secret, error) {
func getOrgSecrets(client *http.Client, host, orgName string, showSelectedRepoInfo bool, app shared.App) ([]Secret, error) {
secrets, err := getSecrets(client, host, fmt.Sprintf("orgs/%s/%s/secrets", orgName, app))
if err != nil {
return nil, err
}
if showSelectedRepoInfo {
err = getSelectedRepositoryInformation(client, secrets)
err = populateSelectedRepositoryInformation(client, host, secrets)
if err != nil {
return nil, err
}
@ -212,14 +211,14 @@ func getOrgSecrets(client httpClient, host, orgName string, showSelectedRepoInfo
return secrets, nil
}
func getUserSecrets(client httpClient, host string, showSelectedRepoInfo bool) ([]*Secret, error) {
func getUserSecrets(client *http.Client, host string, showSelectedRepoInfo bool) ([]Secret, error) {
secrets, err := getSecrets(client, host, "user/codespaces/secrets")
if err != nil {
return nil, err
}
if showSelectedRepoInfo {
err = getSelectedRepositoryInformation(client, secrets)
err = populateSelectedRepositoryInformation(client, host, secrets)
if err != nil {
return nil, err
}
@ -228,96 +227,46 @@ func getUserSecrets(client httpClient, host string, showSelectedRepoInfo bool) (
return secrets, nil
}
func getEnvSecrets(client httpClient, repo ghrepo.Interface, envName string) ([]*Secret, error) {
func getEnvSecrets(client *http.Client, repo ghrepo.Interface, envName string) ([]Secret, error) {
path := fmt.Sprintf("repos/%s/environments/%s/secrets", ghrepo.FullName(repo), envName)
return getSecrets(client, repo.RepoHost(), path)
}
func getRepoSecrets(client httpClient, repo ghrepo.Interface, app shared.App) ([]*Secret, error) {
return getSecrets(client, repo.RepoHost(), fmt.Sprintf("repos/%s/%s/secrets",
ghrepo.FullName(repo), app))
func getRepoSecrets(client *http.Client, repo ghrepo.Interface, app shared.App) ([]Secret, error) {
return getSecrets(client, repo.RepoHost(), fmt.Sprintf("repos/%s/%s/secrets", ghrepo.FullName(repo), app))
}
type secretsPayload struct {
Secrets []*Secret
}
type httpClient interface {
Do(*http.Request) (*http.Response, error)
}
func getSecrets(client httpClient, host, path string) ([]*Secret, error) {
var results []*Secret
url := fmt.Sprintf("%s%s?per_page=100", ghinstance.RESTPrefix(host), path)
for {
var payload secretsPayload
nextURL, err := apiGet(client, url, &payload)
func getSecrets(client *http.Client, host, path string) ([]Secret, error) {
var results []Secret
apiClient := api.NewClientFromHTTP(client)
path = fmt.Sprintf("%s?per_page=100", path)
for path != "" {
response := struct {
Secrets []Secret
}{}
var err error
path, err = apiClient.RESTWithNext(host, "GET", path, nil, &response)
if err != nil {
return nil, err
}
results = append(results, payload.Secrets...)
if nextURL == "" {
break
}
url = nextURL
results = append(results, response.Secrets...)
}
return results, nil
}
func apiGet(client httpClient, url string, data interface{}) (string, error) {
req, err := http.NewRequest("GET", url, nil)
if err != nil {
return "", err
}
req.Header.Set("Content-Type", "application/json; charset=utf-8")
resp, err := client.Do(req)
if err != nil {
return "", err
}
defer resp.Body.Close()
if resp.StatusCode > 299 {
return "", api.HandleHTTPError(resp)
}
dec := json.NewDecoder(resp.Body)
if err := dec.Decode(data); err != nil {
return "", err
}
return findNextPage(resp.Header.Get("Link")), nil
}
var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`)
func findNextPage(link string) string {
for _, m := range linkRE.FindAllStringSubmatch(link, -1) {
if len(m) > 2 && m[2] == "next" {
return m[1]
}
}
return ""
}
func getSelectedRepositoryInformation(client httpClient, secrets []*Secret) error {
type responseData struct {
TotalCount int `json:"total_count"`
}
for _, secret := range secrets {
func populateSelectedRepositoryInformation(client *http.Client, host string, secrets []Secret) error {
apiClient := api.NewClientFromHTTP(client)
for i, secret := range secrets {
if secret.SelectedReposURL == "" {
continue
}
var result responseData
if _, err := apiGet(client, secret.SelectedReposURL, &result); err != nil {
response := struct {
TotalCount int `json:"total_count"`
}{}
if err := apiClient.REST(host, "GET", secret.SelectedReposURL, nil, &response); err != nil {
return fmt.Errorf("failed determining selected repositories for %s: %w", secret.Name, err)
}
secret.NumSelectedRepos = result.TotalCount
secrets[i].NumSelectedRepos = response.TotalCount
}
return nil
}

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

@ -3,8 +3,8 @@ package list
import (
"bytes"
"fmt"
"io"
"net/http"
"net/url"
"strings"
"testing"
"time"
@ -15,7 +15,6 @@ import (
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/cli/cli/v2/test"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
)
@ -112,9 +111,10 @@ func Test_listRun(t *testing.T) {
tty: true,
opts: &ListOptions{},
wantOut: []string{
"SECRET_ONE.*Updated 1988-10-11",
"SECRET_TWO.*Updated 2020-12-04",
"SECRET_THREE.*Updated 1975-11-30",
"NAME UPDATED",
"SECRET_ONE about 34 years ago",
"SECRET_TWO about 2 years ago",
"SECRET_THREE about 47 years ago",
},
},
{
@ -122,9 +122,9 @@ func Test_listRun(t *testing.T) {
tty: false,
opts: &ListOptions{},
wantOut: []string{
"SECRET_ONE\t1988-10-11",
"SECRET_TWO\t2020-12-04",
"SECRET_THREE\t1975-11-30",
"SECRET_ONE\t1988-10-11T00:00:00Z",
"SECRET_TWO\t2020-12-04T00:00:00Z",
"SECRET_THREE\t1975-11-30T00:00:00Z",
},
},
{
@ -134,9 +134,10 @@ func Test_listRun(t *testing.T) {
OrgName: "UmbrellaCorporation",
},
wantOut: []string{
"SECRET_ONE.*Updated 1988-10-11.*Visible to all repositories",
"SECRET_TWO.*Updated 2020-12-04.*Visible to private repositories",
"SECRET_THREE.*Updated 1975-11-30.*Visible to 2 selected repositories",
"NAME UPDATED VISIBILITY",
"SECRET_ONE about 34 years ago Visible to all repositories",
"SECRET_TWO about 2 years ago Visible to private repositories",
"SECRET_THREE about 47 years ago Visible to 2 selected repositories",
},
},
{
@ -146,9 +147,9 @@ func Test_listRun(t *testing.T) {
OrgName: "UmbrellaCorporation",
},
wantOut: []string{
"SECRET_ONE\t1988-10-11\tALL",
"SECRET_TWO\t2020-12-04\tPRIVATE",
"SECRET_THREE\t1975-11-30\tSELECTED",
"SECRET_ONE\t1988-10-11T00:00:00Z\tALL",
"SECRET_TWO\t2020-12-04T00:00:00Z\tPRIVATE",
"SECRET_THREE\t1975-11-30T00:00:00Z\tSELECTED",
},
},
{
@ -158,9 +159,10 @@ func Test_listRun(t *testing.T) {
EnvName: "Development",
},
wantOut: []string{
"SECRET_ONE.*Updated 1988-10-11",
"SECRET_TWO.*Updated 2020-12-04",
"SECRET_THREE.*Updated 1975-11-30",
"NAME UPDATED",
"SECRET_ONE about 34 years ago",
"SECRET_TWO about 2 years ago",
"SECRET_THREE about 47 years ago",
},
},
{
@ -170,9 +172,9 @@ func Test_listRun(t *testing.T) {
EnvName: "Development",
},
wantOut: []string{
"SECRET_ONE\t1988-10-11",
"SECRET_TWO\t2020-12-04",
"SECRET_THREE\t1975-11-30",
"SECRET_ONE\t1988-10-11T00:00:00Z",
"SECRET_TWO\t2020-12-04T00:00:00Z",
"SECRET_THREE\t1975-11-30T00:00:00Z",
},
},
{
@ -182,9 +184,10 @@ func Test_listRun(t *testing.T) {
UserSecrets: true,
},
wantOut: []string{
"SECRET_ONE.*Updated 1988-10-11.*Visible to 1 selected repository",
"SECRET_TWO.*Updated 2020-12-04.*Visible to 2 selected repositories",
"SECRET_THREE.*Updated 1975-11-30.*Visible to 3 selected repositories",
"NAME UPDATED VISIBILITY",
"SECRET_ONE about 34 years ago Visible to 1 selected repository",
"SECRET_TWO about 2 years ago Visible to 2 selected repositories",
"SECRET_THREE about 47 years ago Visible to 3 selected repositories",
},
},
{
@ -194,9 +197,9 @@ func Test_listRun(t *testing.T) {
UserSecrets: true,
},
wantOut: []string{
"SECRET_ONE\t1988-10-11\t",
"SECRET_TWO\t2020-12-04\t",
"SECRET_THREE\t1975-11-30\t",
"SECRET_ONE\t1988-10-11T00:00:00Z\tSELECTED",
"SECRET_TWO\t2020-12-04T00:00:00Z\tSELECTED",
"SECRET_THREE\t1975-11-30T00:00:00Z\tSELECTED",
},
},
{
@ -206,9 +209,10 @@ func Test_listRun(t *testing.T) {
Application: "Dependabot",
},
wantOut: []string{
"SECRET_ONE.*Updated 1988-10-11",
"SECRET_TWO.*Updated 2020-12-04",
"SECRET_THREE.*Updated 1975-11-30",
"NAME UPDATED",
"SECRET_ONE about 34 years ago",
"SECRET_TWO about 2 years ago",
"SECRET_THREE about 47 years ago",
},
},
{
@ -218,9 +222,9 @@ func Test_listRun(t *testing.T) {
Application: "Dependabot",
},
wantOut: []string{
"SECRET_ONE\t1988-10-11",
"SECRET_TWO\t2020-12-04",
"SECRET_THREE\t1975-11-30",
"SECRET_ONE\t1988-10-11T00:00:00Z",
"SECRET_TWO\t2020-12-04T00:00:00Z",
"SECRET_THREE\t1975-11-30T00:00:00Z",
},
},
{
@ -231,9 +235,10 @@ func Test_listRun(t *testing.T) {
OrgName: "UmbrellaCorporation",
},
wantOut: []string{
"SECRET_ONE.*Updated 1988-10-11.*Visible to all repositories",
"SECRET_TWO.*Updated 2020-12-04.*Visible to private repositories",
"SECRET_THREE.*Updated 1975-11-30.*Visible to 2 selected repositories",
"NAME UPDATED VISIBILITY",
"SECRET_ONE about 34 years ago Visible to all repositories",
"SECRET_TWO about 2 years ago Visible to private repositories",
"SECRET_THREE about 47 years ago Visible to 2 selected repositories",
},
},
{
@ -244,9 +249,9 @@ func Test_listRun(t *testing.T) {
OrgName: "UmbrellaCorporation",
},
wantOut: []string{
"SECRET_ONE\t1988-10-11\tALL",
"SECRET_TWO\t2020-12-04\tPRIVATE",
"SECRET_THREE\t1975-11-30\tSELECTED",
"SECRET_ONE\t1988-10-11T00:00:00Z\tALL",
"SECRET_TWO\t2020-12-04T00:00:00Z\tPRIVATE",
"SECRET_THREE\t1975-11-30T00:00:00Z\tSELECTED",
},
},
}
@ -254,32 +259,38 @@ func Test_listRun(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
reg.Verify(t)
path := "repos/owner/repo/actions/secrets"
if tt.opts.EnvName != "" {
path = fmt.Sprintf("repos/owner/repo/environments/%s/secrets", tt.opts.EnvName)
} else if tt.opts.OrgName != "" {
path = fmt.Sprintf("orgs/%s/actions/secrets", tt.opts.OrgName)
}
t0, _ := time.Parse("2006-01-02", "1988-10-11")
t1, _ := time.Parse("2006-01-02", "2020-12-04")
t2, _ := time.Parse("2006-01-02", "1975-11-30")
payload := secretsPayload{}
payload.Secrets = []*Secret{
{
Name: "SECRET_ONE",
UpdatedAt: t0,
},
{
Name: "SECRET_TWO",
UpdatedAt: t1,
},
{
Name: "SECRET_THREE",
UpdatedAt: t2,
payload := struct {
Secrets []Secret
}{
Secrets: []Secret{
{
Name: "SECRET_ONE",
UpdatedAt: t0,
},
{
Name: "SECRET_TWO",
UpdatedAt: t1,
},
{
Name: "SECRET_THREE",
UpdatedAt: t2,
},
},
}
if tt.opts.OrgName != "" {
payload.Secrets = []*Secret{
payload.Secrets = []Secret{
{
Name: "SECRET_ONE",
UpdatedAt: t0,
@ -297,7 +308,6 @@ func Test_listRun(t *testing.T) {
SelectedReposURL: fmt.Sprintf("https://api.github.com/orgs/%s/actions/secrets/SECRET_THREE/repositories", tt.opts.OrgName),
},
}
path = fmt.Sprintf("orgs/%s/actions/secrets", tt.opts.OrgName)
if tt.tty {
reg.Register(
@ -309,7 +319,7 @@ func Test_listRun(t *testing.T) {
}
if tt.opts.UserSecrets {
payload.Secrets = []*Secret{
payload.Secrets = []Secret{
{
Name: "SECRET_ONE",
UpdatedAt: t0,
@ -365,43 +375,36 @@ func Test_listRun(t *testing.T) {
tt.opts.Config = func() (config.Config, error) {
return config.NewBlankConfig(), nil
}
tt.opts.Now = func() time.Time {
t, _ := time.Parse(time.RFC822, "15 Mar 23 00:00 UTC")
return t
}
err := listRun(tt.opts)
assert.NoError(t, err)
reg.Verify(t)
//nolint:staticcheck // prefer exact matchers over ExpectLines
test.ExpectLines(t, stdout.String(), tt.wantOut...)
expected := fmt.Sprintf("%s\n", strings.Join(tt.wantOut, "\n"))
assert.Equal(t, expected, stdout.String())
})
}
}
func Test_getSecrets_pagination(t *testing.T) {
var requests []*http.Request
var client testClient = func(req *http.Request) (*http.Response, error) {
header := make(map[string][]string)
if len(requests) == 0 {
header["Link"] = []string{`<http://example.com/page/0>; rel="previous", <http://example.com/page/2>; rel="next"`}
}
requests = append(requests, req)
return &http.Response{
Request: req,
Body: io.NopCloser(strings.NewReader(`{"secrets":[{},{}]}`)),
Header: header,
}, nil
}
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.QueryMatcher("GET", "path/to", url.Values{"per_page": []string{"100"}}),
httpmock.WithHeader(
httpmock.StringResponse(`{"secrets":[{},{}]}`),
"Link",
`<http://example.com/page/0>; rel="previous", <http://example.com/page/2>; rel="next"`),
)
reg.Register(
httpmock.REST("GET", "page/2"),
httpmock.StringResponse(`{"secrets":[{},{}]}`),
)
client := &http.Client{Transport: reg}
secrets, err := getSecrets(client, "github.com", "path/to")
assert.NoError(t, err)
assert.Equal(t, 2, len(requests))
assert.Equal(t, 4, len(secrets))
assert.Equal(t, "https://api.github.com/path/to?per_page=100", requests[0].URL.String())
assert.Equal(t, "http://example.com/page/2", requests[1].URL.String())
}
type testClient func(*http.Request) (*http.Response, error)
func (c testClient) Do(req *http.Request) (*http.Response, error) {
return c(req)
}

1
pkg/cmd/variable/delete/delete.go
View file

@ -23,7 +23,6 @@ type DeleteOptions struct {
VariableName string
OrgName string
EnvName string
Application string
}
func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Command {

98
pkg/cmd/variable/delete/delete_test.go
View file

@ -84,120 +84,62 @@ func TestNewCmdDelete(t *testing.T) {
}
}
func Test_removeRun_repo(t *testing.T) {
func TestRemoveRun(t *testing.T) {
tests := []struct {
name string
opts *DeleteOptions
wantPath string
}{
{
name: "Actions",
name: "repo",
opts: &DeleteOptions{
Application: "actions",
VariableName: "cool_variable",
},
wantPath: "repos/owner/repo/actions/variables/cool_variable",
},
}
for _, tt := range tests {
reg := &httpmock.Registry{}
reg.Register(
httpmock.REST("DELETE", tt.wantPath),
httpmock.StatusStringResponse(204, "No Content"))
ios, _, _, _ := iostreams.Test()
tt.opts.IO = ios
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
tt.opts.Config = func() (config.Config, error) {
return config.NewBlankConfig(), nil
}
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
}
err := removeRun(tt.opts)
assert.NoError(t, err)
reg.Verify(t)
}
}
func Test_removeRun_env(t *testing.T) {
reg := &httpmock.Registry{}
reg.Register(
httpmock.REST("DELETE", "repositories/owner/repo/environments/development/variables/cool_variable"),
httpmock.StatusStringResponse(204, "No Content"))
ios, _, _, _ := iostreams.Test()
opts := &DeleteOptions{
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
{
name: "env",
opts: &DeleteOptions{
VariableName: "cool_variable",
EnvName: "development",
},
wantPath: "repositories/owner/repo/environments/development/variables/cool_variable",
},
Config: func() (config.Config, error) {
return config.NewBlankConfig(), nil
},
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
},
VariableName: "cool_variable",
EnvName: "development",
}
err := removeRun(opts)
assert.NoError(t, err)
reg.Verify(t)
}
func Test_removeRun_org(t *testing.T) {
tests := []struct {
name string
opts *DeleteOptions
wantPath string
}{
{
name: "org",
opts: &DeleteOptions{
OrgName: "UmbrellaCorporation",
VariableName: "cool_variable",
OrgName: "UmbrellaCorporation",
},
wantPath: "orgs/UmbrellaCorporation/actions/variables/tVirus",
wantPath: "orgs/UmbrellaCorporation/actions/variables/cool_variable",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.REST("DELETE", tt.wantPath),
reg.Register(httpmock.REST("DELETE", tt.wantPath),
httpmock.StatusStringResponse(204, "No Content"))
ios, _, _, _ := iostreams.Test()
tt.opts.IO = ios
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
tt.opts.Config = func() (config.Config, error) {
return config.NewBlankConfig(), nil
}
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
}
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
tt.opts.IO = ios
tt.opts.VariableName = "tVirus"
err := removeRun(tt.opts)
assert.NoError(t, err)
reg.Verify(t)
})
}
}

142
pkg/cmd/variable/list/list.go
View file

@ -1,17 +1,14 @@
package list
import (
"encoding/json"
"fmt"
"net/http"
"regexp"
"strings"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/tableprinter"
"github.com/cli/cli/v2/pkg/cmd/variable/shared"
@ -25,10 +22,10 @@ type ListOptions struct {
IO *iostreams.IOStreams
Config func() (config.Config, error)
BaseRepo func() (ghrepo.Interface, error)
Now func() time.Time
OrgName string
EnvName string
Application string
OrgName string
EnvName string
}
func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command {
@ -36,6 +33,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
IO: f.IOStreams,
Config: f.Config,
HttpClient: f.HttpClient,
Now: time.Now,
}
cmd := &cobra.Command{
@ -93,7 +91,7 @@ func listRun(opts *ListOptions) error {
return err
}
var variables []*Variable
var variables []Variable
showSelectedRepoInfo := opts.IO.IsStdoutTTY()
switch variableEntity {
@ -104,14 +102,11 @@ func listRun(opts *ListOptions) error {
case shared.Organization:
var cfg config.Config
var host string
cfg, err = opts.Config()
if err != nil {
return err
}
host, _ = cfg.Authentication().DefaultHost()
variables, err = getOrgVariables(client, host, orgName, showSelectedRepoInfo)
}
@ -130,24 +125,20 @@ func listRun(opts *ListOptions) error {
}
table := tableprinter.New(opts.IO)
if opts.OrgName != "" {
table.HeaderRow("NAME", "VALUE", "UPDATED AT", "VISIBILITY")
if variableEntity == shared.Organization {
table.HeaderRow("Name", "Value", "Updated", "Visibility")
} else {
table.HeaderRow("NAME", "VALUE", "UPDATED AT")
table.HeaderRow("Name", "Value", "Updated")
}
for _, variable := range variables {
table.AddField(variable.Name)
table.AddField(variable.Value)
updatedAt := variable.UpdatedAt.Format("2006-01-02")
if opts.IO.IsStdoutTTY() {
updatedAt = fmt.Sprintf("Updated %s", updatedAt)
}
table.AddField(updatedAt)
table.AddTimeField(opts.Now(), variable.UpdatedAt, nil)
if variable.Visibility != "" {
if showSelectedRepoInfo {
table.AddField(fmtVisibility(*variable))
table.AddField(fmtVisibility(variable))
} else {
table.AddField(strings.ToUpper(string(variable.Visibility)), nil, nil)
table.AddField(strings.ToUpper(string(variable.Visibility)))
}
}
table.EndRow()
@ -186,14 +177,22 @@ func fmtVisibility(s Variable) string {
return ""
}
func getOrgVariables(client httpClient, host, orgName string, showSelectedRepoInfo bool) ([]*Variable, error) {
func getRepoVariables(client *http.Client, repo ghrepo.Interface) ([]Variable, error) {
return getVariables(client, repo.RepoHost(), fmt.Sprintf("repos/%s/actions/variables", ghrepo.FullName(repo)))
}
func getEnvVariables(client *http.Client, repo ghrepo.Interface, envName string) ([]Variable, error) {
path := fmt.Sprintf("repositories/%s/environments/%s/variables", ghrepo.FullName(repo), envName)
return getVariables(client, repo.RepoHost(), path)
}
func getOrgVariables(client *http.Client, host, orgName string, showSelectedRepoInfo bool) ([]Variable, error) {
variables, err := getVariables(client, host, fmt.Sprintf("orgs/%s/actions/variables", orgName))
if err != nil {
return nil, err
}
if showSelectedRepoInfo {
err = getSelectedRepositoryInformation(client, variables)
err = populateSelectedRepositoryInformation(client, host, variables)
if err != nil {
return nil, err
}
@ -201,96 +200,37 @@ func getOrgVariables(client httpClient, host, orgName string, showSelectedRepoIn
return variables, nil
}
func getEnvVariables(client httpClient, repo ghrepo.Interface, envName string) ([]*Variable, error) {
path := fmt.Sprintf("repositories/%s/environments/%s/variables", ghrepo.FullName(repo), envName)
return getVariables(client, repo.RepoHost(), path)
}
func getRepoVariables(client httpClient, repo ghrepo.Interface) ([]*Variable, error) {
return getVariables(client, repo.RepoHost(), fmt.Sprintf("repos/%s/actions/variables",
ghrepo.FullName(repo)))
}
type variablesPayload struct {
Variables []*Variable
}
type httpClient interface {
Do(*http.Request) (*http.Response, error)
}
func getVariables(client httpClient, host, path string) ([]*Variable, error) {
var results []*Variable
url := fmt.Sprintf("%s%s?per_page=100", ghinstance.RESTPrefix(host), path)
for {
var payload variablesPayload
nextURL, err := apiGet(client, url, &payload)
func getVariables(client *http.Client, host, path string) ([]Variable, error) {
var results []Variable
apiClient := api.NewClientFromHTTP(client)
path = fmt.Sprintf("%s?per_page=100", path)
for path != "" {
response := struct {
Variables []Variable
}{}
var err error
path, err = apiClient.RESTWithNext(host, "GET", path, nil, &response)
if err != nil {
return nil, err
}
results = append(results, payload.Variables...)
if nextURL == "" {
break
}
url = nextURL
results = append(results, response.Variables...)
}
return results, nil
}
func apiGet(client httpClient, url string, data interface{}) (string, error) {
req, err := http.NewRequest("GET", url, nil)
if err != nil {
return "", err
}
req.Header.Set("Content-Type", "application/json; charset=utf-8")
resp, err := client.Do(req)
if err != nil {
return "", err
}
defer resp.Body.Close()
if resp.StatusCode > 299 {
return "", api.HandleHTTPError(resp)
}
dec := json.NewDecoder(resp.Body)
if err := dec.Decode(data); err != nil {
return "", err
}
return findNextPage(resp.Header.Get("Link")), nil
}
var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`)
func findNextPage(link string) string {
for _, m := range linkRE.FindAllStringSubmatch(link, -1) {
if len(m) > 2 && m[2] == "next" {
return m[1]
}
}
return ""
}
func getSelectedRepositoryInformation(client httpClient, variables []*Variable) error {
type responseData struct {
TotalCount int `json:"total_count"`
}
for _, variable := range variables {
func populateSelectedRepositoryInformation(client *http.Client, host string, variables []Variable) error {
apiClient := api.NewClientFromHTTP(client)
for i, variable := range variables {
if variable.SelectedReposURL == "" {
continue
}
var result responseData
if _, err := apiGet(client, variable.SelectedReposURL, &result); err != nil {
response := struct {
TotalCount int `json:"total_count"`
}{}
if err := apiClient.REST(host, "GET", variable.SelectedReposURL, nil, &response); err != nil {
return fmt.Errorf("failed determining selected repositories for %s: %w", variable.Name, err)
}
variable.NumSelectedRepos = result.TotalCount
variables[i].NumSelectedRepos = response.TotalCount
}
return nil
}

136
pkg/cmd/variable/list/list_test.go
View file

@ -3,8 +3,8 @@ package list
import (
"bytes"
"fmt"
"io"
"net/http"
"net/url"
"strings"
"testing"
"time"
@ -19,7 +19,7 @@ import (
"github.com/stretchr/testify/assert"
)
func Test_NewCmdList(t *testing.T) {
func TestNewCmdList(t *testing.T) {
tests := []struct {
name string
cli string
@ -89,10 +89,10 @@ func Test_listRun(t *testing.T) {
tty: true,
opts: &ListOptions{},
wantOut: []string{
"NAME VALUE UPDATED AT",
"VARIABLE_ONE one Updated 1988-10-11",
"VARIABLE_TWO two Updated 2020-12-04",
"VARIABLE_THREE three Updated 1975-11-30",
"NAME VALUE UPDATED",
"VARIABLE_ONE one about 34 years ago",
"VARIABLE_TWO two about 2 years ago",
"VARIABLE_THREE three about 47 years ago",
},
},
{
@ -100,9 +100,9 @@ func Test_listRun(t *testing.T) {
tty: false,
opts: &ListOptions{},
wantOut: []string{
"VARIABLE_ONE\tone\t1988-10-11",
"VARIABLE_TWO\ttwo\t2020-12-04",
"VARIABLE_THREE\tthree\t1975-11-30",
"VARIABLE_ONE\tone\t1988-10-11T00:00:00Z",
"VARIABLE_TWO\ttwo\t2020-12-04T00:00:00Z",
"VARIABLE_THREE\tthree\t1975-11-30T00:00:00Z",
},
},
{
@ -112,10 +112,10 @@ func Test_listRun(t *testing.T) {
OrgName: "UmbrellaCorporation",
},
wantOut: []string{
"NAME VALUE UPDATED AT VISIBILITY",
"VARIABLE_ONE org_one Updated 1988-10-11 Visible to all repositories",
"VARIABLE_TWO org_two Updated 2020-12-04 Visible to private repositories",
"VARIABLE_THREE org_three Updated 1975-11-30 Visible to 2 selected reposito...",
"NAME VALUE UPDATED VISIBILITY",
"VARIABLE_ONE org_one about 34 years ago Visible to all repositories",
"VARIABLE_TWO org_two about 2 years ago Visible to private repositories",
"VARIABLE_THREE org_three about 47 years ago Visible to 2 selected reposito...",
},
},
{
@ -125,9 +125,9 @@ func Test_listRun(t *testing.T) {
OrgName: "UmbrellaCorporation",
},
wantOut: []string{
"VARIABLE_ONE\torg_one\t1988-10-11\tALL",
"VARIABLE_TWO\torg_two\t2020-12-04\tPRIVATE",
"VARIABLE_THREE\torg_three\t1975-11-30\tSELECTED",
"VARIABLE_ONE\torg_one\t1988-10-11T00:00:00Z\tALL",
"VARIABLE_TWO\torg_two\t2020-12-04T00:00:00Z\tPRIVATE",
"VARIABLE_THREE\torg_three\t1975-11-30T00:00:00Z\tSELECTED",
},
},
{
@ -137,10 +137,10 @@ func Test_listRun(t *testing.T) {
EnvName: "Development",
},
wantOut: []string{
"NAME VALUE UPDATED AT",
"VARIABLE_ONE one Updated 1988-10-11",
"VARIABLE_TWO two Updated 2020-12-04",
"VARIABLE_THREE three Updated 1975-11-30",
"NAME VALUE UPDATED",
"VARIABLE_ONE one about 34 years ago",
"VARIABLE_TWO two about 2 years ago",
"VARIABLE_THREE three about 47 years ago",
},
},
{
@ -150,9 +150,9 @@ func Test_listRun(t *testing.T) {
EnvName: "Development",
},
wantOut: []string{
"VARIABLE_ONE\tone\t1988-10-11",
"VARIABLE_TWO\ttwo\t2020-12-04",
"VARIABLE_THREE\tthree\t1975-11-30",
"VARIABLE_ONE\tone\t1988-10-11T00:00:00Z",
"VARIABLE_TWO\ttwo\t2020-12-04T00:00:00Z",
"VARIABLE_THREE\tthree\t1975-11-30T00:00:00Z",
},
},
}
@ -160,35 +160,41 @@ func Test_listRun(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
path := "repos/owner/repo/actions/variables"
if tt.opts.EnvName != "" {
path = fmt.Sprintf("repositories/owner/repo/environments/%s/variables", tt.opts.EnvName)
} else if tt.opts.OrgName != "" {
path = fmt.Sprintf("orgs/%s/actions/variables", tt.opts.OrgName)
}
t0, _ := time.Parse("2006-01-02", "1988-10-11")
t1, _ := time.Parse("2006-01-02", "2020-12-04")
t2, _ := time.Parse("2006-01-02", "1975-11-30")
payload := variablesPayload{}
payload.Variables = []*Variable{
{
Name: "VARIABLE_ONE",
Value: "one",
UpdatedAt: t0,
},
{
Name: "VARIABLE_TWO",
Value: "two",
UpdatedAt: t1,
},
{
Name: "VARIABLE_THREE",
Value: "three",
UpdatedAt: t2,
payload := struct {
Variables []Variable
}{
Variables: []Variable{
{
Name: "VARIABLE_ONE",
Value: "one",
UpdatedAt: t0,
},
{
Name: "VARIABLE_TWO",
Value: "two",
UpdatedAt: t1,
},
{
Name: "VARIABLE_THREE",
Value: "three",
UpdatedAt: t2,
},
},
}
if tt.opts.OrgName != "" {
payload.Variables = []*Variable{
payload.Variables = []Variable{
{
Name: "VARIABLE_ONE",
Value: "org_one",
@ -209,8 +215,6 @@ func Test_listRun(t *testing.T) {
SelectedReposURL: fmt.Sprintf("https://api.github.com/orgs/%s/actions/variables/VARIABLE_THREE/repositories", tt.opts.OrgName),
},
}
path = fmt.Sprintf("orgs/%s/actions/variables", tt.opts.OrgName)
if tt.tty {
reg.Register(
httpmock.REST("GET", fmt.Sprintf("orgs/%s/actions/variables/VARIABLE_THREE/repositories", tt.opts.OrgName)),
@ -236,44 +240,36 @@ func Test_listRun(t *testing.T) {
tt.opts.Config = func() (config.Config, error) {
return config.NewBlankConfig(), nil
}
tt.opts.Now = func() time.Time {
t, _ := time.Parse(time.RFC822, "15 Mar 23 00:00 UTC")
return t
}
err := listRun(tt.opts)
assert.NoError(t, err)
reg.Verify(t)
outputLines := strings.Split(stdout.String(), "\n")
for i, l := range tt.wantOut {
assert.Equal(t, outputLines[i], l)
}
expected := fmt.Sprintf("%s\n", strings.Join(tt.wantOut, "\n"))
assert.Equal(t, expected, stdout.String())
})
}
}
func Test_getVariables_pagination(t *testing.T) {
var requests []*http.Request
var client testClient = func(req *http.Request) (*http.Response, error) {
header := make(map[string][]string)
if len(requests) == 0 {
header["Link"] = []string{`<http://example.com/page/0>; rel="previous", <http://example.com/page/2>; rel="next"`}
}
requests = append(requests, req)
return &http.Response{
Request: req,
Body: io.NopCloser(strings.NewReader(`{"variables":[{},{}]}`)),
Header: header,
}, nil
}
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.QueryMatcher("GET", "path/to", url.Values{"per_page": []string{"100"}}),
httpmock.WithHeader(
httpmock.StringResponse(`{"variables":[{},{}]}`),
"Link",
`<http://example.com/page/0>; rel="previous", <http://example.com/page/2>; rel="next"`),
)
reg.Register(
httpmock.REST("GET", "page/2"),
httpmock.StringResponse(`{"variables":[{},{}]}`),
)
client := &http.Client{Transport: reg}
variables, err := getVariables(client, "github.com", "path/to")
assert.NoError(t, err)
assert.Equal(t, 2, len(requests))
assert.Equal(t, 4, len(variables))
assert.Equal(t, "https://api.github.com/path/to?per_page=100", requests[0].URL.String())
assert.Equal(t, "http://example.com/page/2", requests[1].URL.String())
}
type testClient func(*http.Request) (*http.Response, error)
func (c testClient) Do(req *http.Request) (*http.Response, error) {
return c(req)
}

4
pkg/cmd/variable/set/set.go
View file

@ -50,7 +50,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
cmd := &cobra.Command{
Use: "set <variable-name>",
Short: "Set variables",
Short: "Create or update variables",
Long: heredoc.Doc(`
Set a value for a variable on one of the following levels:
- repository (default): available to Actions runs or Dependabot in a repository
@ -219,7 +219,7 @@ func setRun(opts *SetOptions) error {
if envName != "" {
target += " environment " + envName
}
fmt.Fprintf(opts.IO.Out, "%s %s actions variable %s for %s\n", cs.SuccessIcon(), result.Operation, result.Key, target)
fmt.Fprintf(opts.IO.Out, "%s %s Actions variable %s for %s\n", cs.SuccessIcon(), result.Operation, result.Key, target)
}
return err

7
pkg/cmd/variable/variable.go
View file

@ -14,9 +14,9 @@ func NewCmdVariable(f *cmdutil.Factory) *cobra.Command {
Use: "variable <command>",
Short: "Manage GitHub Actions variables",
Long: heredoc.Doc(`
Variables can be set at the repository, environment or organization level for use in GitHub Actions.
Run "gh help variable set" to learn how to get started.
`),
Variables can be set at the repository, environment or organization level for use in
GitHub Actions or Dependabot. Run "gh help variable set" to learn how to get started.
`),
}
cmdutil.EnableRepoOverride(cmd, f)
@ -24,5 +24,6 @@ func NewCmdVariable(f *cmdutil.Factory) *cobra.Command {
cmd.AddCommand(cmdSet.NewCmdSet(f, nil))
cmd.AddCommand(cmdList.NewCmdList(f, nil))
cmd.AddCommand(cmdDelete.NewCmdDelete(f, nil))
return cmd
}