Repository: apache/trafficcontrol
Pull request #6784 (2022-04-26 16:44:37)

CodeQL workflow for Java
<!--
Thank you for contributing! Please be sure to read our contribution guidelines: https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md
If this closes or relates to an existing issue, please reference it using one of the following:

Closes: #ISSUE
Related: #ISSUE

If this PR fixes a security vulnerability, DO NOT submit! Instead, contact
the Apache Traffic Control Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://apache.org/security regarding vulnerability disclosure.
-->

This PR adds a CodeQL workflow for Java.
<!-- **^ Add meaningful description above** --><hr/>

## Which Traffic Control components are affected by this PR?
<!-- Please delete all components from this list that are NOT affected by this PR.
Feel free to add the name of a tool or script that is affected but not on the list.
-->
- Automation - GitHub Actions<!-- Please specify which (GitHub Actions, Docker images, Ansible Roles, etc.) -->

## What is the best way to verify this PR?
<!-- Please include here ALL the steps necessary to test your PR.
If your PR has tests (and most should), provide the steps needed to run the tests.
If not, please provide step-by-step instructions to test the PR manually and explain why your PR does not need tests. -->
1. Push the branch to the `master` branch of your Traffic Control fork
2. Check the outcome of the *CodeQL - Java* workflow
3. Check the *Security* tab of your fork for any possible Java issues (if there are none, that is fine)

## PR submission checklist
- [x] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
- [ ] This PR has documentation <!-- If not, please delete this text and explain why this PR does not need documentation. -->
- [ ] This PR has a CHANGELOG.md entry <!-- A fix for a bug from an ATC release, an improvement, or a new feature should have a changelog entry. -->
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://apache.org/security) for details)

<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements.  See the NOTICE file
distributed with this work for additional information
regarding copyright ownership.  The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License.  You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied.  See the License for the
specific language governing permissions and limitations
under the License.
-->


Patch:
diff --git a/.github/workflows/codeql.java.yml b/.github/workflows/codeql.java.yml
new file mode 100644
index 000000000..831616d05
--- /dev/null
+++ b/.github/workflows/codeql.java.yml
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: CodeQL - Java
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+    paths:
+      - .github/workflows/codeql.java.yml
+      - "**.java"
+      - "**/pom.xml"
+    types: [opened, reopened, ready_for_review, synchronize]
+
+jobs:
+  analyze:
+    name: Analyze Java
+    runs-on: ubuntu-latest
+    env:
+      BUILD_NUMBER: build_number
+      RHEL_VERSION: rhel_version
+      STARTUP_SCRIPT_DIR: /startup-dir
+      STARTUP_SCRIPT_LOC: ../core/src/main/lib/systemd/system
+      TOMCAT_RELEASE: tomcat_release
+      TOMCAT_VERSION: tomcat_version
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v2
+
+    # Initializes the CodeQL tools for scanning.
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v2
+      with:
+        languages: java
+
+    # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
+    # If this step fails, then you should remove it and run the build manually
+    - name: Autobuild
+      uses: github/codeql-action/autobuild@v2
+
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v2
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3aaee21ca..043163cbf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [Traffic Ops | Traffic Go Clients | T3C] Add additional timestamp fields to server for queuing and dequeueing config and revalidate updates.
 - Added layered profile feature to 4.0 for `GET` /servers/, `POST` /servers/, `PUT` /servers/{id} and `DELETE` /servers/{id}.
 - Added a Traffic Ops endpoint and Traffic Portal page to view all CDNi configuration update requests and approve or deny.
+- Added layered profile feature to 4.0 for `GET` /servers/details.
 - Added layered profile feature to 4.0 for `GET` /deliveryservices/{id}/servers/ and /deliveryservices/{id}/servers/eligible.
 
 ### Fixed
diff --git a/docs/source/admin/quick_howto/ciab.rst b/docs/source/admin/quick_howto/ciab.rst
index 3dbfa2a8f..428152472 100644
--- a/docs/source/admin/quick_howto/ciab.rst
+++ b/docs/source/admin/quick_howto/ciab.rst
@@ -200,7 +200,7 @@ Importing the :abbr:`CA (Certificate Authority)` Certificate on OSX
 #. The CIAB root :abbr:`CA (Certificate Authority)` certificate appears in login.
 #. Copy the CIAB root :abbr:`CA (Certificate Authority)` certificate to System.
 #. Open the CIAB root :abbr:`CA (Certificate Authority)` certificate, expand :guilabel:`Trust`, select :guilabel:`Use System Defaults`, and save your changes.
-#. Reopen the CIAB root :abbr:`CA (Certificate Authority)` certificate, expand :guilabel:`Trust, select :guilabel:`Always Trust`, and save your changes.
+#. Reopen the CIAB root :abbr:`CA (Certificate Authority)` certificate, expand :guilabel:`Trust`, select :guilabel:`Always Trust`, and save your changes.
 #. Delete the CIAB root :abbr:`CA (Certificate Authority)` certificate from login.
 #. Repeat the previous steps with the Intermediate :abbr:`CA (Certificate Authority)` certificate to import it as well
 #. Restart all HTTPS clients (browsers, etc).
