From 89c368f0e518d1eef5e76227628d94946af74f8f Mon Sep 17 00:00:00 2001 From: Swapnil Bawaskar Date: Wed, 10 Jul 2019 23:30:09 -0700 Subject: [PATCH] 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 --- pkg/builder/builder.go | 11 +-- pkg/builder/builder_test.go | 123 +++++++++++++++++++++++++++++--- pkg/duffle/manifest/manifest.go | 2 + 3 files changed, 121 insertions(+), 15 deletions(-) diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go index f5893ea2..86982915 100644 --- a/pkg/builder/builder.go +++ b/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 { diff --git a/pkg/builder/builder_test.go b/pkg/builder/builder_test.go index 14a11d1a..3999d05d 100644 --- a/pkg/builder/builder_test.go +++ b/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,18 +52,26 @@ 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", @@ -70,6 +79,11 @@ func TestPrepareBuild(t *testing.T) { 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 } diff --git a/pkg/duffle/manifest/manifest.go b/pkg/duffle/manifest/manifest.go index e555349c..c7ed7459 100644 --- a/pkg/duffle/manifest/manifest.go +++ b/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