From a17af0e8edf5ad83192a4c96bbd566804f7440db Mon Sep 17 00:00:00 2001
From: juanvallejo <jvallejo@redhat.com>
Date: Mon, 23 Oct 2017 12:08:05 -0400
Subject: [PATCH 1/2] parse resource name before removing deleted secret

Although unlinking deleted secrets from a serviceaccount is currently
supported, `oc secret unlink` failed to unlink a deleted secret if its
name was 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.
---
 pkg/oc/cli/secrets/options.go | 18 +++++++++++++++++-
 test/cmd/secrets.sh           | 13 +++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/pkg/oc/cli/secrets/options.go b/pkg/oc/cli/secrets/options.go
index c5a25f509f83..6b0d0c382ed1 100644
--- a/pkg/oc/cli/secrets/options.go
+++ b/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"
@@ -98,11 +99,26 @@ 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
+	}
+
+	if segs[0] == "secret" || segs[0] == "secrets" {
+		return segs[1]
+	}
+
+	return name
+}
+
 // 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 {
diff --git a/test/cmd/secrets.sh b/test/cmd/secrets.sh
index 0a32b1c57257..8ddc547b4e3a 100755
--- a/test/cmd/secrets.sh
+++ b/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'

From b00c5d1e55fd7e5e2b986e707e78276e97e40137 Mon Sep 17 00:00:00 2001
From: juanvallejo <jvallejo@redhat.com>
Date: Tue, 24 Oct 2017 14:01:54 -0400
Subject: [PATCH 2/2] validate user-specified resource, if given

---
 pkg/oc/cli/secrets/options.go | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/pkg/oc/cli/secrets/options.go b/pkg/oc/cli/secrets/options.go
index 6b0d0c382ed1..31aec5ebe0ae 100644
--- a/pkg/oc/cli/secrets/options.go
+++ b/pkg/oc/cli/secrets/options.go
@@ -69,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
 }
 
@@ -112,11 +122,7 @@ func parseSecretName(name string) string {
 		return name
 	}
 
-	if segs[0] == "secret" || segs[0] == "secrets" {
-		return segs[1]
-	}
-
-	return name
+	return segs[1]
 }
 
 // GetMountSecretNames Get a list of the names of the mount secrets associated