Skip to content

Commit

Permalink
rework: Address feedback provided on the pull request.
Browse files Browse the repository at this point in the history
  • Loading branch information
pvbouwel committed Nov 26, 2022
1 parent c27f2a1 commit 9c4c900
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 73 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -596,14 +596,14 @@ matched against resources defined in the terraform state. The matched value
can be used in the destination definition via a dollar sign and their ordinal number:
`$1`, `$2`, ... When there is ambiguity the ordinal number can be put in curly braces (e.g. `${1}`).

For example if `foo` and `bar` in de `mv` command example above are the only 2 security group resources
For example if `foo` and `bar` in the `mv` command example above are the only 2 security group resources
defined at the top level then you can rename them using:

```hcl
migration "state" "test" {
dir = "dir1"
actions = [
"mv aws_security_group.* aws_security_group.${1}2",
"xmv aws_security_group.* aws_security_group.${1}2",
]
}
```
Expand Down
22 changes: 0 additions & 22 deletions tfexec/test_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,28 +326,6 @@ func UpdateTestAccSource(t *testing.T, tf TerraformCLI, source string) {
}
}

// AddModuleToTestAcc adds a module dir with given contents
func AddModuleToTestAcc(t *testing.T, tf TerraformCLI, moduleName, source string) {
t.Helper()
moduleDir := filepath.Join(tf.Dir(), moduleName)
if err := os.Mkdir(moduleDir, 0700); err != nil {
t.Fatalf("Failed to add module dir to source: %s", err)
}
if err := os.WriteFile(filepath.Join(moduleDir, testAccSourceFileName), []byte(source), 0600); err != nil {
t.Fatalf("failed to update module source: %s", err)
}
}

// RunTestAccInitModule performs a tf init
func RunTestAccInitModule(t *testing.T, tf TerraformCLI) {
t.Helper()
ctx := context.Background()
err := tf.Init(ctx, "-input=false", "-no-color")
if err != nil {
t.Fatalf("failed to run terraform init: %s", err)
}
}

