rob05c commented on a change in pull request #4041: Block server
servercapability delete if associated ds requires it
URL: https://github.com/apache/trafficcontrol/pull/4041#discussion_r340359296
##########
File path: traffic_ops/traffic_ops_golang/server/servers_server_capability.go
##########
@@ -117,6 +120,22 @@ func (ssc *TOServerServerCapability) Read()
([]interface{}, error, error, int) {
}
func (ssc *TOServerServerCapability) Delete() (error, error, int) {
+ // Ensure that the user is not removing a server capability from the
server
+ // that is required by the delivery services the server is assigned to
(if applicable)
+ dsIDs := []int64{}
+ if err := ssc.APIInfo().Tx.QueryRow(checkDSReqCapQuery(), ssc.ServerID,
ssc.ServerCapability).Scan(pq.Array(&dsIDs)); err != nil {
+ return nil, fmt.Errorf("checking removing server server
capability would still suffice delivery service requried capabilites: %v",
err), http.StatusInternalServerError
+ }
+
+ if len(dsIDs) > 0 {
+ dsIdsStr, err := json.Marshal(dsIDs)
+ if err != nil {
+ return nil, fmt.Errorf("formatting response message on
bad request to disassociate server capability from server: %v", err),
http.StatusInternalServerError
+ }
+ return fmt.Errorf("cannot remove the capability %v from the
server %v as the server is assigned to the delivery services %v that require
it", *ssc.ServerCapability, *ssc.ServerID, string(dsIdsStr)), nil,
http.StatusBadRequest
Review comment:
This is a user error. 5xx codes are for server errors, it's a violation of
HTTP to send them for a client error.
Like I commented in #4043, I think this should be a `403 Forbidden` since we
can't really hide DS existence. But if we feel the need to try to obfuscate, a
400 or 404 would be acceptable; but 5xx isn't.
(404 is kind of a little lie; but we can kind of rationalize it as "it
doesn't exist, as far as you're concerned." Which IMO is a much smaller
lie/violation than a 5xx for a request error. But I'd vote for 400 over 404.)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
