[GitHub] [trafficcontrol] rob05c commented on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

rob05c commented on issue #4043: TO: Internal Server error is returned when 
user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547670913
 
 
   Removing unprivileged DSes in `/deliveryservices` endpoints isn't just for 
security, it's a convenience and improves usability. There's no need for a user 
to see DSes they don't own, so it saves them an extra request (or checking some 
field on the DS) to figure out whether they have access to it.
   
   >Just because a DS with id 1 and a DS with id 3 both exist is no guarantee 
that a DS with id 2 (still) exists
   
   True; but there will always be ways to discover existence. Most data has 
name endpoints as well (and IMO we should be moving that direction, away from 
IDs). And again, names are always discoverable.
   
   >An error message for creating DSes with duplicate keys can be changed just 
as easily.
   
   That would be a huge usability loss. If a user is trying to make a DS, and 
doesn't get a message telling them the name is already taken, now they're going 
to call our support and ask why it's failing with a cryptic message. And what 
is our support to do? Cryptically answer that they must pick another name, but 
somehow not let them know that name is taken?
   
   Again, it's a good goal, but obscurity is always a lesser form of security, 
and it comes at a cost: when someone really should have access to a DS and the 
permissions are a mistake, telling them it doesn't exist is going to create a 
lot of confusion. That confusion just doesn't seem worth it, when it isn't 
really buying us security anyway.
   
   Suppose someone malicious knows an ID is valid. What can they do, that they 
couldn't do anyway? If they have an exploit, they can use it to determine 
existence, correlate the name, modify or delete, or whatever else their exploit 
allows.
   
   Obscurity _is_ a small security feature; but in this case it's very small 
(I'm not sure it exists at all), and comes with a high usability cost.


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

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4041: Block server servercapability delete if associated ds requires it

[GitBox]

ocket 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_r340364678
 
 

 ##
 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()); 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:
   Yeah, 400 is fine.


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

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4041: Block server servercapability delete if associated ds requires it

[GitBox]

mhoppa 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_r340362079
 
 

 ##
 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()); 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:
   My first thought it to leave this as a 400 regardless of that decision ( 
which 40x code to use).
   
   I am just checking that you can not remove a server capability from a server 
that is required from an associated ds.  Server server capabilities are not 
under tenancy so returning a 403 seems a little weird to me.


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

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4041: Block server servercapability delete if associated ds requires it

[GitBox]

ocket 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_r340361969
 
 

 ##
 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()); 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:
   So `api.GenericDelete` probably handles returning 404's for keys that aren't 
found, huh? I only suggested a 500 because otherwise it seemed like you'd get a 
404 **only** when you don't have permission, which isn't an obfuscation at all. 
Definitely should be a 404 IMO as long as legitimate 404s are possible.


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

[GitHub] [trafficcontrol] asf-ci commented on issue #3996: Rewrote /user/current to Go

[GitBox]

asf-ci commented on issue #3996: Rewrote /user/current to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#issuecomment-547662575
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4670/
   


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

[GitHub] [trafficcontrol] ocket8888 commented on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

ocket commented on issue #4043: TO: Internal Server error is returned when 
user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547662350
 
 
   Then why bother hiding Delivery Services in the response to 
`/deliveryservices`? Just because a DS with id 1 and a DS with id 3 both exist 
is no guarantee that a DS with id 2 (still) exists. An error message for 
creating DSes with duplicate keys can be changed just as easily.


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

[GitHub] [trafficcontrol] rob05c commented on a change in pull request #4041: Block server servercapability delete if associated ds requires it

[GitBox]

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()); 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

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4041: Block server servercapability delete if associated ds requires it

[GitBox]

mhoppa 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_r340354844
 
 

 ##
 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()); 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:
   based on the result of https://github.com/apache/trafficcontrol/issues/4043 
I am unsure if we would want to change this to a 500 or leave it as is 


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

[GitHub] [trafficcontrol] rob05c commented on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

rob05c commented on issue #4043: TO: Internal Server error is returned when 
user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547654920
 
 
   I think I agree with @lbathina that it should be a `403 Forbidden`.
   
   >A user without tenancy permissions over a DS should never know that the DS 
exists. Responding with a 403 circumvents that, by admitting that the DS exists.
   
   You're right, that's a good goal for security. But in this case, it's not 
possible to hide existence. Postgres IDs are predictable and we've never taken 
the measures to obfuscate that. More than that, it'll always be possible to 
find out if a DS exists, by trying to create one with the same name.
   
   Since it isn't possible to actually hide, a `403 Forbidden` with a user 
message "User X Tenant Y does not have access to that resource" is the most 
helpful to legitimate users.


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

[GitHub] [trafficcontrol] lbathina commented on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

lbathina commented on issue #4043: TO: Internal Server error is returned when 
user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547653383
 
 
   so if we don't agree to be 403, it should be 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

[GitHub] [trafficcontrol] lbathina edited a comment on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

lbathina edited a comment on issue #4043: TO: Internal Server error is returned 
when user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547652483
 
 
   ```
   6.5.3. 403 Forbidden
   
   The 403 (Forbidden) status code indicates that the server understood the 
request but refuses to authorize it. A server that wishes to make public why 
the request has been forbidden can describe that reason in the response payload 
(if any).
   
   If authentication credentials were provided in the request, the
   server considers them insufficient to grant access. The client
   SHOULD NOT automatically repeat the request with the same
   credentials. The client MAY repeat the request with new or different 
credentials. However, a request might be forbidden for reasons
   unrelated to the credentials.
   
   An origin server that wishes to "hide" the current existence of a
   forbidden target resource MAY instead respond with a status code of
   404 (Not Found).
   ```
   ref: https://www.rfc-editor.org/rfc/rfc7231.html#section-6.5.3


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

[GitHub] [trafficcontrol] lbathina edited a comment on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

lbathina edited a comment on issue #4043: TO: Internal Server error is returned 
when user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547652483
 
 
   ```
   6.5.3. 403 Forbidden
   
   The 403 (Forbidden) status code indicates that the server understood the 
request but refuses to authorize it. A server that wishes to make public why 
the request has been forbidden can describe that reason in the response payload 
(if any).
   
   If authentication credentials were provided in the request, the
   server considers them insufficient to grant access. The client
   SHOULD NOT automatically repeat the request with the same
   credentials. The client MAY repeat the request with new or different 
credentials. However, a request might be forbidden for reasons
   unrelated to the credentials.
   
   An origin server that wishes to "hide" the current existence of a
   forbidden target resource MAY instead respond with a status code of
   404 (Not Found).
   ```


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

[GitHub] [trafficcontrol] lbathina commented on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

lbathina commented on issue #4043: TO: Internal Server error is returned when 
user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547652483
 
 
   ```6.5.3. 403 Forbidden
   
   The 403 (Forbidden) status code indicates that the server understood the 
request but refuses to authorize it. A server that wishes to make public why 
the request has been forbidden can describe that reason in the response payload 
(if any).
   
   If authentication credentials were provided in the request, the
   server considers them insufficient to grant access. The client
   SHOULD NOT automatically repeat the request with the same
   credentials. The client MAY repeat the request with new or different 
credentials. However, a request might be forbidden for reasons
   unrelated to the credentials.
   
   An origin server that wishes to "hide" the current existence of a
   forbidden target resource MAY instead respond with a status code of
   404 (Not Found).```


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

[GitHub] [trafficcontrol] lbathina edited a comment on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

lbathina edited a comment on issue #4043: TO: Internal Server error is returned 
when user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547652483
 
 
   ```6.5.3. 403 Forbidden
   
   The 403 (Forbidden) status code indicates that the server understood the 
request but refuses to authorize it. A server that wishes to make public why 
the request has been forbidden can describe that reason in the response payload 
(if any).
   
   If authentication credentials were provided in the request, the
   server considers them insufficient to grant access. The client
   SHOULD NOT automatically repeat the request with the same
   credentials. The client MAY repeat the request with new or different 
credentials. However, a request might be forbidden for reasons
   unrelated to the credentials.
   
   An origin server that wishes to "hide" the current existence of a
   forbidden target resource MAY instead respond with a status code of
   404 (Not Found).
   ```


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

[GitHub] [trafficcontrol] ocket8888 commented on issue #4036: TO: alert message on create server should be fixed.

[GitBox]

ocket commented on issue #4036: TO: alert message on create server should 
be fixed. 
URL: https://github.com/apache/trafficcontrol/issues/4036#issuecomment-547650631
 
 
   You're preaching to the choir here. The API cannot be reworked until API v2 
is released, which mainly means not until ATC 5, with the old API committed to 
existing until at least ATC 6.


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

[GitHub] [trafficcontrol] ocket8888 commented on issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

ocket commented on issue #4043: TO: Internal Server error is returned when 
user not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043#issuecomment-547649192
 
 
   No, this is - at least for DELETE - correct behavior. A user without tenancy 
permissions over a DS should never know that the DS exists. Responding with a 
403 circumvents that, by admitting that the DS exists.
   
   When assigning a required capability to a Delivery Service with improper 
tenancy, the response should be one of:
   
   - `400 Bad Request` - most common but perhaps more general than we need to be
   - `404 Not Found` - immediately tells you that something you were looking 
for didn't exist, but sort of implies that it's the URI that's non-existent, 
which isn't the case here
   - `409 Conflict` - Something about the state of the server is in conflict 
with the request, in this case the fact that the requested Delivery Service 
doesn't exist (as far as the requesting user knows).
   
   For my money one of the bottom two is best, and I sort of go back and forth 
as to which is better. But all three are acceptable.


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

[GitHub] [trafficcontrol] asf-ci commented on issue #4041: Block server servercapability delete if associated ds requires it

[GitBox]

asf-ci commented on issue #4041: Block server servercapability delete if 
associated ds requires it
URL: https://github.com/apache/trafficcontrol/pull/4041#issuecomment-547648825
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4669/
   


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

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4041: Block server servercapability delete if associated ds requires it

[GitBox]

ocket 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_r340344156
 
 

 ##
 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()); 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 check isn't respecting tenancy; if a user doesn't have tenant 
permissions over a DS they're not supposed to know it exists. In that case, you 
should probably give back an Internal Server Error with no user error, I think, 
to maintain obscurity.


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

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4041: Block server servercapability delete if associated ds requires it

[GitBox]

ocket 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_r340344578
 
 

 ##
 File path: traffic_ops/testing/api/v14/cachegroupsdeliveryservices_test.go
 ##
 @@ -19,11 +19,8 @@ import (
"testing"
 )
 
-func TestDeliveryServicesCachegroups(t *testing.T) {
-   WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, 
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, 
DeliveryServices}, func() {
-   CreateTestCachegroupsDeliveryServices(t)
-   DeleteTestCachegroupsDeliveryServices(t)
-   })
+func TestCacheGroupsDeliveryServices(t *testing.T) {
+   WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, 
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, 
DeliveryServices, CacheGroupsDeliveryServices}, func() {})
 
 Review comment:
   You appear to have disabled these unrelated tests; why?


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

[GitHub] [trafficcontrol] lbathina commented on issue #4036: TO: alert message on create server should be fixed.

[GitBox]

lbathina commented on issue #4036: TO: alert message on create server should be 
fixed. 
URL: https://github.com/apache/trafficcontrol/issues/4036#issuecomment-547644180
 
 
   I think message on almost all API should be revisited to make it more 
meaningful and user friendly.


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

[GitHub] [trafficcontrol] asf-ci commented on issue #4042: Foreign key err handling regex was not catching tables with _'s

[GitBox]

asf-ci commented on issue #4042: Foreign key err handling regex was not 
catching tables with _'s
URL: https://github.com/apache/trafficcontrol/pull/4042#issuecomment-547641860
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4668/
   


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

Jenkins build is back to normal : trafficcontrol-PR #4668

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] lbathina opened a new issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigned required capability

[GitBox]

lbathina opened a new issue #4043: TO: Internal Server error is returned when 
user not tenant of the ds assigned required capability
URL: https://github.com/apache/trafficcontrol/issues/4043
 
 
   
   
   
   
   ## I'm submitting a ...
   
   
   - [X] bug report
   - [ ] new feature / enhancement request
   - [ ] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [X] Traffic Ops
   - [ ] Traffic Ops ORT
   - [ ] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
   500 Internal server error
   ## Expected / new behavior:
   
   403 Forbidden access
   ## Minimal reproduction of the problem with instructions:
   
   POST 
https://{{TO_BASE_URL}}/api/{{api_version}}/deliveryservices_required_capabilities
   {
   "deliveryServiceID": 17,
   "requiredCapability": "DISK_CAP"
   }
   ## Anything else:
   
   
   


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

[GitHub] [trafficcontrol] mhoppa opened a new pull request #4042: Foreign key err handling regex was not catching tables with _'s

[GitBox]

mhoppa opened a new pull request #4042: Foreign key err handling regex was not 
catching tables with _'s
URL: https://github.com/apache/trafficcontrol/pull/4042
 
 
   
   ## What does this PR (Pull Request) do?
   
   
   - [x] This PR fixes #4033 
   
   
   ## Which Traffic Control components are affected by this PR?
   
   
   - Traffic Ops
   
   ## What is the best way to verify this PR?
   
   Create a server capability assign it to a server or ds and then attempt to 
delete it. It should now return a 400 instead of a 500. 
   
   ## If this is a bug fix, what versions of Traffic Control are affected?
   
   master
   
   ## The following criteria are ALL met by this PR
   
   
   - [x] This PR includes tests
   - [x] I have explained why documentation is unnecessary
   - [x] This PR includes an update to CHANGELOG.md OR such an update is not 
