hcq/influx-cli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: template apply uses better diff checking (#358)

Browse Source 
* 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
This commit is contained in:
Sam Arnold
2022-02-04 10:59:02 -05:00
committed by GitHub
parent 981be7872d
commit 9ddf110ad6
4 changed files with 56 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

99
clients/apply/apply.go
View File

@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"reflect"
"strconv"
"time"
@ -33,7 +32,6 @@ type Params struct {
Filters []ResourceFilter
Force bool
OverwriteConflicts bool
Quiet bool
RenderTableColors bool
RenderTableBorders bool
@ -114,16 +112,12 @@ func (c Client) Apply(ctx context.Context, params *Params) error {
}
}
if c.StdIO.IsInteractive() && !params.Force {
if !params.Force {
if confirmed := c.StdIO.GetConfirm("Confirm application of the above resources"); !confirmed {
return errors.New("aborted application of template")
}
}
if !params.OverwriteConflicts && hasConflicts(res.Diff) {
return errors.New("template has conflicts with existing resources and cannot safely apply")
}
// Flip the dry-run flag and apply the template.
req.DryRun = false
res, err = c.ApplyTemplate(ctx).TemplateApply(req).Execute()
@ -167,10 +161,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if l.Old != nil {
oldRow = buildRow(l.TemplateMetaName, hexId, *l.Old)
}
if l.New != nil {
if l.New != nil && l.StateStatus != "remove" {
newRow = buildRow(l.TemplateMetaName, hexId, *l.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, false)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -199,10 +193,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if b.Old != nil {
oldRow = buildRow(b.TemplateMetaName, hexId, *b.Old)
}
if b.New != nil {
if b.New != nil && b.StateStatus != "remove" {
newRow = buildRow(b.TemplateMetaName, hexId, *b.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, false)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -223,10 +217,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if c.Old != nil {
oldRow = buildRow(c.TemplateMetaName, hexId, *c.Old)
}
if c.New != nil {
if c.New != nil && c.StateStatus != "remove" {
newRow = buildRow(c.TemplateMetaName, hexId, *c.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, true)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -247,10 +241,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if d.Old != nil {
oldRow = buildRow(d.TemplateMetaName, hexId, *d.Old)
}
if d.New != nil {
if d.New != nil && d.StateStatus != "remove" {
newRow = buildRow(d.TemplateMetaName, hexId, *d.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, true)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -267,10 +261,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if e.Old != nil {
oldRow = buildRow(e.TemplateMetaName, hexId, *e.Old)
}
if e.New != nil {
if e.New != nil && e.StateStatus != "remove" {
newRow = buildRow(e.TemplateMetaName, hexId, *e.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, true)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -292,10 +286,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if r.Old != nil {
oldRow = buildRow(r.TemplateMetaName, hexId, *r.Old)
}
if r.New != nil {
if r.New != nil && r.StateStatus != "remove" {
newRow = buildRow(r.TemplateMetaName, hexId, *r.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, true)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -316,10 +310,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if t.Old != nil {
oldRow = buildRow(t.TemplateMetaName, hexId, *t.Old)
}
if t.New != nil {
if t.New != nil && t.StateStatus != "remove" {
newRow = buildRow(t.TemplateMetaName, hexId, *t.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, true)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -351,10 +345,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if t.Old != nil {
oldRow = buildRow(t.TemplateMetaName, hexId, *t.Old)
}
if t.New != nil {
if t.New != nil && t.StateStatus != "remove" {
newRow = buildRow(t.TemplateMetaName, hexId, *t.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, true)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -378,10 +372,10 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
if v.Old != nil {
oldRow = buildRow(v.TemplateMetaName, hexId, *v.Old)
}
if v.New != nil {
if v.New != nil && v.StateStatus != "remove" {
newRow = buildRow(v.TemplateMetaName, hexId, *v.New)
}
printer.AppendDiff(oldRow, newRow)
printer.AppendDiff(oldRow, newRow, false)
}
printer.Render()
_, _ = c.StdIO.Write([]byte("\n"))
@ -398,11 +392,11 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
row := []string{m.ResourceType, m.ResourceName, resId, m.LabelTemplateMetaName, m.LabelName, labelId}
switch m.StateStatus {
case "new":
printer.AppendDiff(nil, row)
printer.AppendDiff(nil, row, false)
case "remove":
printer.AppendDiff(row, nil)
printer.AppendDiff(row, nil, false)
default:
printer.AppendDiff(row, row)
printer.AppendDiff(row, row, false)
}
}
printer.Render()
@ -411,55 +405,6 @@ func (c Client) printDiff(diff api.TemplateSummaryDiff, params *Params) error {
return nil
}
func hasConflicts(diff api.TemplateSummaryDiff) bool {
for _, l := range diff.Labels {
if l.StateStatus != "new" && l.Old != nil && l.New != nil && !reflect.DeepEqual(l.Old, l.New) {
return true
}
}
for _, b := range diff.Buckets {
if b.StateStatus != "new" && b.Old != nil && b.New != nil && !reflect.DeepEqual(b.Old, b.New) {
return true
}
}
for _, c := range diff.Checks {
if c.StateStatus != "new" && c.Old != nil && c.New != nil && !reflect.DeepEqual(c.Old, c.New) {
return true
}
}
for _, d := range diff.Dashboards {
if d.StateStatus != "new" && d.Old != nil && d.New != nil && !reflect.DeepEqual(d.Old, d.New) {
return true
}
}
for _, e := range diff.NotificationEndpoints {
if e.StateStatus != "new" && e.Old != nil && e.New != nil && !reflect.DeepEqual(e.Old, e.New) {
return true
}
}
for _, r := range diff.NotificationRules {
if r.StateStatus != "new" && r.Old != nil && r.New != nil && !reflect.DeepEqual(r.Old, r.New) {
return true
}
}
for _, t := range diff.TelegrafConfigs {
if t.StateStatus != "new" && t.Old != nil && t.New != nil && !reflect.DeepEqual(t.Old, t.New) {
return true
}
}
for _, t := range diff.Tasks {
if t.StateStatus != "new" && t.Old != nil && t.New != nil && !reflect.DeepEqual(t.Old, t.New) {
return true
}
}
for _, v := range diff.Variables {
if v.StateStatus != "new" && v.Old != nil && v.New != nil && !reflect.DeepEqual(v.Old, v.New) {
return true
}
}
return false
}
func (c Client) printSummary(summary api.TemplateSummaryResources, params *Params) error {
if c.PrintAsJSON {
return c.PrintJSON(struct {

10
cmd/influx/apply.go
View File

@ -133,7 +133,7 @@ https://github.com/influxdata/community-templates.
},
&cli.StringFlag{
Name: "force",
Usage: "Set to 'true' to skip confirmation before applying changes. Set to 'conflict' to skip confirmation and overwrite existing resources",
Usage: "Set to 'yes' to skip confirmation before applying changes (recommended for non-interactive scripts).",
Destination: &params.force,
},
&cli.StringSliceFlag{
@ -240,12 +240,18 @@ https://github.com/influxdata/community-templates.
// Parse our strange way of passing 'force'
switch params.force {
case "":
// no force
case "conflict":
log.Println("WARN: Passing '--force conflict' is deprecated, assuming '--force yes'")
parsedParams.Force = true
parsedParams.OverwriteConflicts = true
case "true":
log.Println("WARN: Passing '--force true' is deprecated, assuming '--force yes'")
parsedParams.Force = true
case "yes":
parsedParams.Force = true
default:
return fmt.Errorf("invalid argument '--force %s', should use '--force yes'", params.force)
}
api := getAPI(ctx)

8
pkg/template/diff_printer.go
View File

@ -106,7 +106,11 @@ func (d *DiffPrinter) Append(slc []string) {
d.writer.Append(d.prepend(slc, ""))
}
func (d *DiffPrinter) AppendDiff(remove, add []string) {
// AppendDiff appends a diff to the diff printer
//
// assumeDiff says to mark remove/add as a diff (with two lines), even if they are the same.
// this is used for types that the CLI does not know how to fully compare.
func (d *DiffPrinter) AppendDiff(remove, add []string, assumeDiff bool) {
defer func() { d.appendCalls++ }()
if d.appendCalls > 0 {
@ -127,7 +131,7 @@ func (d *DiffPrinter) AppendDiff(remove, add []string) {
var (
addColors = make([]tablewriter.Colors, len(preppedAdd))
removeColors = make([]tablewriter.Colors, len(preppedRemove))
hasDiff bool
hasDiff = assumeDiff
)
addColor, removeColor := noColor, noColor
if d.useColor {

24
pkg/template/diff_printer_test.go
View File

@ -29,19 +29,27 @@ func TestDiffPrinter(t *testing.T) {
SetHeaders("Wow", "Such", "A", "Fancy", "Printer")
// Add
printer.AppendDiff(nil, []string{"A", "B", "C", "D", "E"})
printer.AppendDiff(nil, []string{"A", "B", "C", "D", "E"}, false)
// No change
printer.Append([]string{"foo", "bar", "baz", "qux", "wat"})
simple := []string{"foo", "bar", "baz", "qux", "wat"}
printer.Append(simple)
// No change with two arguments
printer.AppendDiff(simple, simple, false)
// No change but force a diff
printer.AppendDiff(simple, simple, true)
// Replace
printer.AppendDiff(
[]string{"1", "200000000000000", "3", "4", "5"},
[]string{"9", "8", "7", "6", "5"},
false,
)
// Remove
printer.AppendDiff([]string{"x y", "z x", "x y z", "", "y z"}, nil)
printer.AppendDiff([]string{"x y", "z x", "x y z", "", "y z"}, nil, false)
printer.Render()
expected := `EXAMPLE +add | -remove | unchanged
@ -53,6 +61,14 @@ func TestDiffPrinter(t *testing.T) {
| | foo | bar | baz | qux | wat |
+-----+-----+-----------------+-------+-------+---------+
+-----+-----+-----------------+-------+-------+---------+
| | foo | bar | baz | qux | wat |
+-----+-----+-----------------+-------+-------+---------+
+-----+-----+-----------------+-------+-------+---------+
| - | foo | bar | baz | qux | wat |
+-----+-----+-----------------+-------+-------+---------+
| + | foo | bar | baz | qux | wat |
+-----+-----+-----------------+-------+-------+---------+
+-----+-----+-----------------+-------+-------+---------+
| - | 1 | 200000000000000 | 3 | 4 | 5 |
+-----+-----+-----------------+-------+-------+---------+
| + | 9 | 8 | 7 | 6 | 5 |
@ -60,7 +76,7 @@ func TestDiffPrinter(t *testing.T) {
+-----+-----+-----------------+-------+-------+---------+
| - | x y | z x | x y z | | y z |
+-----+-----+-----------------+-------+-------+---------+
| TOTAL | 3 |
| TOTAL | 5 |
+-----+-----+-----------------+-------+-------+---------+
`
require.Equal(t, expected, out.String())