@@ -308,7 +308,7 @@ The TightVNC optional container provides a basic lightweight window manager (flu
 			`"-f $PWD/docker-compose.yml "` \
 			`"-f $PWD/docker-compose.expose-ports.yml "` \
 			`"-f $PWD/optional/docker-compose.vnc.yml "` \
-			`"-f $PWD/optional/docker-compose.vnc.expose-ports.yml"
+			`"-f $PWD/optional/docker-compose.vnc.expose-ports.yml "`
 		docker volume prune -f
 		mydc build
 		mydc kill
diff --git a/docs/source/admin/traffic_ops.rst b/docs/source/admin/traffic_ops.rst
index 601391b50..d0b1d8412 100644
--- a/docs/source/admin/traffic_ops.rst
+++ b/docs/source/admin/traffic_ops.rst
@@ -682,7 +682,7 @@ You must then take the output file :file:`trafficops.csr` and submit a request t
 	sudo cp trafficops.crt /etc/pki/tls/certs
 	sudo chown trafops:trafops /etc/pki/tls/certs/trafficops.crt
 
-If necessary, install the :abbr:`CA (Certificate Authority) certificate's ``.pem`` and ``.crt`` files in ``/etc/pki/tls/certs``.
+If necessary, install the :abbr:`CA (Certificate Authority)` certificate's ``.pem`` and ``.crt`` files in ``/etc/pki/tls/certs``.
 
 You will need to update `cdn.conf`_ with any necessary changes.
 
diff --git a/docs/source/admin/traffic_server.rst b/docs/source/admin/traffic_server.rst
index 6cbe8a552..0b90f2a09 100644
--- a/docs/source/admin/traffic_server.rst
+++ b/docs/source/admin/traffic_server.rst
@@ -145,7 +145,7 @@ When processing configuration files, if ``traffic_ops_ort`` encounters any of th
 	| ``__SERVER_TCP_PORT__`` | If the :term:`cache server` on which ``t3c`` is being run has a    |
 	|                         | TCP port configured to something besides ``80``, this will be      |
 	|                         | replaced with that TCP port value. *If it* **is** *set to*         |
-	|                         | *``80``, this string will simply be removed,* **NOT** *replaced*   |
+	|                         | ``80``, *this string will simply be removed,* **NOT** *replaced*   |
 	|                         | *with* **ANYTHING**.                                               |
 	+-------------------------+--------------------------------------------------------------------+
 	| ``##OVERRIDE##``        | This string is only valid in the content of files named            |
diff --git a/docs/source/api/v3/steering.rst b/docs/source/api/v3/steering.rst
index 9a4924773..7695ebfe2 100644
--- a/docs/source/api/v3/steering.rst
+++ b/docs/source/api/v3/steering.rst
@@ -45,7 +45,7 @@ Response Structure
 ------------------
 :deliveryService:               A string that is the :ref:`ds-xmlid` of the steering :term:`Delivery Service`
 :clientSteering:                Whether this is a :ref:`client steering <ds-client-steering>` Delivery Service.
-:targets:                       The delivery services that the :ref`Steering Delivery Service <tr-steering>` targets.
+:targets:                       The delivery services that the :ref:`Steering Delivery Service <tr-steering>` targets.
 
 	:order:                 If this is a :ref:`STEERING_ORDER <ds-steering-order>` target, this is the value of the order. Otherwise, ``0``.
 	:weight:                If this is a :ref:`STEERING_WEIGHT <ds-steering-weight>` target, this is the value of the weight. Otherwise, ``0``.
diff --git a/docs/source/api/v4/servers_details.rst b/docs/source/api/v4/servers_details.rst
index afc95ed6a..dc0a917b5 100644
--- a/docs/source/api/v4/servers_details.rst
+++ b/docs/source/api/v4/servers_details.rst
@@ -98,8 +98,7 @@ Response Structure
 	:mgmtIpNetmask:  The IPv4 subnet mask of the server's management port
 	:offlineReason:         A user-entered reason why the server is in ADMIN_DOWN or OFFLINE status
 	:physLocation:          The name of the physical location where the server resides
-	:profile:               The :ref:`profile-name` of the :term:`Profile` used by this server
-	:profileDesc:           A :ref:`profile-description` of the :term:`Profile` used by this server
+	:profileNames:          List of :ref:`profile-name` of the :term:`Profiles` used by this server
 	:rack:  A string indicating "server rack" location
 	:status:                The status of the server
 
@@ -157,8 +156,7 @@ Response Structure
 				"mgmtIpNetmask": "",
 				"offlineReason": "",
 				"physLocation": "Apachecon North America 2018",
-				"profile": "ATS_EDGE_TIER_CACHE",
-				"profileDesc": "Edge Cache - Apache Traffic Server",
+				"profileNames": ["ATS_EDGE_TIER_CACHE"],
 				"rack": "",
 				"status": "REPORTED",
 				"tcpPort": 80,
diff --git a/docs/source/api/v4/steering.rst b/docs/source/api/v4/steering.rst
index e367fe744..8c36fe463 100644
--- a/docs/source/api/v4/steering.rst
+++ b/docs/source/api/v4/steering.rst
@@ -46,7 +46,7 @@ Response Structure
 ------------------
 :deliveryService:               A string that is the :ref:`ds-xmlid` of the steering :term:`Delivery Service`
 :clientSteering:                Whether this is a :ref:`client steering <ds-client-steering>` Delivery Service.
-:targets:                       The delivery services that the :ref`Steering Delivery Service <tr-steering>` targets.
+:targets:                       The delivery services that the :ref:`Steering Delivery Service <tr-steering>` targets.
 
 	:order:                 If this is a :ref:`STEERING_ORDER <ds-steering-order>` target, this is the value of the order. Otherwise, ``0``.
 	:weight:                If this is a :ref:`STEERING_WEIGHT <ds-steering-weight>` target, this is the value of the weight. Otherwise, ``0``.
diff --git a/docs/source/development/traffic_ops.rst b/docs/source/development/traffic_ops.rst
index 51174f807..68c5eda12 100644
--- a/docs/source/development/traffic_ops.rst
+++ b/docs/source/development/traffic_ops.rst
@@ -478,7 +478,7 @@ The integration tests are run using :manpage:`go-test(1)`, with two configuratio
 
 	Specify the path to a file containing static data for the tests to use. This should almost never be used, because many of the tests depend on the data having a certain content and structure. If not specified, it will attempt to read a file named ``tc-fixtures.json`` in the working directory.
 
-.. option:: --includeSystemTests ``no``/``yes``
+.. option:: --includeSystemTests {no|yes}
 
 	Specify whether to run tests that depend on additional components like an SMTP server or a Traffic Vault server. Default: ``no``
 
diff --git a/docs/source/glossary.rst b/docs/source/glossary.rst
index 84c564fb0..9e89396f7 100644
--- a/docs/source/glossary.rst
+++ b/docs/source/glossary.rst
@@ -131,7 +131,7 @@ Glossary
 		.. seealso:: See :ref:`delivery-service-requests` for a more in-depth explanation of :dfn:`Delivery Service Requests`, including their data model. See :ref:`ds_requests` for more information on how to use :dfn:`Delivery Service Requests` in Traffic Portal.
 
 	Delivery Service required capabilities
-		:dfn:`Delivery Services required capabilities` are capabilities, which correlate to server capabilities, that are required in order to assign a server to a delivery service.`
+		:dfn:`Delivery Services required capabilities` are capabilities, which correlate to server capabilities, that are required in order to assign a server to a delivery service.
 
 	Division
 	Divisions
diff --git a/docs/source/overview/delivery_services.rst b/docs/source/overview/delivery_services.rst
index 50c7d000d..8c7932fa1 100644
--- a/docs/source/overview/delivery_services.rst
+++ b/docs/source/overview/delivery_services.rst
@@ -295,7 +295,7 @@ Geo Limit Redirect URL
 If `Geo Limit`_ is being used with this Delivery Service, this is optionally a URL to which clients will be redirected when Traffic Router determines that they are not in a geographic zone that permits their access to the Delivery Service content. This changes the response from Traffic Router from ``503 Service Unavailable`` to ``302 Found`` with a provided location that will be this URL. There is no restriction on the provided URL; it may even be the path to a resource served by this Delivery Service. In fact, this field need not even be a full URL, it can be a relative path. Both of these cases are handled specially by Traffic Router.
 
 - If the provided URL is a resource served by the Delivery Service (e.g. if the client requests ``http://cdn.dsXMLID.somedomain.example.com/index.html`` but are denied access by `Geo Limit`_ and the Geo Limit Redirect URL is something like ``http://cdn.dsXMLID.somedomain.example.com/help.php``), Traffic Router will find an appropriate :term:`Edge-tier cache server` and redirect the client, ignoring Geo Limit restrictions *for this request only*.
-- If the provided "URL" is actually a relative path, it will be considered *relative to the requested Delivery Service :abbr:`FQDN (Fully Qualified Domain Name)`*. This means that e.g. if the client requests ``http://cdn.dsXMLID.somedomain.example.com/index.html`` but are denied access by `Geo Limit`_ and the Geo Limit Redirect URL is something like ``/help.php``, Traffic Router will find an appropriate :term:`Edge-tier cache server` and redirect the client to it as though they had requested ``http://cdn.dsXMLID.somedomain.example.com/help.php``, ignoring `Geo Limit`_ restrictions *for this request only*.
+- If the provided "URL" is actually a relative path, it will be considered *relative to the requested Delivery Service* :abbr:`FQDN (Fully Qualified Domain Name)`. This means that e.g. if the client requests ``http://cdn.dsXMLID.somedomain.example.com/index.html`` but are denied access by `Geo Limit`_ and the Geo Limit Redirect URL is something like ``/help.php``, Traffic Router will find an appropriate :term:`Edge-tier cache server` and redirect the client to it as though they had requested ``http://cdn.dsXMLID.somedomain.example.com/help.php``, ignoring `Geo Limit`_ restrictions *for this request only*.
 
 .. table:: Aliases
 
diff --git a/lib/go-tc/servers.go b/lib/go-tc/servers.go
index 23971316b..2f435cdb3 100644
--- a/lib/go-tc/servers.go
+++ b/lib/go-tc/servers.go
@@ -87,8 +87,33 @@ type ServerDetailV30 struct {
 
 // ServerDetailV40 is the details for a server for API v4.
 type ServerDetailV40 struct {
-	ServerDetail
-	ServerInterfaces []ServerInterfaceInfoV40 `json:"interfaces"`
+	CacheGroup         *string                  `json:"cachegroup" db:"cachegroup"`
+	CDNName            *string                  `json:"cdnName" db:"cdn_name"`
+	DeliveryServiceIDs []int64                  `json:"deliveryservices,omitempty"`
+	DomainName         *string                  `json:"domainName" db:"domain_name"`
+	GUID               *string                  `json:"guid" db:"guid"`
+	HardwareInfo       map[string]string        `json:"hardwareInfo"`
+	HostName           *string                  `json:"hostName" db:"host_name"`
+	HTTPSPort          *int                     `json:"httpsPort" db:"https_port"`
+	ID                 *int                     `json:"id" db:"id"`
+	ILOIPAddress       *string                  `json:"iloIpAddress" db:"ilo_ip_address"`
+	ILOIPGateway       *string                  `json:"iloIpGateway" db:"ilo_ip_gateway"`
+	ILOIPNetmask       *string                  `json:"iloIpNetmask" db:"ilo_ip_netmask"`
+	ILOPassword        *string                  `json:"iloPassword" db:"ilo_password"`
+	ILOUsername        *string                  `json:"iloUsername" db:"ilo_username"`
+	MgmtIPAddress      *string                  `json:"mgmtIpAddress" db:"mgmt_ip_address"`
+	MgmtIPGateway      *string                  `json:"mgmtIpGateway" db:"mgmt_ip_gateway"`
+	MgmtIPNetmask      *string                  `json:"mgmtIpNetmask" db:"mgmt_ip_netmask"`
+	OfflineReason      *string                  `json:"offlineReason" db:"offline_reason"`
+	PhysLocation       *string                  `json:"physLocation" db:"phys_location"`
+	ProfileNames       []string                 `json:"profileNames" db:"profile_name"`
+	Rack               *string                  `json:"rack" db:"rack"`
+	Status             *string                  `json:"status" db:"status"`
+	TCPPort            *int                     `json:"tcpPort" db:"tcp_port"`
+	Type               string                   `json:"type" db:"server_type"`
+	XMPPID             *string                  `json:"xmppId" db:"xmpp_id"`
+	XMPPPasswd         *string                  `json:"xmppPasswd" db:"xmpp_passwd"`
+	ServerInterfaces   []ServerInterfaceInfoV40 `json:"interfaces"`
 }
 
 // ServersV1DetailResponse is the JSON object returned for a single server for v1.
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index e024126d0..dc029086b 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -2091,6 +2091,43 @@ func UpdateServerProfileTableForV2V3(id *int, newProfile *string, origProfile st
 	return profileName, nil
 }
 
+// GetServerDetailFromV4 function converts server details from V4 to V3/V2
+func GetServerDetailFromV4(sd tc.ServerDetailV40, tx *sql.Tx) (tc.ServerDetail, error) {
+	var profileDesc *string
+	if err := tx.QueryRow(`SELECT p.description FROM profile p WHERE p.name=$1`, sd.ProfileNames[0]).Scan(&profileDesc); err != nil {
+		return tc.ServerDetail{}, fmt.Errorf("querying profile description by profile name: %w", err)
+	}
+	return tc.ServerDetail{
+		CacheGroup:         sd.CacheGroup,
+		CDNName:            sd.CDNName,
+		DeliveryServiceIDs: sd.DeliveryServiceIDs,
+		DomainName:         sd.DomainName,
+		GUID:               sd.GUID,
+		HardwareInfo:       sd.HardwareInfo,
+		HostName:           sd.HostName,
+		HTTPSPort:          sd.HTTPSPort,
+		ID:                 sd.ID,
+		ILOIPAddress:       sd.ILOIPAddress,
+		ILOIPGateway:       sd.ILOIPGateway,
+		ILOIPNetmask:       sd.ILOIPNetmask,
+		ILOPassword:        sd.ILOPassword,
+		ILOUsername:        sd.ILOUsername,
+		MgmtIPAddress:      sd.MgmtIPAddress,
+		MgmtIPGateway:      sd.MgmtIPGateway,
+		MgmtIPNetmask:      sd.MgmtIPNetmask,
+		OfflineReason:      sd.OfflineReason,
+		PhysLocation:       sd.PhysLocation,
+		Profile:            &sd.ProfileNames[0],
+		ProfileDesc:        profileDesc,
+		Rack:               sd.Rack,
+		Status:             sd.Status,
+		TCPPort:            sd.TCPPort,
+		Type:               sd.Type,
+		XMPPID:             sd.XMPPID,
+		XMPPPasswd:         sd.XMPPPasswd,
+	}, nil
+}
+
 // GetProfileIDDesc gets profile ID and desc for V3 servers
 func GetProfileIDDesc(tx *sql.Tx, name string) (id int, desc string) {
 	err := tx.QueryRow(`SELECT id, description from "profile" p WHERE p.name=$1`, name).Scan(&id, &desc)
diff --git a/traffic_ops/traffic_ops_golang/server/detail.go b/traffic_ops/traffic_ops_golang/server/detail.go
index 7f2eb9b1f..aa45dd955 100644
--- a/traffic_ops/traffic_ops_golang/server/detail.go
+++ b/traffic_ops/traffic_ops_golang/server/detail.go
@@ -91,7 +91,11 @@ func GetDetailParamHandler(w http.ResponseWriter, r *http.Request) {
 				routerPortName = interfaces[0].RouterPortName
 			}
 			v11server := tc.ServerDetailV11{}
-			v11server.ServerDetail = server.ServerDetail
+			v11server.ServerDetail, err = dbhelpers.GetServerDetailFromV4(server, inf.Tx.Tx)
+			if err != nil {
+				api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("failed to GetServerDetailFromV4: %w", err))
+				return
+			}
 			v11server.RouterHostName = &routerHostName
 			v11server.RouterPortName = &routerPortName
 			legacyInterface, err := tc.V4InterfaceInfoToLegacyInterfaces(interfaces)
@@ -117,7 +121,11 @@ func GetDetailParamHandler(w http.ResponseWriter, r *http.Request) {
 				routerHostName = interfaces[0].RouterHostName
 				routerPortName = interfaces[0].RouterPortName
 			}
-			v3Server.ServerDetail = server.ServerDetail
+			v3Server.ServerDetail, err = dbhelpers.GetServerDetailFromV4(server, inf.Tx.Tx)
+			if err != nil {
+				api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("failed to GetServerDetailFromV4: %w", err))
+				return
+			}
 			v3Server.RouterHostName = &routerHostName
 			v3Server.RouterPortName = &routerPortName
 			v3Interfaces, err := tc.V4InterfaceInfoToV3Interfaces(interfaces)
@@ -209,8 +217,7 @@ server.mgmt_ip_gateway,
 server.mgmt_ip_netmask,
 server.offline_reason,
 pl.name as phys_location,
-p.name as profile,
-p.description as profile_desc,
+(SELECT ARRAY_AGG(profile_name) FROM server_profile WHERE server_profile.server=server.id) AS profile_name,
 server.rack,
 st.name as status,
 server.tcp_port,
@@ -275,7 +282,38 @@ JOIN type t ON server.type = t.id
 
 	for rows.Next() {
 		s := tc.ServerDetailV40{}
-		if err := rows.Scan(&s.ID, &s.CacheGroup, &s.CDNName, pq.Array(&s.DeliveryServiceIDs), &s.DomainName, &s.GUID, &s.HostName, &s.HTTPSPort, &s.ILOIPAddress, &s.ILOIPGateway, &s.ILOIPNetmask, &s.ILOPassword, &s.ILOUsername, &serviceAddress, &service6Address, &serviceGateway, &service6Gateway, &serviceNetmask, &serviceInterface, &serviceMtu, &s.MgmtIPAddress, &s.MgmtIPGateway, &s.MgmtIPNetmask, &s.OfflineReason, &s.PhysLocation, &s.Profile, &s.ProfileDesc, &s.Rack, &s.Status, &s.TCPPort, &s.Type, &s.XMPPID, &s.XMPPPasswd); err != nil {
+		if err := rows.Scan(&s.ID,
+			&s.CacheGroup,
+			&s.CDNName,
+			pq.Array(&s.DeliveryServiceIDs),
+			&s.DomainName,
+			&s.GUID,
+			&s.HostName,
+			&s.HTTPSPort,
+			&s.ILOIPAddress,
+			&s.ILOIPGateway,
+			&s.ILOIPNetmask,
+			&s.ILOPassword,
+			&s.ILOUsername,
+			&serviceAddress,
+			&service6Address,
+			&serviceGateway,
+			&service6Gateway,
+			&serviceNetmask,
+			&serviceInterface,
+			&serviceMtu,
+			&s.MgmtIPAddress,
+			&s.MgmtIPGateway,
+			&s.MgmtIPNetmask,
+			&s.OfflineReason,
+			&s.PhysLocation,
+			pq.Array(&s.ProfileNames),
+			&s.Rack,
+			&s.Status,
+			&s.TCPPort,
+			&s.Type,
+			&s.XMPPID,
+			&s.XMPPPasswd); err != nil {
 			return nil, errors.New("Error scanning detail server: " + err.Error())
 		}
 		s.ServerInterfaces = []tc.ServerInterfaceInfoV40{}


Test patch:
diff --git a/traffic_ops/testing/api/v3/deliveryservice_request_comments_test.go b/traffic_ops/testing/api/v3/deliveryservice_request_comments_test.go
index f3aab587d..51dae401d 100644
--- a/traffic_ops/testing/api/v3/deliveryservice_request_comments_test.go
+++ b/traffic_ops/testing/api/v3/deliveryservice_request_comments_test.go
@@ -16,6 +16,7 @@ package v3
 */
 
 import (
+	"encoding/json"
 	"net/http"
 	"sort"
 	"testing"
@@ -23,182 +24,176 @@ import (
 
 	"github.com/apache/trafficcontrol/lib/go-rfc"
 	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/assert"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/utils"
+	"github.com/apache/trafficcontrol/traffic_ops/toclientlib"
 )
 
 func TestDeliveryServiceRequestComments(t *testing.T) {
 	WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants, DeliveryServiceRequests, DeliveryServiceRequestComments}, func() {
-		GetTestDeliveryServiceRequestCommentsIMS(t)
-		currentTime := time.Now().UTC().Add(-5 * time.Second)
-		time := currentTime.Format(time.RFC1123)
-		var header http.Header
-		header = make(map[string][]string)
-		header.Set(rfc.IfUnmodifiedSince, time)
-		header.Set(rfc.IfModifiedSince, time)
-		SortTestDeliveryServiceRequestComments(t)
-		UpdateTestDeliveryServiceRequestComments(t)
-		UpdateTestDeliveryServiceRequestCommentsWithHeaders(t, header)
-		header = make(map[string][]string)
-		etag := rfc.ETag(currentTime)
-		header.Set(rfc.IfMatch, etag)
-		UpdateTestDeliveryServiceRequestCommentsWithHeaders(t, header)
-		GetTestDeliveryServiceRequestComments(t)
-		GetTestDeliveryServiceRequestCommentsIMSAfterChange(t, header)
-	})
-}
-
-func UpdateTestDeliveryServiceRequestCommentsWithHeaders(t *testing.T, header http.Header) {
-	comments, _, _ := TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
 
-	if len(comments) > 0 {
-		firstComment := comments[0]
-		newFirstCommentValue := "new comment value"
-		firstComment.Value = newFirstCommentValue
-
-		_, reqInf, err := TOSession.UpdateDeliveryServiceRequestCommentByIDWithHdr(firstComment.ID, firstComment, header)
-		if err == nil {
-			t.Errorf("expected precondition failed error, but got none")
-		}
-		if reqInf.StatusCode != http.StatusPreconditionFailed {
-			t.Errorf("Expected status code 412, got %v", reqInf.StatusCode)
+		currentTime := time.Now().UTC().Add(-15 * time.Second)
+		currentTimeRFC := currentTime.Format(time.RFC1123)
+		tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
+
+		methodTests := utils.V3TestCase{
+			"GET": {
+				"NOT MODIFIED when NO CHANGES made": {
+					ClientSession:  TOSession,
+					RequestHeaders: http.Header{rfc.IfModifiedSince: {tomorrow}},
+					Expectations:   utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)),
+				},
+				"OK when VALID request": {
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"OK when VALID ID parameter": {
+					EndpointId:    GetDSRequestCommentId(t),
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1)),
+				},
+				"VALIDATE SORT when DEFAULT is ASC ORDER": {
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateSortedDSRequestComments()),
+				},
+			},
+			"PUT": {
+				"OK when VALID request": {
+					EndpointId:    GetDSRequestCommentId(t),
+					ClientSession: TOSession,
+					RequestBody: map[string]interface{}{
+						"deliveryServiceRequestId": GetDSRequestId(t, "test-ds1")(),
+						"value":                    "updated comment",
+					},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"PRECONDITION FAILED when updating with IF-UNMODIFIED-SINCE Header": {
+					EndpointId:     GetDSRequestCommentId(t),
+					ClientSession:  TOSession,
+					RequestHeaders: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}},
+					RequestBody:    map[string]interface{}{},
+					Expectations:   utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+				"PRECONDITION FAILED when updating with IFMATCH ETAG Header": {
+					EndpointId:     GetDSRequestCommentId(t),
+					ClientSession:  TOSession,
+					RequestBody:    map[string]interface{}{},
+					RequestHeaders: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}},
+					Expectations:   utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+			},
+			"GET AFTER CHANGES": {
+				"OK when CHANGES made": {
+					ClientSession:  TOSession,
+					RequestHeaders: http.Header{rfc.IfModifiedSince: {currentTimeRFC}},
+					Expectations:   utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+			},
 		}
-	}
-}
 
-func GetTestDeliveryServiceRequestCommentsIMSAfterChange(t *testing.T, header http.Header) {
-	_, reqInf, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
-	if err != nil {
-		t.Fatalf("could not GET delivery service request comments: %v", err)
-	}
-	if reqInf.StatusCode != http.StatusOK {
-		t.Fatalf("Expected 200 status code, got %v", reqInf.StatusCode)
-	}
-	header = make(map[string][]string)
-	futureTime := time.Now().AddDate(0, 0, 1)
-	time := futureTime.Format(time.RFC1123)
-	header.Set(rfc.IfModifiedSince, time)
-	_, reqInf, err = TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
-	if err != nil {
-		t.Fatalf("could not GET delivery service request comments: %v", err)
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
-	}
-}
-
-func CreateTestDeliveryServiceRequestComments(t *testing.T) {
-
-	// Retrieve a delivery service request by xmlId so we can get the ID needed to create a dsr comment
-	dsr := testData.DeliveryServiceRequests[0].DeliveryService
-
-	resp, _, err := TOSession.GetDeliveryServiceRequestByXMLID(dsr.XMLID)
-	if err != nil {
-		t.Errorf("cannot GET delivery service request by xml id: %v - %v", dsr.XMLID, err)
-	}
-	if len(resp) != 1 {
-		t.Errorf("found %d delivery service request by xml id, expected %d: %s", len(resp), 1, dsr.XMLID)
-	} else {
-		respDSR := resp[0]
-
-		for _, comment := range testData.DeliveryServiceRequestComments {
-			comment.DeliveryServiceRequestID = respDSR.ID
-			resp, _, err := TOSession.CreateDeliveryServiceRequestComment(comment)
-			if err != nil {
-				t.Errorf("could not CREATE delivery service request comment: %v - %v", err, resp)
-			}
+		for method, testCases := range methodTests {
+			t.Run(method, func(t *testing.T) {
+				for name, testCase := range testCases {
+					comment := tc.DeliveryServiceRequestComment{}
+
+					if testCase.RequestBody != nil {
+						dat, err := json.Marshal(testCase.RequestBody)
+						assert.NoError(t, err, "Error occurred when marshalling request body: %v", err)
+						err = json.Unmarshal(dat, &comment)
+						assert.NoError(t, err, "Error occurred when unmarshalling request body: %v", err)
+					}
+
+					switch method {
+					case "GET", "GET AFTER CHANGES":
+						t.Run(name, func(t *testing.T) {
+							if name == "OK when VALID ID parameter" {
+								resp, reqInf, err := testCase.ClientSession.GetDeliveryServiceRequestCommentByIDWithHdr(testCase.EndpointId(), testCase.RequestHeaders)
+								for _, check := range testCase.Expectations {
+									check(t, reqInf, resp, tc.Alerts{}, err)
+								}
+							} else {
+								resp, reqInf, err := testCase.ClientSession.GetDeliveryServiceRequestCommentsWithHdr(testCase.RequestHeaders)
+								for _, check := range testCase.Expectations {
+									check(t, reqInf, resp, tc.Alerts{}, err)
+								}
+							}
+						})
+					case "PUT":
+						t.Run(name, func(t *testing.T) {
+							alerts, reqInf, err := testCase.ClientSession.UpdateDeliveryServiceRequestCommentByIDWithHdr(testCase.EndpointId(), comment, testCase.RequestHeaders)
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, nil, alerts, err)
+							}
+						})
+					}
+				}
+			})
 		}
-	}
-
+	})
 }
 
-func SortTestDeliveryServiceRequestComments(t *testing.T) {
-	var header http.Header
-	var sortedList []string
-	resp, _, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
-	if err != nil {
-		t.Fatalf("Expected no error, but got %v", err.Error())
-	}
-	for i, _ := range resp {
-		sortedList = append(sortedList, resp[i].XMLID)
-	}
+func GetDSRequestCommentId(t *testing.T) func() int {
+	return func() int {
+		resp, _, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(http.Header{})
+		assert.RequireNoError(t, err, "Get Delivery Service Request Comments failed with error: %v", err)
+		assert.RequireGreaterOrEqual(t, len(resp), 1, "Expected delivery service request comments response object length of atleast 1, but got %d", len(resp))
+		assert.RequireNotNil(t, resp[0].ID, "Expected id to not be nil")
 
-	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
-		return sortedList[p] < sortedList[q]
-	})
-	if res != true {
-		t.Errorf("list is not sorted by their names: %v", sortedList)
+		return resp[0].ID
 	}
 }
 
-func UpdateTestDeliveryServiceRequestComments(t *testing.T) {
-
-	comments, _, err := TOSession.GetDeliveryServiceRequestComments()
+func GetDSRequestId(t *testing.T, xmlId string) func() int {
+	return func() int {
+		resp, _, err := TOSession.GetDeliveryServiceRequestByXMLIDWithHdr(xmlId, http.Header{})
+		assert.RequireNoError(t, err, "Get Delivery Service Requests failed with error: %v", err)
+		assert.RequireGreaterOrEqual(t, len(resp), 1, "Expected delivery service requests response object length of atleast 1, but got %d", len(resp))
+		assert.RequireNotNil(t, resp[0].ID, "Expected id to not be nil")
 
-	firstComment := comments[0]
-	newFirstCommentValue := "new comment value"
-	firstComment.Value = newFirstCommentValue
-
-	var alert tc.Alerts
-	alert, _, err = TOSession.UpdateDeliveryServiceRequestCommentByID(firstComment.ID, firstComment)
-	if err != nil {
-		t.Errorf("cannot UPDATE delivery service request comment by id: %v - %v", err, alert)
+		return resp[0].ID
 	}
+}
 
-	// Retrieve the delivery service request comment to check that the value got updated
-	resp, _, err := TOSession.GetDeliveryServiceRequestCommentByID(firstComment.ID)
-	if err != nil {
-		t.Errorf("cannot GET delivery service request comment by id: '$%d', %v", firstComment.ID, err)
-	}
-	respDSRC := resp[0]
-	if respDSRC.Value != newFirstCommentValue {
-		t.Errorf("results do not match actual: %s, expected: %s", respDSRC.Value, newFirstCommentValue)
-	}
+func validateSortedDSRequestComments() utils.CkReqFunc {
+	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, err error) {
+		var sortedList []string
+		dsReqComments := resp.([]tc.DeliveryServiceRequestComment)
 
-}
+		for _, comment := range dsReqComments {
+			sortedList = append(sortedList, comment.XMLID)
+		}
 
-func GetTestDeliveryServiceRequestCommentsIMS(t *testing.T) {
-	var header http.Header
-	header = make(map[string][]string)
-	futureTime := time.Now().AddDate(0, 0, 1)
-	time := futureTime.Format(time.RFC1123)
-	header.Set(rfc.IfModifiedSince, time)
-	_, reqInf, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(header)
-	if err != nil {
-		t.Fatalf("could not GET delivery service request comments: %v", err)
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
+		res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+			return sortedList[p] < sortedList[q]
+		})
+		assert.Equal(t, res, true, "List is not sorted by their names: %v", sortedList)
 	}
 }
 
-func GetTestDeliveryServiceRequestComments(t *testing.T) {
-
-	comments, _, _ := TOSession.GetDeliveryServiceRequestComments()
+func CreateTestDeliveryServiceRequestComments(t *testing.T) {
+	for _, comment := range testData.DeliveryServiceRequestComments {
+		resp, _, err := TOSession.GetDeliveryServiceRequestByXMLIDWithHdr(comment.XMLID, http.Header{})
+		assert.NoError(t, err, "Cannot get Delivery Service Request by XMLID '%s': %v", comment.XMLID, err)
+		assert.Equal(t, len(resp), 1, "Found %d Delivery Service request by XMLID '%s, expected exactly one", len(resp), comment.XMLID)
+		assert.NotNil(t, resp[0].ID, "Got Delivery Service Request with xml_id '%s' that had a null ID", comment.XMLID)
 
-	for _, comment := range comments {
-		resp, _, err := TOSession.GetDeliveryServiceRequestCommentByID(comment.ID)
-		if err != nil {
-			t.Errorf("cannot GET delivery service request comment by id: %v - %v", err, resp)
-		}
+		comment.DeliveryServiceRequestID = resp[0].ID
+		alerts, _, err := TOSession.CreateDeliveryServiceRequestComment(comment)
+		assert.NoError(t, err, "Could not create Delivery Service Request Comment: %v - alerts: %+v", err, alerts.Alerts)
 	}
 }
 
 func DeleteTestDeliveryServiceRequestComments(t *testing.T) {
+	resp, _, err := TOSession.GetDeliveryServiceRequestCommentsWithHdr(http.Header{})
+	assert.NoError(t, err, "Unexpected error getting Delivery Service Request Comments: %v", err)
 
-	comments, _, _ := TOSession.GetDeliveryServiceRequestComments()
-
-	for _, comment := range comments {
-		_, _, err := TOSession.DeleteDeliveryServiceRequestCommentByID(comment.ID)
-		if err != nil {
-			t.Errorf("cannot DELETE delivery service request comment by id: '%d' %v", comment.ID, err)
-		}
+	for _, comment := range resp {
+		alerts, _, err := TOSession.DeleteDeliveryServiceRequestCommentByID(comment.ID)
+		assert.NoError(t, err, "Cannot delete Delivery Service Request Comment #%d: %v - alerts: %+v", comment.ID, err, alerts.Alerts)
 
 		// Retrieve the delivery service request comment to see if it got deleted
-		comments, _, err := TOSession.GetDeliveryServiceRequestCommentByID(comment.ID)
-		if err != nil {
-			t.Errorf("error deleting delivery service request comment: %s", err.Error())
-		}
-		if len(comments) > 0 {
-			t.Errorf("expected delivery service request comment: %d to be deleted", comment.ID)
-		}
+		resp, _, err := TOSession.GetDeliveryServiceRequestCommentByIDWithHdr(comment.ID, http.Header{})
+		assert.NoError(t, err, "Unexpected error fetching Delivery Service Request Comment %d after deletion: %v", comment.ID, err)
+		assert.Equal(t, len(resp), 0, "Expected Delivery Service Request Comment #%d to be deleted, but it was found in Traffic Ops", comment.ID)
 	}
 }
diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json b/traffic_ops/testing/api/v3/tc-fixtures.json
index ab8616ae4..7c2f2994f 100644
--- a/traffic_ops/testing/api/v3/tc-fixtures.json
+++ b/traffic_ops/testing/api/v3/tc-fixtures.json
@@ -281,16 +281,20 @@
     ],
     "deliveryServiceRequestComments": [
         {
-            "value": "this is comment one"
+            "value": "this is comment one",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment two"
+            "value": "this is comment two",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment three"
+            "value": "this is comment three",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment four"
+            "value": "this is comment four",
+            "xmlId": "test-ds1"
         }
     ],
     "deliveryServiceRequests": [
diff --git a/traffic_ops/testing/api/v4/deliveryservice_request_comments_test.go b/traffic_ops/testing/api/v4/deliveryservice_request_comments_test.go
index dd5c41080..512289661 100644
--- a/traffic_ops/testing/api/v4/deliveryservice_request_comments_test.go
+++ b/traffic_ops/testing/api/v4/deliveryservice_request_comments_test.go
@@ -16,7 +16,9 @@ package v4
 */
 
 import (
+	"encoding/json"
 	"net/http"
+	"net/url"
 	"sort"
 	"strconv"
 	"testing"
@@ -24,220 +26,180 @@ import (
 
 	"github.com/apache/trafficcontrol/lib/go-rfc"
 	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/assert"
+	"github.com/apache/trafficcontrol/traffic_ops/testing/api/utils"
+	"github.com/apache/trafficcontrol/traffic_ops/toclientlib"
 	client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
 )
 
 func TestDeliveryServiceRequestComments(t *testing.T) {
 	WithObjs(t, []TCObj{CDNs, Types, Parameters, Tenants, DeliveryServiceRequests, DeliveryServiceRequestComments}, func() {
-		GetTestDeliveryServiceRequestCommentsIMS(t)
-		currentTime := time.Now().UTC().Add(-5 * time.Second)
-		time := currentTime.Format(time.RFC1123)
-		var header http.Header
-		header = make(map[string][]string)
-		header.Set(rfc.IfUnmodifiedSince, time)
-		header.Set(rfc.IfModifiedSince, time)
-		SortTestDeliveryServiceRequestComments(t)
-		UpdateTestDeliveryServiceRequestComments(t)
-		UpdateTestDeliveryServiceRequestCommentsWithHeaders(t, header)
-		header = make(map[string][]string)
-		etag := rfc.ETag(currentTime)
-		header.Set(rfc.IfMatch, etag)
-		UpdateTestDeliveryServiceRequestCommentsWithHeaders(t, header)
-		GetTestDeliveryServiceRequestComments(t)
-		GetTestDeliveryServiceRequestCommentsIMSAfterChange(t, header)
-	})
-}
-
-func UpdateTestDeliveryServiceRequestCommentsWithHeaders(t *testing.T, header http.Header) {
-	opts := client.NewRequestOptions()
-	opts.Header = header
-	comments, _, _ := TOSession.GetDeliveryServiceRequestComments(opts)
-
-	if len(comments.Response) > 0 {
-		firstComment := comments.Response[0]
-		newFirstCommentValue := "new comment value"
-		firstComment.Value = newFirstCommentValue
 
-		_, reqInf, err := TOSession.UpdateDeliveryServiceRequestComment(firstComment.ID, firstComment, opts)
-		if err == nil {
-			t.Errorf("expected precondition failed error, but got none")
+		currentTime := time.Now().UTC().Add(-15 * time.Second)
+		currentTimeRFC := currentTime.Format(time.RFC1123)
+		tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
+
+		methodTests := utils.V4TestCase{
+			"GET": {
+				"NOT MODIFIED when NO CHANGES made": {
+					ClientSession: TOSession,
+					RequestOpts:   client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {tomorrow}}},
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)),
+				},
+				"OK when VALID request": {
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"OK when VALID ID parameter": {
+					ClientSession: TOSession,
+					RequestOpts:   client.RequestOptions{QueryParameters: url.Values{"id": {strconv.Itoa(GetDSRequestCommentId(t, "admin")())}}},
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseHasLength(1)),
+				},
+				"VALIDATE SORT when DEFAULT is ASC ORDER": {
+					ClientSession: TOSession,
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateSortedDSRequestComments()),
+				},
+			},
+			"PUT": {
+				"OK when VALID request": {
+					EndpointId:    GetDSRequestCommentId(t, "admin"),
+					ClientSession: TOSession,
+					RequestBody: map[string]interface{}{
+						"deliveryServiceRequestId": GetDSRequestId(t, "test-ds1")(),
+						"value":                    "updated comment",
+					},
+					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+				"PRECONDITION FAILED when updating with IF-UNMODIFIED-SINCE Header": {
+					EndpointId:    GetDSRequestCommentId(t, "admin"),
+					ClientSession: TOSession,
+					RequestOpts:   client.RequestOptions{Header: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}}},
+					RequestBody:   map[string]interface{}{},
+					Expectations:  utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+				"PRECONDITION FAILED when updating with IFMATCH ETAG Header": {
+					EndpointId:    GetDSRequestCommentId(t, "admin"),
+					ClientSession: TOSession,
+					RequestBody:   map[string]interface{}{},
+					RequestOpts:   client.RequestOptions{Header: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}}},
+					Expectations:  utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
+				},
+			},
+			"GET AFTER CHANGES": {
+				"OK when CHANGES made": {
+					ClientSession: TOSession,
+					RequestOpts:   client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {currentTimeRFC}}},
+					Expectations:  utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+				},
+			},
 		}
-		if reqInf.StatusCode != http.StatusPreconditionFailed {
-			t.Errorf("Expected status code 412, got %v", reqInf.StatusCode)
-		}
-	}
-}
 
-func GetTestDeliveryServiceRequestCommentsIMSAfterChange(t *testing.T, header http.Header) {
-	opts := client.NewRequestOptions()
-	opts.Header = header
-	resp, reqInf, err := TOSession.GetDeliveryServiceRequestComments(opts)
-	if err != nil {
-		t.Fatalf("could not get Delivery Service Request Comments: %v - alerts: %+v", err, resp.Alerts)
-	}
-	if reqInf.StatusCode != http.StatusOK {
-		t.Fatalf("Expected 200 status code, got %v", reqInf.StatusCode)
-	}
-	opts.Header = make(map[string][]string)
-	futureTime := time.Now().AddDate(0, 0, 1)
-	time := futureTime.Format(time.RFC1123)
-	opts.Header.Set(rfc.IfModifiedSince, time)
-	resp, reqInf, err = TOSession.GetDeliveryServiceRequestComments(opts)
-	if err != nil {
-		t.Fatalf("could not get Delivery Service Request Comments: %v - alerts: %+v", err, resp.Alerts)
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
-	}
-}
-
-func CreateTestDeliveryServiceRequestComments(t *testing.T) {
-	if len(testData.DeliveryServiceRequests) < 1 {
-		t.Fatal("Need at least one Delivery Service Request to test creating Delivery Service Request Comments")
-	}
-
-	// Retrieve a delivery service request by xmlId so we can get the ID needed to create a dsr comment
-	dsr := testData.DeliveryServiceRequests[0]
-	var ds *tc.DeliveryServiceV4
-	if dsr.ChangeType == tc.DSRChangeTypeDelete {
-		ds = dsr.Original
-	} else {
-		ds = dsr.Requested
-	}
-	resetDS(ds)
-	if ds == nil || ds.XMLID == nil {
-		t.Fatal("first DSR in the test data had a nil Delivery Service, or one with no XMLID")
-	}
-
-	opts := client.NewRequestOptions()
-	opts.QueryParameters.Set("xmlId", *ds.XMLID)
-	resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
-	if err != nil {
-		t.Fatalf("cannot get Delivery Service Request by XMLID '%s': %v - alerts: %+v", *ds.XMLID, err, resp.Alerts)
-	}
-	if len(resp.Response) != 1 {
-		t.Fatalf("found %d Delivery Service request by XMLID '%s, expected exactly one", len(resp.Response), *ds.XMLID)
-	}
-	respDSR := resp.Response[0]
-	if respDSR.ID == nil {
-		t.Fatalf("got Delivery Service Request with xml_id '%s' that had a null ID", *ds.XMLID)
-	}
-
-	for _, comment := range testData.DeliveryServiceRequestComments {
-		comment.DeliveryServiceRequestID = *respDSR.ID
-		resp, _, err := TOSession.CreateDeliveryServiceRequestComment(comment, client.RequestOptions{})
-		if err != nil {
-			t.Errorf("could not create Delivery Service Request Comment: %v - alerts: %+v", err, resp.Alerts)
+		for method, testCases := range methodTests {
+			t.Run(method, func(t *testing.T) {
+				for name, testCase := range testCases {
+					comment := tc.DeliveryServiceRequestComment{}
+
+					if testCase.RequestBody != nil {
+						dat, err := json.Marshal(testCase.RequestBody)
+						assert.NoError(t, err, "Error occurred when marshalling request body: %v", err)
+						err = json.Unmarshal(dat, &comment)
+						assert.NoError(t, err, "Error occurred when unmarshalling request body: %v", err)
+					}
+
+					switch method {
+					case "GET", "GET AFTER CHANGES":
+						t.Run(name, func(t *testing.T) {
+							resp, reqInf, err := testCase.ClientSession.GetDeliveryServiceRequestComments(testCase.RequestOpts)
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, resp.Response, resp.Alerts, err)
+							}
+						})
+					case "PUT":
+						t.Run(name, func(t *testing.T) {
+							alerts, reqInf, err := testCase.ClientSession.UpdateDeliveryServiceRequestComment(testCase.EndpointId(), comment, testCase.RequestOpts)
+							for _, check := range testCase.Expectations {
+								check(t, reqInf, nil, alerts, err)
+							}
+						})
+					}
+				}
+			})
 		}
-	}
+	})
 }
 
-func SortTestDeliveryServiceRequestComments(t *testing.T) {
-	var sortedList []string
-	resp, _, err := TOSession.GetDeliveryServiceRequestComments(client.RequestOptions{})
-	if err != nil {
-		t.Fatalf("Expected no error, but got: %v - alerts: %+v", err, resp.Alerts)
-	}
-	for _, dsrc := range resp.Response {
-		sortedList = append(sortedList, dsrc.XMLID)
-	}
+func GetDSRequestCommentId(t *testing.T, author string) func() int {
+	return func() int {
+		opts := client.NewRequestOptions()
+		opts.QueryParameters.Set("author", author)
 
-	res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
-		return sortedList[p] < sortedList[q]
-	})
-	if res != true {
-		t.Errorf("list is not sorted by their names: %v", sortedList)
+		resp, _, err := TOSession.GetDeliveryServiceRequestComments(opts)
+		assert.RequireNoError(t, err, "Get Delivery Service Request Comments failed with error: %v", err)
+		assert.RequireGreaterOrEqual(t, len(resp.Response), 1, "Expected delivery service request comments response object length of atleast 1, but got %d", len(resp.Response))
+		assert.RequireNotNil(t, resp.Response[0].ID, "Expected id to not be nil")
+
+		return resp.Response[0].ID
 	}
 }
 
-func UpdateTestDeliveryServiceRequestComments(t *testing.T) {
+func GetDSRequestId(t *testing.T, xmlId string) func() int {
+	return func() int {
+		opts := client.NewRequestOptions()
+		opts.QueryParameters.Set("xmlId", xmlId)
 
-	comments, _, err := TOSession.GetDeliveryServiceRequestComments(client.RequestOptions{})
-	if err != nil {
-		t.Errorf("Unexpected error getting Delivery Service Request Comments: %v - alerts: %+v", err, comments.Alerts)
-	}
-	if len(comments.Response) < 1 {
-		t.Fatal("Expected at least one Delivery Service Request Comment to exist in Traffic Ops - none did")
-	}
-	firstComment := comments.Response[0]
-	newFirstCommentValue := "new comment value"
-	firstComment.Value = newFirstCommentValue
-
-	var alert tc.Alerts
-	alert, _, err = TOSession.UpdateDeliveryServiceRequestComment(firstComment.ID, firstComment, client.RequestOptions{})
-	if err != nil {
-		t.Errorf("cannot update Delivery Service Request Comment #%d: %v - alerts: %+v", firstComment.ID, err, alert.Alerts)
-	}
+		resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
+		assert.RequireNoError(t, err, "Get Delivery Service Requests failed with error: %v", err)
+		assert.RequireGreaterOrEqual(t, len(resp.Response), 1, "Expected delivery service requests response object length of atleast 1, but got %d", len(resp.Response))
+		assert.RequireNotNil(t, resp.Response[0].ID, "Expected id to not be nil")
 
-	// Retrieve the delivery service request comment to check that the value got updated
-	opts := client.NewRequestOptions()
-	opts.QueryParameters.Set("id", strconv.Itoa(firstComment.ID))
-	resp, _, err := TOSession.GetDeliveryServiceRequestComments(opts)
-	if err != nil {
-		t.Errorf("cannot get Delivery Service Request Comment #%d: %v - alerts: %+v", firstComment.ID, err, resp.Alerts)
-	}
-	if len(resp.Response) != 1 {
-		t.Fatalf("Expected exactly one Delivery Service Request Comment to exist with ID %d, found: %d", firstComment.ID, len(resp.Response))
-	}
-	respDSRC := resp.Response[0]
-	if respDSRC.Value != newFirstCommentValue {
-		t.Errorf("results do not match actual: %s, expected: %s", respDSRC.Value, newFirstCommentValue)
+		return *resp.Response[0].ID
 	}
-
 }
 
-func GetTestDeliveryServiceRequestCommentsIMS(t *testing.T) {
-	futureTime := time.Now().AddDate(0, 0, 1)
-	time := futureTime.Format(time.RFC1123)
+func validateSortedDSRequestComments() utils.CkReqFunc {
+	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, err error) {
+		var sortedList []string
+		dsReqComments := resp.([]tc.DeliveryServiceRequestComment)
 
-	opts := client.NewRequestOptions()
-	opts.Header.Set(rfc.IfModifiedSince, time)
-	resp, reqInf, err := TOSession.GetDeliveryServiceRequestComments(opts)
-	if err != nil {
-		t.Fatalf("could not get Delivery Service Request Comments: %v - alerts: %+v", err, resp.Alerts)
-	}
-	if reqInf.StatusCode != http.StatusNotModified {
-		t.Fatalf("Expected 304 status code, got %v", reqInf.StatusCode)
+		for _, comment := range dsReqComments {
+			sortedList = append(sortedList, comment.XMLID)
+		}
+
+		res := sort.SliceIsSorted(sortedList, func(p, q int) bool {
+			return sortedList[p] < sortedList[q]
+		})
+		assert.Equal(t, res, true, "List is not sorted by their names: %v", sortedList)
 	}
 }
 
-func GetTestDeliveryServiceRequestComments(t *testing.T) {
-	comments, _, err := TOSession.GetDeliveryServiceRequestComments(client.RequestOptions{})
-	if err != nil {
-		t.Errorf("Unexpected error getting Delivery Service Request Comments: %v - alerts: %+v", err, comments.Alerts)
-	}
+func CreateTestDeliveryServiceRequestComments(t *testing.T) {
+	for _, comment := range testData.DeliveryServiceRequestComments {
+		opts := client.NewRequestOptions()
+		opts.QueryParameters.Set("xmlId", comment.XMLID)
+		resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
+		assert.NoError(t, err, "Cannot get Delivery Service Request by XMLID '%s': %v - alerts: %+v", comment.XMLID, err, resp.Alerts)
+		assert.Equal(t, len(resp.Response), 1, "Found %d Delivery Service request by XMLID '%s, expected exactly one", len(resp.Response), comment.XMLID)
+		assert.NotNil(t, resp.Response[0].ID, "Got Delivery Service Request with xml_id '%s' that had a null ID", comment.XMLID)
 
-	opts := client.NewRequestOptions()
-	for _, comment := range comments.Response {
-		opts.QueryParameters.Set("id", strconv.Itoa(comment.ID))
-		resp, _, err := TOSession.GetDeliveryServiceRequestComments(opts)
-		if err != nil {
-			t.Errorf("cannot get Delivery Service Request Comment by id %d: %v - alerts: %+v", comment.ID, err, resp.Alerts)
-		}
+		comment.DeliveryServiceRequestID = *resp.Response[0].ID
+		alerts, _, err := TOSession.CreateDeliveryServiceRequestComment(comment, client.RequestOptions{})
+		assert.NoError(t, err, "Could not create Delivery Service Request Comment: %v - alerts: %+v", err, alerts.Alerts)
 	}
 }
 
 func DeleteTestDeliveryServiceRequestComments(t *testing.T) {
 	comments, _, err := TOSession.GetDeliveryServiceRequestComments(client.RequestOptions{})
-	if err != nil {
-		t.Errorf("Unexpected error getting Delivery Service Request Comments: %v - alerts: %+v", err, comments.Alerts)
-	}
+	assert.NoError(t, err, "Unexpected error getting Delivery Service Request Comments: %v - alerts: %+v", err, comments.Alerts)
 
-	opts := client.NewRequestOptions()
 	for _, comment := range comments.Response {
 		resp, _, err := TOSession.DeleteDeliveryServiceRequestComment(comment.ID, client.RequestOptions{})
-		if err != nil {
-			t.Errorf("cannot delete Delivery Service Request Comment #%d: %v - alerts: %+v", comment.ID, err, resp.Alerts)
-		}
+		assert.NoError(t, err, "Cannot delete Delivery Service Request Comment #%d: %v - alerts: %+v", comment.ID, err, resp.Alerts)
 
 		// Retrieve the delivery service request comment to see if it got deleted
+		opts := client.NewRequestOptions()
 		opts.QueryParameters.Set("id", strconv.Itoa(comment.ID))
 		comments, _, err := TOSession.GetDeliveryServiceRequestComments(opts)
-		if err != nil {
-			t.Errorf("Unexpected error fetching Delivery Service Request Comment %d after deletion: %v - alerts: %+v", comment.ID, err, comments.Alerts)
-		}
-		if len(comments.Response) > 0 {
-			t.Errorf("expected Delivery Service Request Comment #%d to be deleted, but it was found in Traffic Ops", comment.ID)
-		}
+		assert.NoError(t, err, "Unexpected error fetching Delivery Service Request Comment %d after deletion: %v - alerts: %+v", comment.ID, err, comments.Alerts)
+		assert.Equal(t, len(comments.Response), 0, "Expected Delivery Service Request Comment #%d to be deleted, but it was found in Traffic Ops", comment.ID)
 	}
 }
diff --git a/traffic_ops/testing/api/v4/tc-fixtures.json b/traffic_ops/testing/api/v4/tc-fixtures.json
index 7af7a485c..ebf1fab2a 100644
--- a/traffic_ops/testing/api/v4/tc-fixtures.json
+++ b/traffic_ops/testing/api/v4/tc-fixtures.json
@@ -310,16 +310,20 @@
     ],
     "deliveryServiceRequestComments": [
         {
-            "value": "this is comment one"
+            "value": "this is comment one",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment two"
+            "value": "this is comment two",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment three"
+            "value": "this is comment three",
+            "xmlId": "test-ds1"
         },
         {
-            "value": "this is comment four"
+            "value": "this is comment four",
+            "xmlId": "test-ds1"
         }
     ],
     "deliveryServiceRequests": [
@@ -5005,8 +5009,8 @@
         {
             "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
-			"configUpdateTime": "2022-01-01T17:00:00-07:00",
-			"configApplyTime": "1969-12-31T17:00:00-07:00",
+            "configUpdateTime": "2022-01-01T17:00:00-07:00",
+            "configApplyTime": "1969-12-31T17:00:00-07:00",
             "domainName": "ga.atlanta.kabletown.net",
             "guid": null,
             "hostName": "config-update-time",
@@ -5056,8 +5060,8 @@
         {
             "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
-			"configUpdateTime": "2022-01-01T17:00:00-07:00",
-			"configApplyTime": "1969-12-31T17:00:00-07:00",
+            "configUpdateTime": "2022-01-01T17:00:00-07:00",
+            "configApplyTime": "1969-12-31T17:00:00-07:00",
             "domainName": "ga.atlanta.kabletown.net",
             "guid": null,
             "hostName": "config-update-time-no-updpend",
@@ -5145,8 +5149,8 @@
             "profileNames": ["EDGE1"],
             "rack": "RR 119.02",
             "revalPending": false,
-			"revalUpdateTime": "2022-01-01T17:00:00-07:00",
-			"revalApplyTime": "1969-12-31T17:00:00-07:00",
+            "revalUpdateTime": "2022-01-01T17:00:00-07:00",
+            "revalApplyTime": "1969-12-31T17:00:00-07:00",
             "status": "REPORTED",
             "tcpPort": 80,
             "type": "EDGE",
@@ -5195,8 +5199,8 @@
             "physLocation": "Denver",
             "profileNames": ["EDGE1"],
             "rack": "RR 119.02",
-			"revalUpdateTime": "2022-01-01T17:00:00-07:00",
-			"revalApplyTime": "1969-12-31T17:00:00-07:00",
+            "revalUpdateTime": "2022-01-01T17:00:00-07:00",
+            "revalApplyTime": "1969-12-31T17:00:00-07:00",
             "status": "REPORTED",
             "tcpPort": 80,
             "type": "EDGE",
@@ -5207,8 +5211,8 @@
         {
             "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
-			"configUpdateTime": "2022-01-01T17:00:00-07:00",
-			"configApplyTime": "1969-12-31T17:00:00-07:00",
+            "configUpdateTime": "2022-01-01T17:00:00-07:00",
+            "configApplyTime": "1969-12-31T17:00:00-07:00",
             "domainName": "ga.atlanta.kabletown.net",
             "guid": null,
             "hostName": "config-reval-update-time",
@@ -5248,8 +5252,8 @@
             "profileNames": ["EDGE1"],
             "rack": "RR 119.02",
             "revalPending": false,
-			"revalUpdateTime": "2022-01-01T17:00:00-07:00",
-			"revalApplyTime": "1969-12-31T17:00:00-07:00",
+            "revalUpdateTime": "2022-01-01T17:00:00-07:00",
+            "revalApplyTime": "1969-12-31T17:00:00-07:00",
             "status": "REPORTED",
             "tcpPort": 80,
             "type": "EDGE",
@@ -5260,8 +5264,8 @@
         {
             "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
-			"configUpdateTime": "2022-01-01T17:00:00-07:00",
-			"configApplyTime": "1969-12-31T17:00:00-07:00",
+            "configUpdateTime": "2022-01-01T17:00:00-07:00",
+            "configApplyTime": "1969-12-31T17:00:00-07:00",
             "domainName": "ga.atlanta.kabletown.net",
             "guid": null,
             "hostName": "config-reval-update-time-only",
@@ -5300,8 +5304,8 @@
             "physLocation": "Denver",
             "profileNames": ["EDGE1"],
             "rack": "RR 119.02",
-			"revalUpdateTime": "2022-01-01T17:00:00-07:00",
-			"revalApplyTime": "1969-12-31T17:00:00-07:00",
+            "revalUpdateTime": "2022-01-01T17:00:00-07:00",
+            "revalApplyTime": "1969-12-31T17:00:00-07:00",
             "status": "REPORTED",
             "tcpPort": 80,
             "type": "EDGE",
diff --git a/traffic_ops/traffic_ops_golang/server/detail_test.go b/traffic_ops/traffic_ops_golang/server/detail_test.go
index 9ad95ac56..bbcfa5378 100644
--- a/traffic_ops/traffic_ops_golang/server/detail_test.go
+++ b/traffic_ops/traffic_ops_golang/server/detail_test.go
@@ -13,7 +13,9 @@ package server
 */
 
 import (
+	"fmt"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
@@ -75,8 +77,7 @@ func TestGetDetailServers(t *testing.T) {
 		"mgmt_ip_netmask",
 		"offline_reason",
 		"phys_location",
-		"profile",
-		"profile_desc",
+		"profile_name",
 		"rack",
 		"status",
 		"tcp_port",
@@ -93,8 +94,6 @@ func TestGetDetailServers(t *testing.T) {
 	serviceNetmask := util.StrPtr("")
 	serviceInterface := util.StrPtr("")
 	serviceMtu := util.StrPtr("")
-	//routerHostName := util.StrPtr("")
-	//routerPort := util.StrPtr("")
 
 	for _, sd := range testServerDetails {
 		detailRows = detailRows.AddRow(
@@ -123,8 +122,7 @@ func TestGetDetailServers(t *testing.T) {
 			sd.MgmtIPNetmask,
 			sd.OfflineReason,
 			sd.PhysLocation,
-			sd.Profile,
-			sd.ProfileDesc,
+			fmt.Sprintf("{%s}", strings.Join(sd.ProfileNames, ",")),
 			sd.Rack,
 			sd.Status,
 			sd.TCPPort,
@@ -143,7 +141,7 @@ func TestGetDetailServers(t *testing.T) {
 	mock.ExpectQuery("SELECT serverid").WillReturnRows(hwInfoRows)
 	mock.ExpectCommit()
 
-	actualSrvs, err := getDetailServers(db.MustBegin().Tx, &auth.CurrentUser{PrivLevel: 30}, "test", 1, "id", 10, api.Version{Major: 3})
+	actualSrvs, err := getDetailServers(db.MustBegin().Tx, &auth.CurrentUser{PrivLevel: 30}, "test", 1, "id", 10, api.Version{Major: 4})
 	if err != nil {
 		t.Fatalf("an error '%s' occurred during read", err)
 	}
@@ -170,9 +168,7 @@ func TestGetDetailServers(t *testing.T) {
 
 func getMockServerDetails() []tc.ServerDetailV40 {
 	srvData := tc.ServerDetailV40{
-		ServerDetail: tc.ServerDetail{
-			ID: util.IntPtr(1),
-		},
+		ID:               util.IntPtr(1),
 		ServerInterfaces: []tc.ServerInterfaceInfoV40{}, // left empty because it must be written as json above since sqlmock does not support nested arrays
 	}
 	return []tc.ServerDetailV40{srvData}