Specifying output format (yaml or json) (#102)

* small fixes to inline documentation
* file no longer relevant
* preallocation
* removing deprecated checks, enforcing more rules
* Specifying output format from command-line
* redundant empty line
Signed-off-by: Ziv Nevo <[email protected]>

.golangci.yml

@@ -7,7 +7,7 @@ linters-settings:
   gci:
     local-prefixes: github.com/np-guard
   goconst:
-    min-len: 6
+    min-len: 2
     min-occurrences: 2
   gocritic:
     enabled-tags:
@@ -16,13 +16,6 @@ linters-settings:
       - opinionated
       - performance
       - style
-    disabled-checks:
-      - dupImport # https://github.com/go-critic/go-critic/issues/845
-      - ifElseChain
-      - octalLiteral
-      - whyNoLint
-      - wrapperFunc
-      - unnamedResult
   gocyclo:
     min-complexity: 15
   goimports:
@@ -46,9 +39,6 @@ linters-settings:
     allow-unused: false # report any unused nolint directives
     require-explanation: false # don't require an explanation for nolint directives
     require-specific: false # don't require nolint directives to be specific about which linter is being skipped
-  gosec:
-    # Exclude generated files
-    exclude-generated: true
   revive:
     # see https://github.com/mgechev/revive#available-rules for details.
     ignore-generated-header: true
@@ -68,13 +58,12 @@ linters:
   disable-all: true
   enable:
     - bodyclose
-    - deadcode
     - dogsled
     - dupl
     - errcheck
     - exportloopref
     - funlen
-    # - gochecknoinits
+    - gochecknoinits
     - goconst
     - gocritic
     - gocyclo
@@ -87,34 +76,25 @@ linters:
     - govet
     - ineffassign
     - lll
+    - misspell
     - nakedret
     - noctx
     - nolintlint
+    - prealloc
     - staticcheck
-    - structcheck
     - stylecheck
     - typecheck
     - unconvert
     - unparam
     - unused
-    - varcheck
     - whitespace
     - revive
 
   # don't enable:
-  # - asciicheck
-  # - depguard
-  # - scopelint
   # - gochecknoglobals
   # - gocognit
-  # - godot
-  # - godox
-  # - misspell
   # - goerr113
-  # - interfacer
-  # - maligned
   # - nestif
-  # - prealloc
   # - testpackage
   # - wsl
 issues:
@@ -125,14 +105,6 @@ issues:
        - revive
        - goconst
        - funlen
-    - path: _test\.go
-      linters:
-      - staticcheck
-      text: "SA1019"
-    - linters:
-      - lll
-      source: "^// .kubebuilder:"
-
 
 run:
   timeout: 5m

cmd/nettop/main.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"os"
 
+	"gopkg.in/yaml.v3"
+
 	"github.com/np-guard/cluster-topology-analyzer/pkg/controller"
 )
 
@@ -21,10 +23,37 @@ func writeBufToFile(filepath string, buf []byte) error {
 	return nil
 }
 
