Merge pull request #17004 from juanvallejo/jvallejo/parse-rsrs-arg-wh…

…en-removing-deleted-secret

Automatic merge from submit-queue.

parse resource name before removing deleted secret

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1457602

Although unlinking deleted secrets from a serviceaccount is currently
supported, `oc secret unlink` fails to unlink a deleted secret if its
name is specified as `secrets/deleted-secret-name`.

This patch parses each secret's name, removing the `secrets/` segment
before appending it to a string set of removed secret names.

cc @openshift/cli-review

pkg/oc/cli/secrets/options.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"strings"
 
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -68,6 +69,16 @@ func (o SecretOptions) Validate() error {
 		return errors.New("KubeCoreClient must be present")
 	}
 
+	// if any secret names are of the form <resource>/<name>,
+	// ensure <resource> is a secret.
+	for _, secretName := range o.SecretNames {
+		if segs := strings.Split(secretName, "/"); len(segs) > 1 {
+			if segs[0] != "secret" && segs[0] != "secrets" {
+				return errors.New(fmt.Sprintf("expected resource of type secret, got %q", secretName))
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -98,11 +109,22 @@ func (o SecretOptions) GetServiceAccount() (*kapi.ServiceAccount, error) {
 func (o SecretOptions) GetSecretNames(secrets []*kapi.Secret) sets.String {
 	names := sets.String{}
 	for _, secret := range secrets {
-		names.Insert(secret.Name)
+		names.Insert(parseSecretName(secret.Name))
 	}
 	return names
 }
 
+// parseSecretName receives a resource name as either
+// <resource type> / <name> or <name> and returns only the resource <name>.
+func parseSecretName(name string) string {
+	segs := strings.Split(name, "/")
+	if len(segs) < 2 {
+		return name
+	}
+
+	return segs[1]
+}
+
 // GetMountSecretNames Get a list of the names of the mount secrets associated
 // with a service account
 func (o SecretOptions) GetMountSecretNames(serviceaccount *kapi.ServiceAccount) sets.String {

test/cmd/secrets.sh

@@ -84,6 +84,19 @@ os::cmd::expect_success 'oc secrets add deployer basicauth sshauth --for=pull'
 # make sure we can add as as pull secret and mount secret at once
 os::cmd::expect_success 'oc secrets add deployer basicauth sshauth --for=pull,mount'
 
+# attach secrets to service account
+# test that those secrets can be unlinked
+# after they have been deleted.
+os::cmd::expect_success 'oc create secret generic deleted-secret'
+os::cmd::expect_success 'oc secrets link deployer deleted-secret'
+# confirm our soon-to-be-deleted secret has been linked
+os::cmd::expect_success_and_text "oc get serviceaccount deployer -o jsonpath='{.secrets[?(@.name==\"deleted-secret\")]}'" 'deleted\-secret'
+# delete "deleted-secret" and attempt to unlink from service account
+os::cmd::expect_success 'oc delete secret deleted-secret'
+os::cmd::expect_failure_and_text 'oc secrets unlink deployer secrets/deleted-secret' 'Unlinked deleted secrets'
+# ensure already-deleted secret has been unlinked
+os::cmd::expect_success_and_not_text "oc get serviceaccount deployer -o jsonpath='{.secrets[?(@.name==\"deleted-secret\")]}'" 'deleted\-secret'
+
 # attach secrets to service account
 # single secret with prefix
 os::cmd::expect_success 'oc secrets link deployer basicauth'