necessary
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR 
does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the 
Apache Software Foundation's security 
guidelines](https://www.apache.org/security/) for details)
   
   
   ## Additional Information
   
   
   
   


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

[GitHub] [trafficcontrol] asf-ci commented on issue #4041: Block server servercapability delete if associated ds requires it

[GitBox]

asf-ci commented on issue #4041: Block server servercapability delete if 
associated ds requires it
URL: https://github.com/apache/trafficcontrol/pull/4041#issuecomment-547602091
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4667/
   


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

Build failed in Jenkins: trafficcontrol-PR #4667

[Apache Jenkins Server]

See 


Changes:

[ocket] Feature/add deliveryservice required capability (#4026)

[ocket] Fix query parameter name (#4031)

[ocket] Correct references to traffic vault (#4040)

[rawlin_peters] Docs/influx fix (#4037)

[ocket] Rewrite cache_stats to Golang (#4029)

[Michael_Hoppal] Block server servercapability delete if associated ds requires 
it


--
GitHub pull request #4041 of commit f7f6a4ae317e98e33647a0cc0b839d897095b197, 
no merge conflicts.
Running as SYSTEM
Setting status of f7f6a4ae317e98e33647a0cc0b839d897095b197 to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4667/ and message: 'Build 
started for merge commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H39 (ubuntu xenial) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
Cloning the remote Git repository
Cloning repository git://github.com/apache/trafficcontrol.git
 > git init  # timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse f7f6a4ae317e98e33647a0cc0b839d897095b197^{commit} # timeout=10
Checking out Revision f7f6a4ae317e98e33647a0cc0b839d897095b197 (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f f7f6a4ae317e98e33647a0cc0b839d897095b197
Commit message: "Block server servercapability delete if associated ds requires 
it"
 > git rev-list --no-walk 2c302aa03df2aa81f8e4a66f1baca1063222fe6b # timeout=10
[trafficcontrol-PR] $ /bin/bash /tmp/jenkins3210351136888625276.sh
++ echo jenkins-trafficcontrol-PR-4667
++ sed s/-//g
++ sed s/jenkins//
+ proj=trafficcontrolPR4667
+ yml=infrastructure/docker/build/docker-compose.yml
++ mktemp /tmp/docker-compose-
+ dc=/tmp/docker-compose-zpvO
++ mktemp /tmp/tc-status-
+ st=/tmp/tc-status-OQER
+ trap finish EXIT
++ uname -s
++ uname -m
+ curl -o /tmp/docker-compose-zpvO -L 
https://github.com/docker/compose/releases/download/1.13.0/docker-compose-Linux-x86_64
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 
0100   6170   6170 0   1015  0 --:--:-- --:--:-- --:--:--  1016
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0  
0 00 00 0  0  0 --:--:--  0:00:01 --:--:-- 0 89 
8079k   89 7238k0 0  2484k  0  0:00:03  0:00:02  0:00:01 3746k100 
8079k  100 8079k0 0  2764k  0  0:00:02  0:00:02 --:--:-- 4162k
+ chmod +x /tmp/docker-compose-zpvO
+ rm -rf dist
+ /tmp/docker-compose-zpvO -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4667 up
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ exit 1
+ finish
+ /tmp/docker-compose-zpvO -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4667 down -v
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ /tmp/docker-compose-zpvO -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4667 rm -v -f
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ rm -f /tmp/docker-compose-zpvO
Build step 'Execute shell' marked build as failure
Skipped archiving because build is not successful

[GitHub] [trafficcontrol] mhoppa opened a new pull request #4041: Block server servercapability delete if associated ds requires it

[GitBox]

mhoppa opened a new pull request #4041: Block server servercapability delete if 
associated ds requires it
URL: https://github.com/apache/trafficcontrol/pull/4041
 
 
   
   ## What does this PR (Pull Request) do?
   
   
   - [x] This PR is not related to any Issue 
   
   This PR adds in extra validation in disassociating a server capability from 
a server. If that server is assigned to a DS that requires that server 
capability we now block the DELETE.
   
   ## Which Traffic Control components are affected by this PR?
   
   
   - Documentation
   - Traffic Ops
   
   ## What is the best way to verify this PR?
   
   Besides running API tests it can be manually tested by:
   * Create DS
   * Create server capability "ram" 
   * Assign server capability to DS
   * Create edge server 
   * Assign server capability "ram"  to server
   * Assign server to DS
   * Attempt to DELETE server capability from server (SHOULD NOW BE BLOCKED)
   * Remove  server capability "ram" from DS
   * Attempt to DELETE server capability from server (SHOULD NOW WORK)
   
   ## If this is a bug fix, what versions of Traffic Control are affected?
   
   
   
   ## The following criteria are ALL met by this PR
   
   
   - [x] This PR includes tests 
   - [x] This PR includes documentation 
   - [x] This PR does not included an update to CHANGELOG.md
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR 
does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the 
Apache Software Foundation's security 
guidelines](https://www.apache.org/security/) for details)
   
   
   ## Additional Information
   
   
   
   


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

[GitHub] [trafficcontrol] asf-ci commented on issue #4030: Rewrite PUT /api/1.1/servers/:id/status to Go

[GitBox]

asf-ci commented on issue #4030: Rewrite PUT /api/1.1/servers/:id/status to Go
URL: https://github.com/apache/trafficcontrol/pull/4030#issuecomment-547597578
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4666/
   


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

Jenkins build is back to normal : trafficcontrol-PR #4666

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4030: Rewrite PUT /api/1.1/servers/:id/status to Go

[GitBox]

rawlinp commented on a change in pull request #4030: Rewrite PUT 
/api/1.1/servers/:id/status to Go
URL: https://github.com/apache/trafficcontrol/pull/4030#discussion_r340286906
 
 

 ##
 File path: traffic_ops/testing/api/v14/serverupdatestatus_test.go
 ##
 @@ -0,0 +1,149 @@
+package v14
+
+/*
+
+   Licensed 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.
+*/
+
+import (
+   "testing"
+
+   "github.com/apache/trafficcontrol/lib/go-tc"
+   "github.com/apache/trafficcontrol/lib/go-util"
+)
+
+func TestServerUpdateStatus(t *testing.T) {
+   WithObjs(t, []TCObj{CDNs, Types, Parameters, Profiles, Statuses, 
Divisions, Regions, PhysLocations, CacheGroups, Servers}, func() {
+   UpdateTestServerStatus(t)
+   })
+}
+
+func UpdateTestServerStatus(t *testing.T) {
+
+   edge1cdn1 := tc.Server{}
+   edge2cdn1 := tc.Server{}
+   mid1cdn1 := tc.Server{}
+   edge1cdn2 := tc.Server{}
+
+   getServers := func() {
+   for _, s := range []struct {
+   name   string
+   server *tc.Server
+   }{
+   {
+   "atlanta-edge-01",
+   ,
+   },
+   {
+   "atlanta-edge-03",
+   ,
+   },
+   {
+   "atlanta-mid-16",
+   ,
+   },
+   {
+   "edge1-cdn2",
+   ,
+   },
+   } {
+   resp, _, err := TOSession.GetServerByHostName(s.name)
+   if err != nil {
+   t.Errorf("cannot GET Server by hostname: %v - 
%v\n", s.name, err)
+   }
+   *s.server = resp[0]
+   }
+   }
+   getServers()
+
+   // assert that servers don't have updates pending
+   for _, s := range []tc.Server{
+   edge1cdn1,
+   edge2cdn1,
+   mid1cdn1,
+   edge1cdn2,
+   } {
+   if s.UpdPending {
+   t.Errorf("expected UpdPending: false, actual: true")
+   }
+   }
+
+   // update status of MID server to OFFLINE
+   _, _, err := TOSession.UpdateServerStatus(mid1cdn1.ID, 
tc.ServerPutStatus{
+   Status:util.JSONNameOrIDStr{Name: 
util.StrPtr("OFFLINE")},
+   OfflineReason: util.StrPtr("testing")})
+   if err != nil {
+   t.Errorf("cannot update server status: %v", err)
+   }
+
+   // assert that updates were queued for the proper EDGE servers
+   getServers()
+   if !edge1cdn1.UpdPending {
+   t.Errorf("expected: child %s to have updates pending, actual: 
no updates pending", edge1cdn1.HostName)
+   }
+   if !edge2cdn1.UpdPending {
+   t.Errorf("expected: child %s to have updates pending, actual: 
no updates pending", edge2cdn1.HostName)
+   }
+   if mid1cdn1.UpdPending {
+   t.Errorf("expected: server %s with updated status to have no 
updates pending, actual: updates pending", mid1cdn1.HostName)
+   }
+   if edge1cdn2.UpdPending {
+   t.Errorf("expected: server %s in different CDN than server with 
updated status to have no updates pending, actual: updates pending", 
edge2cdn1.HostName)
+   }
+
+   // update status of MID server to OFFLINE via ID
+   status, _, err := TOSession.GetStatusByName("OFFLINE")
+   if err != nil {
+   t.Errorf("cannot GET status by name: %v", err)
 
 Review comment:
   done


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

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4030: Rewrite PUT /api/1.1/servers/:id/status to Go

[GitBox]

rawlinp commented on a change in pull request #4030: Rewrite PUT 
/api/1.1/servers/:id/status to Go
URL: https://github.com/apache/trafficcontrol/pull/4030#discussion_r340284109
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/server/put_status.go
 ##
 @@ -0,0 +1,137 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+   "database/sql"
+   "encoding/json"
+   "errors"
+   "fmt"
+   "net/http"
+   "strings"
+
+   "github.com/apache/trafficcontrol/lib/go-tc"
+   "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+   
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+func UpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
+   inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id"}, 
[]string{"id"})
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
+   defer inf.Close()
+   reqObj := tc.ServerPutStatus{}
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, 
errors.New("malformed JSON: "+err.Error()), nil)
+   return
+   }
+
+   serverInfo, exists, err := dbhelpers.GetServerInfo(inf.IntParams["id"], 
inf.Tx.Tx)
+   if err != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, 
nil, err)
+   return
+   }
+   if !exists {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, 
fmt.Errorf("server ID %d not found", inf.IntParams["id"]), nil)
+   return
+   }
+
+   status := tc.StatusNullable{}
+   statusExists := false
+   if reqObj.Status.Name != nil {
+   status, statusExists, err = 
dbhelpers.GetStatusByName(*reqObj.Status.Name, inf.Tx.Tx)
+   } else if reqObj.Status.ID != nil {
+   status, statusExists, err = 
dbhelpers.GetStatusByID(*reqObj.Status.ID, inf.Tx.Tx)
+   } else {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, 
errors.New("status is required"), nil)
+   return
+   }
+   if err != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, 
nil, err)
+   return
+   }
+   if !statusExists {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, 
errors.New("invalid status (does not exist)"), nil)
+   return
+   }
+
+   if *status.Name == tc.CacheStatusAdminDown.String() || *status.Name == 
tc.CacheStatusOffline.String() {
+   if reqObj.OfflineReason == nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, 
errors.New("offlineReason is required for "+tc.CacheStatusAdminDown.String()+" 
or "+tc.CacheStatusOffline.String()+" status"), nil)
+   return
+   }
+   *reqObj.OfflineReason = inf.User.UserName + ": " + 
*reqObj.OfflineReason
+   } else {
+   reqObj.OfflineReason = nil
+   }
+   if err := updateServerStatusAndOfflineReason(inf.IntParams["id"], 
*status.ID, reqObj.OfflineReason, inf.Tx.Tx); err != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, 
nil, err)
+   return
+   }
+   offlineReason := ""
+   if reqObj.OfflineReason != nil {
+   offlineReason = *reqObj.OfflineReason
+   }
+   msg := "Updated status [ " + *status.Name + " ] for " + 
serverInfo.HostName + "." + serverInfo.DomainName + " [ " + offlineReason + " ]"
+
+   // queue updates on child servers if server is ^EDGE or ^MID
+   if strings.HasPrefix(serverInfo.Type, tc.CacheTypeEdge.String()) || 
strings.HasPrefix(serverInfo.Type, tc.CacheTypeMid.String()) {
 
 Review comment:
   Yes, but you don't have to take my word for it: 
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/lib/API/Server.pm#L880


This is an automated message from the Apache Git Service.
To respond to the message, 

Build failed in Jenkins: trafficcontrol-traffic_ops-test #1614

[Apache Jenkins Server]

See 


Changes:

[rawlin_peters] Rewrite /federations to Go - POST/PUT/DELETE (#4015)


--
Started by an SCM change
Running as SYSTEM
[EnvInject] - Loading node environment variables.
Building remotely on H38 (ubuntu xenial) in workspace 

using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git rev-parse refs/remotes/origin/master^{commit} # timeout=10
 > git rev-parse refs/remotes/origin/origin/master^{commit} # timeout=10
Checking out Revision 7e62d070db4a3ee1d4ed5d3127970ecba0702bf0 
(refs/remotes/origin/master)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 7e62d070db4a3ee1d4ed5d3127970ecba0702bf0
Commit message: "Rewrite /federations to Go - POST/PUT/DELETE (#4015)"
 > git rev-list --no-walk 0124759417b9c0e94d8e145aee2083f611fe226b # timeout=10
[trafficcontrol-traffic_ops-test] $ /bin/bash /tmp/jenkins215983088548974930.sh
docker-compose version 1.24.1, build 4667896
docker-py version: 3.7.2
CPython version: 2.7.12
OpenSSL version: OpenSSL 1.0.2g  1 Mar 2016
+ trap finish EXIT
+ proj=jenkins-trafficcontrol-traffic_ops-test-1614
++ pwd
+ 
compose=
+ cfile=traffic_ops/app/bin/tests/docker-compose.yml
+ [[ -z 

 ]]
+ [[ ! -x 

 ]]
+ 

 -p jenkins-trafficcontrol-traffic_ops-test-1614 -f 
traffic_ops/app/bin/tests/docker-compose.yml up --build --exit-code-from 
unit_golang unit_golang
using --exit-code-from implies --abort-on-container-exit
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ exit 1
+ finish
+ local st=1
+ [[ 1 -ne 0 ]]
+ echo 'Exiting with status 1'
Exiting with status 1
+ 

 -p jenkins-trafficcontrol-traffic_ops-test-1614 -f 
traffic_ops/app/bin/tests/docker-compose.yml down -v
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
Build step 'Execute shell' marked build as failure

[GitHub] [trafficcontrol] rawlinp closed issue #3790: Rewrite /federations to Go

[GitBox]

rawlinp closed issue #3790: Rewrite /federations to Go
URL: https://github.com/apache/trafficcontrol/issues/3790
 
 
   


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

[GitHub] [trafficcontrol] rawlinp merged pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

rawlinp merged pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015
 
 
   


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

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4030: Rewrite PUT /api/1.1/servers/:id/status to Go

[GitBox]

ocket commented on a change in pull request #4030: Rewrite PUT 
/api/1.1/servers/:id/status to Go
URL: https://github.com/apache/trafficcontrol/pull/4030#discussion_r340269064
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/server/put_status.go
 ##
 @@ -0,0 +1,137 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+   "database/sql"
+   "encoding/json"
+   "errors"
+   "fmt"
+   "net/http"
+   "strings"
+
+   "github.com/apache/trafficcontrol/lib/go-tc"
+   "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+   
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+func UpdateStatusHandler(w http.ResponseWriter, r *http.Request) {
+   inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id"}, 
[]string{"id"})
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
+   defer inf.Close()
+   reqObj := tc.ServerPutStatus{}
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, 
errors.New("malformed JSON: "+err.Error()), nil)
+   return
+   }
+
+   serverInfo, exists, err := dbhelpers.GetServerInfo(inf.IntParams["id"], 
inf.Tx.Tx)
+   if err != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, 
nil, err)
+   return
+   }
+   if !exists {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, 
fmt.Errorf("server ID %d not found", inf.IntParams["id"]), nil)
+   return
+   }
+
+   status := tc.StatusNullable{}
+   statusExists := false
+   if reqObj.Status.Name != nil {
+   status, statusExists, err = 
dbhelpers.GetStatusByName(*reqObj.Status.Name, inf.Tx.Tx)
+   } else if reqObj.Status.ID != nil {
+   status, statusExists, err = 
dbhelpers.GetStatusByID(*reqObj.Status.ID, inf.Tx.Tx)
+   } else {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, 
errors.New("status is required"), nil)
+   return
+   }
+   if err != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, 
nil, err)
+   return
+   }
+   if !statusExists {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, 
errors.New("invalid status (does not exist)"), nil)
+   return
+   }
+
+   if *status.Name == tc.CacheStatusAdminDown.String() || *status.Name == 
tc.CacheStatusOffline.String() {
+   if reqObj.OfflineReason == nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, 
errors.New("offlineReason is required for "+tc.CacheStatusAdminDown.String()+" 
or "+tc.CacheStatusOffline.String()+" status"), nil)
+   return
+   }
+   *reqObj.OfflineReason = inf.User.UserName + ": " + 
*reqObj.OfflineReason
+   } else {
+   reqObj.OfflineReason = nil
+   }
+   if err := updateServerStatusAndOfflineReason(inf.IntParams["id"], 
*status.ID, reqObj.OfflineReason, inf.Tx.Tx); err != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, 
nil, err)
+   return
+   }
+   offlineReason := ""
+   if reqObj.OfflineReason != nil {
+   offlineReason = *reqObj.OfflineReason
+   }
+   msg := "Updated status [ " + *status.Name + " ] for " + 
serverInfo.HostName + "." + serverInfo.DomainName + " [ " + offlineReason + " ]"
+
+   // queue updates on child servers if server is ^EDGE or ^MID
+   if strings.HasPrefix(serverInfo.Type, tc.CacheTypeEdge.String()) || 
strings.HasPrefix(serverInfo.Type, tc.CacheTypeMid.String()) {
 
 Review comment:
   Is this how Perl checked, with a strict prefix? I thought it was more like 
`/.*(MID|EDGE).*/` but I'll take your word for it.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to 

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4030: Rewrite PUT /api/1.1/servers/:id/status to Go

[GitBox]

ocket commented on a change in pull request #4030: Rewrite PUT 
/api/1.1/servers/:id/status to Go
URL: https://github.com/apache/trafficcontrol/pull/4030#discussion_r340265485
 
 

 ##
 File path: traffic_ops/testing/api/v14/serverupdatestatus_test.go
 ##
 @@ -0,0 +1,149 @@
+package v14
+
+/*
+
+   Licensed 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.
+*/
+
+import (
+   "testing"
+
+   "github.com/apache/trafficcontrol/lib/go-tc"
+   "github.com/apache/trafficcontrol/lib/go-util"
+)
+
+func TestServerUpdateStatus(t *testing.T) {
+   WithObjs(t, []TCObj{CDNs, Types, Parameters, Profiles, Statuses, 
Divisions, Regions, PhysLocations, CacheGroups, Servers}, func() {
+   UpdateTestServerStatus(t)
+   })
+}
+
+func UpdateTestServerStatus(t *testing.T) {
+
+   edge1cdn1 := tc.Server{}
+   edge2cdn1 := tc.Server{}
+   mid1cdn1 := tc.Server{}
+   edge1cdn2 := tc.Server{}
+
+   getServers := func() {
+   for _, s := range []struct {
+   name   string
+   server *tc.Server
+   }{
+   {
+   "atlanta-edge-01",
+   ,
+   },
+   {
+   "atlanta-edge-03",
+   ,
+   },
+   {
+   "atlanta-mid-16",
+   ,
+   },
+   {
+   "edge1-cdn2",
+   ,
+   },
+   } {
+   resp, _, err := TOSession.GetServerByHostName(s.name)
+   if err != nil {
+   t.Errorf("cannot GET Server by hostname: %v - 
%v\n", s.name, err)
+   }
+   *s.server = resp[0]
+   }
+   }
+   getServers()
+
+   // assert that servers don't have updates pending
+   for _, s := range []tc.Server{
+   edge1cdn1,
+   edge2cdn1,
+   mid1cdn1,
+   edge1cdn2,
+   } {
+   if s.UpdPending {
+   t.Errorf("expected UpdPending: false, actual: true")
+   }
+   }
+
+   // update status of MID server to OFFLINE
+   _, _, err := TOSession.UpdateServerStatus(mid1cdn1.ID, 
tc.ServerPutStatus{
+   Status:util.JSONNameOrIDStr{Name: 
util.StrPtr("OFFLINE")},
+   OfflineReason: util.StrPtr("testing")})
+   if err != nil {
+   t.Errorf("cannot update server status: %v", err)
+   }
+
+   // assert that updates were queued for the proper EDGE servers
+   getServers()
+   if !edge1cdn1.UpdPending {
+   t.Errorf("expected: child %s to have updates pending, actual: 
no updates pending", edge1cdn1.HostName)
+   }
+   if !edge2cdn1.UpdPending {
+   t.Errorf("expected: child %s to have updates pending, actual: 
no updates pending", edge2cdn1.HostName)
+   }
+   if mid1cdn1.UpdPending {
+   t.Errorf("expected: server %s with updated status to have no 
updates pending, actual: updates pending", mid1cdn1.HostName)
+   }
+   if edge1cdn2.UpdPending {
+   t.Errorf("expected: server %s in different CDN than server with 
updated status to have no updates pending, actual: updates pending", 
edge2cdn1.HostName)
+   }
+
+   // update status of MID server to OFFLINE via ID
+   status, _, err := TOSession.GetStatusByName("OFFLINE")
+   if err != nil {
+   t.Errorf("cannot GET status by name: %v", err)
 
 Review comment:
   error should be fatal, as when an error occurs `status` could be `nil` (in 
fact the way the method is written I think it has to be)


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

[GitHub] [trafficcontrol] asf-ci commented on issue #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

asf-ci commented on issue #4015: Rewrite /federations to Go - POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#issuecomment-547569594
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4665/
   


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

Build failed in Jenkins: trafficcontrol-PR #4665

[Apache Jenkins Server]

See 

Changes:


--
GitHub pull request #4015 of commit 879c4ed196152934e88452f2d1e9c04e6db092ea, 
no merge conflicts.
Running as SYSTEM
Setting status of 879c4ed196152934e88452f2d1e9c04e6db092ea to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4665/ and message: 'Build 
started for merge commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H39 (ubuntu xenial) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
Cloning the remote Git repository
Cloning repository git://github.com/apache/trafficcontrol.git
 > git init  # timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse 879c4ed196152934e88452f2d1e9c04e6db092ea^{commit} # timeout=10
Checking out Revision 879c4ed196152934e88452f2d1e9c04e6db092ea (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 879c4ed196152934e88452f2d1e9c04e6db092ea
Commit message: "Fix or instead of and"
 > git rev-list --no-walk e0337e9378ef11450ee617fd1aebf6c10c5c023c # timeout=10
First time build. Skipping changelog.
[trafficcontrol-PR] $ /bin/bash /tmp/jenkins8587316657309726088.sh
++ echo jenkins-trafficcontrol-PR-4665
++ sed s/-//g
++ sed s/jenkins//
+ proj=trafficcontrolPR4665
+ yml=infrastructure/docker/build/docker-compose.yml
++ mktemp /tmp/docker-compose-
+ dc=/tmp/docker-compose-ZYnr
++ mktemp /tmp/tc-status-
+ st=/tmp/tc-status-vtZZ
+ trap finish EXIT
++ uname -s
++ uname -m
+ curl -o /tmp/docker-compose-ZYnr -L 
https://github.com/docker/compose/releases/download/1.13.0/docker-compose-Linux-x86_64
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 
0100   6170   6170 0   1033  0 --:--:-- --:--:-- --:--:--  1035
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0 
55 8079k   55 4514k0 0  2759k  0  0:00:02  0:00:01  0:00:01 
4399k100 8079k  100 8079k0 0  4617k  0  0:00:01  0:00:01 --:--:-- 
7087k
+ chmod +x /tmp/docker-compose-ZYnr
+ rm -rf dist
+ /tmp/docker-compose-ZYnr -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4665 up
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ exit 1
+ finish
+ /tmp/docker-compose-ZYnr -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4665 down -v
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ /tmp/docker-compose-ZYnr -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4665 rm -v -f
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ rm -f /tmp/docker-compose-ZYnr
Build step 'Execute shell' marked build as failure
Skipped archiving because build is not successful

Jenkins build is back to normal : trafficcontrol-PR #4664

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] asf-ci commented on issue #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

asf-ci commented on issue #4015: Rewrite /federations to Go - POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#issuecomment-547565073
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4664/
   


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

Jenkins build is back to normal : trafficcontrol-master-build #1568

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

ocket commented on a change in pull request #4015: Rewrite /federations to 
Go - POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340247925
 
 

 ##
 File path: traffic_ops/app/conf/cdn.conf
 ##
 @@ -1,7 +1,7 @@
 {
 "hypnotoad" : {
 "listen" : [
-
"https://[::]:60443?cert=/etc/pki/tls/certs/localhost.crt=/etc/pki/tls/private/localhost.key=0x00=AES128-GCM-SHA256:HIGH:!RC4:!MD5:!aNULL:!EDH:!ED;
+
"https://[::]:60443?cert=/home/bwilli415/src/localhost.crt=/home/bwilli415/src/localhost.key=0x00=AES128-GCM-SHA256:HIGH:!RC4:!MD5:!aNULL:!EDH:!ED;
 
 Review comment:
   whoops. Must've popped that for running tests - should look for others like 
that
   


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

[GitHub] [trafficcontrol] ocket8888 closed issue #3847: Rewrite /cache_stats to Go

[GitBox]

ocket closed issue #3847: Rewrite /cache_stats to Go
URL: https://github.com/apache/trafficcontrol/issues/3847
 
 
   


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

[GitHub] [trafficcontrol] ocket8888 merged pull request #4029: Rewrite cache_stats to Golang

[GitBox]

ocket merged pull request #4029: Rewrite cache_stats to Golang
URL: https://github.com/apache/trafficcontrol/pull/4029
 
 
   


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

[GitHub] [trafficcontrol] rawlinp merged pull request #4037: Docs/influx fix

[GitBox]

rawlinp merged pull request #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037
 
 
   


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

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

rawlinp commented on a change in pull request #4015: Rewrite /federations to Go 
- POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340242557
 
 

 ##
 File path: traffic_ops/app/conf/cdn.conf
 ##
 @@ -1,7 +1,7 @@
 {
 "hypnotoad" : {
 "listen" : [
-
"https://[::]:60443?cert=/etc/pki/tls/certs/localhost.crt=/etc/pki/tls/private/localhost.key=0x00=AES128-GCM-SHA256:HIGH:!RC4:!MD5:!aNULL:!EDH:!ED;
+
"https://[::]:60443?cert=/home/bwilli415/src/localhost.crt=/home/bwilli415/src/localhost.key=0x00=AES128-GCM-SHA256:HIGH:!RC4:!MD5:!aNULL:!EDH:!ED;
 
 Review comment:
   I think you may have accidentally committed these files


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

[GitHub] [trafficcontrol] lbathina commented on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

lbathina commented on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-547551139
 
 
   same applies to deleting a server capability associated to a delivery 
service 


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

[GitHub] [trafficcontrol] lbathina edited a comment on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

lbathina edited a comment on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-547551139
 
 
   same issue appears while deleting a server capability associated to a 
delivery service 


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

Jenkins build is back to normal : trafficcontrol-traffic_ops-test #1611

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] asf-ci commented on issue #4037: Docs/influx fix

[GitBox]

asf-ci commented on issue #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037#issuecomment-547550071
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4663/
   


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

Build failed in Jenkins: trafficcontrol-PR #4663

[Apache Jenkins Server]

See 


Changes:

[ocket] Moved some images, unset executable bit and shebang on 
non-executable

[ocket] Fixed some documentation warnings generated from Python files

[ocket] Added kickstart setup QHT

[ocket] Added cache group / region /division overview

[ocket] Moved traffic ops administration into a single sub-section

[ocket] Fix Influx config defaulting to secure

[ocket] Fix inaccurate link, add versionadded directive to smtp config 
section

[ocket] clarify the purpose of 'workers'


--
GitHub pull request #4037 of commit abfd10a97fd57003faa5968dff31ba35d892b32e, 
no merge conflicts.
Running as SYSTEM
Setting status of abfd10a97fd57003faa5968dff31ba35d892b32e to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4663/ and message: 'Build 
started for merge commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H39 (ubuntu xenial) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
Cloning the remote Git repository
Cloning repository git://github.com/apache/trafficcontrol.git
 > git init  # timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse abfd10a97fd57003faa5968dff31ba35d892b32e^{commit} # timeout=10
Checking out Revision abfd10a97fd57003faa5968dff31ba35d892b32e (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f abfd10a97fd57003faa5968dff31ba35d892b32e
Commit message: "clarify the purpose of 'workers'"
 > git rev-list --no-walk 82f1af523fd316385403f1914bf80849d5b16c57 # timeout=10
[trafficcontrol-PR] $ /bin/bash /tmp/jenkins6146903471656486062.sh
++ echo jenkins-trafficcontrol-PR-4663
++ sed s/-//g
++ sed s/jenkins//
+ proj=trafficcontrolPR4663
+ yml=infrastructure/docker/build/docker-compose.yml
++ mktemp /tmp/docker-compose-
+ dc=/tmp/docker-compose-6R1t
++ mktemp /tmp/tc-status-
+ st=/tmp/tc-status-iJnk
+ trap finish EXIT
++ uname -s
++ uname -m
+ curl -o /tmp/docker-compose-6R1t -L 
https://github.com/docker/compose/releases/download/1.13.0/docker-compose-Linux-x86_64
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0  
0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0100 
  6170   6170 0   1003  0 --:--:-- --:--:-- --:--:--  1003
 13 8079k   13 1052k0 0   699k  0  0:00:11  0:00:01  0:00:10  
699k100 8079k  100 8079k0 0  4697k  0  0:00:01  0:00:01 --:--:-- 
31.9M
+ chmod +x /tmp/docker-compose-6R1t
+ rm -rf dist
+ /tmp/docker-compose-6R1t -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4663 up
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ exit 1
+ finish
+ /tmp/docker-compose-6R1t -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4663 down -v
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ /tmp/docker-compose-6R1t -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4663 rm -v -f
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ rm -f /tmp/docker-compose-6R1t
Build step 'Execute shell' marked build as failure
Skipped archiving because build is not successful

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

ocket commented on a change in pull request #4015: Rewrite /federations to 
Go - POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340233091
 
 

 ##
 File path: docs/source/api/federations.rst
 ##
 @@ -150,82 +173,122 @@ Request Structure
 -
 No parameters available
 
+.. code-block:: http
+   :caption: Request Example
+
+   DELETE /api/1.4/federations HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+
 Response Structure
 --
 .. code-block:: http
:caption: Response Example
 
HTTP/1.1 200 OK
Access-Control-Allow-Credentials: true
-   Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept
+   Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept, Set-Cookie, Cookie
Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
Access-Control-Allow-Origin: *
-   Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Content-Type: application/json
-   Date: Mon, 03 Dec 2018 17:55:10 GMT
-   Server: Mojolicious (Perl)
-   Set-Cookie: mojolicious=...; expires=Mon, 03 Dec 2018 21:55:10 GMT; 
path=/; HttpOnly
-   Vary: Accept-Encoding
-   Whole-Content-Sha512: 
b84HraJH6Kiqrz7i1L1juDBJWdkdYbbClnWM0lZDljvpSkVT9adFTTrHiv7Mjtt2RKquGdzFZ6tqt9s+ODxqsw==
-   Content-Length: 93
-
-   { "response": "admin successfully deleted all federation resolvers: [ 
0.0.0.0/32, ::/128 ]." }
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
fd7P45mIiHuYqZZW6+8K+YjY1Pe504Aaw4J4Zp9AhrqLX72ERytTqWtAp1msutzNSRUdUSC72+odNPtpv3O8uw==
+   X-Server-Name: traffic_ops_golang/
+   Date: Wed, 23 Oct 2019 23:34:53 GMT
+   Content-Length: 184
 
+   { "alerts": [
+   {
+   "text": "admin successfully deleted all federation 
resolvers: [ 8.8.8.8 ]",
+   "level": "success"
+   }
+   ],
+   "response": "admin successfully deleted all federation resolvers: [ 
8.8.8.8 ]"
+   }
 
 ``PUT``
 ===
-Replaces **all** federations associated with a user's :term:`Delivery 
Service`\ (s) with those defined inside the request payload.
+Replaces **all** :term:`Federations` associated with a user's :term:`Delivery 
Service`\ (s) with those defined inside the request payload.
 
 :Auth. Required: Yes
 :Roles Required: "admin", "Federation", "operations", "Portal", or "Steering"
 :Response Type:  Object (string)
 
 Request Structure
 -
-:federations: The top-level key that must exist - an array of objects that 
each describe a set of resolvers for a :term:`Delivery Service`'s federation
+.. versionchanged:: 1.4
+   Prior to API version 1.4, the request body had to be wrapped in a 
top-level ``federations`` key, as can be seen in the :ref:`legacy-put-request` 
example. That behavior is still supported but no longer necessary.
 
-   :deliveryService: The 'xml_id' of the :term:`Delivery Service` which 
will use the federation resolvers specified in ``mappings``
-   :mappings:An object containing two arrays of IP addresses to 
use as federation resolvers
+.. _legacy-put-request:
+.. code-block:: json
+   :caption: Legacy Request
 
-   :resolve4: An array of IPv4 addresses that can resolve the 
:term:`Delivery Service`'s federation
-   :resolve6: An array of IPv6 addresses that can resolve the 
:term:`Delivery Service`'s federation
+   {
+   "federations": [{
+   "deliveryService": "demo1",
+   "mappings": {
+   "resolve4": ["0.0.0.0"],
+   "resolve6": ["::1"]
+   }
+   }]
+   }
+
+The request payload is an array of objects that describe Delivery Service 
:term:`Federation` Resolver mappings. Each object in the array must be in the 
following format.
+
+:deliveryService: The :ref:`ds-xmlid` of the :term:`Delivery Service` which 
will use the :term:`Federation` Resolvers specified in ``mappings``
+:mappings:An object containing two arrays of IP addresses (or subnets 
in :abbr:`CIDR (Classless Inter-Domain Routing)` notation) to use as 
:term:`Federation` Resolvers
+
+   :resolve4: An array of IPv4 addresses (or subnets in :abbr:`CIDR 
(Classless Inter-Domain Routing)` notation) that can resolve the 
:term:`Delivery Service`'s :term:`Federation`
+   :resolve6: An array of IPv6 addresses (or subnets in :abbr:`CIDR 
(Classless Inter-Domain Routing)` notation) that can resolve the 
:term:`Delivery Service`'s :term:`Federation`
 
 .. code-block:: http
:caption: Request Example
 
PUT /api/1.4/federations HTTP/1.1
Host: trafficops.infra.ciab.test
-   User-Agent: curl/7.62.0
+   

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4037: Docs/influx fix

[GitBox]

ocket commented on a change in pull request #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037#discussion_r340220705
 
 

 ##
 File path: docs/source/admin/traffic_ops.rst
 ##
 @@ -0,0 +1,708 @@
+..
+..
+.. Licensed 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.
+..
+
+.. role:: bash(code)
+   :language: bash
+
+***
+Traffic Ops
+***
+Traffic Ops is quite possible the single most complex and most important 
Traffic Control component. It has many different configuration options that 
affect a wide range of other components and their interactions.
+
+.. _to-install:
+
+Installing
+==
+
+System Requirements
+---
+The user must have the following for a successful minimal install:
+
+- CentOS 7+
+- Two machines - physical or virtual -, each with at least two (v)CPUs, 4GB of 
RAM, and 20 GB of disk space
+- Access to CentOS Base and EPEL :manpage:`yum(8)` repositories
+- Access to `The Comprehensive Perl Archive Network (CPAN) 
`_
+
+Guide
+-
+#. Install PostgreSQL Database. For a production install it is best to install 
PostgreSQL on its own server/virtual machine.
+
+   .. seealso:: For more information on installing PostgreSQL, see `their 
documentation `_.
+
+   .. code-block:: shell
+   :caption: Example PostgreSQL Install Procedure
+
+   yum update -y
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+   yum install -y postgresql96-server
+   su - postgres -c '/usr/pgsql-9.6/bin/initdb -A md5 -W' #-W 
forces the user to provide a superuser (postgres) password
+
+
+#. Edit :file:`/var/lib/pgsql/9.6/data/pg_hba.conf` to allow the Traffic Ops 
instance to access the PostgreSQL server. For example, if the IP address of the 
machine to be used as the Traffic Ops host is ``192.0.2.1`` add the line ``host 
 all   all 192.0.2.1/32 md5`` to the appropriate section of this file.
+
+#. Edit the :file:`/var/lib/pgsql/9.6/data/postgresql.conf` file to add the 
appropriate listen_addresses or ``listen_addresses = '*'``, set ``timezone = 
'UTC'``, and start the database
+
+   .. code-block:: shell
+   :caption: Starting PostgreSQL with :manpage:`systemd(1)`
+
+   systemctl enable postgresql-9.6
+   systemctl start postgresql-9.6
+   systemctl status postgresql-9.6 # Prints the status of the 
PostgreSQL service, to prove it's running
+
+
+#. Build a :file:`traffic_ops-{version string}.rpm` file using the 
instructions under the :ref:`dev-building` page - or download a pre-built 
release from `the Apache Continuous Integration server 
`_.
+
+#. Install a PostgreSQL client on the Traffic Ops host
+
+   .. code-block:: shell
+   :caption: Installing PostgreSQL Client from a Hosted Source
+
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+
+#. Install the Traffic Ops RPM. The Traffic Ops RPM file should have been 
built in an earlier step.
+
+   .. code-block:: shell
+   :caption: Installing a Generated Traffic Ops RPM
+
+   yum install -y 
./dist/traffic_ops-3.0.0-.yyy.el7.x86_64.rpm
+
+   .. note:: This will install the PostgreSQL client, ``psql`` as a 
dependency.
+
+#. Login to the Database from the Traffic Ops machine. At this point you 
should be able to login from the Traffic Ops (hostname ``to`` in the example) 
host to the PostgreSQL (hostname ``pg`` in the example) host
+
+   .. code-block:: psql
+   :caption: Example Login to Traffic Ops Database from Traffic 
Ops Server
+
+   to-# psql -h pg -U postgres
+   Password for user postgres:
+   psql (9.6.3)
+   Type "help" for help.
+
+   postgres=#
+
+
+#. Create the user and database. By default, Traffic Ops will expect to 
connect as the ``traffic_ops`` user to the ``traffic_ops`` database.
+
+   .. code-block:: console
+   :caption: Creating the Traffic Ops User and Database
+
+   to-# psql -U postgres -h pg -c "CREATE USER traffic_ops WITH 
ENCRYPTED PASSWORD 'tcr0cks';"
+   Password for user postgres:
+  

[GitHub] [trafficcontrol] asf-ci commented on issue #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

asf-ci commented on issue #4015: Rewrite /federations to Go - POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#issuecomment-547534429
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4662/
   


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

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4037: Docs/influx fix

[GitBox]

rawlinp commented on a change in pull request #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037#discussion_r340199081
 
 

 ##
 File path: docs/source/admin/traffic_ops.rst
 ##
 @@ -0,0 +1,708 @@
+..
+..
+.. Licensed 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.
+..
+
+.. role:: bash(code)
+   :language: bash
+
+***
+Traffic Ops
+***
+Traffic Ops is quite possible the single most complex and most important 
Traffic Control component. It has many different configuration options that 
affect a wide range of other components and their interactions.
+
+.. _to-install:
+
+Installing
+==
+
+System Requirements
+---
+The user must have the following for a successful minimal install:
+
+- CentOS 7+
+- Two machines - physical or virtual -, each with at least two (v)CPUs, 4GB of 
RAM, and 20 GB of disk space
+- Access to CentOS Base and EPEL :manpage:`yum(8)` repositories
+- Access to `The Comprehensive Perl Archive Network (CPAN) 
`_
+
+Guide
+-
+#. Install PostgreSQL Database. For a production install it is best to install 
PostgreSQL on its own server/virtual machine.
+
+   .. seealso:: For more information on installing PostgreSQL, see `their 
documentation `_.
+
+   .. code-block:: shell
+   :caption: Example PostgreSQL Install Procedure
+
+   yum update -y
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+   yum install -y postgresql96-server
+   su - postgres -c '/usr/pgsql-9.6/bin/initdb -A md5 -W' #-W 
forces the user to provide a superuser (postgres) password
+
+
+#. Edit :file:`/var/lib/pgsql/9.6/data/pg_hba.conf` to allow the Traffic Ops 
instance to access the PostgreSQL server. For example, if the IP address of the 
machine to be used as the Traffic Ops host is ``192.0.2.1`` add the line ``host 
 all   all 192.0.2.1/32 md5`` to the appropriate section of this file.
+
+#. Edit the :file:`/var/lib/pgsql/9.6/data/postgresql.conf` file to add the 
appropriate listen_addresses or ``listen_addresses = '*'``, set ``timezone = 
'UTC'``, and start the database
+
+   .. code-block:: shell
+   :caption: Starting PostgreSQL with :manpage:`systemd(1)`
+
+   systemctl enable postgresql-9.6
+   systemctl start postgresql-9.6
+   systemctl status postgresql-9.6 # Prints the status of the 
PostgreSQL service, to prove it's running
+
+
+#. Build a :file:`traffic_ops-{version string}.rpm` file using the 
instructions under the :ref:`dev-building` page - or download a pre-built 
release from `the Apache Continuous Integration server 
`_.
+
+#. Install a PostgreSQL client on the Traffic Ops host
+
+   .. code-block:: shell
+   :caption: Installing PostgreSQL Client from a Hosted Source
+
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+
+#. Install the Traffic Ops RPM. The Traffic Ops RPM file should have been 
built in an earlier step.
+
+   .. code-block:: shell
+   :caption: Installing a Generated Traffic Ops RPM
+
+   yum install -y 
./dist/traffic_ops-3.0.0-.yyy.el7.x86_64.rpm
+
+   .. note:: This will install the PostgreSQL client, ``psql`` as a 
dependency.
+
+#. Login to the Database from the Traffic Ops machine. At this point you 
should be able to login from the Traffic Ops (hostname ``to`` in the example) 
host to the PostgreSQL (hostname ``pg`` in the example) host
+
+   .. code-block:: psql
+   :caption: Example Login to Traffic Ops Database from Traffic 
Ops Server
+
+   to-# psql -h pg -U postgres
+   Password for user postgres:
+   psql (9.6.3)
+   Type "help" for help.
+
+   postgres=#
+
+
+#. Create the user and database. By default, Traffic Ops will expect to 
connect as the ``traffic_ops`` user to the ``traffic_ops`` database.
+
+   .. code-block:: console
+   :caption: Creating the Traffic Ops User and Database
+
+   to-# psql -U postgres -h pg -c "CREATE USER traffic_ops WITH 
ENCRYPTED PASSWORD 'tcr0cks';"
+   Password for user postgres:
+   

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

ocket commented on a change in pull request #4015: Rewrite /federations to 
Go - POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340198521
 
 

 ##
 File path: docs/source/api/federations.rst
 ##
 @@ -150,82 +173,122 @@ Request Structure
 -
 No parameters available
 
+.. code-block:: http
+   :caption: Request Example
+
+   DELETE /api/1.4/federations HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+
 Response Structure
 --
 .. code-block:: http
:caption: Response Example
 
HTTP/1.1 200 OK
Access-Control-Allow-Credentials: true
-   Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept
+   Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept, Set-Cookie, Cookie
Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
Access-Control-Allow-Origin: *
-   Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Content-Type: application/json
-   Date: Mon, 03 Dec 2018 17:55:10 GMT
-   Server: Mojolicious (Perl)
-   Set-Cookie: mojolicious=...; expires=Mon, 03 Dec 2018 21:55:10 GMT; 
path=/; HttpOnly
-   Vary: Accept-Encoding
-   Whole-Content-Sha512: 
b84HraJH6Kiqrz7i1L1juDBJWdkdYbbClnWM0lZDljvpSkVT9adFTTrHiv7Mjtt2RKquGdzFZ6tqt9s+ODxqsw==
-   Content-Length: 93
-
-   { "response": "admin successfully deleted all federation resolvers: [ 
0.0.0.0/32, ::/128 ]." }
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
fd7P45mIiHuYqZZW6+8K+YjY1Pe504Aaw4J4Zp9AhrqLX72ERytTqWtAp1msutzNSRUdUSC72+odNPtpv3O8uw==
+   X-Server-Name: traffic_ops_golang/
+   Date: Wed, 23 Oct 2019 23:34:53 GMT
+   Content-Length: 184
 
+   { "alerts": [
+   {
+   "text": "admin successfully deleted all federation 
resolvers: [ 8.8.8.8 ]",
+   "level": "success"
+   }
+   ],
+   "response": "admin successfully deleted all federation resolvers: [ 
8.8.8.8 ]"
+   }
 
 ``PUT``
 ===
-Replaces **all** federations associated with a user's :term:`Delivery 
Service`\ (s) with those defined inside the request payload.
+Replaces **all** :term:`Federations` associated with a user's :term:`Delivery 
Service`\ (s) with those defined inside the request payload.
 
 :Auth. Required: Yes
 :Roles Required: "admin", "Federation", "operations", "Portal", or "Steering"
 :Response Type:  Object (string)
 
 Request Structure
 -
-:federations: The top-level key that must exist - an array of objects that 
each describe a set of resolvers for a :term:`Delivery Service`'s federation
+.. versionchanged:: 1.4
+   Prior to API version 1.4, the request body had to be wrapped in a 
top-level ``federations`` key, as can be seen in the :ref:`legacy-put-request` 
example. That behavior is still supported but no longer necessary.
 
-   :deliveryService: The 'xml_id' of the :term:`Delivery Service` which 
will use the federation resolvers specified in ``mappings``
-   :mappings:An object containing two arrays of IP addresses to 
use as federation resolvers
+.. _legacy-put-request:
+.. code-block:: json
+   :caption: Legacy Request
 
-   :resolve4: An array of IPv4 addresses that can resolve the 
:term:`Delivery Service`'s federation
-   :resolve6: An array of IPv6 addresses that can resolve the 
:term:`Delivery Service`'s federation
+   {
+   "federations": [{
+   "deliveryService": "demo1",
+   "mappings": {
+   "resolve4": ["0.0.0.0"],
+   "resolve6": ["::1"]
+   }
+   }]
+   }
+
+The request payload is an array of objects that describe Delivery Service 
:term:`Federation` Resolver mappings. Each object in the array must be in the 
following format.
+
+:deliveryService: The :ref:`ds-xmlid` of the :term:`Delivery Service` which 
will use the :term:`Federation` Resolvers specified in ``mappings``
+:mappings:An object containing two arrays of IP addresses (or subnets 
in :abbr:`CIDR (Classless Inter-Domain Routing)` notation) to use as 
:term:`Federation` Resolvers
+
+   :resolve4: An array of IPv4 addresses (or subnets in :abbr:`CIDR 
(Classless Inter-Domain Routing)` notation) that can resolve the 
:term:`Delivery Service`'s :term:`Federation`
+   :resolve6: An array of IPv6 addresses (or subnets in :abbr:`CIDR 
(Classless Inter-Domain Routing)` notation) that can resolve the 
:term:`Delivery Service`'s :term:`Federation`
 
 .. code-block:: http
:caption: Request Example
 
PUT /api/1.4/federations HTTP/1.1
Host: trafficops.infra.ciab.test
-   User-Agent: curl/7.62.0
+   

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

rawlinp commented on a change in pull request #4015: Rewrite /federations to Go 
- POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340189103
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/federations/federations.go
 ##
 @@ -167,3 +206,296 @@ ORDER BY
}
return feds, nil
 }
+
+// AddFederationResorverMappingsForCurrentUser is the handler for a POST 
request to /federations.
+// Confusingly, it does not create a federation, but is instead used to 
manipulate the resolvers
+// used by one or more particular Delivery Services for one or more particular 
Federations.
+func AddFederationResolverMappingsForCurrentUser(w http.ResponseWriter, r 
*http.Request) {
+   inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+   tx := inf.Tx.Tx
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+   defer inf.Close()
+
+   mappings, userErr, sysErr := getMappingsFromRequestBody(*inf.Version, 
r.Body)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, sysErr)
+   return
+   }
+
+   if err := mappings.Validate(tx); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("validating request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   userErr, sysErr, errCode = 
addFederationResolverMappingsForCurrentUser(inf.User, tx, mappings)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+
+   msg := fmt.Sprintf("%s successfully created federation resolvers.", 
inf.User.UserName)
+   if inf.Version.Minor <= 3 {
+   api.WriteResp(w, r, msg)
+   } else {
+   api.WriteRespAlertObj(w, r, tc.SuccessLevel, msg, msg)
+   }
+}
+
+// handles the main logic of the POST handler, separated out for convenience
+func addFederationResolverMappingsForCurrentUser(u *auth.CurrentUser, tx 
*sql.Tx, mappings []tc.DeliveryServiceFederationResolverMapping) (error, error, 
int) {
+   for _, fed := range mappings {
+   dsTenant, ok, err := dbhelpers.GetDSTenantIDFromXMLID(tx, 
fed.DeliveryService)
+   if err != nil {
+   return nil, err, http.StatusInternalServerError
+   } else if !ok {
+   return fmt.Errorf("'%s' - no such Delivery Service", 
fed.DeliveryService), nil, http.StatusConflict
+   }
+
+   if ok, err = tenant.IsResourceAuthorizedToUserTx(dsTenant, u, 
tx); err != nil {
+   err = fmt.Errorf("Checking user #%d tenancy permissions 
on DS '%s' (tenant #%d)", u.ID, fed.DeliveryService, dsTenant)
+   return nil, err, http.StatusInternalServerError
+   } else if !ok {
+   userErr := fmt.Errorf("'%s' - no such Delivery 
Service", fed.DeliveryService)
+   sysErr := fmt.Errorf("User '%s' requested unauthorized 
federation resolver mapping modification on the '%s' Delivery Service", 
u.UserName, fed.DeliveryService)
+   return userErr, sysErr, http.StatusConflict
+   }
+
+   fedID, ok, err := dbhelpers.GetFederationIDForUserIDByXMLID(tx, 
u.ID, fed.DeliveryService)
+   if err != nil {
+   return nil, fmt.Errorf("Getting Federation ID: %v", 
err), http.StatusInternalServerError
+   } else if !ok {
+   err = fmt.Errorf("No federation(s) found for user %s on 
delivery service '%s'.", u.UserName, fed.DeliveryService)
+   return err, nil, http.StatusConflict
+   }
+
+   inserted, err := 
addFederationResolverMappingsToFederation(fed.Mappings, fed.DeliveryService, 
fedID, tx)
+   if err != nil {
+   err = fmt.Errorf("Adding federation resolver mapping(s) 
to federation: %v", err)
+   return nil, err, http.StatusInternalServerError
+   }
+
+   changelogMsg := "FEDERATION DELIVERY SERVICE: %s, ID: %d, 
ACTION: User %s successfully added federation resolvers [ %s ]"
+   changelogMsg = fmt.Sprintf(changelogMsg, fed.DeliveryService, 
fedID, u.UserName, inserted)
+   api.CreateChangeLogRawTx(api.ApiChange, changelogMsg, u, tx)
+   }
+   return nil, nil, http.StatusOK
+}
+
+// adds federation resolver mappings for a particular delivery service to a 
given federation, creating said resolvers if
+// they don't already exist.
+func addFederationResolverMappingsToFederation(res tc.ResolverMapping, xmlid 
string, fed uint, tx *sql.Tx) (string, error) {
+   var resp string
+   if len(res.Resolve4) > 0 {
+  

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

rawlinp commented on a change in pull request #4015: Rewrite /federations to Go 
- POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340187114
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/federations/federations.go
 ##
 @@ -167,3 +206,296 @@ ORDER BY
}
return feds, nil
 }
+
+// AddFederationResorverMappingsForCurrentUser is the handler for a POST 
request to /federations.
+// Confusingly, it does not create a federation, but is instead used to 
manipulate the resolvers
+// used by one or more particular Delivery Services for one or more particular 
Federations.
+func AddFederationResolverMappingsForCurrentUser(w http.ResponseWriter, r 
*http.Request) {
+   inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+   tx := inf.Tx.Tx
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+   defer inf.Close()
+
+   mappings, userErr, sysErr := getMappingsFromRequestBody(*inf.Version, 
r.Body)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, sysErr)
+   return
+   }
+
+   if err := mappings.Validate(tx); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("validating request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   userErr, sysErr, errCode = 
addFederationResolverMappingsForCurrentUser(inf.User, tx, mappings)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+
+   msg := fmt.Sprintf("%s successfully created federation resolvers.", 
inf.User.UserName)
+   if inf.Version.Minor <= 3 {
+   api.WriteResp(w, r, msg)
+   } else {
+   api.WriteRespAlertObj(w, r, tc.SuccessLevel, msg, msg)
+   }
+}
+
+// handles the main logic of the POST handler, separated out for convenience
+func addFederationResolverMappingsForCurrentUser(u *auth.CurrentUser, tx 
*sql.Tx, mappings []tc.DeliveryServiceFederationResolverMapping) (error, error, 
int) {
+   for _, fed := range mappings {
+   dsTenant, ok, err := dbhelpers.GetDSTenantIDFromXMLID(tx, 
fed.DeliveryService)
+   if err != nil {
+   return nil, err, http.StatusInternalServerError
+   } else if !ok {
+   return fmt.Errorf("'%s' - no such Delivery Service", 
fed.DeliveryService), nil, http.StatusConflict
+   }
+
+   if ok, err = tenant.IsResourceAuthorizedToUserTx(dsTenant, u, 
tx); err != nil {
+   err = fmt.Errorf("Checking user #%d tenancy permissions 
on DS '%s' (tenant #%d)", u.ID, fed.DeliveryService, dsTenant)
+   return nil, err, http.StatusInternalServerError
+   } else if !ok {
+   userErr := fmt.Errorf("'%s' - no such Delivery 
Service", fed.DeliveryService)
+   sysErr := fmt.Errorf("User '%s' requested unauthorized 
federation resolver mapping modification on the '%s' Delivery Service", 
u.UserName, fed.DeliveryService)
+   return userErr, sysErr, http.StatusConflict
+   }
+
+   fedID, ok, err := dbhelpers.GetFederationIDForUserIDByXMLID(tx, 
u.ID, fed.DeliveryService)
+   if err != nil {
+   return nil, fmt.Errorf("Getting Federation ID: %v", 
err), http.StatusInternalServerError
+   } else if !ok {
+   err = fmt.Errorf("No federation(s) found for user %s on 
delivery service '%s'.", u.UserName, fed.DeliveryService)
+   return err, nil, http.StatusConflict
+   }
+
+   inserted, err := 
addFederationResolverMappingsToFederation(fed.Mappings, fed.DeliveryService, 
fedID, tx)
+   if err != nil {
+   err = fmt.Errorf("Adding federation resolver mapping(s) 
to federation: %v", err)
+   return nil, err, http.StatusInternalServerError
+   }
+
+   changelogMsg := "FEDERATION DELIVERY SERVICE: %s, ID: %d, 
ACTION: User %s successfully added federation resolvers [ %s ]"
+   changelogMsg = fmt.Sprintf(changelogMsg, fed.DeliveryService, 
fedID, u.UserName, inserted)
+   api.CreateChangeLogRawTx(api.ApiChange, changelogMsg, u, tx)
+   }
+   return nil, nil, http.StatusOK
+}
+
+// adds federation resolver mappings for a particular delivery service to a 
given federation, creating said resolvers if
+// they don't already exist.
+func addFederationResolverMappingsToFederation(res tc.ResolverMapping, xmlid 
string, fed uint, tx *sql.Tx) (string, error) {
+   var resp string
+   if len(res.Resolve4) > 0 {
+  

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

rawlinp commented on a change in pull request #4015: Rewrite /federations to Go 
- POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340193455
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/federations/federations.go
 ##
 @@ -167,3 +206,296 @@ ORDER BY
}
return feds, nil
 }
+
+// AddFederationResorverMappingsForCurrentUser is the handler for a POST 
request to /federations.
+// Confusingly, it does not create a federation, but is instead used to 
manipulate the resolvers
+// used by one or more particular Delivery Services for one or more particular 
Federations.
+func AddFederationResolverMappingsForCurrentUser(w http.ResponseWriter, r 
*http.Request) {
+   inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+   tx := inf.Tx.Tx
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+   defer inf.Close()
+
+   mappings, userErr, sysErr := getMappingsFromRequestBody(*inf.Version, 
r.Body)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, sysErr)
+   return
+   }
+
+   if err := mappings.Validate(tx); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("validating request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   userErr, sysErr, errCode = 
addFederationResolverMappingsForCurrentUser(inf.User, tx, mappings)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+
+   msg := fmt.Sprintf("%s successfully created federation resolvers.", 
inf.User.UserName)
+   if inf.Version.Minor <= 3 {
+   api.WriteResp(w, r, msg)
+   } else {
+   api.WriteRespAlertObj(w, r, tc.SuccessLevel, msg, msg)
+   }
+}
+
+// handles the main logic of the POST handler, separated out for convenience
+func addFederationResolverMappingsForCurrentUser(u *auth.CurrentUser, tx 
*sql.Tx, mappings []tc.DeliveryServiceFederationResolverMapping) (error, error, 
int) {
+   for _, fed := range mappings {
+   dsTenant, ok, err := dbhelpers.GetDSTenantIDFromXMLID(tx, 
fed.DeliveryService)
+   if err != nil {
+   return nil, err, http.StatusInternalServerError
+   } else if !ok {
+   return fmt.Errorf("'%s' - no such Delivery Service", 
fed.DeliveryService), nil, http.StatusConflict
+   }
+
+   if ok, err = tenant.IsResourceAuthorizedToUserTx(dsTenant, u, 
tx); err != nil {
+   err = fmt.Errorf("Checking user #%d tenancy permissions 
on DS '%s' (tenant #%d)", u.ID, fed.DeliveryService, dsTenant)
+   return nil, err, http.StatusInternalServerError
+   } else if !ok {
+   userErr := fmt.Errorf("'%s' - no such Delivery 
Service", fed.DeliveryService)
+   sysErr := fmt.Errorf("User '%s' requested unauthorized 
federation resolver mapping modification on the '%s' Delivery Service", 
u.UserName, fed.DeliveryService)
+   return userErr, sysErr, http.StatusConflict
+   }
+
+   fedID, ok, err := dbhelpers.GetFederationIDForUserIDByXMLID(tx, 
u.ID, fed.DeliveryService)
+   if err != nil {
+   return nil, fmt.Errorf("Getting Federation ID: %v", 
err), http.StatusInternalServerError
+   } else if !ok {
+   err = fmt.Errorf("No federation(s) found for user %s on 
delivery service '%s'.", u.UserName, fed.DeliveryService)
+   return err, nil, http.StatusConflict
+   }
+
+   inserted, err := 
addFederationResolverMappingsToFederation(fed.Mappings, fed.DeliveryService, 
fedID, tx)
+   if err != nil {
+   err = fmt.Errorf("Adding federation resolver mapping(s) 
to federation: %v", err)
+   return nil, err, http.StatusInternalServerError
+   }
+
+   changelogMsg := "FEDERATION DELIVERY SERVICE: %s, ID: %d, 
ACTION: User %s successfully added federation resolvers [ %s ]"
+   changelogMsg = fmt.Sprintf(changelogMsg, fed.DeliveryService, 
fedID, u.UserName, inserted)
+   api.CreateChangeLogRawTx(api.ApiChange, changelogMsg, u, tx)
+   }
+   return nil, nil, http.StatusOK
+}
+
+// adds federation resolver mappings for a particular delivery service to a 
given federation, creating said resolvers if
+// they don't already exist.
+func addFederationResolverMappingsToFederation(res tc.ResolverMapping, xmlid 
string, fed uint, tx *sql.Tx) (string, error) {
+   var resp string
+   if len(res.Resolve4) > 0 {
+  

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

rawlinp commented on a change in pull request #4015: Rewrite /federations to Go 
- POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340160646
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/federations/federations.go
 ##
 @@ -167,3 +206,296 @@ ORDER BY
}
return feds, nil
 }
+
+// AddFederationResorverMappingsForCurrentUser is the handler for a POST 
request to /federations.
+// Confusingly, it does not create a federation, but is instead used to 
manipulate the resolvers
+// used by one or more particular Delivery Services for one or more particular 
Federations.
+func AddFederationResolverMappingsForCurrentUser(w http.ResponseWriter, r 
*http.Request) {
+   inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+   tx := inf.Tx.Tx
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+   defer inf.Close()
+
+   mappings, userErr, sysErr := getMappingsFromRequestBody(*inf.Version, 
r.Body)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, sysErr)
+   return
+   }
+
+   if err := mappings.Validate(tx); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("validating request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   userErr, sysErr, errCode = 
addFederationResolverMappingsForCurrentUser(inf.User, tx, mappings)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+
+   msg := fmt.Sprintf("%s successfully created federation resolvers.", 
inf.User.UserName)
+   if inf.Version.Minor <= 3 {
 
 Review comment:
   this should probably also check the major version in case we ever decide to 
"graduate" this API to 2.0


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

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

[GitBox]

rawlinp commented on a change in pull request #4015: Rewrite /federations to Go 
- POST/PUT/DELETE
URL: https://github.com/apache/trafficcontrol/pull/4015#discussion_r340145359
 
 

 ##
 File path: docs/source/api/federations.rst
 ##
 @@ -150,82 +173,122 @@ Request Structure
 -
 No parameters available
 
+.. code-block:: http
+   :caption: Request Example
+
+   DELETE /api/1.4/federations HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+
 Response Structure
 --
 .. code-block:: http
:caption: Response Example
 
HTTP/1.1 200 OK
Access-Control-Allow-Credentials: true
-   Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept
+   Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept, Set-Cookie, Cookie
Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
Access-Control-Allow-Origin: *
-   Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Content-Type: application/json
-   Date: Mon, 03 Dec 2018 17:55:10 GMT
-   Server: Mojolicious (Perl)
-   Set-Cookie: mojolicious=...; expires=Mon, 03 Dec 2018 21:55:10 GMT; 
path=/; HttpOnly
-   Vary: Accept-Encoding
-   Whole-Content-Sha512: 
b84HraJH6Kiqrz7i1L1juDBJWdkdYbbClnWM0lZDljvpSkVT9adFTTrHiv7Mjtt2RKquGdzFZ6tqt9s+ODxqsw==
-   Content-Length: 93
-
-   { "response": "admin successfully deleted all federation resolvers: [ 
0.0.0.0/32, ::/128 ]." }
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
fd7P45mIiHuYqZZW6+8K+YjY1Pe504Aaw4J4Zp9AhrqLX72ERytTqWtAp1msutzNSRUdUSC72+odNPtpv3O8uw==
+   X-Server-Name: traffic_ops_golang/
+   Date: Wed, 23 Oct 2019 23:34:53 GMT
+   Content-Length: 184
 
+   { "alerts": [
+   {
+   "text": "admin successfully deleted all federation 
resolvers: [ 8.8.8.8 ]",
+   "level": "success"
+   }
+   ],
+   "response": "admin successfully deleted all federation resolvers: [ 
8.8.8.8 ]"
+   }
 
 ``PUT``
 ===
-Replaces **all** federations associated with a user's :term:`Delivery 
Service`\ (s) with those defined inside the request payload.
+Replaces **all** :term:`Federations` associated with a user's :term:`Delivery 
Service`\ (s) with those defined inside the request payload.
 
 :Auth. Required: Yes
 :Roles Required: "admin", "Federation", "operations", "Portal", or "Steering"
 :Response Type:  Object (string)
 
 Request Structure
 -
-:federations: The top-level key that must exist - an array of objects that 
each describe a set of resolvers for a :term:`Delivery Service`'s federation
+.. versionchanged:: 1.4
+   Prior to API version 1.4, the request body had to be wrapped in a 
top-level ``federations`` key, as can be seen in the :ref:`legacy-put-request` 
example. That behavior is still supported but no longer necessary.
 
-   :deliveryService: The 'xml_id' of the :term:`Delivery Service` which 
will use the federation resolvers specified in ``mappings``
-   :mappings:An object containing two arrays of IP addresses to 
use as federation resolvers
+.. _legacy-put-request:
+.. code-block:: json
+   :caption: Legacy Request
 
-   :resolve4: An array of IPv4 addresses that can resolve the 
:term:`Delivery Service`'s federation
-   :resolve6: An array of IPv6 addresses that can resolve the 
:term:`Delivery Service`'s federation
+   {
+   "federations": [{
+   "deliveryService": "demo1",
+   "mappings": {
+   "resolve4": ["0.0.0.0"],
+   "resolve6": ["::1"]
+   }
+   }]
+   }
+
+The request payload is an array of objects that describe Delivery Service 
:term:`Federation` Resolver mappings. Each object in the array must be in the 
following format.
+
+:deliveryService: The :ref:`ds-xmlid` of the :term:`Delivery Service` which 
will use the :term:`Federation` Resolvers specified in ``mappings``
+:mappings:An object containing two arrays of IP addresses (or subnets 
in :abbr:`CIDR (Classless Inter-Domain Routing)` notation) to use as 
:term:`Federation` Resolvers
+
+   :resolve4: An array of IPv4 addresses (or subnets in :abbr:`CIDR 
(Classless Inter-Domain Routing)` notation) that can resolve the 
:term:`Delivery Service`'s :term:`Federation`
+   :resolve6: An array of IPv6 addresses (or subnets in :abbr:`CIDR 
(Classless Inter-Domain Routing)` notation) that can resolve the 
:term:`Delivery Service`'s :term:`Federation`
 
 .. code-block:: http
:caption: Request Example
 
PUT /api/1.4/federations HTTP/1.1
Host: trafficops.infra.ciab.test
-   User-Agent: curl/7.62.0
+   User-Agent: 

[GitHub] [trafficcontrol] mhoppa commented on issue #4038: TP, TO: server capabilities are allowed to be assigned to any servers which are not mid or edge

[GitBox]

mhoppa commented on issue #4038: TP, TO: server capabilities are allowed to be 
assigned to any servers which are not mid or edge
URL: https://github.com/apache/trafficcontrol/issues/4038#issuecomment-547510780
 
 
   I can do this issue. I can't assign myself so wanted to post so we dont have 
duplicate effort. 


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

[GitHub] [trafficcontrol] ocket8888 commented on issue #4039: TO, TP: server should not be allowed to be deleted when it has a DS assigned to it.

[GitBox]

ocket commented on issue #4039: TO,TP: server should not be allowed to be 
deleted when it has a DS assigned to it.
URL: https://github.com/apache/trafficcontrol/issues/4039#issuecomment-547508787
 
 
   This seems like mainly a matter of opinion. I can't really see a good reason 
to delete the last cache server assigned to an Active DS, but I think it'd be 
fine for an Inactive DS.
   
   Seems like a feature that could be implemented entirely within Traffic 
Portal through either confirmation dialogs or just explicitly disallowing it, 
while the API remains unchanged.


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

[GitHub] [trafficcontrol] lbathina opened a new issue #4039: TO, TP: server should not be allowed to be deleted when it has a DS assigned to it.

[GitBox]

lbathina opened a new issue #4039: TO,TP: server should not be allowed to be 
deleted when it has a DS assigned to it.
URL: https://github.com/apache/trafficcontrol/issues/4039
 
 
   
   
   
   
   ## I'm submitting a ...
   
   
   - [ ] bug report
   - [ ] new feature / enhancement request
   - [X] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [X] Traffic Ops
   - [ ] Traffic Ops ORT
   - [X] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
   User is able to delete a server when it is assigned to a DS.
   ## Expected / new behavior:
   
   User should not be able to delete a server when it is assigned to a DS. 
   
   ## Minimal reproduction of the problem with instructions:
   
   
   ## Anything else:
   
   This restriction should be in place to avoid having any DS unknowingly be 
not assigned with any caches. When a server is being deleted, all its 
associated links should be deleted before it could be. Which avoids any unknown 
implications.
   


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

[GitHub] [trafficcontrol] mattjackson220 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

mattjackson220 commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r340175558
 
 

 ##
 File path: traffic_ops/app/bin/checks/ToAutorenewCerts.pl
 ##
 @@ -0,0 +1,103 @@
+#!/usr/bin/perl
+#
+#
+# Licensed 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.
+#
+# DNSSEC refresh, checks to see if DNSSEC keys need to be re-generated.
+#
+
+use strict;
+use warnings;
+
+$|++;
+
+use LWP::UserAgent;
+use Data::Dumper;
+use Getopt::Std;
+use Log::Log4perl qw(:easy);
+use JSON;
+use Extensions::Helper;
+
+my $VERSION = "0.01";
+my $hostn   = `hostname`;
+chomp($hostn);
+
+my %args = ();
+getopts( "l:c:", \%args );
+
+Log::Log4perl->easy_init($ERROR);
+if ( defined( $args{l} ) ) {
+   if( $args{l} == 1 ) { Log::Log4perl->easy_init($INFO); }
+   elsif ( $args{l} == 2 ) { Log::Log4perl->easy_init($DEBUG); }
+   elsif ( $args{l} == 3 ) { Log::Log4perl->easy_init($TRACE); }
+   elsif ( $args{l} > 3 )  { Log::Log4perl->easy_init($TRACE); }
+   else{ Log::Log4perl->easy_init($INFO); }
+}
+
+DEBUG( "Including DEBUG messages in output. Config is \'" . $args{c} . "\'" );
+TRACE( "Including TRACE messages in output. Config is \'" . $args{c} . "\'" );
+
+if ( !defined( $args{c} ) ) {
+   ();
 
 Review comment:
   done


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

[GitHub] [trafficcontrol] rob05c edited a comment on issue #4038: TP, TO: server capabilities are allowed to be assigned to any servers which are not mid or edge

[GitBox]

rob05c edited a comment on issue #4038: TP, TO: server capabilities are allowed 
to be assigned to any servers which are not mid or edge
URL: https://github.com/apache/trafficcontrol/issues/4038#issuecomment-547493652
 
 
   Eh, you're right. Postgres check constraints can't reference other tables 
(the pattern is easy, they do support regexes, it's the other table that's 
hard).
   
   It's possible with a trigger. But I'm on the fence whether the ugliness and 
pain of a trigger/function is worth it.
   
   Of course, this wouldn't even be an issue if our data was properly 
normalized, and the server table wasn't overloaded for multiple types.


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

[GitHub] [trafficcontrol] rob05c commented on issue #4038: TP, TO: server capabilities are allowed to be assigned to any servers which are not mid or edge

[GitBox]

rob05c commented on issue #4038: TP, TO: server capabilities are allowed to be 
assigned to any servers which are not mid or edge
URL: https://github.com/apache/trafficcontrol/issues/4038#issuecomment-547493652
 
 
   Eh, you're right. Postgres check constraints can't reference other tables 
(the pattern is easy, they do support regexes, it's the other table that's 
hard).
   
   It's possible with a trigger. But I'm on the fence whether the ugliness and 
pain of a trigger/function is worth it.


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

Jenkins build is back to normal : trafficcontrol-PR #4661

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] asf-ci commented on issue #4037: Docs/influx fix

[GitBox]

asf-ci commented on issue #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037#issuecomment-547481124
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4661/
   


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

[GitHub] [trafficcontrol] rob05c commented on issue #4038: TP, TO: server capabilities are allowed to be assigned to any servers which are not mid or edge

[GitBox]

rob05c commented on issue #4038: TP, TO: server capabilities are allowed to be 
assigned to any servers which are not mid or edge
URL: https://github.com/apache/trafficcontrol/issues/4038#issuecomment-547481110
 
 
   I'd vote we also add a Database Constraint. Multiple failsafes are always 
better.


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

[GitHub] [trafficcontrol] ocket8888 commented on issue #4038: TP, TO: server capabilities are allowed to be assigned to any servers which are not mid or edge

[GitBox]

ocket commented on issue #4038: TP, TO: server capabilities are allowed to 
be assigned to any servers which are not mid or edge
URL: https://github.com/apache/trafficcontrol/issues/4038#issuecomment-547479747
 
 
   So this has two parts, then:
   
   • Disallow assignment of server capabilities to non-cache servers at the API 
level
   • Don't show non-cache servers in the TP UI when linking a capability to 
servers, and probably also remove the option "Manage Capabilities" from the 
"More" menu of servers ineligible for capability assignment.
   
   


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

[GitHub] [trafficcontrol] lbathina opened a new issue #4038: TP, TO: server capabilities are allowed to be assigned to any servers which are not mid or edge

[GitBox]

lbathina opened a new issue #4038: TP, TO: server capabilities are allowed to 
be assigned to any servers which are not mid or edge
URL: https://github.com/apache/trafficcontrol/issues/4038
 
 
   
   
   
   
   ## I'm submitting a ...
   
   
   - [X] bug report
   - [ ] new feature / enhancement request
   - [ ] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [X] Traffic Ops
   - [ ] Traffic Ops ORT
   - [X] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
   Server Capabilities is assignable to any server (like origin)
   ## Expected / new behavior:
   
   Server Capabilities should be assignable only to edge or mid type servers 
   ## Minimal reproduction of the problem with instructions:
   
   create server capabilities 
   assign it to any server of type other than mid or edge
   ## Anything else:
   
   
   


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

[GitHub] [trafficcontrol] mattjackson220 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

mattjackson220 commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r340143304
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/routing/routes.go
 ##
 @@ -479,11 +479,18 @@ func Routes(d ServerData) ([]Route, []RawRoute, 
http.Handler, error) {
 
{1.1, http.MethodGet, 
`deliveryservices/{id}/servers/eligible/?(\.json)?$`, 
deliveryservice.GetServersEligible, auth.PrivLevelReadOnly, Authenticated, nil},
 
+   {1.4, http.MethodGet, 
`deliveryservices/xmlId/{xmlid}/sslkeys$`, 
deliveryservice.GetSSLKeysByXMLIDV14, auth.PrivLevelAdmin, Authenticated, nil},
{1.1, http.MethodGet, 
`deliveryservices/xmlId/{xmlid}/sslkeys$`, deliveryservice.GetSSLKeysByXMLID, 
auth.PrivLevelAdmin, Authenticated, nil},
{1.1, http.MethodGet, 
`deliveryservices/hostname/{hostname}/sslkeys$`, 
deliveryservice.GetSSLKeysByHostName, auth.PrivLevelAdmin, Authenticated, nil},
 
 Review comment:
   good call. done


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

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4037: Docs/influx fix

[GitBox]

ocket commented on a change in pull request #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037#discussion_r340136332
 
 

 ##
 File path: docs/source/admin/traffic_ops.rst
 ##
 @@ -0,0 +1,706 @@
+..
+..
+.. Licensed 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.
+..
+
+.. role:: bash(code)
+   :language: bash
+
+***
+Traffic Ops
+***
+Traffic Ops is quite possible the single most complex and most important 
Traffic Control component. It has many different configuration options that 
affect a wide range of other components and their interactions.
+
+.. _to-install:
+
+Installing
+==
+
+System Requirements
+---
+The user must have the following for a successful minimal install:
+
+- CentOS 7+
+- Two machines - physical or virtual -, each with at least two (v)CPUs, 4GB of 
RAM, and 20 GB of disk space
+- Access to CentOS Base and EPEL :manpage:`yum(8)` repositories
+- Access to `The Comprehensive Perl Archive Network (CPAN) 
`_
+
+Guide
+-
+#. Install PostgreSQL Database. For a production install it is best to install 
PostgreSQL on its own server/virtual machine.
+
+   .. seealso:: For more information on installing PostgreSQL, see `their 
documentation `_.
+
+   .. code-block:: shell
+   :caption: Example PostgreSQL Install Procedure
+
+   yum update -y
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+   yum install -y postgresql96-server
+   su - postgres -c '/usr/pgsql-9.6/bin/initdb -A md5 -W' #-W 
forces the user to provide a superuser (postgres) password
+
+
+#. Edit :file:`/var/lib/pgsql/9.6/data/pg_hba.conf` to allow the Traffic Ops 
instance to access the PostgreSQL server. For example, if the IP address of the 
machine to be used as the Traffic Ops host is ``192.0.2.1`` add the line ``host 
 all   all 192.0.2.1/32 md5`` to the appropriate section of this file.
+
+#. Edit the :file:`/var/lib/pgsql/9.6/data/postgresql.conf` file to add the 
appropriate listen_addresses or ``listen_addresses = '*'``, set ``timezone = 
'UTC'``, and start the database
+
+   .. code-block:: shell
+   :caption: Starting PostgreSQL with :manpage:`systemd(1)`
+
+   systemctl enable postgresql-9.6
+   systemctl start postgresql-9.6
+   systemctl status postgresql-9.6 # Prints the status of the 
PostgreSQL service, to prove it's running
+
+
+#. Build a :file:`traffic_ops-{version string}.rpm` file using the 
instructions under the :ref:`dev-building` page - or simply download a 
pre-built `release `_.
 
 Review comment:
   That is odd. I know we had downloads for those somewhere, but I can't find 
them.


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

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4037: Docs/influx fix

[GitBox]

ocket commented on a change in pull request #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037#discussion_r340130735
 
 

 ##
 File path: docs/source/admin/traffic_ops.rst
 ##
 @@ -0,0 +1,706 @@
+..
+..
+.. Licensed 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.
+..
+
+.. role:: bash(code)
+   :language: bash
+
+***
+Traffic Ops
+***
+Traffic Ops is quite possible the single most complex and most important 
Traffic Control component. It has many different configuration options that 
affect a wide range of other components and their interactions.
+
+.. _to-install:
+
+Installing
+==
+
+System Requirements
+---
+The user must have the following for a successful minimal install:
+
+- CentOS 7+
+- Two machines - physical or virtual -, each with at least two (v)CPUs, 4GB of 
RAM, and 20 GB of disk space
+- Access to CentOS Base and EPEL :manpage:`yum(8)` repositories
+- Access to `The Comprehensive Perl Archive Network (CPAN) 
`_
+
+Guide
+-
+#. Install PostgreSQL Database. For a production install it is best to install 
PostgreSQL on its own server/virtual machine.
+
+   .. seealso:: For more information on installing PostgreSQL, see `their 
documentation `_.
+
+   .. code-block:: shell
+   :caption: Example PostgreSQL Install Procedure
+
+   yum update -y
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+   yum install -y postgresql96-server
+   su - postgres -c '/usr/pgsql-9.6/bin/initdb -A md5 -W' #-W 
forces the user to provide a superuser (postgres) password
+
+
+#. Edit :file:`/var/lib/pgsql/9.6/data/pg_hba.conf` to allow the Traffic Ops 
instance to access the PostgreSQL server. For example, if the IP address of the 
machine to be used as the Traffic Ops host is ``192.0.2.1`` add the line ``host 
 all   all 192.0.2.1/32 md5`` to the appropriate section of this file.
+
+#. Edit the :file:`/var/lib/pgsql/9.6/data/postgresql.conf` file to add the 
appropriate listen_addresses or ``listen_addresses = '*'``, set ``timezone = 
'UTC'``, and start the database
+
+   .. code-block:: shell
+   :caption: Starting PostgreSQL with :manpage:`systemd(1)`
+
+   systemctl enable postgresql-9.6
+   systemctl start postgresql-9.6
+   systemctl status postgresql-9.6 # Prints the status of the 
PostgreSQL service, to prove it's running
+
+
+#. Build a :file:`traffic_ops-{version string}.rpm` file using the 
instructions under the :ref:`dev-building` page - or simply download a 
pre-built `release `_.
+
+#. Install a PostgreSQL client on the Traffic Ops host
+
+   .. code-block:: shell
+   :caption: Installing PostgreSQL Client from a Hosted Source
+
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+
+#. Install the Traffic Ops RPM. The Traffic Ops RPM file should have been 
built in an earlier step.
+
+   .. code-block:: shell
+   :caption: Installing a Generated Traffic Ops RPM
+
+   yum install -y 
./dist/traffic_ops-3.0.0-.yyy.el7.x86_64.rpm
+
+   .. note:: This will install the PostgreSQL client, ``psql`` as a 
dependency.
+
+#. Login to the Database from the Traffic Ops machine. At this point you 
should be able to login from the Traffic Ops (hostname ``to`` in the example) 
host to the PostgreSQL (hostname ``pg`` in the example) host
+
+   .. code-block:: psql
+   :caption: Example Login to Traffic Ops Database from Traffic 
Ops Server
+
+   to-# psql -h pg -U postgres
+   Password for user postgres:
+   psql (9.6.3)
+   Type "help" for help.
+
+   postgres=#
+
+
+#. Create the user and database. By default, Traffic Ops will expect to 
connect as the ``traffic_ops`` user to the ``traffic_ops`` database.
+
+   .. code-block:: console
+   :caption: Creating the Traffic Ops User and Database
+
+   to-# psql -U postgres -h pg -c "CREATE USER traffic_ops WITH 
ENCRYPTED PASSWORD 'tcr0cks';"
+   Password for user postgres:
+   CREATE ROLE
+   to-# createdb 

[GitHub] [trafficcontrol] mattjackson220 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

mattjackson220 commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r340118606
 
 

 ##
 File path: traffic_portal/app/src/common/api/DeliveryServiceSslKeysService.js
 ##
 @@ -17,75 +17,80 @@
  * under the License.
  */
 
-var DeliveryServiceSslKeysService = function($http, $q, locationUtils, 
messageModel, ENV) {
+var DeliveryServiceSslKeysService = function($http, locationUtils, 
messageModel, ENV) {
+this.successMessage = 'SSL Keys generated and updated for ';
+this.letsEncryptSuccessMessage = 'Call to Lets Encrypt has been made 
successfully. This may take a few minutes. Please watch for a notification in 
the Change Log. Delivery Service = ';
+
this.generateSslKeys = function(deliveryService, sslKeys, 
generateSslKeyForm) {
-   if (sslKeys.hasOwnProperty('version')){
-   generateSslKeyForm.version = parseInt(sslKeys.version) 
+ 1;
-   } else {
-   generateSslKeyForm.version = 1;
-   }
+return this.generateSslKeysBase(deliveryService, sslKeys, 
generateSslKeyForm, 'deliveryservices/sslkeys/generate', this.successMessage);
+   };
 
-   generateSslKeyForm.cdn = deliveryService.cdnName;
-   generateSslKeyForm.deliveryservice = deliveryService.xmlId;
-   generateSslKeyForm.key = deliveryService.xmlId;
+this.generateSslKeysWithLetsEncrypt = function(deliveryService, sslKeys, 
generateSslKeyForm) {
+return this.generateSslKeysBase(deliveryService, sslKeys, 
generateSslKeyForm, 'deliveryservices/sslkeys/generate/letsencrypt', 
this.letsEncryptSuccessMessage);
+};
 
-   var request = $q.defer();
-$http.post(ENV.api['root'] + "deliveryservices/sslkeys/generate", 
generateSslKeyForm)
-.then(
-function(result) {
-   messageModel.setMessages([ { level: 'success', text: 'SSL Keys 
generated and updated for ' + deliveryService.xmlId } ], true);
-request.resolve(result.data.response);
-},
-function(fault) {
-   messageModel.setMessages(fault.data.alerts, false);
-request.reject(fault);
-}
-);
-return request.promise;
-   };
+   this.generateSslKeysBase = function(deliveryService, sslKeys, 
generateSslKeyForm, endpoint, message) {
+if (sslKeys.hasOwnProperty('version')){
+generateSslKeyForm.version = parseInt(sslKeys.version, 10) + 1;
+} else {
+generateSslKeyForm.version = 1;
+}
+
+generateSslKeyForm.cdn = deliveryService.cdnName;
+generateSslKeyForm.deliveryservice = deliveryService.xmlId;
+generateSslKeyForm.key = deliveryService.xmlId;
+
+return $http.post(ENV.api['root'] + endpoint, generateSslKeyForm)
+.then(
+function(result) {
+messageModel.setMessages([ { level: 'success', text: 
message + deliveryService.xmlId } ], true);
+return result.data.response;
+},
+function(err) {
+messageModel.setMessages(err.data.alerts, false);
 
 Review comment:
   done


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

[GitHub] [trafficcontrol] mattjackson220 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

mattjackson220 commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r340118459
 
 

 ##
 File path: 
traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/generate/form.GenerateDeliveryServiceSslKeys.tpl.html
 ##
 @@ -30,7 +30,16 @@
 
 
 
-
+
+Use 
Let's Encrypt
 
 Review comment:
   done


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

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4037: Docs/influx fix

[GitBox]

mhoppa commented on a change in pull request #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037#discussion_r340099493
 
 

 ##
 File path: docs/source/admin/traffic_ops.rst
 ##
 @@ -0,0 +1,706 @@
+..
+..
+.. Licensed 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.
+..
+
+.. role:: bash(code)
+   :language: bash
+
+***
+Traffic Ops
+***
+Traffic Ops is quite possible the single most complex and most important 
Traffic Control component. It has many different configuration options that 
affect a wide range of other components and their interactions.
+
+.. _to-install:
+
+Installing
+==
+
+System Requirements
+---
+The user must have the following for a successful minimal install:
+
+- CentOS 7+
+- Two machines - physical or virtual -, each with at least two (v)CPUs, 4GB of 
RAM, and 20 GB of disk space
+- Access to CentOS Base and EPEL :manpage:`yum(8)` repositories
+- Access to `The Comprehensive Perl Archive Network (CPAN) 
`_
+
+Guide
+-
+#. Install PostgreSQL Database. For a production install it is best to install 
PostgreSQL on its own server/virtual machine.
+
+   .. seealso:: For more information on installing PostgreSQL, see `their 
documentation `_.
+
+   .. code-block:: shell
+   :caption: Example PostgreSQL Install Procedure
+
+   yum update -y
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+   yum install -y postgresql96-server
+   su - postgres -c '/usr/pgsql-9.6/bin/initdb -A md5 -W' #-W 
forces the user to provide a superuser (postgres) password
+
+
+#. Edit :file:`/var/lib/pgsql/9.6/data/pg_hba.conf` to allow the Traffic Ops 
instance to access the PostgreSQL server. For example, if the IP address of the 
machine to be used as the Traffic Ops host is ``192.0.2.1`` add the line ``host 
 all   all 192.0.2.1/32 md5`` to the appropriate section of this file.
+
+#. Edit the :file:`/var/lib/pgsql/9.6/data/postgresql.conf` file to add the 
appropriate listen_addresses or ``listen_addresses = '*'``, set ``timezone = 
'UTC'``, and start the database
+
+   .. code-block:: shell
+   :caption: Starting PostgreSQL with :manpage:`systemd(1)`
+
+   systemctl enable postgresql-9.6
+   systemctl start postgresql-9.6
+   systemctl status postgresql-9.6 # Prints the status of the 
PostgreSQL service, to prove it's running
+
+
+#. Build a :file:`traffic_ops-{version string}.rpm` file using the 
instructions under the :ref:`dev-building` page - or simply download a 
pre-built `release `_.
+
+#. Install a PostgreSQL client on the Traffic Ops host
+
+   .. code-block:: shell
+   :caption: Installing PostgreSQL Client from a Hosted Source
+
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+
+#. Install the Traffic Ops RPM. The Traffic Ops RPM file should have been 
built in an earlier step.
+
+   .. code-block:: shell
+   :caption: Installing a Generated Traffic Ops RPM
+
+   yum install -y 
./dist/traffic_ops-3.0.0-.yyy.el7.x86_64.rpm
+
+   .. note:: This will install the PostgreSQL client, ``psql`` as a 
dependency.
+
+#. Login to the Database from the Traffic Ops machine. At this point you 
should be able to login from the Traffic Ops (hostname ``to`` in the example) 
host to the PostgreSQL (hostname ``pg`` in the example) host
+
+   .. code-block:: psql
+   :caption: Example Login to Traffic Ops Database from Traffic 
Ops Server
+
+   to-# psql -h pg -U postgres
+   Password for user postgres:
+   psql (9.6.3)
+   Type "help" for help.
+
+   postgres=#
+
+
+#. Create the user and database. By default, Traffic Ops will expect to 
connect as the ``traffic_ops`` user to the ``traffic_ops`` database.
+
+   .. code-block:: console
+   :caption: Creating the Traffic Ops User and Database
+
+   to-# psql -U postgres -h pg -c "CREATE USER traffic_ops WITH 
ENCRYPTED PASSWORD 'tcr0cks';"
+   Password for user postgres:
+   CREATE ROLE
+   to-# createdb 

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4037: Docs/influx fix

[GitBox]

mhoppa commented on a change in pull request #4037: Docs/influx fix
URL: https://github.com/apache/trafficcontrol/pull/4037#discussion_r340090954
 
 

 ##
 File path: docs/source/admin/traffic_ops.rst
 ##
 @@ -0,0 +1,706 @@
+..
+..
+.. Licensed 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.
+..
+
+.. role:: bash(code)
+   :language: bash
+
+***
+Traffic Ops
+***
+Traffic Ops is quite possible the single most complex and most important 
Traffic Control component. It has many different configuration options that 
affect a wide range of other components and their interactions.
+
+.. _to-install:
+
+Installing
+==
+
+System Requirements
+---
+The user must have the following for a successful minimal install:
+
+- CentOS 7+
+- Two machines - physical or virtual -, each with at least two (v)CPUs, 4GB of 
RAM, and 20 GB of disk space
+- Access to CentOS Base and EPEL :manpage:`yum(8)` repositories
+- Access to `The Comprehensive Perl Archive Network (CPAN) 
`_
+
+Guide
+-
+#. Install PostgreSQL Database. For a production install it is best to install 
PostgreSQL on its own server/virtual machine.
+
+   .. seealso:: For more information on installing PostgreSQL, see `their 
documentation `_.
+
+   .. code-block:: shell
+   :caption: Example PostgreSQL Install Procedure
+
+   yum update -y
+   yum install -y 
https://download.postgresql.org/pub/repos/yum/9.6/redhat/rhel-7-x86_64/pgdg-centos96-9.6-3.noarch.rpm
+   yum install -y postgresql96-server
+   su - postgres -c '/usr/pgsql-9.6/bin/initdb -A md5 -W' #-W 
forces the user to provide a superuser (postgres) password
+
+
+#. Edit :file:`/var/lib/pgsql/9.6/data/pg_hba.conf` to allow the Traffic Ops 
instance to access the PostgreSQL server. For example, if the IP address of the 
machine to be used as the Traffic Ops host is ``192.0.2.1`` add the line ``host 
 all   all 192.0.2.1/32 md5`` to the appropriate section of this file.
+
+#. Edit the :file:`/var/lib/pgsql/9.6/data/postgresql.conf` file to add the 
appropriate listen_addresses or ``listen_addresses = '*'``, set ``timezone = 
'UTC'``, and start the database
+
+   .. code-block:: shell
+   :caption: Starting PostgreSQL with :manpage:`systemd(1)`
+
+   systemctl enable postgresql-9.6
+   systemctl start postgresql-9.6
+   systemctl status postgresql-9.6 # Prints the status of the 
PostgreSQL service, to prove it's running
+
+
+#. Build a :file:`traffic_ops-{version string}.rpm` file using the 
instructions under the :ref:`dev-building` page - or simply download a 
pre-built `release `_.
 
 Review comment:
   I dont see any pre-built rpms attached to the releases which is actually 
very surprising. 


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

[GitHub] [trafficcontrol] mattjackson220 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

mattjackson220 commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r340105300
 
 

 ##
 File path: docs/source/admin/traffic_router.rst
 ##
 @@ -681,8 +681,67 @@ The ordering of certificates within the certificate 
bundle matters. It must be:
 
 To see the ordering of certificates you may have to manually split up your 
certificate chain and use :manpage:`openssl(1ssl)` on each individual 
certificate
 
-Suggested Way of Setting up an HTTPS Delivery Service
--
+Let's Encrypt
+-
+Let’s Encrypt is a free, automated :abbr:`CA (Certificate Authority)` using 
:abbr:`ACME (Automated Certificate Management Environment)` protocol. Let's 
Encrypt performs a domain validation before issuing or renewing a certificate. 
There are several options for domain validation but for this application the 
DNS challenge is used in order to receive wildcard certificates. Let's Encrypt 
sends a token to be used as a TXT record at 
``_acme-challenge.domain.example.com`` and after verifying that the token is 
accessible there, will return the newly generated and signed certificate and 
key. The basic workflow implemented is:
+
+#. ``POST`` to Let's Encrypt and receive the DNS challenge token.
+#. Traffic Ops stores the DNS challenge in the Traffic Ops database.
+#. Traffic Router has a watcher set up to watch for changes in the Traffic Ops 
database table.
 
 Review comment:
   right, i just havent pushed those changes yet. updating with all the 
comments then ill push


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