Fix PerformancePolicy and list filters

go.mod

@@ -1,17 +1,18 @@
-module github.com/Datera/go-sdk
+module github.com/tjcelaya/go-datera
 
 go 1.13
 
 require (
 	github.com/Datera/go-udc v1.1.1
 	github.com/google/go-cmp v0.4.1
-	github.com/google/go-querystring v1.0.0 // indirect
 	github.com/google/uuid v1.1.1
 	github.com/levigross/grequests v0.0.0-20190908174114-253788527a1a
 	github.com/mitchellh/go-homedir v1.1.0 // indirect
 	github.com/mitchellh/mapstructure v1.3.1
+	github.com/pkg/errors v0.9.1 // indirect
 	github.com/sirupsen/logrus v1.6.0
-	github.com/stretchr/objx v0.2.0 // indirect
+	github.com/stretchr/testify v1.3.0 // indirect
 	golang.org/x/net v0.0.0-20200602114024-627f9648deb9 // indirect
 	gopkg.in/h2non/gock.v1 v1.0.15
+	gotest.tools v2.2.0+incompatible
 )

go.sum

@@ -1,6 +1,7 @@
 github.com/Datera/go-udc v1.1.1 h1:DWWnp+0PpD30Lqez7oS4wA60g9J71/RCwfSfiCPA0OA=
 github.com/Datera/go-udc v1.1.1/go.mod h1:Q3d9dF5+bUj/OaTaljAApZTpwVf4nmrpGXAJIN9NC0M=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/google/go-cmp v0.4.1 h1:/exdXoGamhu5ONeUJH0deniYLWYvQwW66yvlfiiKTu0=
 github.com/google/go-cmp v0.4.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
@@ -22,14 +23,17 @@ github.com/mitchellh/mapstructure v1.3.1 h1:cCBH2gTD2K0OtLlv/Y5H01VQCqmlDxz30kS5
 github.com/mitchellh/mapstructure v1.3.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
 github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32 h1:W6apQkHrMkS0Muv8G/TipAy/FJl/rCYT0+EuS8+Z0z4=
 github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms=
+github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
+github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/sirupsen/logrus v1.3.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
 github.com/sirupsen/logrus v1.6.0 h1:UBcNElsrwanuuMsnGSlYmtmgbb23qDR5dG+6X6Oo89I=
 github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
+github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
@@ -46,3 +50,5 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IV
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/h2non/gock.v1 v1.0.15 h1:SzLqcIlb/fDfg7UvukMpNcWsu7sI5tWwL+KCATZqks0=
 gopkg.in/h2non/gock.v1 v1.0.15/go.mod h1:sX4zAkdYX1TRGJ2JY156cFspQn4yRWn6p9EMdODlynE=
+gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=
+gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=

pkg/dsdk/connection.go

@@ -39,6 +39,12 @@ var (
 	logTraceID   = "trace_id"
 )
 
