[GitHub] [trafficcontrol] github-code-scanning[bot] commented on a diff in pull request #7161: DTP open sourcing
github-code-scanning[bot] commented on code in PR #7161:
URL: https://github.com/apache/trafficcontrol/pull/7161#discussion_r1009840274
##
test/fakeOrigin/dtp/dtp.go:
##
@@ -0,0 +1,130 @@
+package dtp
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "crypto/tls"
+ "fmt"
+ "log"
+ "net/http"
+ "strings"
+ "time"
+)
+
+type LogRecorder struct {
+ http.ResponseWriter
+
+ Status int
+ HeaderBytes int64
+ ContentBytes int64
+}
+
+func (rec *LogRecorder) WriteHeader(code int) {
+ rec.Status = code
+ rec.ResponseWriter.WriteHeader(code)
+}
+
+func (rec *LogRecorder) Write(bytes []byte) (int, error) {
+ rec.ContentBytes += int64(len(bytes))
+ return rec.ResponseWriter.Write(bytes)
+}
+
+// this is mostly for hijack
+func isHandlerType(r *http.Request) bool {
+ if strings.Contains(r.URL.EscapedPath(), "~h.") {
+ return true
+ } else if strings.Contains(r.URL.RawQuery, "~h.") {
+ return true
+ } else {
+ for _, part := range r.Header[`X-Dtp`] {
+ if strings.Contains(part, "~h.") {
+ return true
+ }
+ }
+ }
+
+ return false
+}
+
+func Logger(alog *log.Logger, next http.Handler) http.Handler {
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ timeStart := time.Now().UnixNano()
+
+ // the logger interferes with hijacking
+ if isHandlerType(r) {
+ next.ServeHTTP(w, r)
+ alog.Printf("%.3f %s \"%s\" %d b=%d ttms=%d uas=\"%s\"
rr=\"%s\"\n",
+ float64(timeStart)/float64(1.e9),
+ r.Method,
+ r.URL.String(),
Review Comment:
## Log entries created from user input
Log entry depends on a [user-provided value](1).
[Show more
details](https://github.com/apache/trafficcontrol/security/code-scanning/238)
##
test/fakeOrigin/dtp/dtp.go:
##
@@ -0,0 +1,130 @@
+package dtp
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "crypto/tls"
+ "fmt"
+ "log"
+ "net/http"
+ "strings"
+ "time"
+)
+
+type LogRecorder struct {
+ http.ResponseWriter
+
+ Status int
+ HeaderBytes int64
+ ContentBytes int64
+}
+
+func (rec *LogRecorder) WriteHeader(code int) {
+ rec.Status = code
+ rec.ResponseWriter.WriteHeader(code)
+}
+
+func (rec *LogRecorder) Write(bytes []byte) (int, error) {
+ rec.ContentBytes += int64(len(bytes))
+ return rec.ResponseWriter.Write(bytes)
+}
+
+// this is mostly for hijack
+func isHandlerType(r *http.Request) bool {
+ if strings.Contains(r.URL.EscapedPath(), "~h.") {
+ return true
+ } else if strings.Contains(r.URL.RawQuery, "~h.") {
+ return true
+ } else {
+ for _, part := range r.Header[`X-Dtp`] {
+ if strings.Contains(part, "~h.") {
+ return true
+ }
+ }
+ }
+
+ return false
+}
+
+func Logger(alog *log.Logger, next http.Handler) http.Handler {
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request)
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7099: Delivery Service Active Flag Rework
rimashah25 commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1009828166
##
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##
@@ -138,7 +162,7 @@ func CreateV30(w http.ResponseWriter, r *http.Request) {
ds := tc.DeliveryServiceV30{}
if err := json.NewDecoder(r.Body).Decode(); err != nil {
- api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
errors.New("decoding: "+err.Error()), nil)
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
fmt.Errorf("decoding: %w", err), nil)
Review Comment:
Ok, I like that you are taking care of it in each of your PR but I feel that
is a bit of scope creep. Because this is refactor(necessary) but I am not a
100% sure in a PR this big it should have been included. I
f you want, create an issue to tackle this as part of tech-debt and we can
take care of it at one go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ezelkow1 commented on a diff in pull request #7161: DTP open sourcing
ezelkow1 commented on code in PR #7161:
URL: https://github.com/apache/trafficcontrol/pull/7161#discussion_r1009826559
##
test/fakeOrigin/dtp/dtp.go:
##
@@ -0,0 +1,144 @@
+package dtp
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "crypto/tls"
+ "fmt"
+ "log"
+ "net/http"
+ "strings"
+ "time"
+)
+
+type LogRecorder struct {
+ http.ResponseWriter
+
+ Status int
+ HeaderBytes int64
+ ContentBytes int64
+}
+
+func (rec *LogRecorder) WriteHeader(code int) {
+ rec.Status = code
+ rec.ResponseWriter.WriteHeader(code)
+}
+
+func (rec *LogRecorder) Write(bytes []byte) (int, error) {
+ rec.ContentBytes += int64(len(bytes))
+ return rec.ResponseWriter.Write(bytes)
Review Comment:
dismissed (as tests)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [trafficcontrol] ezelkow1 commented on a diff in pull request #7161: DTP open sourcing
ezelkow1 commented on code in PR #7161:
URL: https://github.com/apache/trafficcontrol/pull/7161#discussion_r1009827162
##
test/fakeOrigin/dtp/dtp.go:
##
@@ -0,0 +1,144 @@
+package dtp
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "crypto/tls"
+ "fmt"
+ "log"
+ "net/http"
+ "strings"
+ "time"
+)
+
+type LogRecorder struct {
+ http.ResponseWriter
+
+ Status int
+ HeaderBytes int64
+ ContentBytes int64
+}
+
+func (rec *LogRecorder) WriteHeader(code int) {
+ rec.Status = code
+ rec.ResponseWriter.WriteHeader(code)
+}
+
+func (rec *LogRecorder) Write(bytes []byte) (int, error) {
+ rec.ContentBytes += int64(len(bytes))
+ return rec.ResponseWriter.Write(bytes)
+}
+
+// this is mostly for hijack
+func isHandlerType(r *http.Request) bool {
+ if strings.Contains(r.URL.EscapedPath(), "~h.") {
+ return true
+ } else if strings.Contains(r.URL.RawQuery, "~h.") {
+ return true
+ } else {
+ for _, part := range r.Header[`X-Dtp`] {
+ if strings.Contains(part, "~h.") {
+ return true
+ }
+ }
+ }
+
+ return false
+}
+
+func Logger(alog *log.Logger, next http.Handler) http.Handler {
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ timeStart := time.Now().UnixNano()
+
+ // the logger interferes with hijacking
+ if isHandlerType(r) {
+ next.ServeHTTP(w, r)
+ url := strings.Replace(r.URL.String(), "\n", "", -1)
+ url = strings.Replace(url, "\r", "", -1)
+ ua := strings.Replace(r.UserAgent(), "\n", "", -1)
+ ua = strings.Replace(ua, "\r", "", -1)
+ thisRange := strings.Replace(r.Header.Get("Range"),
"\n", "", -1)
+ thisRange = strings.Replace(thisRange, "\r", "", -1)
+ alog.Printf("%.3f %s \"%s\" %d b=%d ttms=%d uas=\"%s\"
rr=\"%s\"\n",
+ float64(timeStart)/float64(1.e9),
+ r.Method,
+ url,
+ 42, // status code -- why not?
+ 0, // bytes
+ 0, // ttms
+ ua,
+ thisRange,
+ )
+ return
+ }
+
+ tlsstr := "-"
+ if r.TLS != nil {
+ tlsstr = tls.CipherSuiteName(r.TLS.CipherSuite)
+ }
+ /*
+ if nil != r.TLS {
+ tlsstr = fmt.Sprintf("0x%x", r.TLS.CipherSuite)
+ }
+ */
+
+ rec := LogRecorder{w, 200, 0, 0}
+ next.ServeHTTP(, r)
+ timeStop := time.Now().UnixNano()
+ url := strings.Replace(r.URL.String(), "\n", "", -1)
+ url = strings.Replace(url, "\r", "", -1)
+ ua := strings.Replace(r.UserAgent(), "\n", "", -1)
+ ua = strings.Replace(ua, "\r", "", -1)
+ thisRange := strings.Replace(r.Header.Get("Range"), "\n", "",
-1)
+ thisRange = strings.Replace(thisRange, "\r", "", -1)
+ alog.Printf("%.3f %s \"%s\" %s %d b=%d ttms=%d uas=\"%s\"
rr=\"%s\"\n",
+ float64(timeStart)/float64(1.e9),
+ r.Method,
+ url,
+ tlsstr,
+ rec.Status,
+ rec.ContentBytes,
+ int64((timeStop-timeStart)/100),
+ ua,
+ thisRange,
+ )
+
+ if GlobalConfig.Log.RequestHeaders {
+ alog.Print(r.Header)
Review Comment:
dismissed (as tests)!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on
[GitHub] [trafficcontrol] ezelkow1 commented on a diff in pull request #7161: DTP open sourcing
ezelkow1 commented on code in PR #7161:
URL: https://github.com/apache/trafficcontrol/pull/7161#discussion_r1009826299
##
test/fakeOrigin/dtp/dtp.go:
##
@@ -0,0 +1,144 @@
+package dtp
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "crypto/tls"
+ "fmt"
+ "log"
+ "net/http"
+ "strings"
+ "time"
+)
+
+type LogRecorder struct {
+ http.ResponseWriter
+
+ Status int
+ HeaderBytes int64
+ ContentBytes int64
+}
+
+func (rec *LogRecorder) WriteHeader(code int) {
+ rec.Status = code
+ rec.ResponseWriter.WriteHeader(code)
+}
+
+func (rec *LogRecorder) Write(bytes []byte) (int, error) {
+ rec.ContentBytes += int64(len(bytes))
+ return rec.ResponseWriter.Write(bytes)
Review Comment:
dismissed (as tests)!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #7161: DTP open sourcing
github-code-scanning[bot] commented on code in PR #7161:
URL: https://github.com/apache/trafficcontrol/pull/7161#discussion_r1009807225
##
test/fakeOrigin/dtp/dtp.go:
##
@@ -0,0 +1,144 @@
+package dtp
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "crypto/tls"
+ "fmt"
+ "log"
+ "net/http"
+ "strings"
+ "time"
+)
+
+type LogRecorder struct {
+ http.ResponseWriter
+
+ Status int
+ HeaderBytes int64
+ ContentBytes int64
+}
+
+func (rec *LogRecorder) WriteHeader(code int) {
+ rec.Status = code
+ rec.ResponseWriter.WriteHeader(code)
+}
+
+func (rec *LogRecorder) Write(bytes []byte) (int, error) {
+ rec.ContentBytes += int64(len(bytes))
+ return rec.ResponseWriter.Write(bytes)
+}
+
+// this is mostly for hijack
+func isHandlerType(r *http.Request) bool {
+ if strings.Contains(r.URL.EscapedPath(), "~h.") {
+ return true
+ } else if strings.Contains(r.URL.RawQuery, "~h.") {
+ return true
+ } else {
+ for _, part := range r.Header[`X-Dtp`] {
+ if strings.Contains(part, "~h.") {
+ return true
+ }
+ }
+ }
+
+ return false
+}
+
+func Logger(alog *log.Logger, next http.Handler) http.Handler {
+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ timeStart := time.Now().UnixNano()
+
+ // the logger interferes with hijacking
+ if isHandlerType(r) {
+ next.ServeHTTP(w, r)
+ url := strings.Replace(r.URL.String(), "\n", "", -1)
+ url = strings.Replace(url, "\r", "", -1)
+ ua := strings.Replace(r.UserAgent(), "\n", "", -1)
+ ua = strings.Replace(ua, "\r", "", -1)
+ thisRange := strings.Replace(r.Header.Get("Range"),
"\n", "", -1)
+ thisRange = strings.Replace(thisRange, "\r", "", -1)
+ alog.Printf("%.3f %s \"%s\" %d b=%d ttms=%d uas=\"%s\"
rr=\"%s\"\n",
+ float64(timeStart)/float64(1.e9),
+ r.Method,
+ url,
+ 42, // status code -- why not?
+ 0, // bytes
+ 0, // ttms
+ ua,
+ thisRange,
+ )
+ return
+ }
+
+ tlsstr := "-"
+ if r.TLS != nil {
+ tlsstr = tls.CipherSuiteName(r.TLS.CipherSuite)
+ }
+ /*
+ if nil != r.TLS {
+ tlsstr = fmt.Sprintf("0x%x", r.TLS.CipherSuite)
+ }
+ */
+
+ rec := LogRecorder{w, 200, 0, 0}
+ next.ServeHTTP(, r)
+ timeStop := time.Now().UnixNano()
+ url := strings.Replace(r.URL.String(), "\n", "", -1)
+ url = strings.Replace(url, "\r", "", -1)
+ ua := strings.Replace(r.UserAgent(), "\n", "", -1)
+ ua = strings.Replace(ua, "\r", "", -1)
+ thisRange := strings.Replace(r.Header.Get("Range"), "\n", "",
-1)
+ thisRange = strings.Replace(thisRange, "\r", "", -1)
+ alog.Printf("%.3f %s \"%s\" %s %d b=%d ttms=%d uas=\"%s\"
rr=\"%s\"\n",
+ float64(timeStart)/float64(1.e9),
+ r.Method,
+ url,
+ tlsstr,
+ rec.Status,
+ rec.ContentBytes,
+ int64((timeStop-timeStart)/100),
+ ua,
+ thisRange,
+ )
+
+ if GlobalConfig.Log.RequestHeaders {
+ alog.Print(r.Header)
Review Comment:
## Log entries created from user input
Log entry depends on a [user-provided value](1).
[Show
[GitHub] [trafficcontrol] srijeet0406 closed issue #6033: Add ability to assign and remove a server capability for multiple servers at a time
srijeet0406 closed issue #6033: Add ability to assign and remove a server capability for multiple servers at a time URL: https://github.com/apache/trafficcontrol/issues/6033 -- This is an automated message from the 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 #7079: Assign multiple servers to a capability
srijeet0406 merged PR #7079: URL: https://github.com/apache/trafficcontrol/pull/7079 -- This is an automated message from the 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 issue #6533: TR should not rename/recreate log files on rollover
zrhoffman commented on issue #6533: URL: https://github.com/apache/trafficcontrol/issues/6533#issuecomment-1295962551 Assigned! -- This is an automated message from the 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 commented on issue #6533: TR should not rename/recreate log files on rollover
villajo commented on issue #6533: URL: https://github.com/apache/trafficcontrol/issues/6533#issuecomment-1295961247 Can this be assigned to me? I'd like to take a crack at it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. 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_r1008285232
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ 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()
-
[GitHub] [trafficcontrol] rob05c opened a new pull request, #7163: Fix t3c multiple profile generation
rob05c opened a new pull request, #7163: URL: https://github.com/apache/trafficcontrol/pull/7163 Draft - temporarily not a draft to make tests run. ## Which Traffic Control components are affected by this PR? - Documentation - Traffic Control Cache Config (`t3c`, formerly ORT) - Traffic Control Health Client (tc-health-client) - Traffic Control Client - Traffic Monitor - Traffic Ops - Traffic Portal - Traffic Router - Traffic Stats - Grove - CDN in a Box - Automation - unknown ## What is the best way to verify this PR? ## 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] 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_r1007454984
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ 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 {
Review Comment:
I thought the whole point of adding the pagetype was so that we didn't have
to do length checks anymore?
--
This is an automated message from the 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_r1007454708
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ 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()
-
[GitHub] [trafficcontrol] ocket8888 merged pull request #7160: Consider DS Regional field for MaxOriginConnections of Edge Caches
ocket merged PR #7160: URL: https://github.com/apache/trafficcontrol/pull/7160 -- This is an automated message from the 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 #7162: updated tomcat version to 1.2.35
zrhoffman commented on PR #7162: URL: https://github.com/apache/trafficcontrol/pull/7162#issuecomment-1293800151 Although this works, the next time the tomcat-native version is updated, it should be something like `tomcat-native-1.*`, since any tomcat-native version 1 will work (version 2 breaks TR currently) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. 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 #7162: updated tomcat version to 1.2.35
zrhoffman merged PR #7162: URL: https://github.com/apache/trafficcontrol/pull/7162 -- This is an automated message from the 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] ragen14 opened a new pull request, #7162: updated tomcat version to 1.2.35
ragen14 opened a new pull request, #7162: URL: https://github.com/apache/trafficcontrol/pull/7162 ## Which Traffic Control components are affected by this PR? - Traffic Router - Automation ## What is the best way to verify this PR? install traffic router using TR role in ansible ## If this is a bugfix, which Traffic Control versions contained the bug? N/A ## 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 #7160: Consider DS Regional field for MaxOriginConnections of Edge Caches
zrhoffman commented on code in PR #7160:
URL: https://github.com/apache/trafficcontrol/pull/7160#discussion_r1007114313
##
lib/go-atscfg/headerrewritedotconfig_test.go:
##
@@ -496,42 +407,91 @@ func TestGetAssignedMids(t *testing.T) {
ID: util.Ptr(2),
Status: util.Ptr(string(tc.CacheStatusReported)),
},
-
+ }
+ mids := []Server{
{
Cachegroup: util.Ptr("mid1"),
CDNName:util.Ptr("mycdn"),
HostName: util.Ptr("midCache1"),
ID: util.Ptr(3),
Status: util.Ptr(string(tc.CacheStatusReported)),
+ Type: tc.MidTypePrefix,
},
{
Cachegroup: util.Ptr("mid2"),
CDNName:util.Ptr("mycdn"),
HostName: util.Ptr("midCache2"),
ID: util.Ptr(4),
Status: util.Ptr(string(tc.CacheStatusReported)),
+ Type: tc.MidTypePrefix,
},
}
+ allServers := append(edges, mids...)
+ _ = allServers
- allDeliveryServices := []DeliveryService{{}, {}}
+ topology := tc.Topology{
+ Name: "mytopology",
+ Nodes: []tc.TopologyNode{
+ {
+ Cachegroup: "edge1",
+ Parents:[]int{2},
+ },
+ {
+ Cachegroup: "edge2",
+ Parents:[]int{2},
+ },
+ {
+ Cachegroup: "org1",
+ },
+ },
+ }
+ _ = topology
Review Comment:
Removed do-nothing lines in 3b25d1aeae
--
This is an automated message from the 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 #7160: Consider DS Regional field for MaxOriginConnections of Edge Caches
ocket commented on code in PR #7160:
URL: https://github.com/apache/trafficcontrol/pull/7160#discussion_r1007048872
##
lib/go-atscfg/headerrewritedotconfig_test.go:
##
@@ -496,42 +407,91 @@ func TestGetAssignedMids(t *testing.T) {
ID: util.Ptr(2),
Status: util.Ptr(string(tc.CacheStatusReported)),
},
-
+ }
+ mids := []Server{
{
Cachegroup: util.Ptr("mid1"),
CDNName:util.Ptr("mycdn"),
HostName: util.Ptr("midCache1"),
ID: util.Ptr(3),
Status: util.Ptr(string(tc.CacheStatusReported)),
+ Type: tc.MidTypePrefix,
},
{
Cachegroup: util.Ptr("mid2"),
CDNName:util.Ptr("mycdn"),
HostName: util.Ptr("midCache2"),
ID: util.Ptr(4),
Status: util.Ptr(string(tc.CacheStatusReported)),
+ Type: tc.MidTypePrefix,
},
}
+ allServers := append(edges, mids...)
+ _ = allServers
- allDeliveryServices := []DeliveryService{{}, {}}
+ topology := tc.Topology{
+ Name: "mytopology",
+ Nodes: []tc.TopologyNode{
+ {
+ Cachegroup: "edge1",
+ Parents:[]int{2},
+ },
+ {
+ Cachegroup: "edge2",
+ Parents:[]int{2},
+ },
+ {
+ Cachegroup: "org1",
+ },
+ },
+ }
+ _ = topology
Review Comment:
I think this line does literally nothing
--
This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
rimashah25 commented on code in PR #7099: URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1006437034 ## traffic_ops/testing/api/v4/tc-fixtures.json: ## @@ -907,7 +907,7 @@ "regexRemap": null, "regionalGeoBlocking": false, "remapText": null, -"routingName": "cdn", +"routingName": "ccr-ds1", Review Comment: oh.. ok -- This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
rimashah25 commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1006436587
##
traffic_ops/v5-client/deliveryservice_requests.go:
##
@@ -58,31 +58,31 @@ func (to *Session) CreateDeliveryServiceRequest(dsr
tc.DeliveryServiceRequestV4,
dsr.AuthorID = res.Response[0].ID
}
- var ds *tc.DeliveryServiceV4
+ var ds *tc.DeliveryServiceV5
if dsr.ChangeType == tc.DSRChangeTypeDelete {
ds = dsr.Original
} else {
ds = dsr.Requested
}
- if ds.TypeID == nil && ds.Type.String() != "" {
+ if ds.TypeID <= 0 && ds.Type != nil && *ds.Type != "" {
Review Comment:
Ok, Thanks for the explanation. You can resolve this comment.
--
This is an automated message from the 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_r1006427072
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ 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 {
Review Comment:
we can replace it with `PageType` but it is not necessary.
--
This is an automated message from the 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_r1006420423
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ 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()
-
[GitHub] [trafficcontrol] ezelkow1 opened a new pull request, #7161: DTP open sourcing
ezelkow1 opened a new pull request, #7161: URL: https://github.com/apache/trafficcontrol/pull/7161 This PR open sources our internal testing tool, DTP. Originally DTP was a stand alone origin that can provide origin responses based on query parameters to simulate a variety of scenarios. It made sense to include this as another endpoint type within fakeOrigin to expand it's capabilities. There is a new endpoint type of "testing" which adds this functionality. ## Which Traffic Control components are affected by this PR? - unknown ## What is the best way to verify this PR? Build the fakeOrigin and create a testing endpoint and issue some curls against it ## PR submission checklist - [] This PR has tests - [x] 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, #7160: Consider DS Regional field for MaxOriginConnections of Edge Caches
zrhoffman opened a new pull request, #7160: URL: https://github.com/apache/trafficcontrol/pull/7160 This PR fixes a bug where MaxOriginConnections for a *Regional* Delivery Service was not divided over a Server's own Cache Group in the case of Edge Caches directly parented by the Origin. This PR also fixes the logic for the *Regional* MaxOriginConnections for Mid Caches by moving it to the correct block and updating tests. ## Which Traffic Control components are affected by this PR? - Traffic Control Cache Config (`t3c`, formerly ORT) ## What is the best way to verify this PR? Run the t3c integration tests and the t3c and `lib/go-atscfg` integration tests ## If this is a bugfix, which Traffic Control versions contained the bug? - master (994119d23b) ## PR submission checklist - [x] This PR has tests - [ ] Intended behavior unchanged, existing docs are suficient - [x] Existing *Regional* Delivery Service field changelog entry is sufficient - [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] traeak commented on pull request #7155: t3c: initial mustache for remap.config templating
traeak commented on PR #7155: URL: https://github.com/apache/trafficcontrol/pull/7155#issuecomment-1292403015 > Why did you decide on this library? The one linked to on [the mustache website](https://mustache.github.io) is [cbroclie/mustache](https://github.com/cbroglie/mustache). It was one I had already used for some internal work. I'll look at swapping out for this one. This really is just playing around, I made it draft for others to look at. -- This is an automated message from the 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 #7149: Use "twelve" as the password where "twelve12" was used
ocket commented on PR #7149: URL: https://github.com/apache/trafficcontrol/pull/7149#issuecomment-1292288893 I don't agree that we should knowingly use invalid data in any environment, except in a test to ensure such data is rejected by the API. -- This is an automated message from the 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 #7142: Run CDN in a Box for Developers services as unprivileged users
ocket commented on PR #7142: URL: https://github.com/apache/trafficcontrol/pull/7142#issuecomment-1292273358 This is something I've wanted for the dev environment since its inception, but I can't test it because of a bug in the TP/TPv2 containers that prevents them from fetching a resource from npm. I've had another developer confirm that the issue is not specific to my machine, but I still have no idea why that's happening. -- This is an automated message from the 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 #7156: Fix APIv4.0 returning APIv4.1 DS structures
zrhoffman merged PR #7156: URL: https://github.com/apache/trafficcontrol/pull/7156 -- This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
ocket commented on PR #7099: URL: https://github.com/apache/trafficcontrol/pull/7099#issuecomment-1291257249 This PR depends on #7156 -- This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
ocket commented on code in PR #7099: URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005068529 ## traffic_ops/app/db/migrations/2022092107561215_ds_explicit_mso.down.sql: ## @@ -0,0 +1,20 @@ +/* + * 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 Review Comment: Those changes don't need to be reverted. Before the change, `multiSiteOrigin` can be one of three values: `true`, `false`, or `null`. The `up` file changes all `null` values to `false` (because `null` and `false` are treated exactly the same, so there was no reason to allow both). The `up` file needed to do that because after it runs, `null` will no longer be allowed. But after `down` runs, `false` is still allowed - and also it's treated the same as `null` so there's no behavioral impact, and finally you also can't know which `false`s used to be `null`s and which were `false` in the first place, so the information needed to restore that state doesn't exist anyway. -- This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
ocket commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005066584
##
traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go:
##
@@ -1023,6 +1027,7 @@ func scanDSInfoRow(row *sql.Row) (DSInfo, bool, error) {
}
return DSInfo{}, false, fmt.Errorf("querying delivery service
server ds info: %v", err)
}
+ di.Active = active == tc.DSActiveStateActive
Review Comment:
A boolean is not being assigned to a string. A boolean is being assigned to
the outcome of comparing two strings. A Delivery Service where `"active": true`
in the old data model is equivalent to `"active": "ACTIVE"` in the new data
model. So if `active` is exactly `tc.DSActiveStateActive`, then `di.Active`
should be `true`. If it's anything else, `di.Active` should be false. Which is
what this line does.
--
This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
ocket commented on code in PR #7099: URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005065463 ## traffic_ops/testing/api/v4/tc-fixtures.json: ## @@ -907,7 +907,7 @@ "regexRemap": null, "regionalGeoBlocking": false, "remapText": null, -"routingName": "cdn", +"routingName": "ccr-ds1", Review Comment: That value is not invalid, what it was set to *before* was actually hiding an error. Changing this was part of the fix for #7094. -- This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
ocket commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005063876
##
traffic_ops/testing/api/v5/deliveryservice_request_comments_test.go:
##
@@ -178,7 +178,9 @@ func CreateTestDeliveryServiceRequestComments(t *testing.T)
{
opts.QueryParameters.Set("xmlId", comment.XMLID)
resp, _, err := TOSession.GetDeliveryServiceRequests(opts)
assert.NoError(t, err, "Cannot get Delivery Service Request by
XMLID '%s': %v - alerts: %+v", comment.XMLID, err, resp.Alerts)
- assert.Equal(t, len(resp.Response), 1, "Found %d Delivery
Service request by XMLID '%s, expected exactly one", len(resp.Response),
comment.XMLID)
+ if !assert.Equal(t, len(resp.Response), 1, "Found %d Delivery
Service request by XMLID '%s, expected exactly one", len(resp.Response),
comment.XMLID) {
Review Comment:
Because although creation of the currently processed comment has failed (or
at best succeeded wrongly), other comments can still be created, which can help
narrow down what the problem was and/or find other problems by continuing to
run other tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #7099: Delivery Service Active Flag Rework
ocket commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005062709
##
traffic_ops/v5-client/deliveryservice_requests.go:
##
@@ -58,31 +58,31 @@ func (to *Session) CreateDeliveryServiceRequest(dsr
tc.DeliveryServiceRequestV4,
dsr.AuthorID = res.Response[0].ID
}
- var ds *tc.DeliveryServiceV4
+ var ds *tc.DeliveryServiceV5
if dsr.ChangeType == tc.DSRChangeTypeDelete {
ds = dsr.Original
} else {
ds = dsr.Requested
}
- if ds.TypeID == nil && ds.Type.String() != "" {
+ if ds.TypeID <= 0 && ds.Type != nil && *ds.Type != "" {
Review Comment:
The Type is known to humans by its name, so this block was meant to populate
the ID if it wasn't given by using the Type's Name (if that *is* given,
otherwise it's an error). Because `ds.Type` is a pointer, it can be `nil`, so
its value must not be accessed in that case, or a bad error response will be
returned to the user. You also can't look up a Type with a blank Name, because
Type Names can't be blank. The old code called `.String()`, which would
segfault if the `ds.Type` reference was `nil`, so I added the `nil` check to
avoid that. `TypeID` is no longer a pointer, so instead of `nil` I checked for
the "zero" value of the property's datatype (and also invalid values i.e.
negative ones).
--
This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
ocket commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005059975
##
traffic_ops/testing/api/v5/deliveryservices_keys_test.go:
##
@@ -67,30 +67,29 @@ func createBlankCDN(cdnName string, t *testing.T) tc.CDN {
return cdns.Response[0]
}
-func cleanUp(t *testing.T, ds tc.DeliveryServiceV4, oldCDNID int, newCDNID
int, sslKeyVersions []string) {
- if ds.ID == nil || ds.XMLID == nil {
Review Comment:
Because XMLID can no longer be `nil`. If that check was still there, the
tests wouldn't compile.
--
This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
ocket commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005057765
##
traffic_ops/traffic_ops_golang/deliveryservice/safe.go:
##
@@ -144,19 +141,28 @@ func UpdateSafe(w http.ResponseWriter, r *http.Request) {
switch inf.Version.Major {
default:
fallthrough
+ case 5:
+ api.WriteRespAlertObj(w, r, tc.SuccessLevel, alertMsg,
ds)
case 4:
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, alertMsg,
dses)
+ api.WriteRespAlertObj(w, r, tc.SuccessLevel, alertMsg,
[]tc.DeliveryServiceV4{ds.Downgrade()})
case 3:
+ legacyDS := ds.Downgrade()
+ legacyDS.LongDesc1 = dses[0].LongDesc1
+ legacyDS.LongDesc2 = dses[0].LongDesc2
+ ret := legacyDS.DowngradeToV31()
if inf.Version.Minor >= 1 {
- api.WriteRespAlertObj(w, r, tc.SuccessLevel,
alertMsg, []tc.DeliveryServiceV31{tc.DeliveryServiceV31(ds.DowngradeToV31())})
+ api.WriteRespAlertObj(w, r, tc.SuccessLevel,
alertMsg, []tc.DeliveryServiceV31{tc.DeliveryServiceV31(ret)})
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, alertMsg,
[]tc.DeliveryServiceV30{ds.DowngradeToV31().DeliveryServiceV30})
+ api.WriteRespAlertObj(w, r, tc.SuccessLevel, alertMsg,
[]tc.DeliveryServiceV30{ret.DeliveryServiceV30})
case 2:
Review Comment:
Yes, though when I first made these changes we had not, yet.
--
This is an automated message from the 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 #7099: Delivery Service Active Flag Rework
ocket commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005057324
##
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##
@@ -138,7 +162,7 @@ func CreateV30(w http.ResponseWriter, r *http.Request) {
ds := tc.DeliveryServiceV30{}
if err := json.NewDecoder(r.Body).Decode(); err != nil {
- api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
errors.New("decoding: "+err.Error()), nil)
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
fmt.Errorf("decoding: %w", err), nil)
Review Comment:
> Why the change?
Because it preserves the identity of the underlying error that way. Here's
[a playground link to an example program that shows what that
means](https://go.dev/play/p/GOkCOgci1lh). It accomplishes the same thing as
`errors.New(someString+err.Error())` but without destroying any information.
> ...shouldn't we refactor others in the code base too?
In my opinion, yes, and I always request people do it this way in my
reviews. I only changed it in the files I was already editing, though, because
this is done untold thousands of times throughout ATC. Nobody would ever review
that PR. So I just make smaller changes to files as I work on 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 #7156: Fix APIv4.0 returning APIv4.1 DS structures
ocket commented on code in PR #7156:
URL: https://github.com/apache/trafficcontrol/pull/7156#discussion_r1005050496
##
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##
@@ -614,7 +614,7 @@ func (ds *TODeliveryService) Read(h http.Header, useIMS
bool) ([]interface{}, er
for _, ds := range dses {
switch {
// NOTE: it's required to handle minor version cases in a
descending >= manner
- case version.Major >= 4 && version.Minor >= 1:
+ case version.Major > 4 || (version.Major == 4 && version.Minor
>= 1):
Review Comment:
Noted; though I don't want to mess with that any more; I need to rebase the
DS Active flag changes every time it changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #7158: Traffic Vault: Reencrypt utility wipes different SSL Keys during update
ocket closed issue #7158: Traffic Vault: Reencrypt utility wipes different SSL Keys during update URL: https://github.com/apache/trafficcontrol/issues/7158 -- This is an automated message from the 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 #7159: Traffic Vault: Fix `reencrypt` utility to uniquely reencrypt ssl keys
ocket merged PR #7159: URL: https://github.com/apache/trafficcontrol/pull/7159 -- This is an automated message from the 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 #7152: Use Generics in TO API v3 Integration Tests
zrhoffman merged PR #7152: URL: https://github.com/apache/trafficcontrol/pull/7152 -- This is an automated message from the 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 #7156: Fix APIv4.0 returning APIv4.1 DS structures
zrhoffman commented on code in PR #7156:
URL: https://github.com/apache/trafficcontrol/pull/7156#discussion_r1004982343
##
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##
@@ -614,7 +614,7 @@ func (ds *TODeliveryService) Read(h http.Header, useIMS
bool) ([]interface{}, er
for _, ds := range dses {
switch {
// NOTE: it's required to handle minor version cases in a
descending >= manner
- case version.Major >= 4 && version.Minor >= 1:
+ case version.Major > 4 || (version.Major == 4 && version.Minor
>= 1):
Review Comment:
This is fine, though `Version.GreaterThanOrEqualTo()` can be used now, too
https://github.com/apache/trafficcontrol/blob/6c532922e706481cb944820b8fa6af20cc3d11ee/traffic_ops/traffic_ops_golang/cdn/cdns.go#L67
--
This is an automated message from the 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 opened a new pull request, #7159: Traffic Vault: Fix `reencrypt` utility to uniquely reencrypt ssl keys
TaylorCFrey opened a new pull request, #7159: URL: https://github.com/apache/trafficcontrol/pull/7159 Closes: #7158 A Traffic Vault utility called `reencrypt` allows for the ability to take previously encrypted values in the Traffic Vault DB and apply a new AES key for encryption. However, there was a bug that resulted in the "last" ssl key (misnomer) pulled from the DB during encryption would overwrite and wipe all other versions of ssl keys. This PR addresses said bug. ## Which Traffic Control components are affected by this PR? - Traffic Vault (techinically a support utility call `reencrypt`) ## What is the best way to verify this PR? Previously existing "tests" should continue to work. Manually verify: Obtain a dump of an existing DB with the AES key that was initially used to encrypt the data. Create a new AES key. Restore the DB into a test/verification DB location. Run the `reencrypt` utility with the necessary connection info specified in the `reencrypt.conf` file, along with passing in the file path locations for the previous AES key as well as the new AES key. Observe that the _data_ column in the _sslkey_ table has been reencrypted AND unique values exist for each version under a particular Delivery Service. ## If this is a bugfix, which Traffic Control versions contained the bug? ## PR submission checklist - [ ] This PR has tests - [ ] 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 opened a new issue, #7158: Traffic Vault: Reencrypt utility wipes different SSL Keys during update
TaylorCFrey opened a new issue, #7158: URL: https://github.com/apache/trafficcontrol/issues/7158 ## This Bug Report affects these Traffic Control components: - Traffic Vault ## Current behavior: When running the `reencrypt` utility located at `trafficcontrol/traffic_ops/app/db/reencrypt/reencrypt.go` for SSL Keys (misnomer) all versions of the SSL Keys entries are replaced by a single entry multiple times. As an example, if a delivery service has multiple "versions" of SSL Key data information, all of them are reencrypted into a single data blob for every version. Before running `reencrypt`: DeliveryServiceXMLId | Version | Data ---|---|--- sampleDS1 | latest | \xa0f3d800... sampleDS1 | 2| \x93c7b213... sampleDS1 | 1| \x7418e801... sampleDS2 | latest | \x48b1c9ff... sampleDS2 | 3 | \x88b7a3dd... sampleDS2 | 2 | \x1967c3b9... sampleDS2 | 1 | \x19f3a2bb... After running `reencrypt` (notice the data columns have the same value for each DS, regardless of the version): DeliveryServiceXMLId | Version | Data ---|---|--- sampleDS1 | latest | \xc4f1a823... sampleDS1 | 2| \xc4f1a823... sampleDS1 | 1| \xc4f1a823... sampleDS2 | latest | \xaa45b8f1... sampleDS2 | 3 | \xaa45b8f1... sampleDS2 | 2 | \xaa45b8f1... sampleDS2 | 1 | \xaa45b8f1... ## Expected behavior: Each row should be uniquely decrypted and reencrypted resulting in a different correctly encrypted _data_ for each delivery service ssl key version. ## Steps to reproduce: Execute the `reencrypt` utility on a Traffic Vault dump with a delivery service that has 3 or more entries (2 or more versions plus 'latest') for SSL Keys. -- This is an automated message from the 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] ocket8888 commented on pull request #7155: t3c: initial mustache for remap.config templating
ocket commented on PR #7155: URL: https://github.com/apache/trafficcontrol/pull/7155#issuecomment-1291092430 Why did you decide on this library? The one linked to on [the mustache website](https://mustache.github.io) is [cbroclie/mustache](https://github.com/cbroglie/mustache). -- This is an automated message from the 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 #7157: Fix TP DSR edit page not rendering
zrhoffman merged PR #7157: URL: https://github.com/apache/trafficcontrol/pull/7157 -- This is an automated message from the 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 #7156: Fix APIv4.0 returning APIv4.1 DS structures
zrhoffman commented on code in PR #7156:
URL: https://github.com/apache/trafficcontrol/pull/7156#discussion_r1004876267
##
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##
@@ -572,8 +614,10 @@ func (ds *TODeliveryService) Read(h http.Header, useIMS
bool) ([]interface{}, er
for _, ds := range dses {
switch {
// NOTE: it's required to handle minor version cases in a
descending >= manner
- case version.Major > 3:
+ case version.Major >= 4 && version.Minor >= 1:
returnable = append(returnable, ds.RemoveLD1AndLD2())
+ case version.Major >= 4:
+ returnable = append(returnable,
ds.DeliveryServiceV40.RemoveLD1AndLD2())
Review Comment:
The API 4.0 case matched for API 5.0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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, #7157: Fix TP DSR edit page not rendering
ocket opened a new pull request, #7157: URL: https://github.com/apache/trafficcontrol/pull/7157 Fixes a bug introduced in #7075 that prevented Delivery Service Request edit pages from rendering. ## Which Traffic Control components are affected by this PR? - Traffic Portal ## What is the best way to verify this PR? Make sure the page renders. ## If this is a bugfix, which Traffic Control versions contained the bug? - master ## PR submission checklist - [x] This PR uses existing 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 opened a new pull request, #7156: Fix APIv4.0 returning APIv4.1 DS structures
ocket opened a new pull request, #7156: URL: https://github.com/apache/trafficcontrol/pull/7156 This PR fixes a bug introduced by #7111 where APIv4.0 responses containing representations of Delivery Services used the representation from APIv4.1. ## Which Traffic Control components are affected by this PR? - Traffic Ops ## What is the best way to verify this PR? Make sure all the tests still pass, request Delivery Services at APIv4.0 and observe no `regional` property, then again at APIv4.1 and observe that it now *does* have the v4.1 `regional` property. ## If this is a bugfix, which Traffic Control versions contained the bug? - master ## PR submission checklist - [x] This PR utilizes existing tests - [x] This PR needs no documentation because the bug was never released and documentation matches the intended behavior, not the bugged behavior - [x] This PR needs no CHANGELOG.md entry because the bug was never released - [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] smalenfant commented on issue #7081: Traffic Router logs `SocketTimeoutException` for port 53 TCP health checks
smalenfant commented on issue #7081: URL: https://github.com/apache/trafficcontrol/issues/7081#issuecomment-1290868215 I used to turn those messages off in the past when that was a problem. I'm glad there is some resolution to this. ``` log4j.logger.com.comcast.cdn.traffic_control.traffic_router.core.dns.protocol.TCP=off ``` -- This is an automated message from the 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, #7155: t3c: initial mustache for remap.config templating
traeak opened a new pull request, #7155:
URL: https://github.com/apache/trafficcontrol/pull/7155
This allows remap.config to be modified by specifying a DS parameter with a
different template.
Template uses a golang mustache library.
const DefaultEdgeRemapConfigTemplate = `map {{source}} {{destination}}
{{strategy}} {{dscp}} {{header_rewrite}} {{drop_qstring}} {{signing}}
{{regex_remap}} {{cachekey}} {{range_requests}} {{maxmind}} {{pacing}}
{{raw_text}}`
## Which Traffic Control components are affected by this PR?
- Documentation
- Traffic Control Cache Config (`t3c`, formerly ORT)
- Traffic Control Health Client (tc-health-client)
- Traffic Control Client
- Traffic Monitor
- Traffic Ops
- Traffic Portal
- Traffic Router
- Traffic Stats
- Grove
- CDN in a Box
- Automation
- unknown
## What is the best way to verify this PR?
## 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] 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_r1003829984
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ 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 {
Review Comment:
Do you need the length checks if you make use of the `PageType` attribute?
##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ 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
[GitHub] [trafficcontrol] zrhoffman merged pull request #7096: Add health client parent health
zrhoffman merged PR #7096: URL: https://github.com/apache/trafficcontrol/pull/7096 -- This is an automated message from the 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] rob05c commented on a diff in pull request #7096: Add health client parent health
rob05c commented on code in PR #7096:
URL: https://github.com/apache/trafficcontrol/pull/7096#discussion_r1003764988
##
tc-health-client/config/config_test.go:
##
@@ -37,7 +37,7 @@ func TestLoadConfig(t *testing.T) {
}
cfg := Cfg{
- TrafficMonitors:make(map[string]bool, 0),
+ // TrafficMonitors:make(map[string]bool, 0),
Review Comment:
No, the data was moved to a different variable and constructed by default.
Removed
--
This is an automated message from the 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] rob05c commented on a diff in pull request #7096: Add health client parent health
rob05c commented on code in PR #7096:
URL: https://github.com/apache/trafficcontrol/pull/7096#discussion_r1003764401
##
tc-health-client/tmagent/markdownservice.go:
##
@@ -0,0 +1,484 @@
+package tmagent
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "bytes"
+ "errors"
+ "os/exec"
+ "path/filepath"
+ "strconv"
+ "time"
+
+ "github.com/apache/trafficcontrol/lib/go-log"
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-util"
+ "github.com/apache/trafficcontrol/tc-health-client/config"
+)
+
+// ParentStatus contains the trafficserver 'HostStatus' fields that
+// are necessary to interface with the trafficserver 'traffic_ctl' command.
+type ParentStatus struct {
+ Fqdn string
+ ActiveReason bool
+ LocalReason bool
+ ManualReason bool
+ LastTmPoll int64
+ UnavailablePollCount int
+ MarkUpPollCount int
+}
+
+// used to get the overall parent availablity from the
+// HostStatus markdown reasons. all markdown reasons
+// must be true for a parent to be considered available.
+func (p ParentStatus) available(reasonCode string) bool {
+ rc := false
+
+ switch reasonCode {
+ case "active":
+ rc = p.ActiveReason
+ case "local":
+ rc = p.LocalReason
+ case "manual":
+ rc = p.ManualReason
+ }
+ return rc
+}
+
+// used to log that a parent's status is either UP or
+// DOWN based upon the HostStatus reason codes. to
+// be considered UP, all reason codes must be 'true'.
+func (p ParentStatus) Status() string {
+ if !p.ActiveReason {
+ return "DOWN"
+ } else if !p.LocalReason {
+ return "DOWN"
+ } else if !p.ManualReason {
+ return "DOWN"
+ }
+ return "UP"
+}
+
+type StatusReason int
+
+// these are the HostStatus reason codes used within
+// trafficserver.
+const (
+ ACTIVE StatusReason = iota
+ LOCAL
+ MANUAL
+)
+
+// used for logging a parent's HostStatus reason code
+// setting.
+func (s StatusReason) String() string {
+ switch s {
+ case ACTIVE:
+ return "ACTIVE"
+ case LOCAL:
+ return "LOCAL"
+ case MANUAL:
+ return "MANUAL"
+ }
+ return "UNDEFINED"
+}
+
+// MarkdownServer contains symbols for manipulating a running MarkdownService
+// started by StartMarkdownService.
+type MarkdownServer struct {
+ // Shutdown signals the MarkdownService to stop its goroutines and
release all resources.
+ //
+ // It is not buffered. Writes will block until the markdown service is
done with any
+ // ongoing markdown operation. Once a write returns, the
MarkdownService will begin
+ // shutdown.
+ // TODO: add signal to callers when shutdown has finished.
+ Shutdown chan<- struct{}
+
+ // UpdateHealth signals to the markdown service that health status has
changed, and it should process markdowns again.
+ UpdateHealth func()
+}
+
+func StartMarkdownService(pi *ParentInfo, pollInterval time.Duration)
*MarkdownServer {
+ doneCh := make(chan struct{})
+ updateCh := make(chan struct{})
+ updateHealthF := func() {
+ select {
+ case updateCh <- struct{}{}:
+ default:
+ }
+ }
+
+ go markdownServicePoll(pi, pollInterval, doneCh, updateCh)
+ return {Shutdown: doneCh, UpdateHealth: updateHealthF}
+}
+
+func markdownServicePoll(pi *ParentInfo, pollInterval time.Duration, doneChan
<-chan struct{}, updateChan <-chan struct{}) {
+ for {
+ select {
+ case <-doneChan:
+ return
+ default:
+ <-updateChan
+ start := time.Now()
+ doMarkdown(pi)
+ log.Infof("poll-status poll=markdown ms=%v\n",
int(time.Since(start)/time.Millisecond))
+ time.Sleep(pollInterval)
+ }
+ }
[GitHub] [trafficcontrol] rob05c commented on a diff in pull request #7096: Add health client parent health
rob05c commented on code in PR #7096:
URL: https://github.com/apache/trafficcontrol/pull/7096#discussion_r1003762350
##
tc-health-client/sar/ephemeralportholder.go:
##
@@ -0,0 +1,77 @@
+package sar
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "errors"
+ "net"
+ "strconv"
+)
+
+// EphemeralPortHolder serves 2 purposes: it gets an ephemeral TCP port, and
it holds
+// onto the port so the OS doesn't assign it to any other app.
+//
+// It listens on :0 thereby getting a socket on an ephemeral port.
+// It continues listening but never reading from the socket until Close is
called.
+type EphemeralPortHolder struct {
+ listener net.Listener
+ port int
+}
+
+// GetAndHoldEphemeralPort gets an ephemeral port, and listens on it to prevent
+// the OS assigning the port to other apps.
+// Close must be called on the returned EphemeralPortHolder to stop listening.
+func GetAndHoldEphemeralPort(addr string) (*EphemeralPortHolder, error) {
+ addr += ":0"
+ listener, err := net.Listen("tcp", addr)
+ if err != nil {
+ return nil, errors.New("listening: " + err.Error())
+ }
+
+ // get the port now, so EphemeralPortHolder.Port() doesn't need to
return an error
+
+ listenAddr := listener.Addr().String()
+ ipPort := SplitLast(listenAddr, ":")
+ if len(ipPort) < 1 {
+ return nil, errors.New("malformed addr '" + listenAddr + "',
should have been ip:port") // should never happen
+ }
+ portStr := ipPort[1]
+ port, err := strconv.Atoi(portStr)
+ if err != nil {
+ return nil, errors.New("malformed addr '" + listenAddr + "'
port was not an integer") // should never happen
+ }
+ if port > 65535 || port < 0 {
Review Comment:
The number is technically the same, but "the last valid port number" is
semantically different than "the last number a uint16 can store." We could make
a constant, but it seems a bit overkill for just a port check, I'd prefer to
just keep the literal for 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] rob05c opened a new pull request, #7154: Add Traffic Ops Batch Endpoint
rob05c opened a new pull request, #7154:
URL: https://github.com/apache/trafficcontrol/pull/7154
This is a Proof-of-Concept. I just needed to prove to myself that it's
reasonable, it still needs the details worked out, docs, tests, etc. I'm making
a Draft PR in case anyone else is interested in it.
It allows users to make a single atomic request, such as
```
POST https://fqdn.example/api/5.0/batch/ -d '[{
"method": "GET",
"path": "/api/5.0/ping"
}, {
"method": "GET",
"path": "/api/5.0/cdns"
}]'
```
And get a response, with each member of the batch executed with sequential
consistency (that is, GETs may be executed in parallel, but everything will
appear to have been executed sequentially, e.g. GET-POST-GET will have the POST
changes in the second GET but not the first), like:
```
[ { "code": 200,
"headers": {
"Access-Control-Allow-Headers": [ "Origin, Content-Type" ],
"Content-Type": [ "application/json" ] },
"body": { "ping": "pong" }
}, {
"code": 200,
"headers": {
"Access-Control-Allow-Headers": [ "Origin, Content-Type" ],
"Content-Type": [ "application/json" ], },
"body": { "response": [ {
"id": 42,
"lastUpdated": "2001-01-01 12:34:56+00",
"name": "ALL"
}, {
"domainName": "cdn.example.net",
"id": 24,
"lastUpdated": "2001-01-01 12:34:56+00",
"name": "my-cdn"
} ] } } ]
```
The main goal here is atomicity, not convenience. By allowing users to
execute multiple requests atomically, we can make it safe to do things that
aren't safe today. For example, changing a Delivery Service, changing it's
Profile, and seven different Parameters, all at once. This doesn't completely
solve the Snapshot-Queue Automation a.k.a. "Kill the Chicken" problem, but it
addresses a large subset of it, in a much smaller feature than a complete
solution.
It's unlikely I'll have time to finish this. But anyone else is welcome to
fork and make a real PR from it.
## Which Traffic Control components are affected by this PR?
- Traffic Ops
## 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
- [ ] 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 a diff in pull request #7096: Add health client parent health
zrhoffman commented on code in PR #7096:
URL: https://github.com/apache/trafficcontrol/pull/7096#discussion_r1003601203
##
tc-health-client/config/config_test.go:
##
@@ -37,7 +37,7 @@ func TestLoadConfig(t *testing.T) {
}
cfg := Cfg{
- TrafficMonitors:make(map[string]bool, 0),
+ // TrafficMonitors:make(map[string]bool, 0),
Review Comment:
Does this commented need to be here?
##
tc-health-client/sar/ephemeralportholder.go:
##
@@ -0,0 +1,77 @@
+package sar
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "errors"
+ "net"
+ "strconv"
+)
+
+// EphemeralPortHolder serves 2 purposes: it gets an ephemeral TCP port, and
it holds
+// onto the port so the OS doesn't assign it to any other app.
+//
+// It listens on :0 thereby getting a socket on an ephemeral port.
+// It continues listening but never reading from the socket until Close is
called.
+type EphemeralPortHolder struct {
+ listener net.Listener
+ port int
+}
+
+// GetAndHoldEphemeralPort gets an ephemeral port, and listens on it to prevent
+// the OS assigning the port to other apps.
+// Close must be called on the returned EphemeralPortHolder to stop listening.
+func GetAndHoldEphemeralPort(addr string) (*EphemeralPortHolder, error) {
+ addr += ":0"
+ listener, err := net.Listen("tcp", addr)
+ if err != nil {
+ return nil, errors.New("listening: " + err.Error())
+ }
+
+ // get the port now, so EphemeralPortHolder.Port() doesn't need to
return an error
+
+ listenAddr := listener.Addr().String()
+ ipPort := SplitLast(listenAddr, ":")
+ if len(ipPort) < 1 {
+ return nil, errors.New("malformed addr '" + listenAddr + "',
should have been ip:port") // should never happen
+ }
+ portStr := ipPort[1]
+ port, err := strconv.Atoi(portStr)
+ if err != nil {
+ return nil, errors.New("malformed addr '" + listenAddr + "'
port was not an integer") // should never happen
+ }
+ if port > 65535 || port < 0 {
Review Comment:
Nit: `65535` can be `math.MaxUint16`
##
tc-health-client/tmagent/markdownservice.go:
##
@@ -0,0 +1,484 @@
+package tmagent
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "bytes"
+ "errors"
+ "os/exec"
+ "path/filepath"
+ "strconv"
+ "time"
+
+ "github.com/apache/trafficcontrol/lib/go-log"
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-util"
+ "github.com/apache/trafficcontrol/tc-health-client/config"
+)
+
+// ParentStatus contains the trafficserver 'HostStatus' fields that
+// are necessary to interface with the trafficserver 'traffic_ctl' command.
+type ParentStatus struct {
+ Fqdn string
+ ActiveReason bool
+ LocalReason bool
+ ManualReason bool
+ LastTmPoll int64
+ UnavailablePollCount int
+ MarkUpPollCount int
+}
+
+// used to get the overall parent availablity from the
+// HostStatus markdown reasons. all markdown reasons
+// must be true for a parent to be considered available.
+func (p ParentStatus) available(reasonCode string) bool {
+ rc := false
+
+ switch reasonCode {
+ case
[GitHub] [trafficcontrol] traeak opened a new pull request, #7153: T3c bad cert
traeak opened a new pull request, #7153: URL: https://github.com/apache/trafficcontrol/pull/7153 t3c will panic and fail when a pem.Decode fails for a base64 decoded cert. This adds extra logic to identify and skip a cert that fails this way. ## Which Traffic Control components are affected by this PR? - Traffic Control Cache Config (`t3c`, formerly ORT) ## What is the best way to verify this PR? Find some way to corrupt a delivery service SSL keys cert. Look for ERROR with invalid cert message. ## If this is a bugfix, which Traffic Control versions contained the bug? 7.1.0 ## PR submission checklist - [ ] This PR has tests - [ ] This PR has documentation - [x] This PR has a CHANGELOG.md entry - [ ] 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] ericholguin opened a new pull request, #7152: Use Generics in TO Integration Tests
ericholguin opened a new pull request, #7152: URL: https://github.com/apache/trafficcontrol/pull/7152 Updated the tests to use the new generics test struct ## Which Traffic Control components are affected by this PR? - Traffic Ops ## What is the best way to verify this PR? Run TO Integration tests ## 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] ericholguin closed pull request #7151: Use Generics in TO Integration Tests
ericholguin closed pull request #7151: Use Generics in TO Integration Tests URL: https://github.com/apache/trafficcontrol/pull/7151 -- This is an automated message from the 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 a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services
traeak commented on code in PR #7137:
URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r1003289241
##
lib/go-atscfg/parentdotconfig.go:
##
@@ -383,258 +390,112 @@ func makeParentDotConfigData(
continue
}
- isMSO := ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin
+ // manufacture a topology for this DS.
+ if ds.Topology == nil || *ds.Topology == "" {
+ // only populate if there are non topology ds's
+ if len(ocgmap) == 0 {
+ ocgmap = makeOCGMap(parentInfos)
+ if len(ocgmap) == 0 {
+ ocgmap[""] = []string{}
+ }
+ }
- // TODO put these in separate functions. No if-statement should
be this long.
- if ds.Topology != nil && *ds.Topology != "" {
- pasvc, topoWarnings, err := getTopologyParentConfigLine(
- server,
- serversWithParams,
- ,
- serverParams,
- parentConfigParams,
- nameTopologies,
- serverCapabilities,
- dsRequiredCapabilities,
- cacheGroups,
- profileParentConfigParams,
- isMSO,
- atsMajorVersion,
- dsOrigins[DeliveryServiceID(*ds.ID)],
- opt.AddComments,
- )
- warnings = append(warnings, topoWarnings...)
+ orgFQDNStr := *ds.OrgServerFQDN
+ orgURI, orgWarns, err := getOriginURI(orgFQDNStr)
+ warnings = append(warnings, orgWarns...)
if err != nil {
- // we don't want to fail generation with an
error if one ds is malformed
- warnings = append(warnings, err.Error()) //
getTopologyParentConfigLine includes error context
+ warnings = append(warnings, "DS '"+*ds.XMLID+"'
has malformed origin URI: '"+orgFQDNStr+"': skipping!"+err.Error())
continue
}
- if pasvc != nil { // will be nil with no error if this
server isn't in the Topology, or if it doesn't have the Required Capabilities
- parentAbstraction.Services =
append(parentAbstraction.Services, pasvc)
- }
- } else {
- isLastCacheTier :=
noTopologyServerIsLastCacheForDS(server, , cacheGroups)
- serverPlacement := TopologyPlacement{
- IsLastCacheTier: isLastCacheTier,
- IsFirstCacheTier: !isLastCacheTier ||
!ds.Type.UsesMidCache(),
- }
-
- dsParams, dswarns := getParentDSParams(ds,
profileParentConfigParams, serverPlacement, isMSO)
- warnings = append(warnings, dswarns...)
-
- if cacheIsTopLevel {
- parentQStr := false
- if dsParams.QueryStringHandling == "" &&
dsParams.Algorithm == tc.AlgorithmConsistentHash && ds.QStringIgnore != nil &&
tc.QStringIgnore(*ds.QStringIgnore) == tc.QStringIgnoreUseInCacheKeyAndPassUp {
- parentQStr = true
- }
-
- orgFQDNStr := *ds.OrgServerFQDN
- // if this cache isn't the last tier, i.e.
we're not going to the origin, use http not https
- if !isLastCacheTier {
- orgFQDNStr =
strings.Replace(orgFQDNStr, `https://`, `http://`, -1)
- }
- orgURI, orgWarns, err :=
getOriginURI(orgFQDNStr)
- warnings = append(warnings, orgWarns...)
- if err != nil {
- warnings = append(warnings, "DS
'"+*ds.XMLID+"' has malformed origin URI: '"+orgFQDNStr+"':
skipping!"+err.Error())
+ // use the topology name for the fqdn
+ topoName := orgURI.Hostname()
+ cgnames, ok := ocgmap[OriginHost(topoName)]
+ if !ok {
+ topoName = deliveryServicesAllParentsKey
+ cgnames, ok = ocgmap[OriginHost(topoName)]
+ if !ok {
+
[GitHub] [trafficcontrol] traeak commented on a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services
traeak commented on code in PR #7137: URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r1003288538 ## lib/go-atscfg/parentdotconfig.go: ## @@ -383,258 +390,112 @@ func makeParentDotConfigData( continue } - isMSO := ds.MultiSiteOrigin != nil && *ds.MultiSiteOrigin + // manufacture a topology for this DS. Review Comment: moved to CreateTopologies function although there are quite a few args and return variables :( -- This is an automated message from the 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 a diff in pull request #7137: T3C parent.config simulate topology for non topo delivery services
traeak commented on code in PR #7137: URL: https://github.com/apache/trafficcontrol/pull/7137#discussion_r1003288096 ## lib/go-atscfg/parentdotconfig.go: ## @@ -165,6 +165,11 @@ func MakeParentDotConfig( }, nil } +// Check if this ds type is edge only Review Comment: comment changed and moved. -- This is an automated message from the 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 #7151: Use Generics in TO Integration Tests
zrhoffman commented on code in PR #7151:
URL: https://github.com/apache/trafficcontrol/pull/7151#discussion_r1002131580
##
traffic_ops/testing/api/v5/cachegroups_test.go:
##
@@ -65,184 +67,173 @@ func TestCacheGroups(t *testing.T) {
Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)),
},
"OK when VALID request": {
- ClientSession: TOSession, Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+ ClientSession: TOSession,
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
},
"OK when VALID NAME parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name":
{"parentCachegroup"}}},
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"name":
{"parentCachegroup"}}},
Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseHasLength(1),
ValidateExpectedField("Name",
"parentCachegroup")),
},
"OK when VALID SHORTNAME parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"shortName": {"pg2"}}},
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"shortName": {"pg2"}}},
Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseHasLength(1),
ValidateExpectedField("ShortName", "pg2")),
},
"OK when VALID TOPOLOGY parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"topology":
{"mso-topology"}}},
- Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"topology":
{"mso-topology"}}},
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
},
"OK when VALID TYPE parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"type": {"ORG_LOC"}}},
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"type":
{strconv.Itoa(GetTypeId(t, "ORG_LOC"))}}},
Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseLengthGreaterOrEqual(1),
ValidateExpectedField("TypeName", "ORG_LOC")),
},
"EMPTY RESPONSE when INVALID ID parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"id": {"1"}}},
- Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseHasLength(0)),
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"id": {"1"}}},
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseHasLength(0)),
},
"EMPTY RESPONSE when INVALID TYPE parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"type": {"1"}}},
- Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
utils.ResponseHasLength(0)),
+ ClientSession: TOSession,
+ RequestOpts:
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #7151: Use Generics in TO Integration Tests
zrhoffman commented on code in PR #7151:
URL: https://github.com/apache/trafficcontrol/pull/7151#discussion_r1002127862
##
traffic_ops/testing/api/v5/deliveryservices_required_capabilities_test.go:
##
@@ -79,119 +83,123 @@ func TestDeliveryServicesRequiredCapabilities(t
*testing.T) {
Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
validateDSRCPagination("page")),
},
"BAD REQUEST when INVALID LIMIT parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"limit": {"-2"}}},
- Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"limit": {"-2"}}},
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"BAD REQUEST when INVALID OFFSET parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "offset":
{"0"}}},
- Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "offset":
{"0"}}},
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"BAD REQUEST when INVALID PAGE parameter": {
- ClientSession: TOSession, RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "page":
{"0"}}},
- Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "page":
{"0"}}},
+ Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
+ },
+ "OK when CHANGES made": {
+ ClientSession: TOSession,
+ RequestOpts:
client.RequestOptions{Header: http.Header{rfc.IfModifiedSince:
{currentTimeRFC}}},
+ Expectations:
utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
},
},
"POST": {
"BAD REQUEST when REASSIGNING REQUIRED
CAPABILITY to DELIVERY SERVICE": {
- ClientSession: TOSession, RequestBody:
map[string]interface{}{
- "deliveryServiceID":
GetDeliveryServiceId(t, "ds1")(),
- "requiredCapability": "foo",
+ ClientSession: TOSession,
+ RequestBody:
tc.DeliveryServicesRequiredCapability{
+ DeliveryServiceID:
util.IntPtr(GetDeliveryServiceId(t, "ds1")()),
+ RequiredCapability:
util.StrPtr("foo"),
},
Expectations:
utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"BAD REQUEST when SERVERS DONT have
CAPABILITY": {
- ClientSession: TOSession, RequestBody:
map[string]interface{}{
- "deliveryServiceID":
GetDeliveryServiceId(t, "test-ds-server-assignments")(),
- "requiredCapability": "disk",
+ ClientSession: TOSession,
+ RequestBody:
tc.DeliveryServicesRequiredCapability{
+ DeliveryServiceID:
util.IntPtr(GetDeliveryServiceId(t, "test-ds-server-assignments")()),
+ RequiredCapability:
util.StrPtr("disk"),
},
[GitHub] [trafficcontrol] ericholguin opened a new pull request, #7151: Use Generics in TO Integration Tests
ericholguin opened a new pull request, #7151: URL: https://github.com/apache/trafficcontrol/pull/7151 Updated the tests to use the new generics test struct ## 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 - [ ] 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] 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_r1002009697 ## 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: Most likely will be removing this param from prod since there isn't a specific use case. -- This is an automated message from the 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_r1001067763
##
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:
Well what I'm saying is that since they don't use v5 yet, they can just
change the parameter they send to be TRAFFIC_MONITOR instead of RASCAL at that
point.
--
This is an automated message from the 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 #7144: Use Generics in TO Integration Tests
zrhoffman merged PR #7144: URL: https://github.com/apache/trafficcontrol/pull/7144 -- This is an automated message from the 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 #7150: Retract versions v1.1.0 through v1.1.5 from Go modules
zrhoffman commented on PR #7150: URL: https://github.com/apache/trafficcontrol/pull/7150#issuecomment-1285947757 We could also tag it something like `v1.1.5-retraction`, it doesn't need to be named only `v1.1.5` -- This is an automated message from the 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, #7150: Retract versions v1.1.0 through v1.1.5 from Go modules
zrhoffman opened a new pull request, #7150:
URL: https://github.com/apache/trafficcontrol/pull/7150
This PR retracts `v1.1.0`, `v1.1.1`, `v1.1.2`, `v1.1.3`, `v1.1.4`, and
`v1.1.5` from Go modules. If this PR is merged, it should be tagged version
`v1.1.5`.
Without this PR and tag `v1.1.5`, a user running
```shell
go mod init my-module
go get github.com/apache/trafficcontrol@817a702a9de6
```
will retrieve dependency `github.com/apache/trafficcontrol` at version
`v1.1.4-0.20201118150121-817a702a9de6`. With this PR merged and tagged
`v1.1.5`, the same `go get` command will yield version
`v0.0.0-20201118150121-817a702a9de6`.
## Which Traffic Control components are affected by this PR?
- go.mod
## What is the best way to verify this PR?
```shell
git clone https://github.com/apache/trafficcontrol
cd trafficcontrol
# create github.com:my-username/nonfork using the GitHub UI
git remote add nonfork g...@github.com:my-username/nonfork.git
git push nonfork --all
git push nonfork --tags
sed -i 's|github\.com/apache/trafficcontrol|github.com/my-username/nonfork|'
go.mod
git add go.mod
git commit -m 'Update module path to github.com/my-username/nonfork'
git push nonfork HEAD:master
```
In a different window, try getting a 4.1.x commit:
```shell
mkdir my-project
cd my-project
go mod init my-project
go get github.com/my-fork/nonfork@817a702a9d
```
Expected computed version: `v1.1.4-0.20201118150121-817a702a9de6`
Back in the first window, retract v1:
```shell
<<'APPEND' cat >> go.mod
// Although the Apache Trafic Control considers v7.0.1 stable, Go modules
consider it unstable because its major version
// is greater than 1, and without these retractions, a commit like
c076b138a88d will have a computed vesion of
// v1.1.4-0.20220817163951-c076b138a88d
// With these retractions, the same commit will have a computed version of
// v0.0.0-0.20220817163951-c076b138a88d
// See
https://pkg.go.dev/cmd/go@go1.15.15#hdr-Module_compatibility_and_semantic_versioning
for details.
retract (
v1.1.5
v1.1.4
v1.1.3
v1.1.2
v1.1.1
v1.1.0
)
APPEND
git add go.mod
git commit -m 'Retract v1'
git tag v1.1.5
git push nonfork HEAD:master v1.1.5
```
Back in the `my-project` window, try importing the 4.1.x commit again:
```shell
sudo rm -rf ${GOPATH}/pkg
go get github.com/my-username/nonfork@817a702a9de6
```
Expected computed version: `v0.0.0-20201118150121-817a702a9de6`
## PR submission checklist
- [ ] This PR has tests
- [x] This PR has comments
- [ ] 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] ocket8888 merged pull request #7118: Remove ASN from servers
ocket merged PR #7118: URL: https://github.com/apache/trafficcontrol/pull/7118 -- This is an automated message from the 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 #7144: Use Generics in TO Integration Tests
ericholguin commented on code in PR #7144: URL: https://github.com/apache/trafficcontrol/pull/7144#discussion_r1000939988 ## traffic_ops/testing/api/v5/servers_hostname_update_status_test.go: ## @@ -1,18 +1,17 @@ package v5 /* + 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 - 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 - 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. + 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. Review Comment: Oops -- This is an automated message from the 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 #7144: Use Generics in TO Integration Tests
ericholguin commented on code in PR #7144:
URL: https://github.com/apache/trafficcontrol/pull/7144#discussion_r1000939559
##
traffic_ops/testing/api/v4/server_server_capabilities_test.go:
##
@@ -216,25 +189,26 @@ func TestServerServerCapabilities(t *testing.T) {
})
case "POST":
t.Run(name, func(t *testing.T) {
- alerts, reqInf, err :=
testCase.ClientSession.CreateServerServerCapability(ssc, testCase.RequestOpts)
- for _, check := range
testCase.Expectations {
- check(t,
reqInf, nil, alerts, err)
- }
- })
- case "PUT":
Review Comment:
server_server_capabilities doesn't have a PUT method, just moved that test
for that route into its own test file: multiple_server_capabilities_test.go
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #7144: Use Generics in TO Integration Tests
zrhoffman commented on code in PR #7144:
URL: https://github.com/apache/trafficcontrol/pull/7144#discussion_r1000915015
##
traffic_ops/testing/api/v4/server_server_capabilities_test.go:
##
@@ -216,25 +189,26 @@ func TestServerServerCapabilities(t *testing.T) {
})
case "POST":
t.Run(name, func(t *testing.T) {
- alerts, reqInf, err :=
testCase.ClientSession.CreateServerServerCapability(ssc, testCase.RequestOpts)
- for _, check := range
testCase.Expectations {
- check(t,
reqInf, nil, alerts, err)
- }
- })
- case "PUT":
Review Comment:
Why take out PUT?
##
traffic_ops/testing/api/v5/servers_id_queue_update_test.go:
##
@@ -1,18 +1,17 @@
package v5
/*
+ 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
- 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
- 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.
+ 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.
Review Comment:
No need to change the license formatting
##
traffic_ops/testing/api/v5/servers_hostname_update_status_test.go:
##
@@ -1,18 +1,17 @@
package v5
/*
+ 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
- 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
- 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.
+ 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.
Review Comment:
No need to change the license formatting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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_r157834
##
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 name to associate
with a :term:`Server` id
+
+.. code-block:: http
+ :caption: Request Example1
+
+ PUT /api/5.0/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
+
+ PUT /api/5.0/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
+ :caption: Response Example1
+
+ HTTP/1.1 200 OK
+ Access-Control-Allow-Credentials: true
+ Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type,
Accept, Set-Cookie, Cookie
+ Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+ Access-Control-Allow-Origin: *
+ Content-Type: application/json
+ Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 8 Aug 2022 22:40:54
GMT; Max-Age=3600; HttpOnly
+ Whole-Content-Sha512:
eQrl48zWids0kDpfCYmmtYMpegjnFxfOVvlBYxxLSfp7P7p6oWX4uiC+/Cfh2X9i3G+MQ36eH95gukJqOBOGbQ==
+ X-Server-Name: traffic_ops_golang/
+ Date: Mon, 08 Aug 2022 16:15:11 GMT
+ Content-Length: 157
+
+ {
+ "alerts": [{
+ "text": "Multiple Server Capabilities assigned to a
server",
+ "level": "success"
+ }],
+ "response": {
+ "serverIds": [1],
+ "serverCapabilities": ["test", "disk"]
+ }
+ }
+
+.. code-block:: http
+ :caption: Response Example2
+
+ HTTP/1.1 200 OK
+ Access-Control-Allow-Credentials: true
+ Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type,
Accept, Set-Cookie, Cookie
+ Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+ Access-Control-Allow-Origin: *
+ Content-Type: application/json
+ Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 8 Aug 2022 22:40:54
GMT; Max-Age=3600; HttpOnly
+ Whole-Content-Sha512:
eQrl48zWids0kDpfCYmmtYMpegjnFxfOVvlBYxxLSfp7P7p6oWX4uiC+/Cfh2X9i3G+MQ36eH95gukJqOBOGbQ==
+ X-Server-Name: traffic_ops_golang/
+ Date: Mon, 08 Aug 2022 16:15:11 GMT
+ Content-Length: 157
+ {
Review Comment:
Yeah, you already fixed this
> I know for sure one's already been addressed, so I'll resolve it
immediately.
This was supposed to be resolved already.
--
This is an automated message from the Apache Git Service.
To respond to
[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #6532: CDN-in-a-Box for Developers
zrhoffman commented on code in PR #6532: URL: https://github.com/apache/trafficcontrol/pull/6532#discussion_r999826379 ## dev/traffic_router/conf/traffic_ops.properties: ## @@ -0,0 +1,13 @@ +# 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. +traffic_ops.username=admin +traffic_ops.password=twelve12 Review Comment: > The password used here should ideally match the current CiaB for ease of use, but CiaB should not contain data we know to be invalid. Okay, PRed making the password `twelve` in both places in #7149 -- This is an automated message from the 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, #7149: Use "twelve" as the password where "twelve12" was used
zrhoffman opened a new pull request, #7149: URL: https://github.com/apache/trafficcontrol/pull/7149 #4764 changed the default password in some places, but not all, from `twelve` to `twelve12`. This is confusing for all of the people who are used to `twelve` being the default password for all components of the project. This PR reverts all instances of `twelve12` to `twelve`. ## Which Traffic Control components are affected by this PR? - CDN in a Box - CDN in a Box for Developers - Traffic Portal - Traffic Portal v2 ## What is the best way to verify this PR? Verify the tests pass ## PR submission checklist - [x] This PR has tests - [x] 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] shamrickus merged pull request #7148: Check the exit code of API v5 tests also
shamrickus merged PR #7148: URL: https://github.com/apache/trafficcontrol/pull/7148 -- This is an automated message from the 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, #7148: Check the exit code of API v5 tests also
zrhoffman opened a new pull request, #7148: URL: https://github.com/apache/trafficcontrol/pull/7148 This PR updates the TO API tests service in CDN in a Box to check the exit code for the API v5 tests. ## Which Traffic Control components are affected by this PR? - CDN in a Box - TO API tests ## What is the best way to verify this PR? Run API tests in CDN in a Box, verify that if the v5 API tests fail, then the `integration` service exits with a nonzero exit code ## If this is a bugfix, which Traffic Control versions contained the bug? - master branch ## 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] smalenfant commented on issue #7128: API acme_autorenew: Renewal are not being kicked off
smalenfant commented on issue #7128: URL: https://github.com/apache/trafficcontrol/issues/7128#issuecomment-1284356148 I also noticed that the expiration is not set when doing the riak->postgreql migration. Not sure if this could affect the functionality of the renewal. -- This is an automated message from the 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 #7128: API acme_autorenew: Renewal are not being kicked off
mitchell852 commented on issue #7128: URL: https://github.com/apache/trafficcontrol/issues/7128#issuecomment-1284340857 > @mitchell852 It's in there already. Maybe not clear. `After upgrading to 6.1.0 from 5.1.2` cool. guess i didn't see 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] smalenfant commented on issue #7128: API acme_autorenew: Renewal are not being kicked off
smalenfant commented on issue #7128: URL: https://github.com/apache/trafficcontrol/issues/7128#issuecomment-1284334826 @mitchell852 It's in there already. Maybe not clear. `After upgrading to 6.1.0 from 5.1.2` -- This is an automated message from the 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] smalenfant commented on issue #7129: Let's Encrypt certificates sometimes fails to write to Traffic Vault - postgresql
smalenfant commented on issue #7129: URL: https://github.com/apache/trafficcontrol/issues/7129#issuecomment-1284334179 @mitchell852 I added. I'm pretty this should also affect master/7.1.x as I don't see changes regarding the backend. -- This is an automated message from the 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 #7147: Include TO API v5 in CiaB TO API Tests
shamrickus merged PR #7147: URL: https://github.com/apache/trafficcontrol/pull/7147 -- This is an automated message from the 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, #7147: Include TO API v5 in CiaB TO API Tests
zrhoffman opened a new pull request, #7147: URL: https://github.com/apache/trafficcontrol/pull/7147 This PR updates the TO API tests service in CDN in a Box to include API v5 (and not try to print API v2 results). ## Which Traffic Control components are affected by this PR? - CDN in a Box - TO API tests ## What is the best way to verify this PR? Run API tests, verify the generated `junit` directory includes an xml ile for API v5 ## If this is a bugfix, which Traffic Control versions contained the bug? - master branch ## 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 closed issue #7081: Traffic Router logs `SocketTimeoutException` for port 53 TCP health checks
zrhoffman closed issue #7081: Traffic Router logs `SocketTimeoutException` for port 53 TCP health checks URL: https://github.com/apache/trafficcontrol/issues/7081 -- This is an automated message from the 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 #7146: Better messages for Traffic Router exceptions
zrhoffman merged PR #7146: URL: https://github.com/apache/trafficcontrol/pull/7146 -- This is an automated message from the 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 #7146: Better messages for Traffic Router exceptions
zrhoffman commented on code in PR #7146:
URL: https://github.com/apache/trafficcontrol/pull/7146#discussion_r999684460
##
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/protocol/TCP.java:
##
@@ -112,6 +115,20 @@ public void run() {
os.write(response);
} catch (final WireParseException e) {
// This is already recorded in the access log
+} catch (final SocketTimeoutException e) {
+String hostAddress = "unknown";
+if (client != null) {
+hostAddress = client.getHostAddress();
+}
+LOGGER.error("The socket with the Client at: " +
+hostAddress + " has timed out. Error: " +
e.getMessage(), e);
Review Comment:
This still prints the stack trace, which we don't need in this specific case.
```java
ERROR 2022-10-19T16:17:45.535 [pool-13-thread-1]
org.apache.traffic_control.traffic_router.core.dns.protocol.TCP - The socket
with the Client at: 2001:3984:3989:0:0:0:0:3 has timed out. Error: Read timed
out
java.net.SocketTimeoutException: Read timed out
at java.net.SocketInputStream.socketRead0(Native Method) ~[?:?]
at java.net.SocketInputStream.socketRead(SocketInputStream.java:115)
~[?:?]
at java.net.SocketInputStream.read(SocketInputStream.java:168) ~[?:?]
at java.net.SocketInputStream.read(SocketInputStream.java:140) ~[?:?]
at
java.nio.channels.Channels$ReadableByteChannelImpl.read(Channels.java:388)
~[?:?]
at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:65) ~[?:?]
at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:107)
~[?:?]
at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:101)
~[?:?]
at java.io.DataInputStream.readFully(DataInputStream.java:200) ~[?:?]
at java.io.DataInputStream.readFully(DataInputStream.java:170) ~[?:?]
at
org.apache.traffic_control.traffic_router.core.dns.protocol.TCP$TCPSocketHandler.run(TCP.java:111)
[classes/:?]
at
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
[?:?]
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
[?:?]
at java.lang.Thread.run(Thread.java:829) [?:?]
In the dev CDN in a Box, ran
```shell
docker-compose exec trafficops wget -O- trafficrouter:53
```
--
This is an automated message from the 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_r999632370 ## 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 believe this parameter is related to TMs created for distributed TM feature. -- This is an automated message from the 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 #7145: Bump @xmldom/xmldom from 0.7.5 to 0.7.6 in /traffic_portal/test/integration
shamrickus merged PR #7145: URL: https://github.com/apache/trafficcontrol/pull/7145 -- This is an automated message from the 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 closed issue #7134: Remove cookie user errors from TO error.log
shamrickus closed issue #7134: Remove cookie user errors from TO error.log URL: https://github.com/apache/trafficcontrol/issues/7134 -- This is an automated message from the 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 #7138: Remove expired cookie and bad signature cookie errors from error.log
shamrickus merged PR #7138: URL: https://github.com/apache/trafficcontrol/pull/7138 -- This is an automated message from the 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 #7146: Better messages for Traffic Router exceptions
zrhoffman commented on code in PR #7146:
URL: https://github.com/apache/trafficcontrol/pull/7146#discussion_r998716177
##
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/protocol/TCP.java:
##
@@ -112,6 +114,20 @@ public void run() {
os.write(response);
} catch (final WireParseException e) {
// This is already recorded in the access log
+} catch (final SocketTimeoutException e) {
+String hostAddress = "unknown";
+if (client != null) {
+hostAddress = client.getHostAddress();
+}
+LOGGER.error("The socket with the Client at: " +
+hostAddress + " has timed out. Error: " +
e.getMessage(), e);
+} catch (final EOFException e) {
Review Comment:
Syntax error
```
core/dns/protocol/TCP.java:[124,27] error: cannot find symbol
```
--
This is an automated message from the 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 closed issue #2388: Global DNS TTL adjustment
srijeet0406 closed issue #2388: Global DNS TTL adjustment URL: https://github.com/apache/trafficcontrol/issues/2388 -- This is an automated message from the 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 #7143: Override all Delivery Service TTLs in a CDN's Snapshot with a given value
srijeet0406 merged PR #7143: URL: https://github.com/apache/trafficcontrol/pull/7143 -- This is an automated message from the 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 #7118: Remove ASN from servers
srijeet0406 commented on code in PR #7118:
URL: https://github.com/apache/trafficcontrol/pull/7118#discussion_r998504903
##
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:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
