Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix swallowed errors in session package #1763

Open
wants to merge 1 commit into
base: eng
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions go/session/avisession.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ const DEFAULT_API_RETRY_INTERVAL = 500

//NewAviSession initiates a session to AviController and returns it
func NewAviSession(host string, username string, options ...func(*AviSession) error) (*AviSession, error) {
if flag.Parsed() == false {
if !flag.Parsed() {
flag.Parse()
}
avisess := &AviSession{
Expand Down Expand Up @@ -342,20 +342,26 @@ func NewAviSession(host string, username string, options ...func(*AviSession) er
}

func (avisess *AviSession) initiateSession() error {
if avisess.insecure == true {
if avisess.insecure {
glog.Warning("Strict certificate verification is *DISABLED*")
}

// If refresh auth token is provided, use callback function provided
if avisess.isTokenAuth() {
switch {
case avisess.refreshAuthToken != nil:
avisess.setAuthToken(avisess.refreshAuthToken())
err := avisess.setAuthToken(avisess.refreshAuthToken())
if err != nil {
return err
}
case avisess.refreshAuthTokenV2 != nil:
if token, err := avisess.refreshAuthTokenV2(); err != nil {
return err
} else {
avisess.setAuthToken(token)
err := avisess.setAuthToken(token)
if err != nil {
return err
}
}
}
}
Expand Down Expand Up @@ -650,7 +656,10 @@ func (avisess *AviSession) restRequest(verb string, uri string, payload interfac
}

if avisess.lazyAuthentication && avisess.sessionid == "" && !(uri == "" || uri == "login") {
avisess.initiateSession()
err := avisess.initiateSession()
if err != nil {
return nil, err
}
}

var payloadIO io.Reader
Expand Down Expand Up @@ -702,7 +711,7 @@ func (avisess *AviSession) restRequest(verb string, uri string, payload interfac
if retryReq {
if !avisess.disableControllerStatusCheck {
check, httpResp, err := avisess.CheckControllerStatus()
if check == false {
if !check {
if resp != nil && resp.Body != nil {
glog.Infof("Body is not nil, close it.")
resp.Body.Close()
Expand Down Expand Up @@ -787,7 +796,10 @@ func (avisess *AviSession) restMultipartUploadRequest(verb string, uri string, f
}

if avisess.lazyAuthentication && avisess.sessionid == "" && !(uri == "" || uri == "login") {
avisess.initiateSession()
err := avisess.initiateSession()
if err != nil {
return err
}
}

errorResult := AviError{Verb: verb, Url: url}
Expand Down Expand Up @@ -865,7 +877,7 @@ func (avisess *AviSession) restMultipartUploadRequest(verb string, uri string, f

if retryReq {
check, _, err := avisess.CheckControllerStatus()
if check == false {
if !check {
glog.Errorf("restMultipartUploadRequest Error during checking controller state")
return err
}
Expand Down Expand Up @@ -944,7 +956,7 @@ func (avisess *AviSession) restMultipartDownloadRequest(verb string, uri string,

if retryReq {
check, _, err := avisess.CheckControllerStatus()
if check == false {
if !check {
glog.Errorf("restMultipartDownloadRequest Error during checking controller state")
return err
}
Expand Down Expand Up @@ -1349,6 +1361,9 @@ func getOptions(options []ApiOptionsParams) (*ApiOptions, error) {
// GetObject performs GET and return object data
func (avisess *AviSession) GetObject(obj string, options ...ApiOptionsParams) error {
opts, err := getOptions(options)
if err != nil {
return err
}
uri, err := avisess.GetUri(obj, options...)
if err != nil {
return err
Expand Down Expand Up @@ -1409,7 +1424,10 @@ func (avisess *AviSession) Logout() error {
}

func (avisess *AviSession) ResetPassword(password string) error {
avisess.Logout()
err := avisess.Logout()
if err != nil {
return err
}
avisess.password = password
return nil
}
Expand Down
34 changes: 29 additions & 5 deletions go/session/avisession_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ func getValidTokenV2() (string, error) {
SetAuthToken(AVI_AUTH_TOKEN), SetInsecure, SetTenant(AVI_TENANT),
SetVersion(aviVersion))
}
if err != nil {
return "", err
}

err = aviAuthSessionV2.Post(tokenPath, data, &robj)
if err != nil {
glog.Infof("Error while getting auth token. [ERROR]: %s", err.Error())
Expand Down Expand Up @@ -113,12 +117,16 @@ func getSessions(t *testing.T) []*AviSession {
SetTenant(AVI_TENANT), SetAuthToken(AVI_AUTH_TOKEN), SetInsecure, SetVersion(aviVersion))
}

if err != nil {
t.Errorf("Set tenant failed: %s", err.Error())
}

var sessionSetAuthTokenV2 *AviSession
sessionSetAuthTokenV2, err = NewAviSession(AVI_CONTROLLER, AVI_USERNAME,
SetRefreshAuthTokenCallbackV2(getValidTokenV2), SetInsecure, SetTenant(AVI_TENANT),
SetVersion(aviVersion))
if err != nil {
t.Errorf("Session Creation failed: %s", err)
t.Errorf("Session Creation failed: %s", err.Error())
}

if AVI_CONTROLLER != "localhost" {
Expand Down Expand Up @@ -159,17 +167,17 @@ func testControllerStatusCheckLimits(t *testing.T) {

// try to init the session with illegal inputs for controller status check limits.
if AVI_PASSWORD != "" {
aviSession, err = NewAviSession(AVI_CONTROLLER, AVI_USERNAME,
_, err = NewAviSession(AVI_CONTROLLER, AVI_USERNAME,
SetTenant(AVI_TENANT), SetPassword(AVI_PASSWORD), SetInsecure, SetLazyAuthentication(true),
SetControllerStatusCheckLimits(0, -1), SetVersion(aviVersion))
} else {
aviSession, err = NewAviSession(AVI_CONTROLLER, AVI_USERNAME,
_, err = NewAviSession(AVI_CONTROLLER, AVI_USERNAME,
SetTenant(AVI_TENANT), SetAuthToken(AVI_AUTH_TOKEN), SetInsecure, SetLazyAuthentication(true),
SetControllerStatusCheckLimits(-2, -3),
SetVersion(aviVersion))
}
if err == nil {
t.Errorf("The Avi session go created with illegal arguments")
t.Errorf("The Avi session got created with illegal arguments")
}
if AVI_PASSWORD != "" {
aviSession, err = NewAviSession(AVI_CONTROLLER, AVI_USERNAME,
Expand Down Expand Up @@ -460,6 +468,10 @@ func TestTokenAuthRobustness(t *testing.T) {
authTokenSessionCallback, err := NewAviSession(AVI_CONTROLLER, "admin",
SetRefreshAuthTokenCallback(bogusAuthTokenFunction),
SetInsecure)
if err != nil {
t.Errorf("Failed to create new session: %s", err.Error())
}

var res interface{}
err = authTokenSessionCallback.Get("api/tenant", &res)
if err == nil {
Expand All @@ -469,6 +481,10 @@ func TestTokenAuthRobustness(t *testing.T) {
authTokenSession, err := NewAviSession(AVI_CONTROLLER, "admin",
SetAuthToken("wrong-auth-token"),
SetInsecure)
if err != nil {
t.Errorf("Failed to create new session: %s", err.Error())
}

err = authTokenSession.Get("api/tenant", &res)
if err == nil {
t.Errorf("ERROR: Expected an error from incorrect token auth")
Expand All @@ -480,6 +496,10 @@ func TestTokenAuthRobustnessV2(t *testing.T) {
authTokenSessionCallback, err := NewAviSession(AVI_CONTROLLER, "admin",
SetRefreshAuthTokenCallbackV2(bogusAuthTokenFunctionV2),
SetInsecure)
if err != nil {
t.Errorf("Failed to create new session: %s", err.Error())
}

var res interface{}
err = authTokenSessionCallback.Get("api/tenant", &res)
if err.Error() != "Invalid token from callback method" {
Expand Down Expand Up @@ -628,7 +648,11 @@ func testTenantSwitch(t *testing.T, avisess *AviSession) {

// Tests to check logout functionality
func testApiLogout(t *testing.T, avisess *AviSession) {
avisess.Logout()
err := avisess.Logout()
if err != nil {
t.Errorf("Failed to logout: %s", err.Error())
}

prevSsnId := avisess.sessionid
var res interface{}
if err := avisess.Get("api/pool", &res); err == nil {
Expand Down