feat: incorporate feedback from review

* Incorporate feedback from reviews.
* Add extra test cases to increase coverage.
* Tiny refactors for code parity with remove configmap.
* Update copyright notice.

Author: stormqueen1990

kustomize/commands/edit/remove/removesecret.go

@@ -1,4 +1,4 @@
-// Copyright 2019 The Kubernetes Authors.
+// Copyright 2023 The Kubernetes Authors.
 // SPDX-License-Identifier: Apache-2.0
 
 package remove
@@ -12,22 +12,23 @@ import (
 	"github.com/spf13/cobra"
 	"sigs.k8s.io/kustomize/api/konfig"
 	"sigs.k8s.io/kustomize/api/types"
-	"sigs.k8s.io/kustomize/kustomize/v4/commands/internal/kustfile"
+	"sigs.k8s.io/kustomize/kustomize/v5/commands/internal/kustfile"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
 type removeSecretOptions struct {
 	secretNamesToRemove []string
 }
 
-// newCmdRemoveSecret remove the name of a file containing a secret to the kustomization file.
+// newCmdRemoveSecret removes secretGenerator(s) with the specified name(s).
 func newCmdRemoveSecret(fSys filesys.FileSystem) *cobra.Command {
 	var o removeSecretOptions
 
 	cmd := &cobra.Command{
 		Use: "secret",
-		Short: "Removes specified secret" +
+		Short: "Removes the specified secret(s) from " +
 			konfig.DefaultKustomizationFileName(),
+		Long: "",
 		Example: `
 		remove secret my-secret
 		`,
@@ -44,12 +45,13 @@ func newCmdRemoveSecret(fSys filesys.FileSystem) *cobra.Command {
 
 // Validate validates removeSecret command.
 func (o *removeSecretOptions) Validate(args []string) error {
-	if len(args) == 0 {
-		return errors.New("must specify a Secret name")
-	}
-	if len(args) > 1 {
-		return fmt.Errorf("too many arguments: %s; to provide multiple Secrets to remove, please separate Secret names by commas", args)
+	switch {
+	case len(args) == 0:
+		return errors.New("at least one secret name must be specified")
+	case len(args) > 1:
+		return fmt.Errorf("too many arguments: %s; to provide multiple secrets to remove, please separate secret names by commas", args)
 	}
+
 	o.secretNamesToRemove = strings.Split(args[0], ",")
 	return nil
 }
@@ -63,7 +65,7 @@ func (o *removeSecretOptions) RunRemoveSecret(fSys filesys.FileSystem) error {
 
 	m, err := mf.Read()
 	if err != nil {
-		return fmt.Errorf("could not read kustomization file: %w", err)
+		return fmt.Errorf("could not read kustomization file contents: %w", err)
 	}
 
 	foundSecrets := make(map[string]struct{})
@@ -77,6 +79,11 @@ func (o *removeSecretOptions) RunRemoveSecret(fSys filesys.FileSystem) error {
 		newSecrets = append(newSecrets, currentSecret)
 	}
 
+	if len(foundSecrets) == 0 {
+		return fmt.Errorf("no specified secret(s) were found in the %s file",
+			konfig.DefaultKustomizationFileName())
+	}
+
 	for _, name := range o.secretNamesToRemove {
 		if _, found := foundSecrets[name]; !found {
 			log.Printf("secret %s doesn't exist in kustomization file", name)
@@ -86,7 +93,7 @@ func (o *removeSecretOptions) RunRemoveSecret(fSys filesys.FileSystem) error {
 
 	err = mf.Write(m)
 	if err != nil {
-		return fmt.Errorf("secret cannot write back to file, got %w", err)
+		return fmt.Errorf("failed to write kustomization file: %w", err)
 	}
 	return nil
 }

kustomize/commands/edit/remove/removesecret_test.go

@@ -1,15 +1,14 @@
-// Copyright 2019 The Kubernetes Authors.
+// Copyright 2023 The Kubernetes Authors.
 // SPDX-License-Identifier: Apache-2.0
 
 package remove //nolint:testpackage
 
 import (
 	"fmt"
-	"strings"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-	testutils_test "sigs.k8s.io/kustomize/kustomize/v4/commands/internal/testutils"
+	"github.com/stretchr/testify/require"
+	testutils_test "sigs.k8s.io/kustomize/kustomize/v5/commands/internal/testutils"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
@@ -18,9 +17,10 @@ func TestRemoveSecret(t *testing.T) {
 	const secretName02 = "example-secret-02"
 
 	tests := map[string]struct {
-		input       string
-		args        []string
-		expectedErr string
+		input          string
+		args           []string
+		expectedOutput string
+		expectedErr    string
 	}{
 		"happy path": {
 			input: fmt.Sprintf(`
@@ -32,6 +32,10 @@ secretGenerator:
   - longsecret.txt
 `, secretName01),
 			args: []string{secretName01},
+			expectedOutput: `
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+`,
 		},
 		"multiple": {
 			input: fmt.Sprintf(`
@@ -48,6 +52,10 @@ secretGenerator:
 			args: []string{
 				fmt.Sprintf("%s,%s", secretName01, secretName02),
 			},
+			expectedOutput: `
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+`,
 		},
 		"miss": {
 			input: fmt.Sprintf(`
@@ -58,7 +66,31 @@ secretGenerator:
   files:
   - longsecret.txt
 `, secretName01),
-			args: []string{"foo"},
+			args:        []string{"foo"},
+			expectedErr: "no specified secret(s) were found",
+		},
+		"no secret name specified": {
+			args:        []string{},
+			expectedErr: "at least one secret name must be specified",
+		},
+		"too many secret names specified": {
+			args:        []string{"test1", "test2"},
+			expectedErr: "too many arguments",
+		},
+		"one existing and one non-existing": {
+			input: fmt.Sprintf(`
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+secretGenerator:
+- name: %s
+  files:
+  - application.properties
+`, secretName01),
+			args: []string{fmt.Sprintf("%s,%s", secretName01, "foo")},
+			expectedOutput: `
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+`,
 		},
 	}
 
@@ -68,17 +100,17 @@ secretGenerator:
 			testutils_test.WriteTestKustomizationWith(fSys, []byte(tc.input))
 			cmd := newCmdRemoveSecret(fSys)
 			err := cmd.RunE(cmd, tc.args)
+
 			if tc.expectedErr != "" {
-				assert.Error(t, err)
-				assert.Contains(t, err.Error(), tc.expectedErr)
-			} else {
-				assert.NoError(t, err)
-				content, err := testutils_test.ReadTestKustomization(fSys)
-				assert.NoError(t, err)
-				for _, opt := range strings.Split(tc.args[0], ",") {
-					assert.NotContains(t, string(content), opt)
-				}
+				require.Error(t, err)
+				require.Contains(t, err.Error(), tc.expectedErr)
+				return
 			}
+
+			require.NoError(t, err)
+			content, err := testutils_test.ReadTestKustomization(fSys)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedOutput, string(content))
 		})
 	}
 }