return error on long-form or invalid sa name

Returns an error when the long-form name of a ServiceAccount is used
with the --serviceaccount (-z) flag in `oc policy ...' commands, or
if the name given is invalid.

Author: juanvallejo

pkg/oc/admin/policy/modify_roles.go

@@ -4,10 +4,12 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/spf13/cobra"
 
 	kapierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/validation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kapi "k8s.io/kubernetes/pkg/api"
@@ -320,6 +322,17 @@ func (o *RoleModificationOptions) CompleteUserWithSA(f *clientcmd.Factory, cmd *
 		return errors.New("you must specify at least one user or service account")
 	}
 
+	// return an error if a fully-qualified service-account name is used
+	for _, sa := range saNames {
+		if strings.HasPrefix(sa, "system:serviceaccount") {
+			return errors.New("--serviceaccount (-z) should only be used with short-form serviceaccount names (e.g. \"default\")")
+		}
+
+		if len(validation.ValidateServiceAccountName(sa, false)) > 0 {
+			return errors.New(fmt.Sprintf("%q is not a valid serviceaccount name", sa))
+		}
+	}
+
 	authorizationClient, err := f.OpenshiftInternalAuthorizationClient()
 	if err != nil {
 		return err

test/cmd/policy.sh

@@ -40,6 +40,11 @@ os::cmd::expect_success "oc login -u system:admin -n '${project}'"
 os::cmd::expect_success 'oc delete project policy-login'
 os::cmd::expect_failure_and_text 'oc create policybinding default -n myproject' 'error: the server does not support legacy policy resources'
 
+# validate --serviceaccount values
+os::cmd::expect_success_and_text 'oc policy add-role-to-user admin -z default' 'role "admin" added\: "default"'
+os::cmd::expect_failure_and_text 'oc policy add-role-to-user admin -z system:serviceaccount:test:default' 'should only be used with short\-form serviceaccount names'
+os::cmd::expect_failure_and_text 'oc policy add-role-to-user admin -z :invalid' '"\:default" is not a valid serviceaccount name'
+
 # This test validates user level policy
 os::cmd::expect_failure_and_text 'oc policy add-role-to-user' 'you must specify a role'
 os::cmd::expect_failure_and_text 'oc policy add-role-to-user -z NamespaceWithoutRole' 'you must specify a role'