[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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

2022-10-24 Thread GitBox


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