// MatchTerraformVersion returns true if terraform version matches a given constraints.
func MatchTerraformVersion(ctx context.Context, tf TerraformCLI, constraints string) (bool, error) {
tfVersionRaw, err := tf.Version(ctx)
Expand Down
50 changes: 26 additions & 24 deletions tfmigrate/state_xmv_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func NewStateXMvAction(source string, destination string) *StateXMvAction {
}

// StateUpdate updates a given state and returns a new state.
// Source resources have wildcards wich should be matched against the tf state. Each occurrence will generate
// Source resources have wildcards which should be matched against the tf state. Each occurrence will generate
// a move command.
func (a *StateXMvAction) StateUpdate(ctx context.Context, tf tfexec.TerraformCLI, state *tfexec.State) (*tfexec.State, error) {
stateMvActions, err := a.generateMvActions(ctx, tf, state)
Expand All @@ -57,7 +57,7 @@ func (a *StateXMvAction) generateMvActions(ctx context.Context, tf tfexec.Terraf

// When a wildcardChar is used in a path it should only match a single part of the path
// It can therefore not contain a dot(.), whitespace nor square brackets
const matchWildcardRegex = "([^\\]\\[\t\n\v\f\r ]*)"
const matchWildcardRegex = "(.*)"
const wildcardChar = "*"

func (a *StateXMvAction) nrOfWildcards() int {
Expand All @@ -73,36 +73,39 @@ func makeSourceMatchPattern(s string) string {
}

// Get a regex that will do matching based on the wildcard source that was given.
func makeSrcRegex(source string) (r *regexp.Regexp, err error) {
func makeSrcRegex(source string) (*regexp.Regexp, error) {
regPattern := makeSourceMatchPattern(source)
r, err = regexp.Compile(regPattern)
regExpression, err := regexp.Compile(regPattern)
if err != nil {
return nil, fmt.Errorf("could not make pattern out of %s (%s) due to %s", source, regPattern, err.Error())
return nil, fmt.Errorf("could not make pattern out of %s (%s) due to %s", source, regPattern, err)
}
return
return regExpression, nil
}

// Look into the state and find sources that match pattern with wild cards.
func (a *StateXMvAction) getMatchingSourcesFromState(stateList []string) (wildcardMatches []string, err error) {
r, e := makeSrcRegex(a.source)
if e != nil {
return nil, e
func (a *StateXMvAction) getMatchingSourcesFromState(stateList []string) (matchingStateSources []string, err error) {
r, err := makeSrcRegex(a.source)
if err != nil {
return nil, err
}
wildcardMatches = r.FindAllString(strings.Join(stateList, "\n"), -1)
if wildcardMatches == nil {
return []string{}, nil

for _, s := range stateList {
match := r.FindString(s)
if match != "" {
matchingStateSources = append(matchingStateSources, match)
}
}
return
return matchingStateSources, err
}

// When you have the stateXMvAction with wildcards get the destination for a source
func (a *StateXMvAction) getDestinationForStateSrc(stateSource string) (destination string, err error) {
r, e := makeSrcRegex(a.source)
if e != nil {
return "", e
r, err := makeSrcRegex(a.source)
if err != nil {
return "", err
}
destination = r.ReplaceAllString(stateSource, a.destination)
return
return destination, err
}

// Get actions matching wildcard move actions based on the list of resources.
Expand All @@ -112,18 +115,17 @@ func (a *StateXMvAction) getStateMvActionsForStateList(stateList []string) (resp
response[0] = NewStateMvAction(a.source, a.destination)
return response, nil
}
matchingSources, e := a.getMatchingSourcesFromState(stateList)
if e != nil {
return nil, e
matchingSources, err := a.getMatchingSourcesFromState(stateList)
if err != nil {
return nil, err
}
response = make([]*StateMvAction, len(matchingSources))
for i, matchingSource := range matchingSources {
destination, e2 := a.getDestinationForStateSrc(matchingSource)
if e2 != nil {
return nil, e2
}
sma := StateMvAction{matchingSource, destination}
response[i] = &sma
response[i] = NewStateMvAction(matchingSource, destination)
}
return
return response, nil
}
84 changes: 59 additions & 25 deletions tfmigrate/test_xmv_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,30 @@ package tfmigrate

import (
"context"
"fmt"
"reflect"
"testing"

"github.com/minamijoyo/tfmigrate/tfexec"
)

func TestAccStateMvActionWildcardRefactorToModule(t *testing.T) {
func TestAccStateMvActionWildcardRenameSecurityGroupResourceNamesFromDocs(t *testing.T) {
tfexec.SkipUnlessAcceptanceTestEnabled(t)

backend := tfexec.GetTestAccBackendS3Config(t.Name())

source := `
resource "aws_security_group" "foo" {}
resource "aws_security_group" "bar" {}
resource "aws_security_group" "baz" {}
`
tf := tfexec.SetupTestAccWithApply(t, "default", backend+source)
ctx := context.Background()

updatedSource := `
module "sg_module" {
source = "./sg_module"
for_each = toset(["foo", "bar", "baz"])
}
`
sgModuleSource := `
resource "aws_security_group" "sg" {}
resource "aws_security_group" "foo2" {}
resource "aws_security_group" "bar2" {}
`

tfexec.AddModuleToTestAcc(t, tf, "sg_module", sgModuleSource)
tfexec.UpdateTestAccSource(t, tf, backend+updatedSource)
tfexec.RunTestAccInitModule(t, tf)

changed, err := tf.PlanHasChange(ctx, nil)
if err != nil {
Expand All @@ -43,7 +36,7 @@ resource "aws_security_group" "sg" {}
}

actions := []StateAction{
NewStateXMvAction("aws_security_group.*", "module.sg_module[\"$1\"].aws_security_group.sg"),
NewStateXMvAction("aws_security_group.*", "aws_security_group.${1}2"),
}

m := NewStateMigrator(tf.Dir(), "default", actions, &MigratorOption{}, false)
Expand Down Expand Up @@ -81,7 +74,7 @@ func TestGetNrOfWildcard(t *testing.T) {
}
}

func TestSingleResourceStateMoves(t *testing.T) {
func TestGetStateMvActionsForStateList(t *testing.T) {
cases := []struct {
desc string
stateList []string
Expand Down Expand Up @@ -139,27 +132,68 @@ func TestSingleResourceStateMoves(t *testing.T) {
},
outputMvActions: []*StateMvAction{},
},
{
desc: "Documented feature; positional matching for example to allow switching matches from place",
stateList: []string{"module[\"bar\"].aws_vpc.foo"},
inputXMvAction: &StateXMvAction{
source: "module[\"*\"].aws_vpc.*",
destination: "module[\"$2\"].aws_vpc.$1",
},
outputMvActions: []*StateMvAction{
{
source: "module[\"bar\"].aws_vpc.foo",
destination: "module[\"foo\"].aws_vpc.bar",
},
},
},
{
desc: "Multiple resources refactored into a module",
stateList: []string{
"aws_security_group.foo",
"aws_security_group.bar",
"aws_security_group.baz",
},
inputXMvAction: &StateXMvAction{
source: "aws_security_group.*",
destination: "module.sg_module[\"$1\"].aws_security_group.sg",
},
outputMvActions: []*StateMvAction{
{
source: "aws_security_group.foo",
destination: "module.sg_module[\"foo\"].aws_security_group.sg",
},
{
source: "aws_security_group.bar",
destination: "module.sg_module[\"bar\"].aws_security_group.sg",
},
{
source: "aws_security_group.baz",
destination: "module.sg_module[\"baz\"].aws_security_group.sg",
},
},
},
}

for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
got, err := tc.inputXMvAction.getStateMvActionsForStateList(tc.stateList)
// Should be location agnostic
// Errors are not expected. At this stage the only location from which errors are expected is if the regular
// expression that comes from the source cannot compile but since meta-characters are quoted and we only
// introduce matched braces and unmatched meta-characters there are no known cases where we would hit this.
// Still this case gets handled explicitly as it can be helpful info if the author missed a case.
if err != nil {
t.Errorf("Encountered error %v", err)
}
if len(got) != len(tc.outputMvActions) {
t.Errorf("Did not get same amount of matches: %d, %d", len(got), len(tc.outputMvActions))
}

for i, outputMvAction := range got {
if tc.outputMvActions[i].source != outputMvAction.source {
t.Errorf("Mistmatching output sources %d: %v, %v", i, tc.outputMvActions[i].source, outputMvAction.source)
}
if tc.outputMvActions[i].destination != outputMvAction.destination {
t.Errorf("Mistmatching output destination %d: %v, %v", i, tc.outputMvActions[i].destination, outputMvAction.destination)
}
if !reflect.DeepEqual(got, tc.outputMvActions) {
t.Errorf("got: %v, expected: %v", got, tc.outputMvActions)
}

})
}
}

// At this time purely for testing purposes if a real string function is needed for code this can be replaced.
func (a StateMvAction) String() string {
return fmt.Sprintf("mv %s %s", a.source, a.destination)
}

0 comments on commit 9c4c900

Please sign in to comment.