rob05c commented on a change in pull request #5122:
URL: https://github.com/apache/trafficcontrol/pull/5122#discussion_r508886356



##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/config/config.go
##########
@@ -0,0 +1,153 @@
+package config
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "crypto/tls"
+       "errors"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       "github.com/pborman/getopt/v2"
+       "io"
+       "net/http"
+       "os"
+       "strings"
+)
+
+type Creds struct {
+       User     string `json:"u"`
+       Password string `json:"p"`
+}
+
+type Cfg struct {
+       LogLocationErr   string
+       LogLocationInfo  string
+       LogLocationWarn  string
+       LogLocationDebug string
+       TOInsecure       bool
+       TOUser           string
+       TOPass           string
+       TOUrl            string
+       Transport        *http.Transport
+}
+
+type ToResponse struct {
+       Response string `json:"response"`
+}
+
+func Dclose(c io.Closer) {
+       if err := c.Close(); err != nil {
+       }
+}
+
+func ErrCheck(err error) {
+       if err != nil {
+               log.Errorln(err)
+               os.Exit(1)
+       }
+}
+
+func (cfg Cfg) ErrorLog() log.LogLocation   { return 
log.LogLocation(cfg.LogLocationErr) }
+func (cfg Cfg) WarningLog() log.LogLocation { return 
log.LogLocation(cfg.LogLocationWarn) }
+func (cfg Cfg) InfoLog() log.LogLocation    { return 
log.LogLocation(cfg.LogLocationInfo) }
+func (cfg Cfg) DebugLog() log.LogLocation   { return 
log.LogLocation(cfg.LogLocationDebug) }
+func (cfg Cfg) EventLog() log.LogLocation   { return 
log.LogLocation(log.LogLocationNull) } // event logging not used.
+
+func GetCfg() (Cfg, error) {
+       var err error
+       logLocationDebugPtr := getopt.StringLong("log-location-debug", 'd', "", 
"Where to log debugs. May be a file path, stdout, stderr, or null, default ''")
+       logLocationErrorPtr := getopt.StringLong("log-location-error", 'e', 
"stderr", "Where to log errors. May be a file path, stdout, stderr, or null, 
default stderr")
+       logLocationInfoPtr := getopt.StringLong("log-location-info", 'i', 
"stderr", "Where to log info. May be a file path, stdout, stderr, or null, 
default stderr")
+       logLocationWarnPtr := getopt.StringLong("log-location-warning", 'w', 
"stderr", "Where to log warnings. May be a file path, stdout, stderr, or null, 
default stderr")
+       toInsecurePtr := getopt.BoolLong("traffic-ops-insecure", 'I', "[true | 
false] ignore certificate errors from Traffic Ops")
+       toUserPtr := getopt.StringLong("traffic-ops-user", 'u', "", "Traffic 
Ops username. Required.")
+       toPassPtr := getopt.StringLong("traffic-ops-passowrd", 'p', "", 
"Traffic Ops Password. Required")
+       toUrlPtr := getopt.StringLong("traffic-ops-url", 'U', "", "Traffic ops 
base URL. Required.")
+       helpPtr := getopt.BoolLong("help", 'h', "Print usage information and 
exit")
+       getopt.ParseV2()
+
+       logLocationDebug := *logLocationDebugPtr
+       logLocationError := *logLocationErrorPtr
+       logLocationInfo := *logLocationInfoPtr
+       logLocationWarn := *logLocationWarnPtr
+       toInsecure := *toInsecurePtr
+       toURL := *toUrlPtr
+       toUser := *toUserPtr
+       toPass := *toPassPtr
+       transport := &http.Transport{TLSClientConfig: 
&tls.Config{InsecureSkipVerify: toInsecure}}
+       help := *helpPtr
+
+       if help {
+               Usage()
+               return Cfg{}, nil

Review comment:
       If I run `DnssecRefresh -h`, it prints the help, but then segfaults.
   
   Looks like this somehow needs to either convey to the caller that Help was 
called, and it should immediately return, or just `os.Exit(0)` right here, 
either would be fine.

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)

Review comment:
       If you call `./DnssecRefresh` with no argument, it seems like this 
should print the no-argument messages below, but it doesn't, because `ErrCheck` 
writes to the log, but that doesn't exist.
   
   So `./DnssecRefresh` just returns an error code, with no message.
   
   Something (either here, or in `config`) should probably write to `stdout` 
that no Error log location exists.
   
   Since it's possible to tell the logger to not log anything with the string 
`null`, you could just default to stdout with 
   ```
   λ (cfg Cfg) ErrorLog() log.LogLocation   { 
     if stringUtils.TrimSpace(cfg.LogLocationErr) == "" {
       return log.LogLocation(log.LogLocationStdout) 
     }
     return log.LogLocation(cfg.LogLocationErr) 
   }
   ```

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/config/config.go
##########
@@ -0,0 +1,153 @@
+package config
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "crypto/tls"
+       "errors"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       "github.com/pborman/getopt/v2"
+       "io"
+       "net/http"
+       "os"
+       "strings"
+)
+
+type Creds struct {
+       User     string `json:"u"`
+       Password string `json:"p"`
+}
+
+type Cfg struct {
+       LogLocationErr   string
+       LogLocationInfo  string
+       LogLocationWarn  string
+       LogLocationDebug string
+       TOInsecure       bool
+       TOUser           string
+       TOPass           string
+       TOUrl            string
+       Transport        *http.Transport
+}
+
+type ToResponse struct {
+       Response string `json:"response"`
+}
+
+func Dclose(c io.Closer) {
+       if err := c.Close(); err != nil {

Review comment:
       Should this log the error? That'd be a reason to keep it (vs as above, 
`defer body.Close()`)

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/config/config.go
##########
@@ -0,0 +1,153 @@
+package config
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "crypto/tls"
+       "errors"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       "github.com/pborman/getopt/v2"
+       "io"
+       "net/http"
+       "os"
+       "strings"
+)
+
+type Creds struct {
+       User     string `json:"u"`
+       Password string `json:"p"`
+}
+
+type Cfg struct {
+       LogLocationErr   string
+       LogLocationInfo  string
+       LogLocationWarn  string
+       LogLocationDebug string
+       TOInsecure       bool
+       TOUser           string
+       TOPass           string
+       TOUrl            string
+       Transport        *http.Transport
+}
+
+type ToResponse struct {
+       Response string `json:"response"`
+}
+
+func Dclose(c io.Closer) {
+       if err := c.Close(); err != nil {
+       }
+}
+
+func ErrCheck(err error) {
+       if err != nil {
+               log.Errorln(err)
+               os.Exit(1)
+       }
+}
+
+func (cfg Cfg) ErrorLog() log.LogLocation   { return 
log.LogLocation(cfg.LogLocationErr) }
+func (cfg Cfg) WarningLog() log.LogLocation { return 
log.LogLocation(cfg.LogLocationWarn) }
+func (cfg Cfg) InfoLog() log.LogLocation    { return 
log.LogLocation(cfg.LogLocationInfo) }
+func (cfg Cfg) DebugLog() log.LogLocation   { return 
log.LogLocation(cfg.LogLocationDebug) }
+func (cfg Cfg) EventLog() log.LogLocation   { return 
log.LogLocation(log.LogLocationNull) } // event logging not used.
+
+func GetCfg() (Cfg, error) {
+       var err error
+       logLocationDebugPtr := getopt.StringLong("log-location-debug", 'd', "", 
"Where to log debugs. May be a file path, stdout, stderr, or null, default ''")
+       logLocationErrorPtr := getopt.StringLong("log-location-error", 'e', 
"stderr", "Where to log errors. May be a file path, stdout, stderr, or null, 
default stderr")
+       logLocationInfoPtr := getopt.StringLong("log-location-info", 'i', 
"stderr", "Where to log info. May be a file path, stdout, stderr, or null, 
default stderr")
+       logLocationWarnPtr := getopt.StringLong("log-location-warning", 'w', 
"stderr", "Where to log warnings. May be a file path, stdout, stderr, or null, 
default stderr")
+       toInsecurePtr := getopt.BoolLong("traffic-ops-insecure", 'I', "[true | 
false] ignore certificate errors from Traffic Ops")
+       toUserPtr := getopt.StringLong("traffic-ops-user", 'u', "", "Traffic 
Ops username. Required.")
+       toPassPtr := getopt.StringLong("traffic-ops-passowrd", 'p', "", 
"Traffic Ops Password. Required")

Review comment:
       Typo, `traffic-ops-password`

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)
+       jar, _ := cookiejar.New(nil)
+       client := &http.Client{Jar: jar, Transport: cfg.Transport}
+
+       log.Debugf("Posting to: %s", loginUrl)
+
+       res, err := client.Do(req)
+       config.ErrCheck(err)
+       defer config.Dclose(res.Body)
+       refreshUrl := fmt.Sprintf("%s%s", cfg.TOUrl, 
"/api/2.0/cdns/dnsseckeys/refresh")
+       resp, _ := http.NewRequest("GET", refreshUrl, buf)
+
+       log.Debugf("Get req to: %s", refreshUrl)
+
+       refresh, _ := client.Do(resp)
+       respData, _ := ioutil.ReadAll(refresh.Body)
+       defer config.Dclose(refresh.Body)
+       if refresh.StatusCode != 200 {
+               log.Debugln(string(respData))

Review comment:
       I think this should be a `log.Errorln`. This happened to me from a bad 
password, and I didn't have debug logging on, so I just got no output with an 
error code.

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)

Review comment:
       Nitpick: `"POST"` -> `http.MethodPost`

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)
+       jar, _ := cookiejar.New(nil)
+       client := &http.Client{Jar: jar, Transport: cfg.Transport}
+
+       log.Debugf("Posting to: %s", loginUrl)
+
+       res, err := client.Do(req)
+       config.ErrCheck(err)
+       defer config.Dclose(res.Body)
+       refreshUrl := fmt.Sprintf("%s%s", cfg.TOUrl, 
"/api/2.0/cdns/dnsseckeys/refresh")
+       resp, _ := http.NewRequest("GET", refreshUrl, buf)
+
+       log.Debugf("Get req to: %s", refreshUrl)
+
+       refresh, _ := client.Do(resp)
+       respData, _ := ioutil.ReadAll(refresh.Body)
+       defer config.Dclose(refresh.Body)

Review comment:
       Not sure what the purpose of `Dclose` is? This could just be `defer 
refresh.Body.Close()`

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)
+       jar, _ := cookiejar.New(nil)
+       client := &http.Client{Jar: jar, Transport: cfg.Transport}
+
+       log.Debugf("Posting to: %s", loginUrl)
+
+       res, err := client.Do(req)
+       config.ErrCheck(err)
+       defer config.Dclose(res.Body)
+       refreshUrl := fmt.Sprintf("%s%s", cfg.TOUrl, 
"/api/2.0/cdns/dnsseckeys/refresh")
+       resp, _ := http.NewRequest("GET", refreshUrl, buf)
+
+       log.Debugf("Get req to: %s", refreshUrl)
+
+       refresh, _ := client.Do(resp)

Review comment:
       This needs to check and handle the error

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"

Review comment:
       Nitpick: we usually like to separate standard library packages, internal 
ATC packages, and external packages by blank lines, just to make it easier to 
read

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)
+       jar, _ := cookiejar.New(nil)
+       client := &http.Client{Jar: jar, Transport: cfg.Transport}
+
+       log.Debugf("Posting to: %s", loginUrl)
+
+       res, err := client.Do(req)
+       config.ErrCheck(err)
+       defer config.Dclose(res.Body)
+       refreshUrl := fmt.Sprintf("%s%s", cfg.TOUrl, 
"/api/2.0/cdns/dnsseckeys/refresh")
+       resp, _ := http.NewRequest("GET", refreshUrl, buf)
+
+       log.Debugf("Get req to: %s", refreshUrl)
+
+       refresh, _ := client.Do(resp)
+       respData, _ := ioutil.ReadAll(refresh.Body)

Review comment:
       Needs to check and handle the error

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)
+       jar, _ := cookiejar.New(nil)

Review comment:
       Needs to check & handle the error

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)

Review comment:
       Nitpick: it's more idiomatic avoid `new` in Go unless there's no other 
way. 
   This can do the same thing with `buf := &bytes.Buffer{}`

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)

Review comment:
       Needs to check & handle the error

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")

Review comment:
       Nitpick: might be easier to read as string concatenation, 
   ```
   loginUrl := cfg.TOUrl + `/api/2.0/user/login`
   ```

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/config/config.go
##########
@@ -0,0 +1,153 @@
+package config
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "crypto/tls"
+       "errors"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       "github.com/pborman/getopt/v2"
+       "io"
+       "net/http"
+       "os"
+       "strings"
+)
+
+type Creds struct {
+       User     string `json:"u"`
+       Password string `json:"p"`
+}
+
+type Cfg struct {
+       LogLocationErr   string
+       LogLocationInfo  string
+       LogLocationWarn  string
+       LogLocationDebug string
+       TOInsecure       bool
+       TOUser           string
+       TOPass           string
+       TOUrl            string
+       Transport        *http.Transport
+}
+
+type ToResponse struct {
+       Response string `json:"response"`
+}
+
+func Dclose(c io.Closer) {
+       if err := c.Close(); err != nil {
+       }
+}
+
+func ErrCheck(err error) {
+       if err != nil {
+               log.Errorln(err)
+               os.Exit(1)
+       }
+}
+
+func (cfg Cfg) ErrorLog() log.LogLocation   { return 
log.LogLocation(cfg.LogLocationErr) }
+func (cfg Cfg) WarningLog() log.LogLocation { return 
log.LogLocation(cfg.LogLocationWarn) }
+func (cfg Cfg) InfoLog() log.LogLocation    { return 
log.LogLocation(cfg.LogLocationInfo) }
+func (cfg Cfg) DebugLog() log.LogLocation   { return 
log.LogLocation(cfg.LogLocationDebug) }
+func (cfg Cfg) EventLog() log.LogLocation   { return 
log.LogLocation(log.LogLocationNull) } // event logging not used.
+
+func GetCfg() (Cfg, error) {
+       var err error
+       logLocationDebugPtr := getopt.StringLong("log-location-debug", 'd', "", 
"Where to log debugs. May be a file path, stdout, stderr, or null, default ''")
+       logLocationErrorPtr := getopt.StringLong("log-location-error", 'e', 
"stderr", "Where to log errors. May be a file path, stdout, stderr, or null, 
default stderr")
+       logLocationInfoPtr := getopt.StringLong("log-location-info", 'i', 
"stderr", "Where to log info. May be a file path, stdout, stderr, or null, 
default stderr")
+       logLocationWarnPtr := getopt.StringLong("log-location-warning", 'w', 
"stderr", "Where to log warnings. May be a file path, stdout, stderr, or null, 
default stderr")
+       toInsecurePtr := getopt.BoolLong("traffic-ops-insecure", 'I', "[true | 
false] ignore certificate errors from Traffic Ops")
+       toUserPtr := getopt.StringLong("traffic-ops-user", 'u', "", "Traffic 
Ops username. Required.")
+       toPassPtr := getopt.StringLong("traffic-ops-passowrd", 'p', "", 
"Traffic Ops Password. Required")
+       toUrlPtr := getopt.StringLong("traffic-ops-url", 'U', "", "Traffic ops 
base URL. Required.")
+       helpPtr := getopt.BoolLong("help", 'h', "Print usage information and 
exit")
+       getopt.ParseV2()
+
+       logLocationDebug := *logLocationDebugPtr
+       logLocationError := *logLocationErrorPtr
+       logLocationInfo := *logLocationInfoPtr
+       logLocationWarn := *logLocationWarnPtr
+       toInsecure := *toInsecurePtr
+       toURL := *toUrlPtr
+       toUser := *toUserPtr
+       toPass := *toPassPtr
+       transport := &http.Transport{TLSClientConfig: 
&tls.Config{InsecureSkipVerify: toInsecure}}
+       help := *helpPtr
+
+       if help {
+               Usage()
+               return Cfg{}, nil
+       }
+
+       missingArgStr := "Missing required argument"
+       usageStr := "\nBasic usage: ToDnssecRefresh --traffic-ops-url=myurl 
--traffic-ops-user=myuser --traffic-ops-password=mypass"
+       if strings.TrimSpace(toURL) == "" {
+               return Cfg{}, errors.New(missingArgStr + " --traffic-ops-url" + 
usageStr)
+       }
+       if strings.TrimSpace(toUser) == "" {
+               return Cfg{}, errors.New(missingArgStr + " --traffic-ops-user" 
+ usageStr)
+       }
+       if strings.TrimSpace(toPass) == "" {
+               return Cfg{}, errors.New(missingArgStr + " 
--traffic-ops-password" + usageStr)
+       }
+
+       cfg := Cfg{
+               LogLocationDebug: logLocationDebug,
+               LogLocationErr:   logLocationError,
+               LogLocationInfo:  logLocationInfo,
+               LogLocationWarn:  logLocationWarn,
+               TOInsecure:       toInsecure,
+               Transport:        transport,
+               TOUrl:            toURL,
+               TOUser:           toUser,
+               TOPass:           toPass,
+       }
+
+       if err = log.InitCfg(cfg); err != nil {
+               return Cfg{}, errors.New("Initializing loggers: " + err.Error() 
+ "\n")
+       }
+
+       return cfg, nil
+}
+
+func PrintConfig(cfg Cfg) {
+       log.Debugf("TOUrl: %s\n", cfg.TOUrl)
+       log.Debugf("TOUser: %s\n", cfg.TOUser)
+       log.Debugf("TOPass: Pass len: %d\n", len(cfg.TOPass))
+       log.Debugf("TOInsecure: %t\n", cfg.TOInsecure)
+       log.Debugf("LogLocationDebug: %s\n", cfg.LogLocationDebug)
+       log.Debugf("LogLocationErr: %s\n", cfg.LogLocationErr)
+       log.Debugf("LogLocationInfo: %s\n", cfg.LogLocationInfo)
+       log.Debugf("LogLocationWarn: %s\n", cfg.LogLocationWarn)
+}
+
+func Usage() {
+       fmt.Println("\t  --log-location-debug=[value] | -d [value], Where to 
log debugs. May be a file path, stdout, stderr, or null, default stderr")

Review comment:
       Nitpick: this might be easier to read as a raw string literal:
   ```go
   usageStr := `        --log-location-debug=[value] | -d [value], Where to log 
debugs. May be a file path, stdout, stderr, or null, default stderr
        --log-location-error=[value] | -e [value], Where to log errors. May be 
a file path, stdout, stderr, or null, default stderr
        --log-location-info=[value] | -i [value], Where to log info. May be a 
file path, stdout, stderr, or null, default stderr
        --log-location-warning=[value] | -w [value], Where to log warnings. May 
be a file path, stdout, stderr, or null, default stderr
        --traffic-ops-url=[url] | -u [url], Traffic Ops URL. Must be the full 
URL, including the scheme. Required."
        --traffic-ops-user=[username] | -U [username], Traffic Ops username. 
Required."
        --traffic-ops-password=[password] | -P [password], Traffic Ops 
password. Required.
        --help | -h, Print usage information and exit`
        fmt.Println(usageStr)
   ```

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)
+       jar, _ := cookiejar.New(nil)
+       client := &http.Client{Jar: jar, Transport: cfg.Transport}
+
+       log.Debugf("Posting to: %s", loginUrl)
+
+       res, err := client.Do(req)
+       config.ErrCheck(err)
+       defer config.Dclose(res.Body)
+       refreshUrl := fmt.Sprintf("%s%s", cfg.TOUrl, 
"/api/2.0/cdns/dnsseckeys/refresh")
+       resp, _ := http.NewRequest("GET", refreshUrl, buf)

Review comment:
       Needs to check & handle the error

##########
File path: traffic_ops/app/bin/checks/DnssecRefresh/ToDnssecRefresh.go
##########
@@ -0,0 +1,71 @@
+package main
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "bytes"
+       "encoding/json"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-log"
+       
"github.com/apache/trafficcontrol/traffic_ops/app/bin/checks/DnssecRefresh/config"
+       "io/ioutil"
+       "net/http"
+       "net/http/cookiejar"
+       "os"
+)
+
+func main() {
+       cfg, err := config.GetCfg()
+       config.ErrCheck(err)
+       log.Debugln("Including DEBUG messages in output. Config is:")
+       config.PrintConfig(cfg) // only if DEBUG logging is set.
+       body := &config.Creds{
+               User:     cfg.TOUser,
+               Password: cfg.TOPass,
+       }
+       loginUrl := fmt.Sprintf("%s%s", cfg.TOUrl, "/api/2.0/user/login")
+       buf := new(bytes.Buffer)
+       err = json.NewEncoder(buf).Encode(body)
+       config.ErrCheck(err)
+       req, _ := http.NewRequest("POST", loginUrl, buf)
+       jar, _ := cookiejar.New(nil)
+       client := &http.Client{Jar: jar, Transport: cfg.Transport}
+
+       log.Debugf("Posting to: %s", loginUrl)
+
+       res, err := client.Do(req)
+       config.ErrCheck(err)
+       defer config.Dclose(res.Body)
+       refreshUrl := fmt.Sprintf("%s%s", cfg.TOUrl, 
"/api/2.0/cdns/dnsseckeys/refresh")
+       resp, _ := http.NewRequest("GET", refreshUrl, buf)
+
+       log.Debugf("Get req to: %s", refreshUrl)
+
+       refresh, _ := client.Do(resp)
+       respData, _ := ioutil.ReadAll(refresh.Body)
+       defer config.Dclose(refresh.Body)
+       if refresh.StatusCode != 200 {
+               log.Debugln(string(respData))
+               os.Exit(1)
+       }
+       var response config.ToResponse

Review comment:
       Nitpick: Personally, I prefer to always use `:=` to declare variables, 
just so all variables have the same syntax, and also so it's obvious what the 
value is, `response := config.ToResponse{}`.
   But that's not a universal idiom, so feel free to ignore if you don't agree.




----------------------------------------------------------------
This is an automated message from the 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:
[email protected]


Reply via email to