Canonicalize provider version during default provider lookup

changelog/pending/20240502--engine--normalize-provider-version-during-default-provider-lookup.yaml

@@ -0,0 +1,4 @@
+changes:
+- type: fix
+ scope: engine
+ description: Normalize provider version during default provider lookup

pkg/resource/deploy/source_eval.go

@@ -301,17 +301,7 @@ type defaultProviderRequest struct {
  response chan<- defaultProviderResponse
 }
 
-// newRegisterDefaultProviderEvent creates a RegisterResourceEvent and completion channel that can be sent to the
-// engine to register a default provider resource for the indicated package.
-func (d *defaultProviders) newRegisterDefaultProviderEvent(
- req providers.ProviderRequest,
-) (*registerResourceEvent, <-chan *RegisterResult, error) {
- // Attempt to get the config for the package.
- inputs, err := d.config.GetPackageConfig(req.Package())
- if err != nil {
- return nil, nil, err
- }
-
+func (d *defaultProviders) normalizeProviderRequest(req providers.ProviderRequest) providers.ProviderRequest {
  // Request that the engine instantiate a specific version of this provider, if one was requested. We'll figure out
  // what version to request by:
  // 1. Providing the Version field of the ProviderRequest verbatim, if it was provided, otherwise
@@ -323,56 +313,68 @@ func (d *defaultProviders) newRegisterDefaultProviderEvent(
  // especially onerous because the engine selects the "newest" plugin available on the machine, which is generally
  // problematic for a lot of reasons.
  if req.Version() != nil {
- logging.V(5).Infof("newRegisterDefaultProviderEvent(%s): using version %s from request", req, req.Version())
- providers.SetProviderVersion(inputs, req.Version())
+ logging.V(5).Infof("normalizeProviderRequest(%s): using version %s from request", req, req.Version())
  } else {
- logging.V(5).Infof(
- "newRegisterDefaultProviderEvent(%s): no version specified, falling back to default version", req)
  if version := d.defaultProviderInfo[req.Package()].Version; version != nil {
- logging.V(5).Infof("newRegisterDefaultProviderEvent(%s): default version hit on version %s", req, version)
- providers.SetProviderVersion(inputs, version)
+ logging.V(5).Infof("normalizeProviderRequest(%s): default version hit on version %s", req, version)
+ req = providers.NewProviderRequest(version, req.Package(), req.PluginDownloadURL(), req.PluginChecksums())
  } else {
  logging.V(5).Infof(
- "newRegisterDefaultProviderEvent(%s): default provider miss, sending nil version to engine", req)
+ "normalizeProviderRequest(%s): default provider miss, sending nil version to engine", req)
  }
  }
 
  if req.PluginDownloadURL() != "" {
- logging.V(5).Infof("newRegisterDefaultProviderEvent(%s): using pluginDownloadURL %s from request",
+ logging.V(5).Infof("normalizeProviderRequest(%s): using pluginDownloadURL %s from request",
  req, req.PluginDownloadURL())
- providers.SetProviderURL(inputs, req.PluginDownloadURL())
  } else {
- logging.V(5).Infof(
- "newRegisterDefaultProviderEvent(%s): no pluginDownloadURL specified, falling back to default pluginDownloadURL",
- req)
  if pluginDownloadURL := d.defaultProviderInfo[req.Package()].PluginDownloadURL; pluginDownloadURL != "" {
- logging.V(5).Infof("newRegisterDefaultProviderEvent(%s): default pluginDownloadURL hit on %s",
+ logging.V(5).Infof("normalizeProviderRequest(%s): default pluginDownloadURL hit on %s",
  req, pluginDownloadURL)
- providers.SetProviderURL(inputs, pluginDownloadURL)
+ req = providers.NewProviderRequest(req.Version(), req.Package(), pluginDownloadURL, req.PluginChecksums())
  } else {
  logging.V(5).Infof(
- "newRegisterDefaultProviderEvent(%s): default pluginDownloadURL miss, sending empty string to engine", req)
+ "normalizeProviderRequest(%s): default pluginDownloadURL miss, sending empty string to engine", req)
  }
  }
 
  if req.PluginChecksums() != nil {
- logging.V(5).Infof("newRegisterDefaultProviderEvent(%s): using pluginChecksums %v from request",
+ logging.V(5).Infof("normalizeProviderRequest(%s): using pluginChecksums %v from request",
  req, req.PluginChecksums())
- providers.SetProviderChecksums(inputs, req.PluginChecksums())
  } else {
- logging.V(5).Infof(
- "newRegisterDefaultProviderEvent(%s): no pluginChecksums specified, falling back to default pluginChecksums",
- req)
  if pluginChecksums := d.defaultProviderInfo[req.Package()].Checksums; pluginChecksums != nil {
- logging.V(5).Infof("newRegisterDefaultProviderEvent(%s): default pluginChecksums hit on %v",
+ logging.V(5).Infof("normalizeProviderRequest(%s): default pluginChecksums hit on %v",
  req, pluginChecksums)
- providers.SetProviderChecksums(inputs, pluginChecksums)
+ req = providers.NewProviderRequest(req.Version(), req.Package(), req.PluginDownloadURL(), pluginChecksums)
  } else {
  logging.V(5).Infof(
- "newRegisterDefaultProviderEvent(%s): default pluginChecksums miss, sending empty map to engine", req)
+ "normalizeProviderRequest(%s): default pluginChecksums miss, sending empty map to engine", req)
  }
  }
 
+ return req
+}
+
+// newRegisterDefaultProviderEvent creates a RegisterResourceEvent and completion channel that can be sent to the
+// engine to register a default provider resource for the indicated package.
+func (d *defaultProviders) newRegisterDefaultProviderEvent(
+ req providers.ProviderRequest,
+) (*registerResourceEvent, <-chan *RegisterResult, error) {
+ // Attempt to get the config for the package.
+ inputs, err := d.config.GetPackageConfig(req.Package())
+ if err != nil {
+ return nil, nil, err
+ }
+ if req.Version() != nil {
+ providers.SetProviderVersion(inputs, req.Version())
+ }
+ if req.PluginDownloadURL() != "" {
+ providers.SetProviderURL(inputs, req.PluginDownloadURL())
+ }
+ if req.PluginChecksums() != nil {
+ providers.SetProviderChecksums(inputs, req.PluginChecksums())
+ }
+
  // Create the result channel and the event.
  done := make(chan *RegisterResult)
  event := &registerResourceEvent{
@@ -395,6 +397,8 @@ func (d *defaultProviders) newRegisterDefaultProviderEvent(
 func (d *defaultProviders) handleRequest(req providers.ProviderRequest) (providers.Reference, error) {
  logging.V(5).Infof("handling default provider request for package %s", req)
 
+ req = d.normalizeProviderRequest(req)
+
  denyCreation, err := d.shouldDenyRequest(req)
  if err != nil {
  return providers.Reference{}, err

pkg/resource/deploy/source_eval_test.go

@@ -2042,47 +2042,46 @@ func (c *configSourceMock) GetPackageConfig(pkg tokens.Package) (resource.Proper
 
 func TestDefaultProviders(t *testing.T) {
  t.Parallel()
- t.Run("newRegisterDefaultProviderEvent", func(t *testing.T) {
+ t.Run("normalizeProviderRequest", func(t *testing.T) {
  t.Parallel()
- t.Run("error in GetPackageConfig()", func(t *testing.T) {
+ t.Run("use defaultProvider", func(t *testing.T) {
  t.Parallel()
- expectedErr := errors.New("expected error")
+ v1 := semver.MustParse("0.1.0")
  d := &defaultProviders{
+ defaultProviderInfo: map[tokens.Package]workspace.PluginSpec{
+ tokens.Package("pkg"): {
+ Version: &v1,
+ PluginDownloadURL: "github://owner/repo",
+ Checksums: map[string][]byte{"key": []byte("expected-checksum-value")},
+ },
+ },
  config: &configSourceMock{
  GetPackageConfigF: func(pkg tokens.Package) (resource.PropertyMap, error) {
- return nil, expectedErr
+ return resource.PropertyMap{}, nil
  },
  },
  }
- _, _, err := d.newRegisterDefaultProviderEvent(providers.ProviderRequest{})
- assert.ErrorIs(t, err, expectedErr)
+ req := d.normalizeProviderRequest(providers.NewProviderRequest(nil, tokens.Package("pkg"), "", nil))
+ assert.NotNil(t, req)
+ assert.Equal(t, &v1, req.Version())
+ assert.Equal(t, "github://owner/repo", req.PluginDownloadURL())
+ assert.Equal(t, map[string][]byte{"key": []byte("expected-checksum-value")}, req.PluginChecksums())
  })
- t.Run("use defaultProvider checksums", func(t *testing.T) {
+ })
+ t.Run("newRegisterDefaultProviderEvent", func(t *testing.T) {
+ t.Parallel()
+ t.Run("error in GetPackageConfig()", func(t *testing.T) {
  t.Parallel()
+ expectedErr := errors.New("expected error")
  d := &defaultProviders{
- defaultProviderInfo: map[tokens.Package]workspace.PluginSpec{
- tokens.Package(""): {
- Checksums: map[string][]byte{"key": []byte("expected-checksum-value")},
- },
- },
  config: &configSourceMock{
  GetPackageConfigF: func(pkg tokens.Package) (resource.PropertyMap, error) {
- return resource.PropertyMap{}, nil
+ return nil, expectedErr
  },
  },
  }
- evt, done, err := d.newRegisterDefaultProviderEvent(providers.ProviderRequest{})
- assert.NoError(t, err)
- assert.NotNil(t, done)
- assert.Equal(t,
- resource.PropertyValue{
- V: resource.PropertyMap{
- "key": resource.PropertyValue{
- V: "65787065637465642d636865636b73756d2d76616c7565",
- },
- },
- },
- evt.goal.Properties["pluginChecksums"])
+ _, _, err := d.newRegisterDefaultProviderEvent(providers.ProviderRequest{})
+ assert.ErrorIs(t, err, expectedErr)
  })
  })
  t.Run("handleRequest", func(t *testing.T) {