Use terraform remote state backend#3
Open
Vignesh (vigneshr-07) wants to merge 1 commit into
Open
Conversation
Vignesh (vigneshr-07)
force-pushed
the
tf-native-remote-backend
branch
from
18f754a to
3cf4c0e
Compare
The current implementation of state management uses our own implementation for managing local and remote state. On @sl1pm4t's suggestion, this PR attempts to use Terraform's native state backend management instead. ## State backend configuration This implementation dynamically creates a file in the working directory that contains the state backend definition. Migrated from ditto-cloud-bootstrap PR #29
Vignesh (vigneshr-07)
force-pushed
the
tf-native-remote-backend
branch
from
3cf4c0e to
a65d756
Compare
Vignesh (vigneshr-07)
changed the base branch from
main
to
voy/add-remote-state-storage
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
Comment on lines
+131
to
148
| // FinalizeStateTransfer handles copying state files back and cleanup operations | ||
| func (s *StateManager) FinalizeStateTransfer(ctx context.Context) { | ||
| // Only handle finalization if we have both paths (local backend scenario) | ||
| if s.localStatePath == "" || s.workingStatePath == "" { | ||
| return | ||
| } | ||
| defer reader.Close() | ||
|
|
||
| // Create local file | ||
| localFile, err := os.Create(localPath) | ||
| // Copy the state file back to the original location | ||
| s.progressColor.Printf("Copying state file back to %q\n", s.localStatePath) | ||
| stateFileData, err := os.ReadFile(s.workingStatePath) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create local state file: %w", err) | ||
| s.progressColor.Printf("unable to read state file from temporary directory: %v", err) | ||
| return | ||
| } | ||
| if err := os.WriteFile(s.localStatePath, stateFileData, 0600); err != nil { | ||
| s.progressColor.Printf("unable to write state file to %q: %v", s.localStatePath, err) | ||
| return | ||
| } |
There was a problem hiding this comment.
FinalizeStateTransfer now unconditionally copies the temp working state back to localStatePath; when a remote backend is configured Terraform never writes a local terraform.tfstate, so we end up overwriting the user's state file with the pre-migration copy and leaving it stale. Can we skip this copy when a remote backend was used so we don't overwrite the real remote state with outdated local data?
Prompt for AI Agents:
In cmd/internal/bootstrap/state.go around lines 131 to 148, the FinalizeStateTransfer
method unconditionally copies the temporary working state back to localStatePath which
can overwrite a user's remote state when a remote backend was configured. Change this so
FinalizeStateTransfer first checks whether a remote backend was successfully configured
and only performs the copy when no remote backend is in use. Concretely: add a boolean
field (e.g. remoteBackendConfigured) to StateManager, set it to true at the end of
configureRemoteBackend on successful initialization, and update FinalizeStateTransfer to
return early (skip the copy) if remoteBackendConfigured is true; keep current behavior
otherwise.
Finding type: Logical Bugs
Comment on lines
+269
to
+289
| // Try a simple write/read test | ||
| testObjectName := ".test_file_" + time.Now().Format("20060102150405") | ||
| writer, err := bucket.NewWriter(ctx, testObjectName, nil) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create bucket writer: %w", err) | ||
| s.logger.Debug("Remote state bucket doesn't exist or is not accessible", | ||
| "error", err, | ||
| "bucket", bucketURL) | ||
| return false, nil | ||
| } | ||
|
|
||
| if _, err := io.Copy(writer, localFile); err != nil { | ||
| if _, err := writer.Write([]byte("test")); err != nil { | ||
| writer.Close() | ||
| return fmt.Errorf("failed to upload state file content: %w", err) | ||
| return false, nil | ||
| } | ||
|
|
||
| if err := writer.Close(); err != nil { | ||
| return fmt.Errorf("failed to close bucket writer: %w", err) | ||
| return false, nil | ||
| } | ||
|
|
||
| s.logger.Debug("Remote state bucket exists and is accessible", "bucket", bucketURL) | ||
| return true, nil |
There was a problem hiding this comment.
Remote bucket probe writes .test_file_<timestamp> every run but never deletes it, so repeated bootstrap runs leave orphaned test objects in the bucket; can we delete the test object after the probe succeeds?
Prompt for AI Agents:
In cmd/internal/bootstrap/state.go around lines 269-289, the checkBucketAccess function
writes a test object named .test_file_<timestamp> to the bucket but never deletes it,
leaving orphaned probe objects. After the writer.Close() succeeds, delete that test
object by calling bucket.Delete(ctx, testObjectName) and handle any delete error by
logging a debug/warn message without returning an error (so probe still reports
accessible). Ensure the deletion is attempted only when the write/close completed
successfully and that bucket.Close() remains deferred as-is.
Finding type: Logical Bugs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
The current implementation of state management uses our own implementation for managing local and remote state. On Matt Morrison (@sl1pm4t)'s suggestion, this PR attempts to use Terraform's native state backend management instead.
Migrated from ditto-cloud-bootstrap PR #29
Generated description
Below is a concise technical summary of the changes proposed in this PR:
graph LR BootstrapCmd_("BootstrapCmd"):::modified NewStateManager_("NewStateManager"):::added InitializeWithBackend_("InitializeWithBackend"):::added FinalizeStateTransfer_("FinalizeStateTransfer"):::added checkBucketAccess_("checkBucketAccess"):::added configureRemoteBackend_("configureRemoteBackend"):::added ProviderConfig_("ProviderConfig"):::modified checkAndMigrateToRemoteBackend_("checkAndMigrateToRemoteBackend"):::added BootstrapCmd_ -- "Creates StateManager with provider config, workingDir, tf." --> NewStateManager_ BootstrapCmd_ -- "Initializes backend (remote preferred, fallback to local)." --> InitializeWithBackend_ BootstrapCmd_ -- "Defers copying state back and post-apply migration check." --> FinalizeStateTransfer_ InitializeWithBackend_ -- "Probes remote bucket accessibility with timed blob write test." --> checkBucketAccess_ InitializeWithBackend_ -- "Configures backend.tf and reinitializes Terraform to remote." --> configureRemoteBackend_ configureRemoteBackend_ -- "Retrieves provider-specific backend config (bucket, prefix, region)." --> ProviderConfig_ FinalizeStateTransfer_ -- "After copying, checks bucket and migrates local state remotely." --> checkAndMigrateToRemoteBackend_ checkAndMigrateToRemoteBackend_ -- "Reconfigures and reinitializes Terraform to use remote backend." --> configureRemoteBackend_ classDef added stroke:#15AA7A classDef removed stroke:#CD5270 classDef modified stroke:#EDAC4C linkStyle default stroke:#CBD5E1,font-size:13pxImplements Terraform's native remote state backend management to replace the previous custom local and remote state handling. Updates the
StateManagerand provider configurations to support automatic migration from local to remote storage during the bootstrap process.StateManagerto utilizetfexecfor backend initialization and handle the migration of state files between local and remote storage.Modified files (2)
Latest Contributors(1)
TerraformBackendConfigimplementations for AWS and GCP to generate provider-specific backend configuration files and initialization options.Modified files (3)
Latest Contributors(1)