From 090fc3c29fb007954dc7986cdccdad3aabe86223 Mon Sep 17 00:00:00 2001
From: Thibault NORMAND <Zenithar@users.noreply.github.com>
Date: Wed, 10 May 2023 23:57:25 +0200
Subject: [PATCH] Fix yaml serialization from pb (#221)

fix(api/schemas): remove owner from mandatory fields
fix(yaml): Protobuf to YAML conversion

Signed-off-by: Thibault Normand <me@zenithar.org>
---
 api/jsonschema/harp.bundle.v1/RuleSet.json |  4 +--
 pkg/sdk/convert/yaml.go                    | 31 +++++++++++++++++++++
 pkg/sdk/convert/yaml_test.go               | 32 ++++++++++++++++++++++
 pkg/tasks/bundle/diff.go                   |  7 ++---
 pkg/tasks/to/ruleset.go                    |  9 +++---
 5 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/api/jsonschema/harp.bundle.v1/RuleSet.json b/api/jsonschema/harp.bundle.v1/RuleSet.json
index 28191a86..c0276784 100644
--- a/api/jsonschema/harp.bundle.v1/RuleSet.json
+++ b/api/jsonschema/harp.bundle.v1/RuleSet.json
@@ -80,7 +80,7 @@
           "description": "RuleSet name."
         },
         "owner": {
-          "type": "string",
+          "type": ["string", "null"],
           "description": "RuleSet owner."
         },
         "description": {
@@ -88,7 +88,7 @@
           "description": "Short description for ruleset."
         }
       },
-      "required": ["name", "owner", "description"],
+      "required": ["name", "description"],
       "additionalProperties": false,
       "type": "object",
       "title": "Rule Set Meta",
