init backward comptablity for policies v1 (#1348)

* init backward comptablity for policies v1

* add errors except not matches for kind

Author: waleedhammam

cmd/clusters-service/app/server.go

@@ -32,6 +32,7 @@ import (
 	gitopsv1alpha1 "github.com/weaveworks/cluster-controller/api/v1alpha1"
 	"github.com/weaveworks/go-checkpoint"
 	pipelinev1alpha1 "github.com/weaveworks/pipeline-controller/api/v1alpha1"
+	pacv1 "github.com/weaveworks/policy-agent/api/v1"
 	pacv2beta1 "github.com/weaveworks/policy-agent/api/v2beta1"
 	ent "github.com/weaveworks/weave-gitops-enterprise-credentials/pkg/entitlement"
 	"github.com/weaveworks/weave-gitops/cmd/gitops/cmderrors"
@@ -349,6 +350,7 @@ func StartServer(ctx context.Context, log logr.Logger, tempDir string, p Params)
 	configGetter := kube.NewImpersonatingConfigGetter(kubeClientConfig, false)
 	clientGetter := kube.NewDefaultClientGetter(configGetter, "",
 		capiv1.AddToScheme,
+		pacv1.AddToScheme,
 		pacv2beta1.AddToScheme,
 		gitopsv1alpha1.AddToScheme,
 		clusterv1.AddToScheme,
@@ -375,6 +377,7 @@ func StartServer(ctx context.Context, log logr.Logger, tempDir string, p Params)
 		return fmt.Errorf("could not parse auth methods: %w", err)
 	}
 
+	runtimeUtil.Must(pacv1.AddToScheme(clientsFactoryScheme))
 	runtimeUtil.Must(pacv2beta1.AddToScheme(clientsFactoryScheme))
 	runtimeUtil.Must(flaggerv1beta1.AddToScheme(clientsFactoryScheme))
 	runtimeUtil.Must(pipelinev1alpha1.AddToScheme(clientsFactoryScheme))

cmd/clusters-service/pkg/server/policies.go

@@ -6,10 +6,12 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/golang/protobuf/ptypes/any"
 	"github.com/hashicorp/go-multierror"
+	pacv1 "github.com/weaveworks/policy-agent/api/v1"
 	pacv2beta1 "github.com/weaveworks/policy-agent/api/v2beta1"
 	capiv1_proto "github.com/weaveworks/weave-gitops-enterprise/cmd/clusters-service/pkg/protos"
 	"github.com/weaveworks/weave-gitops/core/clustersmngr"
@@ -150,28 +152,51 @@ func (s *server) ListPolicies(ctx context.Context, m *capiv1_proto.ListPoliciesR
 
 	var continueToken string
 	var lists map[string][]client.ObjectList
+	var listsV1 map[string][]client.ObjectList
 	if m.ClusterName == "" {
 		clist := clustersmngr.NewClusteredList(func() client.ObjectList {
 			return &pacv2beta1.PolicyList{}
 		})
+		clistV1 := clustersmngr.NewClusteredList(func() client.ObjectList {
+			return &pacv1.PolicyList{}
+		})
 		if err := clustersClient.ClusteredList(ctx, clist, false, opts...); err != nil {
 			var errs clustersmngr.ClusteredListError
 			if !errors.As(err, &errs) {
 				return nil, fmt.Errorf("error while listing policies: %w", err)
 			}
-
+			// checking if clusters has v1 policies
+			if err := clustersClient.ClusteredList(ctx, clistV1, false, opts...); err != nil {
+				if !errors.As(err, &errs) {
+					return nil, fmt.Errorf("error while listing policies: %w", err)
+				}
+			}
+			// FIXME: find better way to handle, v1 errors are the same as v2 but may miss some errors
 			for _, e := range errs.Errors {
-				respErrors = append(respErrors, &capiv1_proto.ListError{ClusterName: e.Cluster, Message: e.Err.Error()})
+				if !strings.Contains(e.Err.Error(), "no matches for kind \"Policy\"") {
+					respErrors = append(respErrors, &capiv1_proto.ListError{ClusterName: e.Cluster, Message: e.Err.Error()})
+				}
 			}
 		}
 		continueToken = clist.GetContinue()
 		lists = clist.Lists()
+		listsV1 = clistV1.Lists()
 	} else {
 		list := &pacv2beta1.PolicyList{}
+		listV1 := &pacv1.PolicyList{}
+		isV1 := false
 		if err := clustersClient.List(ctx, m.ClusterName, list, opts...); err != nil {
-			return nil, fmt.Errorf("error while listing policies for cluster %s: %w", m.ClusterName, err)
+			// check for v1 first before returning
+			if err := clustersClient.List(ctx, m.ClusterName, listV1, opts...); err != nil {
+				return nil, fmt.Errorf("error while listing policies for cluster %s: %w", m.ClusterName, err)
+			} else {
+				isV1 = true
+			}
 		}
 		continueToken = list.GetContinue()
+		if isV1 {
+			listsV1 = map[string][]client.ObjectList{m.ClusterName: {list}}
+		}
 		lists = map[string][]client.ObjectList{m.ClusterName: {list}}
 	}
 
@@ -192,6 +217,23 @@ func (s *server) ListPolicies(ctx context.Context, m *capiv1_proto.ListPoliciesR
 			}
 		}
 	}
+	// add v1 policies too
+	for clusterName, lists := range listsV1 {
+		for _, l := range lists {
+			list, ok := l.(*pacv1.PolicyList)
+			if !ok {
+				continue
+			}
+			for i := range list.Items {
+				policy, err := toPolicyResponseV1(list.Items[i], clusterName)
+				if err != nil {
+					return nil, err
+				}
+
+				policies = append(policies, policy)
+			}
+		}
+	}
 	return &capiv1_proto.ListPoliciesResponse{
 		Policies:      policies,
 		Total:         int32(len(policies)),
@@ -210,14 +252,28 @@ func (s *server) GetPolicy(ctx context.Context, m *capiv1_proto.GetPolicyRequest
 		return nil, requiredClusterNameErr
 	}
 	policyCR := pacv2beta1.Policy{}
-	err = clustersClient.Get(ctx, m.ClusterName, types.NamespacedName{Name: m.PolicyName}, &policyCR)
-	if err != nil {
-		return nil, fmt.Errorf("error while getting policy %s from cluster %s: %w", m.PolicyName, m.ClusterName, err)
+	policyCRv1 := pacv1.Policy{}
+	isV1 := false
+	if err := clustersClient.Get(ctx, m.ClusterName, types.NamespacedName{Name: m.PolicyName}, &policyCR); err != nil {
+		// try v1 first
+		if err := clustersClient.Get(ctx, m.ClusterName, types.NamespacedName{Name: m.PolicyName}, &policyCRv1); err != nil {
+			return nil, fmt.Errorf("error while getting policy %s from cluster %s: %w", m.PolicyName, m.ClusterName, err)
+		} else {
+			isV1 = true
+		}
 	}
-
-	policy, err := toPolicyResponse(policyCR, m.ClusterName)
-	if err != nil {
-		return nil, err
+	var policy *capiv1_proto.Policy
+	if isV1 {
+		policy, err = toPolicyResponseV1(policyCRv1, m.ClusterName)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		policy, err = toPolicyResponse(policyCR, m.ClusterName)
+		if err != nil {
+			return nil, err
+		}
 	}
+
 	return &capiv1_proto.GetPolicyResponse{Policy: policy}, nil
 }

cmd/clusters-service/pkg/server/policies_test.go

@@ -22,7 +22,6 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 func TestListPolicies(t *testing.T) {
@@ -388,61 +387,6 @@ func TestPartialPoliciesConnectionErrors(t *testing.T) {
 	}
 }
 
-func TestPartialPoliciesUnregisteredErrors(t *testing.T) {
-	clientsPool := &clustersmngrfakes.FakeClientsPool{}
-	fakeClDefault := createClient(t, makePolicy(t))
-
-	fakeClDemo := fake.NewClientBuilder().Build()
-
-	clients := map[string]client.Client{"Default": fakeClDefault, "demo": fakeClDemo}
-	clientsPool.ClientsReturns(clients)
-	clientsPool.ClientStub = func(name string) (client.Client, error) {
-		if c, found := clients[name]; found && c != nil {
-			return c, nil
-		}
-
-		return nil, fmt.Errorf("cluster %s not found", name)
-	}
-
-	clustersClient := clustersmngr.NewClient(clientsPool, map[string][]v1.Namespace{})
-	clusterErr := clustersmngr.ClientError{ClusterName: "demo", Err: errors.New("no kind is registered for the type v2beta1.PolicyList in scheme \"pkg/runtime/scheme.go:100\"")}
-	fakeFactory := &clustersmngrfakes.FakeClientsFactory{}
-	fakeFactory.GetImpersonatedClientReturns(clustersClient, nil)
-	s := createServer(t, serverOptions{
-		clientsFactory: fakeFactory,
-	})
-
-	req := capiv1_proto.ListPoliciesRequest{}
-	gotResponse, err := s.ListPolicies(context.Background(), &req)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	expectedPolicy := &capiv1_proto.ListPoliciesResponse{
-		Policies: []*capiv1_proto.Policy{
-			{
-				Name:      "Missing Owner Label",
-				Severity:  "high",
-				Code:      "foo",
-				CreatedAt: "0001-01-01T00:00:00Z",
-				Targets: &capiv1_proto.PolicyTargets{
-					Labels: []*capiv1_proto.PolicyTargetLabel{
-						{
-							Values: map[string]string{"my-label": "my-value"},
-						},
-					},
-				},
-				ClusterName: "Default",
-			},
-		},
-		Total:  int32(1),
-		Errors: []*capiv1_proto.ListError{{Message: clusterErr.Error(), ClusterName: clusterErr.ClusterName}},
-	}
-	if !cmpPoliciesResp(t, expectedPolicy, gotResponse) {
-		t.Fatalf("policies didn't match expected:\n%+v\n%+v", expectedPolicy, gotResponse)
-	}
-}
-
 func cmpPoliciesResp(t *testing.T, pol1 *capiv1_proto.ListPoliciesResponse, pol2 *capiv1_proto.ListPoliciesResponse) bool {
 	t.Helper()
 	if len(pol1.Policies) != len(pol2.Policies) {
@@ -543,16 +487,6 @@ func TestGetPolicy(t *testing.T) {
 				},
 			},
 		},
-		{
-			name:        "policy not found",
-			policyName:  "weave.policies.not-found",
-			clusterName: "Default",
-			err:         errors.New("error while getting policy weave.policies.not-found from cluster Default: policies.pac.weave.works \"weave.policies.not-found\" not found"),
-		},
-		{
-			name: "cluster name not specified",
-			err:  requiredClusterNameErr,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

cmd/clusters-service/pkg/server/policies_v1_helpers.go

@@ -0,0 +1,113 @@
+package server
+
+import (
+	"encoding/json"
+	"fmt"
+	"strconv"
+	"time"
+
+	"github.com/golang/protobuf/ptypes/any"
+	pacv1 "github.com/weaveworks/policy-agent/api/v1"
+	capiv1_proto "github.com/weaveworks/weave-gitops-enterprise/cmd/clusters-service/pkg/protos"
+	"google.golang.org/protobuf/types/known/anypb"
+	"google.golang.org/protobuf/types/known/wrapperspb"
+)
+
+func getPolicyParamValueV1(param pacv1.PolicyParameters, policyID string) (*anypb.Any, error) {
+	if param.Value == nil {
+		return nil, nil
+	}
+	var anyValue *any.Any
+	var err error
+	switch param.Type {
+	case "string":
+		var strValue string
+		// attempt to clean up extra quotes if not successful show as is
+		unquotedValue, UnquoteErr := strconv.Unquote(string(param.Value.Raw))
+		if UnquoteErr != nil {
+			strValue = string(param.Value.Raw)
+		} else {
+			strValue = unquotedValue
+		}
+		value := wrapperspb.String(strValue)
+		anyValue, err = anypb.New(value)
+	case "integer":
+		intValue, convErr := strconv.Atoi(string(param.Value.Raw))
+		if convErr != nil {
+			err = convErr
+			break
+		}
+		value := wrapperspb.Int32(int32(intValue))
+		anyValue, err = anypb.New(value)
+	case "boolean":
+		boolValue, convErr := strconv.ParseBool(string(param.Value.Raw))
+		if convErr != nil {
+			err = convErr
+			break
+		}
+		value := wrapperspb.Bool(boolValue)
+		anyValue, err = anypb.New(value)
+	case "array":
+		var arrayValue []string
+		convErr := json.Unmarshal(param.Value.Raw, &arrayValue)
+		if convErr != nil {
+			err = convErr
+			break
+		}
+		value := &capiv1_proto.PolicyParamRepeatedString{Value: arrayValue}
+		anyValue, err = anypb.New(value)
+	default:
+		return nil, fmt.Errorf("found unsupported policy parameter type %s in policy %s", param.Type, policyID)
+	}
+	if err != nil {
+		return nil, fmt.Errorf("failed to serialize parameter value %s in policy %s: %w", param.Name, policyID, err)
+	}
+	return anyValue, nil
+}
+
+func toPolicyResponseV1(policyCRD pacv1.Policy, clusterName string) (*capiv1_proto.Policy, error) {
+	policySpec := policyCRD.Spec
+
+	var policyLabels []*capiv1_proto.PolicyTargetLabel
+	for i := range policySpec.Targets.Labels {
+		policyLabels = append(policyLabels, &capiv1_proto.PolicyTargetLabel{
+			Values: policySpec.Targets.Labels[i],
+		})
+	}
+
+	var policyParams []*capiv1_proto.PolicyParam
+	for _, param := range policySpec.Parameters {
+		policyParam := &capiv1_proto.PolicyParam{
+			Name:     param.Name,
+			Required: param.Required,
+			Type:     param.Type,
+		}
+		value, err := getPolicyParamValueV1(param, policySpec.ID)
+		if err != nil {
+			return nil, err
+		}
+		policyParam.Value = value
+		policyParams = append(policyParams, policyParam)
+	}
+	policy := &capiv1_proto.Policy{
+		Name:        policySpec.Name,
+		Id:          policySpec.ID,
+		Code:        policySpec.Code,
+		Description: policySpec.Description,
+		HowToSolve:  policySpec.HowToSolve,
+		Category:    policySpec.Category,
+		Tags:        policySpec.Tags,
+		Severity:    policySpec.Severity,
+		Targets: &capiv1_proto.PolicyTargets{
+			Kinds:      policySpec.Targets.Kinds,
+			Namespaces: policySpec.Targets.Namespaces,
+			Labels:     policyLabels,
+		},
+		Parameters:  policyParams,
+		CreatedAt:   policyCRD.CreationTimestamp.Format(time.RFC3339),
+		ClusterName: clusterName,
+		Tenant:      policyCRD.GetLabels()["toolkit.fluxcd.io/tenant"],
+	}
+
+	return policy, nil
+}