diff --git a/Taskfile.yml b/Taskfile.yml index 35a0ed3..91c3e19 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -8,25 +8,18 @@ tasks: default: deps: [test] - clean: - desc: Delete build artifacts - cmds: [rm -rf bin/*] - - terravalet: + build: desc: Build the terravalet executable - dir: bin cmds: - - go build -v -ldflags="{{.LDFLAGS}}" .. + - go build -o bin/terravalet -v -ldflags="{{.LDFLAGS}}" . vars: &build-vars FULL_VERSION: sh: git describe --long --dirty --always - SHORT_VERSION: - sh: git describe --abbrev=0 - LDFLAGS: -w -s -X main.fullVersion={{.FULL_VERSION}} -X main.shortVersion={{.SHORT_VERSION}} + LDFLAGS: -w -s -X main.fullVersion={{.FULL_VERSION}} test: desc: Run the integration tests - deps: [terravalet] + deps: [build] cmds: - "{{.TESTRUNNER}} ./..." vars: diff --git a/main.go b/main.go index 14a065a..aca7230 100644 --- a/main.go +++ b/main.go @@ -20,8 +20,7 @@ import ( var ( // Filled by the linker. - fullVersion = "unknown" // example: v0.0.9-8-g941583d027-dirty - shortVersion = "unknown" // example: v0.0.9 + fullVersion = "unknown" // example: v0.0.9-8-g941583d027-dirty ) func main() { @@ -132,7 +131,10 @@ func rename(upPath, downPath, planPath, localStatePath string, fuzzyMatch bool) return fmt.Errorf("required fuzzy-match but there is nothing left to match") } if fuzzyMatch { - upMatches, downMatches = matchFuzzy(create, destroy) + upMatches, downMatches, err = matchFuzzy(create, destroy) + if err != nil { + return fmt.Errorf("fuzzyMatch: %v", err) + } msg := collectErrors(create, destroy) if msg != "" { return fmt.Errorf("matchFuzzy: %v", msg) @@ -344,54 +346,56 @@ func matchExact(create, destroy *strset.Set) (map[string]string, map[string]stri // The criterium used to perform a matchFuzzy is that one of the two elements must be a // fuzzy match of the other, according to some definition of fuzzy. // Note that the longest element could be the old or the new one, it depends on the inputs. -func matchFuzzy(create, destroy *strset.Set) (map[string]string, map[string]string) { +func matchFuzzy(create, destroy *strset.Set) (map[string]string, map[string]string, error) { // old -> new (or equvalenty: destroy -> create) upMatches := map[string]string{} downMatches := map[string]string{} type candidate struct { - word string distance int + create string + destroy string } - reverse := map[string]candidate{} - - destroyL := destroy.List() - sort.Strings(destroyL) - createL := create.List() - sort.Strings(createL) + candidates := []candidate{} - for _, d := range destroyL { - for _, c := range createL { + for _, d := range destroy.List() { + for _, c := range create.List() { // Here we could also use a custom NGramSizes via // stringosim.QGramSimilarityOptions dist := stringosim.QGram([]rune(d), []rune(c)) - curr, ok := reverse[c] - if !ok || dist < curr.distance { - reverse[c] = candidate{d, dist} - } + candidates = append(candidates, candidate{dist, c, d}) } } + sort.Slice(candidates, func(i, j int) bool { return candidates[i].distance < candidates[j].distance }) + + for len(candidates) > 0 { + bestCandidate := candidates[0] + tmpCandidates := []candidate{} + + for _, c := range candidates[1:] { + if bestCandidate.distance == c.distance { + if (bestCandidate.create == c.create) || (bestCandidate.destroy == c.destroy) { + return map[string]string{}, map[string]string{}, + fmt.Errorf("ambiguous migration: {%s} -> {%s} or {%s} -> {%s}", + bestCandidate.create, bestCandidate.destroy, + c.create, c.destroy, + ) + } + } + if (bestCandidate.create != c.create) && (bestCandidate.destroy != c.destroy) { + tmpCandidates = append(tmpCandidates, candidate{c.distance, c.create, c.destroy}) + } - // Sort the reverse index for reproducibility - keys := make([]string, 0, len(reverse)) - for k := range reverse { - keys = append(keys, k) - } - sort.Strings(keys) - // Traverse the reverse index, populate the up and down match maps and update the source sets. - fmt.Printf("WARNING fuzzy match enabled. Double-check the following matches:\n") - for _, k := range keys { - v := reverse[k] - fmt.Printf("%2d %-50s -> %s\n", v.distance, v.word, k) - upMatches[v.word] = k - downMatches[k] = v.word + } - // Remove matched elements from the two sets. - destroy.Remove(v.word) - create.Remove(k) + candidates = tmpCandidates + upMatches[bestCandidate.destroy] = bestCandidate.create + downMatches[bestCandidate.create] = bestCandidate.destroy + destroy.Remove(bestCandidate.destroy) + create.Remove(bestCandidate.create) } - return upMatches, downMatches + return upMatches, downMatches, nil } // Given a map old->new, create a script that for each element in the map issues the diff --git a/main_test.go b/main_test.go index d63ce36..361d68b 100644 --- a/main_test.go +++ b/main_test.go @@ -44,6 +44,13 @@ func TestRunRenameSuccess(t *testing.T) { "testdata/03_fuzzy-match.up.sh", "testdata/03_fuzzy-match.down.sh", }, + { + "q-gram fuzzy match complicated (regression)", + []string{"-fuzzy-match"}, + "testdata/07_fuzzy-match.plan.txt", + "testdata/07_fuzzy-match.up.sh", + "testdata/07_fuzzy-match.down.sh", + }, } for _, tc := range testCases { @@ -437,7 +444,10 @@ func TestMatchFuzzyZeroUnmatched(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - gotUpMatches, gotDownMatches := matchFuzzy(tc.create, tc.destroy) + gotUpMatches, gotDownMatches, err := matchFuzzy(tc.create, tc.destroy) + if err != nil { + t.Fatalf("got: %s; want: no error", err) + } if diff := cmp.Diff(tc.wantUpMatches, gotUpMatches); diff != "" { t.Errorf("\nupMatches: mismatch (-want +got):\n%s", diff) @@ -454,3 +464,18 @@ func TestMatchFuzzyZeroUnmatched(t *testing.T) { }) } } + +func TestMatchFuzzyError(t *testing.T) { + t.Run("ambiguous migration: two different items have the same match", func(t *testing.T) { + create := set.NewStringSet(`abcde`, `abdecde`) + destroy := set.NewStringSet(`abdcde`, `hfjabd`) + wantError := "ambiguous migration: {abcde} -> {abdcde} or {abdecde} -> {abdcde}" + _, _, err := matchFuzzy(create, destroy) + if err == nil { + t.Fatalf("got: no error; want: an ambiguous migration error") + } + if err.Error() != wantError { + t.Fatalf("got: %s; want: %s", err.Error(), wantError) + } + }) +} diff --git a/testdata/07_fuzzy-match.down.sh b/testdata/07_fuzzy-match.down.sh new file mode 100644 index 0000000..fd04979 --- /dev/null +++ b/testdata/07_fuzzy-match.down.sh @@ -0,0 +1,28 @@ +#! /usr/bin/sh +# DO NOT EDIT. Generated by terravalet. +# terravalet_output_format=2 +# +# This script will move 5 items. + +set -e + +terraform state mv -lock=false -state=local.tfstate \ + 'module.prometheus_instance.aws_instance.instance' \ + 'module.prometheus.aws_instance.prometheus' + +terraform state mv -lock=false -state=local.tfstate \ + 'module.prometheus_instance.aws_route53_record.internal' \ + 'aws_route53_record.private["prometheus"]' + +terraform state mv -lock=false -state=local.tfstate \ + 'module.prometheus_instance.aws_security_group_rule.extra_rules["reverseproxy_to_prometheus_pushprox"]' \ + 'aws_security_group_rule.reverseproxy_to_prometheus_pushprox' + +terraform state mv -lock=false -state=local.tfstate \ + 'module.prometheus_instance.aws_volume_attachment.volumes["/dev/xvdh"]' \ + 'module.prometheus.aws_volume_attachment.storage_attach' + +terraform state mv -lock=false -state=local.tfstate \ + 'module.prometheus_instance.null_resource.provision' \ + 'module.prometheus.null_resource.provision_prometheus' + diff --git a/testdata/07_fuzzy-match.plan.txt b/testdata/07_fuzzy-match.plan.txt new file mode 100644 index 0000000..eaa091a --- /dev/null +++ b/testdata/07_fuzzy-match.plan.txt @@ -0,0 +1,13 @@ +# aws_route53_record.private["prometheus"] will be destroyed +# aws_security_group_rule.reverseproxy_to_prometheus_pushprox will be destroyed +# null_resource.configure_alerts will be created +# null_resource.provision_monitor_in_nginx must be replaced +# module.prometheus.aws_instance.prometheus will be destroyed +# module.prometheus.aws_volume_attachment.storage_attach will be destroyed +# module.prometheus.null_resource.configure_alerts will be destroyed +# module.prometheus.null_resource.provision_prometheus will be destroyed +# module.prometheus_instance.aws_instance.instance will be created +# module.prometheus_instance.aws_route53_record.internal will be created +# module.prometheus_instance.aws_security_group_rule.extra_rules["reverseproxy_to_prometheus_pushprox"] will be created +# module.prometheus_instance.aws_volume_attachment.volumes["/dev/xvdh"] will be created +# module.prometheus_instance.null_resource.provision will be created diff --git a/testdata/07_fuzzy-match.up.sh b/testdata/07_fuzzy-match.up.sh new file mode 100644 index 0000000..4963267 --- /dev/null +++ b/testdata/07_fuzzy-match.up.sh @@ -0,0 +1,28 @@ +#! /usr/bin/sh +# DO NOT EDIT. Generated by terravalet. +# terravalet_output_format=2 +# +# This script will move 5 items. + +set -e + +terraform state mv -lock=false -state=local.tfstate \ + 'aws_route53_record.private["prometheus"]' \ + 'module.prometheus_instance.aws_route53_record.internal' + +terraform state mv -lock=false -state=local.tfstate \ + 'aws_security_group_rule.reverseproxy_to_prometheus_pushprox' \ + 'module.prometheus_instance.aws_security_group_rule.extra_rules["reverseproxy_to_prometheus_pushprox"]' + +terraform state mv -lock=false -state=local.tfstate \ + 'module.prometheus.aws_instance.prometheus' \ + 'module.prometheus_instance.aws_instance.instance' + +terraform state mv -lock=false -state=local.tfstate \ + 'module.prometheus.aws_volume_attachment.storage_attach' \ + 'module.prometheus_instance.aws_volume_attachment.volumes["/dev/xvdh"]' + +terraform state mv -lock=false -state=local.tfstate \ + 'module.prometheus.null_resource.provision_prometheus' \ + 'module.prometheus_instance.null_resource.provision' +