add removing secrets

pkg/cmd/secret/remove/remove.go

@@ -0,0 +1,93 @@
+package remove
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/MakeNowJust/heredoc"
+	"github.com/cli/cli/api"
+	"github.com/cli/cli/internal/ghinstance"
+	"github.com/cli/cli/internal/ghrepo"
+	"github.com/cli/cli/pkg/cmdutil"
+	"github.com/cli/cli/pkg/iostreams"
+	"github.com/spf13/cobra"
+)
+
+type RemoveOptions struct {
+	HttpClient func() (*http.Client, error)
+	IO         *iostreams.IOStreams
+	BaseRepo   func() (ghrepo.Interface, error)
+
+	SecretName string
+	OrgName    string
+}
+
+func NewCmdRemove(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Command {
+	opts := &RemoveOptions{
+		IO:         f.IOStreams,
+		HttpClient: f.HttpClient,
+	}
+
+	cmd := &cobra.Command{
+		Use:   "remove <secret name>",
+		Short: "Remove an organization or repository secret",
+		Example: heredoc.Doc(`
+			$ gh secret remove REPO_SECRET
+			$ gh secret remove --org ORG_SECRET
+			$ gh secret remove --org="anotherOrg" ORG_SECRET
+		`),
+		Args: cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			// support `-R, --repo` override
+			opts.BaseRepo = f.BaseRepo
+
+			opts.SecretName = args[0]
+
+			if runF != nil {
+				return runF(opts)
+			}
+
+			return removeRun(opts)
+		},
+	}
+	cmd.Flags().StringVar(&opts.OrgName, "org", "", "List secrets for an organization")
+	cmd.Flags().Lookup("org").NoOptDefVal = "@owner"
+
+	return cmd
+}
+
+func removeRun(opts *RemoveOptions) error {
+	c, err := opts.HttpClient()
+	if err != nil {
+		return fmt.Errorf("could not create http client: %w", err)
+	}
+	client := api.NewClientFromHTTP(c)
+
+	var baseRepo ghrepo.Interface
+	if opts.OrgName == "" || opts.OrgName == "@owner" {
+		baseRepo, err = opts.BaseRepo()
+		if err != nil {
+			return fmt.Errorf("could not determine base repo: %w", err)
+		}
+	}
+
+	host := ghinstance.OverridableDefault()
+	if opts.OrgName == "@owner" {
+		opts.OrgName = baseRepo.RepoOwner()
+		host = baseRepo.RepoHost()
+	}
+
+	var path string
+	if opts.OrgName == "" {
+		path = fmt.Sprintf("repos/%s/actions/secrets/%s", ghrepo.FullName(baseRepo), opts.SecretName)
+	} else {
+		path = fmt.Sprintf("orgs/%s/actions/secrets/%s", opts.OrgName, opts.SecretName)
+	}
+
+	err = client.REST(host, "DELETE", path, nil, nil)
+	if err != nil {
+		return fmt.Errorf("failed to delete secret %s: %w", opts.SecretName, err)
+	}
+
+	return nil
+}

pkg/cmd/secret/remove/remove_test.go

