Relocate localterraform.com aliasing to backend configurators

Previously, remote and cloud backends would automatically alias localterraform.com as the configured hostname during configuration. This turned out to be an issue with how backends could potentially be used within the builtin terraform_remote_state data source. Those data sources each configure the same service discovery with different targets for localterraform.com, and do so simultaneously, creating an occasional concurrent map read & write panic when multiple data sources are defined.

localterraform.com is obviously not useful for every backend configuration. Therefore, I relocated the alias configuration to the callers, so they may specify when to use it. The modified design adds a new method to backend.Enhanced to allow configurators to ask which aliases should be defined.
This commit is contained in:
Brandon Croft 2023-06-26 11:02:55 -06:00
parent ad26644578
commit c1a730314d
No known key found for this signature in database
GPG Key ID: B01E32423322EB9D
7 changed files with 160 additions and 32 deletions

View File

@ -14,6 +14,7 @@ import (
"log"
"os"
svchost "github.com/hashicorp/terraform-svchost"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/hashicorp/terraform/internal/command/clistate"
"github.com/hashicorp/terraform/internal/command/views"
@ -119,6 +120,13 @@ type Backend interface {
Workspaces() ([]string, error)
}
// HostAlias describes a list of aliases that should be used when initializing an
// Enhanced Backend
type HostAlias struct {
From svchost.Hostname
To svchost.Hostname
}
// Enhanced implements additional behavior on top of a normal backend.
//
// 'Enhanced' backends are an implementation detail only, and are no longer reflected as an external
@ -136,6 +144,10 @@ type Enhanced interface {
// responsibility of the Backend to lock the state for the duration of the
// running operation.
Operation(context.Context, *Operation) (*RunningOperation, error)
// ServiceDiscoveryAliases returns a mapping of Alias -> Target hosts to
// configure.
ServiceDiscoveryAliases() ([]HostAlias, error)
}
// Local implements additional behavior on a Backend that allows local

View File

@ -183,6 +183,10 @@ func (b *Local) Configure(obj cty.Value) tfdiags.Diagnostics {
return diags
}
func (b *Local) ServiceDiscoveryAliases() ([]backend.HostAlias, error) {
return []backend.HostAlias{}, nil
}
func (b *Local) Workspaces() ([]string, error) {
// If we have a backend handling state, defer to that.
if b.Backend != nil {

View File

@ -198,21 +198,26 @@ func (b *Remote) PrepareConfig(obj cty.Value) (cty.Value, tfdiags.Diagnostics) {
return obj, diags
}
// configureGenericHostname aliases the remote backend hostname configuration
// as a generic "localterraform.com" hostname. This was originally added as a
// Terraform Enterprise feature and is useful for re-using whatever the
// Cloud/Enterprise backend host is in nested module sources in order
// to prevent code churn when re-using config between multiple
// Terraform Enterprise environments.
func (b *Remote) configureGenericHostname() {
// This won't be an error for the given constant value
genericHost, _ := svchost.ForComparison(genericHostname)
func (b *Remote) ServiceDiscoveryAliases() ([]backend.HostAlias, error) {
aliasHostname, err := svchost.ForComparison(genericHostname)
if err != nil {
// This should never happen because the hostname is statically defined.
return nil, fmt.Errorf("failed to create backend alias from alias %q. The hostname is not in the correct format. This is a bug in the backend", genericHostname)
}
// This won't be an error because, by this time, the hostname has been parsed and
// service discovery requests made against it.
targetHost, _ := svchost.ForComparison(b.hostname)
targetHostname, err := svchost.ForComparison(b.hostname)
if err != nil {
// This should never happen because the 'to' alias is the backend host, which has likely
// already been evaluated as a svchost.Hostname by now
return nil, fmt.Errorf("failed to create backend alias to target %q. The hostname is not in the correct format", b.hostname)
}
b.services.Alias(genericHost, targetHost)
return []backend.HostAlias{
{
From: aliasHostname,
To: targetHostname,
},
}, nil
}
// Configure implements backend.Enhanced.
@ -317,8 +322,6 @@ func (b *Remote) Configure(obj cty.Value) tfdiags.Diagnostics {
return diags
}
b.configureGenericHostname()
cfg := &tfe.Config{
Address: service.String(),
BasePath: service.Path,

View File

@ -725,3 +725,29 @@ func TestRemote_VerifyWorkspaceTerraformVersion_ignoreFlagSet(t *testing.T) {
t.Errorf("wrong summary: got %s, want %s", got, wantDetail)
}
}
func TestRemote_ServiceDiscoveryAliases(t *testing.T) {
s := testServer(t)
b := New(testDisco(s))
diag := b.Configure(cty.ObjectVal(map[string]cty.Value{
"hostname": cty.NullVal(cty.String), // Forces aliasing to test server
"organization": cty.StringVal("hashicorp"),
"token": cty.NullVal(cty.String),
"workspaces": cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("prod"),
"prefix": cty.NullVal(cty.String),
}),
}))
if diag.HasErrors() {
t.Fatalf("expected no diagnostic errors, got %s", diag.Err())
}
aliases, err := b.ServiceDiscoveryAliases()
if err != nil {
t.Fatalf("expected no errors, got %s", err)
}
if len(aliases) != 1 {
t.Fatalf("expected 1 alias but got %d", len(aliases))
}
}

View File

@ -204,21 +204,26 @@ func (b *Cloud) PrepareConfig(obj cty.Value) (cty.Value, tfdiags.Diagnostics) {
return obj, diags
}
// configureGenericHostname aliases the cloud backend hostname configuration
// as a generic "localterraform.com" hostname. This was originally added as a
// Terraform Enterprise feature and is useful for re-using whatever the
// Cloud/Enterprise backend host is in nested module sources in order
// to prevent code churn when re-using config between multiple
// Terraform Enterprise environments.
func (b *Cloud) configureGenericHostname() {
// This won't be an error for the given constant value
genericHost, _ := svchost.ForComparison(genericHostname)
func (b *Cloud) ServiceDiscoveryAliases() ([]backend.HostAlias, error) {
aliasHostname, err := svchost.ForComparison(genericHostname)
if err != nil {
// This should never happen because the hostname is statically defined.
return nil, fmt.Errorf("failed to create backend alias from alias %q. The hostname is not in the correct format. This is a bug in the backend", genericHostname)
}
// This won't be an error because, by this time, the hostname has been parsed and
// service discovery requests made against it.
targetHost, _ := svchost.ForComparison(b.hostname)
targetHostname, err := svchost.ForComparison(b.hostname)
if err != nil {
// This should never happen because the 'to' alias is the backend host, which has
// already been ev
return nil, fmt.Errorf("failed to create backend alias to target %q. The hostname is not in the correct format.", b.hostname)
}
b.services.Alias(genericHost, targetHost)
return []backend.HostAlias{
{
From: aliasHostname,
To: targetHostname,
},
}, nil
}
// Configure implements backend.Enhanced.
@ -287,7 +292,6 @@ func (b *Cloud) Configure(obj cty.Value) tfdiags.Diagnostics {
}
b.token = token
b.configureGenericHostname()
if b.client == nil {
cfg := &tfe.Config{

View File

@ -1145,7 +1145,7 @@ func TestCloud_VerifyWorkspaceTerraformVersion_ignoreFlagSet(t *testing.T) {
}
}
func TestClodBackend_DeleteWorkspace_SafeAndForce(t *testing.T) {
func TestCloudBackend_DeleteWorkspace_SafeAndForce(t *testing.T) {
b, bCleanup := testBackendWithTags(t)
defer bCleanup()
safeDeleteWorkspaceName := "safe-delete-workspace"
@ -1211,7 +1211,7 @@ func TestClodBackend_DeleteWorkspace_SafeAndForce(t *testing.T) {
}
}
func TestClodBackend_DeleteWorkspace_DoesNotExist(t *testing.T) {
func TestCloudBackend_DeleteWorkspace_DoesNotExist(t *testing.T) {
b, bCleanup := testBackendWithTags(t)
defer bCleanup()
@ -1220,3 +1220,29 @@ func TestClodBackend_DeleteWorkspace_DoesNotExist(t *testing.T) {
t.Fatalf("expected deleting a workspace which does not exist to succeed")
}
}
func TestCloud_ServiceDiscoveryAliases(t *testing.T) {
s := testServer(t)
b := New(testDisco(s))
diag := b.Configure(cty.ObjectVal(map[string]cty.Value{
"hostname": cty.NullVal(cty.String), // Forces aliasing to test server
"organization": cty.StringVal("hashicorp"),
"token": cty.NullVal(cty.String),
"workspaces": cty.ObjectVal(map[string]cty.Value{
"name": cty.StringVal("prod"),
"tags": cty.NullVal(cty.Set(cty.String)),
}),
}))
if diag.HasErrors() {
t.Fatalf("expected no diagnostic errors, got %s", diag.Err())
}
aliases, err := b.ServiceDiscoveryAliases()
if err != nil {
t.Fatalf("expected no errors, got %s", err)
}
if len(aliases) != 1 {
t.Fatalf("expected 1 alias but got %d", len(aliases))
}
}

View File

@ -291,7 +291,7 @@ func (m *Meta) selectWorkspace(b backend.Backend) error {
}
workspace = workspaces[idx-1]
log.Printf("[TRACE] Meta.selectWorkspace: setting the current workpace according to user selection (%s)", workspace)
log.Printf("[TRACE] Meta.selectWorkspace: setting the current workspace according to user selection (%s)", workspace)
return m.SetWorkspace(workspace)
}
@ -352,6 +352,10 @@ func (m *Meta) BackendForPlan(settings plans.Backend) (backend.Enhanced, tfdiags
// then return that as-is. This works even if b == nil (it will be !ok).
if enhanced, ok := b.(backend.Enhanced); ok {
log.Printf("[TRACE] Meta.BackendForPlan: backend %T supports operations", b)
if err := m.setupEnhancedBackendAliases(enhanced); err != nil {
diags = diags.Append(err)
return nil, diags
}
return enhanced, nil
}
@ -831,6 +835,17 @@ func (m *Meta) backendFromState(ctx context.Context) (backend.Backend, tfdiags.D
return nil, diags
}
// If the result of loading the backend is an enhanced backend,
// then set up enhanced backend service aliases.
if enhanced, ok := b.(backend.Enhanced); ok {
log.Printf("[TRACE] Meta.BackendForPlan: backend %T supports operations", b)
if err := m.setupEnhancedBackendAliases(enhanced); err != nil {
diags = diags.Append(err)
return nil, diags
}
}
return b, diags
}
@ -1277,6 +1292,17 @@ func (m *Meta) savedBackend(sMgr *clistate.LocalState) (backend.Backend, tfdiags
return nil, diags
}
// If the result of loading the backend is an enhanced backend,
// then set up enhanced backend service aliases.
if enhanced, ok := b.(backend.Enhanced); ok {
log.Printf("[TRACE] Meta.BackendForPlan: backend %T supports operations", b)
if err := m.setupEnhancedBackendAliases(enhanced); err != nil {
diags = diags.Append(err)
return nil, diags
}
}
return b, diags
}
@ -1397,9 +1423,36 @@ func (m *Meta) backendInitFromConfig(c *configs.Backend) (backend.Backend, cty.V
configureDiags := b.Configure(newVal)
diags = diags.Append(configureDiags.InConfigBody(c.Config, ""))
// If the result of loading the backend is an enhanced backend,
// then set up enhanced backend service aliases.
if enhanced, ok := b.(backend.Enhanced); ok {
log.Printf("[TRACE] Meta.BackendForPlan: backend %T supports operations", b)
if err := m.setupEnhancedBackendAliases(enhanced); err != nil {
diags = diags.Append(err)
return nil, cty.NilVal, diags
}
}
return b, configVal, diags
}
// Helper method to get aliases from the enhanced backend and alias them
// in the Meta service discovery. It's unfortunate that the Meta backend
// is modifying the service discovery at this level, but the owner
// of the service discovery pointer does not have easy access to the backend.
func (m *Meta) setupEnhancedBackendAliases(b backend.Enhanced) error {
// Set up the service discovery aliases specified by the enhanced backend.
serviceAliases, err := b.ServiceDiscoveryAliases()
if err != nil {
return err
}
for _, alias := range serviceAliases {
m.Services.Alias(alias.From, alias.To)
}
return nil
}
// Helper method to ignore remote/cloud backend version conflicts. Only call this
// for commands which cannot accidentally upgrade remote state files.
func (m *Meta) ignoreRemoteVersionConflict(b backend.Backend) {