You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ra...@apache.org on 2019/12/02 23:12:21 UTC

[trafficcontrol] branch master updated: Deprecate /hwinfo (#4143)

This is an automated email from the ASF dual-hosted git repository.

rawlin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new e1571b1  Deprecate /hwinfo (#4143)
e1571b1 is described below

commit e1571b1ff4798ba6936524d311c5f72273090bc3
Author: ocket8888 <oc...@gmail.com>
AuthorDate: Mon Dec 2 16:12:11 2019 -0700

    Deprecate /hwinfo (#4143)
    
    * Added convenience methods to Alerts structure
    
    * Added deprecation notice to hardware client method
    
    * Fixed Go-based hwinfo handler to be backward-compatible
    
    * Added convenience API methods for writing multiple alerts
    
    * made Go handler handle /hwinfo instead of /hwinfo-wip
    
    * Go fmt
    
    * updated docs
    
    * Fixed the unit tests I broke
    
    * Added GoDocs and examples for alerts-related code
    
    Also go fmt
    
    * Fixed excessive quote escaping in test strings
    
    * Fixed wrong table name
---
 docs/source/api/hwinfo.rst                         |  58 ++++++++----
 lib/go-tc/alerts.go                                |  39 +++++++-
 lib/go-tc/alerts_test.go                           |  83 ++++++++++++++++
 traffic_ops/client/hardware.go                     |   2 +
 .../testing/api/v14/parentdotconfig_test.go        |   2 +-
 traffic_ops/traffic_ops_golang/api/api.go          |  49 +++++++++-
 .../traffic_ops_golang/api/shared_handlers_test.go |   8 +-
 traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go    | 104 +++++++++++++++------
 traffic_ops/traffic_ops_golang/login/login_test.go |   2 +-
 traffic_ops/traffic_ops_golang/routing/routes.go   |   2 +-
 .../traffic_ops_golang/routing/wrappers_test.go    |   2 +-
 11 files changed, 291 insertions(+), 60 deletions(-)

diff --git a/docs/source/api/hwinfo.rst b/docs/source/api/hwinfo.rst
index c706172..b0b5425 100644
--- a/docs/source/api/hwinfo.rst
+++ b/docs/source/api/hwinfo.rst
@@ -51,17 +51,26 @@ Request Structure:
 	+----------------+----------+---------------------------------------------------------------------------------------------------------------+
 	| sortOrder      | no       | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc")                      |
 	+----------------+----------+---------------------------------------------------------------------------------------------------------------+
-	| limit          | no       | Choose the maximum number of results to return                                                                |
+	| limit          | no       | Choose the maximum number of results to return. Default if not specified is 1000.                             |
 	+----------------+----------+---------------------------------------------------------------------------------------------------------------+
 	| offset         | no       | The number of results to skip before beginning to return results. Must use in conjunction with limit          |
 	+----------------+----------+---------------------------------------------------------------------------------------------------------------+
 	| page           | no       | Return the n\ :sup:`th` page of results, where "n" is the value of this parameter, pages are ``limit`` long   |
-	|                |          | and the first page is 1. If ``offset`` was defined, this query parameter has no effect. ``limit`` must be     |
-	|                |          | defined to make use of ``page``.                                                                              |
+	|                |          | and the first page is 1. If ``offset`` was defined, this query parameter has no effect.                       |
 	+----------------+----------+---------------------------------------------------------------------------------------------------------------+
 
 .. caution:: The ``lastUpdated`` query parameter doesn't seem to work properly, and its use is therefore discouraged.
 
+.. code:: http
+	:caption: Request Example
+
+	GET /api/1.3/hwinfo HTTP/1.1
+	User-Agent: python-requests/2.22.0
+	Accept-Encoding: gzip, deflate
+	Accept: */*
+	Connection: keep-alive
+	Cookie: mojolicious=...
+
 Response Structure
 ------------------
 :description:    Freeform description for this specific server's hardware info
@@ -70,21 +79,38 @@ Response Structure
 :serverId:       Local unique identifier for this specific server's hardware info
 :val:            Freeform value used to track anything about a server's hardware info
 
-.. code-block:: json
+Also, in addition to the regular ``response`` field and any and all ``alerts``, this endpoint returns an extra top-level JSON key: ``limit``.
+
+:limit: The number of results to which the result was limited. Should be exactly as specified in the `Request Structure`_.
+
+.. code-block:: http
 	:caption: Response Example
 
-	{ "response": [
+	HTTP/1.1 200 OK
+	Access-Control-Allow-Credentials: true
+	Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+	Access-Control-Allow-Origin: *
+	Content-Encoding: gzip
+	Content-Type: application/json
+	Set-Cookie: mojolicious=...; Path=/; Expires=Fri, 22 Nov 2019 20:28:07 GMT; Max-Age=3600; HttpOnly
+	X-Server-Name: traffic_ops_golang/
+	Date: Fri, 22 Nov 2019 19:28:07 GMT
+	Content-Length: 138
+
+	{ "alerts": [
 		{
-			"serverId": "odol-atsmid-cen-09",
-			"lastUpdated": "2014-05-27 09:06:02",
-			"val": "D1S4",
-			"description": "Physical Disk 0:1:0"
-		},
+			"text": "This endpoint is deprecated, and will be removed in the future",
+			"level": "warning"
+		}
+	],
+	"response": [
 		{
-			"serverId": "odol-atsmid-cen-09",
-			"lastUpdated": "2014-05-27 09:06:02",
-			"val": "D1S4",
-			"description": "Physical Disk 0:1:1"
+			"description": "quest",
+			"lastUpdated": "2019-11-22 19:31:26+00",
+			"serverHostName": "dns",
+			"serverId": 1,
+			"val": "test"
 		}
-	]}
-
+	],
+	"limit": 1000
+	}
diff --git a/lib/go-tc/alerts.go b/lib/go-tc/alerts.go
index 18bb066..be862da 100644
--- a/lib/go-tc/alerts.go
+++ b/lib/go-tc/alerts.go
@@ -28,15 +28,23 @@ import (
 	"github.com/apache/trafficcontrol/lib/go-log"
 )
 
+// Alert represents an informational message, typically returned through the Traffic Ops API.
 type Alert struct {
-	Text  string `json:"text"`
+	// Text is the actual message being conveyed.
+	Text string `json:"text"`
+	// Level describes what kind of message is being relayed. In practice, it should be the string
+	// representation of one of ErrorLevel, WarningLevel, InfoLevel or SuccessLevel.
 	Level string `json:"level"`
 }
 
+// Alerts is merely a collection of arbitrary "Alert"s for ease of use in other structures, most
+// notably those used in Traffic Ops API responses.
 type Alerts struct {
 	Alerts []Alert `json:"alerts"`
 }
 
+// CreateErrorAlerts creates and returns an Alerts structure filled with ErrorLevel-level "Alert"s
+// using the errors to provide text.
 func CreateErrorAlerts(errs ...error) Alerts {
 	alerts := []Alert{}
 	for _, err := range errs {
@@ -47,6 +55,8 @@ func CreateErrorAlerts(errs ...error) Alerts {
 	return Alerts{alerts}
 }
 
+// CreateAlerts creates and returns an Alerts structure filled with "Alert"s that are all of the
+// provided level, each having one of messages as text in turn.
 func CreateAlerts(level AlertLevel, messages ...string) Alerts {
 	alerts := []Alert{}
 	for _, message := range messages {
@@ -55,6 +65,12 @@ func CreateAlerts(level AlertLevel, messages ...string) Alerts {
 	return Alerts{alerts}
 }
 
+// GetHandleErrorsFunc is used to provide an error-handling function. The error handler provides a
+// response to an HTTP request made to the Traffic Ops API and accepts a response code and a set of
+// errors to display as alerts.
+//
+// Deprecated: Traffic Ops API handlers should use
+// github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api.HandleErr instead.
 func GetHandleErrorsFunc(w http.ResponseWriter, r *http.Request) func(status int, errs ...error) {
 	return func(status int, errs ...error) {
 		log.Errorf("%v %v\n", r.RemoteAddr, errs)
@@ -75,6 +91,8 @@ func GetHandleErrorsFunc(w http.ResponseWriter, r *http.Request) func(status int
 	}
 }
 
+// ToStrings converts Alerts to a slice of strings that are their messages. Note that this return
+// value doesn't contain their Levels anywhere.
 func (alerts *Alerts) ToStrings() []string {
 	alertStrs := []string{}
 	for _, alrt := range alerts.Alerts {
@@ -84,4 +102,23 @@ func (alerts *Alerts) ToStrings() []string {
 	return alertStrs
 }
 
+// AddNewAlert constructs a new Alert with the given Level and Text and appends it to the Alerts
+// structure.
+func (self *Alerts) AddNewAlert(level AlertLevel, text string) {
+	self.AddAlert(Alert{Level: level.String(), Text: text})
+}
+
+// AddAlert appends an alert to the Alerts structure.
+func (self *Alerts) AddAlert(alert Alert) {
+	self.Alerts = append(self.Alerts, alert)
+}
+
+// AddAlerts appends all of the "Alert"s in the given Alerts structure to this Alerts structure.
+func (self *Alerts) AddAlerts(alerts Alerts) {
+	newAlerts := make([]Alert, len(self.Alerts), len(self.Alerts)+len(alerts.Alerts))
+	copy(newAlerts, self.Alerts)
+	newAlerts = append(newAlerts, alerts.Alerts...)
+	self.Alerts = newAlerts
+}
+
 var StatusKey = "status"
diff --git a/lib/go-tc/alerts_test.go b/lib/go-tc/alerts_test.go
index 6ee5c56..b66dc3c 100644
--- a/lib/go-tc/alerts_test.go
+++ b/lib/go-tc/alerts_test.go
@@ -20,6 +20,7 @@ package tc
  */
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -27,6 +28,88 @@ import (
 	"testing"
 )
 
+func ExampleCreateErrorAlerts() {
+	alerts := CreateErrorAlerts(errors.New("foo"))
+	fmt.Printf("%v\n", alerts)
+	// Output: {[{foo error}]}
+}
+
+func ExampleCreateAlerts() {
+	alerts := CreateAlerts(InfoLevel, "foo", "bar")
+	fmt.Printf("%d\n", len(alerts.Alerts))
+	fmt.Printf("Level: %s, Text: %s\n", alerts.Alerts[0].Level, alerts.Alerts[0].Text)
+	fmt.Printf("Level: %s, Text: %s\n", alerts.Alerts[1].Level, alerts.Alerts[1].Text)
+
+	// Output: 2
+	// Level: info, Text: foo
+	// Level: info, Text: bar
+}
+
+func ExampleAlerts_ToStrings() {
+	alerts := CreateAlerts(InfoLevel, "foo", "bar")
+	strs := alerts.ToStrings()
+	fmt.Printf("%d\n%s\n%s\n", len(strs), strs[0], strs[1])
+	// Output: 2
+	// foo
+	// bar
+}
+
+func ExampleAlerts_AddNewAlert() {
+	var alerts Alerts
+	fmt.Printf("%d\n", len(alerts.Alerts))
+	alerts.AddNewAlert(InfoLevel, "foo")
+	fmt.Printf("%d\n", len(alerts.Alerts))
+	fmt.Printf("Level: %s, Text: %s\n", alerts.Alerts[0].Level, alerts.Alerts[0].Text)
+
+	// Output: 0
+	// 1
+	// Level: info, Text: foo
+}
+
+func ExampleAlerts_AddAlert() {
+	var alerts Alerts
+	fmt.Printf("%d\n", len(alerts.Alerts))
+	alert := Alert{
+		Level: InfoLevel.String(),
+		Text:  "foo",
+	}
+	alerts.AddAlert(alert)
+	fmt.Printf("%d\n", len(alerts.Alerts))
+	fmt.Printf("Level: %s, Text: %s\n", alerts.Alerts[0].Level, alerts.Alerts[0].Text)
+
+	// Output: 0
+	// 1
+	// Level: info, Text: foo
+}
+
+func ExampleAlerts_AddAlerts() {
+	alerts1 := Alerts{
+		[]Alert{
+			Alert{
+				Level: InfoLevel.String(),
+				Text:  "foo",
+			},
+		},
+	}
+	alerts2 := Alerts{
+		[]Alert{
+			Alert{
+				Level: ErrorLevel.String(),
+				Text:  "bar",
+			},
+		},
+	}
+
+	alerts1.AddAlerts(alerts2)
+	fmt.Printf("%d\n", len(alerts1.Alerts))
+	fmt.Printf("Level: %s, Text: %s\n", alerts1.Alerts[0].Level, alerts1.Alerts[0].Text)
+	fmt.Printf("Level: %s, Text: %s\n", alerts1.Alerts[1].Level, alerts1.Alerts[1].Text)
+
+	// Output: 2
+	// Level: info, Text: foo
+	// Level: error, Text: bar
+}
+
 func TestGetHandleErrorFunc(t *testing.T) {
 	w := httptest.NewRecorder()
 	r, err := http.NewRequest("", ".", nil)
diff --git a/traffic_ops/client/hardware.go b/traffic_ops/client/hardware.go
index a353326..659c002 100644
--- a/traffic_ops/client/hardware.go
+++ b/traffic_ops/client/hardware.go
@@ -29,6 +29,8 @@ func (to *Session) Hardware(limit int) ([]tc.Hardware, error) {
 	return h, err
 }
 
+// GetHardware fetches an array of Hardware up to as many as 'limit' specifies.
+// Deprecated: Hardware is deprecated and will not be exposed through the API in the future.
 func (to *Session) GetHardware(limit int) ([]tc.Hardware, ReqInf, error) {
 	url := "/api/1.2/hwinfo.json"
 	if limit > 0 {
diff --git a/traffic_ops/testing/api/v14/parentdotconfig_test.go b/traffic_ops/testing/api/v14/parentdotconfig_test.go
index 53e946f..1e0b065 100644
--- a/traffic_ops/testing/api/v14/parentdotconfig_test.go
+++ b/traffic_ops/testing/api/v14/parentdotconfig_test.go
@@ -112,7 +112,7 @@ func CreateTestDeliveryServiceServers(t *testing.T) {
 // DeleteTestDeliveryServiceServersCreated deletes the dss assignments created by CreateTestDeliveryServiceServers.
 func DeleteTestDeliveryServiceServersCreated(t *testing.T) {
 	// You gotta do this because TOSession.GetDeliveryServiceServers doesn't fetch the complete response.......
-	dssLen := len(testData.Servers)*len(testData.DeliveryServices)
+	dssLen := len(testData.Servers) * len(testData.DeliveryServices)
 	dsServers, _, err := TOSession.GetDeliveryServiceServersN(dssLen)
 	if err != nil {
 		t.Fatalf("GET delivery service servers: %v", err)
diff --git a/traffic_ops/traffic_ops_golang/api/api.go b/traffic_ops/traffic_ops_golang/api/api.go
index f517949..54f2174 100644
--- a/traffic_ops/traffic_ops_golang/api/api.go
+++ b/traffic_ops/traffic_ops_golang/api/api.go
@@ -106,7 +106,7 @@ func WriteRespVals(w http.ResponseWriter, r *http.Request, v interface{}, vals m
 		return
 	}
 	w.Header().Set("Content-Type", "application/json")
-	w.Write(respBts)
+	w.Write(append(respBts, '\n'))
 }
 
 // HandleErr handles an API error, rolling back the transaction, writing the given statusCode and userErr to the user, and logging the sysErr. If userErr is nil, the text of the HTTP statusCode is written.
@@ -148,7 +148,7 @@ func handleSimpleErr(w http.ResponseWriter, r *http.Request, statusCode int, use
 	log.Debugln(userErr.Error())
 	*r = *r.WithContext(context.WithValue(r.Context(), tc.StatusKey, statusCode))
 	w.Header().Set(tc.ContentType, tc.ApplicationJson)
-	w.Write(respBts)
+	w.Write(append(respBts, '\n'))
 }
 
 // RespWriter is a helper to allow a one-line response, for endpoints with a function that returns the object that needs to be written and an error.
@@ -191,7 +191,7 @@ func WriteRespAlert(w http.ResponseWriter, r *http.Request, level tc.AlertLevel,
 		return
 	}
 	w.Header().Set(tc.ContentType, tc.ApplicationJson)
-	w.Write(respBts)
+	w.Write(append(respBts, '\n'))
 }
 
 // WriteRespAlertObj Writes the given alert, and the given response object.
@@ -216,7 +216,48 @@ func WriteRespAlertObj(w http.ResponseWriter, r *http.Request, level tc.AlertLev
 		return
 	}
 	w.Header().Set(tc.ContentType, tc.ApplicationJson)
-	w.Write(respBts)
+	w.Write(append(respBts, '\n'))
+}
+
+func WriteAlerts(w http.ResponseWriter, r *http.Request, code int, alerts tc.Alerts) {
+	if respWritten(r) {
+		log.Errorf("WriteAlerts called after a write already occurred! Not double-writing! Path %s", r.URL.Path)
+		return
+	}
+	setRespWritten(r)
+
+	resp, err := json.Marshal(alerts)
+	if err != nil {
+		handleSimpleErr(w, r, http.StatusInternalServerError, nil, fmt.Errorf("marshalling JSON: %v", err))
+		return
+	}
+	w.Header().Set(tc.ContentType, tc.ApplicationJson)
+	w.WriteHeader(code)
+	w.Write(append(resp, '\n'))
+}
+
+func WriteAlertsObj(w http.ResponseWriter, r *http.Request, code int, alerts tc.Alerts, obj interface{}) {
+	if respWritten(r) {
+		log.Errorf("WriteAlertsObj called after a write already occurred! Not double-writing! Path %s", r.URL.Path)
+		return
+	}
+	setRespWritten(r)
+
+	resp := struct {
+		tc.Alerts
+		Response interface{} `json:"response"`
+	}{
+		Alerts:   alerts,
+		Response: obj,
+	}
+	respBts, err := json.Marshal(resp)
+	if err != nil {
+		handleSimpleErr(w, r, http.StatusInternalServerError, nil, fmt.Errorf("marshalling JSON: %v", err))
+		return
+	}
+	w.Header().Set(tc.ContentType, tc.ApplicationJson)
+	w.WriteHeader(code)
+	w.Write(append(respBts, '\n'))
 }
 
 // IntParams parses integer parameters, and returns map of the given params, or an error if any integer param is not an integer. The intParams may be nil if no integer parameters are required. Note this does not check existence; if an integer paramter is required, it should be included in the requiredParams given to NewInfo.
diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers_test.go b/traffic_ops/traffic_ops_golang/api/shared_handlers_test.go
index 683c6a6..6cef2fc 100644
--- a/traffic_ops/traffic_ops_golang/api/shared_handlers_test.go
+++ b/traffic_ops/traffic_ops_golang/api/shared_handlers_test.go
@@ -142,7 +142,7 @@ func TestCreateHandler(t *testing.T) {
 	createFunc(w, r)
 
 	//verifies the body is in the expected format
-	body := `{"alerts":[{"text":"tester was created.","level":"success"}],"response":{"ID":1}}`
+	body := `{"alerts":[{"text":"tester was created.","level":"success"}],"response":{"ID":1}}` + "\n"
 	if w.Body.String() != body {
 		t.Error("Expected body", body, "got", w.Body.String())
 	}
@@ -182,7 +182,7 @@ func TestReadHandler(t *testing.T) {
 	readFunc(w, r)
 
 	//verifies the body is in the expected format
-	body := "{\"response\":[{\"ID\":1}]}\n"
+	body := `{"response":[{"ID":1}]}` + "\n"
 	if w.Body.String() != body {
 		t.Error("Expected body", body, "got", w.Body.String())
 	}
@@ -228,7 +228,7 @@ func TestUpdateHandler(t *testing.T) {
 	updateFunc(w, r)
 
 	//verifies the body is in the expected format
-	body := `{"alerts":[{"text":"tester was updated.","level":"success"}],"response":{"ID":1}}`
+	body := `{"alerts":[{"text":"tester was updated.","level":"success"}],"response":{"ID":1}}` + "\n"
 	if w.Body.String() != body {
 		t.Error("Expected body", body, "got", w.Body.String())
 	}
@@ -273,7 +273,7 @@ func TestDeleteHandler(t *testing.T) {
 	deleteFunc(w, r)
 
 	//verifies the body is in the expected format
-	body := `{"alerts":[{"text":"tester was deleted.","level":"success"}]}`
+	body := `{"alerts":[{"text":"tester was deleted.","level":"success"}]}` + "\n"
 	if w.Body.String() != body {
 		t.Error("Expected body", body, "got", w.Body.String())
 	}
diff --git a/traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go b/traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go
index 263f581..644e084 100644
--- a/traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go
+++ b/traffic_ops/traffic_ops_golang/hwinfo/hwinfo.go
@@ -20,74 +20,116 @@ package hwinfo
  */
 
 import (
-	"errors"
+	"encoding/json"
+	"fmt"
 	"net/http"
+	"strconv"
 
 	"github.com/apache/trafficcontrol/lib/go-log"
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/lib/go-util"
+
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+
 	"github.com/jmoiron/sqlx"
 )
 
+const selectHWInfoQuery = `
+SELECT
+	s.host_name as serverhostname,
+	h.id,
+	h.serverid,
+	h.description,
+	h.val,
+	h.last_updated
+FROM hwinfo h
+JOIN server s ON s.id = h.serverid
+`
+
+// Get handles GET requests to /hwinfo
 func Get(w http.ResponseWriter, r *http.Request) {
 	inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+	tx := inf.Tx
 	if userErr != nil || sysErr != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
 		return
 	}
 	defer inf.Close()
-	api.RespWriter(w, r, inf.Tx.Tx)(getHWInfo(inf.Tx, inf.Params))
+
+	alerts := tc.CreateAlerts(tc.WarnLevel, "This endpoint is deprecated, and will be removed in the future")
+
+	// Mimic Perl behavior
+	if _, ok := inf.Params["limit"]; !ok {
+		inf.Params["limit"] = "1000"
+	}
+	limit, err := strconv.ParseUint(inf.Params["limit"], 10, 64)
+	if err != nil || limit == 0 {
+		alerts.AddNewAlert(tc.ErrorLevel, "'limit' parameter must be a positive integer")
+		api.WriteAlerts(w, r, http.StatusBadRequest, alerts)
+		return
+	}
+
+	hwInfo, err := getHWInfo(tx, inf.Params)
+	if err != nil {
+		log.Errorln(err.Error())
+		alerts.AddNewAlert(tc.ErrorLevel, http.StatusText(http.StatusInternalServerError))
+		api.WriteAlerts(w, r, http.StatusInternalServerError, alerts)
+		return
+	}
+
+	resp := struct {
+		tc.Alerts
+		Response []tc.HWInfo `json:"response"`
+		Limit    uint64      `json:"limit"`
+	}{
+		Alerts:   alerts,
+		Response: hwInfo,
+		Limit:    limit,
+	}
+
+	var respBts []byte
+	if respBts, err = json.Marshal(resp); err != nil {
+		log.Errorf("Marshaling JSON: %v", err)
+		alerts.AddNewAlert(tc.ErrorLevel, http.StatusText(http.StatusInternalServerError))
+		api.WriteAlerts(w, r, http.StatusInternalServerError, alerts)
+		return
+	}
+
+	w.Header().Set(tc.ContentType, tc.ApplicationJson)
+	w.Write(append(respBts, '\n'))
 }
 
 func getHWInfo(tx *sqlx.Tx, params map[string]string) ([]tc.HWInfo, error) {
-	// Query Parameters to Database Query column mappings
-	// see the fields mapped in the SQL query
+
 	queryParamsToSQLCols := map[string]dbhelpers.WhereColumnInfo{
 		"id":             dbhelpers.WhereColumnInfo{"h.id", api.IsInt},
 		"serverHostName": dbhelpers.WhereColumnInfo{"s.host_name", nil},
-		"serverId":       dbhelpers.WhereColumnInfo{"s.id", api.IsInt}, // TODO: this can be either s.id or h.serverid not sure what makes the most sense
+		"serverId":       dbhelpers.WhereColumnInfo{"s.id", api.IsInt},
 		"description":    dbhelpers.WhereColumnInfo{"h.description", nil},
 		"val":            dbhelpers.WhereColumnInfo{"h.val", nil},
 		"lastUpdated":    dbhelpers.WhereColumnInfo{"h.last_updated", nil}, //TODO: this doesn't appear to work needs debugging
 	}
 	where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(params, queryParamsToSQLCols)
 	if len(errs) > 0 {
-		return nil, errors.New("getHWInfo building where clause: " + util.JoinErrsStr(errs))
+		return nil, fmt.Errorf("Building hwinfo query clauses: %v", util.JoinErrs(errs))
 	}
-	query := selectHWInfoQuery() + where + orderBy + pagination
-	log.Debugln("Query is ", query)
 
-	rows, err := tx.NamedQuery(query, queryValues)
+	rows, err := tx.NamedQuery(selectHWInfoQuery+where+orderBy+pagination, queryValues)
 	if err != nil {
-		return nil, errors.New("sqlx querying hwInfo: " + err.Error())
+		return nil, fmt.Errorf("querying hwinfo: %v", err)
 	}
 	defer rows.Close()
 
 	hwInfo := []tc.HWInfo{}
 	for rows.Next() {
-		s := tc.HWInfo{}
-		if err = rows.StructScan(&s); err != nil {
-			return nil, errors.New("sqlx scanning hwInfo: " + err.Error())
+		var info tc.HWInfo
+		if err = rows.StructScan(&info); err != nil {
+			return nil, fmt.Errorf("scanning hwinfo: %v", err)
 		}
-		hwInfo = append(hwInfo, s)
-	}
-	return hwInfo, nil
-}
 
-func selectHWInfoQuery() string {
-
-	query := `SELECT
-	s.host_name as serverhostname,
-    h.id,
-    h.serverid,
-    h.description,
-    h.val,
-    h.last_updated
-
-FROM hwInfo h
+		hwInfo = append(hwInfo, info)
+	}
 
-JOIN server s ON s.id = h.serverid`
-	return query
+	return hwInfo, nil
 }
diff --git a/traffic_ops/traffic_ops_golang/login/login_test.go b/traffic_ops/traffic_ops_golang/login/login_test.go
index 3d3c90e..0518fce 100644
--- a/traffic_ops/traffic_ops_golang/login/login_test.go
+++ b/traffic_ops/traffic_ops_golang/login/login_test.go
@@ -48,7 +48,7 @@ func TestLoginWithEmptyCredentials(t *testing.T) {
 		}
 		LoginHandler(nil, config.Config{})(w, r)
 
-		expected := `{"alerts":[{"text":"username and password are required","level":"error"}]}`
+		expected := `{"alerts":[{"text":"username and password are required","level":"error"}]}` + "\n"
 		if w.Body.String() != expected {
 			t.Error("Expected body", expected, "got", w.Body.String())
 		}
diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go
index ff6cfd9..6664ba2 100644
--- a/traffic_ops/traffic_ops_golang/routing/routes.go
+++ b/traffic_ops/traffic_ops_golang/routing/routes.go
@@ -186,7 +186,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
 		{1.1, http.MethodGet, `logs/newcount/?(\.json)?$`, logs.GetNewCount, auth.PrivLevelReadOnly, Authenticated, nil},
 
 		//HWInfo
-		{1.1, http.MethodGet, `hwinfo-wip/?(\.json)?$`, hwinfo.Get, auth.PrivLevelReadOnly, Authenticated, nil},
+		{1.1, http.MethodGet, `hwinfo/?(\.json)?$`, hwinfo.Get, auth.PrivLevelReadOnly, Authenticated, nil},
 
 		//Content invalidation jobs
 		{1.1, http.MethodGet, `jobs(/|\.json/?)?$`, api.ReadHandler(&invalidationjobs.InvalidationJob{}), auth.PrivLevelReadOnly, Authenticated, nil},
diff --git a/traffic_ops/traffic_ops_golang/routing/wrappers_test.go b/traffic_ops/traffic_ops_golang/routing/wrappers_test.go
index 9a2fef4..f2ffc4c 100644
--- a/traffic_ops/traffic_ops_golang/routing/wrappers_test.go
+++ b/traffic_ops/traffic_ops_golang/routing/wrappers_test.go
@@ -222,7 +222,7 @@ func TestWrapAuth(t *testing.T) {
 
 	f(w, r)
 
-	expectedError := `{"alerts":[{"text":"Unauthorized, please log in.","level":"error"}]}`
+	expectedError := `{"alerts":[{"text":"Unauthorized, please log in.","level":"error"}]}` + "\n"
 
 	if *debugLogging {
 		fmt.Printf("received: %s\n expected: %s\n", w.Body.Bytes(), expectedError)