Rename diff args new

cmd/netpolicy/cmd/command_test.go

@@ -384,13 +384,13 @@ func TestCommands(t *testing.T) {
 			},
 			expectedOutput: "Connectivity diff:\n" +
 				"diff-type: added, source: 0.0.0.0-255.255.255.255, destination: default/unicorn[Deployment], dir1:" +
-				"  No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
+				" No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
 				"diff-type: added, source: default/redis-cart[Deployment], destination: default/unicorn[Deployment], dir1:" +
-				"  No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
+				" No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
 				"diff-type: added, source: default/unicorn[Deployment], destination: 0.0.0.0-255.255.255.255, dir1:" +
-				"  No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
+				" No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
 				"diff-type: added, source: default/unicorn[Deployment], destination: default/redis-cart[Deployment], dir1:" +
-				"  No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added",
+				" No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added",
 			exact: true,
 			isErr: false,
 		},
@@ -409,13 +409,13 @@ func TestCommands(t *testing.T) {
 			},
 			expectedOutput: "Connectivity diff:\n" +
 				"diff-type: added, source: 0.0.0.0-255.255.255.255, destination: default/unicorn[Deployment], dir1:" +
-				"  No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
+				" No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
 				"diff-type: added, source: default/redis-cart[Deployment], destination: default/unicorn[Deployment], dir1:" +
-				"  No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
+				" No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
 				"diff-type: added, source: default/unicorn[Deployment], destination: 0.0.0.0-255.255.255.255, dir1:" +
-				"  No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
+				" No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added\n" +
 				"diff-type: added, source: default/unicorn[Deployment], destination: default/redis-cart[Deployment], dir1:" +
-				"  No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added",
+				" No Connections, dir2: All Connections, workloads-diff-info: workload default/unicorn[Deployment] added",
 			exact:   true,
 			isErr:   false,
 			hasFile: true,
@@ -516,7 +516,7 @@ func TestCommands(t *testing.T) {
 				filepath.Join(getTestsDir(), "onlineboutique_with_pods_severe_error")},
 			expectedOutput: "Connectivity diff:\n" +
 				"diff-type: changed, source: default/frontend-99684f7f8[ReplicaSet], " +
-				"destination: default/adservice-77d5cd745d[ReplicaSet], dir1:  TCP 9555, dir2: TCP 8080",
+				"destination: default/adservice-77d5cd745d[ReplicaSet], dir1: TCP 9555, dir2: TCP 8080",
 			exact: true,
 			isErr: false,
 		},

cmd/netpolicy/cmd/diff.go

@@ -32,6 +32,11 @@ var (
 	outFormat string
 )
 
