Allow for install by revision to not require a base, and find the base

data in the response. Also allow for resources to be specified by revision when installing by revision.
sdg0919 · Oct 22, 2021 · de31263 · de31263
1 parent 776c01e
commit de31263
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 59 deletions.
diff --git a/charmhub/refresh.go b/charmhub/refresh.go
@@ -278,8 +278,9 @@ type executeOne struct {
 	Base     RefreshBase
 	// instanceKey is a private unique key that we construct for CharmHub API
 	// asynchronous calls.
-	action      Action
-	instanceKey string
+	action            Action
+	instanceKey       string
+	resourceRevisions []transport.RefreshResourceRevision
 }
 
 // InstanceKey returns the underlying instance key.
@@ -415,19 +416,26 @@ func (c executeOne) Build() (transport.RefreshRequest, error) {
 		name = &c.Name
 	}
 
-	return transport.RefreshRequest{
+	req := transport.RefreshRequest{
 		// Context is required here, even if it looks optional.
 		Context: []transport.RefreshRequestContext{},
 		Actions: []transport.RefreshRequestAction{{
-			Action:      string(c.action),
-			InstanceKey: c.instanceKey,
-			ID:          id,
-			Name:        name,
-			Revision:    c.Revision,
-			Channel:     c.Channel,
-			Base:        &base,
+			Action:            string(c.action),
+			InstanceKey:       c.instanceKey,
+			ID:                id,
+			Name:              name,
+			Base:              &base,
+			ResourceRevisions: c.resourceRevisions,
 		}},
-	}, nil
+		Fields: []string{"bases", "download", "id", "revision", "version", "resources"},
+	}
+	if c.Revision != nil {
+		req.Actions[0].Revision = c.Revision
+	} else if c.Channel != nil {
+		req.Actions[0].Revision = c.Revision
+		req.Actions[0].Channel = c.Channel
+	}
+	return req, nil
 }
 
 // Ensure that the request back contains the information we requested.
@@ -459,6 +467,23 @@ func (c executeOne) String() string {
 		c.action, c.instanceKey, using, revision, channel, c.Base)
 }
 
+// AddResource adds resource revision data to a executeOne config.
+// Used for install by revision.
+func AddResource(config RefreshConfig, name string, revision int) (RefreshConfig, bool) {
+	c, ok := config.(executeOne)
+	if !ok {
+		return config, false
+	}
+	if len(c.resourceRevisions) == 0 {
+		c.resourceRevisions = make([]transport.RefreshResourceRevision, 0)
+	}
+	c.resourceRevisions = append(c.resourceRevisions, transport.RefreshResourceRevision{
+		Name:     name,
+		Revision: revision,
+	})
+	return c, true
+}
+
 type refreshMany struct {
 	Configs []RefreshConfig
 }
@@ -489,7 +514,7 @@ func (c refreshMany) Build() (transport.RefreshRequest, error) {
 		}
 		result.Context = append(result.Context, req.Context...)
 		result.Actions = append(result.Actions, req.Actions...)
-
+		result.Fields = append(result.Fields, req.Fields...)
 	}
 	return result, nil
 }

diff --git a/charmhub/refresh_integration_test.go b/charmhub/refresh_integration_test.go
@@ -23,28 +23,16 @@ var _ = gc.Suite(&RefreshClientSuite{})
 
 func (s *RefreshClientSuite) TestLiveRefreshRequest(c *gc.C) {
 	c.Skip("It works on the cli with curl using the created req.Body.Reader data.")
-	logger := &FakeLogger{}
 
-	config, err := CharmHubConfig(logger)
-	c.Assert(err, jc.ErrorIsNil)
-	basePath, err := config.BasePath()
-	c.Assert(err, jc.ErrorIsNil)
-
-	refreshPath, err := basePath.Join("refresh")
-	c.Assert(err, jc.ErrorIsNil)
-
-	apiRequester := NewAPIRequester(DefaultHTTPTransport(logger), logger)
-	restClient := NewHTTPRESTClient(apiRequester, nil)
-
-	client := NewRefreshClient(refreshPath, restClient, logger)
+	client := refreshClient(c)
 
 	charmConfig, err := RefreshOne("wordpress", 0, "latest/stable", RefreshBase{
 		Channel:      "18.04",
 		Name:         "ubuntu",
 		Architecture: "amd64",
 	})
 	c.Assert(err, jc.ErrorIsNil)
-	charmConfig = DefineID(c, charmConfig, "mny7cXFEre1BFZQnXyyyIhCHBpiLTRNi")
+	charmConfig = defineID(c, charmConfig, "mny7cXFEre1BFZQnXyyyIhCHBpiLTRNi")
 
 	response, err := client.Refresh(context.TODO(), charmConfig)
 	c.Assert(err, jc.ErrorIsNil)
@@ -54,36 +42,24 @@ func (s *RefreshClientSuite) TestLiveRefreshRequest(c *gc.C) {
 
 func (s *RefreshClientSuite) TestLiveRefreshManyRequest(c *gc.C) {
 	c.Skip("It works on the cli with curl using the created req.Body.Reader data.")
-	logger := &FakeLogger{}
 
-	config, err := CharmHubConfig(logger)
-	c.Assert(err, jc.ErrorIsNil)
-	basePath, err := config.BasePath()
-	c.Assert(err, jc.ErrorIsNil)
-
-	refreshPath, err := basePath.Join("refresh")
-	c.Assert(err, jc.ErrorIsNil)
-
-	apiRequester := NewAPIRequester(DefaultHTTPTransport(logger), logger)
-	restClient := NewHTTPRESTClient(apiRequester, nil)
-
-	client := NewRefreshClient(refreshPath, restClient, logger)
+	client := refreshClient(c)
 
 	wordpressConfig, err := RefreshOne("wordpress", 0, "latest/stable", RefreshBase{
 		Name:         "ubuntu",
 		Channel:      "18.04",
 		Architecture: "amd64",
 	})
 	c.Assert(err, jc.ErrorIsNil)
-	wordpressConfig = DefineID(c, wordpressConfig, "mny7cXFEre1BFZQnXyyyIhCHBpiLTRNi")
+	wordpressConfig = defineID(c, wordpressConfig, "mny7cXFEre1BFZQnXyyyIhCHBpiLTRNi")
 
 	mysqlConfig, err := RefreshOne("mysql", 58, "latest/candidate", RefreshBase{
 		Name:         "ubuntu",
 		Channel:      "18.04",
 		Architecture: "amd64",
 	})
 	c.Assert(err, jc.ErrorIsNil)
-	mysqlConfig = DefineID(c, mysqlConfig, "XcESKcQ4R00AM6dOUpCl9YY4QpAEjnXe")
+	mysqlConfig = defineID(c, mysqlConfig, "XcESKcQ4R00AM6dOUpCl9YY4QpAEjnXe")
 
 	charmsConfig := RefreshMany(wordpressConfig, mysqlConfig)
 
@@ -95,34 +71,60 @@ func (s *RefreshClientSuite) TestLiveRefreshManyRequest(c *gc.C) {
 }
 
 func (s *RefreshClientSuite) TestLiveInstallRequest(c *gc.C) {
-	logger := &FakeLogger{}
+	client := refreshClient(c)
 
-	config, err := CharmHubConfig(logger)
+	charmConfig, err := InstallOneFromRevision("wordpress", 0, RefreshBase{
+		Name:         "ubuntu",
+		Channel:      "18.04",
+		Architecture: "amd64",
+	})
 	c.Assert(err, jc.ErrorIsNil)
-	basePath, err := config.BasePath()
+
+	response, err := client.Refresh(context.TODO(), charmConfig)
 	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(response[0].Result, gc.Equals, "install", gc.Commentf("%s", pretty.Sprint(response)))
+}
 
-	refreshPath, err := basePath.Join("refresh")
+func (s *RefreshClientSuite) TestLiveInstallRequestNoBase(c *gc.C) {
+	client := refreshClient(c)
+
+	charmConfig, err := InstallOneFromRevision("wordpress", 0, RefreshBase{
+		Architecture: "NA",
+		Name:         "NA",
+		Channel:      "NA",
+	})
 	c.Assert(err, jc.ErrorIsNil)
 
-	apiRequester := NewAPIRequester(DefaultHTTPTransport(logger), logger)
-	restClient := NewHTTPRESTClient(apiRequester, nil)
+	response, err := client.Refresh(context.TODO(), charmConfig)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(response[0].Result, gc.Equals, "install", gc.Commentf("%s", pretty.Sprint(response)))
+	c.Assert(len(response[0].Entity.Bases), jc.GreaterThan, 0)
+}
 
-	client := NewRefreshClient(refreshPath, restClient, logger)
+func (s *RefreshClientSuite) TestLiveInstallRequestWithResourceRevisions(c *gc.C) {
+	client := refreshClient(c)
 
-	charmConfig, err := InstallOneFromRevision("wordpress", 0, RefreshBase{
-		Name:         "ubuntu",
-		Channel:      "18.04",
-		Architecture: "amd64",
+	charmConfig, err := InstallOneFromRevision("prometheus-ceph-exporter", 13, RefreshBase{
+		Architecture: "NA",
+		Name:         "NA",
+		Channel:      "NA",
 	})
 	c.Assert(err, jc.ErrorIsNil)
 
+	charmConfig, ok := AddResource(charmConfig, "core", 0)
+	c.Assert(ok, jc.IsTrue)
+	charmConfig, ok = AddResource(charmConfig, "prometheus-ceph-exporter", 0)
+	c.Assert(ok, jc.IsTrue)
+	charmConfig, ok = AddResource(charmConfig, "dashboards", 0)
+	c.Assert(ok, jc.IsTrue)
+
 	response, err := client.Refresh(context.TODO(), charmConfig)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(response[0].Result, gc.Equals, "install", gc.Commentf("%s", pretty.Sprint(response)))
+	c.Assert(len(response[0].Entity.Resources), gc.Equals, 3)
 }
 
-func DefineID(c *gc.C, config RefreshConfig, id string) RefreshConfig {
+func defineID(c *gc.C, config RefreshConfig, id string) RefreshConfig {
 	switch t := config.(type) {
 	case refreshOne:
 		t.ID = id
@@ -132,3 +134,20 @@ func DefineID(c *gc.C, config RefreshConfig, id string) RefreshConfig {
 	}
 	return nil
 }
+
+func refreshClient(c *gc.C) *RefreshClient{
+	logger := &FakeLogger{}
+
+	config, err := CharmHubConfig(logger)
+	c.Assert(err, jc.ErrorIsNil)
+	basePath, err := config.BasePath()
+	c.Assert(err, jc.ErrorIsNil)
+
+	refreshPath, err := basePath.Join("refresh")
+	c.Assert(err, jc.ErrorIsNil)
+
+	apiRequester := NewAPIRequester(DefaultHTTPTransport(logger), logger)
+	restClient := NewHTTPRESTClient(apiRequester, nil)
+
+	return NewRefreshClient(refreshPath, restClient, logger)
+}
diff --git a/charmhub/refresh_test.go b/charmhub/refresh_test.go
@@ -500,6 +500,7 @@ func (s *RefreshConfigSuite) TestInstallOneBuildRevision(c *gc.C) {
 				Architecture: arch.DefaultArchitecture,
 			},
 		}},
+		Fields: []string{"bases", "download", "id", "revision", "version", "resources"},
 	})
 }
 
@@ -531,6 +532,7 @@ func (s *RefreshConfigSuite) TestInstallOneBuildChannel(c *gc.C) {
 				Architecture: arch.DefaultArchitecture,
 			},
 		}},
