Skip to content

Conversation

@dsa0x
Copy link
Member

@dsa0x dsa0x commented Feb 12, 2025

This adds a new stacks command for migrating from module terraform state to terraform stack state. The Client is expected to consume this via the rpcapi.

Target Release

1.11.x

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@dsa0x dsa0x force-pushed the sams/tfstacks-migrate branch from 64c0395 to e07a751 Compare February 14, 2025 09:30
@dsa0x dsa0x added the no-changelog-needed Add this to your PR if the change does not require a changelog entry label Feb 14, 2025
@dsa0x dsa0x force-pushed the sams/tfstacks-migrate branch from b151767 to 22bf079 Compare February 17, 2025 12:32
@dsa0x dsa0x changed the title WIP: migrate command for terraform stacks migrate command for terraform stacks Feb 17, 2025
@dsa0x dsa0x force-pushed the sams/tfstacks-migrate branch from 39cf8e9 to c994424 Compare February 17, 2025 13:48
Copy link
Contributor

@liamcervante liamcervante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good overall, just a couple of suggestions!

@dsa0x dsa0x force-pushed the sams/tfstacks-migrate branch from 00a362b to 125e537 Compare March 18, 2025 10:57
@dsa0x dsa0x marked this pull request as ready for review March 18, 2025 11:11
@dsa0x dsa0x requested a review from a team as a code owner March 18, 2025 11:11
@dsa0x dsa0x requested a review from liamcervante March 18, 2025 11:11
}

if resource.Module.IsRoot() {
// If there is no resource mapping, we check for a root module mapping
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be explicit here to begin with - it is safer to loosen restrictions later than to tighten them so I think either return an error for now and we can add this later if needed.

@dsa0x dsa0x requested a review from liamcervante March 19, 2025 08:38
Copy link
Contributor

@liamcervante liamcervante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, small nit - but looks good otherwise

Comment on lines 167 to 180
if ok {
inst, diags := parseComponentInstance(target)
if diags.HasErrors() {
return ret, diags
}
ret.AbsResource = stackaddrs.AbsResource{
Component: inst,
Item: resource,
}
return ret, diags
} else {
diags = diags.Append(tfdiags.Sourceless(tfdiags.Error, "Resource not found", fmt.Sprintf("Resource %q not found in mapping.", resource.Resource.String())))
return ret, diags
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: don't need this if-else statement anymore

@dsa0x dsa0x merged commit 71dbc7d into main Mar 19, 2025
8 checks passed
@dsa0x dsa0x deleted the sams/tfstacks-migrate branch March 19, 2025 09:39
@github-actions
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog-needed Add this to your PR if the change does not require a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants