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

Fix host handling in variable and secret delete

Browse source 
This change updates `gh variable delete` and `gh secret delete` to use the host associated with the repository rather than the default host.  Along with these changes is minor refactoring of the tests to ensure appropriate Dotcom vs GHES targeting works as expected.

Minor edit to `gh variable get` testing to simplify some conditional logic in test setup.
This commit is contained in:
Andy Feller 2024-08-02 15:11:22 -04:00
parent 22a5a4c899
commit c088951944
5 changed files with 176 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

29
pkg/cmd/secret/delete/delete.go
View file

@ -105,24 +105,27 @@ func removeRun(opts *DeleteOptions) error {
}
}
var path string
switch secretEntity {
case shared.Organization:
path = fmt.Sprintf("orgs/%s/%s/secrets/%s", orgName, secretApp, opts.SecretName)
case shared.Environment:
path = fmt.Sprintf("repos/%s/environments/%s/secrets/%s", ghrepo.FullName(baseRepo), envName, opts.SecretName)
case shared.User:
path = fmt.Sprintf("user/codespaces/secrets/%s", opts.SecretName)
case shared.Repository:
path = fmt.Sprintf("repos/%s/%s/secrets/%s", ghrepo.FullName(baseRepo), secretApp, opts.SecretName)
}
cfg, err := opts.Config()
if err != nil {
return err
}
host, _ := cfg.Authentication().DefaultHost()
var path string
var host string
switch secretEntity {
case shared.Organization:
path = fmt.Sprintf("orgs/%s/%s/secrets/%s", orgName, secretApp, opts.SecretName)
host, _ = cfg.Authentication().DefaultHost()
case shared.Environment:
path = fmt.Sprintf("repos/%s/environments/%s/secrets/%s", ghrepo.FullName(baseRepo), envName, opts.SecretName)
host = baseRepo.RepoHost()
case shared.User:
path = fmt.Sprintf("user/codespaces/secrets/%s", opts.SecretName)
host, _ = cfg.Authentication().DefaultHost()
case shared.Repository:
path = fmt.Sprintf("repos/%s/%s/secrets/%s", ghrepo.FullName(baseRepo), secretApp, opts.SecretName)
host = baseRepo.RepoHost()
}
err = client.REST(host, "DELETE", path, nil, nil)
if err != nil {

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

@ -13,6 +13,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdDelete(t *testing.T) {
@ -121,9 +122,10 @@ func TestNewCmdDelete(t *testing.T) {
func Test_removeRun_repo(t *testing.T) {
tests := []struct {
name string
opts *DeleteOptions
wantPath string
name string
opts *DeleteOptions
host string
httpStubs func(*httpmock.Registry)
}{
{
name: "Actions",
@ -131,7 +133,21 @@ func Test_removeRun_repo(t *testing.T) {
Application: "actions",
SecretName: "cool_secret",
},
wantPath: "repos/owner/repo/actions/secrets/cool_secret",
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/actions/secrets/cool_secret"), "api.github.com"), httpmock.StatusStringResponse(204, "No Content"))
},
},
{
name: "Actions GHES",
opts: &DeleteOptions{
Application: "actions",
SecretName: "cool_secret",
},
host: "example.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/actions/secrets/cool_secret"), "example.com"), httpmock.StatusStringResponse(204, "No Content"))
},
},
{
name: "Dependabot",
@ -139,23 +155,38 @@ func Test_removeRun_repo(t *testing.T) {
Application: "dependabot",
SecretName: "cool_dependabot_secret",
},
wantPath: "repos/owner/repo/dependabot/secrets/cool_dependabot_secret",
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/dependabot/secrets/cool_dependabot_secret"), "api.github.com"), httpmock.StatusStringResponse(204, "No Content"))
},
},
{
name: "Dependabot GHES",
opts: &DeleteOptions{
Application: "dependabot",
SecretName: "cool_dependabot_secret",
},
host: "example.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/dependabot/secrets/cool_dependabot_secret"), "example.com"), httpmock.StatusStringResponse(204, "No Content"))
},
},
{
name: "defaults to Actions",
opts: &DeleteOptions{
SecretName: "cool_secret",
},
wantPath: "repos/owner/repo/actions/secrets/cool_secret",
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/actions/secrets/cool_secret"), "api.github.com"), httpmock.StatusStringResponse(204, "No Content"))
},
},
}
for _, tt := range tests {
reg := &httpmock.Registry{}
reg.Register(
httpmock.REST("DELETE", tt.wantPath),
httpmock.StatusStringResponse(204, "No Content"))
tt.httpStubs(reg)
defer reg.Verify(t)
ios, _, _, _ := iostreams.Test()
@ -167,44 +198,70 @@ func Test_removeRun_repo(t *testing.T) {
return config.NewBlankConfig(), nil
}
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
return ghrepo.FromFullNameWithHost("owner/repo", tt.host)
}
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", "repos/owner/repo/environments/development/secrets/cool_secret"),
httpmock.StatusStringResponse(204, "No Content"))
ios, _, _, _ := iostreams.Test()
opts := &DeleteOptions{
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
tests := []struct {
name string
opts *DeleteOptions
host string
httpStubs func(*httpmock.Registry)
}{
{
name: "delete environment secret",
opts: &DeleteOptions{
SecretName: "cool_secret",
EnvName: "development",
},
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/environments/development/secrets/cool_secret"), "api.github.com"),
httpmock.StatusStringResponse(204, "No Content"))
},
},
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
{
name: "delete environment secret GHES",
opts: &DeleteOptions{
SecretName: "cool_secret",
EnvName: "development",
},
host: "example.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/environments/development/secrets/cool_secret"), "example.com"),
httpmock.StatusStringResponse(204, "No Content"))
},
},
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
},
SecretName: "cool_secret",
EnvName: "development",
}
err := removeRun(opts)
assert.NoError(t, err)
for _, tt := range tests {
reg := &httpmock.Registry{}
tt.httpStubs(reg)
defer reg.Verify(t)
reg.Verify(t)
ios, _, _, _ := iostreams.Test()
tt.opts.IO = ios
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
if tt.host != "" {
return ghrepo.FromFullNameWithHost("owner/repo", tt.host)
}
return ghrepo.FromFullName("owner/repo")
}
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
tt.opts.Config = func() (gh.Config, error) {
return config.NewBlankConfig(), nil
}
err := removeRun(tt.opts)
require.NoError(t, err)
}
}
func Test_removeRun_org(t *testing.T) {

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

@ -91,22 +91,24 @@ func removeRun(opts *DeleteOptions) error {
}
}
var path string
switch variableEntity {
case shared.Organization:
path = fmt.Sprintf("orgs/%s/actions/variables/%s", orgName, opts.VariableName)
case shared.Environment:
path = fmt.Sprintf("repos/%s/environments/%s/variables/%s", ghrepo.FullName(baseRepo), envName, opts.VariableName)
case shared.Repository:
path = fmt.Sprintf("repos/%s/actions/variables/%s", ghrepo.FullName(baseRepo), opts.VariableName)
}
cfg, err := opts.Config()
if err != nil {
return err
}
host, _ := cfg.Authentication().DefaultHost()
var path string
var host string
switch variableEntity {
case shared.Organization:
path = fmt.Sprintf("orgs/%s/actions/variables/%s", orgName, opts.VariableName)
host, _ = cfg.Authentication().DefaultHost()
case shared.Environment:
path = fmt.Sprintf("repos/%s/environments/%s/variables/%s", ghrepo.FullName(baseRepo), envName, opts.VariableName)
host = baseRepo.RepoHost()
case shared.Repository:
path = fmt.Sprintf("repos/%s/actions/variables/%s", ghrepo.FullName(baseRepo), opts.VariableName)
host = baseRepo.RepoHost()
}
err = client.REST(host, "DELETE", path, nil, nil)
if err != nil {

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

@ -13,6 +13,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdDelete(t *testing.T) {
@ -87,16 +88,32 @@ func TestNewCmdDelete(t *testing.T) {
func TestRemoveRun(t *testing.T) {
tests := []struct {
name string
opts *DeleteOptions
wantPath string
name string
opts *DeleteOptions
host string
httpStubs func(*httpmock.Registry)
}{
{
name: "repo",
opts: &DeleteOptions{
VariableName: "cool_variable",
},
wantPath: "repos/owner/repo/actions/variables/cool_variable",
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/actions/variables/cool_variable"), "api.github.com"),
httpmock.StatusStringResponse(204, "No Content"))
},
},
{
name: "repo GHES",
opts: &DeleteOptions{
VariableName: "cool_variable",
},
host: "example.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/actions/variables/cool_variable"), "example.com"),
httpmock.StatusStringResponse(204, "No Content"))
},
},
{
name: "env",
@ -104,7 +121,23 @@ func TestRemoveRun(t *testing.T) {
VariableName: "cool_variable",
EnvName: "development",
},
wantPath: "repos/owner/repo/environments/development/variables/cool_variable",
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "repos/owner/repo/environments/development/variables/cool_variable"), "api.github.com"),
httpmock.StatusStringResponse(204, "No Content"))
},
},
{
name: "env GHES",
opts: &DeleteOptions{
VariableName: "cool_variable",
EnvName: "development",
},
host: "example.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "api/v3/repos/owner/repo/environments/development/variables/cool_variable"), "example.com"),
httpmock.StatusStringResponse(204, "No Content"))
},
},
{
name: "org",
@ -112,35 +145,34 @@ func TestRemoveRun(t *testing.T) {
VariableName: "cool_variable",
OrgName: "UmbrellaCorporation",
},
wantPath: "orgs/UmbrellaCorporation/actions/variables/cool_variable",
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("DELETE", "orgs/UmbrellaCorporation/actions/variables/cool_variable"), "api.github.com"),
httpmock.StatusStringResponse(204, "No Content"))
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
tt.httpStubs(reg)
defer reg.Verify(t)
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() (gh.Config, error) {
return config.NewBlankConfig(), nil
}
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("owner/repo")
return ghrepo.FromFullNameWithHost("owner/repo", tt.host)
}
err := removeRun(tt.opts)
assert.NoError(t, err)
require.NoError(t, err)
})
}
}

