From 18ea913752aeadc24b4fd404bfa2bb2a76bc27db Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Wed, 20 Jul 2016 00:39:47 -0700 Subject: [PATCH 1/3] update to allow removing pending changes Signed-off-by: David Lawrence (github: endophage) --- client/changelist/changelist.go | 18 ++++++++++++++++ client/changelist/file_changelist.go | 30 ++++++++++++++++++++++---- client/changelist/interface.go | 3 +++ cmd/notary/prettyprint.go | 5 ++++- cmd/notary/tuf.go | 32 ++++++++++++++++++++++------ 5 files changed, 77 insertions(+), 11 deletions(-) diff --git a/client/changelist/changelist.go b/client/changelist/changelist.go index a91b16cb3..9b52981ad 100644 --- a/client/changelist/changelist.go +++ b/client/changelist/changelist.go @@ -21,6 +21,24 @@ func (cl *memChangelist) Add(c Change) error { return nil } +// Remove deletes the changes found at the given indices +func (cl *memChangelist) Remove(idxs []int) error { + remove := make(map[int]struct{}) + for _, i := range idxs { + remove[i] = struct{}{} + } + var keep []Change + + for i, c := range cl.changes { + if _, ok := remove[i]; ok { + continue + } + keep = append(keep, c) + } + cl.changes = keep + return nil +} + // Clear empties the changelist file. func (cl *memChangelist) Clear(archive string) error { // appending to a nil list initializes it. diff --git a/client/changelist/file_changelist.go b/client/changelist/file_changelist.go index 687427547..fe9fdbea1 100644 --- a/client/changelist/file_changelist.go +++ b/client/changelist/file_changelist.go @@ -5,12 +5,12 @@ import ( "fmt" "io/ioutil" "os" - "path" "sort" "time" "github.com/Sirupsen/logrus" "github.com/docker/distribution/uuid" + "path/filepath" ) // FileChangelist stores all the changes as files @@ -52,7 +52,7 @@ func getFileNames(dirName string) ([]os.FileInfo, error) { // Read a JSON formatted file from disk; convert to TUFChange struct func unmarshalFile(dirname string, f os.FileInfo) (*TUFChange, error) { c := &TUFChange{} - raw, err := ioutil.ReadFile(path.Join(dirname, f.Name())) + raw, err := ioutil.ReadFile(filepath.Join(dirname, f.Name())) if err != nil { return c, err } @@ -89,7 +89,29 @@ func (cl FileChangelist) Add(c Change) error { return err } filename := fmt.Sprintf("%020d_%s.change", time.Now().UnixNano(), uuid.Generate()) - return ioutil.WriteFile(path.Join(cl.dir, filename), cJSON, 0644) + return ioutil.WriteFile(filepath.Join(cl.dir, filename), cJSON, 0644) +} + +// Remove deletes the changes found at the given indices +func (cl FileChangelist) Remove(idxs []int) error { + fileInfos, err := getFileNames(cl.dir) + if err != nil { + return err + } + sort.Sort(fileChanges(fileInfos)) + remove := make(map[int]struct{}) + for _, i := range idxs { + remove[i] = struct{}{} + } + for i, c := range fileInfos { + if _, ok := remove[i]; ok { + file := filepath.Join(cl.dir, c.Name()) + if err := os.Remove(file); err != nil { + logrus.Errorf("could not remove change %d: %s", i, err.Error()) + } + } + } + return nil } // Clear clears the change list @@ -104,7 +126,7 @@ func (cl FileChangelist) Clear(archive string) error { return err } for _, f := range files { - os.Remove(path.Join(cl.dir, f.Name())) + os.Remove(filepath.Join(cl.dir, f.Name())) } return nil } diff --git a/client/changelist/interface.go b/client/changelist/interface.go index 4369623c8..a319b7b89 100644 --- a/client/changelist/interface.go +++ b/client/changelist/interface.go @@ -15,6 +15,9 @@ type Changelist interface { // to save a copy of the changelist in that location Clear(archive string) error + // Remove deletes the changes corresponding with the indices given + Remove(idxs []int) error + // Close syncronizes any pending writes to the underlying // storage and closes the file/connection Close() error diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index 51201870e..e6b0671d6 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -13,7 +13,10 @@ import ( "github.com/docker/notary/tuf/data" ) -const fourItemRow = "%s\t%s\t%s\t%s\n" +const ( + fourItemRow = "%s\t%s\t%s\t%s\n" + fiveItemRow = "%s\t%s\t%s\t%s\t%s\n" +) func initTabWriter(columns []string, writer io.Writer) *tabwriter.Writer { tw := tabwriter.NewWriter(writer, 4, 4, 4, ' ', 0) diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 309291f50..33078e5c7 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -97,6 +97,8 @@ type tufCommander struct { input string output string quiet bool + + changes []int } func (t *tufCommander) AddToCommand(cmd *cobra.Command) { @@ -104,7 +106,10 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmdTUFInit.Flags().StringVar(&t.rootKey, "rootkey", "", "Root key to initialize the repository with") cmd.AddCommand(cmdTUFInit) - cmd.AddCommand(cmdTUFStatusTemplate.ToCommand(t.tufStatus)) + cmdStatus := cmdTUFStatusTemplate.ToCommand(t.tufStatus) + cmdStatus.Flags().IntSliceVarP(&t.changes, "unstage", "u", nil, "Numbers of changes to delete, as show in status list") + cmd.AddCommand(cmdStatus) + cmd.AddCommand(cmdTUFPublishTemplate.ToCommand(t.tufPublish)) cmd.AddCommand(cmdTUFLookupTemplate.ToCommand(t.tufLookup)) @@ -440,17 +445,32 @@ func (t *tufCommander) tufStatus(cmd *cobra.Command, args []string) error { return err } + if len(t.changes) > 0 { + return cl.Remove(t.changes) + } + if len(cl.List()) == 0 { cmd.Printf("No unpublished changes for %s\n", gun) return nil } cmd.Printf("Unpublished changes for %s:\n\n", gun) - cmd.Printf("%-10s%-10s%-12s%s\n", "action", "scope", "type", "path") - cmd.Println("----------------------------------------------------") - for _, ch := range cl.List() { - cmd.Printf("%-10s%-10s%-12s%s\n", ch.Action(), ch.Scope(), ch.Type(), ch.Path()) - } + tw := initTabWriter( + []string{"#", "ACTION", "SCOPE", "TYPE", "PATH"}, + cmd.Out(), + ) + for i, ch := range cl.List() { + fmt.Fprintf( + tw, + fiveItemRow, + fmt.Sprintf("%d", i), + ch.Action(), + ch.Scope(), + ch.Type(), + ch.Path(), + ) + } + tw.Flush() return nil } From b14b5e106116dd5956d3fea4cadda9eacd9c60db Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Wed, 20 Jul 2016 10:27:04 -0700 Subject: [PATCH 2/3] adding reset flag to status command, and adding log statement when a change can't be applied before aborting Signed-off-by: David Lawrence (github: endophage) --- client/helpers.go | 3 ++- cmd/notary/tuf.go | 9 ++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/client/helpers.go b/client/helpers.go index f188c3a7a..f048aa1bb 100644 --- a/client/helpers.go +++ b/client/helpers.go @@ -49,10 +49,11 @@ func applyChangelist(repo *tuf.Repo, cl changelist.Changelist) error { default: logrus.Debug("scope not supported: ", c.Scope()) } - index++ if err != nil { + logrus.Debugf("error attempting to apply change #%d: %s, on scope: %s path: %s type: %s", index, c.Action(), c.Scope(), c.Path(), c.Type()) return err } + index++ } logrus.Debugf("applied %d change(s)", index) return nil diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index 33078e5c7..b8cdc1d2d 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -98,7 +98,9 @@ type tufCommander struct { output string quiet bool - changes []int + changes []int + resetStatus bool + archiveChangelist string } func (t *tufCommander) AddToCommand(cmd *cobra.Command) { @@ -108,6 +110,7 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmdStatus := cmdTUFStatusTemplate.ToCommand(t.tufStatus) cmdStatus.Flags().IntSliceVarP(&t.changes, "unstage", "u", nil, "Numbers of changes to delete, as show in status list") + cmdStatus.Flags().BoolVar(&t.resetStatus, "reset", false, "Reset the changelist for the GUN by deleting all pending changes") cmd.AddCommand(cmdStatus) cmd.AddCommand(cmdTUFPublishTemplate.ToCommand(t.tufPublish)) @@ -445,6 +448,10 @@ func (t *tufCommander) tufStatus(cmd *cobra.Command, args []string) error { return err } + if t.resetStatus { + return cl.Clear(t.archiveChangelist) + } + if len(t.changes) > 0 { return cl.Remove(t.changes) } From 57379e8a01f485d02acbfba1dd397fb0080c93b2 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Mon, 1 Aug 2016 10:17:08 -0700 Subject: [PATCH 3/3] tests and PR comments for status cleanup Signed-off-by: David Lawrence (github: endophage) --- client/changelist/changelist_test.go | 23 ++++++++++ client/changelist/file_changelist.go | 5 +-- cmd/notary/tuf.go | 14 +++--- cmd/notary/tuf_test.go | 64 ++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 10 deletions(-) diff --git a/client/changelist/changelist_test.go b/client/changelist/changelist_test.go index b2b486c9c..c3859c08d 100644 --- a/client/changelist/changelist_test.go +++ b/client/changelist/changelist_test.go @@ -64,3 +64,26 @@ func TestMemChangeIterator(t *testing.T) { var iterError IteratorBoundsError require.IsType(t, iterError, err, "IteratorBoundsError type") } + +func TestMemChangelistRemove(t *testing.T) { + cl := memChangelist{} + it, err := cl.NewIterator() + require.Nil(t, err, "Non-nil error from NewIterator") + require.False(t, it.HasNext(), "HasNext returns false for empty ChangeList") + + c1 := NewTUFChange(ActionCreate, "t1", "target1", "test/targ1", []byte{1}) + cl.Add(c1) + + c2 := NewTUFChange(ActionUpdate, "t2", "target2", "test/targ2", []byte{2}) + cl.Add(c2) + + c3 := NewTUFChange(ActionUpdate, "t3", "target3", "test/targ3", []byte{3}) + cl.Add(c3) + + err = cl.Remove([]int{0, 1}) + require.NoError(t, err) + + chs := cl.List() + require.Len(t, chs, 1) + require.Equal(t, "t3", chs[0].Scope()) +} diff --git a/client/changelist/file_changelist.go b/client/changelist/file_changelist.go index fe9fdbea1..a25215482 100644 --- a/client/changelist/file_changelist.go +++ b/client/changelist/file_changelist.go @@ -46,6 +46,7 @@ func getFileNames(dirName string) ([]os.FileInfo, error) { } fileInfos = append(fileInfos, f) } + sort.Sort(fileChanges(fileInfos)) return fileInfos, nil } @@ -70,7 +71,6 @@ func (cl FileChangelist) List() []Change { if err != nil { return changes } - sort.Sort(fileChanges(fileInfos)) for _, f := range fileInfos { c, err := unmarshalFile(cl.dir, f) if err != nil { @@ -98,7 +98,6 @@ func (cl FileChangelist) Remove(idxs []int) error { if err != nil { return err } - sort.Sort(fileChanges(fileInfos)) remove := make(map[int]struct{}) for _, i := range idxs { remove[i] = struct{}{} @@ -115,6 +114,7 @@ func (cl FileChangelist) Remove(idxs []int) error { } // Clear clears the change list +// N.B. archiving not currently implemented func (cl FileChangelist) Clear(archive string) error { dir, err := os.Open(cl.dir) if err != nil { @@ -143,7 +143,6 @@ func (cl FileChangelist) NewIterator() (ChangeIterator, error) { if err != nil { return &FileChangeListIterator{}, err } - sort.Sort(fileChanges(fileInfos)) return &FileChangeListIterator{dirname: cl.dir, collection: fileInfos}, nil } diff --git a/cmd/notary/tuf.go b/cmd/notary/tuf.go index b8cdc1d2d..7ef3e7f7d 100644 --- a/cmd/notary/tuf.go +++ b/cmd/notary/tuf.go @@ -98,8 +98,8 @@ type tufCommander struct { output string quiet bool - changes []int - resetStatus bool + deleteIdx []int + reset bool archiveChangelist string } @@ -109,8 +109,8 @@ func (t *tufCommander) AddToCommand(cmd *cobra.Command) { cmd.AddCommand(cmdTUFInit) cmdStatus := cmdTUFStatusTemplate.ToCommand(t.tufStatus) - cmdStatus.Flags().IntSliceVarP(&t.changes, "unstage", "u", nil, "Numbers of changes to delete, as show in status list") - cmdStatus.Flags().BoolVar(&t.resetStatus, "reset", false, "Reset the changelist for the GUN by deleting all pending changes") + cmdStatus.Flags().IntSliceVarP(&t.deleteIdx, "unstage", "u", nil, "Numbers of changes to delete, as shown in status list") + cmdStatus.Flags().BoolVar(&t.reset, "reset", false, "Reset the changelist for the GUN by deleting all pending changes") cmd.AddCommand(cmdStatus) cmd.AddCommand(cmdTUFPublishTemplate.ToCommand(t.tufPublish)) @@ -448,12 +448,12 @@ func (t *tufCommander) tufStatus(cmd *cobra.Command, args []string) error { return err } - if t.resetStatus { + if t.reset { return cl.Clear(t.archiveChangelist) } - if len(t.changes) > 0 { - return cl.Remove(t.changes) + if len(t.deleteIdx) > 0 { + return cl.Remove(t.deleteIdx) } if len(cl.List()) == 0 { diff --git a/cmd/notary/tuf_test.go b/cmd/notary/tuf_test.go index be2d5647f..e9c666156 100644 --- a/cmd/notary/tuf_test.go +++ b/cmd/notary/tuf_test.go @@ -3,8 +3,11 @@ package main import ( "net/http" "net/http/httptest" + "os" "testing" + "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/stretchr/testify/require" ) @@ -118,3 +121,64 @@ func TestAdminTokenAuthNon200Non401Status(t *testing.T) { require.NoError(t, err) require.Nil(t, auth) } + +func TestStatusUnstageAndReset(t *testing.T) { + setUp(t) + tempBaseDir := tempDirWithConfig(t, "{}") + defer os.RemoveAll(tempBaseDir) + + tc := &tufCommander{ + configGetter: func() (*viper.Viper, error) { + v := viper.New() + v.SetDefault("trust_dir", tempBaseDir) + return v, nil + }, + } + + tc.reset = true + + // run a reset with an empty changelist and make sure it succeeds + err := tc.tufStatus(&cobra.Command{}, []string{"gun"}) + require.NoError(t, err) + + // add some targets + tc.sha256 = "88b76b34ab83a9e4d5abe3697950fb73f940aab1aa5b534f80cf9de9708942be" + err = tc.tufAddByHash(&cobra.Command{}, []string{"gun", "test1", "100"}) + require.NoError(t, err) + tc.sha256 = "4a7c203ce63b036a1999ea74eebd307c338368eb2b32218b722de6c5fdc7f016" + err = tc.tufAddByHash(&cobra.Command{}, []string{"gun", "test2", "100"}) + require.NoError(t, err) + tc.sha256 = "64bd0565907a6a55fc66fd828a71dbadd976fa875d0a3869f53d02eb8710ecb4" + err = tc.tufAddByHash(&cobra.Command{}, []string{"gun", "test3", "100"}) + require.NoError(t, err) + tc.sha256 = "9d9e890af64dd0f44b8a1538ff5fa0511cc31bf1ab89f3a3522a9a581a70fad8" + err = tc.tufAddByHash(&cobra.Command{}, []string{"gun", "test4", "100"}) + require.NoError(t, err) + + out, err := runCommand(t, tempBaseDir, "status", "gun") + require.NoError(t, err) + require.Contains(t, out, "test1") + require.Contains(t, out, "test2") + require.Contains(t, out, "test3") + require.Contains(t, out, "test4") + + _, err = runCommand(t, tempBaseDir, "status", "gun", "--unstage", "-1,1,3,10") + + out, err = runCommand(t, tempBaseDir, "status", "gun") + require.NoError(t, err) + require.Contains(t, out, "test1") + require.NotContains(t, out, "test2") + require.Contains(t, out, "test3") + require.NotContains(t, out, "test4") + + _, err = runCommand(t, tempBaseDir, "status", "gun", "--reset") + require.NoError(t, err) + + out, err = runCommand(t, tempBaseDir, "status", "gun") + require.NoError(t, err) + require.NotContains(t, out, "test1") + require.NotContains(t, out, "test2") + require.NotContains(t, out, "test3") + require.NotContains(t, out, "test4") + +}