+		Fields: []string{"bases", "download", "id", "revision", "version", "resources"},
 	})
 }
 
@@ -560,6 +562,7 @@ func (s *RefreshConfigSuite) TestInstallOneWithPartialPlatform(c *gc.C) {
 				Architecture: arch.DefaultArchitecture,
 			},
 		}},
+		Fields: []string{"bases", "download", "id", "revision", "version", "resources"},
 	})
 }
 
@@ -651,6 +654,7 @@ func (s *RefreshConfigSuite) TestDownloadOneFromChannelBuild(c *gc.C) {
 				Architecture: arch.DefaultArchitecture,
 			},
 		}},
+		Fields: []string{"bases", "download", "id", "revision", "version", "resources"},
 	})
 }
 
@@ -681,6 +685,7 @@ func (s *RefreshConfigSuite) TestDownloadOneFromChannelBuildK8s(c *gc.C) {
 				Architecture: arch.DefaultArchitecture,
 			},
 		}},
+		Fields: []string{"bases", "download", "id", "revision", "version", "resources"},
 	})
 }
 
@@ -799,6 +804,7 @@ func (s *RefreshConfigSuite) TestRefreshManyBuild(c *gc.C) {
 			},
 			Channel: &channel,
 		}},
+		Fields: []string{"bases", "download", "id", "revision", "version", "resources"},
 	})
 }
 

