[GitHub] [trafficcontrol] rawlinp opened a new pull request, #7124: Do not include non-TR profile params in CRConfig "config" section
rawlinp opened a new pull request, #7124:
URL: https://github.com/apache/trafficcontrol/pull/7124
Instead of querying all profile parameters with the config file
"CRConfig.json" for any servers of any type that belong to a given CDN, query
only profile params assigned to profiles of CCR-type servers for the given CDN.
Fixes #3974.
## Which Traffic Control components are affected by this PR?
- Traffic Ops
## What is the best way to verify this PR?
ATS profiles will generally have a "weight" parameter assigned (with
config_file = CRConfig.json), and this will tend to show up in the CRConfig
"config" section like so:
```
"config": {
"weight": "0.8",
```
With this PR, snapshotting that CDN should remove that "weight" parameter
from the CRConfig's "config" section.
## PR submission checklist
- [x] This PR has tests
- [x] minor tech debt, no docs needed
- [x] minor tech debt, no changelog needed
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the
Apache Software Foundation's security guidelines](https://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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] rawlinp opened a new pull request, #7123: Add missing TR profile parameters to docs, correct interval units
rawlinp opened a new pull request, #7123: URL: https://github.com/apache/trafficcontrol/pull/7123 Fixes #6670. ## Which Traffic Control components are affected by this PR? - Documentation ## What is the best way to verify this PR? Build and read the resulting docs. ## PR submission checklist - [x] docs, no tests needed - [x] This PR has documentation - [x] docs, no changelog necessary - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] rawlinp opened a new pull request, #7122: Minor TM refactor to simplify statecombiner logic
rawlinp opened a new pull request, #7122: URL: https://github.com/apache/trafficcontrol/pull/7122 This does not actually change the behavior in any way -- it just simplifies the logic by removing a hard-coded boolean parameter. ## Which Traffic Control components are affected by this PR? - Traffic Monitor ## What is the best way to verify this PR? Run TM, ensure that it still polls caches normally. ## PR submission checklist - [x] This PR has tests - [x] Minor refactor, no docs needed - [x] Minor refactor, no CHANGELOG.md entry needed - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7079: Assign multiple servers to a capability
ocket commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r992828648
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,178 @@ func AssignMultipleServerCapabilities(w
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
- var msc tc.MultipleServerCapabilities
- if err := json.NewDecoder(r.Body).Decode(); err != nil {
- api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ var mssc tc.MultipleServersCapabilities
+ if err := json.NewDecoder(r.Body).Decode(); err != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest,
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities
struct %w", err), nil)
return
}
- // Check existence prior to checking type
- _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
- if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
- }
- if !exists {
- userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
- return
+ if len(mssc.ServerIDs) >= 1 {
+ errCode, userErr, sysErr = checkExistingServer(tx,
mssc.ServerIDs, inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
}
// Ensure type is correct
- correctType := true
- if err := tx.QueryRow(scCheckServerTypeQuery(),
msc.ServerID).Scan(); err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
fmt.Errorf("checking server type: %w", err))
+ errCode, userErr, sysErr = checkServerType(tx, mssc.ServerIDs)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
- if !correctType {
- userErr := fmt.Errorf("server %d has an incorrect server type.
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
- return
+
+ // Insert rows in DB
+ sid := make([]int64, len(mssc.ServerCapabilities))
+ scs := make([]string, len(mssc.ServerIDs))
+ if len(mssc.ServerIDs) == 1 {
+ if len(mssc.ServerCapabilities) >= 1 {
+ for i := range mssc.ServerCapabilities {
+ sid[i] = mssc.ServerIDs[0]
+ }
+ scs = mssc.ServerCapabilities
+ }
+ } else if len(mssc.ServerCapabilities) == 1 {
+ if len(mssc.ServerIDs) >= 1 {
+ for i := range mssc.ServerIDs {
+ scs[i] = mssc.ServerCapabilities[0]
+ }
+ sid = mssc.ServerIDs
+ }
+ } else {
+ scs = mssc.ServerCapabilities
+ sid = mssc.ServerIDs
}
- cdnName, err := dbhelpers.GetCDNNameFromServerID(tx,
int64(msc.ServerID))
+ msscQuery := `INSERT INTO server_server_capability
+ select "server_capability", "server"
+ FROM UNNEST($1::text[], $2::int[]) AS
tmp("server_capability", "server")`
+ _, err := tx.Query(msscQuery, pq.Array(scs), pq.Array(sid))
if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
+ useErr, sysErr, statusCode := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
return
}
- userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx,
string(cdnName), inf.User.UserName)
+ var alerts tc.Alerts
+ if mssc.PageType == "sc" {
+ alerts = tc.CreateAlerts(tc.SuccessLevel, "Assign Server(s) to
a capability")
+ } else {
+ alerts = tc.CreateAlerts(tc.SuccessLevel, "Assign Server
Capability(ies) to a server")
+ }
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, mssc)
+ return
+}
+
+// DeleteMultipleServersCapabilities deletes multiple servers to a capability
or multiple server capabilities to a server
+func DeleteMultipleServersCapabilities(w http.ResponseWriter, r *http.Request)
{
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx.Tx
if userErr != nil || sysErr != nil {
- api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
+ defer inf.Close()
- //Delete
[GitHub] [trafficcontrol] ocket8888 closed issue #6021: TP: click on change log entry to view in its entirety
ocket closed issue #6021: TP: click on change log entry to view in its entirety URL: https://github.com/apache/trafficcontrol/issues/6021 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7087: TP Allow Change Logs to be "expanded" to see the whole message.
ocket merged PR #7087: URL: https://github.com/apache/trafficcontrol/pull/7087 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
ocket merged PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7104: TP fix child DS tables not loading
ocket merged PR #7104: URL: https://github.com/apache/trafficcontrol/pull/7104 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7104: TP fix child DS tables not loading
ocket commented on code in PR #7104: URL: https://github.com/apache/trafficcontrol/pull/7104#discussion_r992811269 ## traffic_portal/app/src/common/modules/table/serviceCategoryDeliveryServices/table.serviceCategoryDeliveryServices.tpl.html: ## @@ -26,7 +26,6 @@ columns="columns" drop-down-options="dropDownOptions" context-menu-options="contextMenuOptions" - sensitive-columns="sensitiveColumns" Review Comment: I don't know how that slipped through the cracks... -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
ocket commented on code in PR #7111:
URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r992808824
##
traffic_portal/test/integration/CommonUtils/API.ts:
##
@@ -296,7 +296,7 @@ export class API {
const rand = () => Math.floor(Math.random()*255)+1;
data.ipAddress = `${rand()}.${rand()}.${rand()}.${rand()}`;
}
-if(hasProperty(data, 'name') && !hasProperty(data, 'noRandomize')) {
+if(hasProperty(data, 'name') && !(hasProperty(data, "noRandomize") &&
data.noRandomize === true)) {
Review Comment:
nit: booleans shouldn't be compared to boolean constants, in general.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 opened a new pull request, #7121: Obscure sensitive Traffic Portal table fields
ocket opened a new pull request, #7121: URL: https://github.com/apache/trafficcontrol/pull/7121 Implements changes to TP tables to hide potentially sensitive fields behind a checkbox setting. To make things easier, I also eliminated a few tables that could be reproduced as filters on existing tables. ## Which Traffic Control components are affected by this PR? - Traffic Portal ## What is the best way to verify this PR? Check the tables for servers and Delivery Services and verify that sensitive columns aren't shown unless the relevant setting is checked. ## PR submission checklist - [x] This PR has tests - [x] This PR has documentation - [x] This PR has a CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111:
URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r992745127
##
traffic_portal/test/integration/CommonUtils/API.ts:
##
@@ -296,7 +296,7 @@ export class API {
const rand = () => Math.floor(Math.random()*255)+1;
data.ipAddress = `${rand()}.${rand()}.${rand()}.${rand()}`;
}
-if(hasProperty(data, 'name')) {
+if(hasProperty(data, 'name') && !hasProperty(data, 'noRandomize')) {
Review Comment:
Making sure `noRandomize` is not `false` in 3eda6cba43
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7068: Renamed rascal to tm/traffic_monitor
ocket commented on code in PR #7068: URL: https://github.com/apache/trafficcontrol/pull/7068#discussion_r992738599 ## traffic_ops/app/db/migrations/2022100610190300_update_rascal_to_traffic_monitor.up.sql: ## @@ -0,0 +1,35 @@ +/* + * 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. + */ + +UPDATE TYPE +SET name = 'TRAFFIC_MONITOR', +description = 'Traffic Monitor polling & reporting' +WHERE name = 'RASCAL'; + +UPDATE PARAMETER +SET config_file = REPLACE(config_file, 'rascal', 'traffic_monitor') +WHERE config_file = 'rascal-config.txt' OR config_file = 'rascal.properties'; + +UPDATE PARAMETER +SET value = 'TRAFFIC_MONITOR_TOP' +WHERE value = 'RASCAL_TOP' AND name = 'latest_traffic_monitor'; Review Comment: Does that have some kind of use in a proprietary plugin or tool? Because from ATC's perspective, that parameter is meaningless unless it's assigned to a server that runs t3c (i.e. a cache server. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7045: Run tests against Apache Traffic Server 9.1
ocket commented on code in PR #7045: URL: https://github.com/apache/trafficcontrol/pull/7045#discussion_r992733727 ## .github/actions/repo-info/action.yml: ## @@ -25,7 +25,7 @@ inputs: description: 'The owners github repository ie, "trafficcontrol"' required: true branch: -description: 'The repository branch to query ie, "8.1.0-branch"' +description: 'The repository branch to query ie, "9.1.3-branch"' Review Comment: if `9.1.3-branch` is the only allowed value, why is this even an input? -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7115: Update TO v5 API tests `TestAPIBase` to API v5
ocket merged PR #7115: URL: https://github.com/apache/trafficcontrol/pull/7115 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
ocket commented on code in PR #7111:
URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r992729491
##
traffic_portal/test/integration/CommonUtils/API.ts:
##
@@ -296,7 +296,7 @@ export class API {
const rand = () => Math.floor(Math.random()*255)+1;
data.ipAddress = `${rand()}.${rand()}.${rand()}.${rand()}`;
}
-if(hasProperty(data, 'name')) {
+if(hasProperty(data, 'name') && !hasProperty(data, 'noRandomize')) {
Review Comment:
Does this need to handle if `noRandomize` is `false`?
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7068: Renamed rascal to tm/traffic_monitor
rimashah25 commented on code in PR #7068: URL: https://github.com/apache/trafficcontrol/pull/7068#discussion_r992730551 ## traffic_ops/app/db/migrations/2022100610190300_update_rascal_to_traffic_monitor.up.sql: ## @@ -0,0 +1,35 @@ +/* + * 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. + */ + +UPDATE TYPE +SET name = 'TRAFFIC_MONITOR', +description = 'Traffic Monitor polling & reporting' +WHERE name = 'RASCAL'; + +UPDATE PARAMETER +SET config_file = REPLACE(config_file, 'rascal', 'traffic_monitor') +WHERE config_file = 'rascal-config.txt' OR config_file = 'rascal.properties'; + +UPDATE PARAMETER +SET value = 'TRAFFIC_MONITOR_TOP' +WHERE value = 'RASCAL_TOP' AND name = 'latest_traffic_monitor'; Review Comment: One of the parameters in production today https://user-images.githubusercontent.com/22248619/195187661-44422044-f08a-4e99-a9e5-f915675c32b8.png;> -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7050: Add configuration setting for TO client request timeout
ocket commented on code in PR #7050:
URL: https://github.com/apache/trafficcontrol/pull/7050#discussion_r992726936
##
infrastructure/ansible/roles/traffic_stats/templates/traffic_stats.cfg.j2:
##
@@ -15,6 +15,7 @@
"toUser": "{{ ts_toUser }}",
"toPasswd": "{{ ts_toPasswd }}",
"toUrl": "{{ ts_toUrl }}",
+"toRequestTimeout": "{{ ts_toRequestTimeout }}",
Review Comment:
if it's a number you don't need them. The others look like they're strings,
so they do need them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7118: Remove ASN from servers
ocket commented on code in PR #7118:
URL: https://github.com/apache/trafficcontrol/pull/7118#discussion_r992723542
##
traffic_ops/traffic_ops_golang/deliveryservice/keys_test.go:
##
@@ -2260,6 +2261,7 @@ func TestVerifyAndEncodeCertificateSelfSignedX509v1(t
*testing.T) {
certChain, certPrivateKey, unknownAuth, _, err :=
verifyCertKeyPair(SelfSignedX509v1Certificate, SelfSignedX509v1PrivateKey, "",
true)
if err != nil {
+ fmt.Println(err)
t.Fatalf("unexpected result: the x509v1 cert/key pair is valid
and should have passed validation")
Review Comment:
to print the error that occurred, it should be added to the `t.Fatalf` call
instead of printed directly to stdout with `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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7079: Assign multiple servers to a capability
ocket commented on code in PR #7079: URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r987141091 ## docs/source/glossary.rst: ## @@ -414,6 +414,10 @@ Glossary .. seealso:: For a more complete description of Roles, see the :ref:`roles` overview section. + Server + Servers + A :dfn:`Server` implies a :dfn:`cache servers` and/or :dfn:`origin servers` associated with a :dfn:`Delivery Service`. Review Comment: This isn't true, a server could also be a Traffic Monitor, Traffic Ops, Traffic Router, or any other kind of server you could think up. ## docs/source/api/v4/multiple_servers_capabilities.rst: ## @@ -0,0 +1,187 @@ +.. +.. +.. 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-v4-multiple_servers_capabilities: + +* +``multiple_servers_capabilities`` +* + +.. versionadded:: 4.1 + +``POST`` + +Inserts a list of :term:`Server Capability` associated to a server and vice versa i.e insert a list of :term:`Server` associated to a server capability. Review Comment: these terms should be plural; `Server Capabilities` and `Servers`, based on their usage ## docs/source/glossary.rst: ## @@ -414,6 +414,10 @@ Glossary .. seealso:: For a more complete description of Roles, see the :ref:`roles` overview section. + Server + Servers + A :dfn:`Server` implies a :dfn:`cache servers` and/or :dfn:`origin servers` associated with a :dfn:`Delivery Service`. Review Comment: Also, the `dfn` role is for the "defining instance" of a term. The first one you have on `Server` is appropriate, the rest should probably be `term`s ## docs/source/api/v5/multiple_servers_capabilities.rst: ## @@ -0,0 +1,185 @@ +.. +.. +.. 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-multiple_servers_capabilities: + +* +``multiple_servers_capabilities`` +* + +``POST`` + +Inserts a list of :term:`Server Capability` associated to a server and vice versa i.e insert a list of :term:`Server` associated to a server capability. Review Comment: Same as above ## docs/source/api/v5/multiple_servers_capabilities.rst: ## @@ -0,0 +1,123 @@ +.. +.. +.. 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-multiple_servers_capabilities: + +* +``multiple_servers_capabilities`` +* + +``PUT`` + +Associates a list of :term:`Server Capability` to a server. The API call replaces all the server capabilities assigned to a server with the ones specified in the serverCapabilities field. +And also Associates a list of :term:`Servers` to a server capability. The API call replaces all the servers assigned to a server capability with the ones specified in the servers field. + +:Auth. Required: Yes +:Roles Required: "admin" or "operations" +:Permissions Required: SERVER:READ, SERVER:UPDATE, SERVER-CAPABILITY:READ, SERVER-CAPABILITY:UPDATE +:Response Type: Object + +Request Structure +- +:serverIds: List of :term:`Server` ids ((integral, unique identifier) associated with a :term:`Server Capability` +:serverCapabilities: List of :term:`Server Capability`'s
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7068: Renamed rascal to tm/traffic_monitor
ocket commented on code in PR #7068: URL: https://github.com/apache/trafficcontrol/pull/7068#discussion_r992701613 ## traffic_ops/app/db/migrations/2022100610190300_update_rascal_to_traffic_monitor.up.sql: ## @@ -0,0 +1,35 @@ +/* + * 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. + */ + +UPDATE TYPE +SET name = 'TRAFFIC_MONITOR', +description = 'Traffic Monitor polling & reporting' +WHERE name = 'RASCAL'; + +UPDATE PARAMETER +SET config_file = REPLACE(config_file, 'rascal', 'traffic_monitor') +WHERE config_file = 'rascal-config.txt' OR config_file = 'rascal.properties'; + +UPDATE PARAMETER +SET value = 'TRAFFIC_MONITOR_TOP' +WHERE value = 'RASCAL_TOP' AND name = 'latest_traffic_monitor'; Review Comment: I don't see any instances of `RASCAL_TOP` in the repository - what is this supposed to do? -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7068: Renamed rascal to tm/traffic_monitor
ocket commented on code in PR #7068:
URL: https://github.com/apache/trafficcontrol/pull/7068#discussion_r992700325
##
traffic_ops/traffic_ops_golang/server/servers.go:
##
@@ -856,6 +856,12 @@ func getServers(h http.Header, params map[string]string,
tx *sqlx.Tx, user *auth
`
}
+ if version.Major >= 5 {
+ if params["type"] == "RASCAL" {
+ params["type"] = "TRAFFIC_MONITOR"
+ }
+ }
+
Review Comment:
That's a hard question, but the other thing is that t3c doesn't use APIv5,
it uses v4 and v3. So old versions of t3c wouldn't be kept compatible with
this. In a version of t3c that _does_ use APIv5, we could make it just query
for `TRAFFIC_MONITOR` instead, but that doesn't help backwards compatibility.
Maybe the best thing to do would be to make it return *both*
`TRAFFIC_MONITOR` *and* `RASCAL`? That makes things a bit more annoying for
operators and clients, potentially, but it makes it so there's no type you
can't filter for, at least as long as you use your types responsibly.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] github-code-scanning[bot] commented on a diff in pull request #7079: Assign multiple servers to a capability
github-code-scanning[bot] commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r992589196
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,178 @@
}
defer inf.Close()
- var msc tc.MultipleServerCapabilities
- if err := json.NewDecoder(r.Body).Decode(); err != nil {
- api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ var mssc tc.MultipleServersCapabilities
+ if err := json.NewDecoder(r.Body).Decode(); err != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest,
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities
struct %w", err), nil)
return
}
- // Check existence prior to checking type
- _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
- if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
- }
- if !exists {
- userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
- return
+ if len(mssc.ServerIDs) >= 1 {
+ errCode, userErr, sysErr = checkExistingServer(tx,
mssc.ServerIDs, inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
}
// Ensure type is correct
- correctType := true
- if err := tx.QueryRow(scCheckServerTypeQuery(),
msc.ServerID).Scan(); err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
fmt.Errorf("checking server type: %w", err))
+ errCode, userErr, sysErr = checkServerType(tx, mssc.ServerIDs)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
- if !correctType {
- userErr := fmt.Errorf("server %d has an incorrect server type.
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
- return
+
+ // Insert rows in DB
+ sid := make([]int64, len(mssc.ServerCapabilities))
+ scs := make([]string, len(mssc.ServerIDs))
+ if len(mssc.ServerIDs) == 1 {
+ if len(mssc.ServerCapabilities) >= 1 {
+ for i := range mssc.ServerCapabilities {
+ sid[i] = mssc.ServerIDs[0]
+ }
+ scs = mssc.ServerCapabilities
+ }
+ } else if len(mssc.ServerCapabilities) == 1 {
+ if len(mssc.ServerIDs) >= 1 {
+ for i := range mssc.ServerIDs {
+ scs[i] = mssc.ServerCapabilities[0]
+ }
+ sid = mssc.ServerIDs
+ }
+ } else {
+ scs = mssc.ServerCapabilities
+ sid = mssc.ServerIDs
}
- cdnName, err := dbhelpers.GetCDNNameFromServerID(tx,
int64(msc.ServerID))
+ msscQuery := `INSERT INTO server_server_capability
+ select "server_capability", "server"
+ FROM UNNEST($1::text[], $2::int[]) AS
tmp("server_capability", "server")`
+ _, err := tx.Query(msscQuery, pq.Array(scs), pq.Array(sid))
if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
+ useErr, sysErr, statusCode := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
return
}
- userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx,
string(cdnName), inf.User.UserName)
+ var alerts tc.Alerts
+ if mssc.PageType == "sc" {
+ alerts = tc.CreateAlerts(tc.SuccessLevel, "Assign Server(s) to
a capability")
+ } else {
+ alerts = tc.CreateAlerts(tc.SuccessLevel, "Assign Server
Capability(ies) to a server")
+ }
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, mssc)
+ return
+}
+
+// DeleteMultipleServersCapabilities deletes multiple servers to a capability
or multiple server capabilities to a server
+func DeleteMultipleServersCapabilities(w http.ResponseWriter, r *http.Request)
{
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx.Tx
if userErr != nil || sysErr != nil {
- api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
+ defer inf.Close()
- //Delete existing rows from server_server_capability for a given server
-
[GitHub] [trafficcontrol] traeak closed pull request #7090: t3c/parentdotconfig: enforce mso.parent_retry ds parameter
traeak closed pull request #7090: t3c/parentdotconfig: enforce mso.parent_retry ds parameter URL: https://github.com/apache/trafficcontrol/pull/7090 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] traeak commented on pull request #7090: t3c/parentdotconfig: enforce mso.parent_retry ds parameter
traeak commented on PR #7090: URL: https://github.com/apache/trafficcontrol/pull/7090#issuecomment-1274942317 opted to just document the peculiarities. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] traeak opened a new pull request, #7120: For parent_retry docs explain parent_retry deprecation
traeak opened a new pull request, #7120: URL: https://github.com/apache/trafficcontrol/pull/7120 The t3c logic currently ignores the `parent_retry` ds parameter and instead examines the use of parameters - max_simple_retries - simple_server_retry_responses - max_unavailable_server_retries - unavailable_server_retry_responses in order to infer what value to use for parent_retry in parent.config Setting either of the above pairs to '0', '""' will disable, otherwise default is always "both". ## Which Traffic Control components are affected by this PR? - Documentation ## What is the best way to verify this PR? ## If this is a bugfix, which Traffic Control versions contained the bug? ## PR submission checklist - [ ] This PR has tests -- documentation only - [x] This PR has documentation - [x] This PR has a CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] Xuanteo opened a new issue, #7119: Deploy project on docker swarm environment get error
Xuanteo opened a new issue, #7119: URL: https://github.com/apache/trafficcontrol/issues/7119 ## This Bug Report affects these Traffic Control components: - Traffic Control Cache Config (`t3c`, formerly ORT) - Traffic Control Health Client (t3c-health-client) - Traffic Control Client - Traffic Monitor - Traffic Ops - Traffic Portal - Traffic Router - Traffic Stats - Grove - Documentation - CDN in a Box - Automation - unknown ## Current behavior: When deploying the project on the network as a bridge, the project sandbox runs ok. but when deploying it on overlay network, it fails because sync ssl between ops => monitor => enroller => edge components fail. have many module don't communicate. check docker logs only see status: waiting for SSL keys to exist at 108 line in /cache/run.sh file ## Expected behavior: project can support running on overlay network - docker swarm ## Steps to reproduce: -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] srijeet0406 opened a new pull request, #7118: Remove ASN from servers
srijeet0406 opened a new pull request, #7118: URL: https://github.com/apache/trafficcontrol/pull/7118 This PR is not related to any issue. It removes the previously added (in https://github.com/apache/trafficcontrol/pull/7023) `asns` field from the `server` struct. ## Which Traffic Control components are affected by this PR? - Documentation - Traffic Control Client - Traffic Ops - Traffic Portal - CDN in a Box ## What is the best way to verify this PR? Make sure you don't see the `asns` field in `GET` calls that return the `server` object. Make sure all tests pass. ## If this is a bugfix, which Traffic Control versions contained the bug? ## PR submission checklist - [x] This PR has tests - [x] This PR has documentation - [x] This PR has a CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] TaylorCFrey commented on a diff in pull request #7050: Add configuration setting for TO client request timeout
TaylorCFrey commented on code in PR #7050:
URL: https://github.com/apache/trafficcontrol/pull/7050#discussion_r991675941
##
infrastructure/ansible/roles/traffic_stats/templates/traffic_stats.cfg.j2:
##
@@ -15,6 +15,7 @@
"toUser": "{{ ts_toUser }}",
"toPasswd": "{{ ts_toPasswd }}",
"toUrl": "{{ ts_toUrl }}",
+"toRequestTimeout": "{{ ts_toRequestTimeout }}",
Review Comment:
I don't believe so. It follows the same formatting as all of the previous
fields. Unless I'm not seeing it correctly...
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman merged pull request #7117: Remove unused TR profile params from docs
zrhoffman merged PR #7117: URL: https://github.com/apache/trafficcontrol/pull/7117 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] rawlinp opened a new pull request, #7117: Remove unused TR profile params from docs
rawlinp opened a new pull request, #7117: URL: https://github.com/apache/trafficcontrol/pull/7117 These "TR profile parameters" do not actually exist. ## Which Traffic Control components are affected by this PR? - Documentation ## What is the best way to verify this PR? Build and view the resulting docs ## PR submission checklist - [x] docs, no tests needed - [x] This PR has documentation - [x] docs, no CHANGELOG.md entry needed - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on pull request #7079: Assign multiple servers to a capability
rimashah25 commented on PR #7079: URL: https://github.com/apache/trafficcontrol/pull/7079#issuecomment-1273536182 > Sorry just seeing this, where are those checks? I usually do a POST followed by a GET to make sure whether or not the operation actually went through to the database or not. The way new integration tests are written, one doesn't need do 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability
srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r991409061
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +454,166 @@ func AssignMultipleServerCapabilities(w
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
- var msc tc.MultipleServerCapabilities
- if err := json.NewDecoder(r.Body).Decode(); err != nil {
- api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ var mssc tc.MultipleServersCapabilities
+ if err := json.NewDecoder(r.Body).Decode(); err != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest,
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities
struct %w", err), nil)
return
}
- // Check existence prior to checking type
- _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
- if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
- }
- if !exists {
- userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
- return
+ if len(mssc.ServerIDs) == 1 {
+ errCode, userErr, sysErr = checkExistingServer(tx,
mssc.ServerIDs[0], inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
}
- // Ensure type is correct
- correctType := true
- if err := tx.QueryRow(scCheckServerTypeQuery(),
msc.ServerID).Scan(); err != nil {
+ //Check if the server type is MID and/or EDGE
+ var servArray []int64
+ queryType := `SELECT array_agg(s.id)
+ FROM server s
+ JOIN type t ON s.type = t.id
+ WHERE s.id = any ($1)
+ AND t.use_in_table = 'server'
+ AND (t.name LIKE 'MID%' OR t.name LIKE 'EDGE%')`
+ if err := tx.QueryRow(queryType,
pq.Array(mssc.ServerIDs)).Scan(pq.Array()); err != nil {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
fmt.Errorf("checking server type: %w", err))
return
}
- if !correctType {
- userErr := fmt.Errorf("server %d has an incorrect server type.
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
- return
+ cmp := make(map[int64]bool)
+ for _, item := range servArray {
+ cmp[item] = true
+ }
+ for _, sid := range mssc.ServerIDs {
+ if _, ok := cmp[sid]; !ok {
+ userErr := fmt.Errorf("server id: %d has an incorrect
server type. Server capability can only be assigned to EDGE or MID servers",
sid)
+ api.HandleErr(w, r, tx, http.StatusBadRequest, userErr,
nil)
+ return
+ }
}
- cdnName, err := dbhelpers.GetCDNNameFromServerID(tx,
int64(msc.ServerID))
+ // Insert rows in DB
+ sid := make([]int64, len(mssc.ServerCapabilities))
+ scs := make([]string, len(mssc.ServerIDs))
+ if len(mssc.ServerIDs) == 1 {
+ if len(mssc.ServerCapabilities) >= 1 {
+ for i := range mssc.ServerCapabilities {
+ sid[i] = mssc.ServerIDs[0]
+ }
+ scs = mssc.ServerCapabilities
+ }
+ } else if len(mssc.ServerCapabilities) == 1 {
+ if len(mssc.ServerIDs) >= 1 {
+ for i := range mssc.ServerIDs {
+ scs[i] = mssc.ServerCapabilities[0]
+ }
+ sid = mssc.ServerIDs
+ }
+ } else {
+ scs = mssc.ServerCapabilities
+ sid = mssc.ServerIDs
+ }
+
+ msscQuery := `INSERT INTO server_server_capability
+ select "server_capability", "server"
+ FROM UNNEST($1::text[], $2::int[]) AS
tmp("server_capability", "server")`
+ _, err := tx.Query(msscQuery, pq.Array(scs), pq.Array(sid))
if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
+ useErr, sysErr, statusCode := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
return
}
- userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx,
string(cdnName), inf.User.UserName)
+ var alerts tc.Alerts
+ if len(mssc.ServerCapabilities) == 1 && len(mssc.ServerIDs) == 1 {
Review Comment:
But that's what I mean
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7079: Assign multiple servers to a capability
rimashah25 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r991381409
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +454,166 @@ func AssignMultipleServerCapabilities(w
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
- var msc tc.MultipleServerCapabilities
- if err := json.NewDecoder(r.Body).Decode(); err != nil {
- api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ var mssc tc.MultipleServersCapabilities
+ if err := json.NewDecoder(r.Body).Decode(); err != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest,
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities
struct %w", err), nil)
return
}
- // Check existence prior to checking type
- _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
- if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
- }
- if !exists {
- userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
- return
+ if len(mssc.ServerIDs) == 1 {
+ errCode, userErr, sysErr = checkExistingServer(tx,
mssc.ServerIDs[0], inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
}
- // Ensure type is correct
- correctType := true
- if err := tx.QueryRow(scCheckServerTypeQuery(),
msc.ServerID).Scan(); err != nil {
+ //Check if the server type is MID and/or EDGE
+ var servArray []int64
+ queryType := `SELECT array_agg(s.id)
+ FROM server s
+ JOIN type t ON s.type = t.id
+ WHERE s.id = any ($1)
+ AND t.use_in_table = 'server'
+ AND (t.name LIKE 'MID%' OR t.name LIKE 'EDGE%')`
+ if err := tx.QueryRow(queryType,
pq.Array(mssc.ServerIDs)).Scan(pq.Array()); err != nil {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
fmt.Errorf("checking server type: %w", err))
return
}
- if !correctType {
- userErr := fmt.Errorf("server %d has an incorrect server type.
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
- return
+ cmp := make(map[int64]bool)
+ for _, item := range servArray {
+ cmp[item] = true
+ }
+ for _, sid := range mssc.ServerIDs {
+ if _, ok := cmp[sid]; !ok {
+ userErr := fmt.Errorf("server id: %d has an incorrect
server type. Server capability can only be assigned to EDGE or MID servers",
sid)
+ api.HandleErr(w, r, tx, http.StatusBadRequest, userErr,
nil)
+ return
+ }
}
- cdnName, err := dbhelpers.GetCDNNameFromServerID(tx,
int64(msc.ServerID))
+ // Insert rows in DB
+ sid := make([]int64, len(mssc.ServerCapabilities))
+ scs := make([]string, len(mssc.ServerIDs))
+ if len(mssc.ServerIDs) == 1 {
+ if len(mssc.ServerCapabilities) >= 1 {
+ for i := range mssc.ServerCapabilities {
+ sid[i] = mssc.ServerIDs[0]
+ }
+ scs = mssc.ServerCapabilities
+ }
+ } else if len(mssc.ServerCapabilities) == 1 {
+ if len(mssc.ServerIDs) >= 1 {
+ for i := range mssc.ServerIDs {
+ scs[i] = mssc.ServerCapabilities[0]
+ }
+ sid = mssc.ServerIDs
+ }
+ } else {
+ scs = mssc.ServerCapabilities
+ sid = mssc.ServerIDs
+ }
+
+ msscQuery := `INSERT INTO server_server_capability
+ select "server_capability", "server"
+ FROM UNNEST($1::text[], $2::int[]) AS
tmp("server_capability", "server")`
+ _, err := tx.Query(msscQuery, pq.Array(scs), pq.Array(sid))
if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
+ useErr, sysErr, statusCode := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
return
}
- userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx,
string(cdnName), inf.User.UserName)
+ var alerts tc.Alerts
+ if len(mssc.ServerCapabilities) == 1 && len(mssc.ServerIDs) == 1 {
Review Comment:
Not really, since you
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7079: Assign multiple servers to a capability
rimashah25 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r991379510
##
docs/source/api/v4/multiple_servers_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-v4-multiple_servers_capabilities:
+
+*
+``multiple_servers_capabilities``
+*
+
+.. versionadded:: 4.1
+
+``POST``
+
+Associates a list of :term:`Server Capability` to a server. The API call
replaces all the server capabilities assigned to a server with the ones
specified in the serverCapabilities field.
+And also Associates a list of :term:`Servers` to a server capability. The API
call replaces all the servers assigned to a server capability with the ones
specified in the servers field.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Permissions Required: SERVER:READ, SERVER:CREATE, SERVER-CAPABILITY:READ,
SERVER-CAPABILITY:CREATE
+:Response Type: Object
+
+Request Structure
+-
+:serverIds: List of :term:`Server` ids ((integral, unique identifier)
associated with a :term:`Server Capability`
+:serverCapabilities: List of :term:`Server Capability`'s name to associate
with a :term:`Server` id
+
+
+.. code-block:: http
+ :caption: Request Example1
+
+ POST /api/4.1/multiple_servers_capabilities/ HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+ Content-Length: 84
+ Content-Type: application/json
+
+ {
+ "serverIds": [1],
+ "serverCapabilities": ["test", "disk"]
+ }
+
+.. code-block:: http
+ :caption: Request Example2
+
+ POST /api/4.1/multiple_servers_capabilities/ HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+ Content-Length: 84
+ Content-Type: application/json
+
+ {
+ "serverIds": [2, 3]
+ "serverCapabilities": ["eas"],
+ }
+
+Response Structure
+--
+:serverId: List of :term:`Server` ids ((integral, unique identifier)
associated with a server capability.
+:serverCapabilities: List of :term:`Server Capability`'s name to be associated
with a :term:`Server` id.
+
+.. code-block:: http
Review Comment:
Not really.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7079: Assign multiple servers to a capability
rimashah25 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r991379034
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -443,8 +444,8 @@ func getDSTenantIDsByIDs(tx *sqlx.Tx, dsIDs []int64)
([]DSTenant, error) {
return dsTenantIDs, nil
}
-// AssignMultipleServerCapabilities helps assign multiple server capabilities
to a given server.
-func AssignMultipleServerCapabilities(w http.ResponseWriter, r *http.Request) {
+// AssignMultipleServersCapabilities assigns multiple servers to a capability
or multiple server capabilities to a server
+func AssignMultipleServersCapabilities(w http.ResponseWriter, r *http.Request)
{
Review Comment:
It uses locking mechanism `CheckIfCurrentUserCanModifyCDN()`
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] jhg03a commented on issue #2388: Global DNS TTL adjustment
jhg03a commented on issue #2388: URL: https://github.com/apache/trafficcontrol/issues/2388#issuecomment-1272605234 It probably wouldn't need to touch the actual ds, but rather just interpose itself as a higher priority value. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] mitchell852 commented on issue #2388: Global DNS TTL adjustment
mitchell852 commented on issue #2388: URL: https://github.com/apache/trafficcontrol/issues/2388#issuecomment-1272604428 @jhg03a - what exactly does this sql query look like? update the ttl on all delivery services to X? if that was performed how would you revert 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] villajo closed pull request #6803: Documentation fixes
villajo closed pull request #6803: Documentation fixes URL: https://github.com/apache/trafficcontrol/pull/6803 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman merged pull request #7106: Refactor Server ID Status Tests
zrhoffman merged PR #7106: URL: https://github.com/apache/trafficcontrol/pull/7106 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman merged pull request #7109: Remove dnssec optimization flags from Traffic Router
zrhoffman merged PR #7109: URL: https://github.com/apache/trafficcontrol/pull/7109 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] srijeet0406 merged pull request #7116: Use Topology type parameter for Topology API TestCase
srijeet0406 merged PR #7116: URL: https://github.com/apache/trafficcontrol/pull/7116 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7116: Use Topology type parameter for Topology API TestCase
zrhoffman commented on code in PR #7116:
URL: https://github.com/apache/trafficcontrol/pull/7116#discussion_r990491005
##
traffic_ops/testing/api/utils/utils.go:
##
@@ -134,10 +144,31 @@ type V5TestData struct {
Expectations []CkReqFunc
}
+type clientSession interface {
+ v3client.Session | v4client.Session | v5client.Session
Review Comment:
True, removed in 49a7d55de0
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ericholguin commented on a diff in pull request #7116: Use Topology type parameter for Topology API TestCase
ericholguin commented on code in PR #7116:
URL: https://github.com/apache/trafficcontrol/pull/7116#discussion_r990485503
##
traffic_ops/testing/api/utils/utils.go:
##
@@ -134,10 +144,31 @@ type V5TestData struct {
Expectations []CkReqFunc
}
+type clientSession interface {
+ v3client.Session | v4client.Session | v5client.Session
Review Comment:
Can probably remove `v3client.Session` since not in use by v3?
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman opened a new pull request, #7116: Use Topology type parameter for Topology API TestCase
zrhoffman opened a new pull request, #7116: URL: https://github.com/apache/trafficcontrol/pull/7116 This PR uses type parameters to introduce struct type safety into the API test cases where, before Go 1.18, interfaces would need to be used. ## Which Traffic Control components are affected by this PR? - Traffic Ops - tests ## What is the best way to verify this PR? Run API tests, check godocs ## PR submission checklist - [x] This PR has tests - [ ] This PR has documentation - [ ] This PR has a CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman opened a new pull request, #7115: Update TO v5 API tests `TestAPIBase` to API v5
zrhoffman opened a new pull request, #7115: URL: https://github.com/apache/trafficcontrol/pull/7115 The `TestAPIBase` constant in the Traffic Ops v5 API tests references TO API v4: https://github.com/apache/trafficcontrol/blob/13e78e0755c79e13f03688477726fee1b152148d/traffic_ops/testing/api/v4/traffic_ops_test.go#L38 This PR updates it to use TO API v5. ## Which Traffic Control components are affected by this PR? - Traffic Ops - API tests ## What is the best way to verify this PR? Run the v5 API tests, ensure they pass ## If this is a bugfix, which Traffic Control versions contained the bug? - master branch ## PR submission checklist - [x] This PR has tests - [x] Bugfix, no documentation necessary - [x] No CHANGELOG.md entry necessary - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability
srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r990344181
##
docs/source/api/v4/multiple_servers_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-v4-multiple_servers_capabilities:
+
+*
+``multiple_servers_capabilities``
+*
+
+.. versionadded:: 4.1
+
+``POST``
+
+Associates a list of :term:`Server Capability` to a server. The API call
replaces all the server capabilities assigned to a server with the ones
specified in the serverCapabilities field.
+And also Associates a list of :term:`Servers` to a server capability. The API
call replaces all the servers assigned to a server capability with the ones
specified in the servers field.
Review Comment:
Instead of repitition, could we reframe this sentence as:
Associates a list of server capabilities to a server, and vice versa. This
API call replaces all the ..., and vice versa.
##
docs/source/api/v4/multiple_servers_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-v4-multiple_servers_capabilities:
+
+*
+``multiple_servers_capabilities``
+*
+
+.. versionadded:: 4.1
+
+``POST``
+
+Associates a list of :term:`Server Capability` to a server. The API call
replaces all the server capabilities assigned to a server with the ones
specified in the serverCapabilities field.
+And also Associates a list of :term:`Servers` to a server capability. The API
call replaces all the servers assigned to a server capability with the ones
specified in the servers field.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Permissions Required: SERVER:READ, SERVER:CREATE, SERVER-CAPABILITY:READ,
SERVER-CAPABILITY:CREATE
+:Response Type: Object
+
+Request Structure
+-
+:serverIds: List of :term:`Server` ids ((integral, unique identifier)
associated with a :term:`Server Capability`
+:serverCapabilities: List of :term:`Server Capability`'s name to associate
with a :term:`Server` id
+
+
+.. code-block:: http
+ :caption: Request Example1
+
+ POST /api/4.1/multiple_servers_capabilities/ HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+ Content-Length: 84
+ Content-Type: application/json
+
+ {
+ "serverIds": [1],
+ "serverCapabilities": ["test", "disk"]
+ }
+
+.. code-block:: http
+ :caption: Request Example2
+
+ POST /api/4.1/multiple_servers_capabilities/ HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+ Content-Length: 84
+ Content-Type: application/json
+
+ {
+ "serverIds": [2, 3]
+ "serverCapabilities": ["eas"],
+ }
+
+Response Structure
+--
+:serverId: List of :term:`Server` ids ((integral, unique identifier)
associated with a server capability.
+:serverCapabilities: List of :term:`Server Capability`'s name to be associated
with a :term:`Server` id.
Review Comment:
same as above re: `names`
##
docs/source/api/v4/multiple_servers_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,
[GitHub] [trafficcontrol] ericholguin opened a new pull request, #7114: Refactor Snapshot Tests
ericholguin opened a new pull request, #7114:
URL: https://github.com/apache/trafficcontrol/pull/7114
This PR refactors **v5/snapshot_test.go**, **v4/snapshot_test.go**,
**v3/snapshot_test.go**, in order to make tests more granular, providing
transparency on what is being tested and reducing the need to dig through test
code to confirm what was tested. In addition this new test structure should
also make it easier for new tests to be added as well as reduce redundant code.
- Adds data driven testing
- Descriptive test names
- Sub tests for organization
- Use GoLangs test runner
- Use assert functionality
- Provides fundamental assertions
- Streamlines code
- Reduces nested conditionals
- Adds expectation functions
- Test specific expectations in a clear concise way
- Reusable expectations
crconfig was renamed to snapshot to match the actual endpoint name
Tests that are their own endpoint have taken out into their own test files:
**v{x}/cdns_name_config_monitoring_test.go**
**v{x}/cdns_name_snapshot_new_test.go**
**v{x}/cdns_name_snapshot_test.go**
## Which Traffic Control components are affected by this PR?
- Traffic Ops Tests
## What is the best way to verify this PR?
Run TO Integration Tests
## PR submission checklist
- [x] This PR has tests
- [ ] This PR has documentation
- [ ] This PR has a CHANGELOG.md entry
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the
Apache Software Foundation's security guidelines](https://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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman opened a new issue, #7113: Assigning a new Server Server capability changes SSC order for that Server
zrhoffman opened a new issue, #7113: URL: https://github.com/apache/trafficcontrol/issues/7113 ## This Improvement request (usability, performance, tech debt, etc.) affects these Traffic Control components: - Traffic Ops - Traffic Portal ## Steps taken 1. Create Server Capability *My_Capability* 2. Assign Server Capability *My_Capability* to Server *edge* 3. Snapshot *dev* CDN 4. Create Server Capability *Another_Capability* 5. Look at snapshot diff for *dev CDN* ## Actual result Snapshot diff shows that Server Server Capability *My_Capability* for Server *edge* was added ## Expected behavior: * Snapshot diff shows that Server Server Capability *My_Capability* is added to server *edge* at index 1 * Snapshot diff shows that Server Server Capability *My_Capability* is updated to *Another_Capability* at index 0 ## Also Possibly fixed by #7079. @rimashah25 would know for sure -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #7045: Run tests against Apache Traffic Server 9.1
zrhoffman commented on PR #7045: URL: https://github.com/apache/trafficcontrol/pull/7045#issuecomment-127178 Rebased to fix a changelog merge conflict -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 closed issue #6947: Documentation for cdns/{{name}}/federations is missing a property
ocket closed issue #6947: Documentation for cdns/{{name}}/federations is
missing a property
URL: https://github.com/apache/trafficcontrol/issues/6947
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7112: fix : Documentation for cdns/{{name}}/federations is missing a property
ocket merged PR #7112: URL: https://github.com/apache/trafficcontrol/pull/7112 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] everly-gif commented on issue #6947: Documentation for cdns/{{name}}/federations is missing a property
everly-gif commented on issue #6947: URL: https://github.com/apache/trafficcontrol/issues/6947#issuecomment-1271599467 Made PR #7112 please have a look. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] everly-gif opened a new pull request, #7112: fix : Documentation for cdns/{{name}}/federations is missing a property
everly-gif opened a new pull request, #7112: URL: https://github.com/apache/trafficcontrol/pull/7112 ### Closes: #6947 ## Which Traffic Control components are affected by this PR? - Documentation ## What is the best way to verify this PR? Go to below links and verify - `https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/cdns_name_federations.html#id2` - `https://traffic-control-cdn.readthedocs.io/en/latest/api/v4/cdns_name_federations.html#id2` - `https://traffic-control-cdn.readthedocs.io/en/latest/api/v3/cdns_name_federations.html#id2` ## If this is a bugfix, which Traffic Control versions contained the bug? API docs v3, v4 and v5 ## PR submission checklist - [ ] This PR has tests - minor change in documentation - [ ] This PR has documentation - this PR is a minor documentation change - [x] This PR has a CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] everly-gif commented on issue #6947: Documentation for cdns/{{name}}/federations is missing a property
everly-gif commented on issue #6947: URL: https://github.com/apache/trafficcontrol/issues/6947#issuecomment-1271511447 Hi, I would like to work on this issue, please assign it to me. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on pull request #7099: Delivery Service Active Flag Rework
rimashah25 commented on PR #7099: URL: https://github.com/apache/trafficcontrol/pull/7099#issuecomment-1270847030 Tested the API tests locally, all work (v3/v4/v5) fine. Yet to test DS DSRs creation with API v5 curl cmds. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #7104: TP fix child DS tables not loading
shamrickus commented on code in PR #7104: URL: https://github.com/apache/trafficcontrol/pull/7104#discussion_r989386984 ## traffic_portal/app/src/common/modules/table/serviceCategoryDeliveryServices/table.serviceCategoryDeliveryServices.tpl.html: ## @@ -26,7 +26,6 @@ columns="columns" drop-down-options="dropDownOptions" context-menu-options="contextMenuOptions" - sensitive-columns="sensitiveColumns" Review Comment: `sensitiveColumns`is not defined on these child tables or on the parent `TableDeliveryServiceController` -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7109: Remove dnssec optimization flags from Traffic Router
zrhoffman commented on code in PR #7109:
URL: https://github.com/apache/trafficcontrol/pull/7109#discussion_r989385185
##
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/ZoneManager.java:
##
@@ -527,7 +527,7 @@ private static void primeZoneCache(final String domain,
final Name name, final L
}
}
final Zone zone = zc.get(newZoneKey); // cause
the zone to be loaded into the new cache
- if (tr.isDnssecZoneDiffingEnabled()) {
+ if (tr.isDnssecEnabled()) {
Review Comment:
Same here, this part can just be removed
##
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/ZoneManager.java:
##
@@ -511,7 +511,7 @@ private static void primeZoneCache(final String domain,
final Name name, final L
generationTasks.add(() -> {
try {
final ZoneKey newZoneKey =
signatureManager.generateZoneKey(name, list);
- if (tr.isDnssecZoneDiffingEnabled() &&
domainsToZoneKeys.containsKey(domain)) {
Review Comment:
The RRSIG cache makes zone diffing obsolete, so you could just remove this
part
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] shamrickus merged pull request #7102: Tpv2 server fix
shamrickus merged PR #7102: URL: https://github.com/apache/trafficcontrol/pull/7102 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on pull request #7102: Tpv2 server fix
shamrickus commented on PR #7102: URL: https://github.com/apache/trafficcontrol/pull/7102#issuecomment-1270546381 In that case we should make it do something other than `npm ci` (which also blows away node_modules on the host system I might add), but that's not relevant to this 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7068: Renamed rascal to tm/traffic_monitor
rimashah25 commented on code in PR #7068: URL: https://github.com/apache/trafficcontrol/pull/7068#discussion_r989378882 ## traffic_ops/traffic_ops_golang/deliveryservice/capacity.go: ## @@ -249,7 +249,7 @@ JOIN server as s ON s.profile = pr.id JOIN cdn as c ON c.id = s.cdn_id JOIN type as t ON s.type = t.id WHERE t.name LIKE 'EDGE%' -AND pa.config_file = 'rascal-config.txt' +AND pa.config_file = 'tm-config.txt' Review Comment: Included migration file. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7068: Renamed rascal to tm/traffic_monitor
rimashah25 commented on code in PR #7068:
URL: https://github.com/apache/trafficcontrol/pull/7068#discussion_r989371625
##
traffic_ops/traffic_ops_golang/server/servers.go:
##
@@ -856,6 +856,12 @@ func getServers(h http.Header, params map[string]string,
tx *sqlx.Tx, user *auth
`
}
+ if version.Major >= 5 {
+ if params["type"] == "RASCAL" {
+ params["type"] = "TRAFFIC_MONITOR"
+ }
+ }
+
Review Comment:
I see your point but how do I maintain backward compatibility in terms of
T3C?
This change was done to ensure T3c clients and TS that monitors servers and
ask for servers with type rascal, TO sends the servers with type TM.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 closed issue #5782: There is a vulnerability in log4j 1.2.17 ,upgrade recommended
rimashah25 closed issue #5782: There is a vulnerability in log4j 1.2.17 ,upgrade recommended URL: https://github.com/apache/trafficcontrol/issues/5782 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on issue #5782: There is a vulnerability in log4j 1.2.17 ,upgrade recommended
rimashah25 commented on issue #5782: URL: https://github.com/apache/trafficcontrol/issues/5782#issuecomment-1270503537 We have updated log4j to 2.17.1 (PR: #6390, #6438, #6445) -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989325834 ## CHANGELOG.md: ## @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#2101](https://github.com/apache/trafficcontrol/issues/2101) *Traffic Portal* Added the ability to tell if a Delivery Service is the target of another steering DS. - [#6033](https://github.com/apache/trafficcontrol/issues/6033) *Traffic Ops, Traffic Portal* Added ability to assign multiple server capabilities to a server. - [#7032](https://github.com/apache/trafficcontrol/issues/7032) *Cache Config* Add t3c-apply flag to use local ATS version for config generation rather than Server package Parameter, to allow managing the ATS OS package via external tools. See 'man t3c-apply' and 'man t3c-generate' for details. +- [#7097](https://github.com/apache/trafficcontrol/issues/7097) *Traffic Ops, Traffic Portal* Added the `regional` field to Traffic Ops and Traffic Portal, which indicates whether `maxOriginConnections` should be per Cache Group Review Comment: Oh, right. Said *Delivery Service* in 8f0c2cda29 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989324166 ## traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.DNS.tpl.html: ## @@ -475,6 +475,26 @@ Previous Value + +Regional Max Origin Connections * + +If your Delivery Service is Regional, then you only expect it to be used by a single Cache Group. Enable this to make Max Origin Connections per Cache Group instead of a value used over all last-tier Cache Groups. https://traffic-control-cdn.readthedocs.io/en/latest/overview/delivery_services.html#regional; target="_blank">See Regional Delivery Services + + + + + +Enabled +Disabled + Review Comment: Changed to a checkbox in 0aad7be6ff -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111:
URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989319187
##
traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql:
##
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+ALTER TABLE public.deliveryservice
+ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE;
+
+/* Set `regional` to `false` if it does not exist */
+UPDATE public.deliveryservice_request
+SET deliveryservice = deliveryservice || '{"regional": false}'
+WHERE deliveryservice->>'regional' IS NULL;
+
+/* Set `regional` to `false` it does not exist and `original` is not null */
+UPDATE public.deliveryservice_request
+SET original = original || '{"regional": false}'
+WHERE original IS NOT NULL
Review Comment:
Neat, removed from both in 8a099c0af0
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
ocket commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989294749 ## CHANGELOG.md: ## @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#2101](https://github.com/apache/trafficcontrol/issues/2101) *Traffic Portal* Added the ability to tell if a Delivery Service is the target of another steering DS. - [#6033](https://github.com/apache/trafficcontrol/issues/6033) *Traffic Ops, Traffic Portal* Added ability to assign multiple server capabilities to a server. - [#7032](https://github.com/apache/trafficcontrol/issues/7032) *Cache Config* Add t3c-apply flag to use local ATS version for config generation rather than Server package Parameter, to allow managing the ATS OS package via external tools. See 'man t3c-apply' and 'man t3c-generate' for details. +- [#7097](https://github.com/apache/trafficcontrol/issues/7097) *Traffic Ops, Traffic Portal* Added the `regional` field to Traffic Ops and Traffic Portal, which indicates whether `maxOriginConnections` should be per Cache Group Review Comment: lol no I meant "to what object has a field been added?" Delivery Services, that is. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
ocket commented on code in PR #7111:
URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989293452
##
traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql:
##
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+ALTER TABLE public.deliveryservice
+ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE;
+
+/* Set `regional` to `false` if it does not exist */
+UPDATE public.deliveryservice_request
+SET deliveryservice = deliveryservice || '{"regional": false}'
+WHERE deliveryservice->>'regional' IS NULL;
+
+/* Set `regional` to `false` it does not exist and `original` is not null */
+UPDATE public.deliveryservice_request
+SET original = original || '{"regional": false}'
+WHERE original IS NOT NULL
Review Comment:
Well, as it happens, the check is unnecessary because apparently `||` can
operate on `NULL` `jsonb` values. `deliveryservice` will be `NULL` when a DSR
is created to DELETE a DS, fyi.
You can either do the check or not, but it should be done (or not)
consistently either way.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
ocket commented on code in PR #7111:
URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989293452
##
traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql:
##
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+ALTER TABLE public.deliveryservice
+ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE;
+
+/* Set `regional` to `false` if it does not exist */
+UPDATE public.deliveryservice_request
+SET deliveryservice = deliveryservice || '{"regional": false}'
+WHERE deliveryservice->>'regional' IS NULL;
+
+/* Set `regional` to `false` it does not exist and `original` is not null */
+UPDATE public.deliveryservice_request
+SET original = original || '{"regional": false}'
+WHERE original IS NOT NULL
Review Comment:
Well, as it happens, the check is unnecessary because apparently `||` can
operate on `NULL` `jsonb` values. `deliveryservice` will be `NULL~ when a DSR
is created to DELETE a DS, fyi.
You can either do the check or not, but it should be done (or not)
consistently either way.
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989277863 ## docs/source/overview/delivery_services.rst: ## @@ -505,6 +505,9 @@ Max Origin Connections -- The maximum number of TCP connections individual :term:`Mid-tier cache servers` are allowed to make to the `Origin Server Base URL`. A value of ``0`` in this field indicates that there is no maximum. + + .. note:: Max Origin Connections can be made per-:ref:`Cache Group ` by setting the :ref:`ds-regional` field. Review Comment: Unindented in 1640dc9ec3 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111:
URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989260025
##
traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql:
##
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+ALTER TABLE public.deliveryservice
+ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE;
+
+/* Set `regional` to `false` if it does not exist */
+UPDATE public.deliveryservice_request
+SET deliveryservice = deliveryservice || '{"regional": false}'
+WHERE deliveryservice->>'regional' IS NULL;
+
+/* Set `regional` to `false` it does not exist and `original` is not null */
+UPDATE public.deliveryservice_request
+SET original = original || '{"regional": false}'
+WHERE original IS NOT NULL
Review Comment:
When creating a DSR to POST a Delivery Service in Traffic Portal, the
`deliveryservice` field is set but the `original` field is `NULL`. You're right
that the `deliveryservice` field is nullable, too, though. Added a null check
for the `deliveryservice` field in 25d28d426a
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989252029 ## CHANGELOG.md: ## @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#2101](https://github.com/apache/trafficcontrol/issues/2101) *Traffic Portal* Added the ability to tell if a Delivery Service is the target of another steering DS. - [#6033](https://github.com/apache/trafficcontrol/issues/6033) *Traffic Ops, Traffic Portal* Added ability to assign multiple server capabilities to a server. - [#7032](https://github.com/apache/trafficcontrol/issues/7032) *Cache Config* Add t3c-apply flag to use local ATS version for config generation rather than Server package Parameter, to allow managing the ATS OS package via external tools. See 'man t3c-apply' and 'man t3c-generate' for details. +- [#7097](https://github.com/apache/trafficcontrol/issues/7097) *Traffic Ops* Added the `regional` field, which indicates whether `maxOriginConnections` should be per Cache Group Review Comment: Clarified what it was added to in f7903f022e -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
ocket commented on code in PR #7111:
URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989135302
##
CHANGELOG.md:
##
@@ -12,6 +12,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- [#2101](https://github.com/apache/trafficcontrol/issues/2101) *Traffic
Portal* Added the ability to tell if a Delivery Service is the target of
another steering DS.
- [#6033](https://github.com/apache/trafficcontrol/issues/6033) *Traffic Ops,
Traffic Portal* Added ability to assign multiple server capabilities to a
server.
- [#7032](https://github.com/apache/trafficcontrol/issues/7032) *Cache Config*
Add t3c-apply flag to use local ATS version for config generation rather than
Server package Parameter, to allow managing the ATS OS package via external
tools. See 'man t3c-apply' and 'man t3c-generate' for details.
+- [#7097](https://github.com/apache/trafficcontrol/issues/7097) *Traffic Ops*
Added the `regional` field, which indicates whether `maxOriginConnections`
should be per Cache Group
Review Comment:
> Added the `regional` field, which ...
Added to what?
##
docs/source/overview/delivery_services.rst:
##
@@ -505,6 +505,9 @@ Max Origin Connections
--
The maximum number of TCP connections individual :term:`Mid-tier cache
servers` are allowed to make to the `Origin Server Base URL`. A value of ``0``
in this field indicates that there is no maximum.
+
+ .. note:: Max Origin Connections can be made per-:ref:`Cache
Group ` by setting the :ref:`ds-regional` field.
Review Comment:
indentation here is causing this note to be rendered as a block quote - it's
not a quote, so it shouldn't be indented.
##
traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.DNS.tpl.html:
##
@@ -475,6 +475,26 @@ Previous Value
+
+Regional Max Origin Connections *
+
+If your Delivery Service is Regional, then you
only expect it to be used by a single Cache Group. Enable this to make Max
Origin Connections per Cache Group instead of a value used over all last-tier
Cache Groups. https://traffic-control-cdn.readthedocs.io/en/latest/overview/delivery_services.html#regional;
target="_blank">See Regional Delivery Services
+
+
+
+
+
+Enabled
+Disabled
+
Review Comment:
this is pretty standard throughout TP, so you can leave it like this if you
want, but for my money boolean inputs should use `input[type="checkbox"]`
because that's what it's for.
##
traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql:
##
@@ -0,0 +1,30 @@
+/*
+ * 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.
+ */
+
+ALTER TABLE public.deliveryservice
+ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE;
+
+/* Set `regional` to `false` if it does not exist */
+UPDATE public.deliveryservice_request
+SET deliveryservice = deliveryservice || '{"regional": false}'
+WHERE deliveryservice->>'regional' IS NULL;
+
+/* Set `regional` to `false` it does not exist and `original` is not null */
+UPDATE public.deliveryservice_request
+SET original = original || '{"regional": false}'
+WHERE original IS NOT NULL
Review Comment:
why check if original is `NULL` if not doing the same for deliveryservice?
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#issuecomment-1270196692 Added the Regional field to Traffic Portal v1 in 9b4c1f3f2e -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #7102: Tpv2 server fix
ocket commented on PR #7102: URL: https://github.com/apache/trafficcontrol/pull/7102#issuecomment-1270168821 no, the developer environment is for development, so using the development angular server seems more appropriate to me. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r988438970 ## traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql: ## @@ -0,0 +1,19 @@ +/* + * 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. + */ + +ALTER TABLE public.deliveryservice +ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE; Review Comment: Affecting DSRs in the migrations in 7ce5da9079 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r988438970 ## traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql: ## @@ -0,0 +1,19 @@ +/* + * 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. + */ + +ALTER TABLE public.deliveryservice +ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE; Review Comment: Affecting DSRs in the migration in 7ce5da9079 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r988409168 ## traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql: ## @@ -0,0 +1,19 @@ +/* + * 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. + */ + +ALTER TABLE public.deliveryservice +ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE; Review Comment: oh I see what you mean now -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
zrhoffman commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r988402881 ## traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql: ## @@ -0,0 +1,19 @@ +/* + * 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. + */ + +ALTER TABLE public.deliveryservice +ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE; Review Comment: Affect DSRs, like adding a column? But DSRs are still stored as `JSONB`, right? Or is there a different way for this migration to affect DSRs? -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] github-code-scanning[bot] commented on a diff in pull request #7079: Assign multiple servers to a capability
github-code-scanning[bot] commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r988365003
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +454,167 @@
}
defer inf.Close()
- var msc tc.MultipleServerCapabilities
- if err := json.NewDecoder(r.Body).Decode(); err != nil {
- api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ var mssc tc.MultipleServersCapabilities
+ if err := json.NewDecoder(r.Body).Decode(); err != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest,
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities
struct %w", err), nil)
return
}
- // Check existence prior to checking type
- _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
- if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
- }
- if !exists {
- userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
- return
+ if len(mssc.ServerIDs) == 1 {
+ errCode, userErr, sysErr = checkExistingServer(tx,
mssc.ServerIDs[0], inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
}
- // Ensure type is correct
- correctType := true
- if err := tx.QueryRow(scCheckServerTypeQuery(),
msc.ServerID).Scan(); err != nil {
+ //Check if the server type is MID and/or EDGE
+ var servArray []int64
+ queryType := `SELECT array_agg(s.id)
+ FROM server s
+ JOIN type t ON s.type = t.id
+ WHERE s.id = any ($1)
+ AND t.use_in_table = 'server'
+ AND (t.name LIKE 'MID%' OR t.name LIKE 'EDGE%')`
+ if err := tx.QueryRow(queryType,
pq.Array(mssc.ServerIDs)).Scan(pq.Array()); err != nil {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
fmt.Errorf("checking server type: %w", err))
return
}
- if !correctType {
- userErr := fmt.Errorf("server %d has an incorrect server type.
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
- api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
- return
+ cmp := make(map[int64]bool)
+ for _, item := range servArray {
+ cmp[item] = true
+ }
+ for _, sid := range mssc.ServerIDs {
+ if _, ok := cmp[sid]; !ok {
+ userErr := fmt.Errorf("server id: %d has an incorrect
server type. Server capability can only be assigned to EDGE or MID servers",
sid)
+ api.HandleErr(w, r, tx, http.StatusBadRequest, userErr,
nil)
+ return
+ }
+ }
+
+ // Insert rows in DB
+ sid := make([]int64, len(mssc.ServerCapabilities))
+ scs := make([]string, len(mssc.ServerIDs))
+ if len(mssc.ServerIDs) == 1 {
+ if len(mssc.ServerCapabilities) >= 1 {
+ for i := range mssc.ServerCapabilities {
+ sid[i] = mssc.ServerIDs[0]
+ }
+ scs = mssc.ServerCapabilities
+ }
+ } else if len(mssc.ServerCapabilities) == 1 {
+ if len(mssc.ServerIDs) >= 1 {
+ for i := range mssc.ServerIDs {
+ scs[i] = mssc.ServerCapabilities[0]
+ }
+ sid = mssc.ServerIDs
+ }
+ } else {
+ scs = mssc.ServerCapabilities
+ sid = mssc.ServerIDs
}
- cdnName, err := dbhelpers.GetCDNNameFromServerID(tx,
int64(msc.ServerID))
+ msscQuery := `INSERT INTO server_server_capability
+ select "server_capability", "server"
+ FROM UNNEST($1::text[], $2::int[]) AS
tmp("server_capability", "server")`
+ _, err := tx.Query(msscQuery, pq.Array(scs), pq.Array(sid))
if err != nil {
- api.HandleErr(w, r, tx, http.StatusInternalServerError, nil,
err)
+ useErr, sysErr, statusCode := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
return
}
- userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx,
string(cdnName), inf.User.UserName)
+ var alerts tc.Alerts
+ if len(mssc.ServerCapabilities) == 1 && len(mssc.ServerIDs) == 1 {
+ alerts = tc.CreateAlerts(tc.SuccessLevel, "Assigned either a
Server Capability to a server
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7104: TP fix child DS tables not loading
ocket commented on code in PR #7104: URL: https://github.com/apache/trafficcontrol/pull/7104#discussion_r988359735 ## traffic_portal/app/src/common/modules/table/topologyDeliveryServices/table.topologyDeliveryServices.tpl.html: ## @@ -26,7 +26,6 @@ columns="columns" drop-down-options="dropDownOptions" context-menu-options="contextMenuOptions" - sensitive-columns="sensitiveColumns" Review Comment: Same as above on this table ## traffic_portal/app/src/common/modules/table/typeDeliveryServices/table.typeDeliveryServices.tpl.html: ## @@ -26,7 +26,6 @@ columns="columns" drop-down-options="dropDownOptions" context-menu-options="contextMenuOptions" - sensitive-columns="sensitiveColumns" Review Comment: I didn't check this one but with two whole points of data I'm confident that this is broken too ## traffic_portal/app/src/common/modules/table/serviceCategoryDeliveryServices/table.serviceCategoryDeliveryServices.tpl.html: ## @@ -26,7 +26,6 @@ columns="columns" drop-down-options="dropDownOptions" context-menu-options="contextMenuOptions" - sensitive-columns="sensitiveColumns" Review Comment: On the service category/delivery services page I can now see all the content of sensitive columns, which I think is because you removed 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7111: Delivery Service Regional field: Optionally make Max Origin Connections per Cache Group
ocket commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r988299783 ## traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql: ## @@ -0,0 +1,19 @@ +/* + * 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. + */ + +ALTER TABLE public.deliveryservice +ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE; Review Comment: Shouldn't these migrations also affect DSRs? -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7075: Add Last Updated field to Traffic Portal
ocket merged PR #7075: URL: https://github.com/apache/trafficcontrol/pull/7075 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ericholguin commented on issue #6801: Requesting `/servers/{{ID}}/deliveryservices` where `ID` is non-existent causes Internal Server Error
ericholguin commented on issue #6801: URL: https://github.com/apache/trafficcontrol/issues/6801#issuecomment-1265761357 I think this should be reopened it is a separate issue from #6691 error.log shows: ``` ERROR: api.go:263: 2022-10-03T16:55:00.5425Z: [::1]:54360 querying CDN name from server ID: sql: no rows in result set ``` Which is completely different from that other issue. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7103: ATC Collaborators for October 2022
ocket merged PR #7103: URL: https://github.com/apache/trafficcontrol/pull/7103 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] asf-ci opened a new pull request, #7103: ATC Collaborators for October 2022
asf-ci opened a new pull request, #7103: URL: https://github.com/apache/trafficcontrol/pull/7103 This PR uses [`.asf.yaml`](https://s.apache.org/asfyamltriage) to assign the GitHub [Triage role](https://docs.github.com/en/organizations/managing-access-to-your-organizations-repositories/repository-roles-for-an-organization#permissions-for-each-role) to non-committer contributors who fixed 2 or more Issues in the past 31 days: * @rimashah25 fixed 6 Issues: - #7080, fixed by #7098 - #7078, fixed by #7020 - #7055, fixed by #7088 - #7052, fixed by #7074 - #7049, fixed by #7074 - #6335, fixed by #7098 Congrats! For the month of October, @rimashah25 will have the ability to * Apply labels to Issues and Pull Requests * Assign a user to an Issue (note that non-committers can be assigned to an Issue after commenting on it) * Add a user as a Reviewer of a Pull Request, which will send a request to them to review it * Mark Issues and Pull Requests as a duplicate These privileges will expire at the end of October. If you want to be an Apache Traffic Control collaborator next month: 1. Read our [contribution guidelines](https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md) 2. Find an Issue to work on (recommended issues have the [good first issue](https://github.com/apache/trafficcontrol/issues?q=is:issue+is:open+label:good+first+issue+no:assignee) label) and ask to be assigned 3. Get coding! For questions on how to contribute, you can reach the ATC community on - The `#traffic-control` channel of the ASF Slack ([invite link](https://s.apache.org/tc-slack-request)) - The ATC Dev [mailing list](https://trafficcontrol.apache.org/mailing_lists) ([archives](https://lists.apache.org/list?d...@trafficcontrol.apache.org:lte=5y:)) ## Which Traffic Control components are affected by this PR? - Other: [`.asf.yaml`](https://github.com/apache/trafficcontrol/blob/master/.asf.yaml) ## What is the best way to verify this PR? Verify that the fixed Issues listed above are linked to [PRs from the past 31 days](https://github.com/apache/trafficcontrol/pulls?q=is:pr+linked:issue+merged:2022-08-31..2022-10-01) ## PR submission checklist - [ ] This PR has tests - [ ] This PR has documentation - [ ] This PR has a CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #7100: Traffic Router tests GHA: Include integration tests
zrhoffman commented on PR #7100: URL: https://github.com/apache/trafficcontrol/pull/7100#issuecomment-1263985694 Switched back to the old annotations we were using that echoes annotations instead of using the API (I wish more of them did that) in 7483eaa032 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] github-code-scanning[bot] commented on a diff in pull request #7102: Tpv2 server fix
github-code-scanning[bot] commented on code in PR #7102:
URL: https://github.com/apache/trafficcontrol/pull/7102#discussion_r984908529
##
experimental/traffic-portal/server.ts:
##
@@ -198,10 +196,18 @@
return;
}
res.statusCode = 308;
- res.setHeader("Location",
req.url.replace(/^[hH][tT][tT][pP]:/, "https"));
+ res.setHeader("Location",
req.url.replace(/^[hH][tT][tT][pP]:/, "https:"));
Review Comment:
## Server-side URL redirect
Untrusted URL redirection due to [user-provided value](1).
[Show more
details](https://github.com/apache/trafficcontrol/security/code-scanning/231)
--
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.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 opened a new pull request, #7102: Tpv2 server fix
ocket opened a new pull request, #7102: URL: https://github.com/apache/trafficcontrol/pull/7102 Fixed the TPv2 server not running as intended. It also now handles more gracefully the common case of bind permissions being denied on port 80 when running in SSL mode. Instead of the whole server crashing, it'll just print an error and close the failed server, proceeding with HTTPS only. To make sure it actually builds and runs from now on, I switched the end-to-end tests from using `ng serve` to building and using the server. ## Which Traffic Control components are affected by this PR? - Traffic Portal (Experimental v2) - Automation (GHA) ## What is the best way to verify this PR? Make sure the tests pass now that they use the server. ## PR submission checklist - [x] This PR has tests - [x] This PR needs no documentation - [x] This PR needs no CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #7100: Traffic Router tests GHA: Include integration tests
ocket commented on PR #7100: URL: https://github.com/apache/trafficcontrol/pull/7100#issuecomment-1263932500 So you're saying that once this is merged it'll add annotations? Lemme check that out. Or, well, at least check if the warning and error go away. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #7100: Traffic Router tests GHA: Include integration tests
zrhoffman commented on PR #7100: URL: https://github.com/apache/trafficcontrol/pull/7100#issuecomment-1263910426 But we can get annotations on PRs. See the annotations generated from the PR I linked above: https://github.com/apache/unomi/actions/runs/3129456290 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #7100: Traffic Router tests GHA: Include integration tests
ocket commented on PR #7100: URL: https://github.com/apache/trafficcontrol/pull/7100#issuecomment-1263905693 Well if we can't get annotations on PRs, why have the step for converting JUnit to annotations? -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #7100: Traffic Router tests GHA: Include integration tests
zrhoffman commented on PR #7100: URL: https://github.com/apache/trafficcontrol/pull/7100#issuecomment-1263899975 > Also it's not caching Maven dependencies because it can't find the directory at the end of the run: > > > Warning: Path Validation Error: Path(s) specified in the action for caching do(es) not exist, hence no cache is being saved. Fixed in 077d53087c -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman commented on pull request #7100: Traffic Router tests GHA: Include integration tests
zrhoffman commented on PR #7100: URL: https://github.com/apache/trafficcontrol/pull/7100#issuecomment-1263896968 > I'm still seeing the same annotations in the latest run on that JUnit-to-annotation conversion. I think that that is because the *mikepenz/action-junit-report* action is being used in a PR without being in the target branch. See the annotations resulting from the *Build and run tests / Execute integration tests (11)* workflow run on apache/unomi#512, since they already use *mikepenz/action-junit-report*. They don't seem to be [using it](https://github.com/apache/unomi/blob/c916eaf516/.github/workflows/unomi-ci-build-tests.yml#L57-L61) in any special way, either. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on pull request #7100: Traffic Router tests GHA: Include integration tests
ocket commented on PR #7100: URL: https://github.com/apache/trafficcontrol/pull/7100#issuecomment-1263853338 I'm still seeing the same annotations in the latest run on that JUnit-to-annotation conversion. Also it's not caching Maven dependencies because it can't find the directory at the end of the run: > Warning: Path Validation Error: Path(s) specified in the action for caching do(es) not exist, hence no cache is being saved. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] jhg03a opened a new issue, #7101: Traffic portal values should be loaded on click versus on page load
jhg03a opened a new issue, #7101: URL: https://github.com/apache/trafficcontrol/issues/7101 ## This Improvement request (usability, performance, tech debt, etc.) affects these Traffic Control components: - Traffic Portal ## Current behavior: Right now when creating a server for example, the values present to choose from in dropdowns are set on page load. If I need to create a new cachegroup or profile for example, I have to know how to manipulate other fields to trigger a reload or refresh the page and start over with all my data entry. This behavior isn't limited to any one field or type that uses a dropdown in TP. ## New behavior: When I click the dropdown I want to have it make the API call to request information. This is so that if I need to open other tabs to create necessary data, I get consistent behavior that will show what was just made rather than a cached copy missing what I need. -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on pull request #7075: Add Last Updated field to Traffic Portal
rimashah25 commented on PR #7075: URL: https://github.com/apache/trafficcontrol/pull/7075#issuecomment-1263765624 I see the `Last Updated` field in the form correctly  but in the DS table it shows as Invalid Date  -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7084: TP fix filtering on Delivery Service "Targets For" column
ocket merged PR #7084: URL: https://github.com/apache/trafficcontrol/pull/7084 -- 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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [trafficcontrol] zrhoffman opened a new pull request, #7100: Traffic Router tests GHA: Inclue integration tests
zrhoffman opened a new pull request, #7100: URL: https://github.com/apache/trafficcontrol/pull/7100 This Pull Request * renames the [*Traffic Router Unit Tests*](https://github.com/apache/trafficcontrol/actions/workflows/tr.unit.tests.yaml) workflow to [*Traffic Router Tests*](https://github.com/apache/trafficcontrol/actions/workflows/tr.tests.yaml) * Makes the *Traffic Router Tests* GitHub Action run the integration (and external) tests, in addition to the unit tests * Adds specific paths to the *Experimental Traffic Portal v2* workflow's *push* trigger so it no longer runs on every commit ## Which Traffic Control components are affected by this PR? - Automation - GitHub Actions ## What is the best way to verify this PR? Verify that the *Traffic Router Tests* GHA includes the TR integration tests ## PR submission checklist - [x] This PR has tests - [ ] This PR has documentation - [ ] This PR has a CHANGELOG.md entry - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://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. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
