Skip to content

Commit

Permalink
fix: local variables should not be overridden by remote variables dur…
Browse files Browse the repository at this point in the history 
…ing `terraform import` (#29972)

* fix: local variables should not be overridden by remote variables during `terraform import`

* chore: applied the same fix in the 'internal/cloud' package

* backport changes from cloud package to remote package

Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Co-authored-by: uturunku1 <luces.huayhuaca@gmail.com>
  • Loading branch information
@tchupp @alisdair @uturunku1
3 people committed Mar 15, 2022
1 parent 7c0cbaa commit d15a2bc
Show file tree
Hide file tree
Showing 11 changed files with 607 additions and 96 deletions.
39 changes: 23 additions & 16 deletions internal/backend/remote/backend.go
View file
Expand Up @@ -673,19 +673,14 @@ func (b *Remote) StateMgr(name string) (statemgr.Full, error) {
return &remote.State{Client: client}, nil
}

// Operation implements backend.Enhanced.
func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend.RunningOperation, error) {
// Get the remote workspace name.
name := op.Workspace
switch {
case op.Workspace == backend.DefaultStateName:
name = b.workspace
case b.prefix != "" && !strings.HasPrefix(op.Workspace, b.prefix):
name = b.prefix + op.Workspace
}
func isLocalExecutionMode(execMode string) bool {
return execMode == "local"
}

func (b *Remote) fetchWorkspace(ctx context.Context, organization string, name string) (*tfe.Workspace, error) {
remoteWorkspaceName := b.getRemoteWorkspaceName(name)
// Retrieve the workspace for this operation.
w, err := b.client.Workspaces.Read(ctx, b.organization, name)
w, err := b.client.Workspaces.Read(ctx, b.organization, remoteWorkspaceName)
if err != nil {
switch err {
case context.Canceled:
Expand All @@ -695,17 +690,29 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
"workspace %s not found\n\n"+
"The configured \"remote\" backend returns '404 Not Found' errors for resources\n"+
"that do not exist, as well as for resources that a user doesn't have access\n"+
"to. If the resource does exist, please check the rights for the used token.",
"to. If the resource does exist, please check the rights for the used token",
name,
)
default:
return nil, fmt.Errorf(
"The configured \"remote\" backend encountered an unexpected error:\n\n%s",
err := fmt.Errorf(
"the configured \"remote\" backend encountered an unexpected error:\n\n%s",
err,
)
return nil, err
}
}

return w, nil
}

// Operation implements backend.Enhanced.
func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend.RunningOperation, error) {
w, err := b.fetchWorkspace(ctx, b.organization, op.Workspace)

if err != nil {
return nil, err
}

// Terraform remote version conflicts are not a concern for operations. We
// are in one of three states:
//
Expand All @@ -718,7 +725,7 @@ func (b *Remote) Operation(ctx context.Context, op *backend.Operation) (*backend
b.IgnoreVersionConflict()

// Check if we need to use the local backend to run the operation.
if b.forceLocal || !w.Operations {
if b.forceLocal || isLocalExecutionMode(w.ExecutionMode) {
// Record that we're forced to run operations locally to allow the
// command package UI to operate correctly
b.forceLocal = true
Expand Down Expand Up @@ -902,7 +909,7 @@ func (b *Remote) VerifyWorkspaceTerraformVersion(workspaceName string) tfdiags.D

// If the workspace has remote operations disabled, the remote Terraform
// version is effectively meaningless, so we'll skip version verification.
if !workspace.Operations {
if isLocalExecutionMode(workspace.ExecutionMode) {
return nil
}

Expand Down
16 changes: 8 additions & 8 deletions internal/backend/remote/backend_apply_test.go
View file
Expand Up @@ -1526,36 +1526,36 @@ func TestRemote_applyVersionCheck(t *testing.T) {
localVersion string
remoteVersion string
forceLocal bool
hasOperations bool
executionMode string
wantErr string
}{
"versions can be different for remote apply": {
localVersion: "0.14.0",
remoteVersion: "0.13.5",
hasOperations: true,
executionMode: "remote",
},
"versions can be different for local apply": {
localVersion: "0.14.0",
remoteVersion: "0.13.5",
hasOperations: false,
executionMode: "local",
},
"force local with remote operations and different versions is acceptable": {
localVersion: "0.14.0",
remoteVersion: "0.14.0-acme-provider-bundle",
forceLocal: true,
hasOperations: true,
executionMode: "remote",
},
"no error if versions are identical": {
localVersion: "0.14.0",
remoteVersion: "0.14.0",
forceLocal: true,
hasOperations: true,
executionMode: "remote",
},
"no error if force local but workspace has remote operations disabled": {
localVersion: "0.14.0",
remoteVersion: "0.13.5",
forceLocal: true,
hasOperations: false,
executionMode: "local",
},
}

Expand Down Expand Up @@ -1591,7 +1591,7 @@ func TestRemote_applyVersionCheck(t *testing.T) {
b.organization,
b.workspace,
tfe.WorkspaceUpdateOptions{
Operations: tfe.Bool(tc.hasOperations),
ExecutionMode: tfe.String(tc.executionMode),
TerraformVersion: tfe.String(tc.remoteVersion),
},
)
Expand Down Expand Up @@ -1644,7 +1644,7 @@ func TestRemote_applyVersionCheck(t *testing.T) {
hasRemote := strings.Contains(output, "Running apply in the remote backend")
hasSummary := strings.Contains(output, "1 added, 0 changed, 0 destroyed")
hasResources := run.State.HasManagedResourceInstanceObjects()
if !tc.forceLocal && tc.hasOperations {
if !tc.forceLocal && !isLocalExecutionMode(tc.executionMode) {
if !hasRemote {
t.Errorf("missing remote backend header in output: %s", output)
}
Expand Down
57 changes: 34 additions & 23 deletions internal/backend/remote/backend_context.go
View file
Expand Up @@ -82,37 +82,48 @@ func (b *Remote) LocalRun(op *backend.Operation) (*backend.LocalRun, statemgr.Fu
}
ret.Config = config

// The underlying API expects us to use the opaque workspace id to request
// variables, so we'll need to look that up using our organization name
// and workspace name.
remoteWorkspaceID, err := b.getRemoteWorkspaceID(context.Background(), op.Workspace)
if err != nil {
diags = diags.Append(fmt.Errorf("error finding remote workspace: %w", err))
return nil, nil, diags
}

log.Printf("[TRACE] backend/remote: retrieving variables from workspace %s/%s (%s)", remoteWorkspaceName, b.organization, remoteWorkspaceID)
tfeVariables, err := b.client.Variables.List(context.Background(), remoteWorkspaceID, tfe.VariableListOptions{})
if err != nil && err != tfe.ErrResourceNotFound {
diags = diags.Append(fmt.Errorf("error loading variables: %w", err))
return nil, nil, diags
}

if op.AllowUnsetVariables {
// If we're not going to use the variables in an operation we'll be
// more lax about them, stubbing out any unset ones as unknown.
// This gives us enough information to produce a consistent context,
// but not enough information to run a real operation (plan, apply, etc)
ret.PlanOpts.SetVariables = stubAllVariables(op.Variables, config.Module.Variables)
} else {
if tfeVariables != nil {
if op.Variables == nil {
op.Variables = make(map[string]backend.UnparsedVariableValue)
// The underlying API expects us to use the opaque workspace id to request
// variables, so we'll need to look that up using our organization name
// and workspace name.
remoteWorkspaceID, err := b.getRemoteWorkspaceID(context.Background(), op.Workspace)
if err != nil {
diags = diags.Append(fmt.Errorf("error finding remote workspace: %w", err))
return nil, nil, diags
}

w, err := b.fetchWorkspace(context.Background(), b.organization, op.Workspace)
if err != nil {
diags = diags.Append(fmt.Errorf("error loading workspace: %w", err))
return nil, nil, diags
}

if isLocalExecutionMode(w.ExecutionMode) {
log.Printf("[TRACE] skipping retrieving variables from workspace %s/%s (%s), workspace is in Local Execution mode", remoteWorkspaceName, b.organization, remoteWorkspaceID)
} else {
log.Printf("[TRACE] backend/remote: retrieving variables from workspace %s/%s (%s)", remoteWorkspaceName, b.organization, remoteWorkspaceID)
tfeVariables, err := b.client.Variables.List(context.Background(), remoteWorkspaceID, tfe.VariableListOptions{})
if err != nil && err != tfe.ErrResourceNotFound {
diags = diags.Append(fmt.Errorf("error loading variables: %w", err))
return nil, nil, diags
}
for _, v := range tfeVariables.Items {
if v.Category == tfe.CategoryTerraform {
op.Variables[v.Key] = &remoteStoredVariableValue{
definition: v,
if tfeVariables != nil {
if op.Variables == nil {
op.Variables = make(map[string]backend.UnparsedVariableValue)
}
for _, v := range tfeVariables.Items {
if v.Category == tfe.CategoryTerraform {
if _, ok := op.Variables[v.Key]; !ok {
op.Variables[v.Key] = &remoteStoredVariableValue{
definition: v,
}
}
}
}
}
Expand Down

0 comments on commit d15a2bc

Please sign in to comment.