cli/policy: add conflict detection command (#3972)

Adds a new `check-conflicts` subcommand for the
`osm policy` command to detect conflicts among
traffic policies.

Implements conflict detection for the `IngressBackend`
resource and will be extended to other API resources
kinds (ex. `Egress`) when necessary.

The command will be referenced in the troubleshooting
guides for traffic policies.

Signed-off-by: Shashank Ram <shashr2204@gmail.com>

cmd/cli/osm.go

@@ -44,7 +44,7 @@ func newRootCmd(config *action.Configuration, stdin io.Reader, stdout io.Writer,
 		newMetricsCmd(stdout),
 		newVersionCmd(stdout),
 		newProxyCmd(config, stdout),
-		newTrafficPolicyCmd(stdout),
+		newPolicyCmd(stdout, stderr),
 		newUninstallCmd(config, stdin, stdout),
 		newSupportCmd(config, stdout, stderr),
 	)

cmd/cli/trafficpolicy.go → cmd/cli/policy.go

@@ -11,14 +11,15 @@ This command consists of subcommands related to traffic policies
 associated with osm.
 `
 
-func newTrafficPolicyCmd(out io.Writer) *cobra.Command {
+func newPolicyCmd(stdout io.Writer, _ io.Writer) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "policy",
 		Short: "manage and check traffic policies",
 		Long:  trafficPolicyDescription,
 		Args:  cobra.NoArgs,
 	}
-	cmd.AddCommand(newTrafficPolicyCheck(out))
+	cmd.AddCommand(newPolicyCheckPods(stdout))
+	cmd.AddCommand(newPolicyCheckConflicts(stdout))
 
 	return cmd
 }

cmd/cli/policy_check_conflicts.go

@@ -0,0 +1,130 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"strings"
+
+	mapset "github.com/deckarep/golang-set"
+	"github.com/pkg/errors"
+	"github.com/spf13/cobra"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	policyv1alpha1 "github.com/openservicemesh/osm/pkg/apis/policy/v1alpha1"
+	policyClientset "github.com/openservicemesh/osm/pkg/gen/client/policy/clientset/versioned"
+
+	"github.com/openservicemesh/osm/pkg/policy"
+)
+
+const policyCheckConflictsDesc = `
+This command checks whether API resources of the same kind conflict.
+`
+
+const policyCheckConflictsExample = `
+# To check if IngressBackend API resources conflict in the 'test' namespace
+osm policy check-conflicts IngressBackend -n test
+`
+
+type policyCheckConflictsCmd struct {
+	stdout       io.Writer
+	resourceKind string
+	policyClient policyClientset.Interface
+	namespaces   []string
+}
+
+func newPolicyCheckConflicts(stdout io.Writer) *cobra.Command {
+	policyCheckConflictsCmd := &policyCheckConflictsCmd{
+		stdout: stdout,
+	}
+
+	cmd := &cobra.Command{
+		Use:   "check-conflicts RESOURCE_KIND",
+		Short: "check if an API resource conflicts with another resource of the same kind",
+		Long:  policyCheckConflictsDesc,
+		Args:  cobra.ExactArgs(1),
+		RunE: func(_ *cobra.Command, args []string) error {
+			policyCheckConflictsCmd.resourceKind = args[0]
+
+			config, err := settings.RESTClientGetter().ToRESTConfig()
+			if err != nil {
+				return errors.Wrap(err, "Error fetching kubeconfig")
+			}
+
+			policyClient, err := policyClientset.NewForConfig(config)
+			if err != nil {
+				return errors.Wrapf(err, "Error initializing %s client", policyv1alpha1.SchemeGroupVersion)
+			}
+			policyCheckConflictsCmd.policyClient = policyClient
+
+			return policyCheckConflictsCmd.run()
+		},
+		Example: policyCheckConflictsExample,
+	}
+
+	f := cmd.Flags()
+	f.StringSliceVarP(&policyCheckConflictsCmd.namespaces, "namespaces", "n", []string{}, "One or more namespaces to limit conflict checks to")
+
+	return cmd
+}
+
+func (cmd *policyCheckConflictsCmd) run() error {
+	var err error
+
+	switch strings.ToLower(cmd.resourceKind) {
+	case "ingressbackend":
+		err = cmd.checkIngressBackendConflict()
+
+	default:
+		return errors.Errorf("Invalid resource kind %s", cmd.resourceKind)
+	}
+
+	return err
+}
+
+func (cmd *policyCheckConflictsCmd) checkIngressBackendConflict() error {
+	givenNsCount := len(cmd.namespaces)
+	if givenNsCount != 1 {
+		return errors.Errorf("Requires single namespace specified by '-n|--namespace' to check for conflicts, got %d", givenNsCount)
+	}
+
+	ns := cmd.namespaces[0]
+
+	ingressBackends, err := cmd.policyClient.PolicyV1alpha1().IngressBackends(ns).List(context.Background(), metav1.ListOptions{})
+	if err != nil {
+		return errors.Wrapf(err, "Error listing IngressBackend resources in namespace %s", ns)
+	}
+
+	conflictsExist := false
+	computed := mapset.NewSet()
+	for _, x := range ingressBackends.Items {
+		for _, y := range ingressBackends.Items {
+			if x.Name == y.Name {
+				continue
+			}
+
+			xyVisitedKey := fmt.Sprintf("%s:%s", x.Name, y.Name)
+			yxVisitedKey := fmt.Sprintf("%s:%s", y.Name, x.Name)
+
+			if computed.Contains(xyVisitedKey) || computed.Contains(yxVisitedKey) {
+				continue
+			}
+			computed.Add(xyVisitedKey)
+
+			if conflicts := policy.DetectIngressBackendConflicts(x, y); len(conflicts) > 0 {
+				fmt.Fprintf(cmd.stdout, "[+] IngressBackend %s/%s conflicts with %s/%s:\n", ns, x.Name, ns, y.Name)
+				for _, err := range conflicts {
+					fmt.Fprintf(cmd.stdout, "%s\n", err)
+				}
+				fmt.Fprintf(cmd.stdout, "\n")
+				conflictsExist = true
+			}
+		}
+	}
+
+	if !conflictsExist {
+		fmt.Fprintf(cmd.stdout, "No conflicts among IngressBackend resources in namespace %s\n", ns)
+	}
+
+	return nil
+}

cmd/cli/policy_check_conflicts_test.go

@@ -0,0 +1,230 @@
+package main
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	policyv1alpha1 "github.com/openservicemesh/osm/pkg/apis/policy/v1alpha1"
+	fakePolicyClientset "github.com/openservicemesh/osm/pkg/gen/client/policy/clientset/versioned/fake"
+)
+
+func TestPolicyCheckConflictRun(t *testing.T) {
+	testNs := "test"
+
+	testCases := []struct {
+		name                  string
+		resourceKind          string
+		namespaces            []string
+		existingResources     []runtime.Object
+		expectErr             bool
+		expectedRegexMatchOut string
+	}{
+		{
+			name:         "No conflicts among IngressBackend resources",
+			resourceKind: "IngressBackend",
+			namespaces:   []string{testNs},
+			existingResources: []runtime.Object{
+				&policyv1alpha1.IngressBackend{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "ingress-backend-1",
+						Namespace: testNs,
+					},
+					Spec: policyv1alpha1.IngressBackendSpec{
+						Backends: []policyv1alpha1.BackendSpec{
+							{
+								Name: "backend1",
+								Port: policyv1alpha1.PortSpec{
+									Number:   80,
+									Protocol: "http",
+								},
+							},
+						},
+						Sources: []policyv1alpha1.IngressSourceSpec{
+							{
+								Kind:      "Service",
+								Name:      "client",
+								Namespace: "foo",
+							},
+						},
+					},
+				},
+				&policyv1alpha1.IngressBackend{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "ingress-backend-2",
+						Namespace: testNs,
+					},
+					Spec: policyv1alpha1.IngressBackendSpec{
+						Backends: []policyv1alpha1.BackendSpec{
+							{
+								Name: "backend2",
+								Port: policyv1alpha1.PortSpec{
+									Number:   80,
+									Protocol: "http",
+								},
+							},
+						},
+						Sources: []policyv1alpha1.IngressSourceSpec{
+							{
+								Kind:      "Service",
+								Name:      "client",
+								Namespace: "foo",
+							},
+						},
+					},
+				},
+			},
+			expectErr:             false,
+			expectedRegexMatchOut: "No conflicts among IngressBackend resources in namespace",
+		},
+		{
+			name:         "Conflicts among IngressBackend resources",
+			resourceKind: "IngressBackend",
+			namespaces:   []string{testNs},
+			existingResources: []runtime.Object{
+				&policyv1alpha1.IngressBackend{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "ingress-backend-1",
+						Namespace: testNs,
+					},
+					Spec: policyv1alpha1.IngressBackendSpec{
+						Backends: []policyv1alpha1.BackendSpec{
+							{
+								Name: "backend1",
+								Port: policyv1alpha1.PortSpec{
+									Number:   80,
+									Protocol: "http",
+								},
+							},
+						},
+						Sources: []policyv1alpha1.IngressSourceSpec{
+							{
+								Kind:      "Service",
+								Name:      "client",
+								Namespace: "foo",
+							},
+						},
+					},
+				},
+				&policyv1alpha1.IngressBackend{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "ingress-backend-2",
+						Namespace: testNs,
+					},
+					Spec: policyv1alpha1.IngressBackendSpec{
+						Backends: []policyv1alpha1.BackendSpec{
+							{
+								Name: "backend1",
+								Port: policyv1alpha1.PortSpec{
+									Number:   80,
+									Protocol: "http",
+								},
+							},
+						},
+						Sources: []policyv1alpha1.IngressSourceSpec{
+							{
+								Kind:      "Service",
+								Name:      "client",
+								Namespace: "foo",
+							},
+						},
+					},
+				},
+			},
+			expectErr:             false,
+			expectedRegexMatchOut: "IngressBackend .* conflicts with .*",
+		},
+		{
+			name:              "Invalid resource kind results in an error",
+			resourceKind:      "invalid",
+			namespaces:        []string{testNs},
+			existingResources: nil,
+			expectErr:         true,
+		},
+		{
+			name:         "Passing multiple namespaces is an invalid input for the IngressBackend conflict check",
+			resourceKind: "IngressBackend",
+			namespaces:   []string{testNs, "extra"},
+			existingResources: []runtime.Object{
+				&policyv1alpha1.IngressBackend{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "ingress-backend-1",
+						Namespace: testNs,
+					},
+					Spec: policyv1alpha1.IngressBackendSpec{
+						Backends: []policyv1alpha1.BackendSpec{
+							{
+								Name: "backend1",
+								Port: policyv1alpha1.PortSpec{
+									Number:   80,
+									Protocol: "http",
+								},
+							},
+						},
+						Sources: []policyv1alpha1.IngressSourceSpec{
+							{
+								Kind:      "Service",
+								Name:      "client",
+								Namespace: "foo",
+							},
+						},
+					},
+				},
+				&policyv1alpha1.IngressBackend{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "ingress-backend-2",
+						Namespace: testNs,
+					},
+					Spec: policyv1alpha1.IngressBackendSpec{
+						Backends: []policyv1alpha1.BackendSpec{
+							{
+								Name: "backend2",
+								Port: policyv1alpha1.PortSpec{
+									Number:   80,
+									Protocol: "http",
+								},
+							},
+						},
+						Sources: []policyv1alpha1.IngressSourceSpec{
+							{
+								Kind:      "Service",
+								Name:      "client",
+								Namespace: "foo",
+							},
+						},
+					},
+				},
+			},
+			expectErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			a := assert.New(t)
+			stdout := new(bytes.Buffer)
+
+			cmd := &policyCheckConflictsCmd{
+				stdout:       stdout,
+				namespaces:   tc.namespaces,
+				resourceKind: tc.resourceKind,
+			}
+
+			switch tc.resourceKind {
+			case "IngressBackend":
+				cmd.policyClient = fakePolicyClientset.NewSimpleClientset(tc.existingResources...)
+			}
+
+			err := cmd.run()
+			a.Equal(tc.expectErr, err != nil, err)
+			if err != nil {
+				return
+			}
+
+			a.Regexp(tc.expectedRegexMatchOut, stdout.String())
+		})
+	}
+}