diff --git a/pkg/sdk/convert/yaml.go b/pkg/sdk/convert/yaml.go
index d41e8438..6ba078ce 100644
--- a/pkg/sdk/convert/yaml.go
+++ b/pkg/sdk/convert/yaml.go
@@ -25,6 +25,8 @@ import (
 	"io"
 	"reflect"
 
+	"google.golang.org/protobuf/encoding/protojson"
+	"google.golang.org/protobuf/proto"
 	"sigs.k8s.io/yaml"
 
 	"github.com/elastic/harp/pkg/sdk/types"
@@ -47,6 +49,35 @@ func YAMLtoJSON(r io.Reader) (io.Reader, error) {
 	return jsonReader, nil
 }
 
+// PBtoYAML converts a protobuf object to a YAML representation
+func PBtoYAML(msg proto.Message) ([]byte, error) {
+	// Check arguments
+	if types.IsNil(msg) {
+		return nil, fmt.Errorf("msg is nil")
+	}
+
+	// Encode protbuf message as JSON
+	pb, err := protojson.Marshal(msg)
+	if err != nil {
+		return nil, fmt.Errorf("unable to encode protbuf message to JSON: %w", err)
+	}
+
+	// Decode input as JSON
+	var jsonObj interface{}
+	if errDecode := yaml.Unmarshal(pb, &jsonObj); errDecode != nil {
+		return nil, fmt.Errorf("unable to decode JSON input: %w", errDecode)
+	}
+
+	// Marshal as YAML
+	out, errEncode := yaml.Marshal(jsonObj)
+	if errEncode != nil {
+		return nil, fmt.Errorf("unable to produce YAML output: %w", errEncode)
+	}
+
+	// No error
+	return out, nil
+}
+
 // -----------------------------------------------------------------------------
 
 // loadFromYAML reads YAML definition and returns the PB struct.
diff --git a/pkg/sdk/convert/yaml_test.go b/pkg/sdk/convert/yaml_test.go
index 744ef6d8..124a9e53 100644
--- a/pkg/sdk/convert/yaml_test.go
+++ b/pkg/sdk/convert/yaml_test.go
@@ -20,8 +20,40 @@ package convert
 import (
 	"reflect"
 	"testing"
+
+	bundlev1 "github.com/elastic/harp/api/gen/go/harp/bundle/v1"
+	"github.com/google/go-cmp/cmp"
 )
 
+func Test_PBtoYAML(t *testing.T) {
+	spec := &bundlev1.Patch{
+		ApiVersion: "harp.elastic.co/v1",
+		Kind:       "BundlePatch",
+		Meta: &bundlev1.PatchMeta{
+			Name: "test-patch",
+		},
+		Spec: &bundlev1.PatchSpec{
+			Rules: []*bundlev1.PatchRule{
+				{
+					Package:  &bundlev1.PatchPackage{},
+					Selector: &bundlev1.PatchSelector{},
+				},
+			},
+		},
+	}
+
+	expectedOutput := []byte("apiVersion: harp.elastic.co/v1\nkind: BundlePatch\nmeta:\n  name: test-patch\nspec:\n  rules:\n  - package: {}\n    selector: {}\n")
+
+	out, err := PBtoYAML(spec)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if report := cmp.Diff(string(out), string(expectedOutput)); report != "" {
+		t.Errorf("unexpected conversion output:\n%v", report)
+	}
+}
+
 func Test_convertMapStringInterface(t *testing.T) {
 	type args struct {
 		val interface{}
diff --git a/pkg/tasks/bundle/diff.go b/pkg/tasks/bundle/diff.go
index 989db112..61025366 100644
--- a/pkg/tasks/bundle/diff.go
+++ b/pkg/tasks/bundle/diff.go
@@ -23,10 +23,9 @@ import (
 	"errors"
 	"fmt"
 
-	"sigs.k8s.io/yaml"
-
 	"github.com/elastic/harp/pkg/bundle"
 	"github.com/elastic/harp/pkg/bundle/compare"
+	"github.com/elastic/harp/pkg/sdk/convert"
 	"github.com/elastic/harp/pkg/sdk/types"
 	"github.com/elastic/harp/pkg/tasks"
 )
@@ -100,8 +99,8 @@ func (t *DiffTask) Run(ctx context.Context) error {
 			return fmt.Errorf("unable to convert oplog as a bundle patch: %w", err)
 		}
 
-		// Marshal YAML Patch
-		out, err := yaml.Marshal(patch)
+		// Marshal as YAML
+		out, err := convert.PBtoYAML(patch)
 		if err != nil {
 			return fmt.Errorf("unable to marshal patch as YAML: %w", err)
 		}
diff --git a/pkg/tasks/to/ruleset.go b/pkg/tasks/to/ruleset.go
index e4d2213c..ce4bf26a 100644
--- a/pkg/tasks/to/ruleset.go
+++ b/pkg/tasks/to/ruleset.go
@@ -21,10 +21,9 @@ import (
 	"context"
 	"fmt"
 
-	"sigs.k8s.io/yaml"
-
 	"github.com/elastic/harp/pkg/bundle"
 	"github.com/elastic/harp/pkg/bundle/ruleset"
+	"github.com/elastic/harp/pkg/sdk/convert"
 	"github.com/elastic/harp/pkg/tasks"
 )
 
@@ -55,9 +54,9 @@ func (t *RuleSetTask) Run(ctx context.Context) error {
 	}
 
 	// Marshal as YAML
-	ruleSetSpec, err := yaml.Marshal(rs)
+	out, err := convert.PBtoYAML(rs)
 	if err != nil {
-		return fmt.Errorf("unable to generate YAML descriptor: %w", err)
+		return fmt.Errorf("unable to marshal as YAML: %w", err)
 	}
 
 	// Create output writer
@@ -67,7 +66,7 @@ func (t *RuleSetTask) Run(ctx context.Context) error {
 	}
 
 	// Write output
-	fmt.Fprintln(writer, string(ruleSetSpec))
+	fmt.Fprintln(writer, string(out))
 
 	// No error
 	return nil