date:20191009

[GitHub] [trafficcontrol] rob05c commented on issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

rob05c commented on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540240343
 
 
   > Should POST/DELETE only be admin? Right now I have it set to Operations.
   
   I think Ops is right. Admin vs Ops is somewhat nebulous, but I think in 
general, Operations can do pretty much everything except see password and 
security stuff. So I think Operations is right for these endpoints.


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 #3899: Add TO Go server cache configs

2019-10-09 Thread GitBox

ocket merged pull request #3899: Add TO Go server cache configs
URL: https://github.com/apache/trafficcontrol/pull/3899
 
 
   


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] mitchell852 commented on a change in pull request #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

mitchell852 commented on a change in pull request #3972: Add Server 
Capabilities blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333279081
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
+
+## Problem Description
+
+Suppose a Traffic Control operator has servers of a particular type. For 
example, servers with only Memory and no Disk cache. It's possible today to 
only route to those Edges, via manual Delivery Service Server assignments. But 
suppose you have a Mid server with only Memory and no Disk. Then suppose you 
have a certain class of traffic you need to route to this Mid, but not other 
traffic. For example, Delivery Services with small images; but not DSes with 
large binary files, which would destroy the cache. Right now, this isn't 
possible in Traffic Control.
+
+We propose a feature, called "Server Capabilitites" to solve this problem. 
Servers will have "capabilitites" and Delivery Services will have "Required 
Capabilitites," and if a server does not have a required capability, then it 
should not be manually assignable as an Edge, and a Mid must not be added as a 
parent for that DS in the Edge ATS config.
+
+Initially, this is completely backwards-compatible on upgrade, because 
initially no Delivery Service will have any Required Capabilities.
+
+This feature will be completely optional. TC operators who don't need Server 
Capabilities will simply not create them.
+
+Server Capabilities will be arbitrary strings. The ATS project will not "seed" 
any, and impose as little direction as possible. For example, Server 
Capabilitites could be Cache types, Server types, Hardware types, ATS versions, 
Lua support, other ATS plugin support, or any other features an operator needs 
to route (or not route) on.
+
+
+## Proposed Change
+
+- Servers have Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK )
+
+- Delivery Services have Required Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK)
+
+- When generating Configuration (e.g. ATS parent.config):
+  - If a Mid Server which is otherwise parented to an Edge does not have all 
Required Capabilities of a Delivery Service, that Mid will not be inserted as a 
parent for that Delivery Service’s remap and parent rules.
+  - Backward Compatibility is automatic:
+- Initially, no Delivery Services have Required Capabilities. Hence, 
everything behaves like it does today.
+- If an EDGE server does not have all capabilities required by a DS, that 
server SHOULD NOT be assignable to that DS. 
+- If an EDGE server is assigned to a DS which requires capabilities that 
server has, it SHOULD NOT be possible to remove those capabilities from that 
server.
+
+### Traffic Portal Impact
+
+- Server Page
+  - New dropdown to add to a text list: Capabilities
+  - List elements have a button to remove the capability
+  - Dropdown/List is just one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- Delivery Service Page
+  - New dropdown to add to a text list: Required Capabilities
+  - List elements have a button to remove a capability
+  - Dropdown/List is one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- New page for Server Capability Types
+  - Ability to add, delete Server Capability Types
+  - May be a text input, which adds to a list, with list element buttons to 
remove from the list
+
+
+### Traffic Ops Impact
+
+`/server_server_capabilities`
+  - List all server capabilities assigned to a server
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/deliveryservices_required_capabilities`
 
 Review comment:
   readonly users or above for the GET?
   operations users only for the POST/DELETE? (i would suggest operations as 
the operations role allows you to edit ds's currently)


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] mitchell852 commented on a change in pull request #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

mitchell852 commented on a change in pull request #3972: Add Server 
Capabilities blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333278919
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
+
+## Problem Description
+
+Suppose a Traffic Control operator has servers of a particular type. For 
example, servers with only Memory and no Disk cache. It's possible today to 
only route to those Edges, via manual Delivery Service Server assignments. But 
suppose you have a Mid server with only Memory and no Disk. Then suppose you 
have a certain class of traffic you need to route to this Mid, but not other 
traffic. For example, Delivery Services with small images; but not DSes with 
large binary files, which would destroy the cache. Right now, this isn't 
possible in Traffic Control.
+
+We propose a feature, called "Server Capabilitites" to solve this problem. 
Servers will have "capabilitites" and Delivery Services will have "Required 
Capabilitites," and if a server does not have a required capability, then it 
should not be manually assignable as an Edge, and a Mid must not be added as a 
parent for that DS in the Edge ATS config.
+
+Initially, this is completely backwards-compatible on upgrade, because 
initially no Delivery Service will have any Required Capabilities.
+
+This feature will be completely optional. TC operators who don't need Server 
Capabilities will simply not create them.
+
+Server Capabilities will be arbitrary strings. The ATS project will not "seed" 
any, and impose as little direction as possible. For example, Server 
Capabilitites could be Cache types, Server types, Hardware types, ATS versions, 
Lua support, other ATS plugin support, or any other features an operator needs 
to route (or not route) on.
+
+
+## Proposed Change
+
+- Servers have Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK )
+
+- Delivery Services have Required Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK)
+
+- When generating Configuration (e.g. ATS parent.config):
+  - If a Mid Server which is otherwise parented to an Edge does not have all 
Required Capabilities of a Delivery Service, that Mid will not be inserted as a 
parent for that Delivery Service’s remap and parent rules.
+  - Backward Compatibility is automatic:
+- Initially, no Delivery Services have Required Capabilities. Hence, 
everything behaves like it does today.
+- If an EDGE server does not have all capabilities required by a DS, that 
server SHOULD NOT be assignable to that DS. 
+- If an EDGE server is assigned to a DS which requires capabilities that 
server has, it SHOULD NOT be possible to remove those capabilities from that 
server.
+
+### Traffic Portal Impact
+
+- Server Page
+  - New dropdown to add to a text list: Capabilities
+  - List elements have a button to remove the capability
+  - Dropdown/List is just one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- Delivery Service Page
+  - New dropdown to add to a text list: Required Capabilities
+  - List elements have a button to remove a capability
+  - Dropdown/List is one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- New page for Server Capability Types
+  - Ability to add, delete Server Capability Types
+  - May be a text input, which adds to a list, with list element buttons to 
remove from the list
+
+
+### Traffic Ops Impact
+
+`/server_server_capabilities`
+  - List all server capabilities assigned to a server
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/deliveryservices_required_capabilities`
+  - List all DS Required Capabilities
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/server_capabilities`
 
 Review comment:
   readonly users or above for the GET?
   admin users only for the POST/DELETE?


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] mitchell852 commented on a change in pull request #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

mitchell852 commented on a change in pull request #3972: Add Server 
Capabilities blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333279220
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
+
+## Problem Description
+
+Suppose a Traffic Control operator has servers of a particular type. For 
example, servers with only Memory and no Disk cache. It's possible today to 
only route to those Edges, via manual Delivery Service Server assignments. But 
suppose you have a Mid server with only Memory and no Disk. Then suppose you 
have a certain class of traffic you need to route to this Mid, but not other 
traffic. For example, Delivery Services with small images; but not DSes with 
large binary files, which would destroy the cache. Right now, this isn't 
possible in Traffic Control.
+
+We propose a feature, called "Server Capabilitites" to solve this problem. 
Servers will have "capabilitites" and Delivery Services will have "Required 
Capabilitites," and if a server does not have a required capability, then it 
should not be manually assignable as an Edge, and a Mid must not be added as a 
parent for that DS in the Edge ATS config.
+
+Initially, this is completely backwards-compatible on upgrade, because 
initially no Delivery Service will have any Required Capabilities.
+
+This feature will be completely optional. TC operators who don't need Server 
Capabilities will simply not create them.
+
+Server Capabilities will be arbitrary strings. The ATS project will not "seed" 
any, and impose as little direction as possible. For example, Server 
Capabilitites could be Cache types, Server types, Hardware types, ATS versions, 
Lua support, other ATS plugin support, or any other features an operator needs 
to route (or not route) on.
+
+
+## Proposed Change
+
+- Servers have Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK )
+
+- Delivery Services have Required Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK)
+
+- When generating Configuration (e.g. ATS parent.config):
+  - If a Mid Server which is otherwise parented to an Edge does not have all 
Required Capabilities of a Delivery Service, that Mid will not be inserted as a 
parent for that Delivery Service’s remap and parent rules.
+  - Backward Compatibility is automatic:
+- Initially, no Delivery Services have Required Capabilities. Hence, 
everything behaves like it does today.
+- If an EDGE server does not have all capabilities required by a DS, that 
server SHOULD NOT be assignable to that DS. 
+- If an EDGE server is assigned to a DS which requires capabilities that 
server has, it SHOULD NOT be possible to remove those capabilities from that 
server.
+
+### Traffic Portal Impact
+
+- Server Page
+  - New dropdown to add to a text list: Capabilities
+  - List elements have a button to remove the capability
+  - Dropdown/List is just one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- Delivery Service Page
+  - New dropdown to add to a text list: Required Capabilities
+  - List elements have a button to remove a capability
+  - Dropdown/List is one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- New page for Server Capability Types
+  - Ability to add, delete Server Capability Types
+  - May be a text input, which adds to a list, with list element buttons to 
remove from the list
+
+
+### Traffic Ops Impact
+
+`/server_server_capabilities`
 
 Review comment:
   readonly users or above for the GET?
   operations users only for the POST/DELETE? (i would suggest operations as 
the operations role allows you to edit servers currently)


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

ocket commented on issue #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#issuecomment-540221525
 
 
   That thread - though it was swiftly derailed - was only about deprecating 
`/capabilities/{name}`, and while deprecating methods other than GET was 
suggested it didn't get any +1s, whereas the `/capabilities/{name}` deprecation 
recieved unanimous support.
   
   Furthermore, we never settled in that thread on whether or not deprecated 
endpoints ought to be rewritten - there were proponents for both sides.


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] jheitz200 commented on a change in pull request #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

jheitz200 commented on a change in pull request #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#discussion_r333248544
 
 

 ##
 File path: lib/go-tc/server_capability.go
 ##
 @@ -0,0 +1,9 @@
+package tc
 
 Review comment:
   Based on the file naming convention in this PR, should this file be named 
`server_server_capability.go`?


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 #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3972: Add Server Capabilities 
blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333279942
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
+
+## Problem Description
+
+Suppose a Traffic Control operator has servers of a particular type. For 
example, servers with only Memory and no Disk cache. It's possible today to 
only route to those Edges, via manual Delivery Service Server assignments. But 
suppose you have a Mid server with only Memory and no Disk. Then suppose you 
have a certain class of traffic you need to route to this Mid, but not other 
traffic. For example, Delivery Services with small images; but not DSes with 
large binary files, which would destroy the cache. Right now, this isn't 
possible in Traffic Control.
+
+We propose a feature, called "Server Capabilitites" to solve this problem. 
Servers will have "capabilitites" and Delivery Services will have "Required 
Capabilitites," and if a server does not have a required capability, then it 
should not be manually assignable as an Edge, and a Mid must not be added as a 
parent for that DS in the Edge ATS config.
+
+Initially, this is completely backwards-compatible on upgrade, because 
initially no Delivery Service will have any Required Capabilities.
+
+This feature will be completely optional. TC operators who don't need Server 
Capabilities will simply not create them.
+
+Server Capabilities will be arbitrary strings. The ATS project will not "seed" 
any, and impose as little direction as possible. For example, Server 
Capabilitites could be Cache types, Server types, Hardware types, ATS versions, 
Lua support, other ATS plugin support, or any other features an operator needs 
to route (or not route) on.
+
+
+## Proposed Change
+
+- Servers have Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK )
+
+- Delivery Services have Required Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK)
+
+- When generating Configuration (e.g. ATS parent.config):
+  - If a Mid Server which is otherwise parented to an Edge does not have all 
Required Capabilities of a Delivery Service, that Mid will not be inserted as a 
parent for that Delivery Service’s remap and parent rules.
+  - Backward Compatibility is automatic:
+- Initially, no Delivery Services have Required Capabilities. Hence, 
everything behaves like it does today.
+- If an EDGE server does not have all capabilities required by a DS, that 
server SHOULD NOT be assignable to that DS. 
+- If an EDGE server is assigned to a DS which requires capabilities that 
server has, it SHOULD NOT be possible to remove those capabilities from that 
server.
+
+### Traffic Portal Impact
+
+- Server Page
+  - New dropdown to add to a text list: Capabilities
+  - List elements have a button to remove the capability
+  - Dropdown/List is just one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- Delivery Service Page
+  - New dropdown to add to a text list: Required Capabilities
+  - List elements have a button to remove a capability
+  - Dropdown/List is one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- New page for Server Capability Types
+  - Ability to add, delete Server Capability Types
+  - May be a text input, which adds to a list, with list element buttons to 
remove from the list
+
+
+### Traffic Ops Impact
+
+`/server_server_capabilities`
+  - List all Server capabilities
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/deliveryservices_required_capabilities`
+  - List all DS Required Capabilities
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/server_capabilities`
+  - List, create, and delete Server Capabilities.
+  - POST+GET+DELETE
+  - PUT doesn’t make sense, Capabilities are strings, if one is misspelled it 
should be removed and re-added. Simpler, less error-prone than cascading 
changes.
+
+Additionally, ATS config generation (which is currently in Traffic Ops, but in 
the process of being moved to its own library/app) will require changes. 
Primarily `parent.config`, as in the description, to prevent Mids being 
assigned as Parents for DSes for which they lack the capabilitites.
+
+ REST API Impact
+
+See Traffic Ops Impact
+
+ Client Impact
+
+See Traffic Ops Impact - client functions for each endpoint.
+
+ Data Model / Database Impact
+
+New tables for Capabilitites: `server_capability`, `server_server_capability`, 
`delivery_service_required_server_capability`.
+
+### ORT Impact
+
+n/a
+
+### Traffic Monitor Impact
+
+n/a
+
+### Traffic Router Impact
+
+n/a
+
+### Traffic Stats Impact
+
+n/a
+
+### Traffic Vault Impact
+
+n/a
+
+### Documentation Impact
+
+The new UI and API endpoints will be

[GitHub] [trafficcontrol] ocket8888 commented on issue #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

ocket commented on issue #3954: Added deprecation notices for 
/user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#issuecomment-540294066
 
 
   This took **hours** to figure out. Turns out, [ is **VERY** 
different than ( in Perl. 


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] jheitz200 commented on a change in pull request #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

jheitz200 commented on a change in pull request #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#discussion_r333250919
 
 

 ##
 File path: lib/go-tc/server_capability.go
 ##
 @@ -0,0 +1,28 @@
+package tc
 
 Review comment:
   Based on the file naming convention in this PR, should this file be named 
`server_server_capability.go`?


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 issue #3880: Rewrote /capabilities/{{name}} to Go

2019-10-09 Thread GitBox

mhoppa commented on issue #3880: Rewrote /capabilities/{{name}} to Go
URL: https://github.com/apache/trafficcontrol/pull/3880#issuecomment-540218712
 
 
   based on 
https://lists.apache.org/thread.html/cfd3a6d631f2c7510e3a15ee051840a9f04f3f0aa0c2696e460c9ab9@%3Cdev.trafficcontrol.apache.org%3E
 should this be closed and redone as a deprecate message on the perl route? 


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] jheitz200 commented on a change in pull request #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

jheitz200 commented on a change in pull request #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#discussion_r333248544
 
 

 ##
 File path: lib/go-tc/server_capability.go
 ##
 @@ -0,0 +1,9 @@
+package tc
 
 Review comment:
   Based on the file naming convention in this PR, should this file be named 
`server_server_capability.go`?


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 opened a new issue #3975: Environment Variables override explicit configuration in client tests

2019-10-09 Thread GitBox

ocket opened a new issue #3975: Environment Variables override explicit 
configuration in client tests
URL: https://github.com/apache/trafficcontrol/issues/3975
 
 
   ## I'm submitting a ...
   - bug report
   
   ## Traffic Control components affected ...
   - Traffic Control Client (Go) (integration tests with TO)
   
   ## Current behavior:
   If your `TO_URL` environment variable is set, it will override what's 
written in the tests' `trafficOps.URL` field in the configuration file.
   
   ## Expected / new behavior:
   Environmental factors should *never* override explicit configuration, makes 
it very difficult to tell why things aren't working.
   
   ## Minimal reproduction of the problem with instructions:
   ```bash
   TO_URL="https://google.com;
   
   # ensure trafficOps.URL in the config file is set to something
   # besides "https://google.com;
   go test -v --cfg=path/to/traffic-ops-test.conf
   
   # in the debug output you can see that it's attempting to connect
   # to TO_URL not trafficOps.URL
   ```


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 #3899: Add TO Go server cache configs

2019-10-09 Thread GitBox

asf-ci commented on issue #3899: Add TO Go server cache configs
URL: https://github.com/apache/trafficcontrol/pull/3899#issuecomment-540244779
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4437/
   


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 #4438

2019-10-09 Thread Apache Jenkins Server

See 

Changes:


--
GitHub pull request #3875 of commit 55d64f4cd96b6481c71cf02a1b34b837e56d2434, 
has merge conflicts.
Running as SYSTEM
!!! PR mergeability status has changed !!!  
PR now has NO merge conflicts
Setting status of 55d64f4cd96b6481c71cf02a1b34b837e56d2434 to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4438/ and message: 'Build 
started for original commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H34 (ubuntu) 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 55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # timeout=10
 > git rev-parse origin/55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # 
 > timeout=10
 > git rev-parse 55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
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/pull/*:refs/remotes/origin/pr/*
 > git rev-parse 55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # timeout=10
 > git rev-parse origin/55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # 
 > timeout=10
 > git rev-parse 55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
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/pull/*:refs/remotes/origin/pr/*
 > git rev-parse 55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # timeout=10
 > git rev-parse origin/55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # 
 > timeout=10
 > git rev-parse 55d64f4cd96b6481c71cf02a1b34b837e56d2434^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Skipped archiving because build is not successful

[GitHub] [trafficcontrol] asf-ci commented on issue #3875: Add TO Go ATS CDN configs

2019-10-09 Thread GitBox

asf-ci commented on issue #3875: Add TO Go ATS CDN configs
URL: https://github.com/apache/trafficcontrol/pull/3875#issuecomment-540250816
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4438/
   


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 #4440

2019-10-09 Thread Apache Jenkins Server

See

[GitHub] [trafficcontrol] asf-ci commented on issue #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

asf-ci commented on issue #3954: Added deprecation notices for 
/user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#issuecomment-540296449
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4440/
   


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 #3880: Rewrote /capabilities/{{name}} to Go

2019-10-09 Thread GitBox

ocket commented on issue #3880: Rewrote /capabilities/{{name}} to Go
URL: https://github.com/apache/trafficcontrol/pull/3880#issuecomment-540224884
 
 
   That thread never reached a decision as to whether or not deprecated routes 
ought to be rewritten. If a decision to not rewrite these handlers is reached 
I'll close this PR (assuming it's still open), in the meantime I guess it's in 
the hands of people browsing PRs whether or not this is worth merging (once it 
isn't a draft (if ever)).


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 edited a comment on issue #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

ocket edited a comment on issue #3954: Added deprecation notices for 
/user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#issuecomment-540294066
 
 
   This took **hours** to figure out. Turns out, [ is **VERY** 
different from ( in Perl. 


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] mitchell852 commented on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540253926
 
 
   I added this comment to the blueprint regarding the need for bulk assignment 
(or deletion) of servers to a server capability: 
https://github.com/apache/trafficcontrol/pull/3972#pullrequestreview-299755840
   
   


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 #3875: Add TO Go ATS CDN configs

2019-10-09 Thread GitBox

asf-ci commented on issue #3875: Add TO Go ATS CDN configs
URL: https://github.com/apache/trafficcontrol/pull/3875#issuecomment-540262236
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4439/
   


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 #4439

2019-10-09 Thread Apache Jenkins Server

See 


Changes:

[ocket] Add TO Go server cache configs (#3899)

[rob] Add TO Go ATS CDN configs

[rob] Change atscfg ds name trimming to use funcs

[rob] Add CDN configs to generator

[rob] Fix test for function move


--
[...truncated 3.81 MB...]
[32;1mtraffic_portal_build_1   |[0m +-- extend@3.0.2 
[32;1mtraffic_portal_build_1   |[0m +-- forever-agent@0.6.1 
[32;1mtraffic_portal_build_1   |[0m +-- form-data@2.3.3 
[32;1mtraffic_portal_build_1   |[0m | `-- asynckit@0.4.0 
[32;1mtraffic_portal_build_1   |[0m +-- har-validator@5.1.3 
[32;1mtraffic_portal_build_1   |[0m | +-- ajv@6.10.2 
[32;1mtraffic_portal_build_1   |[0m | | +-- fast-deep-equal@2.0.1 
[32;1mtraffic_portal_build_1   |[0m | | +-- 
fast-json-stable-stringify@2.0.0 
[32;1mtraffic_portal_build_1   |[0m | | +-- json-schema-traverse@0.4.1 
[32;1mtraffic_portal_build_1   |[0m | | `-- uri-js@4.2.2 
[32;1mtraffic_portal_build_1   |[0m | |   `-- punycode@2.1.1 
[32;1mtraffic_portal_build_1   |[0m | `-- har-schema@2.0.0 
[32;1mtraffic_portal_build_1   |[0m +-- http-signature@1.2.0 
[32;1mtraffic_portal_build_1   |[0m | +-- assert-plus@1.0.0 
[32;1mtraffic_portal_build_1   |[0m | +-- jsprim@1.4.1 
[32;1mtraffic_portal_build_1   |[0m | | +-- extsprintf@1.3.0 
[32;1mtraffic_portal_build_1   |[0m | | +-- json-schema@0.2.3 
[32;1mtraffic_portal_build_1   |[0m | | `-- verror@1.10.0 
[32;1mtraffic_portal_build_1   |[0m | `-- sshpk@1.16.1 
[32;1mtraffic_portal_build_1   |[0m |   +-- asn1@0.2.4 
[32;1mtraffic_portal_build_1   |[0m |   +-- bcrypt-pbkdf@1.0.2 
[32;1mtraffic_portal_build_1   |[0m |   +-- dashdash@1.14.1 
[32;1mtraffic_portal_build_1   |[0m |   +-- ecc-jsbn@0.1.2 
[32;1mtraffic_portal_build_1   |[0m |   +-- getpass@0.1.7 
[32;1mtraffic_portal_build_1   |[0m |   +-- jsbn@0.1.1 
[32;1mtraffic_portal_build_1   |[0m |   `-- tweetnacl@0.14.5 
[32;1mtraffic_portal_build_1   |[0m +-- is-typedarray@1.0.0 
[32;1mtraffic_portal_build_1   |[0m +-- json-stringify-safe@5.0.1 
[32;1mtraffic_portal_build_1   |[0m +-- oauth-sign@0.9.0 
[32;1mtraffic_portal_build_1   |[0m +-- performance-now@2.1.0 
[32;1mtraffic_portal_build_1   |[0m +-- qs@6.5.2 
[32;1mtraffic_portal_build_1   |[0m +-- tough-cookie@2.4.3 
[32;1mtraffic_portal_build_1   |[0m | +-- psl@1.4.0 
[32;1mtraffic_portal_build_1   |[0m | `-- punycode@1.4.1 
[32;1mtraffic_portal_build_1   |[0m +-- tunnel-agent@0.6.0 
[32;1mtraffic_portal_build_1   |[0m `-- uuid@3.3.3 
[32;1mtraffic_portal_build_1   |[0m 
[32;1mtraffic_portal_build_1   |[0m npm WARN optional SKIPPING OPTIONAL 
DEPENDENCY: fsevents@^1.0.0 (node_modules/chokidar/node_modules/fsevents):
[32;1mtraffic_portal_build_1   |[0m npm WARN notsup SKIPPING OPTIONAL 
DEPENDENCY: Unsupported platform for fsevents@1.2.9: wanted 
{"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})
[32;1mtraffic_portal_build_1   |[0m npm WARN traffic_portal@ No description
[32;1mtraffic_portal_build_1   |[0m npm WARN traffic_portal@ No repository 
field.
[32;1mtraffic_portal_build_1   |[0m npm WARN traffic_portal@ No license field.
[32;1mtraffic_portal_build_1   |[0m 
[32;1mtraffic_portal_build_1   |[0m Done.
[32;1mtraffic_portal_build_1   |[0m 
[32;1mtraffic_portal_build_1   |[0m 
[32;1mtraffic_portal_build_1   |[0m Execution Time (2019-10-10 00:17:38 UTC)
[32;1mtraffic_portal_build_1   |[0m loading tasks  1.4s  
ââ 3%
[32;1mtraffic_portal_build_1   |[0m compass:prod  10.8s  
âââââââââ 27%
[32;1mtraffic_portal_build_1   |[0m browserify2:app-prod 3s  
âââ 7%
[32;1mtraffic_portal_build_1   |[0m browserify2:shared-libs-prod   3.1s  
âââ 8%
[32;1mtraffic_portal_build_1   |[0m install-dependencies  21.8s  
ââââââââââââââââââ 54%
[32;1mtraffic_portal_build_1   |[0m Total 40.5s
[32;1mtraffic_portal_build_1   |[0m 
[32;1mtraffic_portal_build_1   |[0m + exit 0
[32;1mtraffic_portal_build_1   |[0m Executing(%install): /bin/sh -e 
/var/tmp/rpm-tmp.Ki66Pr
[32;1mtraffic_portal_build_1   |[0m + umask 022
[32;1mtraffic_portal_build_1   |[0m + cd /tmp/trafficcontrol/rpmbuild/BUILD
[32;1mtraffic_portal_build_1   |[0m + '[' 
/tmp/trafficcontrol/rpmbuild/BUILDROOT/traffic_portal-3.0.0-10231.7d01cfdb.el7.x86_64
 '!=' / ']'
[32;1mtraffic_portal_build_1   |[0m + rm -rf 
/tmp/trafficcontrol/rpmbuild/BUILDROOT/traffic_portal-3.0.0-10231.7d01cfdb.el7.x86_64
[32;1mtraffic_portal_build_1   |[0m ++ dirname 
/tmp/trafficcontrol/rpmbuild/BUILDROOT/traffic_portal-3.0.0-10231.7d01cfdb.el7.x86_64
[32;1mtraffic_portal_build_1   |[0m + mkdir -p 
/tmp/trafficcontrol/rpmbuild/BUILDROOT

[GitHub] [trafficcontrol] mitchell852 merged pull request #3883: Fixed double-build of TR RPMs in CiaB makefile

2019-10-09 Thread GitBox

mitchell852 merged pull request #3883: Fixed double-build of TR RPMs in CiaB 
makefile
URL: https://github.com/apache/trafficcontrol/pull/3883
 
 
   


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] mitchell852 commented on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540051772
 
 
   > yep! @mitchell852 that would look like:
   > 
   > ```
   >{
   >"response": [
   >{
   >"lastUpdated": "2019-10-05 22:05:31+00",
   >"serverHostName": "foco-1",
   >"serverId": 1,
   >"serverCapability": "foo"
   >},
   >{
   >"lastUpdated": "2019-10-08 22:05:31+00",
   >"serverHostName": "foco-2",
   >"serverId": 2,
   >"serverCapability": "foo"
   >},
   >{
   >"lastUpdated": "2019-10-10 22:05:31+00",
   >"serverHostName": "loveland",
   >"serverId": 3,
   >"serverCapability": "foo"
   >}
   >]
   >}
   > ```
   
   so that will leave the UI pretty lean. i.e. I will only be able to show the 
hostnames of the servers that use the capability and maybe that's fine. What do 
you think @rob05c 
   
   This format will increase the response size obviously but will provide more 
data that can be used in TP:
   
   ```
   [
   {
   "lastUpdated": "2019-10-10 22:05:31+00",
   "server": {
   "hostName": "loveland",
   "id": 3,
   ...
   }
   "serverCapability": "foo"
   },
   ...
   ]
   ```


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 #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

rob05c commented on a change in pull request #3972: Add Server Capabilities 
blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333089204
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
+
+## Problem Description
+
+Suppose a Traffic Control operator has servers of a particular type. For 
example, servers with only Memory and no Disk cache. It's possible today to 
only route to those Edges, via manual Delivery Service Server assignments. But 
suppose you have a Mid server with only Memory and no Disk. Then suppose you 
have a certain class of traffic you need to route to this Mid, but not other 
traffic. For example, Delivery Services with small images; but not DSes with 
large binary files, which would destroy the cache. Right now, this isn't 
possible in Traffic Control.
+
+We propose a feature, called "Server Capabilitites" to solve this problem. 
Servers will have "capabilitites" and Delivery Services will have "Required 
Capabilitites," and if a server does not have a required capability, then it 
should not be manually assignable as an Edge, and a Mid must not be added as a 
parent for that DS in the Edge ATS config.
+
+Initially, this is completely backwards-compatible on upgrade, because 
initially no Delivery Service will have any Required Capabilities.
+
+This feature will be completely optional. TC operators who don't need Server 
Capabilities will simply not create them.
+
+Server Capabilities will be arbitrary strings. The ATS project will not "seed" 
any, and impose as little direction as possible. For example, Server 
Capabilitites could be Cache types, Server types, Hardware types, ATS versions, 
Lua support, other ATS plugin support, or any other features an operator needs 
to route (or not route) on.
+
+
+## Proposed Change
+
+- Servers have Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK )
+
+- Delivery Services have Required Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK)
+
+- When generating Configuration (e.g. ATS parent.config):
+  - If a Mid Server which is otherwise parented to an Edge does not have all 
Required Capabilities of a Delivery Service, that Mid will not be inserted as a 
parent for that Delivery Service’s remap and parent rules.
+  - Backward Compatibility is automatic:
+- Initially, no Delivery Services have Required Capabilities. Hence, 
everything behaves like it does today.
+- If an EDGE server does not have all capabilities required by a DS, that 
server SHOULD NOT be assignable to that DS. 
+- If an EDGE server is assigned to a DS which requires capabilities that 
server has, it SHOULD NOT be possible to remove those capabilities from that 
server.
+
+### Traffic Portal Impact
+
+- Server Page
+  - New dropdown to add to a text list: Capabilities
+  - List elements have a button to remove the capability
+  - Dropdown/List is just one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- Delivery Service Page
+  - New dropdown to add to a text list: Required Capabilities
+  - List elements have a button to remove a capability
+  - Dropdown/List is one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- New page for Server Capability Types
+  - Ability to add, delete Server Capability Types
+  - May be a text input, which adds to a list, with list element buttons to 
remove from the list
+
+
+### Traffic Ops Impact
+
+`/server_server_capabilities`
+  - List all Server capabilities
 
 Review comment:
   Changed


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] mitchell852 commented on issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

mitchell852 commented on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540061661
 
 
   i don't suppose you happened to add entries to seeds.sql for creating a 
"capability" (i'm talking about a user capability here) like the following:
   
   server-capabilities-read
   server-capabilities-write
   
   seems a bit silly since we don't use "user capabilities" yet but i feel like 
it will make our lives easier if every time we add new api endpoints we keep 
the "user capabilities" current.


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] mitchell852 edited a comment on issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

mitchell852 edited a comment on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540061661
 
 
   i don't suppose you happened to add entries to seeds.sql for creating a 
"capability" (i'm talking about a user capability here) like the following:
   
   server-capabilities-read (for the GET)
   server-capabilities-write (for the POST/DELETE)
   
   seems a bit silly since we don't use "user capabilities" yet but i feel like 
it will make our lives easier if every time we add new api endpoints we keep 
the "user capabilities" current.


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] mitchell852 edited a comment on issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

mitchell852 edited a comment on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540061661
 
 
   i don't suppose you happened to add entries to seeds.sql for creating a 
"capability" (i'm talking about a user capability here) like the following:
   
   server-capabilities-read (for the GET) 
   server-capabilities-write (for the POST/DELETE) - attached just to admin 
role?
   
   seems a bit silly since we don't use "user capabilities" yet but i feel like 
it will make our lives easier if every time we add new api endpoints we keep 
the "user capabilities" current.


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] mitchell852 edited a comment on issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

mitchell852 edited a comment on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540061661
 
 
   i don't suppose you happened to add entries to seeds.sql for creating a 
"capability" (i'm talking about a user capability here) like the following:
   
   server-capabilities-read (for the GET)
   server-capabilities-write (for the POST/DELETE)
   
   (both probably just attached to the admin role)
   
   seems a bit silly since we don't use "user capabilities" yet but i feel like 
it will make our lives easier if every time we add new api endpoints we keep 
the "user capabilities" current.


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 #3971: Add server_capabilities API docs

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3971: Add server_capabilities API 
docs
URL: https://github.com/apache/trafficcontrol/pull/3971#discussion_r333043201
 
 

 ##
 File path: docs/source/api/server_capabilities.rst
 ##
 @@ -0,0 +1,190 @@
+..
+..
+.. 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.
+..
+
+.. _to-api-server_capabilities:
+
+***
+``server_capabilities``
+***
+
+``GET``
+===
+Retrieves server capabilities.
+
+:Auth. Required: Yes
+:Roles Required: "read-only"
+:Response Type:  Array
+
+Request Structure
+-
+.. table:: Request Query Parameters
+
+   
++--++
+   |Name| Required |Description

 |
+   
++==++
+   |name| no   | Return the server capability with this name   

 |
+   
++--++
+
+.. code-block:: http
+   :caption: Request Structure
+
+   GET /api/1.4/server_capabilities?name=RAM HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+
+Response Structure
+--
+:name:The name of this server capability
+:lastUpdated: The date and time at which this server capability was last 
updated, in ISO format
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
EH8jo8OrCu79Tz9xpgT3YRyKJ/p2NcTmbS3huwtqRByHz9H6qZLQjA59RIPaVSq3ZxsU6QhTaox5nBkQ9LPSAA==
+   X-Server-Name: traffic_ops_golang/
+   Date: Mon, 07 Oct 2019 21:36:13 GMT
+   Content-Length: 68
+
+   {
+   "response": [
+   {
+   "name": "RAM",
+   "lastUpdated": "2019-10-07 20:38:24+00"
+   }
+   ]
+   }
+
+``POST``
+
+Create a new server capability.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type:  Object
+
+Request Structure
+-
+:name: The name of the server capability
+
+.. code-block:: http
+   :caption: Request Example
+
+   POST /api/1.4/server_capabilities HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 15
+   Content-Type: application/json
+
+   {
+   "name": "RAM"
+   }
+
+Response Structure
+--
+:name:The name of this server capability
+:lastUpdated: The date and time at which this server capability was last 
updated, in ISO format
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
ysdopC//JQI79BRUa61s6M2HzHxYHpo5RdcuauOoqCYxiVOoUhNZfOVydVkv8zDN2qA374XKnym4kWj3VzQIXg==
+   X-Server-Name: traffic_ops_golang/
+   Date: Mon, 07 Oct 2019 22:10:00 GMT
+   Content-Length: 137
+
+   {
+   "alerts": [
+   {
+   "text": "server capability was created.",
+   "level": "success"
+   }
+

[GitHub] [trafficcontrol] mitchell852 commented on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540045010
 
 
   without reading thru the code does DELETE ensure that no delivery services 
are currently assigned to the server that require the server capability that is 
being removed from the server? (confusing sentence)


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 #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3972: Add Server Capabilities 
blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333075565
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
 
 Review comment:
   I don't suppose we could find a better name for this? "Capabilities" is 
already used for user capabilities. I don't really have a better suggestion, 
though.


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 #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3972: Add Server Capabilities 
blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333070312
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
+
+## Problem Description
+
+Suppose a Traffic Control operator has servers of a particular type. For 
example, servers with only Memory and no Disk cache. It's possible today to 
only route to those Edges, via manual Delivery Service Server assignments. But 
suppose you have a Mid server with only Memory and no Disk. Then suppose you 
have a certain class of traffic you need to route to this Mid, but not other 
traffic. For example, Delivery Services with small images; but not DSes with 
large binary files, which would destroy the cache. Right now, this isn't 
possible in Traffic Control.
+
+We propose a feature, called "Server Capabilitites" to solve this problem. 
Servers will have "capabilitites" and Delivery Services will have "Required 
Capabilitites," and if a server does not have a required capability, then it 
should not be manually assignable as an Edge, and a Mid must not be added as a 
parent for that DS in the Edge ATS config.
+
+Initially, this is completely backwards-compatible on upgrade, because 
initially no Delivery Service will have any Required Capabilities.
+
+This feature will be completely optional. TC operators who don't need Server 
Capabilities will simply not create them.
+
+Server Capabilities will be arbitrary strings. The ATS project will not "seed" 
any, and impose as little direction as possible. For example, Server 
Capabilitites could be Cache types, Server types, Hardware types, ATS versions, 
Lua support, other ATS plugin support, or any other features an operator needs 
to route (or not route) on.
+
+
+## Proposed Change
+
+- Servers have Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK )
+
+- Delivery Services have Required Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK)
+
+- When generating Configuration (e.g. ATS parent.config):
+  - If a Mid Server which is otherwise parented to an Edge does not have all 
Required Capabilities of a Delivery Service, that Mid will not be inserted as a 
parent for that Delivery Service’s remap and parent rules.
+  - Backward Compatibility is automatic:
+- Initially, no Delivery Services have Required Capabilities. Hence, 
everything behaves like it does today.
+- If an EDGE server does not have all capabilities required by a DS, that 
server SHOULD NOT be assignable to that DS. 
+- If an EDGE server is assigned to a DS which requires capabilities that 
server has, it SHOULD NOT be possible to remove those capabilities from that 
server.
+
+### Traffic Portal Impact
+
+- Server Page
+  - New dropdown to add to a text list: Capabilities
+  - List elements have a button to remove the capability
+  - Dropdown/List is just one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- Delivery Service Page
+  - New dropdown to add to a text list: Required Capabilities
+  - List elements have a button to remove a capability
+  - Dropdown/List is one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- New page for Server Capability Types
+  - Ability to add, delete Server Capability Types
+  - May be a text input, which adds to a list, with list element buttons to 
remove from the list
+
+
+### Traffic Ops Impact
+
+`/server_server_capabilities`
+  - List all Server capabilities
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/deliveryservices_required_capabilities`
+  - List all DS Required Capabilities
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/server_capabilities`
+  - List, create, and delete Server Capabilities.
+  - POST+GET+DELETE
+  - PUT doesn’t make sense, Capabilities are strings, if one is misspelled it 
should be removed and re-added. Simpler, less error-prone than cascading 
changes.
+
+Additionally, ATS config generation (which is currently in Traffic Ops, but in 
the process of being moved to its own library/app) will require changes. 
Primarily `parent.config`, as in the description, to prevent Mids being 
assigned as Parents for DSes for which they lack the capabilitites.
+
+ REST API Impact
+
+See Traffic Ops Impact
+
+ Client Impact
+
+See Traffic Ops Impact - client functions for each endpoint.
+
+ Data Model / Database Impact
+
+New tables for Capabilitites: `server_capability`, `server_server_capability`, 
`delivery_service_required_server_capability`.
+
+### ORT Impact
+
+n/a
+
+### Traffic Monitor Impact
+
+n/a
+
+### Traffic Router Impact
+
+n/a
+
+### Traffic Stats Impact
+
+n/a
+
+### Traffic Vault Impact
+
+n/a
+
+### Documentation Impact
+
+The new UI and API endpoints will be

[GitHub] [trafficcontrol] mhoppa commented on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mhoppa commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540048971
 
 
   yep! @mitchell852 that would look like:
   
   ```
{
"response": [
{
"lastUpdated": "2019-10-05 22:05:31+00",
"serverHostName": "foco-1",
"serverId": 1,
"serverCapability": "foo"
},
{
"lastUpdated": "2019-10-08 22:05:31+00",
"serverHostName": "foco-2",
"serverId": 2,
"serverCapability": "foo"
},
{
"lastUpdated": "2019-10-10 22:05:31+00",
"serverHostName": "loveland",
"serverId": 3,
"serverCapability": "foo"
}
]
}
   ```


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 #3966: Add server capabilities API

2019-10-09 Thread GitBox

rob05c commented on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540057546
 
 
   >without reading thru the code, does DELETE validate that the "server 
capability" is:
   
   >a. not being used by any server and
   >b. is not required by any ds
   
   Server-Capabilities and DS-Required-Capabilities don't exist in this PR, 
that'll have to be added in a subsequent PR.
   
   But, I'd vote we don't add custom Go logic to handle that - rather, if we 
make them proper Foreign Keys that don't CASCADE DELETE, the DB will 
automatically prevent it. Then, all we have to do is parse the Postgres error 
to return a useful message to the user (we already have a func for that 
`api.ParseDBError`


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 #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

rob05c commented on a change in pull request #3972: Add Server Capabilities 
blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333094054
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
 
 Review comment:
   Ehh. We discussed this quite a bit. Yeah, it's awkward that "capabilities" 
already exist (though those are really "permissions"). But there's not a good 
synonym that means exactly the same thing. There are similar things, but they 
all imply or omit what this actually is. It really is a "capability" of the 
server, something it's capable of doing.
   
   We were hoping by calling the entire feature "Server Capabilitites," e.g. 
the assignments are "server_server_capabilities," it would be clearer that 
they're not the same. It's hard to imagine anyone being confused anyway, users 
and servers are totally different things.


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 #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

asf-ci commented on issue #3972: Add Server Capabilities blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#issuecomment-540064276
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4432/
   


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 #3971: Add server_capabilities API docs

2019-10-09 Thread GitBox

asf-ci commented on issue #3971: Add server_capabilities API docs
URL: https://github.com/apache/trafficcontrol/pull/3971#issuecomment-540068003
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4433/
   


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 #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

ocket commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540075579
 
 
   well you can fetch a server's details from its hostname. What would help is 
if a server automatically came with its list of capabilities...


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 issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mhoppa commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540046101
 
 
   @mitchell852 makes sense to me :) it does not as we do not have the 
association on delivery service to server capability yet. Once we do then I 
will be adding the validation in follow on PR. 


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 issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

mhoppa commented on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540058711
 
 
   > > without reading thru the code, does DELETE validate that the "server 
capability" is:
   > 
   > > a. not being used by any server and
   > > b. is not required by any ds
   > 
   > Server-Capabilities and DS-Required-Capabilities don't exist in this PR, 
that'll have to be added in a subsequent PR.
   > 
   > But, I'd vote we don't add custom Go logic to handle that - rather, if we 
make them proper Foreign Keys that don't CASCADE DELETE, the DB will 
automatically prevent it. Then, all we have to do is parse the Postgres error 
to return a useful message to the user (we already have a func for that 
`api.ParseDBError`
   
   FWIW Did add that constraint to my PR per @rawlinp feedback 
https://github.com/apache/trafficcontrol/pull/3964/files#diff-7d3547a278b38f437ab0196c94a8R26


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 #3972: Add Server Capabilities blueprint

2019-10-09 Thread GitBox

rob05c commented on a change in pull request #3972: Add Server Capabilities 
blueprint
URL: https://github.com/apache/trafficcontrol/pull/3972#discussion_r333092037
 
 

 ##
 File path: blueprints/server-capabilitites.md
 ##
 @@ -0,0 +1,162 @@
+
+# Server Capabilities
+
+## Problem Description
+
+Suppose a Traffic Control operator has servers of a particular type. For 
example, servers with only Memory and no Disk cache. It's possible today to 
only route to those Edges, via manual Delivery Service Server assignments. But 
suppose you have a Mid server with only Memory and no Disk. Then suppose you 
have a certain class of traffic you need to route to this Mid, but not other 
traffic. For example, Delivery Services with small images; but not DSes with 
large binary files, which would destroy the cache. Right now, this isn't 
possible in Traffic Control.
+
+We propose a feature, called "Server Capabilitites" to solve this problem. 
Servers will have "capabilitites" and Delivery Services will have "Required 
Capabilitites," and if a server does not have a required capability, then it 
should not be manually assignable as an Edge, and a Mid must not be added as a 
parent for that DS in the Edge ATS config.
+
+Initially, this is completely backwards-compatible on upgrade, because 
initially no Delivery Service will have any Required Capabilities.
+
+This feature will be completely optional. TC operators who don't need Server 
Capabilities will simply not create them.
+
+Server Capabilities will be arbitrary strings. The ATS project will not "seed" 
any, and impose as little direction as possible. For example, Server 
Capabilitites could be Cache types, Server types, Hardware types, ATS versions, 
Lua support, other ATS plugin support, or any other features an operator needs 
to route (or not route) on.
+
+
+## Proposed Change
+
+- Servers have Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK )
+
+- Delivery Services have Required Capabilities (i.e.: CACHE_MEMORY, CACHE_DISK)
+
+- When generating Configuration (e.g. ATS parent.config):
+  - If a Mid Server which is otherwise parented to an Edge does not have all 
Required Capabilities of a Delivery Service, that Mid will not be inserted as a 
parent for that Delivery Service’s remap and parent rules.
+  - Backward Compatibility is automatic:
+- Initially, no Delivery Services have Required Capabilities. Hence, 
everything behaves like it does today.
+- If an EDGE server does not have all capabilities required by a DS, that 
server SHOULD NOT be assignable to that DS. 
+- If an EDGE server is assigned to a DS which requires capabilities that 
server has, it SHOULD NOT be possible to remove those capabilities from that 
server.
+
+### Traffic Portal Impact
+
+- Server Page
+  - New dropdown to add to a text list: Capabilities
+  - List elements have a button to remove the capability
+  - Dropdown/List is just one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- Delivery Service Page
+  - New dropdown to add to a text list: Required Capabilities
+  - List elements have a button to remove a capability
+  - Dropdown/List is one option - other GUI components may be used
+  - Component should consider that there could be many Capabilities
+
+- New page for Server Capability Types
+  - Ability to add, delete Server Capability Types
+  - May be a text input, which adds to a list, with list element buttons to 
remove from the list
+
+
+### Traffic Ops Impact
+
+`/server_server_capabilities`
+  - List all Server capabilities
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/deliveryservices_required_capabilities`
+  - List all DS Required Capabilities
+  - GET+POST+DELETE
+  - PUT doesn't make sense. To remove one and add another, DELETE and POST.
+
+`/server_capabilities`
+  - List, create, and delete Server Capabilities.
+  - POST+GET+DELETE
+  - PUT doesn’t make sense, Capabilities are strings, if one is misspelled it 
should be removed and re-added. Simpler, less error-prone than cascading 
changes.
+
+Additionally, ATS config generation (which is currently in Traffic Ops, but in 
the process of being moved to its own library/app) will require changes. 
Primarily `parent.config`, as in the description, to prevent Mids being 
assigned as Parents for DSes for which they lack the capabilitites.
+
+ REST API Impact
+
+See Traffic Ops Impact
+
+ Client Impact
+
+See Traffic Ops Impact - client functions for each endpoint.
+
+ Data Model / Database Impact
+
+New tables for Capabilitites: `server_capability`, `server_server_capability`, 
`delivery_service_required_server_capability`.
+
+### ORT Impact
+
+n/a
+
+### Traffic Monitor Impact
+
+n/a
+
+### Traffic Router Impact
+
+n/a
+
+### Traffic Stats Impact
+
+n/a
+
+### Traffic Vault Impact
+
+n/a
+
+### Documentation Impact
+
+The new UI and API endpoints will be

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #3971: Add server_capabilities API docs

2019-10-09 Thread GitBox

rawlinp commented on a change in pull request #3971: Add server_capabilities 
API docs
URL: https://github.com/apache/trafficcontrol/pull/3971#discussion_r333096916
 
 

 ##
 File path: docs/source/api/server_capabilities.rst
 ##
 @@ -0,0 +1,190 @@
+..
+..
+.. 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.
+..
+
+.. _to-api-server_capabilities:
+
+***
+``server_capabilities``
+***
+
+``GET``
+===
+Retrieves server capabilities.
+
+:Auth. Required: Yes
+:Roles Required: "read-only"
+:Response Type:  Array
+
+Request Structure
+-
+.. table:: Request Query Parameters
+
+   
++--++
+   |Name| Required |Description

 |
+   
++==++
+   |name| no   | Return the server capability with this name   

 |
+   
++--++
+
+.. code-block:: http
+   :caption: Request Structure
+
+   GET /api/1.4/server_capabilities?name=RAM HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+
+Response Structure
+--
+:name:The name of this server capability
+:lastUpdated: The date and time at which this server capability was last 
updated, in ISO format
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
EH8jo8OrCu79Tz9xpgT3YRyKJ/p2NcTmbS3huwtqRByHz9H6qZLQjA59RIPaVSq3ZxsU6QhTaox5nBkQ9LPSAA==
+   X-Server-Name: traffic_ops_golang/
+   Date: Mon, 07 Oct 2019 21:36:13 GMT
+   Content-Length: 68
+
+   {
+   "response": [
+   {
+   "name": "RAM",
+   "lastUpdated": "2019-10-07 20:38:24+00"
+   }
+   ]
+   }
+
+``POST``
+
+Create a new server capability.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type:  Object
+
+Request Structure
+-
+:name: The name of the server capability
+
+.. code-block:: http
+   :caption: Request Example
+
+   POST /api/1.4/server_capabilities HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 15
+   Content-Type: application/json
+
+   {
+   "name": "RAM"
+   }
+
+Response Structure
+--
+:name:The name of this server capability
+:lastUpdated: The date and time at which this server capability was last 
updated, in ISO format
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
ysdopC//JQI79BRUa61s6M2HzHxYHpo5RdcuauOoqCYxiVOoUhNZfOVydVkv8zDN2qA374XKnym4kWj3VzQIXg==
+   X-Server-Name: traffic_ops_golang/
+   Date: Mon, 07 Oct 2019 22:10:00 GMT
+   Content-Length: 137
+
+   {
+   "alerts": [
+   {
+   "text": "server capability was created.",
+   "level": "success"
+   }
+

[GitHub] [trafficcontrol] mhoppa commented on issue #3971: Add server_capabilities API docs

2019-10-09 Thread GitBox

mhoppa commented on issue #3971: Add server_capabilities API docs
URL: https://github.com/apache/trafficcontrol/pull/3971#issuecomment-540072058
 
 
   Im fine waiting for that @rawlinp actually might be good because then we can 
include screen shots of the front end and use of the endpoints together 


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 #3962: Add atscfg remap.config tests

2019-10-09 Thread GitBox

rob05c commented on issue #3962: Add atscfg remap.config tests
URL: https://github.com/apache/trafficcontrol/pull/3962#issuecomment-540080137
 
 
   >I do think use of table driven tests 
   
   Huh, TIL. I do that a lot, didn't know it had a name.
   
   But, if we wanted to reduce the "duplicate" lines, I think some kind of 
`MakeDefaultServerInfo` would go farther, and then each test would modify what 
it cared about.
   
   I was on the fence about that. It'd reduce the lines of code, but I also 
kind of feel like it's good for each test to be completely independent, and 
saying "I'm testing exactly this data," and there's no shared func that would 
change all the tests if it were changed. 
   
   I went with the latter, but I'm personally not sure which is a "better" test 
路‍♂ .


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] mitchell852 commented on issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

mitchell852 commented on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540043806
 
 
   without reading thru the code, does DELETE validate that the "server 
capability" is:
   
   a. not being used by any server and
   b. is not required by any ds
   
   with some sort of alert error like "server capability failed. server 
capability used by X servers/delivery services"
   
   sorry, i should just read the code :)


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] mitchell852 commented on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540047268
 
 
   @mhoppa - can you give me an example of what the response to this would look 
like:
   
   GET /server_server_capabilities/?serverCapability=foo
   
   just trying to make sure I have all the data to make a UI to "show all the 
servers that employ a certain server capability"


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] mitchell852 edited a comment on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 edited a comment on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540051772
 
 
   > yep! @mitchell852 that would look like:
   > 
   > ```
   >{
   >"response": [
   >{
   >"lastUpdated": "2019-10-05 22:05:31+00",
   >"serverHostName": "foco-1",
   >"serverId": 1,
   >"serverCapability": "foo"
   >},
   >{
   >"lastUpdated": "2019-10-08 22:05:31+00",
   >"serverHostName": "foco-2",
   >"serverId": 2,
   >"serverCapability": "foo"
   >},
   >{
   >"lastUpdated": "2019-10-10 22:05:31+00",
   >"serverHostName": "loveland",
   >"serverId": 3,
   >"serverCapability": "foo"
   >}
   >]
   >}
   > ```
   
   so that will leave the UI pretty lean. i.e. I will only be able to show the 
hostnames of the servers that use the capability and maybe that's fine. What do 
you think @rob05c 
   
   This format will increase the response size obviously but will provide more 
data that can be used in TP:
   
   ```
   [
   {
   "lastUpdated": "2019-10-10 22:05:31+00",
   "server": {
   "hostName": "loveland",
   "id": 3,
   ... rest of the server properties ...
   }
   "serverCapability": "foo"
   },
   ...
   ]
   ```


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 #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3954: Added deprecation notices 
for /user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#discussion_r333079221
 
 

 ##
 File path: traffic_ops/app/lib/MojoPlugins/Response.pm
 ##
 @@ -119,6 +119,27 @@ sub register {
}
);
 
+   $app->renderer->add_helper(
+   with_deprecation => sub {
 
 Review comment:
   Good work on this!


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 #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3954: Added deprecation notices 
for /user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#discussion_r333078932
 
 

 ##
 File path: traffic_ops/app/lib/API/User.pm
 ##
 @@ -599,6 +599,123 @@ sub current {
}
 }
 
+# Designated handler for the deprecated path to updating current users
+sub user_current_update {
 
 Review comment:
   Why did a new handler needed to be added for deprecation notice? Shouldn't 
we just be adding a deprecation message to the response on the current handler? 


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] mitchell852 edited a comment on issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

mitchell852 edited a comment on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540061661
 
 
   i don't suppose you happened to add entries to seeds.sql for creating a 
"capability" (i'm talking about a user capability here) like the following:
   
   server-capabilities-read (for the GET) 
   server-capabilities-write (for the POST/DELETE) - attached just to admin 
role?
   
   seems a bit silly since we don't use "user capabilities" yet but i feel like 
it will make our lives easier (if/when roles/capabilities is every enabled) if 
every time we add new api endpoints we keep the "user capabilities" current.


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 issue #3971: Add server_capabilities API docs

2019-10-09 Thread GitBox

rawlinp commented on issue #3971: Add server_capabilities API docs
URL: https://github.com/apache/trafficcontrol/pull/3971#issuecomment-540071587
 
 
   Sure, I can take a shot, but we should probably have more holistic 
documentation PR describing how to use all the endpoints together.


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 issue #3929: Rewrote /user/reset_password to Go

2019-10-09 Thread GitBox

mhoppa commented on issue #3929: Rewrote /user/reset_password to Go
URL: https://github.com/apache/trafficcontrol/pull/3929#issuecomment-540077170
 
 
   Since #3925 is merged want to merge this with master and mark it as non 
draft? 
   


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 #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3954: Added deprecation 
notices for /user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#discussion_r333111865
 
 

 ##
 File path: traffic_ops/app/lib/API/User.pm
 ##
 @@ -599,6 +599,123 @@ sub current {
}
 }
 
+# Designated handler for the deprecated path to updating current users
+sub user_current_update {
 
 Review comment:
   The current handler is also the handler for `PUT /user/current`, which isn't 
deprecated. Since the rendering happens before the handler returns, I couldn't 
just wrap it, and if there's a way to access what path was requested from 
within a Mojolicious handler then I don't know it. So this seemed to be my only 
option.


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 #3766: Cachegroup fallbacks deprecation

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3766: Cachegroup fallbacks 
deprecation
URL: https://github.com/apache/trafficcontrol/pull/3766#discussion_r333110700
 
 

 ##
 File path: docs/source/api/cachegroups.rst
 ##
 @@ -123,19 +130,25 @@ Creates a :term:`Cache Group`
 
 Request Structure
 -
-:fallbackToClosest: If ``true``, the Traffic Router will fall back on the 
'closest' :term:`Cache Group` to this one, when this one fails
+:fallbacks: An optional field which, when present, should contain an array of 
names of other :term:`Cache Groups` on which the Traffic Router will fall back 
in the event that this :term:`Cache Group` fails/becomes unavailable\ 
[#fallbacks]_
+
+   .. versionadded:: ATCv4.0
 
 Review comment:
   should this be API version? same comment in the other places that reference 
this version


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 #3766: Cachegroup fallbacks deprecation

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3766: Cachegroup fallbacks 
deprecation
URL: https://github.com/apache/trafficcontrol/pull/3766#discussion_r333110506
 
 

 ##
 File path: docs/source/api/cachegroup_fallbacks.rst
 ##
 @@ -19,6 +19,10 @@
 ``cachegroup_fallbacks``
 
 
+.. deprecated:: ATCv4.0
 
 Review comment:
   I have seen in other deprecation notices that we use the API version not the 
ATC version. Should this match those?


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 #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3954: Added deprecation notices 
for /user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#discussion_r333115367
 
 

 ##
 File path: traffic_ops/app/lib/API/User.pm
 ##
 @@ -599,6 +599,123 @@ sub current {
}
 }
 
+# Designated handler for the deprecated path to updating current users
+sub user_current_update {
 
 Review comment:
   ugh! i hope that we dont have to do that with a lot of the other endpoints 
we want to depreciate. Cool let me pull and test this then! 


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 #3766: Cachegroup fallbacks deprecation

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3766: Cachegroup fallbacks 
deprecation
URL: https://github.com/apache/trafficcontrol/pull/3766#discussion_r333126189
 
 

 ##
 File path: docs/source/api/cachegroups.rst
 ##
 @@ -123,19 +130,25 @@ Creates a :term:`Cache Group`
 
 Request Structure
 -
-:fallbackToClosest: If ``true``, the Traffic Router will fall back on the 
'closest' :term:`Cache Group` to this one, when this one fails
+:fallbacks: An optional field which, when present, should contain an array of 
names of other :term:`Cache Groups` on which the Traffic Router will fall back 
in the event that this :term:`Cache Group` fails/becomes unavailable\ 
[#fallbacks]_
+
+   .. versionadded:: ATCv4.0
 
 Review comment:
   Unfortunately no. The `fallbacks` array was added to the cachegroup response 
object in all API versions in ATCv4.
   
   FWIW this is a big part of why I don't think the API ought to be versioned 
separately from TO. 


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] ajschmidt commented on a change in pull request #3961: added ability to select TLS version in TR

2019-10-09 Thread GitBox

ajschmidt commented on a change in pull request #3961: added ability to select 
TLS version in TR
URL: https://github.com/apache/trafficcontrol/pull/3961#discussion_r333116037
 
 

 ##
 File path: 
traffic_router/connector/src/main/java/com/comcast/cdn/traffic_control/traffic_router/protocol/RouterNioEndpoint.java
 ##
 @@ -120,5 +121,13 @@ protected void doRun(){
}
}
 
+   public String getProtocols() {
+   return protocols;
+   }
+
+   public void setProtocols(final String protocols) {
+   this.protocols = protocols;
+   }
+
 
 Review comment:
   I see you are correct that 'protocols' is a legitimate attribute. I think it 
would be nice to honor all of the attributes that are set in the `Connector` 
element while you are at it instead of selecting specific ones. That is why 
I've suggested this alternate implementation. We could circle back to this in 
the future.  


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] mitchell852 commented on issue #3966: Add server capabilities API

2019-10-09 Thread GitBox

mitchell852 commented on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540090948
 
 
   > Should POST/DELETE only be admin? Right now I have it set to Operations.
   
   probably a question for @rob05c 


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] mitchell852 edited a comment on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 edited a comment on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540091783
 
 
   > well you can fetch a server's details from its hostname. What would help 
is if a server automatically came with its list of capabilities...
   
   right but imagine populating a UI table of servers that utilize capability 
foo. maybe there are 50 servers, so make 50 api calls to populate the table?


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] mitchell852 commented on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540091783
 
 
   > well you can fetch a server's details from its hostname. What would help 
is if a server automatically came with its list of capabilities...
   
   right but imagine populating a table of servers that utilize capability foo. 
maybe there are 50 servers, so make 50 api calls to populate the table?


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] mitchell852 edited a comment on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 edited a comment on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540119364
 
 
   you may need to add "user capabilities" to seeds.sql for these new 
endpoints. i.e.
   
   server-server-capabilities-read
   server-server-capabilities-write
   
   see this comment on another PR: 
https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540061661


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] mitchell852 commented on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

mitchell852 commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540119364
 
 
   you may need to add "user capabilities" for these new endpoints. i.e.
   
   server-server-capabilities-read
   server-server-capabilities-write
   
   see this comment on another PR: 
https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540061661


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 #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

ocket commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540120707
 
 
   Right that's what I'm saying. If you want full server information in this 
payload, then you've basically created an entirely new endpoint just to add a 
field to `/servers`. All this to avoid API versioning hell in the handler.
   
   I think hostname is probably the best you could ask for - maybe an ID too, 
not that that's really what a TP user is interested in seeing.


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 #3966: Add server capabilities API

2019-10-09 Thread GitBox

asf-ci commented on issue #3966: Add server capabilities API
URL: https://github.com/apache/trafficcontrol/pull/3966#issuecomment-540120851
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4435/
   


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 #3766: Cachegroup fallbacks deprecation

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3766: Cachegroup fallbacks 
deprecation
URL: https://github.com/apache/trafficcontrol/pull/3766#discussion_r333126435
 
 

 ##
 File path: docs/source/api/cachegroups.rst
 ##
 @@ -123,19 +130,25 @@ Creates a :term:`Cache Group`
 
 Request Structure
 -
-:fallbackToClosest: If ``true``, the Traffic Router will fall back on the 
'closest' :term:`Cache Group` to this one, when this one fails
+:fallbacks: An optional field which, when present, should contain an array of 
names of other :term:`Cache Groups` on which the Traffic Router will fall back 
in the event that this :term:`Cache Group` fails/becomes unavailable\ 
[#fallbacks]_
+
+   .. versionadded:: ATCv4.0
 
 Review comment:
    agree very confusing to end 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] asf-ci commented on issue #3929: Rewrote /user/reset_password to Go

2019-10-09 Thread GitBox

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


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 #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3954: Added deprecation 
notices for /user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#discussion_r333128810
 
 

 ##
 File path: traffic_ops/app/lib/API/User.pm
 ##
 @@ -599,6 +599,123 @@ sub current {
}
 }
 
+# Designated handler for the deprecated path to updating current users
+sub user_current_update {
 
 Review comment:
   generally I don't think so.


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 #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333199475
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/role/roles.go
 ##
 @@ -122,9 +122,11 @@ func (role *TORole) Create() (error, error, int) {
}
 
//after we have role ID we can associate the capabilities:
-   userErr, sysErr, errCode = 
role.createRoleCapabilityAssociations(role.ReqInfo.Tx)
-   if userErr != nil || sysErr != nil {
-   return userErr, sysErr, errCode
+   if role.Capabilities != nil && *role.Capabilities != nil && 
len(*role.Capabilities) > 0 {
 
 Review comment:
   Yeah, so I've heard. Never knew that about Go, makes life much easier


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 #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333201940
 
 

 ##
 File path: docs/source/api/roles.rst
 ##
 @@ -65,19 +84,236 @@ Response Structure
Access-Control-Allow-Origin: *
Content-Type: application/json
Set-Cookie: mojolicious=...; Path=/; HttpOnly
-   Whole-Content-Sha512: 
caagePqpL6u9Mn1UBIDCJSgiLAKOHm72/DcrkxCuS7oLekMe87BkGhyJzkhQqUJh/CTmokr9x053GQ5FjhSKhg==
+   Whole-Content-Sha512: 
TEDXlQqWMSnJbL10JtFdbw0nqciNpjc4bd6m7iAB8aymakWeF+ghs1k5LayjdzHcjeDE8UNF/HXSxOFvoLFEuA==
X-Server-Name: traffic_ops_golang/
-   Date: Mon, 10 Dec 2018 15:34:05 GMT
-   Transfer-Encoding: chunked
+   Date: Wed, 04 Sep 2019 17:15:36 GMT
+   Content-Length: 120
 
{ "response": [
{
"id": 4,
-   "name": "disallowed",
-   "description": "Block all access",
-   "privLevel": 0,
-   "capabilities": []
+   "name": "admin",
+   "description": "super-user",
+   "privLevel": 30,
+   "capabilities": [
+   "all-write",
+   "all-read"
+   ]
}
]}
 
-.. note:: The response example for this method of this endpoint has been 
truncated to only the last element of the resultant array, as the full response 
was hundreds of lines long.
+``POST``
+
+.. versionadded:: 1.3
+
+Creates a new :term:`Role`.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type: Object
+
+Request Structure
+-
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+:description:  A helpful description of the :term:`Role`'s purpose.
+:name: The name of the new :term:`Role`
+:privLevel:The privilege level of the new :term:`Role`\ [#privlevel]_
+
+.. code-block:: http
+   :caption: Request Example
+
+   POST /api/1.3/roles HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 56
+   Content-Type: application/json
+
+   {
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30
+   }
+
+
+Response Structure
+--
+:capabilities: An array of the names of the Capabilities given to this 
:term:`Role`
+
+   .. tip:: This can be ``null`` *or* empty, depending on whether it was 
present in the request body, or merely empty. Obviously, it can also be a 
populated array.
+
+:description: A description of the :term:`Role`
+:id:  The integral, unique identifier for this :term:`Role`
+:name:The name of the :term:`Role`
+:privLevel:   An integer that allows for comparison between :term:`Roles`
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
gzfc7m/in5vVsVP+Y9h6JJfDhgpXKn9VAzoiPENhKbQfP8Q6jug08Rt2AK/3Nz1cx5zZ8P9IjVxDdIg7mlC8bw==
+   X-Server-Name: traffic_ops_golang/
+   Date: Wed, 04 Sep 2019 17:44:42 GMT
+   Content-Length: 150
+
+   { "alerts": [{
+   "text": "role was created.",
+   "level": "success"
+   }],
+   "response": {
+   "id": 5,
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30,
+   "capabilities": null
+   }}
+
+``PUT``
+===
+.. versionadded:: 1.3
+
+Replaces an existing :term:`Role` with one provided by the request.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type:
+
+Request Structure
+-
+.. table:: Request Query Parameters
+
+   
+--+--++
+   | Name | Required | Description 
   |
+   
+==+==++
+   | id   | yes  | The integral, unique identifier of the :term:`Role` 
to be replaced |
+   
+--+--++
+
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+
+   .. warning:: When not present, the affected :term:`Role`'s Capabilities 
will be

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333201791
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/role/roles.go
 ##
 @@ -177,12 +179,16 @@ func (role *TORole) Update() (error, error, int) {
if userErr != nil || sysErr != nil {
return userErr, sysErr, errCode
}
+
// TODO cascade delete, to automatically do this in SQL?
-   userErr, sysErr, errCode = 
role.deleteRoleCapabilityAssociations(role.ReqInfo.Tx)
-   if userErr != nil || sysErr != nil {
-   return userErr, sysErr, errCode
+   if role.Capabilities != nil && *role.Capabilities != nil {
 
 Review comment:
   Not to fix the bug.
   
   Sounds like it might be a good idea, though, as it's probably doing extra 
work otherwise


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 #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333202515
 
 

 ##
 File path: docs/source/api/roles.rst
 ##
 @@ -65,19 +84,236 @@ Response Structure
Access-Control-Allow-Origin: *
Content-Type: application/json
Set-Cookie: mojolicious=...; Path=/; HttpOnly
-   Whole-Content-Sha512: 
caagePqpL6u9Mn1UBIDCJSgiLAKOHm72/DcrkxCuS7oLekMe87BkGhyJzkhQqUJh/CTmokr9x053GQ5FjhSKhg==
+   Whole-Content-Sha512: 
TEDXlQqWMSnJbL10JtFdbw0nqciNpjc4bd6m7iAB8aymakWeF+ghs1k5LayjdzHcjeDE8UNF/HXSxOFvoLFEuA==
X-Server-Name: traffic_ops_golang/
-   Date: Mon, 10 Dec 2018 15:34:05 GMT
-   Transfer-Encoding: chunked
+   Date: Wed, 04 Sep 2019 17:15:36 GMT
+   Content-Length: 120
 
{ "response": [
{
"id": 4,
-   "name": "disallowed",
-   "description": "Block all access",
-   "privLevel": 0,
-   "capabilities": []
+   "name": "admin",
+   "description": "super-user",
+   "privLevel": 30,
+   "capabilities": [
+   "all-write",
+   "all-read"
+   ]
}
]}
 
-.. note:: The response example for this method of this endpoint has been 
truncated to only the last element of the resultant array, as the full response 
was hundreds of lines long.
+``POST``
+
+.. versionadded:: 1.3
+
+Creates a new :term:`Role`.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type: Object
+
+Request Structure
+-
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+:description:  A helpful description of the :term:`Role`'s purpose.
+:name: The name of the new :term:`Role`
+:privLevel:The privilege level of the new :term:`Role`\ [#privlevel]_
+
+.. code-block:: http
+   :caption: Request Example
+
+   POST /api/1.3/roles HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 56
+   Content-Type: application/json
+
+   {
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30
+   }
+
+
+Response Structure
+--
+:capabilities: An array of the names of the Capabilities given to this 
:term:`Role`
+
+   .. tip:: This can be ``null`` *or* empty, depending on whether it was 
present in the request body, or merely empty. Obviously, it can also be a 
populated array.
+
+:description: A description of the :term:`Role`
+:id:  The integral, unique identifier for this :term:`Role`
+:name:The name of the :term:`Role`
+:privLevel:   An integer that allows for comparison between :term:`Roles`
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
gzfc7m/in5vVsVP+Y9h6JJfDhgpXKn9VAzoiPENhKbQfP8Q6jug08Rt2AK/3Nz1cx5zZ8P9IjVxDdIg7mlC8bw==
+   X-Server-Name: traffic_ops_golang/
+   Date: Wed, 04 Sep 2019 17:44:42 GMT
+   Content-Length: 150
+
+   { "alerts": [{
+   "text": "role was created.",
+   "level": "success"
+   }],
+   "response": {
+   "id": 5,
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30,
+   "capabilities": null
+   }}
+
+``PUT``
+===
+.. versionadded:: 1.3
+
+Replaces an existing :term:`Role` with one provided by the request.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type:
+
+Request Structure
+-
+.. table:: Request Query Parameters
+
+   
+--+--++
+   | Name | Required | Description 
   |
+   
+==+==++
+   | id   | yes  | The integral, unique identifier of the :term:`Role` 
to be replaced |
+   
+--+--++
+
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+
+   .. warning:: When not present, the affected :term:`Role`'s Capabilities 
will be

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333202515
 
 

 ##
 File path: docs/source/api/roles.rst
 ##
 @@ -65,19 +84,236 @@ Response Structure
Access-Control-Allow-Origin: *
Content-Type: application/json
Set-Cookie: mojolicious=...; Path=/; HttpOnly
-   Whole-Content-Sha512: 
caagePqpL6u9Mn1UBIDCJSgiLAKOHm72/DcrkxCuS7oLekMe87BkGhyJzkhQqUJh/CTmokr9x053GQ5FjhSKhg==
+   Whole-Content-Sha512: 
TEDXlQqWMSnJbL10JtFdbw0nqciNpjc4bd6m7iAB8aymakWeF+ghs1k5LayjdzHcjeDE8UNF/HXSxOFvoLFEuA==
X-Server-Name: traffic_ops_golang/
-   Date: Mon, 10 Dec 2018 15:34:05 GMT
-   Transfer-Encoding: chunked
+   Date: Wed, 04 Sep 2019 17:15:36 GMT
+   Content-Length: 120
 
{ "response": [
{
"id": 4,
-   "name": "disallowed",
-   "description": "Block all access",
-   "privLevel": 0,
-   "capabilities": []
+   "name": "admin",
+   "description": "super-user",
+   "privLevel": 30,
+   "capabilities": [
+   "all-write",
+   "all-read"
+   ]
}
]}
 
-.. note:: The response example for this method of this endpoint has been 
truncated to only the last element of the resultant array, as the full response 
was hundreds of lines long.
+``POST``
+
+.. versionadded:: 1.3
+
+Creates a new :term:`Role`.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type: Object
+
+Request Structure
+-
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+:description:  A helpful description of the :term:`Role`'s purpose.
+:name: The name of the new :term:`Role`
+:privLevel:The privilege level of the new :term:`Role`\ [#privlevel]_
+
+.. code-block:: http
+   :caption: Request Example
+
+   POST /api/1.3/roles HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 56
+   Content-Type: application/json
+
+   {
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30
+   }
+
+
+Response Structure
+--
+:capabilities: An array of the names of the Capabilities given to this 
:term:`Role`
+
+   .. tip:: This can be ``null`` *or* empty, depending on whether it was 
present in the request body, or merely empty. Obviously, it can also be a 
populated array.
+
+:description: A description of the :term:`Role`
+:id:  The integral, unique identifier for this :term:`Role`
+:name:The name of the :term:`Role`
+:privLevel:   An integer that allows for comparison between :term:`Roles`
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
gzfc7m/in5vVsVP+Y9h6JJfDhgpXKn9VAzoiPENhKbQfP8Q6jug08Rt2AK/3Nz1cx5zZ8P9IjVxDdIg7mlC8bw==
+   X-Server-Name: traffic_ops_golang/
+   Date: Wed, 04 Sep 2019 17:44:42 GMT
+   Content-Length: 150
+
+   { "alerts": [{
+   "text": "role was created.",
+   "level": "success"
+   }],
+   "response": {
+   "id": 5,
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30,
+   "capabilities": null
+   }}
+
+``PUT``
+===
+.. versionadded:: 1.3
+
+Replaces an existing :term:`Role` with one provided by the request.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type:
+
+Request Structure
+-
+.. table:: Request Query Parameters
+
+   
+--+--++
+   | Name | Required | Description 
   |
+   
+==+==++
+   | id   | yes  | The integral, unique identifier of the :term:`Role` 
to be replaced |
+   
+--+--++
+
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+
+   .. warning:: When not present, the affected :term:`Role`'s Capabilities 
will be

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333203902
 
 

 ##
 File path: docs/source/api/roles.rst
 ##
 @@ -65,19 +84,236 @@ Response Structure
Access-Control-Allow-Origin: *
Content-Type: application/json
Set-Cookie: mojolicious=...; Path=/; HttpOnly
-   Whole-Content-Sha512: 
caagePqpL6u9Mn1UBIDCJSgiLAKOHm72/DcrkxCuS7oLekMe87BkGhyJzkhQqUJh/CTmokr9x053GQ5FjhSKhg==
+   Whole-Content-Sha512: 
TEDXlQqWMSnJbL10JtFdbw0nqciNpjc4bd6m7iAB8aymakWeF+ghs1k5LayjdzHcjeDE8UNF/HXSxOFvoLFEuA==
X-Server-Name: traffic_ops_golang/
-   Date: Mon, 10 Dec 2018 15:34:05 GMT
-   Transfer-Encoding: chunked
+   Date: Wed, 04 Sep 2019 17:15:36 GMT
+   Content-Length: 120
 
{ "response": [
{
"id": 4,
-   "name": "disallowed",
-   "description": "Block all access",
-   "privLevel": 0,
-   "capabilities": []
+   "name": "admin",
+   "description": "super-user",
+   "privLevel": 30,
+   "capabilities": [
+   "all-write",
+   "all-read"
+   ]
}
]}
 
-.. note:: The response example for this method of this endpoint has been 
truncated to only the last element of the resultant array, as the full response 
was hundreds of lines long.
+``POST``
+
+.. versionadded:: 1.3
+
+Creates a new :term:`Role`.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type: Object
+
+Request Structure
+-
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+:description:  A helpful description of the :term:`Role`'s purpose.
+:name: The name of the new :term:`Role`
+:privLevel:The privilege level of the new :term:`Role`\ [#privlevel]_
+
+.. code-block:: http
+   :caption: Request Example
+
+   POST /api/1.3/roles HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 56
+   Content-Type: application/json
+
+   {
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30
+   }
+
+
+Response Structure
+--
+:capabilities: An array of the names of the Capabilities given to this 
:term:`Role`
+
+   .. tip:: This can be ``null`` *or* empty, depending on whether it was 
present in the request body, or merely empty. Obviously, it can also be a 
populated array.
+
+:description: A description of the :term:`Role`
+:id:  The integral, unique identifier for this :term:`Role`
+:name:The name of the :term:`Role`
+:privLevel:   An integer that allows for comparison between :term:`Roles`
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
gzfc7m/in5vVsVP+Y9h6JJfDhgpXKn9VAzoiPENhKbQfP8Q6jug08Rt2AK/3Nz1cx5zZ8P9IjVxDdIg7mlC8bw==
+   X-Server-Name: traffic_ops_golang/
+   Date: Wed, 04 Sep 2019 17:44:42 GMT
+   Content-Length: 150
+
+   { "alerts": [{
+   "text": "role was created.",
+   "level": "success"
+   }],
+   "response": {
+   "id": 5,
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30,
+   "capabilities": null
+   }}
+
+``PUT``
+===
+.. versionadded:: 1.3
+
+Replaces an existing :term:`Role` with one provided by the request.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type:
+
+Request Structure
+-
+.. table:: Request Query Parameters
+
+   
+--+--++
+   | Name | Required | Description 
   |
+   
+==+==++
+   | id   | yes  | The integral, unique identifier of the :term:`Role` 
to be replaced |
+   
+--+--++
+
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+
+   .. warning:: When not present, the affected :term:`Role`'s Capabilities 
will be

[GitHub] [trafficcontrol] asf-ci commented on issue #3973: Fixed Regional Geo Blocking on Steering Delivery Services

2019-10-09 Thread GitBox

asf-ci commented on issue #3973: Fixed Regional Geo Blocking on Steering 
Delivery Services
URL: https://github.com/apache/trafficcontrol/pull/3973#issuecomment-540176605
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4436/
   


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333210764
 
 

 ##
 File path: traffic_ops/testing/api/v14/capabilities_test.go
 ##
 @@ -0,0 +1,159 @@
+package v14
+
+/*
+ * 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 "testing"
+
+import "github.com/apache/trafficcontrol/lib/go-log"
+import "github.com/apache/trafficcontrol/lib/go-tc"
+
+// These capabilities are defined during the setup process in todb.go.
+// ANY TIME THOSE ARE CHANGED THIS MUST BE UPDATED.
+var staticCapabilities = []tc.Capability {
+   tc.Capability{
+   Name: "all-read",
+   Description: "Full read access",
+   },
+   tc.Capability {
+   Name: "all-write",
+   Description: "Full write access",
+   },
+   tc.Capability {
+   Name: "cdn-read",
+   Description: "View CDN configuration",
+   },
+}
+
+func TestCapabilities(t *testing.T) {
+   WithObjs(t, []TCObj{Capabilities}, func() {
+   ReplaceTestCapability(t)
+   GetTestCapabilities(t)
+   })
+}
+
+func CreateTestCapabilities(t *testing.T) {
+   for _,c := range testData.Capabilities {
+   resp, _, err := TOSession.CreateCapability(c)
+   log.Debugln("Response: ", c.Name, " ", resp)
+   if err != nil {
+   t.Errorf("could not create capability: %v", err)
+   }
+   }
+}
+
+func GetTestCapabilities(t *testing.T) {
+   testDataLen := len(testData.Capabilities) + len(staticCapabilities)
+   capMap := make(map[string]string, testDataLen)
+
+   for _,c := range testData.Capabilities {
+   capMap[c.Name] = c.Description
+   cap, _, err := TOSession.GetCapability(c.Name)
+   if err != nil {
+   t.Errorf("could not get capability '%s': %v", c.Name, 
err)
+   continue
+   }
+
+   if cap.Name != c.Name {
+   t.Errorf("requested capacity '%s' but got a capacity 
with the name '%s'", c.Name, cap.Name)
+   }
+   if cap.Description != c.Description {
+   t.Errorf("capacity '%s' has the wrong description, want 
'%s' but got '%s'", c.Name, c.Description, cap.Description)
+   }
+   }
+
+   // Hopefully this won't need to be done for much longer
+   for _,c := range staticCapabilities {
+   capMap[c.Name] = c.Description
+   }
+
+
+   caps, _, err := TOSession.GetCapabilities()
+   if err != nil {
+   t.Fatalf("could not get all capabilities: %v", err)
+   }
+   log.Debugln("capacities:", caps)
 
 Review comment:
   need this?


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333213972
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/capabilities/capabilities.go
 ##
 @@ -0,0 +1,305 @@
+package capabilities
+
+/*
+ * 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"
+import "encoding/json"
+import "errors"
+import "fmt"
+import "net/http"
+
+import "github.com/apache/trafficcontrol/lib/go-tc"
+import "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+
+const readQuery = `
+SELECT description,
+   last_updated,
+   name
+FROM capability
+`
+
+const createQuery = `
+INSERT INTO capability (name, description)
+VALUES ($1, $2)
+RETURNING description, last_updated, name
+`
+
+const replaceQuery = `
+UPDATE capability
+SET name=$1, description=$2
+WHERE name=$3
+RETURNING description, last_updated, name
+`
+
+const deleteQuery = `
+DELETE FROM capability
+WHERE name=$1
+RETURNING description, last_updated, name
+`
+
+func Read(w http.ResponseWriter, r *http.Request) {
+   inf, sysErr, userErr, 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()
+
+
+   var rows *sql.Rows
+   var err error
+   if name, ok := inf.Params["name"]; ok {
+   rows, err = tx.Query(readQuery + "WHERE name=$1", name)
 
 Review comment:
   could we instead use 
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go#L45?


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333210971
 
 

 ##
 File path: traffic_ops/testing/api/v14/capabilities_test.go
 ##
 @@ -0,0 +1,159 @@
+package v14
+
+/*
+ * 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 "testing"
+
+import "github.com/apache/trafficcontrol/lib/go-log"
+import "github.com/apache/trafficcontrol/lib/go-tc"
+
+// These capabilities are defined during the setup process in todb.go.
+// ANY TIME THOSE ARE CHANGED THIS MUST BE UPDATED.
+var staticCapabilities = []tc.Capability {
+   tc.Capability{
+   Name: "all-read",
+   Description: "Full read access",
+   },
+   tc.Capability {
+   Name: "all-write",
+   Description: "Full write access",
+   },
+   tc.Capability {
+   Name: "cdn-read",
+   Description: "View CDN configuration",
+   },
+}
+
+func TestCapabilities(t *testing.T) {
+   WithObjs(t, []TCObj{Capabilities}, func() {
+   ReplaceTestCapability(t)
+   GetTestCapabilities(t)
+   })
+}
+
+func CreateTestCapabilities(t *testing.T) {
+   for _,c := range testData.Capabilities {
+   resp, _, err := TOSession.CreateCapability(c)
+   log.Debugln("Response: ", c.Name, " ", resp)
+   if err != nil {
+   t.Errorf("could not create capability: %v", err)
+   }
+   }
+}
+
+func GetTestCapabilities(t *testing.T) {
+   testDataLen := len(testData.Capabilities) + len(staticCapabilities)
+   capMap := make(map[string]string, testDataLen)
+
+   for _,c := range testData.Capabilities {
+   capMap[c.Name] = c.Description
+   cap, _, err := TOSession.GetCapability(c.Name)
+   if err != nil {
+   t.Errorf("could not get capability '%s': %v", c.Name, 
err)
+   continue
+   }
+
+   if cap.Name != c.Name {
+   t.Errorf("requested capacity '%s' but got a capacity 
with the name '%s'", c.Name, cap.Name)
+   }
+   if cap.Description != c.Description {
+   t.Errorf("capacity '%s' has the wrong description, want 
'%s' but got '%s'", c.Name, c.Description, cap.Description)
+   }
+   }
+
+   // Hopefully this won't need to be done for much longer
+   for _,c := range staticCapabilities {
+   capMap[c.Name] = c.Description
+   }
+
+
+   caps, _, err := TOSession.GetCapabilities()
+   if err != nil {
+   t.Fatalf("could not get all capabilities: %v", err)
+   }
+   log.Debugln("capacities:", caps)
+   if len(caps) != testDataLen {
+   t.Errorf("response returned different number of capabilities 
than those that exist; got %d, want %d", len(caps), testDataLen)
+   return // we can't FATAL because this testing.T object is shared
 
 Review comment:
   huh we have been doing it a lot of other places


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333216351
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/capabilities/capabilities.go
 ##
 @@ -0,0 +1,305 @@
+package capabilities
+
+/*
+ * 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"
+import "encoding/json"
+import "errors"
+import "fmt"
+import "net/http"
+
+import "github.com/apache/trafficcontrol/lib/go-tc"
+import "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+
+const readQuery = `
+SELECT description,
+   last_updated,
+   name
+FROM capability
+`
+
+const createQuery = `
+INSERT INTO capability (name, description)
+VALUES ($1, $2)
+RETURNING description, last_updated, name
+`
+
+const replaceQuery = `
+UPDATE capability
+SET name=$1, description=$2
+WHERE name=$3
+RETURNING description, last_updated, name
+`
+
+const deleteQuery = `
+DELETE FROM capability
+WHERE name=$1
+RETURNING description, last_updated, name
+`
+
+func Read(w http.ResponseWriter, r *http.Request) {
+   inf, sysErr, userErr, 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()
+
+
+   var rows *sql.Rows
+   var err error
+   if name, ok := inf.Params["name"]; ok {
+   rows, err = tx.Query(readQuery + "WHERE name=$1", name)
+   } else {
+   rows, err = tx.Query(readQuery)
+   }
+   if err != nil && err != sql.ErrNoRows {
+   errCode = http.StatusInternalServerError
+   sysErr = fmt.Errorf("querying capabilities: %v", err)
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   defer rows.Close()
+
+   caps := []tc.Capability{}
+   for rows.Next() {
+   cap := tc.Capability{}
+   if err := rows.Scan(, , 
); err != nil {
+   errCode = http.StatusInternalServerError
+   sysErr = fmt.Errorf("Parsing database response: %v", 
err)
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   caps = append(caps, cap)
+   }
+
+   respBts, err := json.Marshal(struct{R []tc.Capability 
`json:"response"`}{caps})
+   if err != nil {
+   errCode = http.StatusInternalServerError
+   sysErr = fmt.Errorf("Marshaling response: %v", err)
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   w.Header().Set(tc.ContentType, tc.ApplicationJson)
+   w.Write(append(respBts, '\n'))
+}
+
+func Create(w http.ResponseWriter, r *http.Request) {
+   inf, sysErr, userErr, 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()
+
+   decoder := json.NewDecoder(r.Body)
+   var cap tc.Capability
+   if err := decoder.Decode(); err != nil {
+   sysErr = fmt.Errorf("Decoding request body: %v")
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   if cap.Name == "" {
+   userErr = errors.New("'name' must be defined! (and not empty)")
+   errCode = http.StatusBadRequest
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   if cap.Description == "" {
+   userErr = errors.New("'description' must be defined! (and not 
empty)")
+   errCode = http.StatusBadRequest
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   if ok, err := capabilityNameExists(cap.Name, tx); err != nil {
+   sysErr = fmt.Errorf("Checking for capability %s's existence: 
%v", cap.Name, err)
+   errCode =

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333211816
 
 

 ##
 File path: traffic_ops/testing/api/v14/capabilities_test.go
 ##
 @@ -0,0 +1,159 @@
+package v14
+
+/*
+ * 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 "testing"
+
+import "github.com/apache/trafficcontrol/lib/go-log"
+import "github.com/apache/trafficcontrol/lib/go-tc"
+
+// These capabilities are defined during the setup process in todb.go.
+// ANY TIME THOSE ARE CHANGED THIS MUST BE UPDATED.
+var staticCapabilities = []tc.Capability {
+   tc.Capability{
+   Name: "all-read",
+   Description: "Full read access",
+   },
+   tc.Capability {
+   Name: "all-write",
+   Description: "Full write access",
+   },
+   tc.Capability {
+   Name: "cdn-read",
+   Description: "View CDN configuration",
+   },
+}
+
+func TestCapabilities(t *testing.T) {
+   WithObjs(t, []TCObj{Capabilities}, func() {
+   ReplaceTestCapability(t)
+   GetTestCapabilities(t)
+   })
+}
+
+func CreateTestCapabilities(t *testing.T) {
+   for _,c := range testData.Capabilities {
+   resp, _, err := TOSession.CreateCapability(c)
+   log.Debugln("Response: ", c.Name, " ", resp)
+   if err != nil {
+   t.Errorf("could not create capability: %v", err)
+   }
+   }
+}
+
+func GetTestCapabilities(t *testing.T) {
+   testDataLen := len(testData.Capabilities) + len(staticCapabilities)
+   capMap := make(map[string]string, testDataLen)
+
+   for _,c := range testData.Capabilities {
+   capMap[c.Name] = c.Description
+   cap, _, err := TOSession.GetCapability(c.Name)
+   if err != nil {
+   t.Errorf("could not get capability '%s': %v", c.Name, 
err)
+   continue
+   }
+
+   if cap.Name != c.Name {
+   t.Errorf("requested capacity '%s' but got a capacity 
with the name '%s'", c.Name, cap.Name)
+   }
+   if cap.Description != c.Description {
+   t.Errorf("capacity '%s' has the wrong description, want 
'%s' but got '%s'", c.Name, c.Description, cap.Description)
+   }
+   }
+
+   // Hopefully this won't need to be done for much longer
+   for _,c := range staticCapabilities {
+   capMap[c.Name] = c.Description
+   }
+
+
+   caps, _, err := TOSession.GetCapabilities()
+   if err != nil {
+   t.Fatalf("could not get all capabilities: %v", err)
+   }
+   log.Debugln("capacities:", caps)
+   if len(caps) != testDataLen {
+   t.Errorf("response returned different number of capabilities 
than those that exist; got %d, want %d", len(caps), testDataLen)
+   return // we can't FATAL because this testing.T object is shared
+   }
+
+   for _,c := range caps {
+   if desc, ok := capMap[c.Name]; !ok {
+   t.Errorf("capability '%s' found in response, but not in 
test data!", c.Name)
+   } else {
+   if desc != c.Description {
+   t.Errorf("capability '%s' has description '%s' 
in response, but had '%s' in the test data", c.Name, c.Description, desc)
+   }
+   delete(capMap, c.Name)
+   }
+   }
+
+   for c,_ := range capMap {
+   t.Errorf("Capability '%s' existed in the test data but didn't 
appear in the response!", c)
+   }
+}
+
+func ReplaceTestCapability(t *testing.T) {
+   if len(testData.Capabilities) < 1 {
+   t.Skip("There aren't any test capabilities, skipping (this 
should probably be rectified!)")
 
 Review comment:
   I wonder if we should fail the test is this case? if this happens something 
is causing it and we want to address it and I worry a skip message will just 
get lost in the big output

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333209076
 
 

 ##
 File path: traffic_ops/client/capability.go
 ##
 @@ -0,0 +1,121 @@
+package client
+
+/*
+ * 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 "encoding/json"
+import "fmt"
+import "net"
+import "net/http"
+
+import "github.com/apache/trafficcontrol/lib/go-tc"
+
+const API_v4_CAPABILITIES = "/api/1.4/capabilities"
+
+// CreateCapability creates the passed capability.
+func (to *Session) CreateCapability(c tc.Capability) (tc.Alerts, ReqInf, 
error) {
+   var remoteAddr net.Addr
+   reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: 
remoteAddr}
+
+   reqBody, err := json.Marshal(c)
+   if err != nil {
+   return tc.Alerts{}, reqInf, err
+   }
+
+   resp, remoteAddr, err := to.request(http.MethodPost, 
API_v4_CAPABILITIES, reqBody)
+   if err != nil {
+   return tc.Alerts{}, reqInf, err
+   }
+   defer resp.Body.Close()
+   reqInf.RemoteAddr = remoteAddr
+
+   var alerts tc.Alerts
+   err = json.NewDecoder(resp.Body).Decode()
+   return alerts, reqInf, err
+}
+
+// ReplaceCapabilityByName replaces the capability named 'name' with the one 
passed as 'c'.
+func (to *Session) ReplaceCapabilityByName(name string, c tc.Capability) 
(tc.Alerts, ReqInf, error) {
+   var remoteAddr net.Addr
+   reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: 
remoteAddr}
+
+   reqBody, err := json.Marshal(c)
+   if err != nil {
+   return tc.Alerts{}, reqInf, err
+   }
+
+   endpoint := fmt.Sprintf("%s?name=%s", API_v4_CAPABILITIES, name)
 
 Review comment:
   I know we do not do this across any of our client code so can ignore this 
commentbut we really should leverage the net/url lib to build up our query 
parameters so they are url encoded. 


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333208376
 
 

 ##
 File path: traffic_ops/client/capability.go
 ##
 @@ -0,0 +1,121 @@
+package client
+
+/*
+ * 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 "encoding/json"
+import "fmt"
+import "net"
+import "net/http"
+
+import "github.com/apache/trafficcontrol/lib/go-tc"
+
+const API_v4_CAPABILITIES = "/api/1.4/capabilities"
 
 Review comment:
   should this be v14?


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333215404
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/capabilities/capabilities.go
 ##
 @@ -0,0 +1,305 @@
+package capabilities
+
+/*
+ * 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"
+import "encoding/json"
+import "errors"
+import "fmt"
+import "net/http"
+
+import "github.com/apache/trafficcontrol/lib/go-tc"
+import "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+
+const readQuery = `
+SELECT description,
+   last_updated,
+   name
+FROM capability
+`
+
+const createQuery = `
+INSERT INTO capability (name, description)
+VALUES ($1, $2)
+RETURNING description, last_updated, name
+`
+
+const replaceQuery = `
+UPDATE capability
+SET name=$1, description=$2
+WHERE name=$3
+RETURNING description, last_updated, name
+`
+
+const deleteQuery = `
+DELETE FROM capability
+WHERE name=$1
+RETURNING description, last_updated, name
+`
+
+func Read(w http.ResponseWriter, r *http.Request) {
+   inf, sysErr, userErr, 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()
+
+
+   var rows *sql.Rows
+   var err error
+   if name, ok := inf.Params["name"]; ok {
+   rows, err = tx.Query(readQuery + "WHERE name=$1", name)
+   } else {
+   rows, err = tx.Query(readQuery)
+   }
+   if err != nil && err != sql.ErrNoRows {
+   errCode = http.StatusInternalServerError
+   sysErr = fmt.Errorf("querying capabilities: %v", err)
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   defer rows.Close()
+
+   caps := []tc.Capability{}
+   for rows.Next() {
+   cap := tc.Capability{}
+   if err := rows.Scan(, , 
); err != nil {
+   errCode = http.StatusInternalServerError
+   sysErr = fmt.Errorf("Parsing database response: %v", 
err)
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   caps = append(caps, cap)
+   }
+
+   respBts, err := json.Marshal(struct{R []tc.Capability 
`json:"response"`}{caps})
+   if err != nil {
+   errCode = http.StatusInternalServerError
+   sysErr = fmt.Errorf("Marshaling response: %v", err)
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   w.Header().Set(tc.ContentType, tc.ApplicationJson)
+   w.Write(append(respBts, '\n'))
+}
+
+func Create(w http.ResponseWriter, r *http.Request) {
+   inf, sysErr, userErr, 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()
+
+   decoder := json.NewDecoder(r.Body)
+   var cap tc.Capability
+   if err := decoder.Decode(); err != nil {
+   sysErr = fmt.Errorf("Decoding request body: %v")
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   if cap.Name == "" {
+   userErr = errors.New("'name' must be defined! (and not empty)")
+   errCode = http.StatusBadRequest
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   if cap.Description == "" {
+   userErr = errors.New("'description' must be defined! (and not 
empty)")
+   errCode = http.StatusBadRequest
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   if ok, err := capabilityNameExists(cap.Name, tx); err != nil {
+   sysErr = fmt.Errorf("Checking for capability %s's existence: 
%v", cap.Name, err)
+   errCode =

[GitHub] [trafficcontrol] mhoppa commented on issue #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on issue #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#issuecomment-540179090
 
 
   Also some merge conflicts to fix 


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333214722
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/capabilities/capabilities.go
 ##
 @@ -0,0 +1,305 @@
+package capabilities
+
+/*
+ * 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"
+import "encoding/json"
+import "errors"
+import "fmt"
+import "net/http"
+
+import "github.com/apache/trafficcontrol/lib/go-tc"
+import "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+
+const readQuery = `
+SELECT description,
+   last_updated,
+   name
+FROM capability
+`
+
+const createQuery = `
+INSERT INTO capability (name, description)
+VALUES ($1, $2)
+RETURNING description, last_updated, name
+`
+
+const replaceQuery = `
+UPDATE capability
+SET name=$1, description=$2
+WHERE name=$3
+RETURNING description, last_updated, name
+`
+
+const deleteQuery = `
+DELETE FROM capability
+WHERE name=$1
+RETURNING description, last_updated, name
+`
+
+func Read(w http.ResponseWriter, r *http.Request) {
+   inf, sysErr, userErr, 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()
+
+
+   var rows *sql.Rows
+   var err error
+   if name, ok := inf.Params["name"]; ok {
+   rows, err = tx.Query(readQuery + "WHERE name=$1", name)
+   } else {
+   rows, err = tx.Query(readQuery)
+   }
+   if err != nil && err != sql.ErrNoRows {
+   errCode = http.StatusInternalServerError
+   sysErr = fmt.Errorf("querying capabilities: %v", err)
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   defer rows.Close()
+
+   caps := []tc.Capability{}
+   for rows.Next() {
+   cap := tc.Capability{}
+   if err := rows.Scan(, , 
); err != nil {
+   errCode = http.StatusInternalServerError
+   sysErr = fmt.Errorf("Parsing database response: %v", 
err)
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   caps = append(caps, cap)
+   }
+
+   respBts, err := json.Marshal(struct{R []tc.Capability 
`json:"response"`}{caps})
 
 Review comment:
   isnt there an API util function that wraps an interface in a response that 
you can call to instead of this?


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3870: Rewrite /capabilities to Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333202014
 
 

 ##
 File path: lib/go-tc/capabilities.go
 ##
 @@ -0,0 +1,38 @@
+package tc
+
+/*
+ * 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.
+ */
+
+// Capability reflects the ability of a user in ATC to perform some operation.
+//
+// In practice, they are assigned to relevant Traffic Ops API endpoints - to 
describe the
+// capabilites of said endpoint - and to user permission Roles - to describe 
the capabilities
+// afforded by said Role. Note that enforcement of Capability-based permisions 
is not currently
+// implemented.
+type Capability struct {
+   Description string `json:"description"`
 
 Review comment:
   have db tag on these? 


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] dneuman64 opened a new pull request #3973: Fixed Regional Geo Blocking on Steering Delivery Services

2019-10-09 Thread GitBox

dneuman64 opened a new pull request #3973: Fixed Regional Geo Blocking on 
Steering Delivery Services
URL: https://github.com/apache/trafficcontrol/pull/3973
 
 
   
   ## What does this PR (Pull Request) do?
   
   
   - [ x] This PR fixes #3910 
   
   
   ## Which Traffic Control components are affected by this PR?
   
   
   - Traffic Router
   - Documentation
   
   ## What is the best way to verify this PR?
   
   - Deploy this release
   - Setup a steering DS with RGB enabled
   - Ensure that both blocked and not blocked use cases work.
   
   ## 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 OR I have explained why tests are unnecessary
   - [x] This PR includes documentation OR 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)
   
   
   


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 edited a comment on issue #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

ocket edited a comment on issue #3954: Added deprecation notices for 
/user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#issuecomment-540151369
 
 
   Well, actually, it is. The success message is in a sub-list for some reason.
   
   I'll try to figure out why it's doing 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 issue #3954: Added deprecation notices for /user/current/update

2019-10-09 Thread GitBox

ocket commented on issue #3954: Added deprecation notices for 
/user/current/update
URL: https://github.com/apache/trafficcontrol/pull/3954#issuecomment-540151369
 
 
   Well, technically, it is
   
   I'll try to figure out why it's doing 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] rawlinp commented on issue #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

rawlinp commented on issue #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#issuecomment-540140162
 
 
   ID and hostname seem fine. That is at least enough information to link to 
the server page if someone wanted to see a particular server's details, right?
   
   


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 #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333195222
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/role/roles.go
 ##
 @@ -177,12 +179,16 @@ func (role *TORole) Update() (error, error, int) {
if userErr != nil || sysErr != nil {
return userErr, sysErr, errCode
}
+
// TODO cascade delete, to automatically do this in SQL?
-   userErr, sysErr, errCode = 
role.deleteRoleCapabilityAssociations(role.ReqInfo.Tx)
-   if userErr != nil || sysErr != nil {
-   return userErr, sysErr, errCode
+   if role.Capabilities != nil && *role.Capabilities != nil {
 
 Review comment:
   do you need to check for len of *role.Capabilities?


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 #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333194946
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/role/roles.go
 ##
 @@ -122,9 +122,11 @@ func (role *TORole) Create() (error, error, int) {
}
 
//after we have role ID we can associate the capabilities:
-   userErr, sysErr, errCode = 
role.createRoleCapabilityAssociations(role.ReqInfo.Tx)
-   if userErr != nil || sysErr != nil {
-   return userErr, sysErr, errCode
+   if role.Capabilities != nil && *role.Capabilities != nil && 
len(*role.Capabilities) > 0 {
 
 Review comment:
   Nit but this can be simplified to `role.Capabilities != nil && 
len(*role.Capabilities) > 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] mhoppa commented on a change in pull request #3901: Fixed runtime panic when creating roles, added docs

2019-10-09 Thread GitBox

mhoppa commented on a change in pull request #3901: Fixed runtime panic when 
creating roles, added docs
URL: https://github.com/apache/trafficcontrol/pull/3901#discussion_r333195701
 
 

 ##
 File path: docs/source/api/roles.rst
 ##
 @@ -65,19 +84,236 @@ Response Structure
Access-Control-Allow-Origin: *
Content-Type: application/json
Set-Cookie: mojolicious=...; Path=/; HttpOnly
-   Whole-Content-Sha512: 
caagePqpL6u9Mn1UBIDCJSgiLAKOHm72/DcrkxCuS7oLekMe87BkGhyJzkhQqUJh/CTmokr9x053GQ5FjhSKhg==
+   Whole-Content-Sha512: 
TEDXlQqWMSnJbL10JtFdbw0nqciNpjc4bd6m7iAB8aymakWeF+ghs1k5LayjdzHcjeDE8UNF/HXSxOFvoLFEuA==
X-Server-Name: traffic_ops_golang/
-   Date: Mon, 10 Dec 2018 15:34:05 GMT
-   Transfer-Encoding: chunked
+   Date: Wed, 04 Sep 2019 17:15:36 GMT
+   Content-Length: 120
 
{ "response": [
{
"id": 4,
-   "name": "disallowed",
-   "description": "Block all access",
-   "privLevel": 0,
-   "capabilities": []
+   "name": "admin",
+   "description": "super-user",
+   "privLevel": 30,
+   "capabilities": [
+   "all-write",
+   "all-read"
+   ]
}
]}
 
-.. note:: The response example for this method of this endpoint has been 
truncated to only the last element of the resultant array, as the full response 
was hundreds of lines long.
+``POST``
+
+.. versionadded:: 1.3
+
+Creates a new :term:`Role`.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type: Object
+
+Request Structure
+-
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+:description:  A helpful description of the :term:`Role`'s purpose.
+:name: The name of the new :term:`Role`
+:privLevel:The privilege level of the new :term:`Role`\ [#privlevel]_
+
+.. code-block:: http
+   :caption: Request Example
+
+   POST /api/1.3/roles HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 56
+   Content-Type: application/json
+
+   {
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30
+   }
+
+
+Response Structure
+--
+:capabilities: An array of the names of the Capabilities given to this 
:term:`Role`
+
+   .. tip:: This can be ``null`` *or* empty, depending on whether it was 
present in the request body, or merely empty. Obviously, it can also be a 
populated array.
+
+:description: A description of the :term:`Role`
+:id:  The integral, unique identifier for this :term:`Role`
+:name:The name of the :term:`Role`
+:privLevel:   An integer that allows for comparison between :term:`Roles`
+
+.. 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, Set-Cookie, Cookie
+   Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+   Access-Control-Allow-Origin: *
+   Content-Type: application/json
+   Set-Cookie: mojolicious=...; Path=/; HttpOnly
+   Whole-Content-Sha512: 
gzfc7m/in5vVsVP+Y9h6JJfDhgpXKn9VAzoiPENhKbQfP8Q6jug08Rt2AK/3Nz1cx5zZ8P9IjVxDdIg7mlC8bw==
+   X-Server-Name: traffic_ops_golang/
+   Date: Wed, 04 Sep 2019 17:44:42 GMT
+   Content-Length: 150
+
+   { "alerts": [{
+   "text": "role was created.",
+   "level": "success"
+   }],
+   "response": {
+   "id": 5,
+   "name": "test",
+   "description": "quest",
+   "privLevel": 30,
+   "capabilities": null
+   }}
+
+``PUT``
+===
+.. versionadded:: 1.3
+
+Replaces an existing :term:`Role` with one provided by the request.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type:
+
+Request Structure
+-
+.. table:: Request Query Parameters
+
+   
+--+--++
+   | Name | Required | Description 
   |
+   
+==+==++
+   | id   | yes  | The integral, unique identifier of the :term:`Role` 
to be replaced |
+   
+--+--++
+
+:capabilities: An optional array of capability names that will be granted to 
the new :term:`Role`
+
+   .. warning:: When not present, the affected :term:`Role`'s Capabilities 
will be

[GitHub] [trafficcontrol] jhg03a opened a new issue #3974: Remove weight parameter from CRConfig.json

2019-10-09 Thread GitBox

jhg03a opened a new issue #3974: Remove weight parameter from CRConfig.json
URL: https://github.com/apache/trafficcontrol/issues/3974
 
 
   
   
   
   
   ## 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
   - [ ] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
   Currently the weight parameter with a configfile of CRConfig.json is 
required of each ATS server profile.  Because this is associated with the 
CRConfig.json even though it's an ATS thing, it's added to the general section 
of the CRConfig.  This causes an unexpected, yet harmless change, in the 
TrafficPortal Diff when updating this parameter on a subset of caches.  At the 
end of the day, the other change to the hashCount of a particular server is 
what matters.
   
   ## Expected / new behavior:
   
   I would like to see the weight parameter excluded from the general section 
of the CRConfig.
   
   ## Minimal reproduction of the problem with instructions:
   
   Create 2 ATS profiles with different weights associated with the 
CRConfig.json config file.  Observe that only the "last" value is present in 
the CRConfig diff in addition to the hashCount.
   
   ## 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] ocket8888 commented on a change in pull request #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3870: Rewrite /capabilities to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333234730
 
 

 ##
 File path: traffic_ops/client/capability.go
 ##
 @@ -0,0 +1,121 @@
+package client
+
+/*
+ * 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 "encoding/json"
+import "fmt"
+import "net"
+import "net/http"
+
+import "github.com/apache/trafficcontrol/lib/go-tc"
+
+const API_v4_CAPABILITIES = "/api/1.4/capabilities"
+
+// CreateCapability creates the passed capability.
+func (to *Session) CreateCapability(c tc.Capability) (tc.Alerts, ReqInf, 
error) {
+   var remoteAddr net.Addr
+   reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: 
remoteAddr}
+
+   reqBody, err := json.Marshal(c)
+   if err != nil {
+   return tc.Alerts{}, reqInf, err
+   }
+
+   resp, remoteAddr, err := to.request(http.MethodPost, 
API_v4_CAPABILITIES, reqBody)
+   if err != nil {
+   return tc.Alerts{}, reqInf, err
+   }
+   defer resp.Body.Close()
+   reqInf.RemoteAddr = remoteAddr
+
+   var alerts tc.Alerts
+   err = json.NewDecoder(resp.Body).Decode()
+   return alerts, reqInf, err
+}
+
+// ReplaceCapabilityByName replaces the capability named 'name' with the one 
passed as 'c'.
+func (to *Session) ReplaceCapabilityByName(name string, c tc.Capability) 
(tc.Alerts, ReqInf, error) {
+   var remoteAddr net.Addr
+   reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: 
remoteAddr}
+
+   reqBody, err := json.Marshal(c)
+   if err != nil {
+   return tc.Alerts{}, reqInf, err
+   }
+
+   endpoint := fmt.Sprintf("%s?name=%s", API_v4_CAPABILITIES, name)
 
 Review comment:
   yes, we really should


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3870: Rewrite /capabilities to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333235112
 
 

 ##
 File path: traffic_ops/client/capability.go
 ##
 @@ -0,0 +1,121 @@
+package client
+
+/*
+ * 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 "encoding/json"
+import "fmt"
+import "net"
+import "net/http"
+
+import "github.com/apache/trafficcontrol/lib/go-tc"
+
+const API_v4_CAPABILITIES = "/api/1.4/capabilities"
 
 Review comment:
   well it oughta be v1.4  
   


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 #3870: Rewrite /capabilities to Go

2019-10-09 Thread GitBox

ocket commented on a change in pull request #3870: Rewrite /capabilities to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3870#discussion_r333235306
 
 

 ##
 File path: traffic_ops/testing/api/v14/capabilities_test.go
 ##
 @@ -0,0 +1,159 @@
+package v14
+
+/*
+ * 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 "testing"
+
+import "github.com/apache/trafficcontrol/lib/go-log"
+import "github.com/apache/trafficcontrol/lib/go-tc"
+
+// These capabilities are defined during the setup process in todb.go.
+// ANY TIME THOSE ARE CHANGED THIS MUST BE UPDATED.
+var staticCapabilities = []tc.Capability {
+   tc.Capability{
+   Name: "all-read",
+   Description: "Full read access",
+   },
+   tc.Capability {
+   Name: "all-write",
+   Description: "Full write access",
+   },
+   tc.Capability {
+   Name: "cdn-read",
+   Description: "View CDN configuration",
+   },
+}
+
+func TestCapabilities(t *testing.T) {
+   WithObjs(t, []TCObj{Capabilities}, func() {
+   ReplaceTestCapability(t)
+   GetTestCapabilities(t)
+   })
+}
+
+func CreateTestCapabilities(t *testing.T) {
+   for _,c := range testData.Capabilities {
+   resp, _, err := TOSession.CreateCapability(c)
+   log.Debugln("Response: ", c.Name, " ", resp)
+   if err != nil {
+   t.Errorf("could not create capability: %v", err)
+   }
+   }
+}
+
+func GetTestCapabilities(t *testing.T) {
+   testDataLen := len(testData.Capabilities) + len(staticCapabilities)
+   capMap := make(map[string]string, testDataLen)
+
+   for _,c := range testData.Capabilities {
+   capMap[c.Name] = c.Description
+   cap, _, err := TOSession.GetCapability(c.Name)
+   if err != nil {
+   t.Errorf("could not get capability '%s': %v", c.Name, 
err)
+   continue
+   }
+
+   if cap.Name != c.Name {
+   t.Errorf("requested capacity '%s' but got a capacity 
with the name '%s'", c.Name, cap.Name)
+   }
+   if cap.Description != c.Description {
+   t.Errorf("capacity '%s' has the wrong description, want 
'%s' but got '%s'", c.Name, c.Description, cap.Description)
+   }
+   }
+
+   // Hopefully this won't need to be done for much longer
+   for _,c := range staticCapabilities {
+   capMap[c.Name] = c.Description
+   }
+
+
+   caps, _, err := TOSession.GetCapabilities()
+   if err != nil {
+   t.Fatalf("could not get all capabilities: %v", err)
+   }
+   log.Debugln("capacities:", caps)
+   if len(caps) != testDataLen {
+   t.Errorf("response returned different number of capabilities 
than those that exist; got %d, want %d", len(caps), testDataLen)
+   return // we can't FATAL because this testing.T object is shared
 
 Review comment:
   yeah, that's wrong. Should be fatal


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] jheitz200 commented on a change in pull request #3964: Add GET/POST/DELETE for server_server_capabilities

2019-10-09 Thread GitBox

jheitz200 commented on a change in pull request #3964: Add GET/POST/DELETE for 
server_server_capabilities
URL: https://github.com/apache/trafficcontrol/pull/3964#discussion_r333238442
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/routing/routes.go
 ##
 @@ -266,6 +266,11 @@ func Routes(d ServerData) ([]Route, []RawRoute, 
http.Handler, error) {
{1.1, http.MethodPost, `servers/?$`, 
api.CreateHandler({}), auth.PrivLevelOperations, Authenticated, 
nil},
{1.1, http.MethodDelete, `servers/{id}$`, 
api.DeleteHandler({}), auth.PrivLevelOperations, Authenticated, 
nil},
 
+   //Server Server Capabilities: CRUD
 
 Review comment:
   consider naming the route `server_capabilities `


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

1 2 >

1 - 100 of 110 matches

Mail list logo