Skip to content

Commit

Permalink
Change charm/bundle resolver signatures to include preferred channel
Browse files Browse the repository at this point in the history
  • Loading branch information
achilleasa committed Aug 7, 2019
1 parent b00f37b commit 1215605
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 33 deletions.
5 changes: 2 additions & 3 deletions charmstore/fakeclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,8 @@ func (r Repository) Resolve(ref *charm.URL) (canonRef *charm.URL, supportedSerie
// ResolveWithChannel disambiguates a charm to a specific revision.
//
// Part of the cmd/juju/application.DeployAPI interface
func (r Repository) ResolveWithChannel(ref *charm.URL) (*charm.URL, params.Channel, []string, error) {
canonRef, supportedSeries, err := r.Resolve(ref)
return canonRef, r.channel, supportedSeries, err
func (r Repository) ResolveWithPreferredChannel(ref *charm.URL, preferredChannel params.Channel) (*charm.URL, params.Channel, []string, error) {
return r.addRevision(ref), preferredChannel, []string{"trusty", "wily", "quantal"}, nil
}

// Get retrieves a charm from the repository.
Expand Down
10 changes: 7 additions & 3 deletions cmd/juju/application/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,16 @@ func (h *bundleHandler) resolveCharmsAndEndpoints() error {
continue
}

h.ctx.Infof("Resolving charm: %s", spec.Charm)
var fromChannel string
if spec.Channel != "" {
fromChannel = fmt.Sprintf(" from channel: %s", spec.Channel)
}
h.ctx.Infof("Resolving charm: %s%s", spec.Charm, fromChannel)
ch, err := charm.ParseURL(spec.Charm)
if err != nil {
return errors.Trace(err)
}
url, _, _, err := resolveCharm(h.api.ResolveWithChannel, ch)
url, _, _, err := resolveCharm(h.api.ResolveWithPreferredChannel, ch, csparams.Channel(spec.Channel))
if err != nil {
return errors.Annotatef(err, "cannot resolve URL %q", spec.Charm)
}
Expand Down Expand Up @@ -515,7 +519,7 @@ func (h *bundleHandler) addCharm(change *bundlechanges.AddCharmChange) error {
return errors.Trace(err)
}

url, channel, _, err := resolveCharm(h.api.ResolveWithChannel, ch)
url, channel, _, err := resolveCharm(h.api.ResolveWithPreferredChannel, ch, csparams.Channel(p.Channel))
if err != nil {
return errors.Annotatef(err, "cannot resolve URL %q", p.Charm)
}
Expand Down
4 changes: 1 addition & 3 deletions cmd/juju/application/bundlediff.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,7 @@ func (c *bundleDiffCommand) bundleDataSource(ctx *cmd.Context) (charm.BundleData
if err != nil {
return nil, errors.Trace(err)
}
bundleURL, _, err := resolveBundleURL(
charmStore, c.bundle,
)
bundleURL, _, err := resolveBundleURL(charmStore, c.bundle, c.channel)
if err != nil && !errors.IsNotValid(err) {
return nil, errors.Trace(err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/juju/application/bundlediff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,8 @@ type mockCharmStore struct {
bundle *mockBundle
}

func (s *mockCharmStore) ResolveWithChannel(url *charm.URL) (*charm.URL, csparams.Channel, []string, error) {
s.stub.AddCall("ResolveWithChannel", url)
func (s *mockCharmStore) ResolveWithPreferredChannel(url *charm.URL, preferredChannel csparams.Channel) (*charm.URL, csparams.Channel, []string, error) {
s.stub.AddCall("ResolveWithPreferredChannel", url, preferredChannel)
return s.url, s.channel, s.series, s.stub.NextErr()
}

Expand Down
22 changes: 12 additions & 10 deletions cmd/juju/application/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ type DeployAPI interface {
Deploy(application.DeployArgs) error
Status(patterns []string) (*apiparams.FullStatus, error)

ResolveWithChannel(*charm.URL) (*charm.URL, params.Channel, []string, error)
ResolveWithPreferredChannel(*charm.URL, params.Channel) (*charm.URL, params.Channel, []string, error)

GetBundle(*charm.URL) (charm.Bundle, error)

Expand Down Expand Up @@ -175,7 +175,7 @@ type modelConfigClient struct {
type charmrepoForDeploy interface {
Get(charmURL *charm.URL) (charm.Charm, error)
GetBundle(bundleURL *charm.URL) (charm.Bundle, error)
ResolveWithChannel(*charm.URL) (*charm.URL, params.Channel, []string, error)
ResolveWithPreferredChannel(*charm.URL, params.Channel) (*charm.URL, params.Channel, []string, error)
}

type charmRepoClient struct {
Expand Down Expand Up @@ -246,13 +246,13 @@ func (a *deployAPIAdapter) Deploy(args application.DeployArgs) error {
return errors.Trace(a.applicationClient.Deploy(args))
}

func (a *deployAPIAdapter) Resolve(cfg *config.Config, url *charm.URL) (
func (a *deployAPIAdapter) Resolve(cfg *config.Config, url *charm.URL, preferredChannel params.Channel) (
*charm.URL,
params.Channel,
[]string,
error,
) {
return resolveCharm(a.charmRepoClient.ResolveWithChannel, url)
return resolveCharm(a.charmRepoClient.ResolveWithPreferredChannel, url, preferredChannel)
}

func (a *deployAPIAdapter) Get(url *charm.URL) (charm.Charm, error) {
Expand Down Expand Up @@ -1477,22 +1477,22 @@ func (c *DeployCommand) maybeReadLocalCharm(apiRoot DeployAPI) (deployFn, error)
// URLResolver is the part of charmrepo.Charmstore that we need to
// resolve a charm url.
type URLResolver interface {
ResolveWithChannel(*charm.URL) (*charm.URL, params.Channel, []string, error)
ResolveWithPreferredChannel(*charm.URL, params.Channel) (*charm.URL, params.Channel, []string, error)
}

// resolveBundleURL tries to interpret maybeBundle as a charmstore
// bundle. If it turns out to be a bundle, the resolved URL and
// channel are returned. If it isn't but there wasn't a problem
// checking it, it returns a nil charm URL.
func resolveBundleURL(store URLResolver, maybeBundle string) (*charm.URL, params.Channel, error) {
func resolveBundleURL(store URLResolver, maybeBundle string, preferredChannel params.Channel) (*charm.URL, params.Channel, error) {
userRequestedURL, err := charm.ParseURL(maybeBundle)
if err != nil {
return nil, "", errors.Trace(err)
}

// Charm or bundle has been supplied as a URL so we resolve and
// deploy using the store.
storeCharmOrBundleURL, channel, _, err := resolveCharm(store.ResolveWithChannel, userRequestedURL)
storeCharmOrBundleURL, channel, _, err := resolveCharm(store.ResolveWithPreferredChannel, userRequestedURL, preferredChannel)
if err != nil {
return nil, "", errors.Trace(err)
}
Expand All @@ -1508,7 +1508,7 @@ func resolveBundleURL(store URLResolver, maybeBundle string) (*charm.URL, params

func (c *DeployCommand) maybeReadCharmstoreBundleFn(apiRoot DeployAPI) func() (deployFn, error) {
return func() (deployFn, error) {
bundleURL, channel, err := resolveBundleURL(apiRoot, c.CharmOrBundle)
bundleURL, channel, err := resolveBundleURL(apiRoot, c.CharmOrBundle, c.Channel)
if charm.IsUnsupportedSeriesError(errors.Cause(err)) {
return nil, errors.Errorf("%v. Use --force to deploy the charm anyway.", err)
}
Expand Down Expand Up @@ -1587,9 +1587,11 @@ func (c *DeployCommand) charmStoreCharm() (deployFn, error) {
return errors.Trace(err)
}

// Charm or bundle has been supplied as a URL so we resolve and deploy using the store.
// Charm or bundle has been supplied as a URL so we resolve and
// deploy using the store but pass in the channel command line
// argument so users can target a specific channel.
storeCharmOrBundleURL, channel, supportedSeries, err := resolveCharm(
apiRoot.ResolveWithChannel, userRequestedURL,
apiRoot.ResolveWithPreferredChannel, userRequestedURL, c.Channel,
)
if charm.IsUnsupportedSeriesError(err) {
return errors.Errorf("%v. Use --force to deploy the charm anyway.", err)
Expand Down
6 changes: 3 additions & 3 deletions cmd/juju/application/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2570,13 +2570,13 @@ func (f *fakeDeployAPI) ModelGet() (map[string]interface{}, error) {
return results[0].(map[string]interface{}), jujutesting.TypeAssertError(results[1])
}

func (f *fakeDeployAPI) ResolveWithChannel(url *charm.URL) (
func (f *fakeDeployAPI) ResolveWithPreferredChannel(url *charm.URL, preferredChannel csclientparams.Channel) (
*charm.URL,
csclientparams.Channel,
[]string,
error,
) {
results := f.MethodCall(f, "ResolveWithChannel", url)
results := f.MethodCall(f, "ResolveWithPreferredChannel", url, preferredChannel)

return results[0].(*charm.URL),
results[1].(csclientparams.Channel),
Expand Down Expand Up @@ -2805,7 +2805,7 @@ func withCharmRepoResolvable(
fakeAPI *fakeDeployAPI,
url *charm.URL,
) {
fakeAPI.Call("ResolveWithChannel", url).Returns(
fakeAPI.Call("ResolveWithPreferredChannel", url, csclientparams.NoChannel).Returns(
url,
csclientparams.Channel(""),
[]string{"bionic"}, // Supported series
Expand Down
12 changes: 8 additions & 4 deletions cmd/juju/application/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,24 @@ type SeriesConfig interface {
DefaultSeries() (string, bool)
}

// ResolveCharmFunc is the type of a function that resolves a charm URL.
// ResolveCharmFunc is the type of a function that resolves a charm URL with
// an optionally specified preferred channel.
type ResolveCharmFunc func(
resolveWithChannel func(*charm.URL) (*charm.URL, csparams.Channel, []string, error),
resolveWithChannel func(*charm.URL, csparams.Channel) (*charm.URL, csparams.Channel, []string, error),
url *charm.URL,
preferredChannel csparams.Channel,
) (*charm.URL, csparams.Channel, []string, error)

func resolveCharm(
resolveWithChannel func(*charm.URL) (*charm.URL, csparams.Channel, []string, error),
resolveWithChannel func(*charm.URL, csparams.Channel) (*charm.URL, csparams.Channel, []string, error),
url *charm.URL,
preferredChannel csparams.Channel,
) (*charm.URL, csparams.Channel, []string, error) {
if url.Schema != "cs" {
return nil, csparams.NoChannel, nil, errors.Errorf("unknown schema for charm URL %q", url)
}
resultURL, channel, supportedSeries, err := resolveWithChannel(url)

resultURL, channel, supportedSeries, err := resolveWithChannel(url, preferredChannel)
if err != nil {
return nil, csparams.NoChannel, nil, errors.Trace(err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/juju/application/upgradecharm.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ func (c *upgradeCharmCommand) addCharm(
}

// Charm has been supplied as a URL so we resolve and deploy using the store.
newURL, channel, supportedSeries, err := c.ResolveCharm(charmRepo.ResolveWithChannel, refURL)
newURL, channel, supportedSeries, err := c.ResolveCharm(charmRepo.ResolveWithPreferredChannel, refURL, c.Channel)
if err != nil {
return id, nil, errors.Trace(err)
}
Expand Down
9 changes: 5 additions & 4 deletions cmd/juju/application/upgradecharm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ func (s *UpgradeCharmSuite) SetUpTest(c *gc.C) {
}

s.resolveCharm = func(
resolveWithChannel func(*charm.URL) (*charm.URL, csclientparams.Channel, []string, error),
resolveWithChannel func(*charm.URL, csclientparams.Channel) (*charm.URL, csclientparams.Channel, []string, error),
url *charm.URL,
preferredChannel csclientparams.Channel,
) (*charm.URL, csclientparams.Channel, []string, error) {
s.AddCall("ResolveCharm", resolveWithChannel, url)
s.AddCall("ResolveCharm", resolveWithChannel, url, preferredChannel)
if err := s.NextErr(); err != nil {
return nil, csclientparams.NoChannel, nil, err
}
Expand Down Expand Up @@ -656,13 +657,13 @@ var upgradeCharmAuthorizationTests = []struct {
uploadURL: "cs:~bob/trusty/wordpress5-10",
switchURL: "cs:~bob/trusty/wordpress5",
readPermUser: "bob",
expectError: `cannot resolve charm URL "cs:~bob/trusty/wordpress5": cannot get "/~bob/trusty/wordpress5/meta/any\?include=id&include=supported-series&include=published": access denied for user "client-username"`,
expectError: `cannot resolve charm URL "cs:~bob/trusty/wordpress5": cannot get "/~bob/trusty/wordpress5/meta/any\?channel=stable&include=id&include=supported-series&include=published": access denied for user "client-username"`,
}, {
about: "non-public charm, fully resolved, access denied",
uploadURL: "cs:~bob/trusty/wordpress6-47",
switchURL: "cs:~bob/trusty/wordpress6-47",
readPermUser: "bob",
expectError: `cannot resolve charm URL "cs:~bob/trusty/wordpress6-47": cannot get "/~bob/trusty/wordpress6-47/meta/any\?include=id&include=supported-series&include=published": access denied for user "client-username"`,
expectError: `cannot resolve charm URL "cs:~bob/trusty/wordpress6-47": cannot get "/~bob/trusty/wordpress6-47/meta/any\?channel=stable&include=id&include=supported-series&include=published": access denied for user "client-username"`,
}}

func (s *UpgradeCharmCharmStoreStateSuite) TestUpgradeCharmAuthorization(c *gc.C) {
Expand Down

0 comments on commit 1215605

Please sign in to comment.