diff --git a/.changelog/11918.txt b/.changelog/11918.txt new file mode 100644 index 0000000000..276a77eefa --- /dev/null +++ b/.changelog/11918.txt @@ -0,0 +1,6 @@ +```release-note:bug +config: include all config errors in the error message, previously some could be hidden. +``` +```release-note:bug +snapshot: the `snapshot save` command now saves the snapshot with read permission for only the current user. +``` diff --git a/agent/config/builder.go b/agent/config/builder.go index 7f3ab5e4ef..987a04fbba 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -1631,7 +1631,7 @@ func (b *builder) serviceVal(v *ServiceDefinition) *structs.ServiceDefinition { meta := make(map[string]string) if err := structs.ValidateServiceMetadata(kind, v.Meta, false); err != nil { - b.err = multierror.Append(fmt.Errorf("invalid meta for service %s: %v", stringVal(v.Name), err)) + b.err = multierror.Append(b.err, fmt.Errorf("invalid meta for service %s: %v", stringVal(v.Name), err)) } else { meta = v.Meta } @@ -1646,13 +1646,13 @@ func (b *builder) serviceVal(v *ServiceDefinition) *structs.ServiceDefinition { } if err := structs.ValidateWeights(serviceWeights); err != nil { - b.err = multierror.Append(fmt.Errorf("Invalid weight definition for service %s: %s", stringVal(v.Name), err)) + b.err = multierror.Append(b.err, fmt.Errorf("Invalid weight definition for service %s: %s", stringVal(v.Name), err)) } if (v.Port != nil || v.Address != nil) && (v.SocketPath != nil) { - b.err = multierror.Append( + b.err = multierror.Append(b.err, fmt.Errorf("service %s cannot have both socket path %s and address/port", - stringVal(v.Name), stringVal(v.SocketPath)), b.err) + stringVal(v.Name), stringVal(v.SocketPath))) } return &structs.ServiceDefinition{ @@ -1890,7 +1890,7 @@ func (b *builder) durationValWithDefault(name string, v *string, defaultVal time } d, err := time.ParseDuration(*v) if err != nil { - b.err = multierror.Append(fmt.Errorf("%s: invalid duration: %q: %s", name, *v, err)) + b.err = multierror.Append(b.err, fmt.Errorf("%s: invalid duration: %q: %s", name, *v, err)) } return d } diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index 5901431a0d..80ad7b368d 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -291,3 +291,39 @@ func TestLoad_EmptyClientAddr(t *testing.T) { }) } } + +func TestBuilder_DurationVal_InvalidDuration(t *testing.T) { + b := builder{} + badDuration1 := "not-a-duration" + badDuration2 := "also-not" + b.durationVal("field1", &badDuration1) + b.durationVal("field1", &badDuration2) + + require.Error(t, b.err) + require.Contains(t, b.err.Error(), "2 errors") + require.Contains(t, b.err.Error(), badDuration1) + require.Contains(t, b.err.Error(), badDuration2) +} + +func TestBuilder_ServiceVal_MultiError(t *testing.T) { + b := builder{} + b.serviceVal(&ServiceDefinition{ + Meta: map[string]string{"": "empty-key"}, + Port: intPtr(12345), + SocketPath: strPtr("/var/run/socket.sock"), + Checks: []CheckDefinition{ + {Interval: strPtr("bad-interval")}, + }, + Weights: &ServiceWeights{Passing: intPtr(-1)}, + }) + require.Error(t, b.err) + require.Contains(t, b.err.Error(), "4 errors") + require.Contains(t, b.err.Error(), "bad-interval") + require.Contains(t, b.err.Error(), "Key cannot be blank") + require.Contains(t, b.err.Error(), "Invalid weight") + require.Contains(t, b.err.Error(), "cannot have both socket path") +} + +func intPtr(v int) *int { + return &v +} diff --git a/agent/consul/client.go b/agent/consul/client.go index ac64d704ab..999f986634 100644 --- a/agent/consul/client.go +++ b/agent/consul/client.go @@ -347,7 +347,7 @@ func (c *Client) SnapshotRPC(args *structs.SnapshotRequest, in io.Reader, out io // Let the caller peek at the reply. if replyFn != nil { if err := replyFn(&reply); err != nil { - return nil + return err } } diff --git a/agent/consul/server.go b/agent/consul/server.go index 91082a7d6b..e076af7214 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -1347,7 +1347,7 @@ func (s *Server) SnapshotRPC(args *structs.SnapshotRequest, in io.Reader, out io // Let the caller peek at the reply. if replyFn != nil { if err := replyFn(&reply); err != nil { - return nil + return err } } diff --git a/command/snapshot/save/snapshot_save.go b/command/snapshot/save/snapshot_save.go index 699cf5f1df..e43dcb6126 100644 --- a/command/snapshot/save/snapshot_save.go +++ b/command/snapshot/save/snapshot_save.go @@ -5,11 +5,12 @@ import ( "fmt" "os" + "github.com/mitchellh/cli" + "github.com/rboyer/safeio" + "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/flags" "github.com/hashicorp/consul/snapshot" - "github.com/mitchellh/cli" - "github.com/rboyer/safeio" ) func New(ui cli.Ui) *cmd { @@ -71,7 +72,7 @@ func (c *cmd) Run(args []string) int { // Save the file first. unverifiedFile := file + ".unverified" - if _, err := safeio.WriteToFile(snap, unverifiedFile, 0666); err != nil { + if _, err := safeio.WriteToFile(snap, unverifiedFile, 0600); err != nil { c.UI.Error(fmt.Sprintf("Error writing unverified snapshot file: %s", err)) return 1 } diff --git a/command/snapshot/save/snapshot_save_test.go b/command/snapshot/save/snapshot_save_test.go index 79df0dfc60..10e8abcfea 100644 --- a/command/snapshot/save/snapshot_save_test.go +++ b/command/snapshot/save/snapshot_save_test.go @@ -94,6 +94,10 @@ func TestSnapshotSaveCommand(t *testing.T) { t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) } + fi, err := os.Stat(file) + require.NoError(t, err) + require.Equal(t, fi.Mode(), os.FileMode(0600)) + f, err := os.Open(file) if err != nil { t.Fatalf("err: %v", err)