You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/08/20 18:29:26 UTC

[GitHub] [trafficcontrol] jrushford opened a new pull request #4980: ort t3c command fixes:

jrushford opened a new pull request #4980:
URL: https://github.com/apache/trafficcontrol/pull/4980


   Fixes the following problems found while integration testing the ORT t3c command.
     
     - use the atstccfg --traffic-ops-insecure flag to allow for self signed certs.
     - insure that atstccfg logging locations do not use 'stdout' as using 'stdout'
       will cause json parsing errors.
     - corrects the --help output to correctly show the default logging locations for
       t3c is 'stderr'.
   
   <!--
   
   ## What does this PR (Pull Request) do?
   Fixes bugs found while integration testing the ORT t3c command:
   
   -  use the atstccfg --traffic-ops-insecure flag to allow for self signed certs.
   - insure that atstccfg logging locations do not use 'stdout' as using 'stdout'
       will cause json parsing errors.
   - corrects the --help output to correctly show the default logging locations for
       t3c is 'stderr'.
   
   - [X] This PR fixe is not related to any Issue 
   
   ## Which Traffic Control components are affected by this PR?
   - Traffic Ops ORT
   
   ## What is the best way to verify this PR?
   Run the integration tests.
   
   ## If this is a bug fix, what versions of Traffic Control are affected?
   
   - master (f55bb5a5)
   
   ## The following criteria are ALL met by this PR
   
   - [X] This PR includes tests OR I have explained why tests are unnecessary
   - [ ] This PR includes documentation OR I have explained why documentation is unnecessary
   - [ ] This PR includes an update to CHANGELOG.md OR such an update is not necessary
   - [ ] This PR includes any and all required license headers
   - [ ] This PR ensures that database migration sequence is correct OR this PR does not include a database migration
   - [ ] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
   
   
   ## Additional Information
   <!-- If you would like to include any additional information on the PR for
   potential reviewers please put it here.
   
   Some examples of this would be:
   
   - Before and after screenshots/gifs of the Traffic Portal if it is affected
   - Links to other dependent Pull Requests
   - References to relevant context (e.g. new/updates to dependent libraries,
   mailing list records, blueprints)
   
   Feel free to leave this section blank (or, preferably, delete it entirely).
   -->
   
   <!--
   Licensed to the Apache Software Foundation (ASF) under one
   or more contributor license agreements.  See the NOTICE file
   distributed with this work for additional information
   regarding copyright ownership.  The ASF licenses this file
   to you under the Apache License, Version 2.0 (the
   "License"); you may not use this file except in compliance
   with the License.  You may obtain a copy of the License at
   
       http://www.apache.org/licenses/LICENSE-2.0
   
   Unless required by applicable law or agreed to in writing,
   software distributed under the License is distributed on an
   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
   KIND, either express or implied.  See the License for the
   specific language governing permissions and limitations
   under the License.
   -->
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficcontrol] jrushford commented on a change in pull request #4980: ORT t3c command fixes:

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #4980:
URL: https://github.com/apache/trafficcontrol/pull/4980#discussion_r474729743