@@ -0,0 +1,159 @@
+package remove
+
+import (
+	"bytes"
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/cli/cli/internal/ghrepo"
+	"github.com/cli/cli/pkg/cmdutil"
+	"github.com/cli/cli/pkg/httpmock"
+	"github.com/cli/cli/pkg/iostreams"
+	"github.com/google/shlex"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewCmdRemove(t *testing.T) {
+	tests := []struct {
+		name     string
+		cli      string
+		wants    RemoveOptions
+		wantsErr bool
+	}{
+		{
+			name:     "no args",
+			wantsErr: true,
+		},
+		{
+			name: "implicit org",
+			cli:  "cool --org",
+			wants: RemoveOptions{
+				SecretName: "cool",
+				OrgName:    "@owner",
+			},
+		},
+		{
+			name: "explicit org",
+			cli:  "cool --org=anOrg",
+			wants: RemoveOptions{
+				SecretName: "cool",
+				OrgName:    "anOrg",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			io, _, _, _ := iostreams.Test()
+			f := &cmdutil.Factory{
+				IOStreams: io,
+			}
+
+			argv, err := shlex.Split(tt.cli)
+			assert.NoError(t, err)
+
+			var gotOpts *RemoveOptions
+			cmd := NewCmdRemove(f, func(opts *RemoveOptions) error {
+				gotOpts = opts
+				return nil
+			})
+			cmd.SetArgs(argv)
+			cmd.SetIn(&bytes.Buffer{})
+			cmd.SetOut(&bytes.Buffer{})
+			cmd.SetErr(&bytes.Buffer{})
+
+			_, err = cmd.ExecuteC()
+			if tt.wantsErr {
+				assert.Error(t, err)
+				return
+			}
+			assert.NoError(t, err)
+
+			assert.Equal(t, tt.wants.SecretName, gotOpts.SecretName)
+			assert.Equal(t, tt.wants.OrgName, gotOpts.OrgName)
+		})
+	}
+
+}
+
+func Test_removeRun_repo(t *testing.T) {
+	reg := &httpmock.Registry{}
+
+	reg.Register(
+		httpmock.REST("DELETE", "repos/owner/repo/actions/secrets/cool_secret"),
+		httpmock.StatusStringResponse(204, "No Content"))
+
+	io, _, _, _ := iostreams.Test()
+
+	opts := &RemoveOptions{
+		IO: io,
+		HttpClient: func() (*http.Client, error) {
+			return &http.Client{Transport: reg}, nil
+		},
+		BaseRepo: func() (ghrepo.Interface, error) {
+			return ghrepo.FromFullName("owner/repo")
+		},
+		SecretName: "cool_secret",
+	}
+
+	err := removeRun(opts)
+	assert.NoError(t, err)
+
+	reg.Verify(t)
+}
+
+func Test_removeRun_org(t *testing.T) {
+	tests := []struct {
+		name string
+		opts *RemoveOptions
+	}{
+		{
+			name: "implicit org",
+			opts: &RemoveOptions{
+				OrgName: "@owner",
+			},
+		},
+		{
+			name: "explicit org",
+			opts: &RemoveOptions{
+				OrgName: "UmbrellaCorporation",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			reg := &httpmock.Registry{}
+
+			impliedOrgName := "NeoUmbrella"
+
+			orgName := tt.opts.OrgName
+			if orgName == "@owner" {
+				orgName = impliedOrgName
+			}
+
+			reg.Register(
+				httpmock.REST("DELETE", fmt.Sprintf("orgs/%s/actions/secrets/tVirus", orgName)),
+				httpmock.StatusStringResponse(204, "No Content"))
+
+			io, _, _, _ := iostreams.Test()
+
+			tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
+				return ghrepo.FromFullName(fmt.Sprintf("%s/repo", impliedOrgName))
+			}
+			tt.opts.HttpClient = func() (*http.Client, error) {
+				return &http.Client{Transport: reg}, nil
+			}
+			tt.opts.IO = io
+			tt.opts.SecretName = "tVirus"
+
+			err := removeRun(tt.opts)
+			assert.NoError(t, err)
+
+			reg.Verify(t)
+
+		})
+	}
+
+}

pkg/cmd/secret/secret.go

@@ -6,6 +6,7 @@ import (
 	"github.com/spf13/cobra"
 
 	cmdList "github.com/cli/cli/pkg/cmd/secret/list"
+	cmdRemove "github.com/cli/cli/pkg/cmd/secret/remove"
 	cmdSet "github.com/cli/cli/pkg/cmd/secret/set"
 )
 
@@ -22,8 +23,9 @@ func NewCmdSecret(f *cmdutil.Factory) *cobra.Command {
 	cmdutil.EnableRepoOverride(cmd, f)
 
 	cmd.AddCommand(cmdList.NewCmdList(f, nil))
+	// TODO add success messages to these:
 	cmd.AddCommand(cmdSet.NewCmdSet(f, nil))
-	// TODO add delete
+	cmd.AddCommand(cmdRemove.NewCmdRemove(f, nil))
 
 	return cmd
 }

pkg/cmd/secret/set/set_test.go

@@ -172,17 +172,15 @@ func Test_setRun_repo(t *testing.T) {
 
 	reg.Register(httpmock.REST("PUT", "repos/owner/repo/actions/secrets/cool_secret"), httpmock.StatusStringResponse(201, `{}`))
 
-	mockClient := func() (*http.Client, error) {
-		return &http.Client{Transport: reg}, nil
-	}
-
 	io, _, _, _ := iostreams.Test()
 
 	opts := &SetOptions{
+		HttpClient: func() (*http.Client, error) {
+			return &http.Client{Transport: reg}, nil
+		},
 		BaseRepo: func() (ghrepo.Interface, error) {
 			return ghrepo.FromFullName("owner/repo")
 		},
-		HttpClient: mockClient,
 		IO:         io,
 		SecretName: "cool_secret",
 		Body:       "a secret",
@@ -240,9 +238,10 @@ func Test_setRun_org(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			reg := &httpmock.Registry{}
 
+			impliedOrgName := "NeoUmbrella"
 			orgName := tt.opts.OrgName
 			if orgName == "@owner" {
-				orgName = "NeoUmbrella"
+				orgName = impliedOrgName
 			}
 
 			reg.Register(httpmock.REST("GET",
@@ -261,7 +260,7 @@ func Test_setRun_org(t *testing.T) {
 			io, _, _, _ := iostreams.Test()
 
 			tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
-				return ghrepo.FromFullName("NeoUmbrella/repo")
+				return ghrepo.FromFullName(fmt.Sprintf("%s/repo", impliedOrgName))
 			}
 			tt.opts.HttpClient = func() (*http.Client, error) {
 				return &http.Client{Transport: reg}, nil