From 5ca458c4bd80c03d56daffeb8b4a36d9f6d671bb Mon Sep 17 00:00:00 2001 From: Tomasz Jonak Date: Wed, 22 Nov 2023 15:15:38 +0100 Subject: [PATCH] [lint] Add golangci-lint automation + fix existing issues --- .github/workflows/golang-linter.yaml | 29 ++++++++ .golangci.yml | 15 +++++ global/global.go | 7 -- main.go | 18 ++--- scanner/config.go | 2 +- scanner/executor.go | 47 ++----------- scanner/result_parser.go | 68 ------------------- scanner/scanner.go | 99 ++++++++++++++-------------- tools/sidekick/sidekick.go | 2 +- util/type.go | 18 ++--- 10 files changed, 120 insertions(+), 185 deletions(-) create mode 100644 .github/workflows/golang-linter.yaml create mode 100644 .golangci.yml delete mode 100644 global/global.go delete mode 100644 scanner/result_parser.go diff --git a/.github/workflows/golang-linter.yaml b/.github/workflows/golang-linter.yaml new file mode 100644 index 0000000..dff4bdf --- /dev/null +++ b/.github/workflows/golang-linter.yaml @@ -0,0 +1,29 @@ +name: golangci-lint +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + pull-requests: read + +jobs: + lint: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v3 + with: + submodules: recursive + - uses: actions/setup-go@v4 + with: + go-version: '1.21' + cache: false + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + version: v1.55 + only-new-issues: true + env: + CGO_ENABLED: 0 diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..b2b3265 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,15 @@ +linters: + enable: + - stylecheck + - gocritic + # - dupl + - durationcheck + # - goconst + - gofmt + - goimports + # - misspell + # - nestif + +run: + skip-dirs: + - share diff --git a/global/global.go b/global/global.go deleted file mode 100644 index a4e36ca..0000000 --- a/global/global.go +++ /dev/null @@ -1,7 +0,0 @@ -package global - -import ( - "github.com/deepfence/compliance/share/system" -) - -var SYS *system.SystemTools diff --git a/main.go b/main.go index 505e76a..9b05ad2 100644 --- a/main.go +++ b/main.go @@ -9,25 +9,25 @@ import ( ) func main() { - benchId := flag.String("bench-id", "", "The id of set of scripts to be run for compliance check") + benchID := flag.String("bench-id", "", "The id of set of scripts to be run for compliance check") flag.String("NODE_TYPE", "", "Kubernetes node role master/worker") flag.Parse() config, err := scanner.LoadConfig() if err != nil { return } - if *benchId == "" { - log.Error("Bench Id is required. Exiting.") + if *benchID == "" { + log.Error("bench-id is required. Exiting.") return } - script, found := config[*benchId] + script, found := config[*benchID] if !found { - log.Error("BenchId not found. Exiting. ") + log.Error("bench-id not found. Exiting. ") return } - b := scanner.Bench{ - Script: script, - } + b := scanner.Bench{Script: script} - b.RunScripts(context.Background()) + if _, err := b.RunScripts(context.Background()); err != nil { + log.Errorf("RunScripts: %v", err) + } } diff --git a/scanner/config.go b/scanner/config.go index bd0a5ce..5555c57 100644 --- a/scanner/config.go +++ b/scanner/config.go @@ -27,7 +27,7 @@ var configFile = getDfInstallDir() + "/usr/local/bin/compliance_check/config.jso func LoadConfig() (map[string]Script, error) { configFile, err := os.Open(configFile) - defer configFile.Close() + defer func() { _ = configFile.Close() }() if err != nil { log.Error("error in reading config json file:" + err.Error()) return nil, err diff --git a/scanner/executor.go b/scanner/executor.go index e1a99dd..49cb2a4 100644 --- a/scanner/executor.go +++ b/scanner/executor.go @@ -6,25 +6,22 @@ import ( "context" "encoding/json" "fmt" - "io/ioutil" "os" "os/exec" "strings" "syscall" - "text/template" log "github.com/sirupsen/logrus" ) type Bench struct { - Script Script - daemonOpts []string - childCmd *exec.Cmd + Script Script + childCmd *exec.Cmd } type DockerReplaceOpts struct { - Replace_docker_daemon_opts string - Replace_container_list string + ReplaceDockerDaemonOpts string + ReplaceContainerList string } type benchItem struct { @@ -65,7 +62,7 @@ func (b *Bench) RunScripts(ctx context.Context) ([]benchItem, error) { } return nil, err } - // global.SYS.AddToolProcess(pgid, 1, "host-bench", destPath) + err = cmd.Wait() out := outb.Bytes() @@ -91,40 +88,6 @@ func (b *Bench) RunScripts(ctx context.Context) ([]benchItem, error) { return nil, nil } -// replace the docker daemon config line, so that can run the script without pid=host -func (b *Bench) replaceTemplateVars(srcPath, dstPath string, containers []string) error { - dat, err := ioutil.ReadFile(srcPath) - if err != nil { - return err - } - f, err := os.Create(dstPath) - if err != nil { - return err - } - defer f.Close() - - //containers only apply to container.sh, no effect to host.sh, because no <<>> in it - var containerLines string - if len(containers) > 0 { - containerLines = "containers=\"\n" + strings.Join(containers, "\n") + "\"\n" - } else { - containerLines = "containers=\"\"\n" - } - r := DockerReplaceOpts{ - Replace_docker_daemon_opts: strings.Join(b.daemonOpts, " "), - Replace_container_list: containerLines, - } - t := template.New("bench") - t.Delims("<<<", ">>>") - t.Parse(string(dat)) - - if err = t.Execute(f, r); err != nil { - log.WithFields(log.Fields{"error": err}).Error("Executing template error") - return err - } - return nil -} - func (b *Bench) getBenchMsg(out []byte) []benchItem { list := make([]benchItem, 0) scanner := bufio.NewScanner(strings.NewReader(string(out))) diff --git a/scanner/result_parser.go b/scanner/result_parser.go deleted file mode 100644 index 9cc25bf..0000000 --- a/scanner/result_parser.go +++ /dev/null @@ -1,68 +0,0 @@ -package scanner - -import ( - "github.com/deepfence/compliance/share" - "strings" -) - -func (b *Bench) parseBenchMsg(line string) (*benchItem, bool) { - var level, id, msg, profile string - var scored, automated bool - - if strings.Contains(line, "[INFO]") { - level = share.BenchLevelInfo - } else if strings.Contains(line, "[PASS]") { - level = share.BenchLevelPass - } else if strings.Contains(line, "[WARN]") { - level = share.BenchLevelWarn - } else if strings.Contains(line, "[NOTE]") { - level = share.BenchLevelNote - } else { - return nil, false - } - - a := strings.Index(line, "0m ") - if a == -1 { - return nil, false - } - c := strings.Index(line, " - ") - if c != -1 { - // Item headline - id = strings.TrimSpace(line[a+3 : c]) - - // Ignore the section title - if strings.Index(id, ".") == -1 { - return nil, false - } - - if x := strings.Index(line, "[Scored]"); x != -1 { - scored = true - } - if x := strings.Index(line, "[Automated]"); x != -1 { - automated = true - } - if x := strings.Index(line, "[Level 1]"); x != -1 { - profile = share.BenchProfileL1 - } else if x = strings.Index(line, "[Level 2]"); x != -1 { - profile = share.BenchProfileL2 - } - } else { - // Item's following line - c = strings.Index(line, " * ") - if c == -1 { - return nil, false - } - } - - msg = line[c+3:] - msg = strings.ReplaceAll(msg, "(Scored)", "") - msg = strings.ReplaceAll(msg, "(Not Scored)", "") - msg = strings.ReplaceAll(msg, "(Automated)", "") - msg = strings.ReplaceAll(msg, "(Manual)", "") - msg = strings.TrimSpace(msg) - - return &benchItem{ - Level: level, TestNum: id, Header: msg, - Scored: scored, Automated: automated, Profile: profile, - }, true -} diff --git a/scanner/scanner.go b/scanner/scanner.go index 2697074..a3282fd 100644 --- a/scanner/scanner.go +++ b/scanner/scanner.go @@ -19,7 +19,13 @@ type ComplianceScanner struct { config util.Config } -var scanMap sync.Map +var ( + scanMap sync.Map + + ErrScanNotFound = errors.New("failed to stop scan, may have already completed") + ErrScanCancelTypecast = errors.New("failed to stop scan, cancel function is not an instance of context.CancelFunc") + ErrComplianceCheckType = errors.New("compliance check type not found") +) func init() { lvl, ok := os.LookupEnv("LOG_LEVEL") @@ -50,30 +56,29 @@ func NewComplianceScanner(config util.Config) (*ComplianceScanner, error) { for _, complianceCheckType := range config.ComplianceCheckTypes { _, exists := scriptConfig[complianceCheckType] if !exists { - return nil, errors.New(fmt.Sprintf("invalid scan_type %s", complianceCheckType)) + return nil, fmt.Errorf("invalid scan_type %s", complianceCheckType) } } - if config.ScanId == "" { + if config.ScanID == "" { return nil, errors.New("scan_id is empty") } return &ComplianceScanner{config: config}, nil } -func StopScan(scanId string) bool { - cancelFnObj, found := scanMap.Load(scanId) - logMsg := "" - successFlag := true +func StopScan(scanID string) error { + cancelFnObj, found := scanMap.Load(scanID) if !found { - logMsg = "Failed to Stop scan, may have already completed" - successFlag = false - } else { - cancelFn := cancelFnObj.(context.CancelFunc) - cancelFn() - logMsg = "Stop Compliance scan request submitted" + return ErrScanNotFound + } + + cancelFn, ok := cancelFnObj.(context.CancelFunc) + if !ok { + return ErrScanCancelTypecast } - logrus.Infof("%s, scanid:%s", logMsg, scanId) - return successFlag + cancelFn() + + return nil } func (c *ComplianceScanner) RunComplianceScan() error { @@ -81,7 +86,7 @@ func (c *ComplianceScanner) RunComplianceScan() error { if err != nil { return err } - tempFileName := fmt.Sprintf("/tmp/tmp-%s.json", c.config.ScanId) + tempFileName := fmt.Sprintf("/tmp/tmp-%s.json", c.config.ScanID) defer os.Remove(tempFileName) scriptConfig, err := LoadConfig() if err != nil { @@ -90,18 +95,18 @@ func (c *ComplianceScanner) RunComplianceScan() error { var complianceScanResults []util.ComplianceDoc ctx, cancel := context.WithCancel(context.Background()) - logrus.Infof("Adding to scanMap, scanid:%s", c.config.ScanId) - scanMap.Store(c.config.ScanId, cancel) + logrus.Infof("Adding to scanMap, scanid:%s", c.config.ScanID) + scanMap.Store(c.config.ScanID, cancel) defer func() { - logrus.Infof("Removing from scanMap, scanid:%s", c.config.ScanId) - scanMap.Delete(c.config.ScanId) + logrus.Infof("Removing from scanMap, scanid:%s", c.config.ScanID) + scanMap.Delete(c.config.ScanID) }() stopped := false for _, complianceCheckType := range c.config.ComplianceCheckTypes { script, found := scriptConfig[complianceCheckType] if !found { - return errors.New("Compliance Check Type not found. Exiting. ") + return ErrComplianceCheckType } b := Bench{ Script: script, @@ -129,11 +134,11 @@ func (c *ComplianceScanner) RunComplianceScan() error { Status: strings.ToLower(item.Level), RemediationScript: item.Remediation, RemediationPuppet: item.RemediationImpact, - NodeId: fmt.Sprintf("%x", md5.Sum([]byte(c.config.NodeId+c.config.ScanId+item.TestNum+item.TestCategory))), + NodeID: fmt.Sprintf("%x", md5.Sum([]byte(c.config.NodeID+c.config.ScanID+item.TestNum+item.TestCategory))), NodeType: "host", NodeName: c.config.NodeName, ComplianceCheckType: complianceCheckType, - ScanId: c.config.ScanId, + ScanID: c.config.ScanID, } complianceScanResults = append(complianceScanResults, compScan) } @@ -142,33 +147,26 @@ func (c *ComplianceScanner) RunComplianceScan() error { return err } } - if stopped == true { - logrus.Infof("Scan stopped by user request, scanid:%s", c.config.ScanId) - c.PublishScanStatus("Scan stopped by user request", "CANCELLED", nil) - return nil - } - err = c.PublishScanStatus("", "COMPLETE", nil) - return err -} -func (c *ComplianceScanner) publishErrorStatus(scanMsg string) { - err := c.PublishScanStatus(scanMsg, "ERROR", nil) - if err != nil { - logrus.Error(err) + if stopped { + logrus.Infof("Scan stopped by user request, scanid:%s", c.config.ScanID) + return c.PublishScanStatus("Scan stopped by user request", "CANCELLED", nil) } + + return c.PublishScanStatus("", "COMPLETE", nil) } func (c *ComplianceScanner) PublishScanStatus(scanMsg string, status string, extras map[string]interface{}) error { - scanMsg = strings.Replace(scanMsg, "\n", " ", -1) + scanMsg = strings.ReplaceAll(scanMsg, "\n", " ") scanLog := map[string]interface{}{ - "scan_id": c.config.ScanId, + "scan_id": c.config.ScanID, "time_stamp": util.GetIntTimestamp(), "@timestamp": util.GetDatetimeNow(), "scan_message": scanMsg, "scan_status": status, "type": util.ComplianceScanLogs, "node_name": c.config.NodeName, - "node_id": c.config.NodeId, + "node_id": c.config.NodeID, "node_type": "host", "host_name": c.config.NodeName, "compliance_check_types": c.config.ComplianceCheckTypes, @@ -177,21 +175,26 @@ func (c *ComplianceScanner) PublishScanStatus(scanMsg string, status string, ext for k, v := range extras { scanLog[k] = v } - err := os.MkdirAll(filepath.Dir(c.config.ComplianceStatusFilePath), 0755) + + dir := filepath.Dir(c.config.ComplianceStatusFilePath) + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("os.MkdirAll %s: %w", dir, err) + } f, err := os.OpenFile(c.config.ComplianceStatusFilePath, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600) if err != nil { logrus.Errorf("error opening file:%v", err) - return err + return fmt.Errorf("os.Openfile %s: %w", c.config.ComplianceStatusFilePath, err) } - byteJson, err := json.Marshal(scanLog) + byteJSON, err := json.Marshal(scanLog) if err != nil { logrus.Errorf("Error in formatting json: %+v", scanLog) - return err + return fmt.Errorf("json.Marshal: %w", err) } - if _, err = f.WriteString(string(byteJson) + "\n"); err != nil { + if _, err = f.WriteString(string(byteJSON) + "\n"); err != nil { logrus.Errorf("%+v \n", err) + return fmt.Errorf("f.WriteString: %w", err) } - return err + return nil } func (c *ComplianceScanner) IngestComplianceResults(complianceDocs []util.ComplianceDoc) error { @@ -215,14 +218,14 @@ func (c *ComplianceScanner) IngestComplianceResults(complianceDocs []util.Compli } defer f.Close() for _, d := range data { - byteJson, err := json.Marshal(d) + byteJSON, err := json.Marshal(d) if err != nil { logrus.Errorf("%+v \n", err) continue } - strJson := string(byteJson) - strJson = strings.Replace(strJson, "\n", " ", -1) - if _, err = f.WriteString(strJson + "\n"); err != nil { + strJSON := string(byteJSON) + strJSON = strings.ReplaceAll(strJSON, "\n", " ") + if _, err = f.WriteString(strJSON + "\n"); err != nil { logrus.Errorf("%+v \n", err) } } diff --git a/tools/sidekick/sidekick.go b/tools/sidekick/sidekick.go index 019018d..7249d7f 100644 --- a/tools/sidekick/sidekick.go +++ b/tools/sidekick/sidekick.go @@ -32,7 +32,7 @@ func main() { case "ports": ifaces := sk.GetGlobalAddrs() value, _ := json.Marshal(ifaces) - log.Infof("%v", string(value[:])) + log.Infof("%v", string(value)) case "route": ip := net.ParseIP(*argIP) if ip == nil { diff --git a/util/type.go b/util/type.go index 4e9c626..025970a 100644 --- a/util/type.go +++ b/util/type.go @@ -6,13 +6,13 @@ const ( ) type Config struct { - ManagementConsoleUrl string `json:"management_console_url,omitempty"` + ManagementConsoleURL string `json:"management_console_url,omitempty"` ManagementConsolePort string `json:"management_console_port,omitempty"` DeepfenceKey string `json:"deepfence_key,omitempty"` - ScanId string `json:"scan_id,omitempty"` + ScanID string `json:"scan_id,omitempty"` NodeType string `json:"node_type,omitempty"` NodeName string `json:"node_name"` - NodeId string `json:"node_id,omitempty"` + NodeID string `json:"node_id,omitempty"` HostName string `json:"host_name,omitempty"` ComplianceCheckTypes []string `json:"compliance_check_types"` ComplianceResultsFilePath string @@ -24,10 +24,10 @@ type ComplianceDoc struct { TimeStamp int64 `json:"time_stamp"` Timestamp string `json:"@timestamp"` Masked bool `json:"masked"` - NodeId string `json:"node_id"` + NodeID string `json:"node_id"` NodeType string `json:"node_type"` KubernetesClusterName string `json:"kubernetes_cluster_name"` - KubernetesClusterId string `json:"kubernetes_cluster_id"` + KubernetesClusterID string `json:"kubernetes_cluster_id"` NodeName string `json:"node_name"` TestCategory string `json:"test_category"` TestNumber string `json:"test_number"` @@ -40,7 +40,7 @@ type ComplianceDoc struct { TestDesc string `json:"test_desc"` Status string `json:"status"` ComplianceCheckType string `json:"compliance_check_type"` - ScanId string `json:"scan_id"` + ScanID string `json:"scan_id"` } type ComplianceScanLog struct { @@ -48,15 +48,15 @@ type ComplianceScanLog struct { TimeStamp int64 `json:"time_stamp"` Timestamp string `json:"@timestamp"` Masked bool `json:"masked"` - NodeId string `json:"node_id"` + NodeID string `json:"node_id"` NodeType string `json:"node_type"` KubernetesClusterName string `json:"kubernetes_cluster_name"` - KubernetesClusterId string `json:"kubernetes_cluster_id"` + KubernetesClusterID string `json:"kubernetes_cluster_id"` NodeName string `json:"node_name"` ScanStatus string `json:"scan_status"` ScanMessage string `json:"scan_message"` ComplianceCheckType string `json:"compliance_check_type"` TotalChecks int `json:"total_checks"` Result map[string]int `json:"result"` - ScanId string `json:"scan_id"` + ScanID string `json:"scan_id"` }