Jenkins build is back to normal : trafficcontrol-PR #4700

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] asf-ci commented on issue #4063: Add atstccfg meta config

[GitBox]

asf-ci commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548996650
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4700/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rob05c commented on issue #4063: Add atstccfg meta config

[GitBox]

rob05c commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548993667
 
 
   Retest this please.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #4063: Add atstccfg meta config

[GitBox]

asf-ci commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548993634
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4699/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

Build failed in Jenkins: trafficcontrol-PR #4699

[Apache Jenkins Server]

See 

Changes:


--
GitHub pull request #4063 of commit 6664e0e8f9b48f0931b66f6ef430b65f4131b1f7, 
no merge conflicts.
Running as SYSTEM
Setting status of 6664e0e8f9b48f0931b66f6ef430b65f4131b1f7 to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4699/ and message: 'Build 
started for merge commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H39 (ubuntu xenial) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
Cloning the remote Git repository
Cloning repository git://github.com/apache/trafficcontrol.git
 > git init  # timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse 6664e0e8f9b48f0931b66f6ef430b65f4131b1f7^{commit} # timeout=10
Checking out Revision 6664e0e8f9b48f0931b66f6ef430b65f4131b1f7 (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 6664e0e8f9b48f0931b66f6ef430b65f4131b1f7
Commit message: "Add atstccfg meta config"
 > git rev-list --no-walk 4a24e1e0075c8562f737bc2054fbd8f30e98c217 # timeout=10
First time build. Skipping changelog.
[trafficcontrol-PR] $ /bin/bash /tmp/jenkins7434655840363902794.sh
++ echo jenkins-trafficcontrol-PR-4699
++ sed s/-//g
++ sed s/jenkins//
+ proj=trafficcontrolPR4699
+ yml=infrastructure/docker/build/docker-compose.yml
++ mktemp /tmp/docker-compose-
+ dc=/tmp/docker-compose-APn5
++ mktemp /tmp/tc-status-
+ st=/tmp/tc-status-MWSl
+ trap finish EXIT
++ uname -s
++ uname -m
+ curl -o /tmp/docker-compose-APn5 -L 
https://github.com/docker/compose/releases/download/1.13.0/docker-compose-Linux-x86_64
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 
0100   6170   6170 0   1044  0 --:--:-- --:--:-- --:--:--  1045
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 
0100 8079k  100 8079k0 0  4848k  0  0:00:01  0:00:01 --:--:-- 9550k
+ chmod +x /tmp/docker-compose-APn5
+ rm -rf dist
+ /tmp/docker-compose-APn5 -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4699 up
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ exit 1
+ finish
+ /tmp/docker-compose-APn5 -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4699 down -v
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ /tmp/docker-compose-APn5 -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4699 rm -v -f
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ rm -f /tmp/docker-compose-APn5
Build step 'Execute shell' marked build as failure
Skipped archiving because build is not successful

[GitHub] [trafficcontrol] rob05c edited a comment on issue #4063: Add atstccfg meta config

[GitBox]

rob05c edited a comment on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548993389
 
 
   >I'm getting a TO API test failure on this:
   
   Fixed. My fault for not running the API tests again after fixing Chris' 
version comment. And for making the API test brittle. I also fixed it to be 
less brittle.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rob05c commented on issue #4063: Add atstccfg meta config

[GitBox]

rob05c commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548993389
 
 
   >I'm getting a TO API test failure on this:
   Fixed. My fault for not running the API tests again after fixing Chris' 
version comment. And for making the API test brittle. I also fixed it to be 
less brittle.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #4063: Add atstccfg meta config

[GitBox]

asf-ci commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548991411
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4698/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rob05c commented on issue #4063: Add atstccfg meta config

[GitBox]

rob05c commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548990899
 
 
   >Also, I don't have a line to attach this comment to, but in routes.go I 
noticed the ATS meta config API route is duplicated (L436 and L469) if you'd 
like to remove one of those extra routes.
   
   Fixed. Good catch! Guessing that was a non-conflict somewhere in all the 
merging around atstccfg PRs.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #4063: Add atstccfg meta config

[GitBox]

asf-ci commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548990913
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4697/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

Build failed in Jenkins: trafficcontrol-PR #4697

[Apache Jenkins Server]

See 

Changes:


--
GitHub pull request #4063 of commit 6798c0665b454d2760948fd3e978b10fcb470a8d, 
no merge conflicts.
Running as SYSTEM
Setting status of 6798c0665b454d2760948fd3e978b10fcb470a8d to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4697/ and message: 'Build 
started for merge commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H30 (ubuntu) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
Cloning the remote Git repository
Cloning repository git://github.com/apache/trafficcontrol.git
 > git init  # timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse 6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # timeout=10
 > git rev-parse origin/6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # 
 > timeout=10
 > git rev-parse 6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse 6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # timeout=10
 > git rev-parse origin/6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # 
 > timeout=10
 > git rev-parse 6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse 6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # timeout=10
 > git rev-parse origin/6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # 
 > timeout=10
 > git rev-parse 6798c0665b454d2760948fd3e978b10fcb470a8d^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Skipped archiving because build is not successful

[GitHub] [trafficcontrol] rob05c commented on a change in pull request #4063: Add atstccfg meta config

[GitBox]

rob05c commented on a change in pull request #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#discussion_r341788398
 
 

 ##
 File path: lib/go-atscfg/meta.go
 ##
 @@ -0,0 +1,195 @@
+package atscfg
+
+/*
+ * 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 (
+   "encoding/json"
+   "strings"
+
+   "github.com/apache/trafficcontrol/lib/go-log"
+   "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+type ConfigProfileParams struct {
+   FileNameOnDisk string
+   Location   string
+   URLstring
+   APIURI string
+}
+
+func MakeMetaConfig(
+   serverHostName tc.CacheName,
+   server *ServerInfo,
+   tmURL string, // global tm.url Parameter
+   tmReverseProxyURL string, // global tm.rev_proxy.url Parameter
+   locationParams map[string]ConfigProfileParams, // 
map[configFile]params; 'location' and 'URL' Parameters on serverHostName's 
Profile
+   uriSignedDSes []tc.DeliveryServiceName,
+   scopeParams map[string]string, // map[configFileName]scopeParam
+) string {
+   if tmURL == "" {
+   log.Errorln("ats.GetConfigMetadata: global tm.url parameter 
missing or empty! Setting empty in meta config!")
+   }
+
+   atsData := tc.ATSConfigMetaData{
+   Info: tc.ATSConfigMetaDataInfo{
+   ProfileID: int(server.ProfileID),
+   TOReverseProxyURL: tmReverseProxyURL,
+   TOURL: tmURL,
+   ServerIPv4:server.IP,
+   ServerPort:server.Port,
+   ServerName:server.HostName,
+   CDNID: server.CDNID,
+   CDNName:   string(server.CDN),
+   ServerID:  server.ID,
+   ProfileName:   server.ProfileName,
+   },
+   ConfigFiles: []tc.ATSConfigMetaDataConfigFile{},
+   }
+
+   if locationParams["remap.config"].Location != "" {
+   configLocation := locationParams["remap.config"].Location
+   for _, ds := range uriSignedDSes {
+   cfgName := "uri_signing_" + string(ds) + ".config"
+   // If there's already a parameter for it, don't clobber 
it. The user may wish to override the location.
+   if _, ok := locationParams[cfgName]; !ok {
+   p := locationParams[cfgName]
+   p.FileNameOnDisk = cfgName
+   p.Location = configLocation
+   locationParams[cfgName] = p
+   }
+   }
+   }
+
+   for cfgFile, cfgParams := range locationParams {
+   atsCfg := tc.ATSConfigMetaDataConfigFile{
+   FileNameOnDisk: cfgParams.FileNameOnDisk,
+   Location:   cfgParams.Location,
+   }
+
+   scope := getServerScope(cfgFile, server.Type, scopeParams)
+
+   if cfgParams.URL != "" {
+   scope = tc.ATSConfigMetaDataConfigFileScopeCDNs
+   atsCfg.URL = cfgParams.URL
+   } else {
+   scopeID := ""
+   if scope == tc.ATSConfigMetaDataConfigFileScopeCDNs {
+   scopeID = string(server.CDN)
+   } else if scope == 
tc.ATSConfigMetaDataConfigFileScopeProfiles {
+   scopeID = server.ProfileName
+   } else { // ATSConfigMetaDataConfigFileScopeServers
+   scopeID = server.HostName
+   }
+   atsCfg.APIURI = "/api/1.2/" + string(scope) + "/" + 
scopeID + "/configfiles/ats/" + cfgFile
+   }
+
+   atsCfg.Scope = string(scope)
+
+   atsData.ConfigFiles = append(atsData.ConfigFiles, atsCfg)
+   }
+
+   bts, err := json.Marshal(atsData)
+   if err != nil {
+   // should never happen
+   

[GitHub] [trafficcontrol] rob05c commented on a change in pull request #4063: Add atstccfg meta config

[GitBox]

rob05c commented on a change in pull request #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#discussion_r341788378
 
 

 ##
 File path: lib/go-atscfg/meta.go
 ##
 @@ -0,0 +1,195 @@
+package atscfg
+
+/*
+ * 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 (
+   "encoding/json"
+   "strings"
+
+   "github.com/apache/trafficcontrol/lib/go-log"
+   "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+type ConfigProfileParams struct {
+   FileNameOnDisk string
+   Location   string
+   URLstring
+   APIURI string
+}
+
+func MakeMetaConfig(
+   serverHostName tc.CacheName,
+   server *ServerInfo,
+   tmURL string, // global tm.url Parameter
+   tmReverseProxyURL string, // global tm.rev_proxy.url Parameter
+   locationParams map[string]ConfigProfileParams, // 
map[configFile]params; 'location' and 'URL' Parameters on serverHostName's 
Profile
+   uriSignedDSes []tc.DeliveryServiceName,
+   scopeParams map[string]string, // map[configFileName]scopeParam
+) string {
+   if tmURL == "" {
+   log.Errorln("ats.GetConfigMetadata: global tm.url parameter 
missing or empty! Setting empty in meta config!")
+   }
+
+   atsData := tc.ATSConfigMetaData{
+   Info: tc.ATSConfigMetaDataInfo{
+   ProfileID: int(server.ProfileID),
+   TOReverseProxyURL: tmReverseProxyURL,
+   TOURL: tmURL,
+   ServerIPv4:server.IP,
+   ServerPort:server.Port,
+   ServerName:server.HostName,
+   CDNID: server.CDNID,
+   CDNName:   string(server.CDN),
+   ServerID:  server.ID,
+   ProfileName:   server.ProfileName,
+   },
+   ConfigFiles: []tc.ATSConfigMetaDataConfigFile{},
+   }
+
+   if locationParams["remap.config"].Location != "" {
+   configLocation := locationParams["remap.config"].Location
+   for _, ds := range uriSignedDSes {
+   cfgName := "uri_signing_" + string(ds) + ".config"
+   // If there's already a parameter for it, don't clobber 
it. The user may wish to override the location.
+   if _, ok := locationParams[cfgName]; !ok {
+   p := locationParams[cfgName]
+   p.FileNameOnDisk = cfgName
+   p.Location = configLocation
+   locationParams[cfgName] = p
+   }
+   }
+   }
+
+   for cfgFile, cfgParams := range locationParams {
+   atsCfg := tc.ATSConfigMetaDataConfigFile{
+   FileNameOnDisk: cfgParams.FileNameOnDisk,
+   Location:   cfgParams.Location,
+   }
+
+   scope := getServerScope(cfgFile, server.Type, scopeParams)
+
+   if cfgParams.URL != "" {
+   scope = tc.ATSConfigMetaDataConfigFileScopeCDNs
+   atsCfg.URL = cfgParams.URL
+   } else {
+   scopeID := ""
+   if scope == tc.ATSConfigMetaDataConfigFileScopeCDNs {
+   scopeID = string(server.CDN)
+   } else if scope == 
tc.ATSConfigMetaDataConfigFileScopeProfiles {
+   scopeID = server.ProfileName
+   } else { // ATSConfigMetaDataConfigFileScopeServers
+   scopeID = server.HostName
+   }
+   atsCfg.APIURI = "/api/1.2/" + string(scope) + "/" + 
scopeID + "/configfiles/ats/" + cfgFile
 
 Review comment:
   The config generator doesn't actually care what this number is. 
Unfortunately, the only place it's really defined is in the client, and we 
don't want this library to depend on that.
   I changed it to a const.

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4063: Add atstccfg meta config

[GitBox]

rawlinp commented on a change in pull request #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#discussion_r341785740
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/ats/atsserver/meta.go
 ##
 @@ -85,136 +70,14 @@ func GetConfigMetaData(w http.ResponseWriter, r 
*http.Request) {
return
}
 
-   if locationParams["remap.config"].Location != "" {
-   configLocation := locationParams["remap.config"].Location
-   uriSignedDSes, err := ats.GetServerURISignedDSes(inf.Tx.Tx, 
server.HostName, server.Port)
-   if err != nil {
-   api.HandleErr(w, r, inf.Tx.Tx, 
http.StatusInternalServerError, nil, errors.New("GetConfigMetaData getting 
server uri-signed dses: "+err.Error()))
-   return
-   }
-   for _, ds := range uriSignedDSes {
-   cfgName := "uri_signing_" + string(ds) + ".config"
-   // If there's already a parameter for it, don't clobber 
it. The user may wish to override the location.
-   if _, ok := locationParams[cfgName]; !ok {
-   p := locationParams[cfgName]
-   p.FileNameOnDisk = cfgName
-   p.Location = configLocation
-   }
-   }
-   }
-
-   for cfgFile, cfgParams := range locationParams {
-   atsCfg := tc.ATSConfigMetaDataConfigFile{
-   FileNameOnDisk: cfgParams.FileNameOnDisk,
-   Location:   cfgParams.Location,
-   }
-
-   scope, err := getServerScope(inf.Tx.Tx, cfgFile, server.Type)
-   if err != nil {
-   api.HandleErr(w, r, inf.Tx.Tx, 
http.StatusInternalServerError, nil, errors.New("GetConfigMetaData getting 
scope: "+err.Error()))
-   return
-   }
-
-   if cfgParams.URL != "" {
-   scope = tc.ATSConfigMetaDataConfigFileScopeCDNs
-   atsCfg.URL = cfgParams.URL
-   } else {
-   scopeID := ""
-   if scope == tc.ATSConfigMetaDataConfigFileScopeCDNs {
-   scopeID = string(server.CDN)
-   } else if scope == 
tc.ATSConfigMetaDataConfigFileScopeProfiles {
-   scopeID = server.ProfileName
-   } else { // ATSConfigMetaDataConfigFileScopeServers
-   scopeID = server.HostName
-   }
-   atsCfg.APIURI = "/api/1.2/" + string(scope) + "/" + 
scopeID + "/configfiles/ats/" + cfgFile
-   }
-
-   atsCfg.Scope = string(scope)
-
-   atsData.ConfigFiles = append(atsData.ConfigFiles, atsCfg)
-   }
-
-   api.WriteRespRaw(w, r, atsData)
-}
-
-func getServerScope(tx *sql.Tx, cfgFile string, serverType string) 
(tc.ATSConfigMetaDataConfigFileScope, error) {
-   switch {
-   case cfgFile == "cache.config" && tc.CacheTypeFromString(serverType) == 
tc.CacheTypeMid:
-   return tc.ATSConfigMetaDataConfigFileScopeServers, nil
-   default:
-   return getScope(tx, cfgFile)
-   }
-}
-
-// getScope returns the ATSConfigMetaDataConfigFileScope for the given config 
file, and potentially the given server. If the config is not a Server scope, 
i.e. was part of an endpoint which does not include a server name or id, the 
server may be nil.
-func getScope(tx *sql.Tx, cfgFile string) 
(tc.ATSConfigMetaDataConfigFileScope, error) {
-   switch {
-   case cfgFile == "ip_allow.config":
-   fallthrough
-   case cfgFile == "parent.config":
-   fallthrough
-   case cfgFile == "hosting.config":
-   fallthrough
-   case cfgFile == "packages":
-   fallthrough
-   case cfgFile == "chkconfig":
-   fallthrough
-   case cfgFile == "remap.config":
-   fallthrough
-   case strings.HasPrefix(cfgFile, "to_ext_") && 
strings.HasSuffix(cfgFile, ".config"):
-   return tc.ATSConfigMetaDataConfigFileScopeServers, nil
-   case cfgFile == "12M_facts":
-   fallthrough
-   case cfgFile == "50-ats.rules":
-   fallthrough
-   case cfgFile == "astats.config":
-   fallthrough
-   case cfgFile == "cache.config":
-   fallthrough
-   case cfgFile == "drop_qstring.config":
-   fallthrough
-   case cfgFile == "logs_xml.config":
-   fallthrough
-   case cfgFile == "logging.config":
-   fallthrough
-   case cfgFile == "plugin.config":
-   fallthrough
-   case cfgFile == "records.config":
-   fallthrough
-   case cfgFile == "storage.config":
-   

Build failed in Jenkins: trafficcontrol-PR #4696

[Apache Jenkins Server]

See 


Changes:

[ocket] TP: adds the ability to add/remove required server capabilities 
from DS

[ocket] Rewrite PUT /api/1.1/servers/:id/status to Go (#4030)

[ocket] Block server servercapability delete if associated ds requires it

[mitchell852] Ensure server_capability can only be assigned to edges or mids 
(#4044)

[ocket] return 403 when unauthorized user assigns/deletes ds required 
capability

[ocket] fetches tenant users with the proper query param and removes 
tenantId

[rawlin_peters] Block allowing requiring caps on ds that their servers dont 
have (#4060)

[rob] Add atscfg logic for Server Capabilities.


--
GitHub pull request #3980 of commit c91f6b8608bd51803f8f36c0a8efa73c02de97e2, 
no merge conflicts.
Running as SYSTEM
Setting status of c91f6b8608bd51803f8f36c0a8efa73c02de97e2 to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4696/ and message: 'Build 
started for merge commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H39 (ubuntu xenial) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
Cloning the remote Git repository
Cloning repository git://github.com/apache/trafficcontrol.git
 > git init  # timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
using GIT_SSH to set credentials 
 > git fetch --tags --progress git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse c91f6b8608bd51803f8f36c0a8efa73c02de97e2^{commit} # timeout=10
Checking out Revision c91f6b8608bd51803f8f36c0a8efa73c02de97e2 (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f c91f6b8608bd51803f8f36c0a8efa73c02de97e2
Commit message: "Add atscfg logic for Server Capabilities."
 > git rev-list --no-walk c92c1ad98f4cee0ce27039f2966e18d51c237428 # timeout=10
[trafficcontrol-PR] $ /bin/bash /tmp/jenkins7591057574634330490.sh
++ echo jenkins-trafficcontrol-PR-4696
++ sed s/-//g
++ sed s/jenkins//
+ proj=trafficcontrolPR4696
+ yml=infrastructure/docker/build/docker-compose.yml
++ mktemp /tmp/docker-compose-
+ dc=/tmp/docker-compose-COrG
++ mktemp /tmp/tc-status-
+ st=/tmp/tc-status-z6bC
+ trap finish EXIT
++ uname -s
++ uname -m
+ curl -o /tmp/docker-compose-COrG -L 
https://github.com/docker/compose/releases/download/1.13.0/docker-compose-Linux-x86_64
  % Total% Received % Xferd  Average Speed   TimeTime Time  Current
 Dload  Upload   Total   SpentLeft  Speed
  0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0  
0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0100 
  6170   6170 0   1066  0 --:--:-- --:--:-- --:--:--  1065
  3 8079k3  253k0 0   192k  0  0:00:41  0:00:01  0:00:40  
192k100 8079k  100 8079k0 0  4699k  0  0:00:01  0:00:01 --:--:-- 
18.9M
+ chmod +x /tmp/docker-compose-COrG
+ rm -rf dist
+ /tmp/docker-compose-COrG -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4696 up
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ exit 1
+ finish
+ /tmp/docker-compose-COrG -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4696 down -v
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ /tmp/docker-compose-COrG -f infrastructure/docker/build/docker-compose.yml -p 
trafficcontrolPR4696 rm -v -f
Couldn't connect to Docker daemon at http+docker://localunixsocket - is it 
running?

If it's at a non-standard location, specify the URL with the DOCKER_HOST 
environment variable.
+ rm -f /tmp/docker-compose-COrG
Build step 'Execute shell' marked build as failure
Skipped archiving because build is not successful

[GitHub] [trafficcontrol] asf-ci commented on issue #3980: Add atscfg logic for Server Capabilities

[GitBox]

asf-ci commented on issue #3980: Add atscfg logic for Server Capabilities
URL: https://github.com/apache/trafficcontrol/pull/3980#issuecomment-548983845
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4696/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa commented on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

mhoppa commented on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548977418
 
 
   Sure! I can do the first and take a look at the second 


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #3980: Add atscfg logic for Server Capabilities

[GitBox]

asf-ci commented on issue #3980: Add atscfg logic for Server Capabilities
URL: https://github.com/apache/trafficcontrol/pull/3980#issuecomment-548976463
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4695/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

Build failed in Jenkins: trafficcontrol-PR #4695

[Apache Jenkins Server]

See 

Changes:


--
GitHub pull request #3980 of commit f47dde1e1ed244a61889ebd58d53911ad14c25ac, 
no merge conflicts.
Running as SYSTEM
Setting status of f47dde1e1ed244a61889ebd58d53911ad14c25ac to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4695/ and message: 'Build 
started for merge commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H33 (ubuntu) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
Cloning the remote Git repository
Cloning repository git://github.com/apache/trafficcontrol.git
 > git init  # timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # timeout=10
 > git rev-parse origin/f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # 
 > timeout=10
 > git rev-parse f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # timeout=10
 > git rev-parse origin/f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # 
 > timeout=10
 > git rev-parse f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # timeout=10
 > git rev-parse origin/f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # 
 > timeout=10
 > git rev-parse f47dde1e1ed244a61889ebd58d53911ad14c25ac^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Skipped archiving because build is not successful

[GitHub] [trafficcontrol] ocket8888 commented on issue #4068: Admin can assign User to a Federation with wrong PARAMETER

[GitBox]

ocket commented on issue #4068: Admin can assign User to a Federation with 
wrong PARAMETER
URL: https://github.com/apache/trafficcontrol/issues/4068#issuecomment-548974137
 
 
   That's because Go's json parsing can't determine the difference between a 
"zero-valued" field and a non-existent field. To oversimplify: `null` is the 
same as `[]` is the same as `undefined`. So from the parser's perspective, not 
including an array key is the same as including it but it's empty. When an 
array field is "required" in the API, that also means it can't be empty. The 
converse is also true.
   
   So it's not a bug. That's by design. However, there's no reason that 
parameter shouldn't be required, IMO.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina opened a new issue #4070: TO: Server Server capability is not allowed to be removed from a server when its not associated with any DS

[GitBox]

lbathina opened a new issue #4070: TO: Server Server capability is not allowed 
to be removed from a server when its not associated with any DS
URL: https://github.com/apache/trafficcontrol/issues/4070
 
 
   
   
   
   
   ## I'm submitting a ...
   
   
   - [X] bug report
   - [ ] new feature / enhancement request
   - [ ] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [X] Traffic Ops
   - [ ] Traffic Ops ORT
   - [ ] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
ds with cap1, cap2 and have a server assigned to it which has cap1, cap2, 
cap3
   
deleting cap3 isn't allowed
   Response Code 400
   {"alerts":[{"text":"cannot delete server_capability because it is being used 
by a server_server_capability","level":"error"}]}
   
   ## Expected / new behavior:
   
deleting cap3 should be allowed as its not a requirement of the only ds 
that the server is assigned to
   response code should be 200 
   ## Minimal reproduction of the problem with instructions:
   
   
   ## Anything else:
   
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 edited a comment on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

mitchell852 edited a comment on issue #4033: To: INternal server error when 
server capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548961279
 
 
   
https://github.com/apache/trafficcontrol/blob/master/traffic_portal/app/src/common/api/ServerCapabilityService.js#L65
   
   ^^ @mhoppa do you want to take this and just change that line to false to 
make sure error message displays in TP on delete fail? also not sure if you can 
change error message to "cannot delete server capability because it is being 
used by a server or required by a delivery service"


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 edited a comment on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

mitchell852 edited a comment on issue #4033: To: INternal server error when 
server capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548961279
 
 
   
https://github.com/apache/trafficcontrol/blob/master/traffic_portal/app/src/common/api/ServerCapabilityService.js#L65
   
   ^^ @mhoppa do you want to take this and just change that line to false to 
make sure error message displays? also not sure if you can change error message 
to "cannot delete server capability because it is being used by a server or 
required by a delivery service"


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 edited a comment on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

mitchell852 edited a comment on issue #4033: To: INternal server error when 
server capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548961279
 
 
   
https://github.com/apache/trafficcontrol/blob/master/traffic_portal/app/src/common/api/ServerCapabilityService.js#L65
   
   ^^ @mhoppa do you want to take this and just change that line to false to 
make sure error message displays on fail? also not sure if you can change error 
message to "cannot delete server capability because it is being used by a 
server or required by a delivery service"


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 edited a comment on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

mitchell852 edited a comment on issue #4033: To: INternal server error when 
server capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548961279
 
 
   
https://github.com/apache/trafficcontrol/blob/master/traffic_portal/app/src/common/api/ServerCapabilityService.js#L61-L65
   
   ^^ @mhoppa do you want to take this and just change those to false to make 
sure error message displays? also not sure if you can change error message to 
"cannot delete server capability because it is being used by a server or 
required by a delivery service"


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 edited a comment on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

mitchell852 edited a comment on issue #4033: To: INternal server error when 
server capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548961279
 
 
   
https://github.com/apache/trafficcontrol/blob/master/traffic_portal/app/src/common/api/ServerCapabilityService.js#L61-L65
   
   ^^ @mhoppa do you want to take this and just change that to false to make 
sure error message displays? also not sure if you can change error message to 
"cannot delete server capability because it is being used by a server or 
required by a delivery service"


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 commented on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

mitchell852 commented on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548961279
 
 
   
https://github.com/apache/trafficcontrol/blob/master/traffic_portal/app/src/common/api/ServerCapabilityService.js#L65
   
   ^^ @mhoppa do you want to take this and just change that to false to make 
sure error message displays? also not sure if you can change error message to 
"cannot delete server capability because it is being used by a server or 
required by a delivery service"


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina commented on issue #4068: Admin can assign User to a Federation with wrong PARAMETER

[GitBox]

lbathina commented on issue #4068: Admin can assign User to a Federation with 
wrong PARAMETER
URL: https://github.com/apache/trafficcontrol/issues/4068#issuecomment-548959788
 
 
   this is a bug. I have seen this working working for other endpoints. when 
the parameter in json body is not specified correctly it spits error saying 
that the parameter is required.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina commented on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

lbathina commented on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548956981
 
 
   also the message displayed for api is partially correct 
{"alerts":[{"text":"cannot delete server_capability because it is being used by 
a server_server_capability","level":"error"}]}
   it could be server_server_capability or required capability - can this be 
updated to user friendly message without exposing db tables?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rawlinp merged pull request #4060: Block allowing requiring caps on ds that their servers dont have

[GitBox]

rawlinp merged pull request #4060: Block allowing requiring caps on ds that 
their servers dont have
URL: https://github.com/apache/trafficcontrol/pull/4060
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4060: Block allowing requiring caps on ds that their servers dont have

[GitBox]

rawlinp commented on a change in pull request #4060: Block allowing requiring 
caps on ds that their servers dont have
URL: https://github.com/apache/trafficcontrol/pull/4060#discussion_r341747808
 
 

 ##
 File path: 
traffic_ops/testing/api/v14/deliveryservices_required_capabilities_test.go
 ##
 @@ -155,6 +156,87 @@ func CreateTestDeliveryServicesRequiredCapabilities(t 
*testing.T) {
}
})
}
+}
+
+func InvalidDeliveryServicesRequiredCapabilityAddition(t *testing.T) {
+   // Tests that a capability cannot be made required if the DS's services 
do not have it assigned
 
 Review comment:
   services -> servers


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #4060: Block allowing requiring caps on ds that their servers dont have

[GitBox]

rawlinp commented on a change in pull request #4060: Block allowing requiring 
caps on ds that their servers dont have
URL: https://github.com/apache/trafficcontrol/pull/4060#discussion_r341754792
 
 

 ##
 File path: 
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go
 ##
 @@ -259,6 +268,62 @@ func (rc *RequiredCapability) Create() (error, error, 
int) {
return nil, nil, http.StatusOK
 }
 
+func (rc *RequiredCapability) ensureDSServerCap() (error, error, int) {
+   tx := rc.APIInfo().Tx
+
+   // Get assigned DS server IDs
+   dsServerIDs := []int64{}
+   if err := tx.Tx.QueryRow(`
+   SELECT ARRAY(
+   SELECT server 
+   FROM deliveryservice_server 
+   WHERE deliveryservice=$1
+   )`, rc.DeliveryServiceID).Scan(pq.Array()); err != nil && 
err != sql.ErrNoRows {
+   return nil, fmt.Errorf("reading delivery service %v servers: 
%v", *rc.DeliveryServiceID, err), http.StatusInternalServerError
+   }
+
+   if len(dsServerIDs) == 0 { // no attached servers can return success 
right away
+   return nil, nil, http.StatusOK
+   }
+
+   // Get servers IDs that have the new capability
+   capServerIDs := []int64{}
+   if err := tx.QueryRow(`
+   SELECT ARRAY(
+   SELECT server
+   FROM server_server_capability 
+   WHERE server IN (
+   SELECT server 
 
 Review comment:
   Technically you already have the answer to this subquery via the query above 
and could pass that data into this query, but I wouldn't hold off merging this 
for that reason.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina opened a new issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

lbathina opened a new issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033
 
 
   
   
   
   
   ## I'm submitting a ...
   
   
   - [X] bug report
   - [ ] new feature / enhancement request
   - [ ] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [X] Traffic Ops
   - [ ] Traffic Ops ORT
   - [X] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
   When a server capability associated with a server is deleted, we get 
internal server error
   ## Expected / new behavior:
   
   Should return error `Server capability  cannot be deleted when servers 
are associated to it.` 
   
   ## Minimal reproduction of the problem with instructions:
   
   1. create server capability 
   2. Associate it to a server 
   3. delete the server capability created in step1
   ## Anything else:
   
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina commented on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

lbathina commented on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548955398
 
 
   /reopen pls


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina edited a comment on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

lbathina edited a comment on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548954692
 
 
   this has been validated, works fine for TO. TP doesn't show any message


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina edited a comment on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

lbathina edited a comment on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548954692
 
 
   this has been validated, works fine for TO. TP doesn't show any message- 
needs to be fixed for TP


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina commented on issue #4033: To: INternal server error when server capability associated with Server/ds is deleted

[GitBox]

lbathina commented on issue #4033: To: INternal server error when server 
capability associated with Server/ds is deleted
URL: https://github.com/apache/trafficcontrol/issues/4033#issuecomment-548954692
 
 
   this has been validated and approve fixed


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on issue #4069: TP: DS-> Manage Required Capabilities is displayed as Manage Required Server Capabilities

[GitBox]

ocket commented on issue #4069: TP: DS-> Manage Required Capabilities is 
displayed as Manage Required Server Capabilities
URL: https://github.com/apache/trafficcontrol/issues/4069#issuecomment-548953054
 
 
   Is that really a problem? This page shows the server capabilities required 
by this Delivery Service on the servers which service it (tongue-twister). So 
it seems like a fine title, and consistent in the two places it's shown, as 
you've noted.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 closed issue #4056: tenant users returned is not filtered list of users belong to the tenant

[GitBox]

ocket closed issue #4056: tenant users returned is not filtered list of 
users belong to the tenant
URL: https://github.com/apache/trafficcontrol/issues/4056
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 merged pull request #4061: TP: fetches tenant users with the proper query param and removes tenantId…

[GitBox]

ocket merged pull request #4061: TP: fetches tenant users with the proper 
query param and removes tenantId…
URL: https://github.com/apache/trafficcontrol/pull/4061
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina opened a new issue #4069: TP: DS-> Manage Required Capabilities is displayed as Manage Required Server Capabilities

[GitBox]

lbathina opened a new issue #4069: TP: DS-> Manage Required Capabilities is 
displayed as Manage Required Server Capabilities
URL: https://github.com/apache/trafficcontrol/issues/4069
 
 
   
   
   
   
   ## I'm submitting a ...
   
   
   - [X] bug report
   - [ ] new feature / enhancement request
   - [ ] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [ ] Traffic Ops
   - [ ] Traffic Ops ORT
   - [X] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
   DS-> Manage Required Capabilities is displayed as Manage Required Server 
Capabilities
   
   in menu as well as the page header
   
   https://user-images.githubusercontent.com/26607771/68056710-6a0e9280-fcb9-11e9-9b36-ba7d8472cb53.png;>
   
   ## Expected / new behavior:
   
   it should be `Manage Required Capabilities` in context menu and Required 
Capabilities in bread crumb/header
   
   ## Minimal reproduction of the problem with instructions:
   
   
   ## Anything else:
   
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #3996: Rewrote /user/current to Go

[GitBox]

asf-ci commented on issue #3996: Rewrote /user/current to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#issuecomment-548945756
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4694/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rob05c commented on issue #4068: Admin can assign User to a Federation with wrong PARAMETER

[GitBox]

rob05c commented on issue #4068: Admin can assign User to a Federation with 
wrong PARAMETER
URL: https://github.com/apache/trafficcontrol/issues/4068#issuecomment-548940913
 
 
   Rejecting that would also be a violation of the Robustness Principle, "Be 
liberal in what you accept, conservative in what you send." 
(https://tools.ietf.org/html/rfc1122#section-1.2.2, 
https://en.wikipedia.org/wiki/Robustness_principle). 
   
   I know it's common for a lot of APIs to reject unknown things like that, but 
in general, it makes servers more robust, and makes integration and 
interoperability better and work more and fail less, when the Robustness 
Principle is followed.
   
   TC mostly follows it, though there's not a hard rule and I'm sure there are 
places it doesn't. I know I've said this before, but I'll keep saying it: we 
should follow the Robustness Principle wherever possible. There are cases like 
this one (`"userId": [2],`) that can be confusing, but for the vast 
majority of cases it makes TO and TC more robust and resilient to imperfect 
input, and easier for clients to integrate and work with.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] lbathina commented on issue #4049: deliveryservices_required_capabilities - sort by lastUpdated doesn't work - it is also true for all endpoints that has sort & lastUpdated f

[GitBox]

lbathina commented on issue #4049: deliveryservices_required_capabilities - 
sort by lastUpdated doesn't work - it is also true for all endpoints that has 
sort & lastUpdated field
URL: https://github.com/apache/trafficcontrol/issues/4049#issuecomment-548936296
 
 
   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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341735912
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
+   sysErr = fmt.Errorf("Current user (#%d) doesn't exist... ??", 
inf.User.ID)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   if err := userRequest.User.ValidateAndUnmarshal(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   changePasswd := false
+
+   // obfuscate passwords (ValidateAndUnmarshal checks for equality with 
ConfirmLocalPassword)
+   // TODO: check for valid password via bad password list like Perl did? 
User creation doesn't...
+   if user.LocalPassword != nil && *user.LocalPassword != "" {
+   hashPass, err := auth.DerivePassword(*user.LocalPassword)
+   if err != nil {
+   sysErr = fmt.Errorf("Hashing new password: %v", err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   changePasswd = true
+   user.LocalPassword = util.StrPtr(hashPass)
+   user.ConfirmLocalPassword = util.StrPtr(hashPass)
+   }
+
+   if *user.Role != inf.User.Role {
+   privLevel, exists, err := dbhelpers.GetPrivLevelFromRoleID(tx, 
*user.Role)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting privLevel for Role #%d: 
%v", *user.Role, err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   if !exists {
+   userErr = fmt.Errorf("role: no such role: %d", 
*user.Role)
+   errCode = http.StatusNotFound
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+   if privLevel > inf.User.PrivLevel {
+   userErr = errors.New("role: cannot have greater 
permissions than user's current role")
+   errCode = http.StatusForbidden
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+   }
+
+   if ok, err := tenant.IsResourceAuthorizedToUserTx(*user.TenantID, 
inf.User, tx); err != nil {
+   if err == sql.ErrNoRows {
+   userErr = errors.New("No such tenant!")
+   errCode = http.StatusNotFound
+   } else {
+   sysErr = fmt.Errorf("Checking user %s permissions on 
tenant #%d: %v", inf.User.UserName, *user.TenantID, err)
+   errCode = http.StatusInternalServerError
+   }
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   } else if !ok {
+   // unlike Perl, this endpoint will not disclose the existence 
of tenants over which the current
+   // user has no permission - in keeping with the behavior of the 
'/tenants' endpoint.
+   userErr = errors.New("No such tenant!")
+   errCode = http.StatusNotFound
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+
+   if *user.Username != inf.User.UserName {
+
+   if ok, err := 

[GitHub] [trafficcontrol] ocket8888 commented on issue #4068: Admin can assign User to a Federation with wrong PARAMETER

[GitBox]

ocket commented on issue #4068: Admin can assign User to a Federation with 
wrong PARAMETER
URL: https://github.com/apache/trafficcontrol/issues/4068#issuecomment-548935237
 
 
   This is because our API always ignores keys that aren't a part of the 
defined request structure, as that's what the Go parser does. Implementing it 
to do anything else would be a lot of effort for little return, so I don't see 
this improvement happening any time soon.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 commented on issue #4049: deliveryservices_required_capabilities - sort doesn't work

[GitBox]

mitchell852 commented on issue #4049: deliveryservices_required_capabilities - 
sort doesn't work
URL: https://github.com/apache/trafficcontrol/issues/4049#issuecomment-548927741
 
 
   ok so `orderby=lastUpdated` doesn't work on across the API? can @lbathina 
change the title/description of this bug to better reflect 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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 closed issue #4043: TO: Internal Server error is returned when user not tenant of the ds assigns/deletes required capability

[GitBox]

ocket closed issue #4043: TO: Internal Server error is returned when user 
not tenant of the ds assigns/deletes required capability
URL: https://github.com/apache/trafficcontrol/issues/4043
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 merged pull request #4059: return 403 when unauthorized user assigns/deletes ds required capability

[GitBox]

ocket merged pull request #4059: return 403 when unauthorized user 
assigns/deletes ds required capability
URL: https://github.com/apache/trafficcontrol/pull/4059
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ChrisHines commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ChrisHines commented on a change in pull request #3996: Rewrote /user/current 
to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341722207
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
+   sysErr = fmt.Errorf("Current user (#%d) doesn't exist... ??", 
inf.User.ID)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   if err := userRequest.User.ValidateAndUnmarshal(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   changePasswd := false
+
+   // obfuscate passwords (ValidateAndUnmarshal checks for equality with 
ConfirmLocalPassword)
+   // TODO: check for valid password via bad password list like Perl did? 
User creation doesn't...
+   if user.LocalPassword != nil && *user.LocalPassword != "" {
+   hashPass, err := auth.DerivePassword(*user.LocalPassword)
+   if err != nil {
+   sysErr = fmt.Errorf("Hashing new password: %v", err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   changePasswd = true
+   user.LocalPassword = util.StrPtr(hashPass)
+   user.ConfirmLocalPassword = util.StrPtr(hashPass)
+   }
+
+   if *user.Role != inf.User.Role {
+   privLevel, exists, err := dbhelpers.GetPrivLevelFromRoleID(tx, 
*user.Role)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting privLevel for Role #%d: 
%v", *user.Role, err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   if !exists {
+   userErr = fmt.Errorf("role: no such role: %d", 
*user.Role)
+   errCode = http.StatusNotFound
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+   if privLevel > inf.User.PrivLevel {
+   userErr = errors.New("role: cannot have greater 
permissions than user's current role")
+   errCode = http.StatusForbidden
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+   }
+
+   if ok, err := tenant.IsResourceAuthorizedToUserTx(*user.TenantID, 
inf.User, tx); err != nil {
+   if err == sql.ErrNoRows {
+   userErr = errors.New("No such tenant!")
+   errCode = http.StatusNotFound
+   } else {
+   sysErr = fmt.Errorf("Checking user %s permissions on 
tenant #%d: %v", inf.User.UserName, *user.TenantID, err)
+   errCode = http.StatusInternalServerError
+   }
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   } else if !ok {
+   // unlike Perl, this endpoint will not disclose the existence 
of tenants over which the current
+   // user has no permission - in keeping with the behavior of the 
'/tenants' endpoint.
+   userErr = errors.New("No such tenant!")
+   errCode = http.StatusNotFound
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+
+   if *user.Username != inf.User.UserName {
+
+   if ok, err := 

[GitHub] [trafficcontrol] dandypham opened a new issue #4068: Admin can assign User to a Federation with wrong PARAMETER

[GitBox]

dandypham opened a new issue #4068: Admin can assign User to a Federation with 
wrong PARAMETER
URL: https://github.com/apache/trafficcontrol/issues/4068
 
 
   
   
   
   
   ## I'm submitting a ...
   
   
   - [x] bug report
   - [ ] new feature / enhancement request
   - [ ] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [x] Traffic Ops
   - [ ] Traffic Ops ORT
   - [ ] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
   Login as ADMIN
   POSTRequest:
   
`https://{{TO_BASE_URL}}/api/{{api_version}}/federations/{{federationsID}}/users.
 `
   With wrong json payload parameter like "userId"
   ```
   {
   "userId": [2],
   "replaces": true
   }
   ```
   It will return 
   ```
   Status 200 OK{
   "alerts": [
   {
   "text": "0 user(s) were Assign to the test.quest. federation",
   "level": "success"
   }
   ],
   "response": {
   "userIds": null,
   "replace": null
   }
   }
   ```
   ## Expected / new behavior:
   
   Should return 400 Bad Request
   ## Minimal reproduction of the problem with instructions:
   
   
   ## Anything else:
   
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341720932
 
 

 ##
 File path: lib/go-tc/users.go
 ##
 @@ -101,6 +110,179 @@ type UserCurrent struct {
commonUserFields
 }
 
+// CurrentUserUpdateRequest differs from a regular User/UserCurrent in that 
many of its fields are
+// *parsed* but not *unmarshaled*. This allows a handler to distinguish 
between "null" and
+// "undefined" values.
+type CurrentUserUpdateRequest struct {
+   // User, for whatever reason, contains all of the actual data.
+   User CurrentUserUpdateRequestUser `json:"user"`
+}
+
+// CurrentUserUpdateRequestUser holds all of the actual data in a request to 
update the current user.
+type CurrentUserUpdateRequestUser struct {
+   AddressLine1   json.RawMessage `json:"addressLine1"`
+   AddressLine2   json.RawMessage `json:"addressLine2"`
+   City   json.RawMessage `json:"city"`
+   Companyjson.RawMessage `json:"company"`
+   ConfirmLocalPasswd *string `json:"confirmLocalPasswd"`
+   Countryjson.RawMessage `json:"country"`
+   Email  json.RawMessage `json:"email"`
+   FullName   json.RawMessage `json:"fullName"`
+   GIDjson.RawMessage `json:"gid"`
+   ID json.RawMessage `json:"id"`
+   LocalPasswd*string `json:"localPasswd"`
+   PhoneNumberjson.RawMessage `json:"phoneNumber"`
+   PostalCode json.RawMessage `json:"postalCode"`
+   PublicSSHKey   json.RawMessage `json:"publicSshKey"`
+   Role   json.RawMessage `json:"role"`
+   StateOrProvincejson.RawMessage `json:"stateOrProvince"`
+   TenantID   json.RawMessage `json:"tenantId"`
+   UIDjson.RawMessage `json:"uid"`
+   Username   json.RawMessage `json:"username"`
+}
+
+// ValidateAndUnmarshal validates the request and returns a User into which 
the request's information
+// has been unmarshalled. This allows many fields to be "null", but explicitly 
checks that they are
+// present in the JSON payload.
+func (u *CurrentUserUpdateRequestUser) ValidateAndUnmarshal(user *User) error {
+   errs := []error{}
+   if u.AddressLine1 != nil {
+   if err := json.Unmarshal(u.AddressLine1, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine1: %v", err))
+   }
+   }
+
+   if u.AddressLine2 != nil {
+   if err := json.Unmarshal(u.AddressLine2, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine2: %v", err))
+   }
+   }
+
+   if u.City != nil {
+   if err := json.Unmarshal(u.City, ); err != nil {
+   errs = append(errs, fmt.Errorf("city: %v", err))
+   }
+   }
+
+   if u.Company != nil {
+   if err := json.Unmarshal(u.Company, ); err != nil {
+   errs = append(errs, fmt.Errorf("company: %v", err))
+   }
+   }
+
+   if u.LocalPasswd != nil && *u.LocalPasswd != "" {
+   if u.ConfirmLocalPasswd == nil || *u.ConfirmLocalPasswd == "" {
+   errs = append(errs, errors.New("confirmLocalPasswd: 
required when changing password"))
+   } else if *u.LocalPasswd != *u.ConfirmLocalPasswd {
+   errs = append(errs, errors.New("localPasswd and 
confirmLocalPasswd do not match"))
+   }
+   } else if u.ConfirmLocalPasswd != nil && *u.ConfirmLocalPasswd != "" {
+   errs = append(errs, errors.New("localPasswd: required when 
changing password"))
+   }
+   user.ConfirmLocalPassword = u.ConfirmLocalPasswd
+   user.LocalPassword = u.LocalPasswd
+
+   if u.Country != nil {
+   if err := json.Unmarshal(u.Country, ); err != nil {
+   errs = append(errs, fmt.Errorf("country: %v", err))
+   }
+   }
+
+   if u.Email != nil {
+   if err := json.Unmarshal(u.Email, ); err != nil {
+   errs = append(errs, fmt.Errorf("email: %v", err))
+   } else if err = validation.Validate(*user.Email, is.Email); err 
!= nil {
+   errs = append(errs, err)
+   }
+   }
+
+   if u.FullName != nil {
+   if err := json.Unmarshal(u.FullName, ); err != 
nil {
+   errs = append(errs, fmt.Errorf("fullName: %v", err))
+   } else if user.FullName == nil || *user.FullName == "" {
+   // Perl enforced this
+   errs = append(errs, fmt.Errorf("fullName: cannot be set 
to 'null' or empty string"))
+   }
+   }
 
 Review comment:
   Oh, yeah, I didn't update the PR. Pursuant to a [mailing list 

[GitHub] [trafficcontrol] ChrisHines commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ChrisHines commented on a change in pull request #3996: Rewrote /user/current 
to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341720076
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
 
 Review comment:
   Which ever way you think the code is simpler and most clear. I personally 
think removing the `else` reduces the cognitive load of reading the code.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341719904
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
+   sysErr = fmt.Errorf("Current user (#%d) doesn't exist... ??", 
inf.User.ID)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   if err := userRequest.User.ValidateAndUnmarshal(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   changePasswd := false
+
+   // obfuscate passwords (ValidateAndUnmarshal checks for equality with 
ConfirmLocalPassword)
+   // TODO: check for valid password via bad password list like Perl did? 
User creation doesn't...
+   if user.LocalPassword != nil && *user.LocalPassword != "" {
+   hashPass, err := auth.DerivePassword(*user.LocalPassword)
+   if err != nil {
+   sysErr = fmt.Errorf("Hashing new password: %v", err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   changePasswd = true
+   user.LocalPassword = util.StrPtr(hashPass)
+   user.ConfirmLocalPassword = util.StrPtr(hashPass)
+   }
+
+   if *user.Role != inf.User.Role {
+   privLevel, exists, err := dbhelpers.GetPrivLevelFromRoleID(tx, 
*user.Role)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting privLevel for Role #%d: 
%v", *user.Role, err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   if !exists {
+   userErr = fmt.Errorf("role: no such role: %d", 
*user.Role)
+   errCode = http.StatusNotFound
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+   if privLevel > inf.User.PrivLevel {
+   userErr = errors.New("role: cannot have greater 
permissions than user's current role")
+   errCode = http.StatusForbidden
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+   }
+
+   if ok, err := tenant.IsResourceAuthorizedToUserTx(*user.TenantID, 
inf.User, tx); err != nil {
+   if err == sql.ErrNoRows {
+   userErr = errors.New("No such tenant!")
+   errCode = http.StatusNotFound
+   } else {
+   sysErr = fmt.Errorf("Checking user %s permissions on 
tenant #%d: %v", inf.User.UserName, *user.TenantID, err)
+   errCode = http.StatusInternalServerError
+   }
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   } else if !ok {
+   // unlike Perl, this endpoint will not disclose the existence 
of tenants over which the current
+   // user has no permission - in keeping with the behavior of the 
'/tenants' endpoint.
+   userErr = errors.New("No such tenant!")
+   errCode = http.StatusNotFound
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+
+   if *user.Username != inf.User.UserName {
+
+   if ok, err := 

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341719513
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
+   sysErr = fmt.Errorf("Current user (#%d) doesn't exist... ??", 
inf.User.ID)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   if err := userRequest.User.ValidateAndUnmarshal(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   changePasswd := false
+
+   // obfuscate passwords (ValidateAndUnmarshal checks for equality with 
ConfirmLocalPassword)
+   // TODO: check for valid password via bad password list like Perl did? 
User creation doesn't...
+   if user.LocalPassword != nil && *user.LocalPassword != "" {
+   hashPass, err := auth.DerivePassword(*user.LocalPassword)
+   if err != nil {
+   sysErr = fmt.Errorf("Hashing new password: %v", err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   changePasswd = true
 
 Review comment:
   Yep. If there were no errors from ValidateAndUnmarshal, it means that either 
`localPasswd` and `confirmLocalPasswd` were both one of `null`, `undefined`, or 
an empty string, *or* they were both none of those and were strings that 
matched. So if `user.LocalPassword` is not `nil` and points to a non-empty 
string, then the user submitted strings of non-zero length in the 
`confirmLocalPasswd` and `localPasswd` fields that were identical. It could've 
been their current password, but in that case updating it is essentially a 
no-op.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ChrisHines commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ChrisHines commented on a change in pull request #3996: Rewrote /user/current 
to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341719046
 
 

 ##
 File path: lib/go-tc/users.go
 ##
 @@ -101,6 +110,179 @@ type UserCurrent struct {
commonUserFields
 }
 
+// CurrentUserUpdateRequest differs from a regular User/UserCurrent in that 
many of its fields are
+// *parsed* but not *unmarshaled*. This allows a handler to distinguish 
between "null" and
+// "undefined" values.
+type CurrentUserUpdateRequest struct {
+   // User, for whatever reason, contains all of the actual data.
+   User CurrentUserUpdateRequestUser `json:"user"`
+}
+
+// CurrentUserUpdateRequestUser holds all of the actual data in a request to 
update the current user.
+type CurrentUserUpdateRequestUser struct {
+   AddressLine1   json.RawMessage `json:"addressLine1"`
+   AddressLine2   json.RawMessage `json:"addressLine2"`
+   City   json.RawMessage `json:"city"`
+   Companyjson.RawMessage `json:"company"`
+   ConfirmLocalPasswd *string `json:"confirmLocalPasswd"`
+   Countryjson.RawMessage `json:"country"`
+   Email  json.RawMessage `json:"email"`
+   FullName   json.RawMessage `json:"fullName"`
+   GIDjson.RawMessage `json:"gid"`
+   ID json.RawMessage `json:"id"`
+   LocalPasswd*string `json:"localPasswd"`
+   PhoneNumberjson.RawMessage `json:"phoneNumber"`
+   PostalCode json.RawMessage `json:"postalCode"`
+   PublicSSHKey   json.RawMessage `json:"publicSshKey"`
+   Role   json.RawMessage `json:"role"`
+   StateOrProvincejson.RawMessage `json:"stateOrProvince"`
+   TenantID   json.RawMessage `json:"tenantId"`
+   UIDjson.RawMessage `json:"uid"`
+   Username   json.RawMessage `json:"username"`
+}
+
+// ValidateAndUnmarshal validates the request and returns a User into which 
the request's information
+// has been unmarshalled. This allows many fields to be "null", but explicitly 
checks that they are
+// present in the JSON payload.
+func (u *CurrentUserUpdateRequestUser) ValidateAndUnmarshal(user *User) error {
 
 Review comment:
   I don't think it is a big deal. Just something I noticed that each field is 
first unmarshaled and then validated. It also makes sense in general that 
parsing usually happens before validation in most code. It was just a 
suggestion.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ChrisHines commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ChrisHines commented on a change in pull request #3996: Rewrote /user/current 
to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341718150
 
 

 ##
 File path: lib/go-tc/users.go
 ##
 @@ -101,6 +110,179 @@ type UserCurrent struct {
commonUserFields
 }
 
+// CurrentUserUpdateRequest differs from a regular User/UserCurrent in that 
many of its fields are
+// *parsed* but not *unmarshaled*. This allows a handler to distinguish 
between "null" and
+// "undefined" values.
+type CurrentUserUpdateRequest struct {
+   // User, for whatever reason, contains all of the actual data.
+   User CurrentUserUpdateRequestUser `json:"user"`
+}
+
+// CurrentUserUpdateRequestUser holds all of the actual data in a request to 
update the current user.
+type CurrentUserUpdateRequestUser struct {
+   AddressLine1   json.RawMessage `json:"addressLine1"`
+   AddressLine2   json.RawMessage `json:"addressLine2"`
+   City   json.RawMessage `json:"city"`
+   Companyjson.RawMessage `json:"company"`
+   ConfirmLocalPasswd *string `json:"confirmLocalPasswd"`
+   Countryjson.RawMessage `json:"country"`
+   Email  json.RawMessage `json:"email"`
+   FullName   json.RawMessage `json:"fullName"`
+   GIDjson.RawMessage `json:"gid"`
+   ID json.RawMessage `json:"id"`
+   LocalPasswd*string `json:"localPasswd"`
+   PhoneNumberjson.RawMessage `json:"phoneNumber"`
+   PostalCode json.RawMessage `json:"postalCode"`
+   PublicSSHKey   json.RawMessage `json:"publicSshKey"`
+   Role   json.RawMessage `json:"role"`
+   StateOrProvincejson.RawMessage `json:"stateOrProvince"`
+   TenantID   json.RawMessage `json:"tenantId"`
+   UIDjson.RawMessage `json:"uid"`
+   Username   json.RawMessage `json:"username"`
+}
+
+// ValidateAndUnmarshal validates the request and returns a User into which 
the request's information
+// has been unmarshalled. This allows many fields to be "null", but explicitly 
checks that they are
+// present in the JSON payload.
+func (u *CurrentUserUpdateRequestUser) ValidateAndUnmarshal(user *User) error {
+   errs := []error{}
+   if u.AddressLine1 != nil {
+   if err := json.Unmarshal(u.AddressLine1, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine1: %v", err))
+   }
+   }
+
+   if u.AddressLine2 != nil {
+   if err := json.Unmarshal(u.AddressLine2, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine2: %v", err))
+   }
+   }
+
+   if u.City != nil {
+   if err := json.Unmarshal(u.City, ); err != nil {
+   errs = append(errs, fmt.Errorf("city: %v", err))
+   }
+   }
+
+   if u.Company != nil {
+   if err := json.Unmarshal(u.Company, ); err != nil {
+   errs = append(errs, fmt.Errorf("company: %v", err))
+   }
+   }
+
+   if u.LocalPasswd != nil && *u.LocalPasswd != "" {
+   if u.ConfirmLocalPasswd == nil || *u.ConfirmLocalPasswd == "" {
+   errs = append(errs, errors.New("confirmLocalPasswd: 
required when changing password"))
+   } else if *u.LocalPasswd != *u.ConfirmLocalPasswd {
+   errs = append(errs, errors.New("localPasswd and 
confirmLocalPasswd do not match"))
+   }
+   } else if u.ConfirmLocalPasswd != nil && *u.ConfirmLocalPasswd != "" {
+   errs = append(errs, errors.New("localPasswd: required when 
changing password"))
+   }
+   user.ConfirmLocalPassword = u.ConfirmLocalPasswd
+   user.LocalPassword = u.LocalPasswd
+
+   if u.Country != nil {
+   if err := json.Unmarshal(u.Country, ); err != nil {
+   errs = append(errs, fmt.Errorf("country: %v", err))
+   }
+   }
+
+   if u.Email != nil {
+   if err := json.Unmarshal(u.Email, ); err != nil {
+   errs = append(errs, fmt.Errorf("email: %v", err))
+   } else if err = validation.Validate(*user.Email, is.Email); err 
!= nil {
+   errs = append(errs, err)
+   }
+   }
+
+   if u.FullName != nil {
+   if err := json.Unmarshal(u.FullName, ); err != 
nil {
+   errs = append(errs, fmt.Errorf("fullName: %v", err))
+   } else if user.FullName == nil || *user.FullName == "" {
+   // Perl enforced this
+   errs = append(errs, fmt.Errorf("fullName: cannot be set 
to 'null' or empty string"))
+   }
+   }
 
 Review comment:
   Your top PR comment led me to believe that the new Go implementation 

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341717527
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
 
 Review comment:
   That's true. I don't really think it's a big deal either way, but I can 
change it if you want.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341716891
 
 

 ##
 File path: lib/go-tc/users.go
 ##
 @@ -101,6 +110,179 @@ type UserCurrent struct {
commonUserFields
 }
 
+// CurrentUserUpdateRequest differs from a regular User/UserCurrent in that 
many of its fields are
+// *parsed* but not *unmarshaled*. This allows a handler to distinguish 
between "null" and
+// "undefined" values.
+type CurrentUserUpdateRequest struct {
+   // User, for whatever reason, contains all of the actual data.
+   User CurrentUserUpdateRequestUser `json:"user"`
+}
+
+// CurrentUserUpdateRequestUser holds all of the actual data in a request to 
update the current user.
+type CurrentUserUpdateRequestUser struct {
+   AddressLine1   json.RawMessage `json:"addressLine1"`
+   AddressLine2   json.RawMessage `json:"addressLine2"`
+   City   json.RawMessage `json:"city"`
+   Companyjson.RawMessage `json:"company"`
+   ConfirmLocalPasswd *string `json:"confirmLocalPasswd"`
+   Countryjson.RawMessage `json:"country"`
+   Email  json.RawMessage `json:"email"`
+   FullName   json.RawMessage `json:"fullName"`
+   GIDjson.RawMessage `json:"gid"`
+   ID json.RawMessage `json:"id"`
+   LocalPasswd*string `json:"localPasswd"`
+   PhoneNumberjson.RawMessage `json:"phoneNumber"`
+   PostalCode json.RawMessage `json:"postalCode"`
+   PublicSSHKey   json.RawMessage `json:"publicSshKey"`
+   Role   json.RawMessage `json:"role"`
+   StateOrProvincejson.RawMessage `json:"stateOrProvince"`
+   TenantID   json.RawMessage `json:"tenantId"`
+   UIDjson.RawMessage `json:"uid"`
+   Username   json.RawMessage `json:"username"`
+}
+
+// ValidateAndUnmarshal validates the request and returns a User into which 
the request's information
+// has been unmarshalled. This allows many fields to be "null", but explicitly 
checks that they are
+// present in the JSON payload.
+func (u *CurrentUserUpdateRequestUser) ValidateAndUnmarshal(user *User) error {
 
 Review comment:
   It does both concurrently, so I'm not sure the ordering is important.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341716689
 
 

 ##
 File path: lib/go-tc/users.go
 ##
 @@ -101,6 +110,179 @@ type UserCurrent struct {
commonUserFields
 }
 
+// CurrentUserUpdateRequest differs from a regular User/UserCurrent in that 
many of its fields are
+// *parsed* but not *unmarshaled*. This allows a handler to distinguish 
between "null" and
+// "undefined" values.
+type CurrentUserUpdateRequest struct {
+   // User, for whatever reason, contains all of the actual data.
+   User CurrentUserUpdateRequestUser `json:"user"`
+}
+
+// CurrentUserUpdateRequestUser holds all of the actual data in a request to 
update the current user.
+type CurrentUserUpdateRequestUser struct {
+   AddressLine1   json.RawMessage `json:"addressLine1"`
+   AddressLine2   json.RawMessage `json:"addressLine2"`
+   City   json.RawMessage `json:"city"`
+   Companyjson.RawMessage `json:"company"`
+   ConfirmLocalPasswd *string `json:"confirmLocalPasswd"`
+   Countryjson.RawMessage `json:"country"`
+   Email  json.RawMessage `json:"email"`
+   FullName   json.RawMessage `json:"fullName"`
+   GIDjson.RawMessage `json:"gid"`
+   ID json.RawMessage `json:"id"`
+   LocalPasswd*string `json:"localPasswd"`
+   PhoneNumberjson.RawMessage `json:"phoneNumber"`
+   PostalCode json.RawMessage `json:"postalCode"`
+   PublicSSHKey   json.RawMessage `json:"publicSshKey"`
+   Role   json.RawMessage `json:"role"`
+   StateOrProvincejson.RawMessage `json:"stateOrProvince"`
+   TenantID   json.RawMessage `json:"tenantId"`
+   UIDjson.RawMessage `json:"uid"`
+   Username   json.RawMessage `json:"username"`
+}
+
+// ValidateAndUnmarshal validates the request and returns a User into which 
the request's information
+// has been unmarshalled. This allows many fields to be "null", but explicitly 
checks that they are
+// present in the JSON payload.
+func (u *CurrentUserUpdateRequestUser) ValidateAndUnmarshal(user *User) error {
+   errs := []error{}
+   if u.AddressLine1 != nil {
+   if err := json.Unmarshal(u.AddressLine1, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine1: %v", err))
+   }
+   }
+
+   if u.AddressLine2 != nil {
+   if err := json.Unmarshal(u.AddressLine2, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine2: %v", err))
+   }
+   }
+
+   if u.City != nil {
+   if err := json.Unmarshal(u.City, ); err != nil {
+   errs = append(errs, fmt.Errorf("city: %v", err))
+   }
+   }
+
+   if u.Company != nil {
+   if err := json.Unmarshal(u.Company, ); err != nil {
+   errs = append(errs, fmt.Errorf("company: %v", err))
+   }
+   }
+
+   if u.LocalPasswd != nil && *u.LocalPasswd != "" {
+   if u.ConfirmLocalPasswd == nil || *u.ConfirmLocalPasswd == "" {
+   errs = append(errs, errors.New("confirmLocalPasswd: 
required when changing password"))
+   } else if *u.LocalPasswd != *u.ConfirmLocalPasswd {
+   errs = append(errs, errors.New("localPasswd and 
confirmLocalPasswd do not match"))
+   }
+   } else if u.ConfirmLocalPasswd != nil && *u.ConfirmLocalPasswd != "" {
+   errs = append(errs, errors.New("localPasswd: required when 
changing password"))
+   }
+   user.ConfirmLocalPassword = u.ConfirmLocalPasswd
+   user.LocalPassword = u.LocalPasswd
+
+   if u.Country != nil {
+   if err := json.Unmarshal(u.Country, ); err != nil {
+   errs = append(errs, fmt.Errorf("country: %v", err))
+   }
+   }
+
+   if u.Email != nil {
+   if err := json.Unmarshal(u.Email, ); err != nil {
+   errs = append(errs, fmt.Errorf("email: %v", err))
+   } else if err = validation.Validate(*user.Email, is.Email); err 
!= nil {
+   errs = append(errs, err)
+   }
+   }
+
+   if u.FullName != nil {
+   if err := json.Unmarshal(u.FullName, ); err != 
nil {
+   errs = append(errs, fmt.Errorf("fullName: %v", err))
+   } else if user.FullName == nil || *user.FullName == "" {
+   // Perl enforced this
+   errs = append(errs, fmt.Errorf("fullName: cannot be set 
to 'null' or empty string"))
+   }
+   }
 
 Review comment:
   Actually, I think NaN and the infinities are extensions of the Python 

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341716336
 
 

 ##
 File path: lib/go-tc/users.go
 ##
 @@ -101,6 +110,179 @@ type UserCurrent struct {
commonUserFields
 }
 
+// CurrentUserUpdateRequest differs from a regular User/UserCurrent in that 
many of its fields are
+// *parsed* but not *unmarshaled*. This allows a handler to distinguish 
between "null" and
+// "undefined" values.
+type CurrentUserUpdateRequest struct {
+   // User, for whatever reason, contains all of the actual data.
+   User CurrentUserUpdateRequestUser `json:"user"`
+}
+
+// CurrentUserUpdateRequestUser holds all of the actual data in a request to 
update the current user.
+type CurrentUserUpdateRequestUser struct {
+   AddressLine1   json.RawMessage `json:"addressLine1"`
+   AddressLine2   json.RawMessage `json:"addressLine2"`
+   City   json.RawMessage `json:"city"`
+   Companyjson.RawMessage `json:"company"`
+   ConfirmLocalPasswd *string `json:"confirmLocalPasswd"`
+   Countryjson.RawMessage `json:"country"`
+   Email  json.RawMessage `json:"email"`
+   FullName   json.RawMessage `json:"fullName"`
+   GIDjson.RawMessage `json:"gid"`
+   ID json.RawMessage `json:"id"`
+   LocalPasswd*string `json:"localPasswd"`
+   PhoneNumberjson.RawMessage `json:"phoneNumber"`
+   PostalCode json.RawMessage `json:"postalCode"`
+   PublicSSHKey   json.RawMessage `json:"publicSshKey"`
+   Role   json.RawMessage `json:"role"`
+   StateOrProvincejson.RawMessage `json:"stateOrProvince"`
+   TenantID   json.RawMessage `json:"tenantId"`
+   UIDjson.RawMessage `json:"uid"`
+   Username   json.RawMessage `json:"username"`
+}
+
+// ValidateAndUnmarshal validates the request and returns a User into which 
the request's information
+// has been unmarshalled. This allows many fields to be "null", but explicitly 
checks that they are
+// present in the JSON payload.
+func (u *CurrentUserUpdateRequestUser) ValidateAndUnmarshal(user *User) error {
+   errs := []error{}
+   if u.AddressLine1 != nil {
+   if err := json.Unmarshal(u.AddressLine1, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine1: %v", err))
+   }
+   }
+
+   if u.AddressLine2 != nil {
+   if err := json.Unmarshal(u.AddressLine2, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine2: %v", err))
+   }
+   }
+
+   if u.City != nil {
+   if err := json.Unmarshal(u.City, ); err != nil {
+   errs = append(errs, fmt.Errorf("city: %v", err))
+   }
+   }
+
+   if u.Company != nil {
+   if err := json.Unmarshal(u.Company, ); err != nil {
+   errs = append(errs, fmt.Errorf("company: %v", err))
+   }
+   }
+
+   if u.LocalPasswd != nil && *u.LocalPasswd != "" {
+   if u.ConfirmLocalPasswd == nil || *u.ConfirmLocalPasswd == "" {
+   errs = append(errs, errors.New("confirmLocalPasswd: 
required when changing password"))
+   } else if *u.LocalPasswd != *u.ConfirmLocalPasswd {
+   errs = append(errs, errors.New("localPasswd and 
confirmLocalPasswd do not match"))
+   }
+   } else if u.ConfirmLocalPasswd != nil && *u.ConfirmLocalPasswd != "" {
+   errs = append(errs, errors.New("localPasswd: required when 
changing password"))
+   }
+   user.ConfirmLocalPassword = u.ConfirmLocalPasswd
+   user.LocalPassword = u.LocalPasswd
+
+   if u.Country != nil {
+   if err := json.Unmarshal(u.Country, ); err != nil {
+   errs = append(errs, fmt.Errorf("country: %v", err))
+   }
+   }
+
+   if u.Email != nil {
+   if err := json.Unmarshal(u.Email, ); err != nil {
+   errs = append(errs, fmt.Errorf("email: %v", err))
+   } else if err = validation.Validate(*user.Email, is.Email); err 
!= nil {
+   errs = append(errs, err)
+   }
+   }
+
+   if u.FullName != nil {
+   if err := json.Unmarshal(u.FullName, ); err != 
nil {
+   errs = append(errs, fmt.Errorf("fullName: %v", err))
+   } else if user.FullName == nil || *user.FullName == "" {
+   // Perl enforced this
+   errs = append(errs, fmt.Errorf("fullName: cannot be set 
to 'null' or empty string"))
+   }
+   }
 
 Review comment:
   When a `json.RawMessage` compares like `rm == nil` it means the value 

[GitHub] [trafficcontrol] ChrisHines commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ChrisHines commented on a change in pull request #3996: Rewrote /user/current 
to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341710600
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
 
 Review comment:
   `else` is redundant here since the `if` block aways returns.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ChrisHines commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ChrisHines commented on a change in pull request #3996: Rewrote /user/current 
to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341714459
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
+   sysErr = fmt.Errorf("Current user (#%d) doesn't exist... ??", 
inf.User.ID)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   if err := userRequest.User.ValidateAndUnmarshal(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   changePasswd := false
+
+   // obfuscate passwords (ValidateAndUnmarshal checks for equality with 
ConfirmLocalPassword)
+   // TODO: check for valid password via bad password list like Perl did? 
User creation doesn't...
+   if user.LocalPassword != nil && *user.LocalPassword != "" {
+   hashPass, err := auth.DerivePassword(*user.LocalPassword)
+   if err != nil {
+   sysErr = fmt.Errorf("Hashing new password: %v", err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   changePasswd = true
+   user.LocalPassword = util.StrPtr(hashPass)
+   user.ConfirmLocalPassword = util.StrPtr(hashPass)
+   }
+
+   if *user.Role != inf.User.Role {
+   privLevel, exists, err := dbhelpers.GetPrivLevelFromRoleID(tx, 
*user.Role)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting privLevel for Role #%d: 
%v", *user.Role, err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   if !exists {
+   userErr = fmt.Errorf("role: no such role: %d", 
*user.Role)
+   errCode = http.StatusNotFound
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+   if privLevel > inf.User.PrivLevel {
+   userErr = errors.New("role: cannot have greater 
permissions than user's current role")
+   errCode = http.StatusForbidden
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+   }
+
+   if ok, err := tenant.IsResourceAuthorizedToUserTx(*user.TenantID, 
inf.User, tx); err != nil {
+   if err == sql.ErrNoRows {
+   userErr = errors.New("No such tenant!")
+   errCode = http.StatusNotFound
+   } else {
+   sysErr = fmt.Errorf("Checking user %s permissions on 
tenant #%d: %v", inf.User.UserName, *user.TenantID, err)
+   errCode = http.StatusInternalServerError
+   }
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   } else if !ok {
+   // unlike Perl, this endpoint will not disclose the existence 
of tenants over which the current
+   // user has no permission - in keeping with the behavior of the 
'/tenants' endpoint.
+   userErr = errors.New("No such tenant!")
+   errCode = http.StatusNotFound
+   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   return
+   }
+
+   if *user.Username != inf.User.UserName {
+
+   if ok, err := 

[GitHub] [trafficcontrol] ChrisHines commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ChrisHines commented on a change in pull request #3996: Rewrote /user/current 
to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341705696
 
 

 ##
 File path: lib/go-tc/users.go
 ##
 @@ -101,6 +110,179 @@ type UserCurrent struct {
commonUserFields
 }
 
+// CurrentUserUpdateRequest differs from a regular User/UserCurrent in that 
many of its fields are
+// *parsed* but not *unmarshaled*. This allows a handler to distinguish 
between "null" and
+// "undefined" values.
+type CurrentUserUpdateRequest struct {
+   // User, for whatever reason, contains all of the actual data.
+   User CurrentUserUpdateRequestUser `json:"user"`
+}
+
+// CurrentUserUpdateRequestUser holds all of the actual data in a request to 
update the current user.
+type CurrentUserUpdateRequestUser struct {
+   AddressLine1   json.RawMessage `json:"addressLine1"`
+   AddressLine2   json.RawMessage `json:"addressLine2"`
+   City   json.RawMessage `json:"city"`
+   Companyjson.RawMessage `json:"company"`
+   ConfirmLocalPasswd *string `json:"confirmLocalPasswd"`
+   Countryjson.RawMessage `json:"country"`
+   Email  json.RawMessage `json:"email"`
+   FullName   json.RawMessage `json:"fullName"`
+   GIDjson.RawMessage `json:"gid"`
+   ID json.RawMessage `json:"id"`
+   LocalPasswd*string `json:"localPasswd"`
+   PhoneNumberjson.RawMessage `json:"phoneNumber"`
+   PostalCode json.RawMessage `json:"postalCode"`
+   PublicSSHKey   json.RawMessage `json:"publicSshKey"`
+   Role   json.RawMessage `json:"role"`
+   StateOrProvincejson.RawMessage `json:"stateOrProvince"`
+   TenantID   json.RawMessage `json:"tenantId"`
+   UIDjson.RawMessage `json:"uid"`
+   Username   json.RawMessage `json:"username"`
+}
+
+// ValidateAndUnmarshal validates the request and returns a User into which 
the request's information
+// has been unmarshalled. This allows many fields to be "null", but explicitly 
checks that they are
+// present in the JSON payload.
+func (u *CurrentUserUpdateRequestUser) ValidateAndUnmarshal(user *User) error {
 
 Review comment:
   Ignore this if it bucks an existing convention I am not aware of.
   
   Consider wether the name `UnmarshalAndValidate` does a better job conveying 
the order of operations this method performs.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ChrisHines commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ChrisHines commented on a change in pull request #3996: Rewrote /user/current 
to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341712506
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,197 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
+   sysErr = fmt.Errorf("Current user (#%d) doesn't exist... ??", 
inf.User.ID)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+
+   if err := userRequest.User.ValidateAndUnmarshal(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   changePasswd := false
+
+   // obfuscate passwords (ValidateAndUnmarshal checks for equality with 
ConfirmLocalPassword)
+   // TODO: check for valid password via bad password list like Perl did? 
User creation doesn't...
+   if user.LocalPassword != nil && *user.LocalPassword != "" {
+   hashPass, err := auth.DerivePassword(*user.LocalPassword)
+   if err != nil {
+   sysErr = fmt.Errorf("Hashing new password: %v", err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   }
+   changePasswd = true
 
 Review comment:
   Reading this raises a question in my mind: Is it true that a `nil` or empty 
`*user.LocalPassword` means "no password change" requested?


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:
us...@infra.apache.org


With regards,
Apache Git Services

Jenkins build is back to normal : trafficcontrol-PR #4693

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] asf-ci commented on issue #4063: Add atstccfg meta config

[GitBox]

asf-ci commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548910331
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4693/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #4063: Add atstccfg meta config

[GitBox]

asf-ci commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548907935
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4692/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

Build failed in Jenkins: trafficcontrol-PR #4692

[Apache Jenkins Server]

See 

Changes:


--
GitHub pull request #4063 of commit b651255ebf6d29f2cd9576cf1715cae88c80d014, 
no merge conflicts.
Running as SYSTEM
Setting status of b651255ebf6d29f2cd9576cf1715cae88c80d014 to PENDING with url 
https://builds.apache.org/job/trafficcontrol-PR/4692/ and message: 'Build 
started for merge commit.'
Using context: default
[EnvInject] - Loading node environment variables.
Building remotely on H29 (ubuntu) in workspace 

[WS-CLEANUP] Deleting project workspace...
[WS-CLEANUP] Deferred wipeout is used...
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
Cloning the remote Git repository
Cloning repository git://github.com/apache/trafficcontrol.git
 > git init  # timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/heads/*:refs/remotes/origin/*
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # 
 > timeout=10
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # timeout=10
 > git rev-parse origin/b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # 
 > timeout=10
 > git rev-parse b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # timeout=10
 > git rev-parse origin/b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # 
 > timeout=10
 > git rev-parse b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Retrying after 10 seconds
using credential b205a645-1ea7-4dfd-973d-c14ac43cab07
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url git://github.com/apache/trafficcontrol.git # 
 > timeout=10
Fetching upstream changes from git://github.com/apache/trafficcontrol.git
 > git --version # timeout=10
using GIT_SSH to set credentials 
 > git fetch --tags --progress -- git://github.com/apache/trafficcontrol.git 
 > +refs/pull/*:refs/remotes/origin/pr/*
 > git rev-parse b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # timeout=10
 > git rev-parse origin/b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # 
 > timeout=10
 > git rev-parse b651255ebf6d29f2cd9576cf1715cae88c80d014^{commit} # timeout=10
ERROR: Couldn't find any revision to build. Verify the repository and branch 
configuration for this job.
Skipped archiving because build is not successful

[GitHub] [trafficcontrol] asf-ci commented on issue #4067: adds ds required capabilities link to using TP ds section

[GitBox]

asf-ci commented on issue #4067: adds ds required capabilities link to using TP 
ds section
URL: https://github.com/apache/trafficcontrol/pull/4067#issuecomment-548906988
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4691/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] alficles commented on a change in pull request #4063: Add atstccfg meta config

[GitBox]

alficles commented on a change in pull request #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#discussion_r341693792
 
 

 ##
 File path: lib/go-atscfg/meta.go
 ##
 @@ -0,0 +1,195 @@
+package atscfg
+
+/*
+ * 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 (
+   "encoding/json"
+   "strings"
+
+   "github.com/apache/trafficcontrol/lib/go-log"
+   "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+type ConfigProfileParams struct {
+   FileNameOnDisk string
+   Location   string
+   URLstring
+   APIURI string
+}
+
+func MakeMetaConfig(
+   serverHostName tc.CacheName,
+   server *ServerInfo,
+   tmURL string, // global tm.url Parameter
+   tmReverseProxyURL string, // global tm.rev_proxy.url Parameter
+   locationParams map[string]ConfigProfileParams, // 
map[configFile]params; 'location' and 'URL' Parameters on serverHostName's 
Profile
+   uriSignedDSes []tc.DeliveryServiceName,
+   scopeParams map[string]string, // map[configFileName]scopeParam
+) string {
+   if tmURL == "" {
+   log.Errorln("ats.GetConfigMetadata: global tm.url parameter 
missing or empty! Setting empty in meta config!")
+   }
+
+   atsData := tc.ATSConfigMetaData{
+   Info: tc.ATSConfigMetaDataInfo{
+   ProfileID: int(server.ProfileID),
+   TOReverseProxyURL: tmReverseProxyURL,
+   TOURL: tmURL,
+   ServerIPv4:server.IP,
+   ServerPort:server.Port,
+   ServerName:server.HostName,
+   CDNID: server.CDNID,
+   CDNName:   string(server.CDN),
+   ServerID:  server.ID,
+   ProfileName:   server.ProfileName,
+   },
+   ConfigFiles: []tc.ATSConfigMetaDataConfigFile{},
+   }
+
+   if locationParams["remap.config"].Location != "" {
+   configLocation := locationParams["remap.config"].Location
+   for _, ds := range uriSignedDSes {
+   cfgName := "uri_signing_" + string(ds) + ".config"
+   // If there's already a parameter for it, don't clobber 
it. The user may wish to override the location.
+   if _, ok := locationParams[cfgName]; !ok {
+   p := locationParams[cfgName]
+   p.FileNameOnDisk = cfgName
+   p.Location = configLocation
+   locationParams[cfgName] = p
+   }
+   }
+   }
+
+   for cfgFile, cfgParams := range locationParams {
+   atsCfg := tc.ATSConfigMetaDataConfigFile{
+   FileNameOnDisk: cfgParams.FileNameOnDisk,
+   Location:   cfgParams.Location,
+   }
+
+   scope := getServerScope(cfgFile, server.Type, scopeParams)
+
+   if cfgParams.URL != "" {
+   scope = tc.ATSConfigMetaDataConfigFileScopeCDNs
+   atsCfg.URL = cfgParams.URL
+   } else {
+   scopeID := ""
+   if scope == tc.ATSConfigMetaDataConfigFileScopeCDNs {
+   scopeID = string(server.CDN)
+   } else if scope == 
tc.ATSConfigMetaDataConfigFileScopeProfiles {
+   scopeID = server.ProfileName
+   } else { // ATSConfigMetaDataConfigFileScopeServers
+   scopeID = server.HostName
+   }
+   atsCfg.APIURI = "/api/1.2/" + string(scope) + "/" + 
scopeID + "/configfiles/ats/" + cfgFile
 
 Review comment:
   Hardcoded 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.
 
For queries about 

[GitHub] [trafficcontrol] alficles commented on a change in pull request #4063: Add atstccfg meta config

[GitBox]

alficles commented on a change in pull request #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#discussion_r341695918
 
 

 ##
 File path: lib/go-atscfg/meta.go
 ##
 @@ -0,0 +1,195 @@
+package atscfg
+
+/*
+ * 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 (
+   "encoding/json"
+   "strings"
+
+   "github.com/apache/trafficcontrol/lib/go-log"
+   "github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+type ConfigProfileParams struct {
+   FileNameOnDisk string
+   Location   string
+   URLstring
+   APIURI string
+}
+
+func MakeMetaConfig(
+   serverHostName tc.CacheName,
+   server *ServerInfo,
+   tmURL string, // global tm.url Parameter
+   tmReverseProxyURL string, // global tm.rev_proxy.url Parameter
+   locationParams map[string]ConfigProfileParams, // 
map[configFile]params; 'location' and 'URL' Parameters on serverHostName's 
Profile
+   uriSignedDSes []tc.DeliveryServiceName,
+   scopeParams map[string]string, // map[configFileName]scopeParam
+) string {
+   if tmURL == "" {
+   log.Errorln("ats.GetConfigMetadata: global tm.url parameter 
missing or empty! Setting empty in meta config!")
+   }
+
+   atsData := tc.ATSConfigMetaData{
+   Info: tc.ATSConfigMetaDataInfo{
+   ProfileID: int(server.ProfileID),
+   TOReverseProxyURL: tmReverseProxyURL,
+   TOURL: tmURL,
+   ServerIPv4:server.IP,
+   ServerPort:server.Port,
+   ServerName:server.HostName,
+   CDNID: server.CDNID,
+   CDNName:   string(server.CDN),
+   ServerID:  server.ID,
+   ProfileName:   server.ProfileName,
+   },
+   ConfigFiles: []tc.ATSConfigMetaDataConfigFile{},
+   }
+
+   if locationParams["remap.config"].Location != "" {
+   configLocation := locationParams["remap.config"].Location
+   for _, ds := range uriSignedDSes {
+   cfgName := "uri_signing_" + string(ds) + ".config"
+   // If there's already a parameter for it, don't clobber 
it. The user may wish to override the location.
+   if _, ok := locationParams[cfgName]; !ok {
+   p := locationParams[cfgName]
+   p.FileNameOnDisk = cfgName
+   p.Location = configLocation
+   locationParams[cfgName] = p
+   }
+   }
+   }
+
+   for cfgFile, cfgParams := range locationParams {
+   atsCfg := tc.ATSConfigMetaDataConfigFile{
+   FileNameOnDisk: cfgParams.FileNameOnDisk,
+   Location:   cfgParams.Location,
+   }
+
+   scope := getServerScope(cfgFile, server.Type, scopeParams)
+
+   if cfgParams.URL != "" {
+   scope = tc.ATSConfigMetaDataConfigFileScopeCDNs
+   atsCfg.URL = cfgParams.URL
+   } else {
+   scopeID := ""
+   if scope == tc.ATSConfigMetaDataConfigFileScopeCDNs {
+   scopeID = string(server.CDN)
+   } else if scope == 
tc.ATSConfigMetaDataConfigFileScopeProfiles {
+   scopeID = server.ProfileName
+   } else { // ATSConfigMetaDataConfigFileScopeServers
+   scopeID = server.HostName
+   }
+   atsCfg.APIURI = "/api/1.2/" + string(scope) + "/" + 
scopeID + "/configfiles/ats/" + cfgFile
+   }
+
+   atsCfg.Scope = string(scope)
+
+   atsData.ConfigFiles = append(atsData.ConfigFiles, atsCfg)
+   }
+
+   bts, err := json.Marshal(atsData)
+   if err != nil {
+   // should never happen
+   

[GitHub] [trafficcontrol] jheitz200 commented on a change in pull request #4059: return 403 when unauthorized user assigns/deletes ds required capability

[GitBox]

jheitz200 commented on a change in pull request #4059: return 403 when 
unauthorized user assigns/deletes ds required capability
URL: https://github.com/apache/trafficcontrol/pull/4059#discussion_r341696590
 
 

 ##
 File path: 
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go
 ##
 @@ -219,22 +219,51 @@ func (rc *RequiredCapability) getCapabilities(tenantIDs 
[]int) ([]tc.DeliverySer
 // Delete implements the api.CRUDer interface.
 func (rc *RequiredCapability) Delete() (error, error, int) {
authorized, err := rc.isTenantAuthorized()
-   if err != nil {
-   return nil, errors.New("checking tenant: " + err.Error()), 
http.StatusInternalServerError
-   } else if !authorized {
+   if !authorized {
return errors.New("not authorized on this tenant"), nil, 
http.StatusForbidden
+   } else if err != nil {
+   return nil, fmt.Errorf("checking authorization for existing DS 
ID: %s" + err.Error()), http.StatusInternalServerError
+   }
+
+   // Check if the Delivery Service is associated with the Required 
Capability
+   statusCode, err := rc.exists()
+   if err != nil {
+   return err, nil, statusCode
}
 
return api.GenericDelete(rc)
 }
 
+// exists checks to see if a required capability is assigned to a delivery 
service
+func (rc *RequiredCapability) exists() (int, error) {
 
 Review comment:
   I dont have my head wrapped around all of these interfaces yet
   
   this was added before i realized the auth if check needed switched. thanks 
for pointing out the functionality of this interface. removing this code. 
updates coming


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 opened a new pull request #4067: adds ds required capabilities link to using TP ds section

[GitBox]

mitchell852 opened a new pull request #4067: adds ds required capabilities link 
to using TP ds section
URL: https://github.com/apache/trafficcontrol/pull/4067
 
 
   ## What does this PR (Pull Request) do?
   
   - [ ] This PR fixes #4062 
   
   ## Which Traffic Control components are affected by this PR?
   
   - Documentation
   
   ## What is the best way to verify this PR?
   Build the docs and open 
/docs/build/html/admin/traffic_portal/usingtrafficportal.html#delivery-services 
and check the `Manage Delivery Service required server capabilities` link that 
was added.
   
   As this is a simple documentation change, no tests or changelog.md entry.
   
   ## The following criteria are ALL met by this PR
   
   - [x] This PR includes tests OR I have explained why tests are unnecessary
   - [x] This PR includes documentation OR I have explained why documentation 
is unnecessary
   - [x] This PR includes an update to CHANGELOG.md OR such an update is not 
necessary
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR 
does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the 
Apache Software Foundation's security 
guidelines](https://www.apache.org/security/) for details)
   
   
   ## Additional Information
   
   
   
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] jheitz200 commented on a change in pull request #4059: return 403 when unauthorized user assigns/deletes ds required capability

[GitBox]

jheitz200 commented on a change in pull request #4059: return 403 when 
unauthorized user assigns/deletes ds required capability
URL: https://github.com/apache/trafficcontrol/pull/4059#discussion_r341690403
 
 

 ##
 File path: 
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go
 ##
 @@ -219,22 +219,51 @@ func (rc *RequiredCapability) getCapabilities(tenantIDs 
[]int) ([]tc.DeliverySer
 // Delete implements the api.CRUDer interface.
 func (rc *RequiredCapability) Delete() (error, error, int) {
authorized, err := rc.isTenantAuthorized()
-   if err != nil {
-   return nil, errors.New("checking tenant: " + err.Error()), 
http.StatusInternalServerError
-   } else if !authorized {
+   if !authorized {
return errors.New("not authorized on this tenant"), nil, 
http.StatusForbidden
+   } else if err != nil {
+   return nil, fmt.Errorf("checking authorization for existing DS 
ID: %s" + err.Error()), http.StatusInternalServerError
+   }
+
+   // Check if the Delivery Service is associated with the Required 
Capability
+   statusCode, err := rc.exists()
+   if err != nil {
+   return err, nil, statusCode
}
 
return api.GenericDelete(rc)
 }
 
+// exists checks to see if a required capability is assigned to a delivery 
service
+func (rc *RequiredCapability) exists() (int, error) {
+   where, orderBy, pagination, queryValues, errs := 
dbhelpers.BuildWhereAndOrderByAndPagination(rc.APIInfo().Params, 
rc.ParamColumns())
+   if len(errs) > 0 {
+   return http.StatusBadRequest, util.JoinErrs(errs)
+   }
+   fmt.Printf("\n\nwhere: %+v\norderby: %+v\npag: %+v\nqueryvalues: 
%+v\n\n", where, orderBy, pagination, queryValues)
 
 Review comment:
   thanks, i complete forgot i wanted to see what that helper function did and 
use it if possible. looking into this 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4059: return 403 when unauthorized user assigns/deletes ds required capability

[GitBox]

mhoppa commented on a change in pull request #4059: return 403 when 
unauthorized user assigns/deletes ds required capability
URL: https://github.com/apache/trafficcontrol/pull/4059#discussion_r341685615
 
 

 ##
 File path: 
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go
 ##
 @@ -219,22 +219,51 @@ func (rc *RequiredCapability) getCapabilities(tenantIDs 
[]int) ([]tc.DeliverySer
 // Delete implements the api.CRUDer interface.
 func (rc *RequiredCapability) Delete() (error, error, int) {
authorized, err := rc.isTenantAuthorized()
-   if err != nil {
-   return nil, errors.New("checking tenant: " + err.Error()), 
http.StatusInternalServerError
-   } else if !authorized {
+   if !authorized {
return errors.New("not authorized on this tenant"), nil, 
http.StatusForbidden
+   } else if err != nil {
+   return nil, fmt.Errorf("checking authorization for existing DS 
ID: %s" + err.Error()), http.StatusInternalServerError
+   }
+
+   // Check if the Delivery Service is associated with the Required 
Capability
+   statusCode, err := rc.exists()
+   if err != nil {
+   return err, nil, statusCode
}
 
return api.GenericDelete(rc)
 }
 
+// exists checks to see if a required capability is assigned to a delivery 
service
+func (rc *RequiredCapability) exists() (int, error) {
 
 Review comment:
   Is this not caught by 
https://github.com/apache/trafficcontrol/blob/master/traffic_ops/traffic_ops_golang/api/generic_crud.go#L169?
 is this just to clean up the error message? 


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4059: return 403 when unauthorized user assigns/deletes ds required capability

[GitBox]

mhoppa commented on a change in pull request #4059: return 403 when 
unauthorized user assigns/deletes ds required capability
URL: https://github.com/apache/trafficcontrol/pull/4059#discussion_r341684050
 
 

 ##
 File path: 
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go
 ##
 @@ -219,22 +219,51 @@ func (rc *RequiredCapability) getCapabilities(tenantIDs 
[]int) ([]tc.DeliverySer
 // Delete implements the api.CRUDer interface.
 func (rc *RequiredCapability) Delete() (error, error, int) {
authorized, err := rc.isTenantAuthorized()
-   if err != nil {
-   return nil, errors.New("checking tenant: " + err.Error()), 
http.StatusInternalServerError
-   } else if !authorized {
+   if !authorized {
return errors.New("not authorized on this tenant"), nil, 
http.StatusForbidden
+   } else if err != nil {
+   return nil, fmt.Errorf("checking authorization for existing DS 
ID: %s" + err.Error()), http.StatusInternalServerError
+   }
+
+   // Check if the Delivery Service is associated with the Required 
Capability
+   statusCode, err := rc.exists()
+   if err != nil {
+   return err, nil, statusCode
}
 
return api.GenericDelete(rc)
 }
 
+// exists checks to see if a required capability is assigned to a delivery 
service
+func (rc *RequiredCapability) exists() (int, error) {
+   where, orderBy, pagination, queryValues, errs := 
dbhelpers.BuildWhereAndOrderByAndPagination(rc.APIInfo().Params, 
rc.ParamColumns())
+   if len(errs) > 0 {
+   return http.StatusBadRequest, util.JoinErrs(errs)
+   }
+   fmt.Printf("\n\nwhere: %+v\norderby: %+v\npag: %+v\nqueryvalues: 
%+v\n\n", where, orderBy, pagination, queryValues)
 
 Review comment:
   I am guessing you did not mean to add this?


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4059: return 403 when unauthorized user assigns/deletes ds required capability

[GitBox]

mhoppa commented on a change in pull request #4059: return 403 when 
unauthorized user assigns/deletes ds required capability
URL: https://github.com/apache/trafficcontrol/pull/4059#discussion_r341685485
 
 

 ##
 File path: 
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go
 ##
 @@ -219,22 +219,51 @@ func (rc *RequiredCapability) getCapabilities(tenantIDs 
[]int) ([]tc.DeliverySer
 // Delete implements the api.CRUDer interface.
 func (rc *RequiredCapability) Delete() (error, error, int) {
authorized, err := rc.isTenantAuthorized()
-   if err != nil {
-   return nil, errors.New("checking tenant: " + err.Error()), 
http.StatusInternalServerError
-   } else if !authorized {
+   if !authorized {
return errors.New("not authorized on this tenant"), nil, 
http.StatusForbidden
+   } else if err != nil {
+   return nil, fmt.Errorf("checking authorization for existing DS 
ID: %s" + err.Error()), http.StatusInternalServerError
+   }
+
+   // Check if the Delivery Service is associated with the Required 
Capability
+   statusCode, err := rc.exists()
+   if err != nil {
+   return err, nil, statusCode
}
 
return api.GenericDelete(rc)
 }
 
+// exists checks to see if a required capability is assigned to a delivery 
service
+func (rc *RequiredCapability) exists() (int, error) {
+   where, orderBy, pagination, queryValues, errs := 
dbhelpers.BuildWhereAndOrderByAndPagination(rc.APIInfo().Params, 
rc.ParamColumns())
+   if len(errs) > 0 {
+   return http.StatusBadRequest, util.JoinErrs(errs)
+   }
+   fmt.Printf("\n\nwhere: %+v\norderby: %+v\npag: %+v\nqueryvalues: 
%+v\n\n", where, orderBy, pagination, queryValues)
+
+   var count int
+   query := `SELECT count(deliveryservice_id) FROM 
deliveryservices_required_capability where deliveryservice_id = $1 and 
required_capability = $2`
 
 Review comment:
   nothing to address but instead of COUNT you could simplify this to SELECT 
EXISTS and read that into a bool


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on issue #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on issue #3996: Rewrote /user/current to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#issuecomment-548887605
 
 
   Eh. I don't think that difference is actually going to break anyone, and I 
did make sure identifiers can't be null. And to be honest I'm sick to death of 
working on this.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #4066: Fix federation user JSON company key

[GitBox]

asf-ci commented on issue #4066: Fix federation user JSON company key
URL: https://github.com/apache/trafficcontrol/pull/4066#issuecomment-548886205
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4690/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa opened a new pull request #4066: Fix federation user JSON company key

[GitBox]

mhoppa opened a new pull request #4066: Fix federation user JSON company key
URL: https://github.com/apache/trafficcontrol/pull/4066
 
 
   
   ## What does this PR (Pull Request) do?
   
   
   - [x] This PR fixes #4065  
   
   
   ## Which Traffic Control components are affected by this PR?
   
   
   - Traffic Ops
   
   ## What is the best way to verify this PR?
   
   Make a request to federation user and ensure json company has the key of 
company
   
   ## If this is a bug fix, what versions of Traffic Control are affected?
   
   
   
   ## The following criteria are ALL met by this PR
   
   
   - [x] This PR includes tests OR I have explained why tests are unnecessary
   - [x] This PR includes documentation OR I have explained why documentation 
is unnecessary
   - [x] This PR includes an update to CHANGELOG.md OR such an update is not 
necessary
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR 
does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the 
Apache Software Foundation's security 
guidelines](https://www.apache.org/security/) for details)
   
   
   ## Additional Information
   
   
   
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #3996: Rewrote /user/current to Go

[GitBox]

asf-ci commented on issue #3996: Rewrote /user/current to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#issuecomment-548883642
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4689/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

ocket commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r341380162
 
 

 ##
 File path: 
traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/form.deliveryServiceSslKeys.tpl.html
 ##
 @@ -41,39 +41,53 @@
 
 
 Review comment:
   The "More" button on this page has no `click` MouseEvent handler. I can't 
click on "Generate SSL Keys" because clicking on "More" doesn't show the menu. 
It needs an `ng-click` attribute that toggles `more.isopen`. I think. Don't 
quote me on that. Truth be told all I really know is there's no event handler 
on that and when I click on it nothing happens.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

ocket commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r341407605
 
 

 ##
 File path: 
traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/form.deliveryServiceSslKeys.tpl.html
 ##
 @@ -41,39 +41,53 @@
 
 
 Review comment:
   Actually, once I was able to generate certs, the page seemed to work 
normally. Still, master has no issue displaying that page when a DS has no 
certs.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

ocket commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r341382173
 
 

 ##
 File path: 
traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/form.deliveryServiceSslKeys.tpl.html
 ##
 @@ -41,39 +41,53 @@
 
 
 Review comment:
   Also also: when I manually reveal the menu by editing the styling in a dev 
console, I can click on "Generate SSL Keys" but strangely nothing happens. Dev 
console also indicates that there's no event listener attached to that node, 
which is odd because it does have an `ng-click` attribute...
   I have to guess that this is something related to your changes to the 
controller because on master this page works fine. Probably `generateKeys` 
couldn't be found and so no event listener was registered?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3534: TP Delivery Service Generate SSL update, new letsencrypt generate and…

[GitBox]

ocket commented on a change in pull request #3534: TP Delivery Service 
Generate SSL update, new letsencrypt generate and…
URL: https://github.com/apache/trafficcontrol/pull/3534#discussion_r341385501
 
 

 ##
 File path: 
traffic_portal/app/src/common/modules/form/deliveryServiceSslKeys/form.deliveryServiceSslKeys.tpl.html
 ##
 @@ -41,39 +41,53 @@
 
 
 Review comment:
   Finally, a manual navigation to 
`#!/delivery-services/{{ID}}/ssl-keys/generate` triggers two XHR requests:
   • `/api/1.4/deliveryservices/{{ID}}`
   returns: The full DS as expected
   • `/api/1.4/deliveryservices/xmlid/{{xmlid}}/sslkeys?decode=true`
   returns:
   ```http
   HTTP/1.1 200 OK
   Content-Type: application/json
   Other-Headers: Omitted
   
   { "alerts": [{
   "text": "no object found for the specified key",
   "level": "info"
   }]}
   ```
   
   Which looks more like an error than an "info", and the page never loads but 
immediately redirects to `#!/delivery-services/{{ID}}/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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa commented on issue #4065: Delivery Services ID result return null when doing GET Request for Federations Users

[GitBox]

mhoppa commented on issue #4065: Delivery Services ID result return null when 
doing GET Request for Federations Users
URL: https://github.com/apache/trafficcontrol/issues/4065#issuecomment-548881405
 
 
   Talked to @dandypham on the rewrite the wrong json tag was used for company 
https://github.com/apache/trafficcontrol/blob/master/lib/go-tc/federation.go#L171.
   
   Needs to be fixed to company. PR coming soon
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] dandypham opened a new issue #4065: Delivery Services ID result return null when doing GET Request for Federations Users

[GitBox]

dandypham opened a new issue #4065: Delivery Services ID result return null 
when doing GET Request for Federations Users
URL: https://github.com/apache/trafficcontrol/issues/4065
 
 
   
   
   
   
   ## I'm submitting a ...
   
   
   - [x] bug report
   - [ ] new feature / enhancement request
   - [ ] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [x] Traffic Ops
   - [ ] Traffic Ops ORT
   - [ ] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   
   First create a Federation with a Delivery Service on TP. If using TO please 
do this:
   Create a federation POST Request: 
   `https://{{TO_BASE_URL}}/api/{{api_version}}/cdns/999/federations`
   With this json payload:
   ```
   {
   "cname": "test.quest.",
   "ttl": 48,
   "description": "A test federation"
   }
   ```
   Add Delivery Services to Federations. POST Request:
   
`https://{{TO_BASE_URL}}/api/{{api_version}}/federations/{{federationsID}}/deliveryservices`
   With this json payload:
   ```
   {
   "dsIds": [1],
   "replace": true
   }
   ```
   So now we have a Federations with a Delivery Service. Check again with GET 
Request:
   
`https://{{TO_BASE_URL}}/api/{{api_version}}/federations/{{fedID}}/deliveryservices`
 It will display the CDN connecting to that Delivery Service.
   Now add a Users to Federations. POST Request:
   `https://{{TO_BASE_URL}}/api/{{api_version}}/federations/{{fedID}}//users`
   And now is the error, when return the users information with GET Request:
   `https://{{TO_BASE_URL}}/api/{{api_version}}/federations/{{fedID}}//users`
   The dsIds will be null.
   ## Expected / new behavior:
   
   
   ## Minimal reproduction of the problem with instructions:
   
   
   ## Anything else:
   
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] asf-ci commented on issue #4063: Add atstccfg meta config

[GitBox]

asf-ci commented on issue #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063#issuecomment-548879954
 
 
   
   Refer to this link for build results (access rights to CI server needed): 
   https://builds.apache.org/job/trafficcontrol-PR/4688/
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

Jenkins build is back to normal : trafficcontrol-PR #4688

[Apache Jenkins Server]

See 

[GitHub] [trafficcontrol] rawlinp opened a new issue #4064: TO Tenancy/Users/Roles improvements

[GitBox]

rawlinp opened a new issue #4064: TO Tenancy/Users/Roles improvements
URL: https://github.com/apache/trafficcontrol/issues/4064
 
 
   ## I'm submitting a ...
   - [ ] bug report
   - [ ] new feature / enhancement request
   - [x] improvement request (usability, performance, tech debt, etc.)
   - [ ] other 
   
   ## Traffic Control components affected ...
   
   - [ ] CDN in a Box
   - [ ] Documentation
   - [ ] Grove
   - [ ] Traffic Control Client
   - [ ] Traffic Monitor
   - [x] Traffic Ops
   - [ ] Traffic Ops ORT
   - [ ] Traffic Portal
   - [ ] Traffic Router
   - [ ] Traffic Stats
   - [ ] Traffic Vault
   - [ ] unknown
   
   ## Current behavior:
   1. users can change their own tenant (only to a child of their existing 
tenant)
   2. users within a tenant have the ability to edit that tenant itself (e.g. 
set the tenant to inactive)
   3. users can change their own role (only to a role with equal or lower 
`priv_level`)
   4. users with a certain role have the ability to edit that role itself (e.g. 
lower the `priv_level`)
   
   ## Expected / new behavior:
   The above behaviors should be prohibited. For the most part, I cannot think 
of a valid use case for any of the above behaviors, and it seems like an 
accident waiting to happen. For example, a user could accidentally inactivate 
their own tenant, preventing the entire tenant from making changes to their 
tenantable resources. This would require someone above their tenant to 
reactivate. In general, Tenants should only be editable by users in a parent 
Tenant (or above).
   
   ## Minimal reproduction of the problem with instructions:
   The basic behaviors can be reproduced easily through Traffic Portal 
(starting role should be `admin` so that you actually have permission to edit 
roles in the first place).
   1. click username at top right > manage user profile > change tenant to a 
child tenant > update
   2. user admin > tenants > click your tenant > set to inactive > update
   3. click username at top right > manage user profile > change role to 
something lower > update
   4. user admin > roles > click your role > change description > update


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 closed issue #4038: TP, TO: server capabilities are allowed to be assigned to any servers which are not mid or edge

[GitBox]

mitchell852 closed issue #4038: TP, TO: server capabilities are allowed to be 
assigned to any servers which are not mid or edge
URL: https://github.com/apache/trafficcontrol/issues/4038
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 merged pull request #4044: Ensure server_capability can only be assigned to edges or mids

[GitBox]

mitchell852 merged pull request #4044: Ensure server_capability can only be 
assigned to edges or mids
URL: https://github.com/apache/trafficcontrol/pull/4044
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rob05c opened a new pull request #4063: Add atstccfg meta config

[GitBox]

rob05c opened a new pull request #4063: Add atstccfg meta config
URL: https://github.com/apache/trafficcontrol/pull/4063
 
 
   ## What does this PR (Pull Request) do?
   
   Adds meta config to atstccfg config generator.
   
   - [x] This PR is not related to any other Issue
   
   Includes tests.
   Includes changelog.
   No documentation, no interface change.
   
   ## Which Traffic Control components are affected by this PR?
   - Traffic Ops
   - Traffic Ops ORT
   
   ## What is the best way to verify this PR?
   
   Run unit tests.
   
   Run `atstccfg` with request to the meta config endpoint, verify response is 
identical to Go and old Perl endpoints.
   
   ## If this is a bug fix, what versions of Traffic Control are affected?
   
   Not a bug fix.
   
   ## The following criteria are ALL met by this PR
   - [x] This PR includes tests OR I have explained why tests are unnecessary
   - [x] This PR includes documentation OR I have explained why documentation 
is unnecessary
   - [x] This PR includes an update to CHANGELOG.md OR such an update is not 
necessary
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR 
does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the 
Apache Software Foundation's security 
guidelines](https://www.apache.org/security/) for details)
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 commented on issue #4062: doc for TP DSRC needs to be updated

[GitBox]

mitchell852 commented on issue #4062: doc for TP DSRC needs to be updated 
URL: https://github.com/apache/trafficcontrol/issues/4062#issuecomment-548864373
 
 
   > 
https://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_portal/usingtrafficportal.html#delivery-services
 - doesn't have a clue about DSRC
   
   yeah, i need to mention Delivery Service Required Capabilities on that page.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 commented on issue #4062: doc for TP DSRC needs to be updated

[GitBox]

mitchell852 commented on issue #4062: doc for TP DSRC needs to be updated 
URL: https://github.com/apache/trafficcontrol/issues/4062#issuecomment-548863519
 
 
   > 
https://traffic-control-cdn.readthedocs.io/en/latest/admin/quick_howto/index.html
 - has only how to assign , no details on unlink or delete
   
   Not sure UI documentation needs to describe every interaction. If it does 
need every action explained, it is a bad UI. :)


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 commented on issue #4062: doc for TP DSRC needs to be updated

[GitBox]

mitchell852 commented on issue #4062: doc for TP DSRC needs to be updated 
URL: https://github.com/apache/trafficcontrol/issues/4062#issuecomment-548862052
 
 
   DSRC for those following along at home means Delivery Service Required 
Capabilities


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

mhoppa commented on a change in pull request #3996: Rewrote /user/current to Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341654890
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,213 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
+   sysErr = fmt.Errorf("Current user (#%d) doesn't exist... ??", 
inf.User.ID)
+   errCode = http.StatusInternalServerError
 
 Review comment:
   agreed on reasoning ^


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341651553
 
 

 ##
 File path: lib/go-tc/users.go
 ##
 @@ -101,6 +110,179 @@ type UserCurrent struct {
commonUserFields
 }
 
+// CurrentUserUpdateRequest differs from a regular User/UserCurrent in that 
many of its fields are
+// *parsed* but not *unmarshaled*. This allows a handler to distinguish 
between "null" and
+// "undefined" values.
+type CurrentUserUpdateRequest struct {
+   // User, for whatever reason, contains all of the actual data.
+   User CurrentUserUpdateRequestUser `json:"user"`
+}
+
+// CurrentUserUpdateRequestUser holds all of the actual data in a request to 
update the current user.
+type CurrentUserUpdateRequestUser struct {
+   AddressLine1   json.RawMessage `json:"addressLine1"`
+   AddressLine2   json.RawMessage `json:"addressLine2"`
+   City   json.RawMessage `json:"city"`
+   Companyjson.RawMessage `json:"company"`
+   ConfirmLocalPasswd *string `json:"confirmLocalPasswd"`
+   Countryjson.RawMessage `json:"country"`
+   Email  json.RawMessage `json:"email"`
+   FullName   json.RawMessage `json:"fullName"`
+   GIDjson.RawMessage `json:"gid"`
+   ID json.RawMessage `json:"id"`
+   LocalPasswd*string `json:"localPasswd"`
+   PhoneNumberjson.RawMessage `json:"phoneNumber"`
+   PostalCode json.RawMessage `json:"postalCode"`
+   PublicSSHKey   json.RawMessage `json:"publicSshKey"`
+   Role   json.RawMessage `json:"role"`
+   StateOrProvincejson.RawMessage `json:"stateOrProvince"`
+   TenantID   json.RawMessage `json:"tenantId"`
+   UIDjson.RawMessage `json:"uid"`
+   Username   json.RawMessage `json:"username"`
+}
+
+// ValidateAndUnmarshal validates the request and returns a User into which 
the request's information
+// has been unmarshalled. This allows many fields to be "null", but explicitly 
checks that they are
+// present in the JSON payload.
+func (u *CurrentUserUpdateRequestUser) ValidateAndUnmarshal(user *User) error {
+   errs := []error{}
+   if u.AddressLine1 != nil {
+   if err := json.Unmarshal(u.AddressLine1, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine1: %v", err))
+   }
+   }
+
+   if u.AddressLine2 != nil {
+   if err := json.Unmarshal(u.AddressLine2, ); 
err != nil {
+   errs = append(errs, fmt.Errorf("addressLine2: %v", err))
+   }
+   }
+
+   if u.City != nil {
+   if err := json.Unmarshal(u.City, ); err != nil {
+   errs = append(errs, fmt.Errorf("city: %v", err))
+   }
+   }
+
+   if u.Company != nil {
+   if err := json.Unmarshal(u.Company, ); err != nil {
+   errs = append(errs, fmt.Errorf("company: %v", err))
+   }
+   }
+
+   if u.LocalPasswd != nil && *u.LocalPasswd != "" {
+   if u.ConfirmLocalPasswd == nil || *u.ConfirmLocalPasswd == "" {
+   errs = append(errs, errors.New("confirmLocalPasswd: 
required when changing password"))
+   } else if *u.LocalPasswd != *u.ConfirmLocalPasswd {
+   errs = append(errs, errors.New("localPasswd and 
confirmLocalPasswd do not match"))
+   }
+   } else if u.ConfirmLocalPasswd != nil && *u.ConfirmLocalPasswd != "" {
+   errs = append(errs, errors.New("localPasswd: required when 
changing password"))
+   }
+   user.ConfirmLocalPassword = u.ConfirmLocalPasswd
+   user.LocalPassword = u.LocalPasswd
 
 Review comment:
   nah, they can be `nil`. It's just that if one isn't then the other can't be, 
and when they both aren't they have to match.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #3996: Rewrote /user/current to Go

[GitBox]

ocket commented on a change in pull request #3996: Rewrote /user/current to 
Go
URL: https://github.com/apache/trafficcontrol/pull/3996#discussion_r341650245
 
 

 ##
 File path: traffic_ops/traffic_ops_golang/user/current.go
 ##
 @@ -80,3 +190,213 @@ WHERE u.id=$1
u.LocalUser = util.BoolPtr(localPassword.Valid)
return u, nil
 }
+
+func ReplaceCurrent(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)
+   return
+   }
+   defer inf.Close()
+
+   var userRequest tc.CurrentUserUpdateRequest
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   errCode = http.StatusBadRequest
+   userErr = fmt.Errorf("Couldn't parse request: %v", err)
+   api.HandleErr(w, r, tx, errCode, userErr, nil)
+   return
+   }
+
+   user, exists, err := dbhelpers.GetUserByID(inf.User.ID, tx)
+   if err != nil {
+   sysErr = fmt.Errorf("Getting user by ID %d: %v", inf.User.ID, 
err)
+   errCode = http.StatusInternalServerError
+   api.HandleErr(w, r, tx, errCode, nil, sysErr)
+   return
+   } else if !exists {
+   sysErr = fmt.Errorf("Current user (#%d) doesn't exist... ??", 
inf.User.ID)
+   errCode = http.StatusInternalServerError
 
 Review comment:
   if you get that the currently logged in user doesn't exist, then that can 
only mean the user was deleted in between when the request handling began and 
when it got to this point. As users cannot be deleted through the API that 
indicates a corrupt or compromised database, so I figured an internal server 
error was appropriate.


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:
us...@infra.apache.org


With regards,
Apache Git Services