diff --git a/charmhub/transport/error.go b/charmhub/transport/error.go
@@ -58,6 +58,7 @@ const (
 	ErrorCodeAccessByRevisionNotAllowed        APIErrorCode = "access-by-revision-not-allowed"
 	ErrorCodeAPIError                          APIErrorCode = "api-error"
 	ErrorCodeBadArgument                       APIErrorCode = "bad-argument"
+	ErrorCodeCharmResourceNotFound             APIErrorCode = "charm-resource-not-found"
 	ErrorCodeChannelNotFound                   APIErrorCode = "channel-not-found"
 	ErrorCodeDeviceAuthorizationNeedsRefresh   APIErrorCode = "device-authorization-needs-refresh"
 	ErrorCodeDeviceServiceDisallowed           APIErrorCode = "device-service-disallowed"
@@ -69,6 +70,7 @@ const (
 	ErrorCodeInstanceKeyNotUnique              APIErrorCode = "instance-key-not-unique"
 	ErrorCodeInvalidChannel                    APIErrorCode = "invalid-channel"
 	ErrorCodeInvalidCharmBase                  APIErrorCode = "invalid-charm-base"
+	ErrorCodeInvalidCharmResource              APIErrorCode = "invalid-charm-resource"
 	ErrorCodeInvalidCohortKey                  APIErrorCode = "invalid-cohort-key"
 	ErrorCodeInvalidGrade                      APIErrorCode = "invalid-grade"
 	ErrorCodeInvalidMetric                     APIErrorCode = "invalid-metric"

diff --git a/charmhub/transport/refresh.go b/charmhub/transport/refresh.go
@@ -14,6 +14,7 @@ type RefreshRequest struct {
 	Context []RefreshRequestContext `json:"context"`
 	Actions []RefreshRequestAction  `json:"actions"`
 	Metrics RequestMetrics          `json:"metrics,omitempty"`
+	Fields  []string                `json:"fields,omitempty"`
 }
 
 // RequestMetrics are a map of key value pairs of metrics for the controller
@@ -44,12 +45,13 @@ type RefreshRequestAction struct {
 	// InstanceKey should be unique for every action, as results may not be
 	// ordered in the same way, so it is expected to use this to ensure
 	// completeness and ordering.
-	InstanceKey string  `json:"instance-key"`
-	ID          *string `json:"id,omitempty"`
-	Name        *string `json:"name,omitempty"`
-	Channel     *string `json:"channel,omitempty"`
-	Revision    *int    `json:"revision,omitempty"`
-	Base        *Base   `json:"base,omitempty"`
+	InstanceKey       string                    `json:"instance-key"`
+	ID                *string                   `json:"id,omitempty"`
+	Name              *string                   `json:"name,omitempty"`
+	Channel           *string                   `json:"channel,omitempty"`
+	Revision          *int                      `json:"revision,omitempty"`
+	Base              *Base                     `json:"base,omitempty"`
+	ResourceRevisions []RefreshResourceRevision `json:"resource-revisions,omitempty"`
 }
 
 // RefreshResponses holds a series of typed RefreshResponse or a series of
@@ -85,8 +87,16 @@ type RefreshEntity struct {
 	Name      string             `json:"name"`
 	Publisher map[string]string  `json:"publisher,omitempty"`
 	Resources []ResourceRevision `json:"resources"`
+	Bases     []Base             `json:"bases,omitempty"`
 	Revision  int                `json:"revision"`
 	Summary   string             `json:"summary"`
 	Version   string             `json:"version"`
 	CreatedAt time.Time          `json:"created-at"`
 }
+
+// RefreshResourceRevision represents a resource name revision pair for
+// install by revision.
+type RefreshResourceRevision struct {
+	Name     string `json:"name"`
+	Revision int    `json:"revision"`
+}