##########
File path: traffic_ops_ort/t3c/torequest/torequest.go
##########
@@ -193,16 +193,35 @@ func (r *TrafficOpsReq) atsTcExec(cmdstr string) ([]byte, error) {
 
 // atsTcExecCommand is used to run the atstccfg command.
 func (r *TrafficOpsReq) atsTcExecCommand(cmdstr string, queueState int, revalState int) ([]byte, error) {
+	// adjust log locations used for atstccfg
+	// cannot use stdout as this will cause json parsing errors.
+	errorLocation := r.Cfg.LogLocationErr
+	if errorLocation == "stdout" {
+		errorLocation = "stderr"
+		log.Infoln("atstccfg error logging has been re-directed to 'stderr'")
+	}
+	infoLocation := r.Cfg.LogLocationInfo
+	if infoLocation == "stdout" {
+		infoLocation = "stderr"
+		log.Infoln("atstccfg info logging has been re-directed to 'stderr'")
+	}
+	warningLocation := r.Cfg.LogLocationWarn
+	if warningLocation == "stdout" {
+		warningLocation = "stderr"
+		log.Infoln("atstccfg warning logging has been re-directed to 'stderr'")
+	}
+
 	args := []string{
 		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(r.Cfg.TOTimeoutMS), 10),
 		"--traffic-ops-disable-proxy=" + strconv.FormatBool(r.Cfg.ReverseProxyDisable),
 		"--traffic-ops-user=" + r.Cfg.TOUser,
 		"--traffic-ops-password=" + r.Cfg.TOPass,
 		"--traffic-ops-url=" + r.Cfg.TOURL,
 		"--cache-host-name=" + r.Cfg.CacheHostName,
-		"--log-location-error=" + r.Cfg.LogLocationErr,
-		"--log-location-info=" + r.Cfg.LogLocationInfo,
-		"--log-location-warning=" + r.Cfg.LogLocationWarn,
+		"--log-location-error=" + errorLocation,
+		"--log-location-info=" + infoLocation,
+		"--log-location-warning=" + warningLocation,
+		"--traffic-ops-insecure",

Review comment:
       Sure, I'll add that.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficcontrol] rob05c commented on a change in pull request #4980: ORT t3c command fixes:

Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4980:
URL: https://github.com/apache/trafficcontrol/pull/4980#discussion_r474294201



##########
File path: traffic_ops_ort/t3c/torequest/torequest.go
##########
@@ -193,16 +193,35 @@ func (r *TrafficOpsReq) atsTcExec(cmdstr string) ([]byte, error) {
 
 // atsTcExecCommand is used to run the atstccfg command.
 func (r *TrafficOpsReq) atsTcExecCommand(cmdstr string, queueState int, revalState int) ([]byte, error) {
+	// adjust log locations used for atstccfg
+	// cannot use stdout as this will cause json parsing errors.
+	errorLocation := r.Cfg.LogLocationErr
+	if errorLocation == "stdout" {
+		errorLocation = "stderr"
+		log.Infoln("atstccfg error logging has been re-directed to 'stderr'")
+	}
+	infoLocation := r.Cfg.LogLocationInfo
+	if infoLocation == "stdout" {
+		infoLocation = "stderr"
+		log.Infoln("atstccfg info logging has been re-directed to 'stderr'")
+	}
+	warningLocation := r.Cfg.LogLocationWarn
+	if warningLocation == "stdout" {
+		warningLocation = "stderr"
+		log.Infoln("atstccfg warning logging has been re-directed to 'stderr'")
+	}
+
 	args := []string{
 		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(r.Cfg.TOTimeoutMS), 10),
 		"--traffic-ops-disable-proxy=" + strconv.FormatBool(r.Cfg.ReverseProxyDisable),
 		"--traffic-ops-user=" + r.Cfg.TOUser,
 		"--traffic-ops-password=" + r.Cfg.TOPass,
 		"--traffic-ops-url=" + r.Cfg.TOURL,
 		"--cache-host-name=" + r.Cfg.CacheHostName,
-		"--log-location-error=" + r.Cfg.LogLocationErr,
-		"--log-location-info=" + r.Cfg.LogLocationInfo,
-		"--log-location-warning=" + r.Cfg.LogLocationWarn,
+		"--log-location-error=" + errorLocation,
+		"--log-location-info=" + infoLocation,
+		"--log-location-warning=" + warningLocation,
+		"--traffic-ops-insecure",

Review comment:
       I don't think we should always set this. We should use proper HTTPS, unless someone specifically needs it. Especially in production.
   
   Mind adding an arg to `t3c`, to pass through here, that defaults to false?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficcontrol] rob05c merged pull request #4980: ORT t3c command fixes:

Posted by GitBox <gi...@apache.org>.
rob05c merged pull request #4980:
URL: https://github.com/apache/trafficcontrol/pull/4980


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficcontrol] jrushford commented on a change in pull request #4980: ORT t3c command fixes:

Posted by GitBox <gi...@apache.org>.
jrushford commented on a change in pull request #4980:
URL: https://github.com/apache/trafficcontrol/pull/4980#discussion_r474790741



##########
File path: traffic_ops_ort/t3c/torequest/torequest.go
##########
@@ -193,16 +193,35 @@ func (r *TrafficOpsReq) atsTcExec(cmdstr string) ([]byte, error) {
 
 // atsTcExecCommand is used to run the atstccfg command.
 func (r *TrafficOpsReq) atsTcExecCommand(cmdstr string, queueState int, revalState int) ([]byte, error) {
+	// adjust log locations used for atstccfg
+	// cannot use stdout as this will cause json parsing errors.
+	errorLocation := r.Cfg.LogLocationErr
+	if errorLocation == "stdout" {
+		errorLocation = "stderr"
+		log.Infoln("atstccfg error logging has been re-directed to 'stderr'")
+	}
+	infoLocation := r.Cfg.LogLocationInfo
+	if infoLocation == "stdout" {
+		infoLocation = "stderr"
+		log.Infoln("atstccfg info logging has been re-directed to 'stderr'")
+	}
+	warningLocation := r.Cfg.LogLocationWarn
+	if warningLocation == "stdout" {
+		warningLocation = "stderr"
+		log.Infoln("atstccfg warning logging has been re-directed to 'stderr'")
+	}
+
 	args := []string{
 		"--traffic-ops-timeout-milliseconds=" + strconv.FormatInt(int64(r.Cfg.TOTimeoutMS), 10),
 		"--traffic-ops-disable-proxy=" + strconv.FormatBool(r.Cfg.ReverseProxyDisable),
 		"--traffic-ops-user=" + r.Cfg.TOUser,
 		"--traffic-ops-password=" + r.Cfg.TOPass,
 		"--traffic-ops-url=" + r.Cfg.TOURL,
 		"--cache-host-name=" + r.Cfg.CacheHostName,
-		"--log-location-error=" + r.Cfg.LogLocationErr,
-		"--log-location-info=" + r.Cfg.LogLocationInfo,
-		"--log-location-warning=" + r.Cfg.LogLocationWarn,
+		"--log-location-error=" + errorLocation,
+		"--log-location-info=" + infoLocation,
+		"--log-location-warning=" + warningLocation,
+		"--traffic-ops-insecure",

Review comment:
       @rob05c I've added a --traffic-ops-insecure flag to t3c.  When the flag is true, --traffic-ops-insecure is used with atstccfg.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org