17
pkg/cmd/variable/get/get_test.go
View file

@ -135,9 +135,9 @@ func Test_getRun(t *testing.T) {
opts: &GetOptions{
VariableName: "VARIABLE_ONE",
},
host: "ghe.io",
host: "example.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/actions/variables/VARIABLE_ONE"), "ghe.io"),
reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/actions/variables/VARIABLE_ONE"), "example.com"),
httpmock.JSONResponse(shared.Variable{
Value: "repo_var",
}))
@ -150,6 +150,7 @@ func Test_getRun(t *testing.T) {
OrgName: "TestOrg",
VariableName: "VARIABLE_ONE",
},
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "orgs/TestOrg/actions/variables/VARIABLE_ONE"),
httpmock.JSONResponse(shared.Variable{
@ -179,9 +180,9 @@ func Test_getRun(t *testing.T) {
EnvName: "Development",
VariableName: "VARIABLE_ONE",
},
host: "ghe.io",
host: "example.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/environments/Development/variables/VARIABLE_ONE"), "ghe.io"),
reg.Register(httpmock.WithHost(httpmock.REST("GET", "api/v3/repos/owner/repo/environments/Development/variables/VARIABLE_ONE"), "example.com"),
httpmock.JSONResponse(shared.Variable{
Value: "env_var",
}))
@ -193,6 +194,7 @@ func Test_getRun(t *testing.T) {
opts: &GetOptions{
VariableName: "VARIABLE_ONE",
},
host: "github.com",
jsonFields: []string{"name", "value", "visibility", "updatedAt", "createdAt", "numSelectedRepos", "selectedReposURL"},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "repos/owner/repo/actions/variables/VARIABLE_ONE"),
@ -225,6 +227,7 @@ func Test_getRun(t *testing.T) {
opts: &GetOptions{
VariableName: "VARIABLE_ONE",
},
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "repos/owner/repo/actions/variables/VARIABLE_ONE"),
httpmock.StatusStringResponse(404, "not found"),
@ -237,6 +240,7 @@ func Test_getRun(t *testing.T) {
opts: &GetOptions{
VariableName: "VARIABLE_ONE",
},
host: "github.com",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "repos/owner/repo/actions/variables/VARIABLE_ONE"),
httpmock.StatusStringResponse(400, "not found"),
@ -258,10 +262,7 @@ func Test_getRun(t *testing.T) {
tt.opts.IO = ios
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
if tt.host != "" {
return ghrepo.FromFullNameWithHost("owner/repo", tt.host)
}
return ghrepo.FromFullName("owner/repo")
return ghrepo.FromFullNameWithHost("owner/repo", tt.host)
}
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil