Fixes Server Capabilities apis to respond with RFC3339 date/time Format #7408 #7482

jagan-parthiban · 2023-05-03T07:19:32Z

Closes: #7465
Related: #5911

Documentation
Traffic Ops

What is the best way to verify this PR?

Make Api calls to service_category 5.0

curl --request GET \
  --url https://localhost:8443/api/5.0/server_capabilities

{
	"response": [
		{
			"name": "disk",
			"lastUpdated": "2023-05-03T12:24:40.408349+05:30",
			"description": "server capability disk"
		},
		{
			"name": "ram",
			"lastUpdated": "2023-05-03T12:24:40.406796+05:30",
			"description": "server capability ram"
		}
	]
}

If this is a bugfix, which Traffic Control versions contained the bug?

7.0.1

PR submission checklist

This PR has tests
This PR has documentation
This PR has a CHANGELOG.md entry
This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

zrhoffman · 2023-05-17T21:11:12Z

traffic_ops/traffic_ops_golang/servercapability/servercapability.go

+
+	selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+	query := selectQuery + where + orderBy + pagination
+	rows, err := tx.NamedQuery(query, queryValues)


This alert is valid but already exists in the current version, this is not a new vuln.

codecov · 2023-05-03T16:39:17Z

Codecov Report

Merging #7482 (272d11b) into master (f87bcdf) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #7482      +/-   ##
============================================
- Coverage     30.32%   30.31%   -0.01%     
  Complexity       98       98              
============================================
  Files           789      789              
  Lines         82307    82304       -3     
  Branches        815      815              
============================================
- Hits          24957    24954       -3     
  Misses        55245    55245              
  Partials       2105     2105

Flag	Coverage Δ
traffic_ops_integration	`69.42% <100.00%> (ø)`
v3	`57.79% <ø> (ø)`
v4	`79.18% <ø> (ø)`
v5	`78.63% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
traffic_ops/traffic_ops_golang/routing/routes.go	`95.53% <ø> (-0.02%)`	⬇️
...ic_ops_golang/servercapability/servercapability.go	`15.20% <ø> (ø)`
traffic_ops/v5-client/servercapability.go	`87.50% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

zrhoffman · 2023-05-04T18:20:15Z

@jagan-parthiban If you rebase this onto the master branch, the CiaB workflow should pass, since #7489 is what makes it fail

zrhoffman

Type parameters were not in Go when this code was first written. Now that we need to support a new API version, we should be able to save a lot of lines of code using type parameters, rather than copying.

zrhoffman · 2023-05-17T21:10:46Z

traffic_ops/traffic_ops_golang/servercapability/servercapability.go

@@ -367,3 +367,157 @@ func (v *TOServerCapability) SelectMaxLastUpdatedQuery(where, orderBy, paginatio

 func (v *TOServerCapability) Create() (error, error, int) { return api.GenericCreateNameBasedID(v) }
 func (v *TOServerCapability) Delete() (error, error, int) { return api.GenericDelete(v) }
+
+func GetServerCapabilityV5(w http.ResponseWriter, r *http.Request) {


Rather than copying all of the content from GetServerCapability to GetServerCapabilityV5, we should be able to use the same function for either and use a type parameter for which ServerCapability type to use.

zrhoffman · 2023-05-17T21:11:12Z

traffic_ops/traffic_ops_golang/servercapability/servercapability.go

+
+	selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+	query := selectQuery + where + orderBy + pagination
+	rows, err := tx.NamedQuery(query, queryValues)


This alert is valid but already exists in the current version, this is not a new vuln.

zrhoffman · 2023-05-17T21:12:17Z

traffic_ops/traffic_ops_golang/servercapability/servercapability.go

+	return
+}
+
+func CreateServerCapabilityV5(w http.ResponseWriter, r *http.Request) {


Same here: CreateServerCapability can be reused if it accepts a type parameter.

zrhoffman · 2023-05-17T21:12:35Z

traffic_ops/traffic_ops_golang/servercapability/servercapability.go

+	return
+}
+
+func UpdateServerCapabilityV5(w http.ResponseWriter, r *http.Request) {


Same here: UpdateServerCapability can be reused if it accepts a type parameter.

zrhoffman · 2023-05-18T17:30:52Z

On second thought, maybe this is a little complicated to be able to easily save time with generics. Once the branch is rebased onto the master branch, #7482 should be good to merge.

…RFC3339 date/time strings

Issue: apache#7465

zrhoffman

Looks good, thanks!

jagan-parthiban marked this pull request as ready for review May 3, 2023 09:17

ocket8888 added Traffic Ops related to Traffic Ops low impact affects only a small portion of a CDN, and cannot itself break one tech debt rework due to choosing easy/limited solution labels May 3, 2023

github-advanced-security bot found potential problems May 3, 2023

View reviewed changes

zrhoffman requested changes May 17, 2023

View reviewed changes

jagan-parthiban added 3 commits May 18, 2023 23:03

Fixes Server Capabilities V5 Traffic Ops API to use and respond with …

43a5f58

…RFC3339 date/time strings

Update CHANGELOG.md with issue details

06e511f

Update .rst doc files for RF3339 in server capabilities.

0682d59

Issue: apache#7465

jagan-parthiban force-pushed the improve/rfc3339-server-capabilities branch from be7638c to 0682d59 Compare May 18, 2023 17:40

zrhoffman approved these changes May 18, 2023

View reviewed changes

Updated error message

272d11b

zrhoffman merged commit 147db8d into apache:master May 18, 2023

jagan-parthiban deleted the improve/rfc3339-server-capabilities branch June 8, 2023 10:50

rimashah25 added this to the 8.0.0 milestone Jul 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes Server Capabilities apis to respond with RFC3339 date/time Format #7408 #7482

Fixes Server Capabilities apis to respond with RFC3339 date/time Format #7408 #7482

jagan-parthiban commented May 3, 2023 •

edited

Loading

zrhoffman May 17, 2023

codecov bot commented May 3, 2023 •

edited

Loading

zrhoffman commented May 4, 2023

zrhoffman left a comment

zrhoffman May 17, 2023

zrhoffman May 17, 2023

zrhoffman May 17, 2023 •

edited

Loading

zrhoffman May 17, 2023 •

edited

Loading

zrhoffman commented May 18, 2023

zrhoffman left a comment

Fixes Server Capabilities apis to respond with RFC3339 date/time Format #7408 #7482

Fixes Server Capabilities apis to respond with RFC3339 date/time Format #7408 #7482

Conversation

jagan-parthiban commented May 3, 2023 • edited Loading

What is the best way to verify this PR?

If this is a bugfix, which Traffic Control versions contained the bug?

PR submission checklist

zrhoffman May 17, 2023

Choose a reason for hiding this comment

codecov bot commented May 3, 2023 • edited Loading

Codecov Report

zrhoffman commented May 4, 2023

zrhoffman left a comment

Choose a reason for hiding this comment

zrhoffman May 17, 2023

Choose a reason for hiding this comment

zrhoffman May 17, 2023

Choose a reason for hiding this comment

zrhoffman May 17, 2023 • edited Loading

Choose a reason for hiding this comment

zrhoffman May 17, 2023 • edited Loading

Choose a reason for hiding this comment

zrhoffman commented May 18, 2023

zrhoffman left a comment

Choose a reason for hiding this comment

jagan-parthiban commented May 3, 2023 •

edited

Loading

codecov bot commented May 3, 2023 •

edited

Loading

zrhoffman May 17, 2023 •

edited

Loading

zrhoffman May 17, 2023 •

edited

Loading