keep bundle and manifest in sync (#794)

* keep bundle and manifest in sync

copy all fields from manifest to bundle to ensure that bundle is not
missing any configuration that the bundle author intended.
Added unit tests to keep the two structure in sync

Co-Authored-By: Glyn Normington <gnormington@pivotal.io>

Author: 2 people

pkg/builder/builder.go

@@ -67,16 +67,19 @@ func (b *Builder) PrepareBuild(bldr *Builder, mfst *manifest.Manifest, appDir st
 	}
 
 	bf := &bundle.Bundle{
-		Name:          ctx.Manifest.Name,
+		Actions:       ctx.Manifest.Actions,
+		Credentials:   ctx.Manifest.Credentials,
+		Custom:        ctx.Manifest.Custom,
+		Definitions:   ctx.Manifest.Definitions,
 		Description:   ctx.Manifest.Description,
 		Images:        ctx.Manifest.Images,
 		Keywords:      ctx.Manifest.Keywords,
 		Maintainers:   ctx.Manifest.Maintainers,
-		Actions:       ctx.Manifest.Actions,
+		Name:          ctx.Manifest.Name,
+		Outputs:       ctx.Manifest.Outputs,
 		Parameters:    ctx.Manifest.Parameters,
-		Credentials:   ctx.Manifest.Credentials,
-		Version:       ctx.Manifest.Version,
 		SchemaVersion: ctx.Manifest.SchemaVersion,
+		Version:       ctx.Manifest.Version,
 	}
 
 	for _, imb := range imageBuilders {

pkg/builder/builder_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/deislabs/cnab-go/bundle"
+	"github.com/deislabs/cnab-go/bundle/definition"
 
 	"github.com/deislabs/duffle/pkg/duffle/manifest"
 	"github.com/deislabs/duffle/pkg/imagebuilder"
@@ -51,25 +52,38 @@ func (tc testImage) Build(ctx context.Context, log io.WriteCloser) error {
 }
 
 func TestPrepareBuild(t *testing.T) {
+	outputs := &bundle.OutputsDefinition{
+		Fields: map[string]bundle.OutputDefinition{"output1": {}},
+	}
+	params := &bundle.ParametersDefinition{
+		Fields: map[string]bundle.ParameterDefinition{"param1": {}},
+	}
 	mfst := &manifest.Manifest{
-		Name:          "foo",
-		Version:       "0.1.0",
-		SchemaVersion: "v1.0.0",
-		Description:   "description",
-		Keywords:      []string{"test"},
+		Actions:     map[string]bundle.Action{"act1": {}},
+		Credentials: map[string]bundle.Credential{"cred1": {}},
+		Custom:      map[string]interface{}{"cus1": nil},
+		Definitions: map[string]*definition.Schema{"def1": {}},
+		Description: "description",
+		Images:      map[string]bundle.Image{"img1": {}},
 		InvocationImages: map[string]*manifest.InvocationImage{
 			"cnab": {
 				Name:          "cnab",
 				Configuration: map[string]string{"registry": "registry"},
 			},
 		},
+		Keywords: []string{"test"},
 		Maintainers: []bundle.Maintainer{
 			{
 				Name:  "test",
 				Email: "test@test.com",
 				URL:   "https://test.com",
 			},
 		},
+		Name:          "foo",
+		Outputs:       outputs,
+		Parameters:    params,
+		SchemaVersion: "v1.0.0",
+		Version:       "0.1.0",
 	}
 
 	components := []imagebuilder.ImageBuilder{
@@ -86,21 +100,108 @@ func TestPrepareBuild(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
+	checksPerformed := 0
 
-	if len(b.InvocationImages) != 1 {
-		t.Fatalf("expected there to be 1 image, got %d. Full output: %v", len(b.Images), b)
+	if !reflect.DeepEqual(b.Actions, mfst.Actions) {
+		t.Errorf("expected actions to be %+v but was %+v", mfst.Actions, b.Actions)
 	}
+	checksPerformed++
 
-	if b.Version != mfst.Version {
-		t.Errorf("expected version %v, got %v", mfst.Version, b.Version)
+	if !reflect.DeepEqual(b.Credentials, mfst.Credentials) {
+		t.Errorf("expected credentials to be %+v but was %+v", mfst.Credentials, b.Credentials)
 	}
-	if b.SchemaVersion != mfst.SchemaVersion {
-		t.Errorf("expected schemaVersion %v, got %v", mfst.SchemaVersion, b.SchemaVersion)
+	checksPerformed++
+
+	if !reflect.DeepEqual(b.Custom, mfst.Custom) {
+		t.Errorf("expected custom to be %+v but was %+v", mfst.Custom, b.Custom)
+	}
+	checksPerformed++
+
+	if !reflect.DeepEqual(b.Definitions, mfst.Definitions) {
+		t.Errorf("expected definitions to be %+v but was %+v", mfst.Definitions, b.Definitions)
 	}
+	checksPerformed++
+
+	if b.Description != mfst.Description {
+		t.Errorf("expected description to be %+v but was %+v", mfst.Description, b.Description)
+	}
+	checksPerformed++
+
+	if len(b.InvocationImages) != 1 {
+		t.Fatalf("expected there to be 1 image, got %d. Full output: %v", len(b.Images), b)
+	}
+	checksPerformed++
+
 	expected := bundle.InvocationImage{}
 	expected.Image = "cnab:0.1.0"
 	expected.ImageType = "docker"
 	if !reflect.DeepEqual(b.InvocationImages[0], expected) {
 		t.Errorf("expected %v, got %v", expected, b.InvocationImages[0])
 	}
+	checksPerformed++
+
+	if !reflect.DeepEqual(b.Keywords, mfst.Keywords) {
+		t.Errorf("expected keywords to be %+v but was %+v", mfst.Keywords, b.Keywords)
+	}
+	checksPerformed++
+
+	if !reflect.DeepEqual(b.Maintainers, mfst.Maintainers) {
+		t.Errorf("expected maintainers to be %+v but was %+v", mfst.Maintainers, b.Maintainers)
+	}
+	checksPerformed++
+
+	if b.Name != mfst.Name {
+		t.Errorf("expected name to be %+v but was %+v", mfst.Name, b.Name)
+	}
+	checksPerformed++
+
+	if !reflect.DeepEqual(b.Outputs, mfst.Outputs) {
+		t.Errorf("expected outputs to be %+v but was %+v", mfst.Outputs, b.Outputs)
+	}
+	checksPerformed++
+
+	if !reflect.DeepEqual(b.Parameters, mfst.Parameters) {
+		t.Errorf("expected parameters to be %+v but was %+v", mfst.Parameters, b.Parameters)
+	}
+	checksPerformed++
+
+	if b.SchemaVersion != mfst.SchemaVersion {
+		t.Errorf("expected schemaVersion %v, got %v", mfst.SchemaVersion, b.SchemaVersion)
+	}
+	checksPerformed++
+
+	if b.Version != mfst.Version {
+		t.Errorf("expected version %v, got %v", mfst.Version, b.Version)
+	}
+	checksPerformed++
+
+	// Ensure that all the fields have been checked. If the structures need to diverge in the future, this test should be modified.
+	mfstFields := getFields(manifest.Manifest{})
+	if len(mfstFields) != checksPerformed {
+		t.Errorf("expected to check %v fields for equality, but checked only %v fields", len(mfstFields), checksPerformed)
+	}
+}
+
+func TestBundleAndManifestHaveSameFields(t *testing.T) {
+	mfst := manifest.Manifest{}
+	mfstFields := getFields(mfst)
+
+	b := bundle.Bundle{}
+	bundleFields := getFields(b)
+
+	if !reflect.DeepEqual(bundleFields, mfstFields) {
+		t.Errorf("manifest and bundle have different fields.\nmanifest: %+v\nbundle: %+v\n", mfstFields, bundleFields)
+	}
+}
+
+func getFields(i interface{}) map[string]struct{} {
+	fields := make(map[string]struct{}, 15)
+
+	v := reflect.ValueOf(i)
+	typeOf := reflect.TypeOf(i)
+
+	for i := 0; i < v.NumField(); i++ {
+		fields[typeOf.Field(i).Name] = struct{}{}
+	}
+	return fields
 }

pkg/duffle/manifest/manifest.go

@@ -24,6 +24,8 @@ type Manifest struct {
 	Parameters       *bundle.ParametersDefinition `json:"parameters,omitempty" mapstructure:"parameters"`
 	Credentials      map[string]bundle.Credential `json:"credentials,omitempty" mapstructure:"credentials"`
 	Definitions      definition.Definitions       `json:"definitions,omitempty" mapstructure:"definitions"`
+	Outputs          *bundle.OutputsDefinition    `json:"outputs,omitempty" mapstructure:"outputs"`
+	Custom           map[string]interface{}       `json:"custom,omitempty" mapstructure:"custom"`
 }
 
 // InvocationImage represents an invocation image component of a CNAB bundle