From 5d8f4d2db78c4212f0c6fd4c6dd61aa954d5d5d7 Mon Sep 17 00:00:00 2001 From: "Signed-off-by: Fritz Duchardt" Date: Mon, 7 Aug 2023 09:16:54 +0200 Subject: [PATCH 01/10] feat: added lazy sync functionality to only do a sync, if the config has changed since the last sync. Added extra field to lock file in order to keep track of config changes. Signed-off-by: Fritz Durchardt --- pkg/vendir/cmd/sync.go | 50 +++++++++++++++++++++++++++-- pkg/vendir/config/lock_config.go | 7 ++++ pkg/vendir/config/lock_directory.go | 1 + pkg/vendir/directory/directory.go | 15 ++++++++- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/pkg/vendir/cmd/sync.go b/pkg/vendir/cmd/sync.go index c60882b9..62259e92 100644 --- a/pkg/vendir/cmd/sync.go +++ b/pkg/vendir/cmd/sync.go @@ -4,7 +4,10 @@ package cmd import ( + "crypto/sha256" + "encoding/hex" "fmt" + "gopkg.in/yaml.v3" "os" "path/filepath" "strings" @@ -30,6 +33,7 @@ type SyncOptions struct { Directories []string Locked bool + Lazy bool Chdir string AllowAllSymlinkDestinations bool @@ -50,6 +54,7 @@ func NewSyncCmd(o *SyncOptions) *cobra.Command { cmd.Flags().StringSliceVarP(&o.Directories, "directory", "d", nil, "Sync specific directory (format: dir/sub-dir[=local-dir])") cmd.Flags().BoolVarP(&o.Locked, "locked", "l", false, "Consult lock file to pull exact references (e.g. use git sha instead of branch name)") + cmd.Flags().BoolVar(&o.Lazy, "lazy", false, "Only fetch remote if vendir.yaml diverges from lock file") cmd.Flags().StringVar(&o.Chdir, "chdir", "", "Set current directory for process") cmd.Flags().BoolVar(&o.AllowAllSymlinkDestinations, "dangerous-allow-all-symlink-destinations", false, "Symlinks to all destinations are allowed") @@ -57,6 +62,37 @@ func NewSyncCmd(o *SyncOptions) *cobra.Command { return cmd } +func configUnchanged(vendirConfig ctlconf.Config, lockConfig ctlconf.LockConfig) bool { + for _, dir := range vendirConfig.Directories { + for _, content := range dir.Contents { + if !matchesLockConfig(dir.Path, content, lockConfig) { + return false + } + } + } + return true +} + +func matchesLockConfig(dir string, content ctlconf.DirectoryContents, lockConfig ctlconf.LockConfig) bool { + for _, lockDir := range lockConfig.Directories { + for _, lockContent := range lockDir.Contents { + if lockDir.Path == dir && lockContent.Path == content.Path { + yaml, err := yaml.Marshal(content) + if err != nil { + return false + } + hash := sha256.Sum256(yaml) + hashStr := hex.EncodeToString(hash[:]) + + if hashStr == lockContent.Hash { + return true + } + } + } + } + return false +} + func (o *SyncOptions) Run() error { if len(o.Chdir) > 0 { err := os.Chdir(o.Chdir) @@ -95,13 +131,21 @@ func (o *SyncOptions) Run() error { o.ui.PrintBlock(configBs) } - // If syncing against a lock file, apply lock information - // on top of existing config - if o.Locked { + if o.Lazy && ctlconf.LockFileExists(o.LockFile) { existingLockConfig, err := ctlconf.NewLockConfigFromFile(o.LockFile) if err != nil { return err } + if configUnchanged(conf, existingLockConfig) { + o.ui.PrintLinef("No changes in vendir.yaml since last sync. No need to sync.") + return nil + } + } + + // If syncing against a lock file, apply lock information + // on top of existing config + if o.Locked { + existingLockConfig, err := ctlconf.NewLockConfigFromFile(o.LockFile) err = conf.Lock(existingLockConfig) if err != nil { diff --git a/pkg/vendir/config/lock_config.go b/pkg/vendir/config/lock_config.go index d2a0c2cb..e50e517e 100644 --- a/pkg/vendir/config/lock_config.go +++ b/pkg/vendir/config/lock_config.go @@ -28,6 +28,13 @@ func NewLockConfig() LockConfig { } } +func LockFileExists(path string) bool { + if _, err := os.Stat(path); err != nil { + return false + } + return true +} + func NewLockConfigFromFile(path string) (LockConfig, error) { bs, err := os.ReadFile(path) if err != nil { diff --git a/pkg/vendir/config/lock_directory.go b/pkg/vendir/config/lock_directory.go index 41651e47..52811206 100644 --- a/pkg/vendir/config/lock_directory.go +++ b/pkg/vendir/config/lock_directory.go @@ -10,6 +10,7 @@ type LockDirectory struct { type LockDirectoryContents struct { Path string `json:"path"` + Hash string `json:"hash"` Git *LockDirectoryContentsGit `json:"git,omitempty"` Hg *LockDirectoryContentsHg `json:"hg,omitempty"` diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index 8baf5b07..5ea59daf 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -4,7 +4,10 @@ package directory import ( + "crypto/sha256" + "encoding/hex" "fmt" + "gopkg.in/yaml.v3" "os" "path/filepath" @@ -57,7 +60,17 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { return lockConfig, err } - lockDirContents := ctlconf.LockDirectoryContents{Path: contents.Path} + yaml, err := yaml.Marshal(contents) + if err != nil { + return lockConfig, err + } + hash := sha256.Sum256(yaml) + hashStr := hex.EncodeToString(hash[:]) + + lockDirContents := ctlconf.LockDirectoryContents{ + Path: contents.Path, + Hash: hashStr, + } skipFileFilter := false skipNewRootPath := false From dca81071f09cab2e68fbfd588f00af379c921aa7 Mon Sep 17 00:00:00 2001 From: Fritz Durchardt Date: Thu, 10 Aug 2023 13:31:46 +0200 Subject: [PATCH 02/10] feat: reimplementation of lazy sync functionality Signed-off-by: Fritz Durchardt --- go.mod | 3 +- pkg/vendir/cmd/sync.go | 46 ++++--------------------------- pkg/vendir/config/config.go | 14 ++++++++++ pkg/vendir/config/directory.go | 6 ++++ pkg/vendir/directory/directory.go | 42 ++++++++++++++++++++++++---- 5 files changed, 65 insertions(+), 46 deletions(-) diff --git a/go.mod b/go.mod index 559bdda6..4bc045d0 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,8 @@ require ( sigs.k8s.io/yaml v1.3.0 ) +require gopkg.in/yaml.v3 v3.0.1 + require ( cloud.google.com/go/compute v1.18.0 // indirect cloud.google.com/go/compute/metadata v0.2.3 // indirect @@ -106,7 +108,6 @@ require ( google.golang.org/appengine v1.6.7 // indirect google.golang.org/protobuf v1.29.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c // indirect k8s.io/klog v1.0.0 // indirect k8s.io/klog/v2 v2.70.1 // indirect diff --git a/pkg/vendir/cmd/sync.go b/pkg/vendir/cmd/sync.go index 62259e92..1007ac71 100644 --- a/pkg/vendir/cmd/sync.go +++ b/pkg/vendir/cmd/sync.go @@ -4,10 +4,7 @@ package cmd import ( - "crypto/sha256" - "encoding/hex" "fmt" - "gopkg.in/yaml.v3" "os" "path/filepath" "strings" @@ -33,7 +30,7 @@ type SyncOptions struct { Directories []string Locked bool - Lazy bool + Eager bool Chdir string AllowAllSymlinkDestinations bool @@ -54,7 +51,7 @@ func NewSyncCmd(o *SyncOptions) *cobra.Command { cmd.Flags().StringSliceVarP(&o.Directories, "directory", "d", nil, "Sync specific directory (format: dir/sub-dir[=local-dir])") cmd.Flags().BoolVarP(&o.Locked, "locked", "l", false, "Consult lock file to pull exact references (e.g. use git sha instead of branch name)") - cmd.Flags().BoolVar(&o.Lazy, "lazy", false, "Only fetch remote if vendir.yaml diverges from lock file") + cmd.Flags().BoolVar(&o.Eager, "eager", false, "Ignore lazy setting in vendir yml and eagerly fetch all remote content") cmd.Flags().StringVar(&o.Chdir, "chdir", "", "Set current directory for process") cmd.Flags().BoolVar(&o.AllowAllSymlinkDestinations, "dangerous-allow-all-symlink-destinations", false, "Symlinks to all destinations are allowed") @@ -62,37 +59,6 @@ func NewSyncCmd(o *SyncOptions) *cobra.Command { return cmd } -func configUnchanged(vendirConfig ctlconf.Config, lockConfig ctlconf.LockConfig) bool { - for _, dir := range vendirConfig.Directories { - for _, content := range dir.Contents { - if !matchesLockConfig(dir.Path, content, lockConfig) { - return false - } - } - } - return true -} - -func matchesLockConfig(dir string, content ctlconf.DirectoryContents, lockConfig ctlconf.LockConfig) bool { - for _, lockDir := range lockConfig.Directories { - for _, lockContent := range lockDir.Contents { - if lockDir.Path == dir && lockContent.Path == content.Path { - yaml, err := yaml.Marshal(content) - if err != nil { - return false - } - hash := sha256.Sum256(yaml) - hashStr := hex.EncodeToString(hash[:]) - - if hashStr == lockContent.Hash { - return true - } - } - } - } - return false -} - func (o *SyncOptions) Run() error { if len(o.Chdir) > 0 { err := os.Chdir(o.Chdir) @@ -131,14 +97,14 @@ func (o *SyncOptions) Run() error { o.ui.PrintBlock(configBs) } - if o.Lazy && ctlconf.LockFileExists(o.LockFile) { + if ctlconf.LockFileExists(o.LockFile) { existingLockConfig, err := ctlconf.NewLockConfigFromFile(o.LockFile) if err != nil { return err } - if configUnchanged(conf, existingLockConfig) { - o.ui.PrintLinef("No changes in vendir.yaml since last sync. No need to sync.") - return nil + err = conf.TransferHash(existingLockConfig) + if err != nil { + return err } } diff --git a/pkg/vendir/config/config.go b/pkg/vendir/config/config.go index 0ec3aef1..649bf33f 100644 --- a/pkg/vendir/config/config.go +++ b/pkg/vendir/config/config.go @@ -257,6 +257,20 @@ func (c Config) Lock(lockConfig LockConfig) error { return nil } +func (c Config) TransferHash(lockConfig LockConfig) error { + for _, dir := range c.Directories { + for _, con := range dir.Contents { + lockContents, err := lockConfig.FindContents(dir.Path, con.Path) + if err != nil { + return err + } + con.LockHash(lockContents.Hash) + } + } + + return nil +} + func (c Config) checkOverlappingPaths() error { paths := []string{} diff --git a/pkg/vendir/config/directory.go b/pkg/vendir/config/directory.go index a386cdd9..875bd661 100644 --- a/pkg/vendir/config/directory.go +++ b/pkg/vendir/config/directory.go @@ -33,7 +33,9 @@ type Directory struct { } type DirectoryContents struct { + Hash string Path string `json:"path"` + Lazy bool `json:"lazy,omitempty"` Git *DirectoryContentsGit `json:"git,omitempty"` Hg *DirectoryContentsHg `json:"hg,omitempty"` @@ -333,6 +335,10 @@ func (c DirectoryContents) Lock(lockConfig LockDirectoryContents) error { } } +func (c *DirectoryContents) LockHash(hash string) { + c.Hash = hash +} + func (c *DirectoryContentsGit) Lock(lockConfig *LockDirectoryContentsGit) error { if lockConfig == nil { return fmt.Errorf("Expected git lock configuration to be non-empty") diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index 5ea59daf..7afcd015 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -42,6 +42,27 @@ type SyncOpts struct { Cache ctlcache.Cache } +func createContentHash(contents ctlconf.DirectoryContents) (string, error) { + yaml, err := yaml.Marshal(contents) + if err != nil { + return "", fmt.Errorf("error during hash creation for path '%s': %s", contents.Path, err) + } + hash := sha256.Sum256(yaml) + hashStr := hex.EncodeToString(hash[:]) + return hashStr, nil +} + +func configUnchanged(contents ctlconf.DirectoryContents) (bool, error) { + hash, err := createContentHash(contents) + if err != nil { + return false, err + } + if hash == contents.Hash { + return true, nil + } + return false, nil +} + func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { lockConfig := ctlconf.LockDirectory{Path: d.opts.Path} @@ -60,16 +81,25 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { return lockConfig, err } - yaml, err := yaml.Marshal(contents) + // check if vendir config has changed. If not, skip syncing + if contents.Lazy { + changed, err := configUnchanged(contents) + if err != nil { + return lockConfig, err + } + if changed { + continue + } + } + + // adds hash to lockfile of current content + hash, err := createContentHash(contents) if err != nil { return lockConfig, err } - hash := sha256.Sum256(yaml) - hashStr := hex.EncodeToString(hash[:]) - lockDirContents := ctlconf.LockDirectoryContents{ Path: contents.Path, - Hash: hashStr, + Hash: hash, } skipFileFilter := false @@ -245,6 +275,8 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { return lockConfig, nil } +// + // maybeChmod will chmod the path with the first non-nil permission provided. // If no permission is handed in or all of them are nil, no chmod will be done. func maybeChmod(path string, potentialPerms ...*os.FileMode) error { From 6fa6535725331da73bdaa5f7f6a447c39c5fa97a Mon Sep 17 00:00:00 2001 From: Fritz Durchardt Date: Thu, 10 Aug 2023 16:14:02 +0200 Subject: [PATCH 03/10] feat: first working draft Signed-off-by: Fritz Durchardt --- examples/helm-chart/vendir.yml | 2 +- pkg/vendir/cmd/sync.go | 18 ++--- pkg/vendir/config/config.go | 15 +--- pkg/vendir/config/directory.go | 16 ++++- pkg/vendir/directory/directory.go | 109 +++++++++++++++++------------- 5 files changed, 82 insertions(+), 78 deletions(-) diff --git a/examples/helm-chart/vendir.yml b/examples/helm-chart/vendir.yml index 57d3afe5..d17ab083 100644 --- a/examples/helm-chart/vendir.yml +++ b/examples/helm-chart/vendir.yml @@ -11,6 +11,6 @@ directories: - path: custom-repo-custom-version helmChart: name: contour - version: "7.10.1" + version: "7.10.0" repository: url: https://charts.bitnami.com/bitnami diff --git a/pkg/vendir/cmd/sync.go b/pkg/vendir/cmd/sync.go index 1007ac71..c6a4d7f8 100644 --- a/pkg/vendir/cmd/sync.go +++ b/pkg/vendir/cmd/sync.go @@ -97,12 +97,10 @@ func (o *SyncOptions) Run() error { o.ui.PrintBlock(configBs) } + var existingLockConfig ctlconf.LockConfig + if ctlconf.LockFileExists(o.LockFile) { - existingLockConfig, err := ctlconf.NewLockConfigFromFile(o.LockFile) - if err != nil { - return err - } - err = conf.TransferHash(existingLockConfig) + existingLockConfig, err = ctlconf.NewLockConfigFromFile(o.LockFile) if err != nil { return err } @@ -111,8 +109,6 @@ func (o *SyncOptions) Run() error { // If syncing against a lock file, apply lock information // on top of existing config if o.Locked { - existingLockConfig, err := ctlconf.NewLockConfigFromFile(o.LockFile) - err = conf.Lock(existingLockConfig) if err != nil { return err @@ -140,11 +136,12 @@ func (o *SyncOptions) Run() error { GithubAPIToken: os.Getenv("VENDIR_GITHUB_API_TOKEN"), HelmBinary: os.Getenv("VENDIR_HELM_BINARY"), Cache: cache, + Eager: o.Eager, } newLockConfig := ctlconf.NewLockConfig() for _, dirConf := range conf.Directories { - dirLockConf, err := ctldir.NewDirectory(dirConf, o.ui).Sync(syncOpts) + dirLockConf, err := ctldir.NewDirectory(dirConf, existingLockConfig, o.ui).Sync(syncOpts) if err != nil { return fmt.Errorf("Syncing directory '%s': %s", dirConf.Path, err) } @@ -160,11 +157,6 @@ func (o *SyncOptions) Run() error { // Update only selected directories in lock file if len(dirs) > 0 { - existingLockConfig, err := ctlconf.NewLockConfigFromFile(o.LockFile) - if err != nil { - return err - } - err = existingLockConfig.Merge(newLockConfig) if err != nil { return err diff --git a/pkg/vendir/config/config.go b/pkg/vendir/config/config.go index 649bf33f..a5842324 100644 --- a/pkg/vendir/config/config.go +++ b/pkg/vendir/config/config.go @@ -183,6 +183,7 @@ func (c Config) UseDirectory(path, dirPath string) error { ExcludePaths: con.ExcludePaths, IgnorePaths: con.IgnorePaths, LegalPaths: con.LegalPaths, + Lazy: con.Lazy, } dir.Contents[j] = newCon c.Directories[i] = dir @@ -257,20 +258,6 @@ func (c Config) Lock(lockConfig LockConfig) error { return nil } -func (c Config) TransferHash(lockConfig LockConfig) error { - for _, dir := range c.Directories { - for _, con := range dir.Contents { - lockContents, err := lockConfig.FindContents(dir.Path, con.Path) - if err != nil { - return err - } - con.LockHash(lockContents.Hash) - } - } - - return nil -} - func (c Config) checkOverlappingPaths() error { paths := []string{} diff --git a/pkg/vendir/config/directory.go b/pkg/vendir/config/directory.go index 875bd661..1dcd2bfe 100644 --- a/pkg/vendir/config/directory.go +++ b/pkg/vendir/config/directory.go @@ -32,8 +32,15 @@ type Directory struct { Permissions *os.FileMode `json:"permissions,omitempty"` } -type DirectoryContents struct { +type HashContainer struct { Hash string +} + +func (h *HashContainer) Write(hash string) { + h.Hash = hash +} + +type DirectoryContents struct { Path string `json:"path"` Lazy bool `json:"lazy,omitempty"` @@ -180,6 +187,10 @@ type DirectoryContentsHelmChartRepo struct { type DirectoryContentsManual struct{} +type Hash struct { + Hash string +} + type DirectoryContentsDirectory struct { Path string `json:"path"` } @@ -335,8 +346,9 @@ func (c DirectoryContents) Lock(lockConfig LockDirectoryContents) error { } } -func (c *DirectoryContents) LockHash(hash string) { +func (c *Hash) Lock(hash string) error { c.Hash = hash + return nil } func (c *DirectoryContentsGit) Lock(lockConfig *LockDirectoryContentsGit) error { diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index 7afcd015..da136f63 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -27,12 +27,13 @@ import ( ) type Directory struct { - opts ctlconf.Directory - ui ui.UI + opts ctlconf.Directory + lockConfig ctlconf.LockConfig + ui ui.UI } -func NewDirectory(opts ctlconf.Directory, ui ui.UI) *Directory { - return &Directory{opts, ui} +func NewDirectory(opts ctlconf.Directory, lockConfig ctlconf.LockConfig, ui ui.UI) *Directory { + return &Directory{opts, lockConfig, ui} } type SyncOpts struct { @@ -40,6 +41,7 @@ type SyncOpts struct { GithubAPIToken string HelmBinary string Cache ctlcache.Cache + Eager bool } func createContentHash(contents ctlconf.DirectoryContents) (string, error) { @@ -52,12 +54,13 @@ func createContentHash(contents ctlconf.DirectoryContents) (string, error) { return hashStr, nil } -func configUnchanged(contents ctlconf.DirectoryContents) (bool, error) { +func (d *Directory) configUnchanged(path string, contents ctlconf.DirectoryContents) (bool, error) { hash, err := createContentHash(contents) if err != nil { return false, err } - if hash == contents.Hash { + lockContents, _ := d.lockConfig.FindContents(path, contents.Path) + if hash == lockContents.Hash { return true, nil } return false, nil @@ -80,27 +83,29 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { if err != nil { return lockConfig, err } - - // check if vendir config has changed. If not, skip syncing - if contents.Lazy { - changed, err := configUnchanged(contents) - if err != nil { - return lockConfig, err - } - if changed { - continue - } - } - // adds hash to lockfile of current content hash, err := createContentHash(contents) if err != nil { return lockConfig, err } + lockDirContents := ctlconf.LockDirectoryContents{ Path: contents.Path, Hash: hash, } + // check if vendir config has changed. If not, skip syncing + unchanged := false + var oldLock ctlconf.LockDirectoryContents + if contents.Lazy && syncOpts.Eager == false { + unchanged, err = d.configUnchanged(d.opts.Path, contents) + if err != nil { + return lockConfig, err + } + if unchanged { + d.ui.PrintLinef("Skipping fetch since config has not changed: %s%s", d.opts.Path, contents.Path) + oldLock, _ = d.lockConfig.FindContents(d.opts.Path, contents.Path) + } + } skipFileFilter := false skipNewRootPath := false @@ -111,13 +116,16 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { d.ui.PrintLinef("Fetching: %s + %s (git from %s)", d.opts.Path, contents.Path, gitSync.Desc()) - lock, err := gitSync.Sync(stagingDstPath, stagingDir.TempArea()) - if err != nil { - return lockConfig, fmt.Errorf("Syncing directory '%s' with git contents: %s", contents.Path, err) + if unchanged == false { + lock, err := gitSync.Sync(stagingDstPath, stagingDir.TempArea()) + if err != nil { + return lockConfig, fmt.Errorf("Syncing directory '%s' with git contents: %s", contents.Path, err) + } + lockDirContents.Git = &lock + } else { + lockDirContents.Git = oldLock.Git } - lockDirContents.Git = &lock - case contents.Hg != nil: hgSync := ctlhg.NewSync(*contents.Hg, NewInfoLog(d.ui), syncOpts.RefFetcher) @@ -186,13 +194,16 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { d.ui.PrintLinef("Fetching: %s + %s (helm chart from %s)", d.opts.Path, contents.Path, helmChartSync.Desc()) - lock, err := helmChartSync.Sync(stagingDstPath, stagingDir.TempArea()) - if err != nil { - return lockConfig, fmt.Errorf("Syncing directory '%s' with helm chart contents: %s", contents.Path, err) + if unchanged == false { + lock, err := helmChartSync.Sync(stagingDstPath, stagingDir.TempArea()) + if err != nil { + return lockConfig, fmt.Errorf("Syncing directory '%s' with helm chart contents: %s", contents.Path, err) + } + lockDirContents.HelmChart = &lock + } else { + lockDirContents.HelmChart = oldLock.HelmChart } - lockDirContents.HelmChart = &lock - case contents.Manual != nil: d.ui.PrintLinef("Fetching: %s + %s (manual)", d.opts.Path, contents.Path) @@ -231,31 +242,33 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { return lockConfig, fmt.Errorf("Unknown contents type for directory '%s'", contents.Path) } - if !skipFileFilter { - err = FileFilter{contents}.Apply(stagingDstPath) - if err != nil { - return lockConfig, fmt.Errorf("Filtering paths in directory '%s': %s", contents.Path, err) + if unchanged == false { + if !skipFileFilter { + err = FileFilter{contents}.Apply(stagingDstPath) + if err != nil { + return lockConfig, fmt.Errorf("Filtering paths in directory '%s': %s", contents.Path, err) + } } - } - if !skipNewRootPath && len(contents.NewRootPath) > 0 { - err = NewSubPath(contents.NewRootPath).Extract(stagingDstPath, stagingDstPath, stagingDir.TempArea()) - if err != nil { - return lockConfig, fmt.Errorf("Changing to new root path '%s': %s", contents.Path, err) + if !skipNewRootPath && len(contents.NewRootPath) > 0 { + err = NewSubPath(contents.NewRootPath).Extract(stagingDstPath, stagingDstPath, stagingDir.TempArea()) + if err != nil { + return lockConfig, fmt.Errorf("Changing to new root path '%s': %s", contents.Path, err) + } } - } - // Copy files from current source if values are supposed to be ignored - err = stagingDir.CopyExistingFiles(d.opts.Path, stagingDstPath, contents.IgnorePaths) - if err != nil { - return lockConfig, fmt.Errorf("Copying existing content to staging '%s': %s", d.opts.Path, err) - } + // Copy files from current source if values are supposed to be ignored + err = stagingDir.CopyExistingFiles(d.opts.Path, stagingDstPath, contents.IgnorePaths) + if err != nil { + return lockConfig, fmt.Errorf("Copying existing content to staging '%s': %s", d.opts.Path, err) + } - // after everything else is done, ensure the inner dir's access perms are set - // chmod to the content's permission, fall back to the directory's - err = maybeChmod(stagingDstPath, contents.Permissions, d.opts.Permissions) - if err != nil { - return lockConfig, fmt.Errorf("chmod on '%s': %s", stagingDstPath, err) + // after everything else is done, ensure the inner dir's access perms are set + // chmod to the content's permission, fall back to the directory's + err = maybeChmod(stagingDstPath, contents.Permissions, d.opts.Permissions) + if err != nil { + return lockConfig, fmt.Errorf("chmod on '%s': %s", stagingDstPath, err) + } } lockConfig.Contents = append(lockConfig.Contents, lockDirContents) From 77b9440694533df0dd121ec8e8a86eed69380de3 Mon Sep 17 00:00:00 2001 From: Fritz Durchardt Date: Fri, 11 Aug 2023 08:54:10 +0200 Subject: [PATCH 04/10] Remove hash container Signed-off-by: Fritz Durchardt --- examples/helm-chart/vendir.yml | 2 +- pkg/vendir/config/directory.go | 17 ----------------- pkg/vendir/directory/directory.go | 2 +- 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/examples/helm-chart/vendir.yml b/examples/helm-chart/vendir.yml index d17ab083..57d3afe5 100644 --- a/examples/helm-chart/vendir.yml +++ b/examples/helm-chart/vendir.yml @@ -11,6 +11,6 @@ directories: - path: custom-repo-custom-version helmChart: name: contour - version: "7.10.0" + version: "7.10.1" repository: url: https://charts.bitnami.com/bitnami diff --git a/pkg/vendir/config/directory.go b/pkg/vendir/config/directory.go index 1dcd2bfe..58ec782a 100644 --- a/pkg/vendir/config/directory.go +++ b/pkg/vendir/config/directory.go @@ -32,14 +32,6 @@ type Directory struct { Permissions *os.FileMode `json:"permissions,omitempty"` } -type HashContainer struct { - Hash string -} - -func (h *HashContainer) Write(hash string) { - h.Hash = hash -} - type DirectoryContents struct { Path string `json:"path"` Lazy bool `json:"lazy,omitempty"` @@ -187,10 +179,6 @@ type DirectoryContentsHelmChartRepo struct { type DirectoryContentsManual struct{} -type Hash struct { - Hash string -} - type DirectoryContentsDirectory struct { Path string `json:"path"` } @@ -346,11 +334,6 @@ func (c DirectoryContents) Lock(lockConfig LockDirectoryContents) error { } } -func (c *Hash) Lock(hash string) error { - c.Hash = hash - return nil -} - func (c *DirectoryContentsGit) Lock(lockConfig *LockDirectoryContentsGit) error { if lockConfig == nil { return fmt.Errorf("Expected git lock configuration to be non-empty") diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index da136f63..cae5ad60 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -242,7 +242,7 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { return lockConfig, fmt.Errorf("Unknown contents type for directory '%s'", contents.Path) } - if unchanged == false { + if !unchanged { if !skipFileFilter { err = FileFilter{contents}.Apply(stagingDstPath) if err != nil { From a1beab73b9193bc205792385c30294b7e068f243 Mon Sep 17 00:00:00 2001 From: Fritz Duchardt Date: Sun, 8 Oct 2023 11:15:07 +0200 Subject: [PATCH 05/10] Revamp of entire lazy sync logic Signed-off-by: Fritz Duchardt --- pkg/vendir/cmd/sync.go | 6 +- pkg/vendir/config/lock_directory.go | 4 +- pkg/vendir/directory/directory.go | 137 ++++++++++++------------- pkg/vendir/directory/directory_test.go | 60 +++++++++++ 4 files changed, 129 insertions(+), 78 deletions(-) create mode 100644 pkg/vendir/directory/directory_test.go diff --git a/pkg/vendir/cmd/sync.go b/pkg/vendir/cmd/sync.go index c6a4d7f8..2379e824 100644 --- a/pkg/vendir/cmd/sync.go +++ b/pkg/vendir/cmd/sync.go @@ -30,7 +30,7 @@ type SyncOptions struct { Directories []string Locked bool - Eager bool + Lazy bool Chdir string AllowAllSymlinkDestinations bool @@ -51,7 +51,7 @@ func NewSyncCmd(o *SyncOptions) *cobra.Command { cmd.Flags().StringSliceVarP(&o.Directories, "directory", "d", nil, "Sync specific directory (format: dir/sub-dir[=local-dir])") cmd.Flags().BoolVarP(&o.Locked, "locked", "l", false, "Consult lock file to pull exact references (e.g. use git sha instead of branch name)") - cmd.Flags().BoolVar(&o.Eager, "eager", false, "Ignore lazy setting in vendir yml and eagerly fetch all remote content") + cmd.Flags().BoolVar(&o.Lazy, "lazy", true, "Ignore lazy setting in vendir yml if set to false") cmd.Flags().StringVar(&o.Chdir, "chdir", "", "Set current directory for process") cmd.Flags().BoolVar(&o.AllowAllSymlinkDestinations, "dangerous-allow-all-symlink-destinations", false, "Symlinks to all destinations are allowed") @@ -136,7 +136,7 @@ func (o *SyncOptions) Run() error { GithubAPIToken: os.Getenv("VENDIR_GITHUB_API_TOKEN"), HelmBinary: os.Getenv("VENDIR_HELM_BINARY"), Cache: cache, - Eager: o.Eager, + Lazy: o.Lazy, } newLockConfig := ctlconf.NewLockConfig() diff --git a/pkg/vendir/config/lock_directory.go b/pkg/vendir/config/lock_directory.go index 52811206..116a4b35 100644 --- a/pkg/vendir/config/lock_directory.go +++ b/pkg/vendir/config/lock_directory.go @@ -9,8 +9,8 @@ type LockDirectory struct { } type LockDirectoryContents struct { - Path string `json:"path"` - Hash string `json:"hash"` + Path string `json:"path"` + ConfigDigest string `json:"configDigest,omitempty"` Git *LockDirectoryContentsGit `json:"git,omitempty"` Hg *LockDirectoryContentsHg `json:"hg,omitempty"` diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index cae5ad60..cbf515c8 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -41,29 +41,17 @@ type SyncOpts struct { GithubAPIToken string HelmBinary string Cache ctlcache.Cache - Eager bool + Lazy bool } -func createContentHash(contents ctlconf.DirectoryContents) (string, error) { +func createConfigDigest(contents ctlconf.DirectoryContents) (string, error) { yaml, err := yaml.Marshal(contents) if err != nil { - return "", fmt.Errorf("error during hash creation for path '%s': %s", contents.Path, err) + return "", fmt.Errorf("error during creating for config digest for path '%s': %s", contents.Path, err) } - hash := sha256.Sum256(yaml) - hashStr := hex.EncodeToString(hash[:]) - return hashStr, nil -} - -func (d *Directory) configUnchanged(path string, contents ctlconf.DirectoryContents) (bool, error) { - hash, err := createContentHash(contents) - if err != nil { - return false, err - } - lockContents, _ := d.lockConfig.FindContents(path, contents.Path) - if hash == lockContents.Hash { - return true, nil - } - return false, nil + digest := sha256.Sum256(yaml) + digestStr := hex.EncodeToString(digest[:]) + return digestStr, nil } func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { @@ -83,28 +71,25 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { if err != nil { return lockConfig, err } - // adds hash to lockfile of current content - hash, err := createContentHash(contents) + + // creates config digest for current content config + configDigest, err := createConfigDigest(contents) if err != nil { return lockConfig, err } lockDirContents := ctlconf.LockDirectoryContents{ Path: contents.Path, - Hash: hash, } - // check if vendir config has changed. If not, skip syncing - unchanged := false - var oldLock ctlconf.LockDirectoryContents - if contents.Lazy && syncOpts.Eager == false { - unchanged, err = d.configUnchanged(d.opts.Path, contents) - if err != nil { - return lockConfig, err - } - if unchanged { - d.ui.PrintLinef("Skipping fetch since config has not changed: %s%s", d.opts.Path, contents.Path) - oldLock, _ = d.lockConfig.FindContents(d.opts.Path, contents.Path) - } + + // error is safe to ignore, since it indicates that no lock file entry for the given path exists + oldLockContent, _ := d.lockConfig.FindContents(d.opts.Path, contents.Path) + skipFetching, lazySyncAddConfigDigest := handleLazySync(oldLockContent.ConfigDigest, configDigest, syncOpts.Lazy, contents.Lazy) + + if skipFetching { + d.ui.PrintLinef("Skipping fetch: %s + %s (flagged as lazy, config has not changed since last sync)", d.opts.Path, contents.Path) + lockConfig.Contents = append(lockConfig.Contents, oldLockContent) + continue } skipFileFilter := false @@ -116,15 +101,11 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { d.ui.PrintLinef("Fetching: %s + %s (git from %s)", d.opts.Path, contents.Path, gitSync.Desc()) - if unchanged == false { - lock, err := gitSync.Sync(stagingDstPath, stagingDir.TempArea()) - if err != nil { - return lockConfig, fmt.Errorf("Syncing directory '%s' with git contents: %s", contents.Path, err) - } - lockDirContents.Git = &lock - } else { - lockDirContents.Git = oldLock.Git + lock, err := gitSync.Sync(stagingDstPath, stagingDir.TempArea()) + if err != nil { + return lockConfig, fmt.Errorf("Syncing directory '%s' with git contents: %s", contents.Path, err) } + lockDirContents.Git = &lock case contents.Hg != nil: hgSync := ctlhg.NewSync(*contents.Hg, NewInfoLog(d.ui), syncOpts.RefFetcher) @@ -194,15 +175,11 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { d.ui.PrintLinef("Fetching: %s + %s (helm chart from %s)", d.opts.Path, contents.Path, helmChartSync.Desc()) - if unchanged == false { - lock, err := helmChartSync.Sync(stagingDstPath, stagingDir.TempArea()) - if err != nil { - return lockConfig, fmt.Errorf("Syncing directory '%s' with helm chart contents: %s", contents.Path, err) - } - lockDirContents.HelmChart = &lock - } else { - lockDirContents.HelmChart = oldLock.HelmChart + lock, err := helmChartSync.Sync(stagingDstPath, stagingDir.TempArea()) + if err != nil { + return lockConfig, fmt.Errorf("Syncing directory '%s' with helm chart contents: %s", contents.Path, err) } + lockDirContents.HelmChart = &lock case contents.Manual != nil: d.ui.PrintLinef("Fetching: %s + %s (manual)", d.opts.Path, contents.Path) @@ -242,35 +219,37 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { return lockConfig, fmt.Errorf("Unknown contents type for directory '%s'", contents.Path) } - if !unchanged { - if !skipFileFilter { - err = FileFilter{contents}.Apply(stagingDstPath) - if err != nil { - return lockConfig, fmt.Errorf("Filtering paths in directory '%s': %s", contents.Path, err) - } - } - - if !skipNewRootPath && len(contents.NewRootPath) > 0 { - err = NewSubPath(contents.NewRootPath).Extract(stagingDstPath, stagingDstPath, stagingDir.TempArea()) - if err != nil { - return lockConfig, fmt.Errorf("Changing to new root path '%s': %s", contents.Path, err) - } - } - - // Copy files from current source if values are supposed to be ignored - err = stagingDir.CopyExistingFiles(d.opts.Path, stagingDstPath, contents.IgnorePaths) + if !skipFileFilter { + err = FileFilter{contents}.Apply(stagingDstPath) if err != nil { - return lockConfig, fmt.Errorf("Copying existing content to staging '%s': %s", d.opts.Path, err) + return lockConfig, fmt.Errorf("Filtering paths in directory '%s': %s", contents.Path, err) } + } - // after everything else is done, ensure the inner dir's access perms are set - // chmod to the content's permission, fall back to the directory's - err = maybeChmod(stagingDstPath, contents.Permissions, d.opts.Permissions) + if !skipNewRootPath && len(contents.NewRootPath) > 0 { + err = NewSubPath(contents.NewRootPath).Extract(stagingDstPath, stagingDstPath, stagingDir.TempArea()) if err != nil { - return lockConfig, fmt.Errorf("chmod on '%s': %s", stagingDstPath, err) + return lockConfig, fmt.Errorf("Changing to new root path '%s': %s", contents.Path, err) } } + // Copy files from current source if values are supposed to be ignored + err = stagingDir.CopyExistingFiles(d.opts.Path, stagingDstPath, contents.IgnorePaths) + if err != nil { + return lockConfig, fmt.Errorf("Copying existing content to staging '%s': %s", d.opts.Path, err) + } + + // after everything else is done, ensure the inner dir's access perms are set + // chmod to the content's permission, fall back to the directory's + err = maybeChmod(stagingDstPath, contents.Permissions, d.opts.Permissions) + if err != nil { + return lockConfig, fmt.Errorf("chmod on '%s': %s", stagingDstPath, err) + } + + if lazySyncAddConfigDigest { + lockDirContents.ConfigDigest = configDigest + } + lockConfig.Contents = append(lockConfig.Contents, lockDirContents) } @@ -288,8 +267,6 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { return lockConfig, nil } -// - // maybeChmod will chmod the path with the first non-nil permission provided. // If no permission is handed in or all of them are nil, no chmod will be done. func maybeChmod(path string, potentialPerms ...*os.FileMode) error { @@ -301,3 +278,17 @@ func maybeChmod(path string, potentialPerms ...*os.FileMode) error { return nil } + +func handleLazySync(oldConfigDigest string, newConfigDigest string, fetchLazyGlobalOverride bool, fetchLazy bool) (bool, bool) { + skipFetching := false + addConfigDigest := false + // if lazy sync is enabled and config remains unchanged, skip fetching + if fetchLazyGlobalOverride && fetchLazy && oldConfigDigest == newConfigDigest { + skipFetching = true + } + // config digest is always added if lazy syncing is enabled locally and globally + if fetchLazy { + addConfigDigest = true + } + return skipFetching, addConfigDigest +} diff --git a/pkg/vendir/directory/directory_test.go b/pkg/vendir/directory/directory_test.go new file mode 100644 index 00000000..0564511d --- /dev/null +++ b/pkg/vendir/directory/directory_test.go @@ -0,0 +1,60 @@ +package directory + +import "testing" + +func Test_handleLazySync(t *testing.T) { + type args struct { + oldConfigDigest string + newConfigDigest string + fetchLazyOverride bool + fetchContentLazy bool + } + tests := []struct { + name string + args args + skipFetching bool + addConfigDigest bool + }{ + { + "no skipping of sync or adding of config digest, if lazy sync is disabled globally and locally", + args{"same", "same", false, false}, + false, + false, + }, + { + "no skipping of sync or adding of config digest, if lazy sync is enabled locally", + args{"same", "same", true, false}, + false, + false, + }, + { + "no skipping of sync but adding of config digest, if lazy sync is disabled globally", + args{"same", "same", false, true}, + false, + true, + }, + { + "no skipping of sync but adding of config digest, if lazy sync is enabled but config digests don't match", + args{"same", "not-same", true, true}, + false, + true, + }, + { + "skipping of sync and adding of config digest, if lazy sync is enabled and config digests match", + args{"same", "same", true, true}, + true, + true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + skipFetching, addConfigDigest := handleLazySync(tt.args.oldConfigDigest, tt.args.newConfigDigest, tt.args.fetchLazyOverride, tt.args.fetchContentLazy) + if skipFetching != tt.skipFetching { + t.Errorf("handleLazySync() got = %v, want %v", skipFetching, tt.skipFetching) + } + if addConfigDigest != tt.addConfigDigest { + t.Errorf("handleLazySync() got1 = %v, want %v", addConfigDigest, tt.addConfigDigest) + } + }) + } +} From e0a6b99c67e91da973aa6dade005afd95039b2cf Mon Sep 17 00:00:00 2001 From: Fritz Duchardt Date: Sun, 8 Oct 2023 14:58:38 +0200 Subject: [PATCH 06/10] Tweaked handling of lock file Signed-off-by: Fritz Duchardt --- pkg/vendir/cmd/sync.go | 16 +++++++--------- pkg/vendir/config/lock_config.go | 10 ++++++++++ pkg/vendir/config/lock_directory.go | 12 ++++++++++++ pkg/vendir/directory/directory.go | 12 ++++++------ 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/pkg/vendir/cmd/sync.go b/pkg/vendir/cmd/sync.go index 2379e824..4891e0c0 100644 --- a/pkg/vendir/cmd/sync.go +++ b/pkg/vendir/cmd/sync.go @@ -51,7 +51,7 @@ func NewSyncCmd(o *SyncOptions) *cobra.Command { cmd.Flags().StringSliceVarP(&o.Directories, "directory", "d", nil, "Sync specific directory (format: dir/sub-dir[=local-dir])") cmd.Flags().BoolVarP(&o.Locked, "locked", "l", false, "Consult lock file to pull exact references (e.g. use git sha instead of branch name)") - cmd.Flags().BoolVar(&o.Lazy, "lazy", true, "Ignore lazy setting in vendir yml if set to false") + cmd.Flags().BoolVar(&o.Lazy, "lazy", true, "Override \"lazy\" value in directory content configuration") cmd.Flags().StringVar(&o.Chdir, "chdir", "", "Set current directory for process") cmd.Flags().BoolVar(&o.AllowAllSymlinkDestinations, "dangerous-allow-all-symlink-destinations", false, "Symlinks to all destinations are allowed") @@ -97,13 +97,9 @@ func (o *SyncOptions) Run() error { o.ui.PrintBlock(configBs) } - var existingLockConfig ctlconf.LockConfig - - if ctlconf.LockFileExists(o.LockFile) { - existingLockConfig, err = ctlconf.NewLockConfigFromFile(o.LockFile) - if err != nil { - return err - } + existingLockConfig, err := ctlconf.NewLockConfigFromFile(o.LockFile) + if err != nil && o.Locked { + return err } // If syncing against a lock file, apply lock information @@ -141,7 +137,9 @@ func (o *SyncOptions) Run() error { newLockConfig := ctlconf.NewLockConfig() for _, dirConf := range conf.Directories { - dirLockConf, err := ctldir.NewDirectory(dirConf, existingLockConfig, o.ui).Sync(syncOpts) + // error safe to ignore, since lock file might not exist + dirExistingLockConf, _ := existingLockConfig.FindLockDirectory(dirConf.Path) + dirLockConf, err := ctldir.NewDirectory(dirConf, dirExistingLockConf, o.ui).Sync(syncOpts) if err != nil { return fmt.Errorf("Syncing directory '%s': %s", dirConf.Path, err) } diff --git a/pkg/vendir/config/lock_config.go b/pkg/vendir/config/lock_config.go index e50e517e..e3dc2064 100644 --- a/pkg/vendir/config/lock_config.go +++ b/pkg/vendir/config/lock_config.go @@ -121,6 +121,16 @@ func (c LockConfig) FindContents(dirPath, conPath string) (LockDirectoryContents "Expected to find directory '%s' within lock config, but did not", dirPath) } +func (c LockConfig) FindLockDirectory(dirPath string) (LockDirectory, error) { + for _, dir := range c.Directories { + if dir.Path == dirPath { + return dir, nil + } + } + return LockDirectory{}, fmt.Errorf( + "Expected to find directory '%s' within lock config, but did not", dirPath) +} + func (c LockConfig) Merge(other LockConfig) error { for _, dir := range other.Directories { for _, con := range dir.Contents { diff --git a/pkg/vendir/config/lock_directory.go b/pkg/vendir/config/lock_directory.go index 116a4b35..3e239f7c 100644 --- a/pkg/vendir/config/lock_directory.go +++ b/pkg/vendir/config/lock_directory.go @@ -3,6 +3,8 @@ package config +import "fmt" + type LockDirectory struct { Path string `json:"path"` Contents []LockDirectoryContents `json:"contents"` @@ -63,3 +65,13 @@ type LockDirectoryContentsManual struct{} type LockDirectoryContentsDirectory struct{} type LockDirectoryContentsInline struct{} + +func (d LockDirectory) FindContents(conPath string) (LockDirectoryContents, error) { + for _, con := range d.Contents { + if con.Path == conPath { + return con, nil + } + } + return LockDirectoryContents{}, fmt.Errorf("Expected to find contents '%s' "+ + "within directory '%s' in lock config, but did not", conPath, d.Path) +} diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index cbf515c8..580606f5 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -27,13 +27,13 @@ import ( ) type Directory struct { - opts ctlconf.Directory - lockConfig ctlconf.LockConfig - ui ui.UI + opts ctlconf.Directory + lockDirectory ctlconf.LockDirectory + ui ui.UI } -func NewDirectory(opts ctlconf.Directory, lockConfig ctlconf.LockConfig, ui ui.UI) *Directory { - return &Directory{opts, lockConfig, ui} +func NewDirectory(opts ctlconf.Directory, lockDirectory ctlconf.LockDirectory, ui ui.UI) *Directory { + return &Directory{opts, lockDirectory, ui} } type SyncOpts struct { @@ -83,7 +83,7 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { } // error is safe to ignore, since it indicates that no lock file entry for the given path exists - oldLockContent, _ := d.lockConfig.FindContents(d.opts.Path, contents.Path) + oldLockContent, _ := d.lockDirectory.FindContents(contents.Path) skipFetching, lazySyncAddConfigDigest := handleLazySync(oldLockContent.ConfigDigest, configDigest, syncOpts.Lazy, contents.Lazy) if skipFetching { From c004cacd213a822768ddad7488aa2834f208c0ff Mon Sep 17 00:00:00 2001 From: Fritz Duchardt Date: Sun, 8 Oct 2023 15:49:37 +0200 Subject: [PATCH 07/10] Change naming Signed-off-by: Fritz Duchardt --- pkg/vendir/cmd/sync.go | 2 +- pkg/vendir/config/lock_config.go | 2 +- pkg/vendir/directory/directory.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/vendir/cmd/sync.go b/pkg/vendir/cmd/sync.go index 4891e0c0..4b74c5b8 100644 --- a/pkg/vendir/cmd/sync.go +++ b/pkg/vendir/cmd/sync.go @@ -138,7 +138,7 @@ func (o *SyncOptions) Run() error { for _, dirConf := range conf.Directories { // error safe to ignore, since lock file might not exist - dirExistingLockConf, _ := existingLockConfig.FindLockDirectory(dirConf.Path) + dirExistingLockConf, _ := existingLockConfig.FindDirectory(dirConf.Path) dirLockConf, err := ctldir.NewDirectory(dirConf, dirExistingLockConf, o.ui).Sync(syncOpts) if err != nil { return fmt.Errorf("Syncing directory '%s': %s", dirConf.Path, err) diff --git a/pkg/vendir/config/lock_config.go b/pkg/vendir/config/lock_config.go index e3dc2064..c449ffff 100644 --- a/pkg/vendir/config/lock_config.go +++ b/pkg/vendir/config/lock_config.go @@ -121,7 +121,7 @@ func (c LockConfig) FindContents(dirPath, conPath string) (LockDirectoryContents "Expected to find directory '%s' within lock config, but did not", dirPath) } -func (c LockConfig) FindLockDirectory(dirPath string) (LockDirectory, error) { +func (c LockConfig) FindDirectory(dirPath string) (LockDirectory, error) { for _, dir := range c.Directories { if dir.Path == dirPath { return dir, nil diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index 580606f5..bae4a158 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -83,12 +83,12 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { } // error is safe to ignore, since it indicates that no lock file entry for the given path exists - oldLockContent, _ := d.lockDirectory.FindContents(contents.Path) - skipFetching, lazySyncAddConfigDigest := handleLazySync(oldLockContent.ConfigDigest, configDigest, syncOpts.Lazy, contents.Lazy) + oldLockContents, _ := d.lockDirectory.FindContents(contents.Path) + skipFetching, lazySyncAddConfigDigest := handleLazySync(oldLockContents.ConfigDigest, configDigest, syncOpts.Lazy, contents.Lazy) if skipFetching { d.ui.PrintLinef("Skipping fetch: %s + %s (flagged as lazy, config has not changed since last sync)", d.opts.Path, contents.Path) - lockConfig.Contents = append(lockConfig.Contents, oldLockContent) + lockConfig.Contents = append(lockConfig.Contents, oldLockContents) continue } From 622b00c418c22454c89f61f426b5f490e53f9630 Mon Sep 17 00:00:00 2001 From: Fritz Duchardt Date: Sun, 15 Oct 2023 12:08:55 +0200 Subject: [PATCH 08/10] Added e2e test, removed unit test, improved help message. Signed-off-by: Fritz Duchardt --- examples/lazy/some-content/some-file.txt | 0 examples/lazy/vendir-lazy.yml | 9 +++ examples/lazy/vendir-nonlazy.yml | 8 ++ examples/lazy/vendir.lock.yml | 7 ++ go.mod | 3 +- pkg/vendir/cmd/sync.go | 2 +- pkg/vendir/directory/directory.go | 6 +- pkg/vendir/directory/directory_test.go | 60 --------------- test/e2e/example_lazy_test.go | 94 ++++++++++++++++++++++++ 9 files changed, 123 insertions(+), 66 deletions(-) create mode 100644 examples/lazy/some-content/some-file.txt create mode 100644 examples/lazy/vendir-lazy.yml create mode 100644 examples/lazy/vendir-nonlazy.yml create mode 100644 examples/lazy/vendir.lock.yml delete mode 100644 pkg/vendir/directory/directory_test.go create mode 100644 test/e2e/example_lazy_test.go diff --git a/examples/lazy/some-content/some-file.txt b/examples/lazy/some-content/some-file.txt new file mode 100644 index 00000000..e69de29b diff --git a/examples/lazy/vendir-lazy.yml b/examples/lazy/vendir-lazy.yml new file mode 100644 index 00000000..46d64abf --- /dev/null +++ b/examples/lazy/vendir-lazy.yml @@ -0,0 +1,9 @@ +apiVersion: vendir.k14s.io/v1alpha1 +kind: Config +directories: +- path: vendor + contents: + - path: dir + lazy: true + directory: + path: ./some-content diff --git a/examples/lazy/vendir-nonlazy.yml b/examples/lazy/vendir-nonlazy.yml new file mode 100644 index 00000000..e8e0a528 --- /dev/null +++ b/examples/lazy/vendir-nonlazy.yml @@ -0,0 +1,8 @@ +apiVersion: vendir.k14s.io/v1alpha1 +kind: Config +directories: +- path: vendor + contents: + - path: dir + directory: + path: ./some-content diff --git a/examples/lazy/vendir.lock.yml b/examples/lazy/vendir.lock.yml new file mode 100644 index 00000000..38c07d1c --- /dev/null +++ b/examples/lazy/vendir.lock.yml @@ -0,0 +1,7 @@ +apiVersion: vendir.k14s.io/v1alpha1 +directories: +- contents: + - directory: {} + path: dir + path: vendor +kind: LockConfig diff --git a/go.mod b/go.mod index 4bc045d0..559bdda6 100644 --- a/go.mod +++ b/go.mod @@ -27,8 +27,6 @@ require ( sigs.k8s.io/yaml v1.3.0 ) -require gopkg.in/yaml.v3 v3.0.1 - require ( cloud.google.com/go/compute v1.18.0 // indirect cloud.google.com/go/compute/metadata v0.2.3 // indirect @@ -108,6 +106,7 @@ require ( google.golang.org/appengine v1.6.7 // indirect google.golang.org/protobuf v1.29.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c // indirect k8s.io/klog v1.0.0 // indirect k8s.io/klog/v2 v2.70.1 // indirect diff --git a/pkg/vendir/cmd/sync.go b/pkg/vendir/cmd/sync.go index 4b74c5b8..4357ca30 100644 --- a/pkg/vendir/cmd/sync.go +++ b/pkg/vendir/cmd/sync.go @@ -51,7 +51,7 @@ func NewSyncCmd(o *SyncOptions) *cobra.Command { cmd.Flags().StringSliceVarP(&o.Directories, "directory", "d", nil, "Sync specific directory (format: dir/sub-dir[=local-dir])") cmd.Flags().BoolVarP(&o.Locked, "locked", "l", false, "Consult lock file to pull exact references (e.g. use git sha instead of branch name)") - cmd.Flags().BoolVar(&o.Lazy, "lazy", true, "Override \"lazy\" value in directory content configuration") + cmd.Flags().BoolVar(&o.Lazy, "lazy", true, "Set to 'false' it ignores the 'lazy' flag in the directory content configuration") cmd.Flags().StringVar(&o.Chdir, "chdir", "", "Set current directory for process") cmd.Flags().BoolVar(&o.AllowAllSymlinkDestinations, "dangerous-allow-all-symlink-destinations", false, "Symlinks to all destinations are allowed") diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index bae4a158..d0ba0bb1 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -7,9 +7,9 @@ import ( "crypto/sha256" "encoding/hex" "fmt" - "gopkg.in/yaml.v3" "os" "path/filepath" + "sigs.k8s.io/yaml" "github.com/cppforlife/go-cli-ui/ui" dircopy "github.com/otiai10/copy" @@ -84,7 +84,7 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { // error is safe to ignore, since it indicates that no lock file entry for the given path exists oldLockContents, _ := d.lockDirectory.FindContents(contents.Path) - skipFetching, lazySyncAddConfigDigest := handleLazySync(oldLockContents.ConfigDigest, configDigest, syncOpts.Lazy, contents.Lazy) + skipFetching, lazySyncAddConfigDigest := d.handleLazySync(oldLockContents.ConfigDigest, configDigest, syncOpts.Lazy, contents.Lazy) if skipFetching { d.ui.PrintLinef("Skipping fetch: %s + %s (flagged as lazy, config has not changed since last sync)", d.opts.Path, contents.Path) @@ -279,7 +279,7 @@ func maybeChmod(path string, potentialPerms ...*os.FileMode) error { return nil } -func handleLazySync(oldConfigDigest string, newConfigDigest string, fetchLazyGlobalOverride bool, fetchLazy bool) (bool, bool) { +func (d *Directory) handleLazySync(oldConfigDigest string, newConfigDigest string, fetchLazyGlobalOverride bool, fetchLazy bool) (bool, bool) { skipFetching := false addConfigDigest := false // if lazy sync is enabled and config remains unchanged, skip fetching diff --git a/pkg/vendir/directory/directory_test.go b/pkg/vendir/directory/directory_test.go deleted file mode 100644 index 0564511d..00000000 --- a/pkg/vendir/directory/directory_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package directory - -import "testing" - -func Test_handleLazySync(t *testing.T) { - type args struct { - oldConfigDigest string - newConfigDigest string - fetchLazyOverride bool - fetchContentLazy bool - } - tests := []struct { - name string - args args - skipFetching bool - addConfigDigest bool - }{ - { - "no skipping of sync or adding of config digest, if lazy sync is disabled globally and locally", - args{"same", "same", false, false}, - false, - false, - }, - { - "no skipping of sync or adding of config digest, if lazy sync is enabled locally", - args{"same", "same", true, false}, - false, - false, - }, - { - "no skipping of sync but adding of config digest, if lazy sync is disabled globally", - args{"same", "same", false, true}, - false, - true, - }, - { - "no skipping of sync but adding of config digest, if lazy sync is enabled but config digests don't match", - args{"same", "not-same", true, true}, - false, - true, - }, - { - "skipping of sync and adding of config digest, if lazy sync is enabled and config digests match", - args{"same", "same", true, true}, - true, - true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - skipFetching, addConfigDigest := handleLazySync(tt.args.oldConfigDigest, tt.args.newConfigDigest, tt.args.fetchLazyOverride, tt.args.fetchContentLazy) - if skipFetching != tt.skipFetching { - t.Errorf("handleLazySync() got = %v, want %v", skipFetching, tt.skipFetching) - } - if addConfigDigest != tt.addConfigDigest { - t.Errorf("handleLazySync() got1 = %v, want %v", addConfigDigest, tt.addConfigDigest) - } - }) - } -} diff --git a/test/e2e/example_lazy_test.go b/test/e2e/example_lazy_test.go new file mode 100644 index 00000000..e3087150 --- /dev/null +++ b/test/e2e/example_lazy_test.go @@ -0,0 +1,94 @@ +// Copyright 2020 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + ctlconf "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/config" + "os" + "testing" +) + +func TestExampleLazy(t *testing.T) { + env := BuildEnv(t) + vendir := Vendir{t, env.BinaryPath, Logger{}} + + osEnv := []string{"VENDIR_HELM_BINARY=" + env.Helm2Binary} + + dir := "examples/lazy" + path := "../../" + dir + + _, err := vendir.RunWithOpts([]string{"sync", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) + if err != nil { + t.Fatalf("Expected no err") + } + + lockConf, err := ctlconf.NewLockConfigFromFile(path + "/vendir.lock.yml") + if err != nil { + t.Fatalf("Expected no err: %s", err) + } + + // find content digest + if len(lockConf.Directories[0].Contents[0].ConfigDigest) == 0 { + t.Fatalf("Expected Config Digest in Lock File") + } + + // remove some directory + err = os.RemoveAll(path + "/vendor/dir") + if err != nil { + t.Fatalf("Expected no err") + } + + // resync lazily + _, err = vendir.RunWithOpts([]string{"sync", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) + if err != nil { + t.Fatalf("Expected no err") + } + + _, err = os.Stat(path + "/vendor/dir") + if err == nil { + t.Fatalf("Expected err") + } else if !os.IsNotExist(err) { + t.Fatalf("Expected IsNotExist err") + } + + // resync with lazy override + _, err = vendir.RunWithOpts([]string{"sync", "--lazy=false", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) + if err != nil { + t.Fatalf("Expected no err") + } + + stat, err := os.Stat(path + "/vendor/dir") + if err != nil { + t.Fatalf("Expected no err") + } + if !stat.IsDir() { + t.Fatalf("Expected Directory") + } + + // content digest is kept during lazy sync override + lockConf, err = ctlconf.NewLockConfigFromFile(path + "/vendir.lock.yml") + if err != nil { + t.Fatalf("Expected no err") + } + if len(lockConf.Directories[0].Contents[0].ConfigDigest) == 0 { + t.Fatalf("Expected Config Digest in Lock File") + } + + // if synced without lazy flag in vendir.yml, no config digest is kept in lock file + _, err = vendir.RunWithOpts([]string{"sync", "-f=vendir-nonlazy.yml"}, RunOpts{Dir: path, Env: osEnv}) + if err != nil { + t.Fatalf("Expected no err") + } + + lockConf, err = ctlconf.NewLockConfigFromFile(path + "/vendir.lock.yml") + if err != nil { + t.Fatalf("Expected no err") + } + + // find content digest + if len(lockConf.Directories[0].Contents[0].ConfigDigest) != 0 { + t.Fatalf("Expected No Config Digest in Lock File") + } + +} From e9af3bad3241aefcf173078b94e7f636af2ce3d3 Mon Sep 17 00:00:00 2001 From: Fritz Duchardt Date: Sun, 15 Oct 2023 13:01:05 +0200 Subject: [PATCH 09/10] Some more comments Signed-off-by: Fritz Duchardt --- test/e2e/example_lazy_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/e2e/example_lazy_test.go b/test/e2e/example_lazy_test.go index e3087150..2b1fa442 100644 --- a/test/e2e/example_lazy_test.go +++ b/test/e2e/example_lazy_test.go @@ -18,6 +18,7 @@ func TestExampleLazy(t *testing.T) { dir := "examples/lazy" path := "../../" + dir + // run lazy sync _, err := vendir.RunWithOpts([]string{"sync", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) if err != nil { t.Fatalf("Expected no err") @@ -39,7 +40,7 @@ func TestExampleLazy(t *testing.T) { t.Fatalf("Expected no err") } - // resync lazily + // resync lazily, should not sync. Removed dir has not been reinstated _, err = vendir.RunWithOpts([]string{"sync", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) if err != nil { t.Fatalf("Expected no err") @@ -52,7 +53,7 @@ func TestExampleLazy(t *testing.T) { t.Fatalf("Expected IsNotExist err") } - // resync with lazy override + // resync with lazy override, should not affect config digest _, err = vendir.RunWithOpts([]string{"sync", "--lazy=false", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) if err != nil { t.Fatalf("Expected no err") @@ -75,7 +76,7 @@ func TestExampleLazy(t *testing.T) { t.Fatalf("Expected Config Digest in Lock File") } - // if synced without lazy flag in vendir.yml, no config digest is kept in lock file + // if synced without lazy flag in vendir.yml, no config digest should be kept in lock file _, err = vendir.RunWithOpts([]string{"sync", "-f=vendir-nonlazy.yml"}, RunOpts{Dir: path, Env: osEnv}) if err != nil { t.Fatalf("Expected no err") @@ -86,9 +87,8 @@ func TestExampleLazy(t *testing.T) { t.Fatalf("Expected no err") } - // find content digest + // Ensure no content digest set if len(lockConf.Directories[0].Contents[0].ConfigDigest) != 0 { t.Fatalf("Expected No Config Digest in Lock File") } - } From 82c3c9f4b9ef0d4e125e7b49057bca51b112be68 Mon Sep 17 00:00:00 2001 From: Fritz Duchardt Date: Thu, 19 Oct 2023 08:15:55 +0200 Subject: [PATCH 10/10] Introduced require test function in e2e test Signed-off-by: Fritz Duchardt --- test/e2e/example_lazy_test.go | 68 ++++++++--------------------------- 1 file changed, 15 insertions(+), 53 deletions(-) diff --git a/test/e2e/example_lazy_test.go b/test/e2e/example_lazy_test.go index 2b1fa442..e460a74f 100644 --- a/test/e2e/example_lazy_test.go +++ b/test/e2e/example_lazy_test.go @@ -4,6 +4,7 @@ package e2e import ( + "github.com/stretchr/testify/require" ctlconf "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/config" "os" "testing" @@ -20,75 +21,36 @@ func TestExampleLazy(t *testing.T) { // run lazy sync _, err := vendir.RunWithOpts([]string{"sync", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) - if err != nil { - t.Fatalf("Expected no err") - } + require.NoError(t, err) + // check that the lock file has config digest lockConf, err := ctlconf.NewLockConfigFromFile(path + "/vendir.lock.yml") - if err != nil { - t.Fatalf("Expected no err: %s", err) - } - - // find content digest - if len(lockConf.Directories[0].Contents[0].ConfigDigest) == 0 { - t.Fatalf("Expected Config Digest in Lock File") - } + require.NoError(t, err) + require.NotEmpty(t, lockConf.Directories[0].Contents[0].ConfigDigest, "Expected Config Digest in Lock File") // remove some directory err = os.RemoveAll(path + "/vendor/dir") - if err != nil { - t.Fatalf("Expected no err") - } + require.NoError(t, err) // resync lazily, should not sync. Removed dir has not been reinstated _, err = vendir.RunWithOpts([]string{"sync", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) - if err != nil { - t.Fatalf("Expected no err") - } - - _, err = os.Stat(path + "/vendor/dir") - if err == nil { - t.Fatalf("Expected err") - } else if !os.IsNotExist(err) { - t.Fatalf("Expected IsNotExist err") - } + require.NoError(t, err) + require.NoDirExists(t, path+"/vendor/dir") // resync with lazy override, should not affect config digest _, err = vendir.RunWithOpts([]string{"sync", "--lazy=false", "-f=vendir-lazy.yml"}, RunOpts{Dir: path, Env: osEnv}) - if err != nil { - t.Fatalf("Expected no err") - } - - stat, err := os.Stat(path + "/vendor/dir") - if err != nil { - t.Fatalf("Expected no err") - } - if !stat.IsDir() { - t.Fatalf("Expected Directory") - } + require.NoError(t, err) + require.DirExists(t, path+"/vendor/dir") // content digest is kept during lazy sync override lockConf, err = ctlconf.NewLockConfigFromFile(path + "/vendir.lock.yml") - if err != nil { - t.Fatalf("Expected no err") - } - if len(lockConf.Directories[0].Contents[0].ConfigDigest) == 0 { - t.Fatalf("Expected Config Digest in Lock File") - } + require.NoError(t, err) + require.NotEmpty(t, lockConf.Directories[0].Contents[0].ConfigDigest, "Expected Config Digest in Lock File") // if synced without lazy flag in vendir.yml, no config digest should be kept in lock file _, err = vendir.RunWithOpts([]string{"sync", "-f=vendir-nonlazy.yml"}, RunOpts{Dir: path, Env: osEnv}) - if err != nil { - t.Fatalf("Expected no err") - } - + require.NoError(t, err) lockConf, err = ctlconf.NewLockConfigFromFile(path + "/vendir.lock.yml") - if err != nil { - t.Fatalf("Expected no err") - } - - // Ensure no content digest set - if len(lockConf.Directories[0].Contents[0].ConfigDigest) != 0 { - t.Fatalf("Expected No Config Digest in Lock File") - } + require.NoError(t, err) + require.Empty(t, lockConf.Directories[0].Contents[0].ConfigDigest, "Expected No Config Digest in Lock File") }