From 42d301087664c201a6a40bebcd95e94f9d33f7d3 Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Mon, 16 May 2022 17:42:57 -0600 Subject: [PATCH 1/7] Added profileName as query param for APIv4 --- docs/source/api/v4/servers.rst | 2 +- traffic_ops/traffic_ops_golang/server/servers.go | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/source/api/v4/servers.rst b/docs/source/api/v4/servers.rst index 92bdbb6020..c12039de8b 100644 --- a/docs/source/api/v4/servers.rst +++ b/docs/source/api/v4/servers.rst @@ -51,7 +51,7 @@ Request Structure +----------------+----------+-------------------------------------------------------------------------------------------------------------------+ | id | no | Return only the server with this integral, unique identifier | +----------------+----------+-------------------------------------------------------------------------------------------------------------------+ - | profileId | no | Return only those servers that are using the :term:`Profile` that has this :ref:`profile-id` | + | profileName | no | Return only those servers that are using the :term:`Profile` that has this :ref:`profile-name` | +----------------+----------+-------------------------------------------------------------------------------------------------------------------+ | status | no | Return only those servers with this status - see :ref:`health-proto` | +----------------+----------+-------------------------------------------------------------------------------------------------------------------+ diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index 5c719e7fb0..2114c2106d 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -898,7 +898,6 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth "id": {Column: "s.id", Checker: api.IsInt}, "hostName": {Column: "s.host_name", Checker: nil}, "physLocation": {Column: "s.phys_location", Checker: api.IsInt}, - "profileId": {Column: "s.profile", Checker: api.IsInt}, "status": {Column: "st.name", Checker: nil}, "topology": {Column: "tc.topology", Checker: nil}, "type": {Column: "t.name", Checker: nil}, @@ -912,6 +911,20 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth } } + if version.Major <= 3 { + queryParamsToSQLCols["profileId"] = dbhelpers.WhereColumnInfo{ + Column: "s.profile", + Checker: api.IsInt, + } + } + + if version.Major >= 4 { + queryParamsToSQLCols["profileName"] = dbhelpers.WhereColumnInfo{ + Column: "p.name", + Checker: nil, + } + } + usesMids := false queryAddition := "" dsHasRequiredCapabilities := false From 71a5939dec9ce305a9e415d235cf7cb1c63cee58 Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Fri, 20 May 2022 09:14:17 -0600 Subject: [PATCH 2/7] Updated select query for APIv4 and APIv3 --- .../traffic_ops_golang/server/servers.go | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index 2114c2106d..e43bc96260 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -57,11 +57,16 @@ FROM server AS s JOIN cachegroup cg ON s.cachegroup = cg.id JOIN cdn cdn ON s.cdn_id = cdn.id JOIN phys_location pl ON s.phys_location = pl.id -JOIN profile p ON s.profile = p.id JOIN status st ON s.status = st.id JOIN type t ON s.type = t.id ` +const joinProfileV3 = `JOIN profile p ON s.profile = p.id +` + +const joinProfileV4 = `JOIN server_profile sp ON s.id = sp.server +` + /* language=SQL */ const dssTopologiesJoinSubquery = ` (SELECT @@ -920,7 +925,7 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth if version.Major >= 4 { queryParamsToSQLCols["profileName"] = dbhelpers.WhereColumnInfo{ - Column: "p.name", + Column: "sp.profile_name", Checker: nil, } } @@ -986,10 +991,18 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth return nil, 0, util.JoinErrs(errs), nil, http.StatusBadRequest, nil } - countQuery := serverCountQuery + queryAddition + where + var queryString, countQueryString string + if _, ok := params["profileName"]; ok { + queryString = selectQuery + joinProfileV4 + countQueryString = serverCountQuery + joinProfileV4 + } + queryString = selectQuery + joinProfileV3 + countQueryString = serverCountQuery + joinProfileV3 + + countQuery := countQueryString + queryAddition + where // If we are querying for a DS that has reqd capabilities, we need to make sure that we also include all the ORG servers directly assigned to this DS if _, ok := params["dsId"]; ok && dsHasRequiredCapabilities { - countQuery = `SELECT (` + countQuery + `) + (` + serverCountQuery + originServerQuery + `) AS total` + countQuery = `SELECT (` + countQuery + `) + (` + countQueryString + originServerQuery + `) AS total` } serverCount, err = getServerCount(tx, countQuery, queryValues) if err != nil { @@ -1008,10 +1021,10 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth log.Debugln("Non IMS request") } - query := selectQuery + queryAddition + where + orderBy + pagination + query := queryString + queryAddition + where + orderBy + pagination // If you're looking to get the servers for a particular delivery service, make sure you're also querying the ORG servers from the deliveryservice_server table if _, ok := params[`dsId`]; ok { - query = `(` + selectQuery + queryAddition + where + orderBy + pagination + `) UNION ` + selectQuery + originServerQuery + query = `(` + queryString + queryAddition + where + orderBy + pagination + `) UNION ` + queryString + originServerQuery } log.Debugln("Query is ", query) @@ -1214,7 +1227,7 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerV40, dsID int, cdnID filters["mid_ids"] = pq.Array(midIDs) // Query to select only those mids that match the required capabilities of the DS - query = selectQuery + midWhereClause + ` + query = selectQuery + joinProfileV3 + midWhereClause + ` AND s.id IN ( WITH capabilities AS ( SELECT ARRAY_AGG(ssc.server_capability), server @@ -1232,7 +1245,7 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerV40, dsID int, cdnID )` } else { // TODO: include secondary parent? - query = selectQuery + midWhereClause + query = selectQuery + joinProfileV3 + midWhereClause } if cdnID > 0 { @@ -1669,7 +1682,12 @@ func Update(w http.ResponseWriter, r *http.Request) { } where := `WHERE s.id = $1` - selquery := selectQuery + where + var selquery string + if version.Major <= 4 { + selquery = selectQuery + joinProfileV4 + where + } else { + selquery = selectQuery + joinProfileV3 + where + } var srvr tc.ServerV40 err = inf.Tx.QueryRow(selquery, serverID).Scan(&srvr.Cachegroup, &srvr.CachegroupID, @@ -1908,7 +1926,7 @@ func createV2(inf *api.APIInfo, w http.ResponseWriter, r *http.Request) { } where := `WHERE s.id = $1` - selquery := selectQuery + where + selquery := selectQuery + joinProfileV3 + where var s4 tc.ServerV40 err = inf.Tx.QueryRow(selquery, serverID).Scan(&s4.Cachegroup, &s4.CachegroupID, @@ -2049,7 +2067,7 @@ func createV3(inf *api.APIInfo, w http.ResponseWriter, r *http.Request) { } where := `WHERE s.id = $1` - selquery := selectQuery + where + selquery := selectQuery + joinProfileV3 + where var s4 tc.ServerV40 err = inf.Tx.QueryRow(selquery, serverID).Scan(&s4.Cachegroup, &s4.CachegroupID, @@ -2183,7 +2201,7 @@ func createV4(inf *api.APIInfo, w http.ResponseWriter, r *http.Request) { } where := `WHERE s.id = $1` - selquery := selectQuery + where + selquery := selectQuery + joinProfileV4 + where var srvr tc.ServerV40 err = inf.Tx.QueryRow(selquery, serverID).Scan(&srvr.Cachegroup, &srvr.CachegroupID, From c834c6d48006210e18e382614c7b4956e23f669a Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Fri, 20 May 2022 09:43:22 -0600 Subject: [PATCH 3/7] Shuffled variable --- traffic_ops/traffic_ops_golang/server/servers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index 18b7fb1d4c..2fe58faf6f 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -993,12 +993,12 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth } var queryString, countQueryString string + queryString = selectQuery + joinProfileV3 + countQueryString = serverCountQuery + joinProfileV3 if _, ok := params["profileName"]; ok { queryString = selectQuery + joinProfileV4 countQueryString = serverCountQuery + joinProfileV4 } - queryString = selectQuery + joinProfileV3 - countQueryString = serverCountQuery + joinProfileV3 countQuery := countQueryString + queryAddition + where // If we are querying for a DS that has reqd capabilities, we need to make sure that we also include all the ORG servers directly assigned to this DS From fbde9dfd908a7ec85e86b55223c798d32660abb8 Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Fri, 20 May 2022 09:51:21 -0600 Subject: [PATCH 4/7] Added changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47ecd946c3..b8e85c1d01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - Correction where using the placeholder __HOSTNAME__ in "unknown" files (others than the defaults ones), was being replaced by the full FQDN instead of the shot hostname. - [#6800](https://github.com/apache/trafficcontrol/issues/6800) Fixed incorrect error message for `/server/details` associated with query parameters. - [#6712](https://github.com/apache/trafficcontrol/issues/6712) - Fixed error when loading the Traffic Vault schema from `create_tables.sql` more than once. +- [#6834](https://github.com/apache/trafficcontrol/issues/6834) - In API 4.0, fixed `GET` for `/servers` to display all profiles irrespective of the index position. Also, replaced query param `profileId` with `profileName`. ### Removed - Remove traffic\_portal dependencies to mitigate `npm audit` issues, specifically `grunt-concurrent`, `grunt-contrib-concat`, `grunt-contrib-cssmin`, `grunt-contrib-jsmin`, `grunt-contrib-uglify`, `grunt-contrib-htmlmin`, `grunt-newer`, and `grunt-wiredep` From 5684f321816f51ca4824bec7f8ed00ea657db2c0 Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Fri, 20 May 2022 11:32:31 -0600 Subject: [PATCH 5/7] Updated test --- .../api/v4/cdn_queue_updates_by_type_profile_test.go | 2 +- traffic_ops/testing/api/v4/servers_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/traffic_ops/testing/api/v4/cdn_queue_updates_by_type_profile_test.go b/traffic_ops/testing/api/v4/cdn_queue_updates_by_type_profile_test.go index 74179ce5cf..1a1bccbdd9 100644 --- a/traffic_ops/testing/api/v4/cdn_queue_updates_by_type_profile_test.go +++ b/traffic_ops/testing/api/v4/cdn_queue_updates_by_type_profile_test.go @@ -175,7 +175,7 @@ func QueueUpdatesByProfile(t *testing.T) { // Get all the servers for the same CDN and profile as that of the first server opts.QueryParameters.Set("cdn", strconv.Itoa(cdns.Response[0].ID)) - opts.QueryParameters.Set("profileId", strconv.Itoa(profiles.Response[0].ID)) + opts.QueryParameters.Set("profileName", profiles.Response[0].Name) serverIDMap := make(map[int]bool, 0) resp, _, err := TOSession.GetServers(opts) if err != nil { diff --git a/traffic_ops/testing/api/v4/servers_test.go b/traffic_ops/testing/api/v4/servers_test.go index a6f1680a4d..af9adf9dd5 100644 --- a/traffic_ops/testing/api/v4/servers_test.go +++ b/traffic_ops/testing/api/v4/servers_test.go @@ -935,12 +935,12 @@ func GetTestServersQueryParameters(t *testing.T) { if len(pr.Response) != 1 { t.Error("Found server with no Profile ID") } else { - profileID := pr.Response[0].ID - opts.QueryParameters.Add("profileId", strconv.Itoa(profileID)) + profileName := pr.Response[0].Name + opts.QueryParameters.Add("profileName", profileName) if resp, _, err := TOSession.GetServers(opts); err != nil { t.Errorf("Error getting servers by Profile ID: %v - alerts: %+v", err, resp.Alerts) } - opts.QueryParameters.Del("profileId") + opts.QueryParameters.Del("profileName") } cgs, _, err := TOSession.GetCacheGroups(client.RequestOptions{}) From 4d4e039ef1b19aea068f6cf3c540d0de3d2a4155 Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Fri, 20 May 2022 15:24:02 -0600 Subject: [PATCH 6/7] Updated based on review comments --- docs/source/api/v4/servers.rst | 2 +- traffic_ops/traffic_ops_golang/server/servers.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/api/v4/servers.rst b/docs/source/api/v4/servers.rst index c12039de8b..78e5d1f397 100644 --- a/docs/source/api/v4/servers.rst +++ b/docs/source/api/v4/servers.rst @@ -51,7 +51,7 @@ Request Structure +----------------+----------+-------------------------------------------------------------------------------------------------------------------+ | id | no | Return only the server with this integral, unique identifier | +----------------+----------+-------------------------------------------------------------------------------------------------------------------+ - | profileName | no | Return only those servers that are using the :term:`Profile` that has this :ref:`profile-name` | + | profileName | no | Return only those servers that are using the :term:`Profile` that has this :ref:`profile-name` | +----------------+----------+-------------------------------------------------------------------------------------------------------------------+ | status | no | Return only those servers with this status - see :ref:`health-proto` | +----------------+----------+-------------------------------------------------------------------------------------------------------------------+ diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index 2fe58faf6f..2f59ca3ce4 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -995,7 +995,7 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth var queryString, countQueryString string queryString = selectQuery + joinProfileV3 countQueryString = serverCountQuery + joinProfileV3 - if _, ok := params["profileName"]; ok { + if version.Major >= 4 { queryString = selectQuery + joinProfileV4 countQueryString = serverCountQuery + joinProfileV4 } From 4906aef916330403ebd1b6415e940b25fcebeed7 Mon Sep 17 00:00:00 2001 From: Rima Shah Date: Fri, 20 May 2022 16:53:15 -0600 Subject: [PATCH 7/7] Updated select queries --- .../traffic_ops_golang/server/servers.go | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index 2f59ca3ce4..76040416da 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -57,14 +57,12 @@ FROM server AS s JOIN cachegroup cg ON s.cachegroup = cg.id JOIN cdn cdn ON s.cdn_id = cdn.id JOIN phys_location pl ON s.phys_location = pl.id +JOIN profile p ON s.profile = p.id JOIN status st ON s.status = st.id JOIN type t ON s.type = t.id ` -const joinProfileV3 = `JOIN profile p ON s.profile = p.id -` - -const joinProfileV4 = `JOIN server_profile sp ON s.id = sp.server +const joinProfileV4 = `JOIN server_profile sp ON p.name = sp.profile_name AND s.id = sp.server ` /* language=SQL */ @@ -993,10 +991,14 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth } var queryString, countQueryString string - queryString = selectQuery + joinProfileV3 - countQueryString = serverCountQuery + joinProfileV3 + queryString = selectQuery + countQueryString = serverCountQuery if version.Major >= 4 { - queryString = selectQuery + joinProfileV4 + if _, ok := params["profileName"]; ok { + queryString = selectQuery + `JOIN server_profile sp ON s.id = sp.server` + } else { + queryString = selectQuery + joinProfileV4 + } countQueryString = serverCountQuery + joinProfileV4 } @@ -1228,7 +1230,7 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerV40, dsID int, cdnID filters["mid_ids"] = pq.Array(midIDs) // Query to select only those mids that match the required capabilities of the DS - query = selectQuery + joinProfileV3 + midWhereClause + ` + query = selectQuery + midWhereClause + ` AND s.id IN ( WITH capabilities AS ( SELECT ARRAY_AGG(ssc.server_capability), server @@ -1246,7 +1248,7 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerV40, dsID int, cdnID )` } else { // TODO: include secondary parent? - query = selectQuery + joinProfileV3 + midWhereClause + query = selectQuery + midWhereClause } if cdnID > 0 { @@ -1699,7 +1701,7 @@ func Update(w http.ResponseWriter, r *http.Request) { if version.Major <= 4 { selquery = selectQuery + joinProfileV4 + where } else { - selquery = selectQuery + joinProfileV3 + where + selquery = selectQuery + where } var srvr tc.ServerV40 err = inf.Tx.QueryRow(selquery, serverID).Scan(&srvr.Cachegroup, @@ -1943,7 +1945,7 @@ func createV2(inf *api.APIInfo, w http.ResponseWriter, r *http.Request) { } where := `WHERE s.id = $1` - selquery := selectQuery + joinProfileV3 + where + selquery := selectQuery + where var s4 tc.ServerV40 err = inf.Tx.QueryRow(selquery, serverID).Scan(&s4.Cachegroup, &s4.CachegroupID, @@ -2088,7 +2090,7 @@ func createV3(inf *api.APIInfo, w http.ResponseWriter, r *http.Request) { } where := `WHERE s.id = $1` - selquery := selectQuery + joinProfileV3 + where + selquery := selectQuery + where var s4 tc.ServerV40 err = inf.Tx.QueryRow(selquery, serverID).Scan(&s4.Cachegroup, &s4.CachegroupID,