Skip to content

Commit 9ddf110

Browse files
lesamlesam
authored
fix: template apply uses better diff checking (#358)
* fix: template apply uses better diff checking Previously, we did a DeepEqual of all the returned data about each changed entity, but due to our template overrides that is not actually all the information available for each entity. So we marked trivial things as 'conflicts' (e.g. telegraf config ID's 'changing' from the empty string to the real, current value) while not catching important conflicts like flux script changes in checks and tasks. Changes to make things more straightforward: * Change the --force behaviour to be more similar to `apt install`, where even in non-interactive mode `--force yes` is required to bypass the prompt to apply. * Before, there were two stages of diff checking - once to print diffs, and once after the 'Yes/No' prompt. If any conflicts were detected in the second check, the user got an inscrutable error message that did not highlight what the difference was or how to force it to apply. `--force conflict` was required to avoid this error. Instead, we now have simplified to `--force yes` to bypass the 'Yes/No' prompt, and we never do a second stage of diff checking. * Because we do not currently properly account for more complicated diffs (e.g. flux tasks), we now assume in the diff printing that every object has changes, except for Labels, Buckets, Variables, and Label Mappings. This could be improved in the future. * fix: when statestatus is 'remove', mark as removed
1 parent 981be78 commit 9ddf110

File tree

4 files changed

+56
-85
lines changed

4 files changed

+56
-85
lines changed

‎clients/apply/apply.go‎

Lines changed: 22 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"reflect"
87
"strconv"
98
"time"
109

@@ -33,7 +32,6 @@ type Params struct {
3332
Filters []ResourceFilter
3433

3534
Force bool
36-
OverwriteConflicts bool
3735
Quiet bool
3836
RenderTableColors bool
3937
RenderTableBorders bool
@@ -114,16 +112,12 @@ func (c Client) Apply(ctx context.Context, params *Params) error {
114112
}
115113
}
116114

117-
if c.StdIO.IsInteractive() && !params.Force {
115+
if !params.Force {
118116
if confirmed := c.StdIO.GetConfirm("Confirm application of the above resources"); !confirmed {
119117
return errors.New("aborted application of template")
120118
}
121119
}
122120

123-
if !params.OverwriteConflicts && hasConflicts(res.Diff) {
124-
return errors.New("template has conflicts with existing resources and cannot safely apply")
125-
}
126-
127121
// Flip the dry-run flag and apply the template.
128122
req.DryRun = false
129123
res, err = c.ApplyTemplate(ctx).TemplateApply(req).Execute()
@@ -167,10 +161,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
167161
if l.Old != nil {
168162
oldRow = buildRow(l.TemplateMetaName, hexId, *l.Old)
169163
}
170-
if l.New != nil {
164+
if l.New != nil && l.StateStatus != "remove" {
171165
newRow = buildRow(l.TemplateMetaName, hexId, *l.New)
172166
}
173-
printer.AppendDiff(oldRow, newRow)
167+
printer.AppendDiff(oldRow, newRow, false)
174168
}
175169
printer.Render()
176170
_, _ = c.StdIO.Write([]byte("\n"))
@@ -199,10 +193,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
199193
if b.Old != nil {
200194
oldRow = buildRow(b.TemplateMetaName, hexId, *b.Old)
201195
}
202-
if b.New != nil {
196+
if b.New != nil && b.StateStatus != "remove" {
203197
newRow = buildRow(b.TemplateMetaName, hexId, *b.New)
204198
}
205-
printer.AppendDiff(oldRow, newRow)
199+
printer.AppendDiff(oldRow, newRow, false)
206200
}
207201
printer.Render()
208202
_, _ = c.StdIO.Write([]byte("\n"))
@@ -223,10 +217,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
223217
if c.Old != nil {
224218
oldRow = buildRow(c.TemplateMetaName, hexId, *c.Old)
225219
}
226-
if c.New != nil {
220+
if c.New != nil && c.StateStatus != "remove" {
227221
newRow = buildRow(c.TemplateMetaName, hexId, *c.New)
228222
}
229-
printer.AppendDiff(oldRow, newRow)
223+
printer.AppendDiff(oldRow, newRow, true)
230224
}
231225
printer.Render()
232226
_, _ = c.StdIO.Write([]byte("\n"))
@@ -247,10 +241,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
247241
if d.Old != nil {
248242
oldRow = buildRow(d.TemplateMetaName, hexId, *d.Old)
249243
}
250-
if d.New != nil {
244+
if d.New != nil && d.StateStatus != "remove" {
251245
newRow = buildRow(d.TemplateMetaName, hexId, *d.New)
252246
}
253-
printer.AppendDiff(oldRow, newRow)
247+
printer.AppendDiff(oldRow, newRow, true)
254248
}
255249
printer.Render()
256250
_, _ = c.StdIO.Write([]byte("\n"))
@@ -267,10 +261,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
267261
if e.Old != nil {
268262
oldRow = buildRow(e.TemplateMetaName, hexId, *e.Old)
269263
}
270-
if e.New != nil {
264+
if e.New != nil && e.StateStatus != "remove" {
271265
newRow = buildRow(e.TemplateMetaName, hexId, *e.New)
272266
}
273-
printer.AppendDiff(oldRow, newRow)
267+
printer.AppendDiff(oldRow, newRow, true)
274268
}
275269
printer.Render()
276270
_, _ = c.StdIO.Write([]byte("\n"))
@@ -292,10 +286,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
292286
if r.Old != nil {
293287
oldRow = buildRow(r.TemplateMetaName, hexId, *r.Old)
294288
}
295-
if r.New != nil {
289+
if r.New != nil && r.StateStatus != "remove" {
296290
newRow = buildRow(r.TemplateMetaName, hexId, *r.New)
297291
}
298-
printer.AppendDiff(oldRow, newRow)
292+
printer.AppendDiff(oldRow, newRow, true)
299293
}
300294
printer.Render()
301295
_, _ = c.StdIO.Write([]byte("\n"))
@@ -316,10 +310,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
316310
if t.Old != nil {
317311
oldRow = buildRow(t.TemplateMetaName, hexId, *t.Old)
318312
}
319-
if t.New != nil {
313+
if t.New != nil && t.StateStatus != "remove" {
320314
newRow = buildRow(t.TemplateMetaName, hexId, *t.New)
321315
}
322-
printer.AppendDiff(oldRow, newRow)
316+
printer.AppendDiff(oldRow, newRow, true)
323317
}
324318
printer.Render()
325319
_, _ = c.StdIO.Write([]byte("\n"))
@@ -351,10 +345,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
351345
if t.Old != nil {
352346
oldRow = buildRow(t.TemplateMetaName, hexId, *t.Old)
353347
}
354-
if t.New != nil {
348+
if t.New != nil && t.StateStatus != "remove" {
355349
newRow = buildRow(t.TemplateMetaName, hexId, *t.New)
356350
}
357-
printer.AppendDiff(oldRow, newRow)
351+
printer.AppendDiff(oldRow, newRow, true)
358352
}
359353
printer.Render()
360354
_, _ = c.StdIO.Write([]byte("\n"))
@@ -378,10 +372,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
378372
if v.Old != nil {
379373
oldRow = buildRow(v.TemplateMetaName, hexId, *v.Old)
380374
}
381-
if v.New != nil {
375+
if v.New != nil && v.StateStatus != "remove" {
382376
newRow = buildRow(v.TemplateMetaName, hexId, *v.New)
383377
}
384-
printer.AppendDiff(oldRow, newRow)
378+
printer.AppendDiff(oldRow, newRow, false)
385379
}
386380
printer.Render()
387381
_, _ = c.StdIO.Write([]byte("\n"))
@@ -398,11 +392,11 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
398392
row := []string{m.ResourceType, m.ResourceName, resId, m.LabelTemplateMetaName, m.LabelName, labelId}
399393
switch m.StateStatus {
400394
case "new":
401-
printer.AppendDiff(nil, row)
395+
printer.AppendDiff(nil, row, false)
402396
case "remove":
403-
printer.AppendDiff(row, nil)
397+
printer.AppendDiff(row, nil, false)
404398
default:
405-
printer.AppendDiff(row, row)
399+
printer.AppendDiff(row, row, false)
406400
}
407401
}
408402
printer.Render()
@@ -411,55 +405,6 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
411405
return nil
412406
}
413407

414-
func hasConflicts(diff api.TemplateSummaryDiff) bool {
415-
for _, l := range diff.Labels {
416-
if l.StateStatus != "new" && l.Old != nil && l.New != nil && !reflect.DeepEqual(l.Old, l.New) {
417-
return true
418-
}
419-
}
420-
for _, b := range diff.Buckets {
421-
if b.StateStatus != "new" && b.Old != nil && b.New != nil && !reflect.DeepEqual(b.Old, b.New) {
422-
return true
423-
}
424-
}
425-
for _, c := range diff.Checks {
426-
if c.StateStatus != "new" && c.Old != nil && c.New != nil && !reflect.DeepEqual(c.Old, c.New) {
427-
return true
428-
}
429-
}
430-
for _, d := range diff.Dashboards {
431-
if d.StateStatus != "new" && d.Old != nil && d.New != nil && !reflect.DeepEqual(d.Old, d.New) {
432-
return true
433-
}
434-
}
435-
for _, e := range diff.NotificationEndpoints {
436-
if e.StateStatus != "new" && e.Old != nil && e.New != nil && !reflect.DeepEqual(e.Old, e.New) {
437-
return true
438-
}
439-
}
440-
for _, r := range diff.NotificationRules {
441-
if r.StateStatus != "new" && r.Old != nil && r.New != nil && !reflect.DeepEqual(r.Old, r.New) {
442-
return true
443-
}
444-
}
445-
for _, t := range diff.TelegrafConfigs {
446-
if t.StateStatus != "new" && t.Old != nil && t.New != nil && !reflect.DeepEqual(t.Old, t.New) {
447-
return true
448-
}
449-
}
450-
for _, t := range diff.Tasks {
451-
if t.StateStatus != "new" && t.Old != nil && t.New != nil && !reflect.DeepEqual(t.Old, t.New) {
452-
return true
453-
}
454-
}
455-
for _, v := range diff.Variables {
456-
if v.StateStatus != "new" && v.Old != nil && v.New != nil && !reflect.DeepEqual(v.Old, v.New) {
457-
return true
458-
}
459-
}
460-
return false
461-
}
462-
463408
func (c Client) printSummary(summary api.TemplateSummaryResources, params *Params) error {
464409
if c.PrintAsJSON {
465410
return c.PrintJSON(struct {

‎cmd/influx/apply.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ https://github.com/influxdata/community-templates.
133133
},
134134
&cli.StringFlag{
135135
Name: "force",
136-
Usage: "Set to 'true' to skip confirmation before applying changes. Set to 'conflict' to skip confirmation and overwrite existing resources",
136+
Usage: "Set to 'yes' to skip confirmation before applying changes (recommended for non-interactive scripts).",
137137
Destination: &params.force,
138138
},
139139
&cli.StringSliceFlag{
@@ -240,12 +240,18 @@ https://github.com/influxdata/community-templates.
240240

241241
// Parse our strange way of passing 'force'
242242
switch params.force {
243+
case "":
244+
// no force
243245
case "conflict":
246+
log.Println("WARN: Passing '--force conflict' is deprecated, assuming '--force yes'")
244247
parsedParams.Force = true
245-
parsedParams.OverwriteConflicts = true
246248
case "true":
249+
log.Println("WARN: Passing '--force true' is deprecated, assuming '--force yes'")
250+
parsedParams.Force = true
251+
case "yes":
247252
parsedParams.Force = true
248253
default:
254+
return fmt.Errorf("invalid argument '--force %s', should use '--force yes'", params.force)
249255
}
250256

251257
api := getAPI(ctx)

‎pkg/template/diff_printer.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,11 @@ func (d *DiffPrinter) Append(slc []string) {
106106
d.writer.Append(d.prepend(slc, ""))
107107
}
108108

109-
func (d *DiffPrinter) AppendDiff(remove, add []string) {
109+
// AppendDiff appends a diff to the diff printer
110+
//
111+
// assumeDiff says to mark remove/add as a diff (with two lines), even if they are the same.
112+
// this is used for types that the CLI does not know how to fully compare.
113+
func (d *DiffPrinter) AppendDiff(remove, add []string, assumeDiff bool) {
110114
defer func() { d.appendCalls++ }()
111115

112116
if d.appendCalls > 0 {
@@ -127,7 +131,7 @@ func (d *DiffPrinter) AppendDiff(remove, add []string) {
127131
var (
128132
addColors = make([]tablewriter.Colors, len(preppedAdd))
129133
removeColors = make([]tablewriter.Colors, len(preppedRemove))
130-
hasDiff bool
134+
hasDiff = assumeDiff
131135
)
132136
addColor, removeColor := noColor, noColor
133137
if d.useColor {

‎pkg/template/diff_printer_test.go‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,27 @@ func TestDiffPrinter(t *testing.T) {
2929
SetHeaders("Wow", "Such", "A", "Fancy", "Printer")
3030

3131
// Add
32-
printer.AppendDiff(nil, []string{"A", "B", "C", "D", "E"})
32+
printer.AppendDiff(nil, []string{"A", "B", "C", "D", "E"}, false)
3333

3434
// No change
35-
printer.Append([]string{"foo", "bar", "baz", "qux", "wat"})
35+
simple := []string{"foo", "bar", "baz", "qux", "wat"}
36+
printer.Append(simple)
37+
38+
// No change with two arguments
39+
printer.AppendDiff(simple, simple, false)
40+
41+
// No change but force a diff
42+
printer.AppendDiff(simple, simple, true)
3643

3744
// Replace
3845
printer.AppendDiff(
3946
[]string{"1", "200000000000000", "3", "4", "5"},
4047
[]string{"9", "8", "7", "6", "5"},
48+
false,
4149
)
4250

4351
// Remove
44-
printer.AppendDiff([]string{"x y", "z x", "x y z", "", "y z"}, nil)
52+
printer.AppendDiff([]string{"x y", "z x", "x y z", "", "y z"}, nil, false)
4553

4654
printer.Render()
4755
expected := `EXAMPLE +add | -remove | unchanged
@@ -53,14 +61,22 @@ func TestDiffPrinter(t *testing.T) {
5361
| | foo | bar | baz | qux | wat |
5462
+-----+-----+-----------------+-------+-------+---------+
5563
+-----+-----+-----------------+-------+-------+---------+
64+
| | foo | bar | baz | qux | wat |
65+
+-----+-----+-----------------+-------+-------+---------+
66+
+-----+-----+-----------------+-------+-------+---------+
67+
| - | foo | bar | baz | qux | wat |
68+
+-----+-----+-----------------+-------+-------+---------+
69+
| + | foo | bar | baz | qux | wat |
70+
+-----+-----+-----------------+-------+-------+---------+
71+
+-----+-----+-----------------+-------+-------+---------+
5672
| - | 1 | 200000000000000 | 3 | 4 | 5 |
5773
+-----+-----+-----------------+-------+-------+---------+
5874
| + | 9 | 8 | 7 | 6 | 5 |
5975
+-----+-----+-----------------+-------+-------+---------+
6076
+-----+-----+-----------------+-------+-------+---------+
6177
| - | x y | z x | x y z | | y z |
6278
+-----+-----+-----------------+-------+-------+---------+
63-
| TOTAL | 3 |
79+
| TOTAL | 5 |
6480
+-----+-----+-----------------+-------+-------+---------+
6581
`
6682
require.Equal(t, expected, out.String())

0 commit comments

Comments
 (0)