+const (
+	dir1Arg = "dir1"
+	dir2Arg = "dir2"
+)
+
 func runDiffCommand() error {
 	var connsDiff diff.ConnectivityDiff
 	var err error
@@ -55,7 +60,7 @@ func runDiffCommand() error {
 }
 
 func getDiffOptions(l *logger.DefaultLogger) []diff.DiffAnalyzerOption {
-	res := []diff.DiffAnalyzerOption{diff.WithLogger(l), diff.WithOutputFormat(outFormat)}
+	res := []diff.DiffAnalyzerOption{diff.WithLogger(l), diff.WithOutputFormat(outFormat), diff.WithArgNames(dir1Arg, dir2Arg)}
 	if stopOnFirstError {
 		res = append(res, diff.WithStopOnError())
 	}
@@ -93,8 +98,8 @@ func newCommandDiff() *cobra.Command {
 	}
 
 	// define any flags and configuration settings.
-	c.Flags().StringVarP(&dir1, "dir1", "", "", "Original Resources path to be compared")
-	c.Flags().StringVarP(&dir2, "dir2", "", "", "New Resources path to compare with original resources path")
+	c.Flags().StringVarP(&dir1, dir1Arg, "", "", "Original Resources path to be compared")
+	c.Flags().StringVarP(&dir2, dir2Arg, "", "", "New Resources path to compare with original resources path")
 	supportedDiffFormats := strings.Join(diff.ValidDiffFormats, ",")
 	c.Flags().StringVarP(&outFormat, "output", "o", common.DefaultFormat, getOutputFormatDescription(supportedDiffFormats))
 	// out file

pkg/netpol/diff/connectivity_diff.go

@@ -9,17 +9,17 @@ import (
 
 // ConnectivityDiff captures the set of differences in terms of connectivity between two input k8s resource sets
 type ConnectivityDiff interface {
-	// RemovedConnections is a list of differences where the specified conn only exists in dir1
+	// RemovedConnections is a list of differences where the specified conn only exists in ref1
 	RemovedConnections() []SrcDstDiff
 
-	// AddedConnections  is a list of differences where the specified conn only exists in dir2
+	// AddedConnections  is a list of differences where the specified conn only exists in ref2
 	AddedConnections() []SrcDstDiff
 
-	// ChangedConnections is a list of differences where the specified conn exists in dir1 and dir2 but not identical
+	// ChangedConnections is a list of differences where the specified conn exists in ref1 and ref2 but not identical
 	// connection properties
 	ChangedConnections() []SrcDstDiff
 
-	// UnchangedConnections is a list of connections that exists in dir1 and dir2, and are identical
+	// UnchangedConnections is a list of connections that exists in ref1 and ref2, and are identical
 	UnchangedConnections() []SrcDstDiff
 
 	// IsEmpty returns true if there is no diff in connectivity, i.e. removed, added and changed connections are empty
@@ -32,17 +32,17 @@ type SrcDstDiff interface {
 	Src() Peer
 	// Dst returns the destination peer
 	Dst() Peer
-	// Dir1Connectivity returns the AllowedConnectivity from src to dst in dir1
-	Dir1Connectivity() AllowedConnectivity
-	// Dir2Connectivity returns the AllowedConnectivity from src to dst in dir2
-	Dir2Connectivity() AllowedConnectivity
-	// IsSrcNewOrRemoved returns true if the src peer exists only in dir2 (if DiffType is Added) or if
-	// the src peer exists only in dir1 (if DiffType is Removed)
+	// Ref1Connectivity returns the AllowedConnectivity from src to dst in ref1
+	Ref1Connectivity() AllowedConnectivity
+	// Ref2Connectivity returns the AllowedConnectivity from src to dst in ref2
+	Ref2Connectivity() AllowedConnectivity
+	// IsSrcNewOrRemoved returns true if the src peer exists only in ref2 (if DiffType is Added) or if
+	// the src peer exists only in ref1 (if DiffType is Removed)
 	IsSrcNewOrRemoved() bool
-	// IsDstNewOrRemoved returns true if the dst peer exists only in dir2 (if DiffType is Added) or if
-	// the dst peer exists only in dir1 (if DiffType is Removed)
+	// IsDstNewOrRemoved returns true if the dst peer exists only in ref2 (if DiffType is Added) or if
+	// the dst peer exists only in ref1 (if DiffType is Removed)
 	IsDstNewOrRemoved() bool
-	// DiffType returns the diff type of dir2 w.r.t dir1, which can be ChangedType/RemovedType/AddedType/NonChangedType
+	// DiffType returns the diff type of ref2 w.r.t ref1, which can be ChangedType/RemovedType/AddedType/UnchangedType
 	DiffType() DiffTypeStr
 }
 

pkg/netpol/diff/diff.go

@@ -7,6 +7,7 @@ package diff
 
 import (
 	"errors"
+	"fmt"
 	"os"
 
 	v1 "k8s.io/api/core/v1"
@@ -29,20 +30,22 @@ type DiffAnalyzer struct {
 	stopOnError  bool
 	errors       []DiffError
 	outputFormat string
+	ref1Name     string
+	ref2Name     string
 }
 
 // ConnDiffFromResourceInfos returns the connectivity diffs from two lists of resource.Info objects,
 // representing two versions of manifest sets to compare
 func (da *DiffAnalyzer) ConnDiffFromResourceInfos(infos1, infos2 []*resource.Info) (ConnectivityDiff, error) {
 	// connectivity analysis for first dir
-	// TODO: should add input arg dirPath to this API func? so that log msgs can specify the dir, rather then just "dir1"/"dir2"
-	conns1, workloads1, shouldStop, cDiff, errVal := da.getConnlistAnalysis(infos1, true, false, "")
+	// TODO: should add input arg dirPath to this API func? so that log msgs can specify the dir, rather then just "ref1"/"ref2"
+	conns1, workloads1, shouldStop, cDiff, errVal := da.getConnlistAnalysis(infos1, true, "")
 	if shouldStop {
 		return cDiff, errVal
 	}
 
 	// connectivity analysis for second dir
-	conns2, workloads2, shouldStop, cDiff, errVal := da.getConnlistAnalysis(infos2, false, true, "")
+	conns2, workloads2, shouldStop, cDiff, errVal := da.getConnlistAnalysis(infos2, false, "")
 	if shouldStop {
 		return cDiff, errVal
 	}
@@ -65,7 +68,7 @@ func (da *DiffAnalyzer) ConnDiffFromDirPaths(dirPath1, dirPath2 string) (Connect
 			if len(errs1) == 0 {
 				dirPath = dirPath2
 			}
-			da.logger.Errorf(err, "Error getting resourceInfos from dir paths dir1/dir2 ")
+			da.logger.Errorf(err, "Error getting resourceInfos from dir paths %s/%s ", da.ref1Name, da.ref2Name)
 			da.errors = append(da.errors, parser.FailedReadingFile(dirPath, err))
 			return nil, err // return as fatal error if both infos-lists are empty, or if stopOnError is on,
 			// or if at least one input dir does not exist
@@ -74,12 +77,12 @@ func (da *DiffAnalyzer) ConnDiffFromDirPaths(dirPath1, dirPath2 string) (Connect
 		// split err if it's an aggregated error to a list of separate errors
 		errReadingFile := "error reading file"
 		for _, err := range errs1 {
-			da.logger.Errorf(err, atDir1Prefix+errReadingFile)                     // print to log the error from builder
+			da.logger.Errorf(err, da.errPrefixSpecifyRefName(true)+errReadingFile) // print to log the error from builder
 			da.errors = append(da.errors, parser.FailedReadingFile(dirPath1, err)) // add the error from builder to accumulated errors
 		}
 		for _, err := range errs2 {
-			da.logger.Errorf(err, atDir2Prefix+errReadingFile)                     // print to log the error from builder
-			da.errors = append(da.errors, parser.FailedReadingFile(dirPath2, err)) // add the error from builder to accumulated errors
+			da.logger.Errorf(err, da.errPrefixSpecifyRefName(false)+errReadingFile) // print to log the error from builder
+			da.errors = append(da.errors, parser.FailedReadingFile(dirPath2, err))  // add the error from builder to accumulated errors
 		}
 	}
 	return da.ConnDiffFromResourceInfos(infos1, infos2)
@@ -161,6 +164,15 @@ func WithStopOnError() DiffAnalyzerOption {
 	}
 }
 
+// WithArgNames is a functional option that sets the names to be used for the two sets of analyzed resources
+// (default is ref1,ref2) in the output reports and log messages.
+func WithArgNames(ref1Name, ref2Name string) DiffAnalyzerOption {
+	return func(d *DiffAnalyzer) {
+		d.ref1Name = ref1Name
+		d.ref2Name = ref2Name
+	}
+}
+
 // NewDiffAnalyzer creates a new instance of DiffAnalyzer, and applies the provided functional options.
 func NewDiffAnalyzer(options ...DiffAnalyzerOption) *DiffAnalyzer {
 	// object with default behavior options
@@ -169,6 +181,8 @@ func NewDiffAnalyzer(options ...DiffAnalyzerOption) *DiffAnalyzer {
 		stopOnError:  false,
 		errors:       []DiffError{},
 		outputFormat: common.DefaultFormat,
+		ref1Name:     "ref1",
+		ref2Name:     "ref2",
 	}
 	for _, o := range options {
 		o(da)
@@ -203,7 +217,7 @@ func (da *DiffAnalyzer) hasFatalError() error {
 }
 
 // return a []ConnlistAnalyzerOption with mute errs/warns, so that logging of err/wanr is not duplicated, and
-// added to log only by getConnlistAnalysis function, which adds the context of dir1/dir2
+// added to log only by getConnlistAnalysis function, which adds the context of ref1/ref2
 func (da *DiffAnalyzer) determineConnlistAnalyzerOptions() []connlist.ConnlistAnalyzerOption {
 	if da.stopOnError {
 		return []connlist.ConnlistAnalyzerOption{connlist.WithMuteErrsAndWarns(), connlist.WithLogger(da.logger), connlist.WithStopOnError()}
@@ -220,8 +234,7 @@ func (da *DiffAnalyzer) determineConnlistAnalyzerOptions() []connlist.ConnlistAn
 // the main function, if the analysis should stop.
 func (da *DiffAnalyzer) getConnlistAnalysis(
 	infos []*resource.Info,
-	dir1 bool,
-	dir2 bool,
+	isRef1 bool,
 	dirPath string) (
 	[]connlist.Peer2PeerConnection,
 	[]connlist.Peer,
@@ -233,9 +246,10 @@ func (da *DiffAnalyzer) getConnlistAnalysis(
 	conns, workloads, err := connlistaAnalyzer.ConnlistFromResourceInfos(infos)
 
 	// append all fatal/severe errors and warnings returned by connlistaAnalyzer
+	errPrefix := da.errPrefixSpecifyRefName(isRef1)
 	for _, e := range connlistaAnalyzer.Errors() {
-		// wrap err/warn with new err type that includes context of dir1/dir2
-		daErr := newConnectivityAnalysisError(e.Error(), dir1, dir2, dirPath, e.IsSevere(), e.IsFatal())
+		// wrap err/warn with new err type that includes context of ref1/ref2
+		daErr := newConnectivityAnalysisError(e.Error(), errPrefix, dirPath, e.IsSevere(), e.IsFatal())
 		da.errors = append(da.errors, daErr)
 		logErrOrWarning(daErr, da.logger)
 	}
@@ -244,7 +258,7 @@ func (da *DiffAnalyzer) getConnlistAnalysis(
 		// check it exists, if not, append a new fatal err to the da.errors array
 		if da.hasFatalError() == nil {
 			// append the fatal error (indicates an issue in connlist analyzer, that did not append this err as expected)
-			da.errors = append(da.errors, newConnectivityAnalysisError(err, dir1, dir2, dirPath, false, true))
+			da.errors = append(da.errors, newConnectivityAnalysisError(err, errPrefix, dirPath, false, true))
 		}
 	}
 
@@ -264,6 +278,17 @@ func (da *DiffAnalyzer) getConnlistAnalysis(
 	return conns, workloads, shouldStop, cDiff, errVal
 }
 
+func (da *DiffAnalyzer) errPrefixSpecifyRefName(isRef1 bool) string {
+	if isRef1 {
+		return getErrPrefix(da.ref1Name)
+	}
+	return getErrPrefix(da.ref2Name)
+}
+
+func getErrPrefix(location string) string {
+	return fmt.Sprintf("at %s: ", location)
+}
+
 func logErrOrWarning(d DiffError, l logger.Logger) {
 	if d.IsSevere() || d.IsFatal() {
 		l.Errorf(d.Error(), "")
@@ -351,7 +376,7 @@ func (c *connsPair) Dst() Peer {
 	return c.firstConn.Dst()
 }
 
-func (c *connsPair) Dir1Connectivity() AllowedConnectivity {
+func (c *connsPair) Ref1Connectivity() AllowedConnectivity {
 	if c.diffType == AddedType {
 		return &allowedConnectivity{
 			allProtocolsAndPorts: false,
@@ -364,7 +389,7 @@ func (c *connsPair) Dir1Connectivity() AllowedConnectivity {
 	}
 }
 
-func (c *connsPair) Dir2Connectivity() AllowedConnectivity {
+func (c *connsPair) Ref2Connectivity() AllowedConnectivity {
 	if c.diffType == RemovedType {
 		return &allowedConnectivity{
 			allProtocolsAndPorts: false,
@@ -689,7 +714,7 @@ func (da *DiffAnalyzer) ConnectivityDiffToString(connectivityDiff ConnectivityDi
 		return "", nil
 	}
 	da.logger.Infof("Found connections diffs")
-	diffFormatter, err := getFormatter(da.outputFormat)
+	diffFormatter, err := getFormatter(da.outputFormat, da.ref1Name, da.ref2Name)
 	if err != nil {
 		da.errors = append(da.errors, newResultFormattingError(err))
 		return "", err
@@ -703,21 +728,21 @@ func (da *DiffAnalyzer) ConnectivityDiffToString(connectivityDiff ConnectivityDi
 }
 
 // returns the relevant formatter for the analyzer's outputFormat
-func getFormatter(format string) (diffFormatter, error) {
+func getFormatter(format, ref1Name, ref2Name string) (diffFormatter, error) {
 	if err := ValidateDiffOutputFormat(format); err != nil {
 		return nil, err
 	}
 	switch format {
 	case common.TextFormat:
-		return &diffFormatText{}, nil
+		return &diffFormatText{ref1: ref1Name, ref2: ref2Name}, nil
 	case common.CSVFormat:
-		return &diffFormatCSV{}, nil
+		return &diffFormatCSV{ref1: ref1Name, ref2: ref2Name}, nil
 	case common.MDFormat:
-		return &diffFormatMD{}, nil
+		return &diffFormatMD{ref1: ref1Name, ref2: ref2Name}, nil
 	case common.DOTFormat:
-		return &diffFormatDOT{}, nil
+		return &diffFormatDOT{ref1: ref1Name, ref2: ref2Name}, nil
 	default:
-		return &diffFormatText{}, nil
+		return &diffFormatText{ref1: ref1Name, ref2: ref2Name}, nil
 	}
 }
 

pkg/netpol/diff/diff_errors.go

@@ -26,10 +26,9 @@ type handlingIPpeersError struct {
 }
 
 type connectivityAnalysisError struct {
-	origErr error
-	dir1    bool
-	dir2    bool
-	dirPath string
+	origErr   error
+	errPrefix string
+	dirPath   string
 }
 
 ///////////////////////////
@@ -64,20 +63,13 @@ func (e *handlingIPpeersError) Error() string {
 	return e.origErr.Error()
 }
 
-const (
-	atDir1Prefix = "at dir1: "
-	atDir2Prefix = "at dir2: "
-)
-
 func (e *connectivityAnalysisError) Error() string {
 	var prefix string
 	switch {
 	case e.dirPath != "":
-		prefix = "at " + e.dirPath + ": "
-	case e.dir1:
-		prefix = atDir1Prefix
-	case e.dir2:
-		prefix = atDir2Prefix
+		prefix = getErrPrefix(e.dirPath)
+	case e.errPrefix != "": // prefix of ref1/ref2 names
+		prefix = e.errPrefix
 	}
 	return prefix + e.origErr.Error()
 }
@@ -91,7 +83,7 @@ func newHandlingIPpeersError(err error) *diffGeneratingError {
 	return &diffGeneratingError{err: &handlingIPpeersError{err}, fatal: true, severe: false}
 }
 
-func newConnectivityAnalysisError(err error, dir1, dir2 bool, dirPath string, isSevere, isFatal bool) *diffGeneratingError {
+func newConnectivityAnalysisError(err error, errPrefix, dirPath string, isSevere, isFatal bool) *diffGeneratingError {
 	return &diffGeneratingError{err: &connectivityAnalysisError{
-		origErr: err, dir1: dir1, dir2: dir2, dirPath: dirPath}, fatal: isFatal, severe: isSevere}
+		origErr: err, errPrefix: errPrefix, dirPath: dirPath}, fatal: isFatal, severe: isSevere}
 }