Skip to content

Commit

Permalink
Verify endpoint bindings when deploying bundles
Browse files Browse the repository at this point in the history 
Similar to how bindings with --bind are verified here:
https://github.com/juju/juju/blob/5a29872ebdae8b293a77ca33f8b43da01401edbe/cmd/juju/application/binding.go#L57-L59
Verify endpoint bindigns when adding handling application deployments
inside of bundles

This involved having access to SpaceAPI.ListSpaces inside of deployer
package
  • Loading branch information
@jack-w-shaw
jack-w-shaw committed Apr 7, 2022
1 parent 5a29872 commit f1454a9
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 22 deletions.
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Go can be [installed](https://golang.org/doc/install#install) from the official

Juju uses Go modules and does not depend on GOPATH, therefore you can check juju out anywhere.

### Change to the Juju source code directory
### Change to the Juju source code directory

cd juju

Expand Down Expand Up @@ -254,7 +254,7 @@ Testing
-------

Some tests may require local lxd to be installed, see
[installing lxd via snap](https://stgraber.org/2016/10/17/lxd-snap-available/).
[installing lxd via snap](https://stgraber.org/2016/10/17/lxd-snap-available/).

Juju uses the `gocheck` testing framework, which is automatically installed
as a dependency of `juju`. You can read more about `gocheck` at
Expand Down
11 changes: 2 additions & 9 deletions cmd/juju/application/deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,6 @@ type SpacesAPI interface {

var supportedJujuSeries = series.WorkloadSeries

type DeployAPI interface {
deployer.DeployerAPI
SpacesAPI
// PlanURL returns the configured URL prefix for the metering plan API.
PlanURL() string
}

type CharmsAPI interface {
store.CharmsAPI
BestAPIVersion() int
Expand Down Expand Up @@ -211,7 +204,7 @@ func newDeployCommand() *DeployCommand {

return charmhub.NewClient(cfg)
}
deployCmd.NewDeployAPI = func() (DeployAPI, error) {
deployCmd.NewDeployAPI = func() (deployer.DeployerAPI, error) {
apiRoot, err := deployCmd.newAPIRoot()
if err != nil {
return nil, errors.Trace(err)
Expand Down Expand Up @@ -344,7 +337,7 @@ type DeployCommand struct {
BundleMachines map[string]string

// NewDeployAPI stores a function which returns a new deploy client.
NewDeployAPI func() (DeployAPI, error)
NewDeployAPI func() (deployer.DeployerAPI, error)

// NewCharmRepo stores a function which returns a charm store client.
NewCharmRepo func() (*store.CharmStoreAdaptor, error)
Expand Down
24 changes: 16 additions & 8 deletions cmd/juju/application/deploy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ type DeploySuiteBase struct {
// charm store and the controller deploy API.
func (s *DeploySuiteBase) deployCommand() *DeployCommand {
deploy := s.deployCommandForState()
deploy.NewDeployAPI = func() (DeployAPI, error) {
deploy.NewDeployAPI = func() (deployer.DeployerAPI, error) {
return s.fakeAPI, nil
}
deploy.NewModelConfigAPI = func(api base.APICallCloser) ModelConfigGetter {
Expand Down Expand Up @@ -776,6 +776,8 @@ func (s *DeploySuite) TestDeployBundleWithChannel(c *gc.C) {
NumUnits: 1,
}).Returns([]string{"ubuntu-lite/0"}, error(nil))

s.fakeAPI.Call("ListSpaces").Returns([]params.Space{{Name: "alpha", Id: "0"}}, error(nil))

deploy := s.deployCommand()
bundlePath := testcharms.RepoWithSeries("bionic").ClonedBundleDirPath(c.MkDir(), "basic")
_, err := cmdtesting.RunCommand(c, modelcmd.Wrap(deploy), bundlePath, "--channel", "edge")
Expand Down Expand Up @@ -836,6 +838,8 @@ func (s *DeploySuite) TestDeployBundlesRequiringTrust(c *gc.C) {
NumUnits: 1,
}).Returns([]string{"aws-integrator/0"}, error(nil))

s.fakeAPI.Call("ListSpaces").Returns([]params.Space{{Name: "alpha", Id: "0"}}, error(nil))

// The second charm from the bundle does not require trust so no
// additional configuration should be injected
ubURL := charm.MustParseURL("cs:~jameinel/ubuntu-lite-7")
Expand Down Expand Up @@ -905,6 +909,8 @@ func (s *DeploySuite) TestDeployBundleWithOffers(c *gc.C) {
[]string{"controller.my-offer"},
).Returns(nil)

s.fakeAPI.Call("ListSpaces").Returns([]params.Space{{Name: "alpha", Id: "0"}}, error(nil))

deploy := s.deployCommand()
bundlePath := testcharms.RepoWithSeries("bionic").ClonedBundleDirPath(c.MkDir(), "apache2-with-offers")
_, err := cmdtesting.RunCommand(c, modelcmd.Wrap(deploy), bundlePath)
Expand Down Expand Up @@ -985,6 +991,8 @@ func (s *DeploySuite) TestDeployBundleWithSAAS(c *gc.C) {
error(nil),
)

s.fakeAPI.Call("ListSpaces").Returns([]params.Space{{Name: "alpha", Id: "0"}}, error(nil))

deploy := s.deployCommand()
bundlePath := testcharms.RepoWithSeries("bionic").ClonedBundleDirPath(c.MkDir(), "wordpress-with-saas")
_, err = cmdtesting.RunCommand(c, modelcmd.Wrap(deploy), bundlePath)
Expand All @@ -993,7 +1001,7 @@ func (s *DeploySuite) TestDeployBundleWithSAAS(c *gc.C) {

type CAASDeploySuiteBase struct {
jujutesting.IsolationSuite
DeployAPI
deployer.DeployerAPI
Store *jujuclient.MemStore
DeployResources resourceadapters.DeployResourcesFunc

Expand Down Expand Up @@ -1055,7 +1063,7 @@ func (s *CAASDeploySuiteBase) makeCharmDir(c *gc.C, cloneCharm string) *charm.Ch

func (s *CAASDeploySuiteBase) runDeploy(c *gc.C, fakeAPI *fakeDeployAPI, args ...string) (*cmd.Context, error) {
deployCmd := &DeployCommand{
NewDeployAPI: func() (DeployAPI, error) {
NewDeployAPI: func() (deployer.DeployerAPI, error) {
return fakeAPI, nil
},
DeployResources: s.DeployResources,
Expand Down Expand Up @@ -2287,7 +2295,7 @@ func (s *ParseMachineMapSuite) TestErrors(c *gc.C) {

type DeployUnitTestSuite struct {
jujutesting.IsolationSuite
DeployAPI
deployer.DeployerAPI
deployer *mocks.MockDeployer
factory *mocks.MockDeployerFactory
}
Expand Down Expand Up @@ -2539,7 +2547,7 @@ func newWrappedDeployCommandForTest(fakeApi *fakeDeployAPI) modelcmd.ModelComman
// newDeployCommandForTest returns a command to deploy applications.
func newDeployCommandForTest(fakeAPI *fakeDeployAPI) *DeployCommand {
deployCmd := &DeployCommand{
NewDeployAPI: func() (DeployAPI, error) {
NewDeployAPI: func() (deployer.DeployerAPI, error) {
return fakeAPI, nil
},
DeployResources: func(
Expand All @@ -2558,7 +2566,7 @@ func newDeployCommandForTest(fakeAPI *fakeDeployAPI) *DeployCommand {
},
}
if fakeAPI == nil {
deployCmd.NewDeployAPI = func() (DeployAPI, error) {
deployCmd.NewDeployAPI = func() (deployer.DeployerAPI, error) {
apiRoot, err := deployCmd.ModelCommandBase.NewAPIRoot()
if err != nil {
return nil, errors.Trace(err)
Expand Down Expand Up @@ -2623,10 +2631,10 @@ func newDeployCommandForTest(fakeAPI *fakeDeployAPI) *DeployCommand {
}

// fakeDeployAPI is a mock of the API used by the deploy command. It's
// a little muddled at the moment, but as the DeployAPI interface is
// a little muddled at the moment, but as the deployer.DeployerAPI interface is
// sharpened, this will become so as well.
type fakeDeployAPI struct {
DeployAPI
deployer.DeployerAPI
*store.CharmStoreAdaptor
*jujutesting.CallMocker
planURL string
Expand Down
21 changes: 18 additions & 3 deletions cmd/juju/application/deployer/bundle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/juju/charm/v8"
"github.com/juju/cmd/v3"
"github.com/juju/collections/set"
"github.com/juju/errors"

"github.com/juju/juju/api/client/application"
Expand Down Expand Up @@ -163,7 +164,10 @@ Please repeat the deploy command with the --trust argument if you consent to tru
}
}
}
spec := d.makeBundleDeploySpec(ctx, deployAPI)
spec, err := d.makeBundleDeploySpec(ctx, deployAPI)
if err != nil {
return errors.Trace(err)
}

// TODO(ericsnow) Do something with the CS macaroons that were returned?
// Deploying bundles does not allow the use force, it's expected that the
Expand All @@ -174,7 +178,7 @@ Please repeat the deploy command with the --trust argument if you consent to tru
return nil
}

func (d *deployBundle) makeBundleDeploySpec(ctx *cmd.Context, apiRoot DeployerAPI) bundleDeploySpec {
func (d *deployBundle) makeBundleDeploySpec(ctx *cmd.Context, apiRoot DeployerAPI) (bundleDeploySpec, error) {
// set the consumer details API factory method on the spec, so it makes it
// possible to communicate with other controllers, that are found within
// the local cache.
Expand All @@ -188,6 +192,16 @@ func (d *deployBundle) makeBundleDeploySpec(ctx *cmd.Context, apiRoot DeployerAP
return d.newConsumeDetailsAPI(url)
}

knownSpaces, err := apiRoot.ListSpaces()
if err != nil {
return bundleDeploySpec{}, errors.Trace(err)
}

knownSpaceNames := set.NewStrings()
for _, space := range knownSpaces {
knownSpaceNames.Add(space.Name)
}

return bundleDeploySpec{
ctx: ctx,
filesystem: d.model.Filesystem(),
Expand All @@ -213,7 +227,8 @@ func (d *deployBundle) makeBundleDeploySpec(ctx *cmd.Context, apiRoot DeployerAP
targetModelUUID: d.targetModelUUID,
controllerName: d.controllerName,
accountUser: d.accountUser,
}
knownSpaceNames: knownSpaceNames,
}, nil
}

type localBundle struct {
Expand Down
21 changes: 21 additions & 0 deletions cmd/juju/application/deployer/bundlehandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ type bundleDeploySpec struct {
targetModelUUID string
controllerName string
accountUser string

knownSpaceNames set.Strings
}

// deployBundle deploys the given bundle data using the given API client and
Expand Down Expand Up @@ -187,6 +189,10 @@ type bundleHandler struct {
// each origin.
origins map[charm.URL]map[string]commoncharm.Origin

// knownSpaceNames is a set of the names of existing spaces an application
// can bind to
knownSpaceNames set.Strings

// watcher holds an environment mega-watcher used to keep the environment
// status up to date.
watcher api.AllWatch
Expand Down Expand Up @@ -243,6 +249,7 @@ func makeBundleHandler(defaultCharmSchema charm.Schema, bundleData *charm.Bundle
unitStatus: make(map[string]string),
macaroons: make(map[charm.URL]*macaroon.Macaroon),
origins: make(map[charm.URL]map[string]commoncharm.Origin),
knownSpaceNames: spec.knownSpaceNames,

targetModelName: spec.targetModelName,
targetModelUUID: spec.targetModelUUID,
Expand Down Expand Up @@ -801,6 +808,11 @@ func (h *bundleHandler) addApplication(change *bundlechanges.AddApplicationChang
return errors.Errorf("unexpected application charm URL %q", p.Charm)
}

err = verifyEndpointBindings(p.EndpointBindings, h.knownSpaceNames)
if err != nil {
return errors.Trace(err)
}

channel, err := constructNormalizedChannel(p.Channel)
if err != nil {
// This should never happen, essentially we have a charm url that has
Expand Down Expand Up @@ -1711,3 +1723,12 @@ func addCharmConstraintsParser(s string) bundlechanges.ArchConstraint {
constraints: s,
}
}

func verifyEndpointBindings(endpointBindings map[string]string, knownSpaceNames set.Strings) error {
for _, spaceName := range endpointBindings {
if !knownSpaceNames.Contains(spaceName) {
return errors.NotFoundf("space %q", spaceName)
}
}
return nil
}
4 changes: 4 additions & 0 deletions cmd/juju/application/deployer/interface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ type DeployerAPI interface {
ModelAPI
OfferAPI

// PlanURL returns the configured URL prefix for the metering plan API.
PlanURL() string

ListSpaces() ([]apiparams.Space, error)
Deploy(application.DeployArgs) error
Status(patterns []string) (*apiparams.FullStatus, error)
WatchAll() (api.AllWatch, error)
Expand Down
29 changes: 29 additions & 0 deletions cmd/juju/application/deployer/mocks/deploy_mock.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f1454a9

Please sign in to comment.