You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/03/18 16:12:58 UTC
[GitHub] [trafficcontrol] jehunte opened a new pull request #4518: Add
service category feature to TO and TP
jehunte opened a new pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518
<!--
************ STOP!! ************
If this Pull Request is intended to fix a security vulnerability, DO NOT submit it! Instead, contact
the Apache Software Foundation Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://www.apache.org/security/ regarding vulnerability disclosure.
-->
## What does this PR (Pull Request) do?
<!-- Explain the changes you made here. If this fixes an Issue, identify it by
replacing the text in the checkbox item with the Issue number e.g.
- [x] This PR enables the user to associate a delivery service with a specific service category. The user can create, add, update, delete categories as necessary and the value will be used in combination with the xmlid to set the header "X-CDN-SVC"
Be sure you check the box properly, see the "The following criteria are ALL
met by this PR" section for details.
-->
- [x] This PR is not related to any Issue <!-- You can check for an issue here: https://github.com/apache/trafficcontrol/issues -->
## Which Traffic Control components are affected by this PR?
- Documentation
- Traffic Control Client Go
- Traffic Ops
- Traffic Ops ORT - atstccfg
- Traffic Portal
## What is the best way to verify this PR?
1. Validate database changes
- New table service category with columns (id, name, last_updated)
- service_category column added to delivery service table
2. Verify in TP that you can:
- create a new delivery service with a service category
- Add, update, delete, get service categories
3. Run the ORT script and verify that the header is being set properly
X-CDN-SVC "{xmlid}|{service-category}"
## 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)
## Additional Information
These SQL queries will get any existing service categories from the edge header rewrite and populate the column/table
INSERT INTO service_category(name) SELECT DISTINCT substring(edge_header_rewrite FROM '(?<=\|)(.*?)(?=\")') FROM deliveryservice WHERE substring(edge_header_rewrite FROM '(?<=\|)(.*?)(?=\")') IS NOT NULL;
UPDATE deliveryservice SET service_category = service_category.id FROM service_category WHERE substring(edge_header_rewrite FROM '(?<=\|)(.*?)(?=\")') = service_category.name;
<img width="1675" alt="Screen Shot 2020-03-18 at 9 41 30 AM" src="https://user-images.githubusercontent.com/61031690/76981697-93465d00-6900-11ea-9d4b-68ba8809356a.png">
<img width="222" alt="Screen Shot 2020-03-18 at 9 41 41 AM" src="https://user-images.githubusercontent.com/61031690/76981730-9a6d6b00-6900-11ea-97e5-87d24227e7f2.png">
<img width="1667" alt="Screen Shot 2020-03-18 at 9 41 51 AM" src="https://user-images.githubusercontent.com/61031690/76981752-9f321f00-6900-11ea-815f-4ad1b0cc0260.png">
<img width="1674" alt="Screen Shot 2020-03-18 at 9 42 01 AM" src="https://user-images.githubusercontent.com/61031690/76981777-a48f6980-6900-11ea-8b82-d24403935dec.png">
<img width="1679" alt="Screen Shot 2020-03-18 at 9 42 12 AM" src="https://user-images.githubusercontent.com/61031690/76981787-a78a5a00-6900-11ea-855e-26948c2819a2.png">
<img width="1416" alt="Screen Shot 2020-03-18 at 9 42 58 AM" src="https://user-images.githubusercontent.com/61031690/76981803-ab1de100-6900-11ea-9e96-03a9a34ee744.png">
<!--
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.
-->
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395145038
##########
File path: traffic_ops/client/serviceCategory.go
##########
@@ -0,0 +1,132 @@
+/*
+
+ 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.
+*/
+
+package client
+
+import (
+ "encoding/json"
+ "fmt"
+ "net"
+ "net/http"
+
+ "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+const (
+ API_serviceCategories = apiBase + "/servicecategories"
+)
+
+// Create a service category
+func (to *Session) CreateServiceCategory(serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
Review comment:
nit but the go docs for the functions should start with the function name
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395155457
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -904,6 +987,7 @@ func readGetDeliveryServices(params map[string]string, tx *sqlx.Tx, user *auth.C
"logsEnabled": dbhelpers.WhereColumnInfo{"ds.logs_enabled", api.IsBool},
"tenant": dbhelpers.WhereColumnInfo{"ds.tenant_id", api.IsInt},
"signingAlgorithm": dbhelpers.WhereColumnInfo{"ds.signing_algorithm", nil},
+ "serviceCategory": dbhelpers.WhereColumnInfo{"ds.service_category", api.IsInt},
Review comment:
this was added as a query parameter 👍 the docs should be updated with it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395146461
##########
File path: traffic_ops/testing/api/v2/withobjs_test.go
##########
@@ -99,6 +100,7 @@ var withFuncs = map[TCObj]TCObjFuncs{
ServerChecks: {CreateTestServerChecks, DeleteTestServerChecks},
ServerServerCapabilities: {CreateTestServerServerCapabilities, DeleteTestServerServerCapabilities},
Servers: {CreateTestServers, DeleteTestServers},
+ ServiceCategories: {CreateTestServiceCategory, DeleteTestServiceCategories},
Review comment:
is there tests cases related to these? looks like it was added to the json fixtures but nothing 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 edited a comment on issue #4518: Add
service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 edited a comment on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600757547
> Do we expect Service Categories to be created by tenants? Or is it something only Operators will create?
everybody is scoped to a tenant. operators and non operators. all users are tenant scoped. for example, if i am an operator in the 2.1 tenant, i should probably only be able to create service categories that apply to the 2.1 delivery services.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395142106
##########
File path: traffic_ops/client/serviceCategory.go
##########
@@ -0,0 +1,132 @@
+/*
+
+ 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.
+*/
+
+package client
+
+import (
+ "encoding/json"
+ "fmt"
+ "net"
+ "net/http"
+
+ "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+const (
+ API_serviceCategories = apiBase + "/servicecategories"
+)
+
+// Create a service category
+func (to *Session) CreateServiceCategory(serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
+
+ var remoteAddr net.Addr
+ reqBody, err := json.Marshal(serviceCategory)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ resp, remoteAddr, err := to.request(http.MethodPost, API_serviceCategories, reqBody)
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ defer resp.Body.Close()
+ var alerts tc.Alerts
+ err = json.NewDecoder(resp.Body).Decode(&alerts)
+ return alerts, reqInf, nil
+}
+
+// Update a service category by ID
+func (to *Session) UpdateServiceCategoryByID(id int, serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
+
+ var remoteAddr net.Addr
+ reqBody, err := json.Marshal(serviceCategory)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ route := fmt.Sprintf("%s/%d", API_serviceCategories, id)
+ resp, remoteAddr, err := to.request(http.MethodPut, route, reqBody)
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ defer resp.Body.Close()
+ var alerts tc.Alerts
+ err = json.NewDecoder(resp.Body).Decode(&alerts)
+ return alerts, reqInf, nil
+}
+
+// Returns a list of service categories
+func (to *Session) GetServiceCategories() ([]tc.ServiceCategory, ReqInf, error) {
+ resp, remoteAddr, err := to.request(http.MethodGet, API_serviceCategories, nil)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return nil, reqInf, err
+ }
+ defer resp.Body.Close()
+
+ var data tc.ServiceCategoriesResponse
+ err = json.NewDecoder(resp.Body).Decode(&data)
+ return data.Response, reqInf, nil
+}
+
+// GET a service category by the service category id
+func (to *Session) GetServiceCategoryByID(id int) ([]tc.ServiceCategory, ReqInf, error) {
+ route := fmt.Sprintf("%s?id=%d", API_serviceCategories, id)
+ resp, remoteAddr, err := to.request(http.MethodGet, route, nil)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return nil, reqInf, err
+ }
+ defer resp.Body.Close()
+
+ var data tc.ServiceCategoriesResponse
+ if err := json.NewDecoder(resp.Body).Decode(&data); err != nil {
+ return nil, reqInf, err
+ }
+
+ return data.Response, reqInf, nil
+}
+
+// GET a service category by the service category name
+func (to *Session) GetServiceCategoryByName(name string) ([]tc.ServiceCategory, ReqInf, error) {
+ url := fmt.Sprintf("%s?name=%s", API_serviceCategories, name)
Review comment:
this should use url formatting on the query parameter.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] rob05c edited a comment on issue #4518: Add
service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
rob05c edited a comment on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600756285
>If I am a user in tenant 2.1 and i create a Foo service category
I think the question for anything is, "Will delivery service customers who are not CDN operators be able to create this?" If so, it needs to be under tenancy, and invisible to other tenants.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] rob05c commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
rob05c commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600759309
Right. But for example, Delivery Service Types aren't "scoped" to tenants, because they're created by operators of the system, not customers of the system. I think it's a valid distinction, to decide if something applies across the entire CDN, or only to the Tenant who created it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] jehunte commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
jehunte commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r396551895
##########
File path: traffic_ops/client/serviceCategory.go
##########
@@ -0,0 +1,132 @@
+/*
+
+ 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.
+*/
+
+package client
+
+import (
+ "encoding/json"
+ "fmt"
+ "net"
+ "net/http"
+
+ "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+const (
+ API_serviceCategories = apiBase + "/servicecategories"
+)
+
+// Create a service category
+func (to *Session) CreateServiceCategory(serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
Review comment:
All of the other client files' functions are set up this way. Would you still like me to change it or keep it consistent with everything 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395132890
##########
File path: docs/source/overview/delivery_services.rst
##########
@@ -697,6 +697,12 @@ Servers
-------
Servers can be assigned to Delivery Services using the :ref:`tp-configure-servers` and :ref:`tp-services-delivery-service` Traffic Portal sections, or by directly using the :ref:`to-api-deliveryserviceserver` endpoint. Only :term:`Edge-tier cache servers` can be assigned to a Delivery Service, and once they are so assigned they will begin to serve content for the Delivery Service (after updates are queued and then applied). Any servers assigned to a Delivery Service must also belong to the same CDN_ as the Delivery Service itself. At least one server must be assigned to a Delivery Service in order for it to serve any content.
+.. _ds-service-category:
+
+Service Category
+------------
Review comment:
`-` line needs to match in length to title length
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395132077
##########
File path: docs/source/api/v2/service_categories.rst
##########
@@ -0,0 +1,156 @@
+..
+..
+.. 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-service-categories:
+
+************
+``service categories``
+************
+
+
+``GET``
+=======
+Get all requested :term:`Service Categories`.
+
+:Auth. Required: Yes
+:Roles Required: None
+:Response Type: Array
+
+Request Structure
+-----------------
+.. table:: Request Query Parameters
+
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +===========+===============================================================================================================+
+ | id | Filter for :term:`Service Categories` having this integral, unique identifier |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | name | Filter for :term:`Service Categories` with this name |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | orderby | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` |
+ | | array |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | sortOrder | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | limit | Choose the maximum number of results to return |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | offset | The number of results to skip before beginning to return results. Must use in conjunction with limit |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | page | Return the n\ :sup:`th` page of results, where "n" is the value of this parameter, pages are ``limit`` long |
+ | | and the first page is 1. If ``offset`` was defined, this query parameter has no effect. ``limit`` must be |
+ | | defined to make use of ``page``. |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+
+.. code-block:: http
+ :caption: Request Example
+
+ GET /api/2.0/servicecategories?name=SERVICE_CATEGORY_NAME HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+
+Response Structure
+------------------
+:id: The integral, unique identifier of this :term:`Service Category`
+:name: This :term:`Service Category`'s name
+:lastUpdated: The date and time at which this :term:`Service Category` was last modified, 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=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
+ Whole-Content-Sha512: Yzr6TfhxgpZ3pbbrr4TRG4wC3PlnHDDzgs2igtz/1ppLSy2MzugqaGW4y5yzwzl5T3+7q6HWej7GQZt1XIVeZQ==
+ X-Server-Name: traffic_ops_golang/
+ Date: Wed, 11 Mar 2020 20:02:47 GMT
+ Content-Length: 102
+
+{
Review comment:
the response json should be indented to match the HTTP Response headers
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 edited a comment on issue #4518: Add
service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 edited a comment on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600763820
> IMO it's sufficiently likely someone will need the latter, to go ahead and support it from the start.
Yeah, it's harder to add tenancy after the fact imo. It seems to me if i were to create a service category i might want to specify that it applies to all ds's (tenant=root) or only applies to ds's of the 2.1 tenant (tenant=2.1) or like you said grant users the ability to create service categories ONLY in there tenancy.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395157835
##########
File path: traffic_ops/traffic_ops_golang/servicecategory/servicecategories.go
##########
@@ -0,0 +1,124 @@
+package servicecategory
+/*
+ * 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 (
+"strconv"
Review comment:
gofmt
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r398862642
##########
File path: traffic_ops/client/serviceCategory.go
##########
@@ -0,0 +1,132 @@
+/*
+
+ 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.
+*/
+
+package client
+
+import (
+ "encoding/json"
+ "fmt"
+ "net"
+ "net/http"
+
+ "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+const (
+ API_serviceCategories = apiBase + "/servicecategories"
+)
+
+// Create a service category
+func (to *Session) CreateServiceCategory(serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
+
+ var remoteAddr net.Addr
+ reqBody, err := json.Marshal(serviceCategory)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ resp, remoteAddr, err := to.request(http.MethodPost, API_serviceCategories, reqBody)
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ defer resp.Body.Close()
+ var alerts tc.Alerts
+ err = json.NewDecoder(resp.Body).Decode(&alerts)
+ return alerts, reqInf, nil
+}
+
+// Update a service category by ID
+func (to *Session) UpdateServiceCategoryByID(id int, serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
+
+ var remoteAddr net.Addr
+ reqBody, err := json.Marshal(serviceCategory)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ route := fmt.Sprintf("%s/%d", API_serviceCategories, id)
+ resp, remoteAddr, err := to.request(http.MethodPut, route, reqBody)
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ defer resp.Body.Close()
+ var alerts tc.Alerts
+ err = json.NewDecoder(resp.Body).Decode(&alerts)
+ return alerts, reqInf, nil
+}
+
+// Returns a list of service categories
+func (to *Session) GetServiceCategories() ([]tc.ServiceCategory, ReqInf, error) {
+ resp, remoteAddr, err := to.request(http.MethodGet, API_serviceCategories, nil)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return nil, reqInf, err
+ }
+ defer resp.Body.Close()
+
+ var data tc.ServiceCategoriesResponse
+ err = json.NewDecoder(resp.Body).Decode(&data)
+ return data.Response, reqInf, nil
+}
+
+// GET a service category by the service category id
+func (to *Session) GetServiceCategoryByID(id int) ([]tc.ServiceCategory, ReqInf, error) {
+ route := fmt.Sprintf("%s?id=%d", API_serviceCategories, id)
+ resp, remoteAddr, err := to.request(http.MethodGet, route, nil)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return nil, reqInf, err
+ }
+ defer resp.Body.Close()
+
+ var data tc.ServiceCategoriesResponse
+ if err := json.NewDecoder(resp.Body).Decode(&data); err != nil {
+ return nil, reqInf, err
+ }
+
+ return data.Response, reqInf, nil
+}
+
+// GET a service category by the service category name
+func (to *Session) GetServiceCategoryByName(name string) ([]tc.ServiceCategory, ReqInf, error) {
+ url := fmt.Sprintf("%s?name=%s", API_serviceCategories, name)
Review comment:
https://golang.org/pkg/net/url/#QueryEscape
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600753108
Something to think about. Should "service categories" be restricted to certain tenants? For example, imagine the following tenant tree:
```
- root
-- tenant 1
--- tenant 1.1
-- tenant 2
--- tenant 2.1
```
If I am a user in tenant 2.1 and i create a Foo service category, should users in root, tenant 1, 1.1, 2 be able to attach the Foo service category to their delivery services? Maybe that's not a problem as the tenant 2.1 user will still only see delivery services scoped to tenant 2.1 in that category. However, this could potentially result in information leakage across tenants. I.e. if i'm in the company A tenant and i create a company-a-service-category-1, everybody across all tenants can see/use that service category.
The more I think about this, service categories should be scoped to tenancy. most tenants don't want to be concerned with the service categories created by other tenants.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395133061
##########
File path: lib/go-atscfg/headerrewritedotconfig.go
##########
@@ -40,6 +44,7 @@ type HeaderRewriteDS struct {
MaxOriginConnections int
MidHeaderRewrite string
Type tc.DSType
+ ServiceCategoryName string
Review comment:
run gofmt please
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] jehunte commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
jehunte commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r396552280
##########
File path: traffic_ops/client/serviceCategory.go
##########
@@ -0,0 +1,132 @@
+/*
+
+ 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.
+*/
+
+package client
+
+import (
+ "encoding/json"
+ "fmt"
+ "net"
+ "net/http"
+
+ "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+const (
+ API_serviceCategories = apiBase + "/servicecategories"
+)
+
+// Create a service category
+func (to *Session) CreateServiceCategory(serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
+
+ var remoteAddr net.Addr
+ reqBody, err := json.Marshal(serviceCategory)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ resp, remoteAddr, err := to.request(http.MethodPost, API_serviceCategories, reqBody)
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ defer resp.Body.Close()
+ var alerts tc.Alerts
+ err = json.NewDecoder(resp.Body).Decode(&alerts)
+ return alerts, reqInf, nil
+}
+
+// Update a service category by ID
+func (to *Session) UpdateServiceCategoryByID(id int, serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
+
+ var remoteAddr net.Addr
+ reqBody, err := json.Marshal(serviceCategory)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ route := fmt.Sprintf("%s/%d", API_serviceCategories, id)
+ resp, remoteAddr, err := to.request(http.MethodPut, route, reqBody)
+ if err != nil {
+ return tc.Alerts{}, reqInf, err
+ }
+ defer resp.Body.Close()
+ var alerts tc.Alerts
+ err = json.NewDecoder(resp.Body).Decode(&alerts)
+ return alerts, reqInf, nil
+}
+
+// Returns a list of service categories
+func (to *Session) GetServiceCategories() ([]tc.ServiceCategory, ReqInf, error) {
+ resp, remoteAddr, err := to.request(http.MethodGet, API_serviceCategories, nil)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return nil, reqInf, err
+ }
+ defer resp.Body.Close()
+
+ var data tc.ServiceCategoriesResponse
+ err = json.NewDecoder(resp.Body).Decode(&data)
+ return data.Response, reqInf, nil
+}
+
+// GET a service category by the service category id
+func (to *Session) GetServiceCategoryByID(id int) ([]tc.ServiceCategory, ReqInf, error) {
+ route := fmt.Sprintf("%s?id=%d", API_serviceCategories, id)
+ resp, remoteAddr, err := to.request(http.MethodGet, route, nil)
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+ if err != nil {
+ return nil, reqInf, err
+ }
+ defer resp.Body.Close()
+
+ var data tc.ServiceCategoriesResponse
+ if err := json.NewDecoder(resp.Body).Decode(&data); err != nil {
+ return nil, reqInf, err
+ }
+
+ return data.Response, reqInf, nil
+}
+
+// GET a service category by the service category name
+func (to *Session) GetServiceCategoryByName(name string) ([]tc.ServiceCategory, ReqInf, error) {
+ url := fmt.Sprintf("%s?name=%s", API_serviceCategories, name)
Review comment:
Can you be more specific on what you mean by url formatting?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600757067
> I think the question for anything is, "Will delivery service customers who are not CDN operators be able to create this?" If so, it needs to be under tenancy, and invisible to other tenants.
Well i'm assuming only operators (role=operations or above) can create service categories but those service categories will show up as an option of EVERY delivery service across all tenants which seems problematic.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395130632
##########
File path: docs/source/api/v2/deliveryservices.rst
##########
@@ -205,6 +207,7 @@ Response Structure
"regionalGeoBlocking": false,
"remapText": null,
"routingName": "video",
+ "serviceCategory": null,
Review comment:
Seems like this is a mismatch from the definition of the response structure above.
Response structure:
`serviceCategoryId`
`serviceCategoryName`
Response example:
`serviceCategory`
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395145207
##########
File path: traffic_ops/testing/api/v1/todb_test.go
##########
@@ -335,6 +335,7 @@ func Teardown(db *sql.DB) error {
DELETE FROM cachegroup;
DELETE FROM coordinate;
DELETE FROM type;
+ DELETE FROM service_category;
Review comment:
I dont think this needs to be in the v1 tests?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600763820
> IMO it's sufficiently likely someone will need the latter, to go ahead and support it from the start.
Yeah, it's harder to add tenancy after the fact imo. It seems to me if i were to create a service category i might want to specify that it applies to all ds's (tenant=root) or only applies to ds's of the 2.1 tenant (tenant=2.1)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395136497
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -159,9 +159,15 @@ type DeliveryServiceV11 struct {
XMLID string `json:"xmlId"`
}
-type DeliveryServiceNullableV15 DeliveryServiceNullable // this type alias should always alias the latest minor version of the deliveryservices endpoints
+type DeliveryServiceNullableV16 DeliveryServiceNullable // this type alias should always alias the latest minor version of the deliveryservices endpoints
Review comment:
I think you should make this v20.
There is no API version 1.6
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395136685
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -159,9 +159,15 @@ type DeliveryServiceV11 struct {
XMLID string `json:"xmlId"`
}
-type DeliveryServiceNullableV15 DeliveryServiceNullable // this type alias should always alias the latest minor version of the deliveryservices endpoints
+type DeliveryServiceNullableV16 DeliveryServiceNullable // this type alias should always alias the latest minor version of the deliveryservices endpoints
type DeliveryServiceNullable struct {
+ DeliveryServiceNullableV15
+ ServiceCategoryId *int `json:"serviceCategory" db:"service_category"`
+ ServiceCategoryName *string `json:"serviceCategoryName"`
Review comment:
go fmt
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395132234
##########
File path: docs/source/api/v2/service_categories.rst
##########
@@ -0,0 +1,156 @@
+..
+..
+.. 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-service-categories:
+
+************
+``service categories``
+************
+
+
+``GET``
+=======
+Get all requested :term:`Service Categories`.
+
+:Auth. Required: Yes
+:Roles Required: None
+:Response Type: Array
+
+Request Structure
+-----------------
+.. table:: Request Query Parameters
+
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +===========+===============================================================================================================+
+ | id | Filter for :term:`Service Categories` having this integral, unique identifier |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | name | Filter for :term:`Service Categories` with this name |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | orderby | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` |
+ | | array |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | sortOrder | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | limit | Choose the maximum number of results to return |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | offset | The number of results to skip before beginning to return results. Must use in conjunction with limit |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | page | Return the n\ :sup:`th` page of results, where "n" is the value of this parameter, pages are ``limit`` long |
+ | | and the first page is 1. If ``offset`` was defined, this query parameter has no effect. ``limit`` must be |
+ | | defined to make use of ``page``. |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+
+.. code-block:: http
+ :caption: Request Example
+
+ GET /api/2.0/servicecategories?name=SERVICE_CATEGORY_NAME HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+
+Response Structure
+------------------
+:id: The integral, unique identifier of this :term:`Service Category`
+:name: This :term:`Service Category`'s name
+:lastUpdated: The date and time at which this :term:`Service Category` was last modified, 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=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
+ Whole-Content-Sha512: Yzr6TfhxgpZ3pbbrr4TRG4wC3PlnHDDzgs2igtz/1ppLSy2MzugqaGW4y5yzwzl5T3+7q6HWej7GQZt1XIVeZQ==
+ X-Server-Name: traffic_ops_golang/
+ Date: Wed, 11 Mar 2020 20:02:47 GMT
+ Content-Length: 102
+
+{
+ "response": [
+ {
+ "id": 5,
+ "lastUpdated": "2020-03-04 15:46:20-07",
+ "name": "SERVICE_CATEGORY_NAME"
+ }
+ ]
+}
+
+``POST``
+========
+Create a new service category.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+:name: The name of the new service category
+
+.. code-block:: http
+ :caption: Request Example
+
+ POST /api/2.0/servicecategories HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+ Content-Length: 48
+ Content-Type: application/json
+
+ {
+ "name": "test",
Review comment:
invalid json
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] rob05c edited a comment on issue #4518: Add
service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
rob05c edited a comment on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600757234
Do we expect Service Categories to be created by DS customers? Or is it something only Operators will create?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] rob05c commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
rob05c commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600757234
Do we expect Service Categories to be created by tenants? Or is it something only Operators will create?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r399520470
##########
File path: traffic_ops/traffic_ops_golang/routing/routes.go
##########
@@ -333,6 +334,12 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
{api.Version{2, 0}, http.MethodPost, `server_server_capabilities/?$`, api.CreateHandler(&server.TOServerServerCapability{}), auth.PrivLevelOperations, Authenticated, nil, 2293166834, noPerlBypass},
{api.Version{2, 0}, http.MethodDelete, `server_server_capabilities/?$`, api.DeleteHandler(&server.TOServerServerCapability{}), auth.PrivLevelOperations, Authenticated, nil, 2058714058, noPerlBypass},
+ //Service Categories: CRUD
+ {api.Version{2, 0}, http.MethodGet, `servicecategories/?$`, api.ReadHandler(&servicecategory.TOServiceCategory{}), auth.PrivLevelReadOnly, Authenticated, nil, 1085181543, noPerlBypass},
Review comment:
Before I forget, when adding new api routes, you need to make some changes to the seeds.sql file to account for the day when rather than enforcing privLevel on a route, we enforce a role's "capabilities":
1. create 2 new capabilities (service-categories-read and service-categories-write) here: https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/db/seeds.sql#L93
2. add both capabilities to the admin and operations role:
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/db/seeds.sql#L209
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/db/seeds.sql#L326
3. add service-categories-read to the read-only role:
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/db/seeds.sql#L285
4. Actually define which routes are in each of the 2 capabilities
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/app/db/seeds.sql#L406
when this is done, you will basically have granted the admin/operations role with the service-categories-read and service-categories-write capability and the read-only role with the service-categories-read 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395147067
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -184,6 +183,29 @@ func CreateV15(w http.ResponseWriter, r *http.Request) {
api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceNullableV15{*res})
}
+// TODO allow users to post names (type, cdn, etc) and get the IDs from the names. This isn't trivial to do in a single query, without dynamically building the entire insert query, and ideally inserting would be one query. But it'd be much more convenient for users. Alternatively, remove IDs from the database entirely and use real candidate keys.
+func CreateV16(w http.ResponseWriter, r *http.Request) {
Review comment:
This should be v20. V16 is non existent
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600757547
> Do we expect Service Categories to be created by tenants? Or is it something only Operators will create?
everybody is scoped to a tenant. operators and non operators. all users are tenant scoped.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395132478
##########
File path: docs/source/api/v2/service_categories.rst
##########
@@ -0,0 +1,156 @@
+..
+..
+.. 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-service-categories:
+
+************
+``service categories``
+************
+
+
+``GET``
+=======
+Get all requested :term:`Service Categories`.
+
+:Auth. Required: Yes
+:Roles Required: None
+:Response Type: Array
+
+Request Structure
+-----------------
+.. table:: Request Query Parameters
+
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +===========+===============================================================================================================+
+ | id | Filter for :term:`Service Categories` having this integral, unique identifier |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | name | Filter for :term:`Service Categories` with this name |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | orderby | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` |
+ | | array |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | sortOrder | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | limit | Choose the maximum number of results to return |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | offset | The number of results to skip before beginning to return results. Must use in conjunction with limit |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+ | page | Return the n\ :sup:`th` page of results, where "n" is the value of this parameter, pages are ``limit`` long |
+ | | and the first page is 1. If ``offset`` was defined, this query parameter has no effect. ``limit`` must be |
+ | | defined to make use of ``page``. |
+ +-----------+---------------------------------------------------------------------------------------------------------------+
+
+.. code-block:: http
+ :caption: Request Example
+
+ GET /api/2.0/servicecategories?name=SERVICE_CATEGORY_NAME HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+
+Response Structure
+------------------
+:id: The integral, unique identifier of this :term:`Service Category`
+:name: This :term:`Service Category`'s name
+:lastUpdated: The date and time at which this :term:`Service Category` was last modified, 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=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
+ Whole-Content-Sha512: Yzr6TfhxgpZ3pbbrr4TRG4wC3PlnHDDzgs2igtz/1ppLSy2MzugqaGW4y5yzwzl5T3+7q6HWej7GQZt1XIVeZQ==
+ X-Server-Name: traffic_ops_golang/
+ Date: Wed, 11 Mar 2020 20:02:47 GMT
+ Content-Length: 102
+
+{
+ "response": [
+ {
+ "id": 5,
+ "lastUpdated": "2020-03-04 15:46:20-07",
+ "name": "SERVICE_CATEGORY_NAME"
+ }
+ ]
+}
+
+``POST``
+========
+Create a new service category.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+:name: The name of the new service category
+
+.. code-block:: http
+ :caption: Request Example
+
+ POST /api/2.0/servicecategories HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+ Content-Length: 48
+ Content-Type: application/json
+
+ {
+ "name": "test",
+ }
+
+Response Structure
+------------------
+:id: An integral, unique identifier for this Service Category
+:lastUpdated: The date and time at which this Service Category was last modified, in ISO format
+:name: The :term:`Service Category` name
+
+.. 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=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
+ Whole-Content-Sha512: +pJm4c3O+JTaSXNt+LP+u240Ba/SsvSSDOQ4rDc6hcyZ0FIL+iY/WWrMHhpLulRGKGY88bM4YPCMaxGn3FZ9yQ==
+ X-Server-Name: traffic_ops_golang/
+ Date: Wed, 11 Mar 2020 20:12:20 GMT
+ Content-Length: 154
+
+{
Review comment:
indented here as well
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] rob05c commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
rob05c commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600762344
@mitchell852 I think I agree with you. It's possible a TC Operator will want to make a hard list of categories, e.g. "Linear," "VOD," "Images." But it's also possible the TC Operator will want to allow any DS owner to make any categories they want.
If Service Categories are scoped to Tenants, and under Roles&Capabilities, both are possible.
For the former, the Operator just doesn't give any tenants the Create Service Category Capability, and they make the hard-coded categories on the Root tenant.
For the latter, they give everyone the Capability.
Keeping YAGNI in mind, IMO it's sufficiently likely someone will need the latter, to go ahead and support it from the start.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600801253
also, those 2 sql scripts under additional info seem like something very specific to a certain companies use of edge header rewrites. probably not something needed as part of this PR. also, @jehunte can you make sure your screenshots are generic? i.e. no company-specific info.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] jehunte commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
jehunte commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395177214
##########
File path: traffic_ops/traffic_ops_golang/servicecategory/servicecategories.go
##########
@@ -0,0 +1,124 @@
+package servicecategory
+/*
+ * 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 (
+"strconv"
+"strings"
+
+"github.com/apache/trafficcontrol/lib/go-tc"
+"github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
+"github.com/apache/trafficcontrol/lib/go-util"
+"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+
+validation "github.com/go-ozzo/ozzo-validation"
+)
+
+//we need a type alias to define functions on
+type TOServiceCategory struct {
+ api.APIInfoImpl `json:"-"`
+ tc.ServiceCategoryNullable
+}
+
+func (v *TOServiceCategory) SetLastUpdated(t tc.TimeNoMod) { v.LastUpdated = &t }
+func (v *TOServiceCategory) InsertQuery() string { return insertQuery() }
+func (v *TOServiceCategory) NewReadObj() interface{} { return &tc.ServiceCategory{} }
+func (v *TOServiceCategory) SelectQuery() string { return selectQuery() }
+func (v *TOServiceCategory) ParamColumns() map[string]dbhelpers.WhereColumnInfo {
+ return map[string]dbhelpers.WhereColumnInfo{
+ "id": dbhelpers.WhereColumnInfo{"id", api.IsInt},
+ "name": dbhelpers.WhereColumnInfo{"name", nil},
+ }
+}
+func (v *TOServiceCategory) UpdateQuery() string { return updateQuery() }
+func (v *TOServiceCategory) DeleteQuery() string { return deleteQuery() }
+
+func (serviceCategory TOServiceCategory) GetAuditName() string {
+ if serviceCategory.Name != nil {
+ return *serviceCategory.Name
+ }
+ if serviceCategory.ID != nil {
+ return strconv.Itoa(*serviceCategory.ID)
+ }
+ return "unknown"
+}
+
+func (serviceCategory TOServiceCategory) GetKeyFieldsInfo() []api.KeyFieldInfo {
+ return []api.KeyFieldInfo{{"id", api.GetIntKey}}
+}
+
+//Implementation of the Identifier, Validator interface functions
+func (serviceCategory TOServiceCategory) GetKeys() (map[string]interface{}, bool) {
+ if serviceCategory.ID == nil {
+ return map[string]interface{}{"id": 0}, false
+ }
+ return map[string]interface{}{"id": *serviceCategory.ID}, true
+}
+
+func (serviceCategory *TOServiceCategory) SetKeys(keys map[string]interface{}) {
+ i, _ := keys["id"].(int) //this utilizes the non panicking type assertion, if the thrown away ok variable is false i will be the zero of the type, 0 here.
+ serviceCategory.ID = &i
+}
+
+func (serviceCategory TOServiceCategory) GetType() string {
+ return "serviceCategory"
+}
+
+func (serviceCategory TOServiceCategory) Validate() error {
+ errs := validation.Errors{
+ "name": validation.Validate(serviceCategory.Name, validation.NotNil, validation.Required),
+ }
+ return util.JoinErrs(tovalidate.ToErrors(errs))
+}
+
+func (dv *TOServiceCategory) Create() (error, error, int) { return api.GenericCreate(dv) }
+func (dv *TOServiceCategory) Read() ([]interface{}, error, error, int) {
+ params := dv.APIInfo().Params
+ // TODO move to router, and do for all endpoints
+ if strings.HasSuffix(params["name"], ".json") {
Review comment:
Fixing this - Realized it wasn't necessary when making the tenant changes.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r398861342
##########
File path: traffic_ops/client/serviceCategory.go
##########
@@ -0,0 +1,132 @@
+/*
+
+ 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.
+*/
+
+package client
+
+import (
+ "encoding/json"
+ "fmt"
+ "net"
+ "net/http"
+
+ "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+const (
+ API_serviceCategories = apiBase + "/servicecategories"
+)
+
+// Create a service category
+func (to *Session) CreateServiceCategory(serviceCategory tc.ServiceCategory) (tc.Alerts, ReqInf, error) {
Review comment:
Would you mind changing it ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] jehunte commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
jehunte commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r396555307
##########
File path: traffic_ops/traffic_ops_golang/routing/routes.go
##########
@@ -477,8 +484,9 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
////DeliveryServices
{api.Version{2, 0}, http.MethodGet, `deliveryservices/?$`, api.ReadHandler(&deliveryservice.TODeliveryService{}), auth.PrivLevelReadOnly, Authenticated, nil, 2238317294, noPerlBypass},
- {api.Version{2, 0}, http.MethodPost, `deliveryservices/?$`, deliveryservice.CreateV15, auth.PrivLevelOperations, Authenticated, nil, 206431432, noPerlBypass},
- {api.Version{2, 0}, http.MethodPut, `deliveryservices/{id}/?$`, deliveryservice.UpdateV15, auth.PrivLevelOperations, Authenticated, nil, 2766567527, noPerlBypass},
+ {api.Version{2, 0}, http.MethodGet, `deliveryservices/{id}/?$`, api.ReadHandler(&deliveryservice.TODeliveryService{}), auth.PrivLevelReadOnly, Authenticated, nil, 244348195, noPerlBypass},
Review comment:
How is the update done in 2.0 without deliveryservices/id? It looks like that path still exists.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395140709
##########
File path: traffic_ops/app/db/migrations/20200315000000_add_deliveryservice_service_category.sql
##########
@@ -0,0 +1,25 @@
+/*
+ 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.
+*/
+
+-- +goose Up
+-- SQL in section 'Up' is executed when this migration is applied
+ALTER TABLE deliveryservice ADD COLUMN service_category int REFERENCES service_category(id);
+CREATE TABLE IF NOT EXISTS service_category (
Review comment:
doesnt the table need to be created prior to adding the reference in the deliveryservice 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mitchell852 edited a comment on issue #4518: Add
service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mitchell852 edited a comment on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600801253
also, those 2 sql scripts under additional info seem like something very specific to a certain company's use of edge header rewrites. probably not something needed as part of this PR. also, @jehunte can you make sure your screenshots are generic? i.e. no company-specific info.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395156469
##########
File path: traffic_ops/traffic_ops_golang/routing/routes.go
##########
@@ -477,8 +484,9 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
////DeliveryServices
{api.Version{2, 0}, http.MethodGet, `deliveryservices/?$`, api.ReadHandler(&deliveryservice.TODeliveryService{}), auth.PrivLevelReadOnly, Authenticated, nil, 2238317294, noPerlBypass},
- {api.Version{2, 0}, http.MethodPost, `deliveryservices/?$`, deliveryservice.CreateV15, auth.PrivLevelOperations, Authenticated, nil, 206431432, noPerlBypass},
- {api.Version{2, 0}, http.MethodPut, `deliveryservices/{id}/?$`, deliveryservice.UpdateV15, auth.PrivLevelOperations, Authenticated, nil, 2766567527, noPerlBypass},
+ {api.Version{2, 0}, http.MethodGet, `deliveryservices/{id}/?$`, api.ReadHandler(&deliveryservice.TODeliveryService{}), auth.PrivLevelReadOnly, Authenticated, nil, 244348195, noPerlBypass},
Review comment:
the deliveryservices/id got deprecated in 2.0.
Also the .json should be removed from regex on the routes
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395158605
##########
File path: traffic_ops/traffic_ops_golang/servicecategory/servicecategories.go
##########
@@ -0,0 +1,124 @@
+package servicecategory
+/*
+ * 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 (
+"strconv"
+"strings"
+
+"github.com/apache/trafficcontrol/lib/go-tc"
+"github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
+"github.com/apache/trafficcontrol/lib/go-util"
+"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+
+validation "github.com/go-ozzo/ozzo-validation"
+)
+
+//we need a type alias to define functions on
+type TOServiceCategory struct {
+ api.APIInfoImpl `json:"-"`
+ tc.ServiceCategoryNullable
+}
+
+func (v *TOServiceCategory) SetLastUpdated(t tc.TimeNoMod) { v.LastUpdated = &t }
+func (v *TOServiceCategory) InsertQuery() string { return insertQuery() }
+func (v *TOServiceCategory) NewReadObj() interface{} { return &tc.ServiceCategory{} }
+func (v *TOServiceCategory) SelectQuery() string { return selectQuery() }
+func (v *TOServiceCategory) ParamColumns() map[string]dbhelpers.WhereColumnInfo {
+ return map[string]dbhelpers.WhereColumnInfo{
+ "id": dbhelpers.WhereColumnInfo{"id", api.IsInt},
+ "name": dbhelpers.WhereColumnInfo{"name", nil},
+ }
+}
+func (v *TOServiceCategory) UpdateQuery() string { return updateQuery() }
+func (v *TOServiceCategory) DeleteQuery() string { return deleteQuery() }
+
+func (serviceCategory TOServiceCategory) GetAuditName() string {
+ if serviceCategory.Name != nil {
+ return *serviceCategory.Name
+ }
+ if serviceCategory.ID != nil {
+ return strconv.Itoa(*serviceCategory.ID)
+ }
+ return "unknown"
+}
+
+func (serviceCategory TOServiceCategory) GetKeyFieldsInfo() []api.KeyFieldInfo {
+ return []api.KeyFieldInfo{{"id", api.GetIntKey}}
+}
+
+//Implementation of the Identifier, Validator interface functions
+func (serviceCategory TOServiceCategory) GetKeys() (map[string]interface{}, bool) {
+ if serviceCategory.ID == nil {
+ return map[string]interface{}{"id": 0}, false
+ }
+ return map[string]interface{}{"id": *serviceCategory.ID}, true
+}
+
+func (serviceCategory *TOServiceCategory) SetKeys(keys map[string]interface{}) {
+ i, _ := keys["id"].(int) //this utilizes the non panicking type assertion, if the thrown away ok variable is false i will be the zero of the type, 0 here.
+ serviceCategory.ID = &i
+}
+
+func (serviceCategory TOServiceCategory) GetType() string {
+ return "serviceCategory"
+}
+
+func (serviceCategory TOServiceCategory) Validate() error {
+ errs := validation.Errors{
+ "name": validation.Validate(serviceCategory.Name, validation.NotNil, validation.Required),
+ }
+ return util.JoinErrs(tovalidate.ToErrors(errs))
+}
+
+func (dv *TOServiceCategory) Create() (error, error, int) { return api.GenericCreate(dv) }
+func (dv *TOServiceCategory) Read() ([]interface{}, error, error, int) {
+ params := dv.APIInfo().Params
+ // TODO move to router, and do for all endpoints
+ if strings.HasSuffix(params["name"], ".json") {
Review comment:
huh?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] rob05c commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
rob05c commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600756285
>If I am a user in tenant 2.1 and i create a Foo service category
I think the question for anything is, "Will delivery service customers who are not CDN operators be able to create this?" If so, it needs to be under tenancy, and invisible to other tenants.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395131104
##########
File path: docs/source/api/v2/service_categories.rst
##########
@@ -0,0 +1,156 @@
+..
+..
+.. 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-service-categories:
+
+************
+``service categories``
+************
Review comment:
these `*` lines need to line up to the title length
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r398861038
##########
File path: traffic_ops/traffic_ops_golang/routing/routes.go
##########
@@ -477,8 +484,9 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
////DeliveryServices
{api.Version{2, 0}, http.MethodGet, `deliveryservices/?$`, api.ReadHandler(&deliveryservice.TODeliveryService{}), auth.PrivLevelReadOnly, Authenticated, nil, 2238317294, noPerlBypass},
- {api.Version{2, 0}, http.MethodPost, `deliveryservices/?$`, deliveryservice.CreateV15, auth.PrivLevelOperations, Authenticated, nil, 206431432, noPerlBypass},
- {api.Version{2, 0}, http.MethodPut, `deliveryservices/{id}/?$`, deliveryservice.UpdateV15, auth.PrivLevelOperations, Authenticated, nil, 2766567527, noPerlBypass},
+ {api.Version{2, 0}, http.MethodGet, `deliveryservices/{id}/?$`, api.ReadHandler(&deliveryservice.TODeliveryService{}), auth.PrivLevelReadOnly, Authenticated, nil, 244348195, noPerlBypass},
Review comment:
oh I meant just the `GET` was deprecated in 2.0 for deliveryservices/id - the other verbs are still supported
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] jehunte commented on issue #4518: Add service
category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
jehunte commented on issue #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#issuecomment-600818054
@rob05c @mitchell852 These are great points. I will require a tenant to be associated with the service categories so tenants can only see the service categories they've created. I've also replaced the screenshots and removed the sql queries.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [trafficcontrol] mhoppa commented on a change in pull request
#4518: Add service category feature to TO and TP
Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4518: Add service category feature to TO and TP
URL: https://github.com/apache/trafficcontrol/pull/4518#discussion_r395145313
##########
File path: traffic_ops/testing/api/v2/withobjs_test.go
##########
@@ -99,6 +100,7 @@ var withFuncs = map[TCObj]TCObjFuncs{
ServerChecks: {CreateTestServerChecks, DeleteTestServerChecks},
ServerServerCapabilities: {CreateTestServerServerCapabilities, DeleteTestServerServerCapabilities},
Servers: {CreateTestServers, DeleteTestServers},
+ ServiceCategories: {CreateTestServiceCategory, DeleteTestServiceCategories},
Review comment:
gofmt
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services