add multi-part yaml support to GetChanges for the bundle facade

api/facadeversions.go

@@ -23,7 +23,7 @@ var facadeVersions = map[string]int{
 	"ApplicationScaler":            1,
 	"Backups":                      2,
 	"Block":                        2,
-	"Bundle":                       5,
+	"Bundle":                       6,
 	"CAASAgent":                    2,
 	"CAASAdmission":                1,
 	"CAASApplication":              1,

apiserver/allfacades.go

@@ -177,6 +177,7 @@ func AllFacades() *facade.Registry {
 	reg("Bundle", 3, bundle.NewFacadeV3)
 	reg("Bundle", 4, bundle.NewFacadeV4)
 	reg("Bundle", 5, bundle.NewFacadeV5)
+	reg("Bundle", 6, bundle.NewFacadeV6)
 	reg("CharmHub", 1, charmhub.NewFacade)
 	reg("CharmRevisionUpdater", 2, charmrevisionupdater.NewCharmRevisionUpdaterAPI)
 	reg("Charms", 2, charms.NewFacadeV2)

apiserver/facades/client/bundle/bundle.go

@@ -63,6 +63,13 @@ type APIv4 struct {
 // identical to V4 with the exception that the V5 adds an arg to export
 // bundle to control what is exported..
 type APIv5 struct {
+	*APIv6
+}
+
+// APIv6 provides the Bundle API facade for version 6. It is otherwise
+// identical to V5 with the exception that the V6 adds the support for
+// multi-part yaml handling to GetChanges and GetChangesMapArgs.
+type APIv6 struct {
 	*BundleAPI
 }
 
@@ -118,13 +125,23 @@ func NewFacadeV4(ctx facade.Context) (*APIv4, error) {
 // NewFacadeV5 provides the signature required for facade registration
 // for version 5.
 func NewFacadeV5(ctx facade.Context) (*APIv5, error) {
-	api, err := newFacade(ctx)
+	api, err := NewFacadeV6(ctx)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
 	return &APIv5{api}, nil
 }
 
+// NewFacadeV6 provides the signature required for facade registration
+// for version 6.
+func NewFacadeV6(ctx facade.Context) (*APIv6, error) {
+	api, err := newFacade(ctx)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return &APIv6{api}, nil
+}
+
 // NewFacade provides the required signature for facade registration.
 func newFacade(ctx facade.Context) (*BundleAPI, error) {
 	authorizer := ctx.Auth()
@@ -166,7 +183,7 @@ func NewBundleAPIv1(
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	return &APIv1{&APIv2{&APIv3{&APIv4{&APIv5{api}}}}}, nil
+	return &APIv1{&APIv2{&APIv3{&APIv4{&APIv5{&APIv6{api}}}}}}, nil
 }
 
 func (b *BundleAPI) checkCanRead() error {
@@ -196,7 +213,7 @@ func (b *APIv1) GetChanges(args params.BundleChangesParams) (params.BundleChange
 		},
 		verifyDevices: nil,
 	}
-	return getChanges(args, vs, func(changes []bundlechanges.Change, results *params.BundleChangesResults) error {
+	return b.getChanges(args, vs, func(changes []bundlechanges.Change, results *params.BundleChangesResults) error {
 		results.Changes = make([]*params.BundleChange, len(changes))
 		for i, c := range changes {
 			results.Changes[i] = &params.BundleChange{
@@ -216,7 +233,9 @@ type validators struct {
 	verifyDevices     func(string) error
 }
 
-func getBundleChanges(args params.BundleChangesParams,
+// getBundleChanges (<= V5 API) ignores the extra parts in a
+// multi-part yaml
+func (b *APIv5) getBundleChanges(args params.BundleChangesParams,
 	vs validators,
 ) ([]bundlechanges.Change, []error, error) {
 	data, err := charm.ReadBundleData(strings.NewReader(args.BundleDataYAML))
@@ -246,13 +265,44 @@ func getBundleChanges(args params.BundleChangesParams,
 	return changes, nil, nil
 }
 
-func getChanges(
+func (b *BundleAPI) getBundleChanges(args params.BundleChangesParams,
+	vs validators,
+) ([]bundlechanges.Change, []error, error) {
+	dataSource, _ := charm.StreamBundleDataSource(strings.NewReader(args.BundleDataYAML), args.BundleURL)
+	data, err := charm.ReadAndMergeBundleData(dataSource)
+	if err != nil {
+		return nil, nil, errors.Annotate(err, "cannot read bundle YAML")
+	}
+	if err := data.Verify(vs.verifyConstraints, vs.verifyStorage, vs.verifyDevices); err != nil {
+		if verificationError, ok := err.(*charm.VerificationError); ok {
+			validationErrors := make([]error, len(verificationError.Errors))
+			for i, e := range verificationError.Errors {
+				validationErrors[i] = e
+			}
+			return nil, validationErrors, nil
+		}
+		// This should never happen as Verify only returns verification errors.
+		return nil, nil, errors.Annotate(err, "cannot verify bundle")
+	}
+	changes, err := bundlechanges.FromData(
+		bundlechanges.ChangesConfig{
+			Bundle:    data,
+			BundleURL: args.BundleURL,
+			Logger:    loggo.GetLogger("juju.apiserver.bundlechanges"),
+		})
+	if err != nil {
+		return nil, nil, errors.Trace(err)
+	}
+	return changes, nil, nil
+}
+
+func (b *BundleAPI) getChanges(
 	args params.BundleChangesParams,
 	vs validators,
 	postProcess func([]bundlechanges.Change, *params.BundleChangesResults) error,
 ) (params.BundleChangesResults, error) {
 	var results params.BundleChangesResults
-	changes, validationErrors, err := getBundleChanges(args, vs)
+	changes, validationErrors, err := b.getBundleChanges(args, vs)
 	if err != nil {
 		return results, errors.Trace(err)
 	}
@@ -288,7 +338,7 @@ func (b *BundleAPI) GetChanges(args params.BundleChangesParams) (params.BundleCh
 			return err
 		},
 	}
-	return getChanges(args, vs, func(changes []bundlechanges.Change, results *params.BundleChangesResults) error {
+	return b.getChanges(args, vs, func(changes []bundlechanges.Change, results *params.BundleChangesResults) error {
 		results.Changes = make([]*params.BundleChange, len(changes))
 		for i, c := range changes {
 			var guiArgs []interface{}
@@ -332,7 +382,7 @@ func (b *BundleAPI) GetChangesMapArgs(args params.BundleChangesParams) (params.B
 			return err
 		},
 	}
-	return getChangesMapArgs(args, vs, func(changes []bundlechanges.Change, results *params.BundleChangesMapArgsResults) error {
+	return b.getChangesMapArgs(args, vs, func(changes []bundlechanges.Change, results *params.BundleChangesMapArgsResults) error {
 		results.Changes = make([]*params.BundleChangesMapArgs, len(changes))
 		for i, c := range changes {
 			args, err := c.Args()
@@ -351,13 +401,13 @@ func (b *BundleAPI) GetChangesMapArgs(args params.BundleChangesParams) (params.B
 	})
 }
 
-func getChangesMapArgs(
+func (b *BundleAPI) getChangesMapArgs(
 	args params.BundleChangesParams,
 	vs validators,
 	postProcess func([]bundlechanges.Change, *params.BundleChangesMapArgsResults) error,
 ) (params.BundleChangesMapArgsResults, error) {
 	var results params.BundleChangesMapArgsResults
-	changes, validationErrors, err := getBundleChanges(args, vs)
+	changes, validationErrors, err := b.getBundleChanges(args, vs)
 	if err != nil {
 		return results, errors.Trace(err)
 	}

apiserver/facades/client/bundle/bundle_test.go

@@ -25,7 +25,7 @@ import (
 type bundleSuite struct {
 	coretesting.BaseSuite
 	auth     *apiservertesting.FakeAuthorizer
-	facade   *bundle.APIv5
+	facade   *bundle.APIv6
 	st       *mockState
 	modelTag names.ModelTag
 }
@@ -43,22 +43,22 @@ func (s *bundleSuite) SetUpTest(c *gc.C) {
 	s.facade = s.makeAPI(c)
 }
 
-func (s *bundleSuite) makeAPI(c *gc.C) *bundle.APIv5 {
+func (s *bundleSuite) makeAPI(c *gc.C) *bundle.APIv6 {
 	api, err := bundle.NewBundleAPI(
 		s.st,
 		s.auth,
 		s.modelTag,
 	)
 	c.Assert(err, jc.ErrorIsNil)
-	return &bundle.APIv5{api}
+	return &bundle.APIv6{api}
 }
 
 func (s *bundleSuite) TestGetChangesBundleContentError(c *gc.C) {
 	args := params.BundleChangesParams{
 		BundleDataYAML: ":",
 	}
 	r, err := s.facade.GetChanges(args)
-	c.Assert(err, gc.ErrorMatches, `cannot read bundle YAML: unmarshal document 0: yaml: did not find expected key`)
+	c.Assert(err, gc.ErrorMatches, `cannot read bundle YAML: malformed bundle: bundle is empty not valid`)
 	c.Assert(r, gc.DeepEquals, params.BundleChangesResults{})
 }
 
@@ -213,6 +213,109 @@ func (s *bundleSuite) TestGetChangesSuccessV2(c *gc.C) {
 	c.Assert(r.Errors, gc.IsNil)
 }
 
+func (s *bundleSuite) TestGetChangesSuccessWithOverlays(c *gc.C) {
+	args := params.BundleChangesParams{
+		BundleDataYAML: `
+            applications:
+                django:
+                    charm: django
+                    options:
+                        debug: true
+                    storage:
+                        tmpfs: tmpfs,1G
+                    devices:
+                        bitcoinminer: 2,nvidia.com/gpu
+                haproxy:
+                    charm: cs:trusty/haproxy-42
+            relations:
+                - - django:web
+                  - haproxy:web
+--- # overlay
+description: remove haproxy
+applications:
+    haproxy:
+        `,
+	}
+	r, err := s.facade.GetChanges(args)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(r.Changes, jc.DeepEquals, []*params.BundleChange{{
+		Id:     "addCharm-0",
+		Method: "addCharm",
+		Args:   []interface{}{"django", "", ""},
+	}, {
+		Id:     "deploy-1",
+		Method: "deploy",
+		Args: []interface{}{
+			"$addCharm-0",
+			"",
+			"django",
+			map[string]interface{}{"debug": true},
+			"",
+			map[string]string{"tmpfs": "tmpfs,1G"},
+			map[string]string{"bitcoinminer": "2,nvidia.com/gpu"},
+			map[string]string{},
+			map[string]int{},
+			0,
+			"",
+		},
+		Requires: []string{"addCharm-0"},
+	}})
+	c.Assert(r.Errors, gc.IsNil)
+}
+
+func (s *bundleSuite) TestGetChangesFailV5WithOverlays(c *gc.C) {
+	args := params.BundleChangesParams{
+		BundleDataYAML: `
+            applications:
+                django:
+                    charm: django
+                    options:
+                        debug: true
+                    storage:
+                        tmpfs: tmpfs,1G
+                    devices:
+                        bitcoinminer: 2,nvidia.com/gpu
+                haproxy:
+                    charm: cs:trusty/haproxy-42
+            relations:
+                - - django:web
+                  - haproxy:web
+--- # overlay
+description: remove haproxy
+applications:
+    haproxy:
+        `,
+	}
+	api := s.makeAPI(c)
+	apiv1 := &bundle.APIv1{&bundle.APIv2{&bundle.APIv3{&bundle.APIv4{&bundle.APIv5{api}}}}}
+	r, err := apiv1.GetChanges(args)
+	c.Assert(err, jc.ErrorIsNil)
+	// This is the inverted test of TestGetChangesSuccessV2WithOverlays
+	c.Assert(r.Changes, gc.Not(jc.DeepEquals), []*params.BundleChange{{
+		Id:     "addCharm-0",
+		Method: "addCharm",
+		Args:   []interface{}{"django", "", ""},
+	}, {
+		Id:     "deploy-1",
+		Method: "deploy",
+		Args: []interface{}{
+			"$addCharm-0",
+			"",
+			"django",
+			map[string]interface{}{"debug": true},
+			"",
+			map[string]string{"tmpfs": "tmpfs,1G"},
+			map[string]string{"bitcoinminer": "2,nvidia.com/gpu"},
+			map[string]string{},
+			map[string]int{},
+			0,
+			"",
+		},
+		Requires: []string{"addCharm-0"},
+	}})
+	c.Assert(r.Errors, gc.IsNil)
+}
+
 func (s *bundleSuite) TestGetChangesKubernetes(c *gc.C) {
 	args := params.BundleChangesParams{
 		BundleDataYAML: `
@@ -307,7 +410,7 @@ func (s *bundleSuite) TestGetChangesSuccessV1(c *gc.C) {
         `,
 	}
 	api := s.makeAPI(c)
-	apiv1 := &bundle.APIv1{&bundle.APIv2{&bundle.APIv3{&bundle.APIv4{api}}}}
+	apiv1 := &bundle.APIv1{&bundle.APIv2{&bundle.APIv3{&bundle.APIv4{&bundle.APIv5{api}}}}}
 
 	r, err := apiv1.GetChanges(args)
 	c.Assert(err, jc.ErrorIsNil)
@@ -403,7 +506,7 @@ func (s *bundleSuite) TestGetChangesMapArgsBundleContentError(c *gc.C) {
 		BundleDataYAML: ":",
 	}
 	r, err := s.facade.GetChangesMapArgs(args)
-	c.Assert(err, gc.ErrorMatches, `cannot read bundle YAML: unmarshal document 0: yaml: did not find expected key`)
+	c.Assert(err, gc.ErrorMatches, `cannot read bundle YAML: malformed bundle: bundle is empty not valid`)
 	c.Assert(r, gc.DeepEquals, params.BundleChangesMapArgsResults{})
 }
 

apiserver/facades/schema.json

@@ -5820,8 +5820,8 @@
     },
     {
         "Name": "Bundle",
-        "Description": "APIv5 provides the Bundle API facade for version 5. It is otherwise\nidentical to V4 with the exception that the V5 adds an arg to export\nbundle to control what is exported..",
-        "Version": 5,
+        "Description": "APIv6 provides the Bundle API facade for version 6. It is otherwise\nidentical to V5 with the exception that the V6 adds the support for\nmulti-part yaml handling to GetChanges and GetChangesMapArgs.",
+        "Version": 6,
         "AvailableTo": [
             "controller-user",
             "model-user"