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

2019-10-29 Thread 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 commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

2019-10-29 Thread 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 #4015: Rewrite /federations to Go - POST/PUT/DELETE

2019-10-29 Thread 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] ocket8888 commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

2019-10-28 Thread 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_r339835934
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/api/api.go
 ##
 @@ -86,7 +86,7 @@ func WriteRespRaw(w http.ResponseWriter, r *http.Request, v 
interface{}) {
return
}
w.Header().Set("Content-Type", "application/json")
-   w.Write(bts)
+   w.Write(append(bts, '\n'))
 
 Review comment:
   whoops :P


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 #4015: Rewrite /federations to Go - POST/PUT/DELETE

2019-10-28 Thread 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_r339835857
 
 

 ##
 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] ocket8888 commented on a change in pull request #4015: Rewrite /federations to Go - POST/PUT/DELETE

2019-10-28 Thread 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_r339835100
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
 ##
 @@ -346,3 +430,17 @@ func GetCacheGroupNameFromID(tx *sql.Tx, id int64) 
(tc.CacheGroupName, bool, err
}
return tc.CacheGroupName(name), true, nil
 }
+
+// GetFederationIDForUserIDByXMLID retrieves the ID of the Federation assigned 
to the user defined by
+// userID on the Delivery Service identified by xmlid. If no such federation 
exists, the boolean
+// returned will be 'false', while the error indicates unexpected errors that 
occurred when querying.
+func GetFederationIDForUserIDByXMLID(tx *sql.Tx, userID int, xmlid string) 
(uint, bool, error) {
+   var id uint
 
 Review comment:
   because IDs can't be negative, and that's about 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


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

2019-10-28 Thread 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_r339834865
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
 ##
 @@ -42,6 +42,75 @@ const BaseOrderBy = "\nORDER BY"
 const BaseLimit = "\nLIMIT"
 const BaseOffset = "\nOFFSET"
 
+const UserIDHasAccessToDeliveryServiceXMLIDQuery = `
 
 Review comment:
   You know it might not. I think that's a remnant from the old way I wanted to 
do it, but I couldn't figure out the query for it (that one doesn't work). I'll 
remove 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


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

2019-10-28 Thread 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_r339834661
 
 

 ##
 File path: lib/go-tc/federation.go
 ##
 @@ -67,36 +80,85 @@ type FederationMapping struct {
TTL   int`json:"ttl"`
 }
 
-// AllFederation is the JSON object returned by /api/1.x/federations?all
-type AllFederation struct {
-   Mappings[]AllFederationMapping `json:"mappings"`
-   DeliveryService DeliveryServiceName`json:"deliveryService"`
+// AllDeliveryServiceFederationsMapping is a structure that contains 
identifying information for a
+// Delivery Service as well as any and all Federation Resolver mapping 
assigned to it (or all those
+// getting assigned to it).
+type AllDeliveryServiceFederationsMapping struct {
+   Mappings[]FederationResolverMapping `json:"mappings"`
+   DeliveryService DeliveryServiceName `json:"deliveryService"`
 }
 
-func (a AllFederation) IsAllFederations() bool { return true }
+// IsAllFederations implements the IAllFederation interface. Always returns 
true.
+func (a AllDeliveryServiceFederationsMapping) IsAllFederations() bool { return 
true }
 
 // AllFederation is the JSON object returned by 
/api/1.x/federations?all=my-cdn-name
 type AllFederationCDN struct {
CDNName *CDNName `json:"cdnName"`
 }
 
+// IsAllFederations implements the IAllFederation interface. Always returns 
true.
 func (a AllFederationCDN) IsAllFederations() bool { return true }
 
-type AllFederationMapping struct {
-   TTL  *int `json:"ttl"`
-   CName*string  `json:"cname"`
+type ResolverMapping struct {
Resolve4 []string `json:"resolve4,omitempty"`
Resolve6 []string `json:"resolve6,omitempty"`
 }
 
+func (r *ResolverMapping) Validate(tx *sql.Tx) error {
+   errs := []error{}
+   for _, res := range r.Resolve4 {
 
 Review comment:
   They do but it doesn't cover CIDRs


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