+const (
+	canRetry    = true
+	isSensitive = true
+	allowLogin  = true
+)
+
 type ApiConnection struct {
 	m          *sync.RWMutex
 	username   string
@@ -225,9 +231,9 @@ func makeBaseUrl(h, apiv string, secure bool) (*url.URL, error) {
 	return url.Parse(fmt.Sprintf("http://%s:7717/v%s", h, apiv))
 }
 
-func translateErrors(resp *greq.Response, err error) (*ApiErrorResponse, error) {
+func translateErrors(ctxt context.Context, resp *greq.Response, err error) (*ApiErrorResponse, error) {
 	if err != nil {
-		Log().Error(err)
+		WithUserFields(ctxt, Log()).Error(err)
 		if strings.Contains(err.Error(), "connect: connection refused") {
 			return nil, badStatus[ConnectionError]
 		}
@@ -238,7 +244,7 @@ func translateErrors(resp *greq.Response, err error) (*ApiErrorResponse, error)
 		eresp := &ApiErrorResponse{}
 		err := resp.JSON(eresp)
 		if err != nil {
-			eresp.Message = fmt.Sprintf("failed to unmarshal ApiErrorResponse: %v", err)
+			WithUserFields(ctxt, Log()).Error(fmt.Sprintf("failed to unmarshal ApiErrorResponse %+v: %v", eresp, err))
 		}
 
 		// in some cases (like 503s) the response JSON doesn't contain
@@ -259,12 +265,13 @@ func (c *ApiConnection) hasLoggedIn() bool {
 	return c.apikey != ""
 }
 
-func (c *ApiConnection) retry(ctxt context.Context, method, url string, ro *greq.RequestOptions, rs interface{}, sensitive bool) (*ApiErrorResponse, error) {
+func (c *ApiConnection) retry(ctxt context.Context, method, url string, ro *greq.RequestOptions, rs interface{}, sensitive, allowLogin bool) (*ApiErrorResponse, error) {
 	t1 := time.Now().Unix()
 	backoff := 1
 	var apiresp *ApiErrorResponse
 	for time.Now().Unix()-t1 < RetryTimeout {
-		apiresp, err := c.do(ctxt, method, url, ro, rs, false, sensitive)
+		// any call to `do` from within a retry must use `false` for retry param
+		apiresp, err := c.do(ctxt, method, url, ro, rs, !canRetry, sensitive, allowLogin)
 		if apiresp == nil && err == nil {
 			return nil, nil
 		}
@@ -282,13 +289,13 @@ func (c *ApiConnection) retry(ctxt context.Context, method, url string, ro *greq
 	return apiresp, ErrRetryTimeout
 }
 
-func (c *ApiConnection) do(ctxt context.Context, method, url string, ro *greq.RequestOptions, rs interface{}, retry, sensitive bool) (*ApiErrorResponse, error) {
+func (c *ApiConnection) do(ctxt context.Context, method, url string, ro *greq.RequestOptions, rs interface{}, retry, sensitive, allowLogin bool) (*ApiErrorResponse, error) {
 	gurl := *c.baseUrl
 	gurl.Path = path.Join(gurl.Path, url)
 	reqId := uuid.Must(uuid.NewRandom()).String()
 	sdata, err := json.Marshal(ro.JSON)
 	if err != nil {
-		Log().Errorf("Couldn't stringify data, %s", ro.JSON)
+		WithUserFields(ctxt, Log()).Errorf("Couldn't stringify data, %s", ro.JSON)
 	}
 	// Strip all CHAP credentails before printing to logs
 	if strings.Contains(string(sdata), "target_user_name") == true {
@@ -324,14 +331,15 @@ func (c *ApiConnection) do(ctxt context.Context, method, url string, ro *greq.Re
 		ro.BeforeRequest = func(h *http.Request) error {
 			sheaders, err := json.Marshal(h.Header)
 			if err != nil {
-				Log().Errorf("Couldn't stringify headers, %s", h.Header)
+				WithUserFields(ctxt, Log()).Errorf("Couldn't stringify headers, %s", h.Header)
 			}
 
-			Log().WithFields(log.Fields{
+			WithUserFields(ctxt, Log()).WithFields(log.Fields{
 				logTraceID:        tid,
 				"request_id":      reqId,
 				"request_method":  method,
 				"request_url":     gurl.String(),
+				"request_route":   canonicalizeRoute(gurl.Path, c.apiVersion),
 				"request_headers": sheaders,
 				"request_payload": string(sdata),
 				"query_params":    ro.Params,
@@ -350,25 +358,30 @@ func (c *ApiConnection) do(ctxt context.Context, method, url string, ro *greq.Re
 	if _, ok := ctxt.Value("quiet").(bool); ok {
 		rdata = "<muted>"
 	}
-	detailLog := Log().WithFields(log.Fields{
+	detailLog := WithUserFields(ctxt, Log()).WithFields(log.Fields{
 		logTraceID:           tid,
 		"request_id":         reqId,
 		"response_timedelta": tDelta.Seconds(),
 		"request_method":     method,
 		"request_url":        gurl.String(),
 		"request_payload":    string(sdata),
+		"request_route":      canonicalizeRoute(gurl.Path, c.apiVersion),
 		"response_payload":   rdata,
 		"response_code":      resp.StatusCode,
 	})
 
 	detailLog.Debugf("Datera SDK response received")
 
-	eresp, err := translateErrors(resp, err)
+	eresp, err := translateErrors(ctxt, resp, err)
 
 	if err == badStatus[PermissionDenied] {
 		// if we have logged in successfully before we may just need to refresh the apikey
 		// and retry the original request
-		if c.hasLoggedIn() {
+		// However, because Login holds the mutex then if we got here as the result of a 401 during
+		// a Login we can't do anything without deadlocking.  In this case we need to just return
+		// the error
+
+		if allowLogin && c.hasLoggedIn() {
 			c.Logout()
 			if apiresp, err2 := c.Login(ctxt); apiresp != nil || err2 != nil {
 				detailLog.Errorf("failed to re-authenticate before retrying request: %s", err2)
@@ -377,7 +390,7 @@ func (c *ApiConnection) do(ctxt context.Context, method, url string, ro *greq.Re
 			c.m.RLock()
 			ro.Headers["Auth-Token"] = c.apikey
 			c.m.RUnlock()
-			return c.do(ctxt, method, url, ro, rs, false, sensitive)
+			return c.do(ctxt, method, url, ro, rs, !canRetry, sensitive, allowLogin)
 		}
 
 		// but if we get here while logged out then then credentials may no longer be valid and we shouldn't
@@ -386,7 +399,7 @@ func (c *ApiConnection) do(ctxt context.Context, method, url string, ro *greq.Re
 
 	}
 	if retry && (err == badStatus[Retry503] || err == badStatus[ConnectionError]) {
-		return c.retry(ctxt, method, url, ro, rs, sensitive)
+		return c.retry(ctxt, method, url, ro, rs, sensitive, allowLogin)
 	}
 	if eresp != nil {
 		detailLog.Errorf("Received API Error %s", Pretty(eresp))
@@ -408,16 +421,18 @@ func (c *ApiConnection) doWithAuth(ctxt context.Context, method, url string, ro
 	if ro == nil {
 		ro = &greq.RequestOptions{}
 	}
+	// don't need to check the loggingIn flag first because doWithAuth is not called from Login
+	// so that won't deadlock
 	if !c.hasLoggedIn() {
 		if apierr, err := c.Login(ctxt); apierr != nil || err != nil {
-			Log().Errorf("Login failure: %s, %s", Pretty(apierr), err)
+			WithUserFields(ctxt, Log()).Errorf("Login failure: %s, %s", Pretty(apierr), err)
 			return apierr, err
 		}
 	}
 	c.m.RLock()
 	ro.Headers = map[string]string{"tenant": c.tenant, "Auth-Token": c.apikey}
 	c.m.RUnlock()
-	return c.do(ctxt, method, url, ro, rs, true, false)
+	return c.do(ctxt, method, url, ro, rs, canRetry, !isSensitive, allowLogin)
 }
 
 func NewApiConnection(c *udc.UDC, secure bool) *ApiConnection {
@@ -521,6 +536,18 @@ func (c *ApiConnection) ApiVersions() []string {
 }
 
 func (c *ApiConnection) Login(ctxt context.Context) (*ApiErrorResponse, error) {
+	c.m.Lock()
+	defer c.m.Unlock()
+
+	// can't call hasLoggedIn since that needs to RLock but this is equivalent
+	if c.apikey != "" {
+		// any time the connection has an apikey we can skip the login because
+		// the apikey gets cleared after a session expiration before attempting to login
+		// therefore a non-empty apikey can be assumed to be valid
+
+		return nil, nil
+	}
+
 	login := &ApiLogin{}
 	ro := &greq.RequestOptions{
 		Data: map[string]string{
@@ -531,15 +558,15 @@ func (c *ApiConnection) Login(ctxt context.Context) (*ApiErrorResponse, error) {
 	if c.ldap != "" {
 		ro.Data["remote_server"] = c.ldap
 	}
-	apiresp, err := c.do(ctxt, "PUT", "login", ro, login, true, true)
-	c.m.Lock()
+
+	apiresp, err := c.do(ctxt, "PUT", "login", ro, login, canRetry, isSensitive, !allowLogin)
+
 	if (apiresp != nil && apiresp.Http == PermissionDenied) || (err != nil && err == badStatus[PermissionDenied]) {
 		c.apikey = ""
 	} else {
 		c.apikey = login.Key
 	}
 
-	c.m.Unlock()
 	return apiresp, err
 }