-func writeContent(outputFile string, content interface{}) error {
-	const indent = "    "
+func yamlMarshalUsingJSON(content interface{}) ([]byte, error) {
+	// Directly marshaling content into YAML, results in malformed Kubernetes resources.
+	// This is because K8s NetworkPolicy struct has json field tags, but no yaml field tags (also true for other resources).
+	// The (somewhat ugly) solution is to first marshal content to json, unmarshal to an interface{} var and marshal to yaml
+	buf, err := json.Marshal(content)
+	if err != nil {
+		return nil, err
+	}
+
+	var contentFromJSON interface{}
+	err = json.Unmarshal(buf, &contentFromJSON)
+	if err != nil {
+		return nil, err
+	}
+
+	buf, err = yaml.Marshal(contentFromJSON)
+	if err != nil {
+		return nil, err
+	}
+	return buf, nil
+}
 
-	buf, err := json.MarshalIndent(content, "", indent)
+func writeContent(outputFile, outputFormat string, content interface{}) error {
+	var buf []byte
+	var err error
+	if outputFormat == YamlFormat {
+		buf, err = yamlMarshalUsingJSON(content)
+	} else {
+		const indent = "    "
+		buf, err = json.MarshalIndent(content, "", indent)
+	}
 	if err != nil {
 		return err
 	}
@@ -72,7 +101,7 @@ func detectTopology(args InArgs) error {
 		}
 	}
 
-	if err := writeContent(*args.OutputFile, content); err != nil {
+	if err := writeContent(*args.OutputFile, *args.OutputFormat, content); err != nil {
 		logger.Errorf(err, "error writing results")
 		return err
 	}

cmd/nettop/main_test.go

@@ -15,7 +15,31 @@ func TestConnectionsOutput(t *testing.T) {
 	dirPath := filepath.Join(testsDir, "onlineboutique", "kubernetes-manifests.yaml")
 	outFile := filepath.Join(testsDir, "onlineboutique", "output.json")
 	expectedOutput := filepath.Join(testsDir, "onlineboutique", "expected_output.json")
-	args := getTestArgs(dirPath, outFile, false, false, false)
+	args := getTestArgs(dirPath, outFile, JSONFormat, false, false, false)
+
+	err := detectTopology(args)
+	if err != nil {
+		t.Fatalf("expected err to be nil, but got %v", err)
+	}
+
+	res, err := compareFiles(expectedOutput, outFile)
+
+	if err != nil {
+		t.Fatalf("expected err to be nil, but got %v", err)
+	}
+	if !res {
+		t.Fatalf("expected res to be true, but got false")
+	}
+
+	os.Remove(outFile)
+}
+
+func TestConnectionsYamlOutput(t *testing.T) {
+	testsDir := getTestsDir()
+	dirPath := filepath.Join(testsDir, "onlineboutique", "kubernetes-manifests.yaml")
+	outFile := filepath.Join(testsDir, "onlineboutique", "output.yaml")
+	expectedOutput := filepath.Join(testsDir, "onlineboutique", "expected_output.yaml")
+	args := getTestArgs(dirPath, outFile, YamlFormat, false, false, false)
 
 	err := detectTopology(args)
 	if err != nil {
@@ -39,7 +63,7 @@ func TestDirScan(t *testing.T) {
 	dirPath := filepath.Join(testsDir, "onlineboutique")
 	outFile := filepath.Join(dirPath, "output.json")
 	expectedOutput := filepath.Join(dirPath, "expected_dirscan_output.json")
-	args := getTestArgs(dirPath, outFile, false, true, false)
+	args := getTestArgs(dirPath, outFile, JSONFormat, false, true, false)
 
 	err := detectTopology(args)
 	if err != nil {
@@ -84,7 +108,7 @@ func TestNetpolsJsonOutput(t *testing.T) {
 		expectedOutput: filepath.Join(testsDir, "bookinfo", "expected_netpol_output.json")}
 
 	for testName, testDetails := range tests {
-		args := getTestArgs(testDetails.dirPath, testDetails.outFile, true, false, true)
+		args := getTestArgs(testDetails.dirPath, testDetails.outFile, JSONFormat, true, false, true)
 		err := detectTopology(args)
 		if err != nil {
 			t.Fatalf("Test %v: expected Start to return no error, but got %v", testName, err)
@@ -100,15 +124,40 @@ func TestNetpolsJsonOutput(t *testing.T) {
 	}
 }
 
+func TestNetpolsYamlOutput(t *testing.T) {
+	testsDir := getTestsDir()
+	dirPath := filepath.Join(testsDir, "onlineboutique", "kubernetes-manifests.yaml")
+	outFile := filepath.Join(testsDir, "onlineboutique", "output.yaml")
+	expectedOutput := filepath.Join(testsDir, "onlineboutique", "expected_netpol_output.yaml")
+	args := getTestArgs(dirPath, outFile, YamlFormat, true, false, false)
+
+	err := detectTopology(args)
+	if err != nil {
+		t.Fatalf("expected err to be nil, but got %v", err)
+	}
+
+	res, err := compareFiles(expectedOutput, outFile)
+
+	if err != nil {
+		t.Fatalf("expected err to be nil, but got %v", err)
+	}
+	if !res {
+		t.Fatalf("expected res to be true, but got false")
+	}
+
+	os.Remove(outFile)
+}
+
 func getTestsDir() string {
 	currentDir, _ := os.Getwd()
 	return filepath.Join(currentDir, "..", "..", "tests")
 }
 
-func getTestArgs(dirPath, outFile string, netpols, quiet, verbose bool) InArgs {
+func getTestArgs(dirPath, outFile, outFormat string, netpols, quiet, verbose bool) InArgs {
 	args := InArgs{}
 	args.DirPath = &dirPath
 	args.OutputFile = &outFile
+	args.OutputFormat = &outFormat
 	args.SynthNetpols = &netpols
 	args.Quiet = &quiet
 	args.Verbose = &verbose

cmd/nettop/parse_args.go

@@ -5,9 +5,15 @@ import (
 	"fmt"
 )
 
+const (
+	JSONFormat = "json"
+	YamlFormat = "yaml"
+)
+
 type InArgs struct {
 	DirPath      *string
 	OutputFile   *string
+	OutputFormat *string
 	SynthNetpols *bool
 	Quiet        *bool
 	Verbose      *bool
@@ -16,6 +22,7 @@ type InArgs struct {
 func ParseInArgs(args *InArgs) error {
 	args.DirPath = flag.String("dirpath", "", "input directory path")
 	args.OutputFile = flag.String("outputfile", "", "file path to store results")
+	args.OutputFormat = flag.String("format", JSONFormat, "output format (must be either json or yaml)")
 	args.SynthNetpols = flag.Bool("netpols", false, "whether to synthesize NetworkPolicies to allow only the discovered connections")
 	args.Quiet = flag.Bool("q", false, "runs quietly, reports only severe errors and results")
 	args.Verbose = flag.Bool("v", false, "runs with more informative messages printed to log")
@@ -29,6 +36,10 @@ func ParseInArgs(args *InArgs) error {
 		flag.PrintDefaults()
 		return fmt.Errorf("-q and -v cannot be specified together")
 	}
+	if *args.OutputFormat != JSONFormat && *args.OutputFormat != YamlFormat {
+		flag.PrintDefaults()
+		return fmt.Errorf("wrong output format %s; must be either json or yaml", *args.OutputFormat)
+	}
 
 	return nil
 }

pkg/controller/error_types.go

@@ -16,7 +16,7 @@ type FileProcessingError struct {
 	severe   bool // a severe error is recoverable. However, outputs should be used with care
 }
 
-// constructs a FileProcessingError object and prints an error/warning to the log
+// constructs a FileProcessingError object
 func newFileProcessingError(origErr error, msg, filePath string, lineNum, docID int, fatal, severe bool) *FileProcessingError {
 	err := errors.New(msg)
 	if origErr != nil {
@@ -71,7 +71,7 @@ func (e *FileProcessingError) IsFatal() bool {
 	return e.fatal
 }
 
-// IsFatal returns whether the error is considered severe
+// IsSevere returns whether the error is considered severe
 // (further processing is possible, but results may not be useable)
 func (e *FileProcessingError) IsSevere() bool {
 	return e.severe

pkg/controller/synth_netpols.go

@@ -115,7 +115,7 @@ func getDeployConnSelector(deployConn *deploymentConnectivity) *metaV1.LabelSele
 }
 
 func toNetpolPorts(ports []common.SvcNetworkAttr) []network.NetworkPolicyPort {
-	var netpolPorts []network.NetworkPolicyPort
+	netpolPorts := make([]network.NetworkPolicyPort, 0, len(ports))
 	for _, port := range ports {
 		protocol := toCoreProtocol(port.Protocol)
 		portNum := port.TargetPort
@@ -145,7 +145,7 @@ func toCoreProtocol(protocol string) core.Protocol {
 }
 
 func buildNetpolPerDeployment(deployConnectivity []*deploymentConnectivity) []*network.NetworkPolicy {
-	var netpols []*network.NetworkPolicy
+	netpols := make([]*network.NetworkPolicy, 0, len(deployConnectivity))
 	for _, deployConn := range deployConnectivity {
 		if len(deployConn.egressConns) > 0 {
 			deployConn.addEgressRule(nil, []network.NetworkPolicyPort{getDNSPort()})