Add error messages to test framework when changing user

Author: tnozicka

pkg/cmd/util/tokencmd/request_token.go

@@ -9,15 +9,16 @@ import (
 	"net/url"
 	"strings"
 
+	"github.com/RangelReale/osincli"
+	"github.com/golang/glog"
+	"github.com/pkg/errors"
+
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	restclient "k8s.io/client-go/rest"
 
 	"github.com/openshift/origin/pkg/oauth/util"
-
-	"github.com/RangelReale/osincli"
-	"github.com/golang/glog"
 )
 
 const (
@@ -106,23 +107,23 @@ func (o *RequestTokenOptions) SetDefaultOsinConfig() error {
 	// get the OAuth metadata from the server
 	rt, err := restclient.TransportFor(o.ClientConfig)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "failed to get transport for client")
 	}
 
 	requestURL := strings.TrimRight(o.ClientConfig.Host, "/") + oauthMetadataEndpoint
 	resp, err := request(rt, requestURL, nil)
 	if err != nil {
-		return err
+		return errors.Wrapf(err, "failed to request OAuth metadata at url %q", requestURL)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("couldn't get %v: unexpected response status %v", requestURL, resp.StatusCode)
+		return errors.Errorf("couldn't get %q: unexpected response status %q", requestURL, resp.Status)
 	}
 
 	metadata := &util.OauthAuthorizationServerMetadata{}
 	if err := json.NewDecoder(resp.Body).Decode(metadata); err != nil {
-		return err
+		return errors.Wrap(err, "failed to decode OAuth metadata")
 	}
 
 	// use the metadata to build the osin config
@@ -134,7 +135,7 @@ func (o *RequestTokenOptions) SetDefaultOsinConfig() error {
 	}
 	if !o.TokenFlow && sets.NewString(metadata.CodeChallengeMethodsSupported...).Has(pkce_s256) {
 		if err := osincli.PopulatePKCE(config); err != nil {
-			return err
+			return errors.Wrap(err, "failed to populate PKCE")
 		}
 	}
 
@@ -159,18 +160,18 @@ func (o *RequestTokenOptions) RequestToken() (string, error) {
 
 	rt, err := restclient.TransportFor(o.ClientConfig)
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, "failed to create transport for client")
 	}
 
 	if o.OsinConfig == nil {
 		if err := o.SetDefaultOsinConfig(); err != nil {
-			return "", err
+			return "", errors.Wrapf(err, "failed to set default osin config")
 		}
 	}
 
 	client, err := osincli.NewClient(o.OsinConfig)
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, "failed to create new client")
 	}
 	client.Transport = rt
 	authorizeRequest := client.NewAuthorizeRequest(osincli.CODE) // assume code flow to start with
@@ -200,7 +201,7 @@ func (o *RequestTokenOptions) RequestToken() (string, error) {
 		// Make the request
 		resp, err := request(rt, requestURL, requestHeaders)
 		if err != nil {
-			return "", err
+			return "", errors.Wrapf(err, "failed to request token")
 		}
 		defer resp.Body.Close()
 
@@ -212,7 +213,7 @@ func (o *RequestTokenOptions) RequestToken() (string, error) {
 				// Handle the challenge
 				newRequestHeaders, shouldRetry, err := o.Handler.HandleChallenge(requestURL, resp.Header)
 				if err != nil {
-					return "", err
+					return "", errors.Wrapf(err, "failed to handle challenge")
 				}
 				if !shouldRetry {
 					return "", apierrs.NewUnauthorized("challenger chose not to retry the request")
@@ -246,7 +247,7 @@ func (o *RequestTokenOptions) RequestToken() (string, error) {
 		// this is required for negotiate flows with mutual authentication.
 		if handledChallenge {
 			if err := o.Handler.CompleteChallenge(requestURL, resp.Header); err != nil {
-				return "", err
+				return "", errors.Wrapf(err, "failed to complete challenge")
 			}
 		}
 
@@ -256,7 +257,7 @@ func (o *RequestTokenOptions) RequestToken() (string, error) {
 			// OAuth response case
 			accessToken, err := oauthTokenFunc(redirectURL)
 			if err != nil {
-				return "", err
+				return "", errors.Wrapf(err, "failed to get token after redirect")
 			}
 			if len(accessToken) > 0 {
 				return accessToken, nil

pkg/oc/cli/cmd/login/login.go

@@ -1,12 +1,12 @@
 package login
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"net/url"
 	"os"
 
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
 	kapierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -69,10 +69,12 @@ func NewCmdLogin(fullName string, f *osclientcmd.Factory, reader io.Reader, out,
 
 			err := RunLogin(cmd, options)
 
-			if kapierrors.IsUnauthorized(err) {
+			// We need to unwrap error so it's type can be checked later
+			unwrappedErr := errors.Cause(err)
+			if kapierrors.IsUnauthorized(unwrappedErr) {
 				fmt.Fprintln(out, "Login failed (401 Unauthorized)")
 
-				if err, isStatusErr := err.(*kapierrors.StatusError); isStatusErr {
+				if err, isStatusErr := unwrappedErr.(*kapierrors.StatusError); isStatusErr {
 					if details := err.Status().Details; details != nil {
 						for _, cause := range details.Causes {
 							fmt.Fprintln(out, cause.Message)

pkg/oc/cli/cmd/login/loginoptions_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/pkg/errors"
 
 	"github.com/openshift/origin/pkg/client/config"
 	"github.com/openshift/origin/pkg/oauth/util"
@@ -331,7 +332,7 @@ func TestPreserveErrTypeAuthInfo(t *testing.T) {
 		t.Fatalf("expecting unauthorized error when gathering authinfo")
 	}
 
-	if !kapierrs.IsUnauthorized(err) {
+	if !kapierrs.IsUnauthorized(errors.Cause(err)) {
 		t.Fatalf("expecting error of type metav1.StatusReasonUnauthorized, but got %T", err)
 	}
 }

test/integration/oauth_basicauth_test.go

@@ -10,6 +10,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/pkg/errors"
+
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	restclient "k8s.io/client-go/rest"
@@ -360,7 +362,7 @@ func TestOAuthBasicAuthPassword(t *testing.T) {
 		if !tc.ExpectSuccess {
 			if err == nil {
 				t.Errorf("%s: Expected error, got token=%v", k, accessToken)
-			} else if statusErr, ok := err.(*apierrs.StatusError); !ok {
+			} else if statusErr, ok := errors.Cause(err).(*apierrs.StatusError); !ok {
 				t.Errorf("%s: expected status error, got %#v", k, err)
 			} else if statusErr.ErrStatus.Code != tc.ExpectErrStatus {
 				t.Errorf("%s: expected error status %d, got %#v", k, tc.ExpectErrStatus, statusErr)

test/util/client.go

@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/pkg/errors"
+
 	kerrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
@@ -65,15 +67,15 @@ func GetClusterAdminClientConfigOrDie(adminKubeConfigFile string) *restclient.Co
 func GetClientForUser(clientConfig *restclient.Config, username string) (kclientset.Interface, *restclient.Config, error) {
 	token, err := tokencmd.RequestToken(clientConfig, nil, username, "password")
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "failed to request token")
 	}
 
 	userClientConfig := restclient.AnonymousClientConfig(clientConfig)
 	userClientConfig.BearerToken = token
 
 	kubeClientset, err := kclientset.NewForConfig(userClientConfig)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "failed to create new kube clientset config")
 	}
 
 	return kubeClientset, userClientConfig, nil