Include resource revision when downloading for charmhub.

Fix LP 1982406. Update the test to use a non zero revision. Setting the value to a default for int was a bad idea and masked the problem.
addyess · Jul 21, 2022 · b6ecfa7 · b6ecfa7
1 parent 0761c60
commit b6ecfa7
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 26 deletions.
diff --git a/resource/charmhub.go b/resource/charmhub.go
@@ -7,12 +7,10 @@ import (
 	"context"
 	"net/url"
 
+	charmresource "github.com/juju/charm/v8/resource"
 	"github.com/juju/errors"
 	"github.com/kr/pretty"
 
-	"github.com/juju/charm/v8"
-	charmresource "github.com/juju/charm/v8/resource"
-
 	"github.com/juju/juju/charmhub"
 	"github.com/juju/juju/charmhub/transport"
 	corelogger "github.com/juju/juju/core/logger"
@@ -79,30 +77,26 @@ func (ch *CharmHubClient) GetResource(req ResourceRequest) (ResourceData, error)
 	ch.logger.Tracef("GetResource(%s)", pretty.Sprint(req))
 	var data ResourceData
 
+	// GetResource is called after a charm is installed, therefore the
+	// origin must have an ID. Error if not.
 	origin := req.CharmID.Origin
-
-	stChannel := origin.Channel
-	if stChannel == nil {
-		return data, errors.Errorf("missing channel for %q", req.CharmID.URL.Name)
-	}
-	channel, err := charm.MakeChannel(stChannel.Track, stChannel.Risk, stChannel.Branch)
-	if err != nil {
-		return data, errors.Trace(err)
-	}
-
-	if req.CharmID.URL == nil {
-		return data, errors.Errorf("missing charm url for resource %q", req.Name)
+	if origin.Revision == nil {
+		return data, errors.BadRequestf("empty charm origin revision")
 	}
 
-	cfg, err := charmhub.DownloadOneFromChannel(origin.ID, channel.String(), charmhub.RefreshBase{
-		Architecture: origin.Platform.Architecture,
-		Name:         origin.Platform.OS,
-		Channel:      origin.Platform.Series,
-	})
+	// The charm revision isn't really required here, just handy for
+	// getting the correct resource revision. Using a channel would
+	// limit resource revisions found. The resource revision is set
+	// during deploy when a resolving resources for add pending resources.
+	// This also closes a timing window where a charm and resource
+	// is updated in the channel in between deploy and resource use.
+	cfg, err := charmhub.DownloadOneFromRevision(origin.ID, *origin.Revision)
 	if err != nil {
 		return data, errors.Trace(err)
 	}
-
+	if newCfg, ok := charmhub.AddResource(cfg, req.Name, req.Revision); ok {
+		cfg = newCfg
+	}
 	refreshResp, err := ch.client.Refresh(context.TODO(), cfg)
 	if err != nil {
 		return data, errors.Trace(err)

diff --git a/resource/charmhub_test.go b/resource/charmhub_test.go
@@ -34,12 +34,14 @@ func (s *CharmHubSuite) TestGetResource(c *gc.C) {
 
 	cl := s.newCharmHubClient()
 	curl, _ := charm.ParseURL("ch:postgresql")
+	rev := 42
 	result, err := cl.GetResource(resource.ResourceRequest{
 		CharmID: resource.CharmID{
 			URL: curl,
 			Origin: state.CharmOrigin{
-				ID:      "mycharmhubid",
-				Channel: &state.Channel{Risk: "stable"},
+				ID:       "mycharmhubid",
+				Channel:  &state.Channel{Risk: "stable"},
+				Revision: &rev,
 				Platform: &state.Platform{
 					Architecture: "amd64",
 					OS:           "ubuntu",
@@ -48,7 +50,7 @@ func (s *CharmHubSuite) TestGetResource(c *gc.C) {
 			},
 		},
 		Name:     "wal-e",
-		Revision: 0,
+		Revision: 8,
 	})
 	c.Assert(err, jc.ErrorIsNil)
 
@@ -59,7 +61,7 @@ func (s *CharmHubSuite) TestGetResource(c *gc.C) {
 			Type: 1,
 		},
 		Origin:      2,
-		Revision:    0,
+		Revision:    8,
 		Fingerprint: fp,
 		Size:        0,
 	})
@@ -90,7 +92,7 @@ func (s *CharmHubSuite) expectRefresh() {
 							Size:       0,
 							URL:        "https://api.staging.charmhub.io/api/v1/resources/download/charm_jmeJLrjWpJX9OglKSeUHCwgyaCNuoQjD.wal-e_0"},
 						Name:     "wal-e",
-						Revision: 0,
+						Revision: 8,
 						Type:     "file",
 					},
 				},