From f8740a488293b2407ea0ed01926ad48f36c95561 Mon Sep 17 00:00:00 2001 From: Parthiba-Hazra Date: Sun, 10 Mar 2024 23:02:05 +0530 Subject: [PATCH] refactor: Refactor image JSON handling into a dedicated manager - This commit introduces an ImagePullPolicyManager struct responsible for handling image JSON operations, including reading and writing the image.json file. The ReadImageJSON and WriteFile functions have been refactored to methods of this manager. Signed-off-by: Parthiba-Hazra --- internal/commands/build.go | 2 +- internal/commands/builder_create.go | 2 +- internal/commands/buildpack_package.go | 2 +- internal/commands/config_prune_interval.go | 26 ++-- internal/commands/config_pull_policy.go | 6 +- internal/commands/create_builder.go | 2 +- internal/commands/extension_package.go | 2 +- internal/commands/package_buildpack.go | 2 +- internal/commands/rebase.go | 2 +- pkg/client/client.go | 2 +- pkg/image/fetcher.go | 67 ++++------ pkg/image/fetcher_test.go | 78 ++++++----- pkg/image/pull_policy.go | 142 +++++++++++---------- pkg/image/pull_policy_test.go | 18 +-- 14 files changed, 172 insertions(+), 181 deletions(-) diff --git a/internal/commands/build.go b/internal/commands/build.go index 3dcf6cc43b..a137c968ad 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -132,7 +132,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index 56ac071fe2..d6604e3576 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -50,7 +50,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index adb95c3e03..35f99ffc98 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -62,7 +62,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/config_prune_interval.go b/internal/commands/config_prune_interval.go index a4b00d9c0e..eae27bb307 100644 --- a/internal/commands/config_prune_interval.go +++ b/internal/commands/config_prune_interval.go @@ -1,7 +1,6 @@ package commands import ( - "encoding/json" "fmt" "regexp" @@ -17,6 +16,7 @@ import ( func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath string) *cobra.Command { var unset bool var intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) + pullPolicyManager := image.NewPullPolicyManager(logger) cmd := &cobra.Command{ Use: "prune-interval", @@ -28,30 +28,31 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin "* To unset the pruning interval, run `pack config prune-interval --unset`.\n" + fmt.Sprintf("Unsetting the pruning interval will reset the interval to the default, which is %s.", style.Symbol("7 days")), RunE: logError(logger, func(cmd *cobra.Command, args []string) error { + imageJSONPath, err := image.DefaultImageJSONPath() + if err != nil { + return err + } + switch { case unset: if len(args) > 0 { return errors.Errorf("prune interval and --unset cannot be specified simultaneously") } - imageJSON, err := image.ReadImageJSON(logger) + imageJSON, err := pullPolicyManager.Read(imageJSONPath) if err != nil { return err } oldPruneInterval := imageJSON.Interval.PruningInterval imageJSON.Interval.PruningInterval = "7d" - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - return errors.Wrap(err, "failed to marshal updated records") - } - err = image.WriteFile(updatedJSON) + err = pullPolicyManager.Write(imageJSON, imageJSONPath) if err != nil { return err } logger.Infof("Successfully unset pruning interval %s", style.Symbol(oldPruneInterval)) logger.Infof("Pruning interval has been set to %s", style.Symbol(imageJSON.Interval.PruningInterval)) case len(args) == 0: // list - imageJSON, err := image.ReadImageJSON(logger) + imageJSON, err := pullPolicyManager.Read(imageJSONPath) if err != nil { return err } @@ -64,7 +65,7 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin default: // set newPruneInterval := args[0] - imageJSON, err := image.ReadImageJSON(logger) + imageJSON, err := pullPolicyManager.Read(imageJSONPath) if err != nil { return err } @@ -84,11 +85,8 @@ func ConfigPruneInterval(logger logging.Logger, cfg config.Config, cfgPath strin } imageJSON.Interval.PruningInterval = newPruneInterval - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - return errors.Wrap(err, "failed to marshal updated records") - } - err = image.WriteFile(updatedJSON) + + err = pullPolicyManager.Write(imageJSON, imageJSONPath) if err != nil { return err } diff --git a/internal/commands/config_pull_policy.go b/internal/commands/config_pull_policy.go index 54711b26d6..a5d1968b99 100644 --- a/internal/commands/config_pull_policy.go +++ b/internal/commands/config_pull_policy.go @@ -36,7 +36,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) return errors.Wrapf(err, "writing config to %s", cfgPath) } - pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy) + pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy, logger) if err != nil { return err } @@ -44,7 +44,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) logger.Infof("Successfully unset pull policy %s", style.Symbol(oldPullPolicy)) logger.Infof("Pull policy has been set to %s", style.Symbol(pullPolicy.String())) case len(args) == 0: // list - pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy) + pullPolicy, err := image.ParsePullPolicy(cfg.PullPolicy, logger) if err != nil { return err } @@ -58,7 +58,7 @@ func ConfigPullPolicy(logger logging.Logger, cfg config.Config, cfgPath string) return nil } - pullPolicy, err := image.ParsePullPolicy(newPullPolicy) + pullPolicy, err := image.ParsePullPolicy(newPullPolicy, logger) if err != nil { return err } diff --git a/internal/commands/create_builder.go b/internal/commands/create_builder.go index 9999392ef6..4ac3f6a09a 100644 --- a/internal/commands/create_builder.go +++ b/internal/commands/create_builder.go @@ -43,7 +43,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", flags.Policy) } diff --git a/internal/commands/extension_package.go b/internal/commands/extension_package.go index 0415823e57..cea78a629a 100644 --- a/internal/commands/extension_package.go +++ b/internal/commands/extension_package.go @@ -45,7 +45,7 @@ func ExtensionPackage(logger logging.Logger, cfg config.Config, packager Extensi stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/package_buildpack.go b/internal/commands/package_buildpack.go index 1bd17c4561..a5fc9b6da9 100644 --- a/internal/commands/package_buildpack.go +++ b/internal/commands/package_buildpack.go @@ -41,7 +41,7 @@ func PackageBuildpack(logger logging.Logger, cfg config.Config, packager Buildpa if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - pullPolicy, err := image.ParsePullPolicy(stringPolicy) + pullPolicy, err := image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrap(err, "parsing pull policy") } diff --git a/internal/commands/rebase.go b/internal/commands/rebase.go index 0eed5f7281..3116ad7e93 100644 --- a/internal/commands/rebase.go +++ b/internal/commands/rebase.go @@ -33,7 +33,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co if stringPolicy == "" { stringPolicy = cfg.PullPolicy } - opts.PullPolicy, err = image.ParsePullPolicy(stringPolicy) + opts.PullPolicy, err = image.ParsePullPolicy(stringPolicy, logger) if err != nil { return errors.Wrapf(err, "parsing pull policy %s", stringPolicy) } diff --git a/pkg/client/client.go b/pkg/client/client.go index 9786b61e0b..ad974cd44b 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -231,7 +231,7 @@ func NewClient(opts ...Option) (*Client, error) { } if client.imageFetcher == nil { - imagePullChecker := image.NewPullChecker(client.logger) + imagePullChecker := image.NewPullPolicyManager(client.logger) client.imageFetcher = image.NewFetcher(client.logger, client.docker, imagePullChecker, image.WithRegistryMirrors(client.registryMirrors), image.WithKeychain(client.keychain)) } diff --git a/pkg/image/fetcher.go b/pkg/image/fetcher.go index 6f584ec843..9f26c2921e 100644 --- a/pkg/image/fetcher.go +++ b/pkg/image/fetcher.go @@ -37,22 +37,19 @@ type LayoutOption struct { } type ImagePullChecker interface { - CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) - ReadImageJSON(l logging.Logger) (*ImageJSON, error) - PruneOldImages(l logging.Logger, f *Fetcher) error - UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error + CheckImagePullInterval(imageID string, path string) (bool, error) + Read(path string) (*ImageJSON, error) + PruneOldImages(f *Fetcher) error + UpdateImagePullRecord(path string, imageID string, timestamp string) error + Write(imageJSON *ImageJSON, path string) error } func intervalPolicy(options FetchOptions) bool { return options.PullPolicy == PullWithInterval || options.PullPolicy == PullHourly || options.PullPolicy == PullDaily || options.PullPolicy == PullWeekly } -type PullChecker struct { - logger logging.Logger -} - -func NewPullChecker(logger logging.Logger) *PullChecker { - return &PullChecker{logger: logger} +func NewPullPolicyManager(logger logging.Logger) *ImagePullPolicyManager { + return &ImagePullPolicyManager{Logger: logger} } // WithRegistryMirrors supply your own mirrors for registry. @@ -119,6 +116,11 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return f.fetchRemoteImage(name) } + imageJSONpath, err := DefaultImageJSONPath() + if err != nil { + return nil, err + } + switch options.PullPolicy { case PullNever: img, err := f.fetchDaemonImage(name) @@ -129,28 +131,24 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) return img, err } case PullWithInterval, PullDaily, PullHourly, PullWeekly: - pull, err := f.imagePullChecker.CheckImagePullInterval(name, f.logger) + pull, err := f.imagePullChecker.CheckImagePullInterval(name, imageJSONpath) if err != nil { f.logger.Warnf("failed to check pulling interval for image %s, %s", name, err) } if !pull { img, err := f.fetchDaemonImage(name) if errors.Is(err, ErrNotFound) { - imageJSON, _ := f.imagePullChecker.ReadImageJSON(f.logger) + imageJSON, _ := f.imagePullChecker.Read(imageJSONpath) delete(imageJSON.Image.ImageIDtoTIME, name) - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - f.logger.Errorf("failed to marshal updated records %s", err) - } - if err := WriteFile(updatedJSON); err != nil { + if err := f.imagePullChecker.Write(imageJSON, imageJSONpath); err != nil { f.logger.Errorf("failed to write updated image.json %s", err) } } return img, err } - err = f.imagePullChecker.PruneOldImages(f.logger, f) + err = f.imagePullChecker.PruneOldImages(f) if err != nil { f.logger.Warnf("Failed to prune images, %s", err) } @@ -175,7 +173,7 @@ func (f *Fetcher) Fetch(ctx context.Context, name string, options FetchOptions) if intervalPolicy(options) { // Update image pull record in the JSON file - if err := f.imagePullChecker.UpdateImagePullRecord(f.logger, name, time.Now().Format(time.RFC3339)); err != nil { + if err := f.imagePullChecker.UpdateImagePullRecord(imageJSONpath, name, time.Now().Format(time.RFC3339)); err != nil { return nil, err } } @@ -307,8 +305,8 @@ func (w *colorizedWriter) Write(p []byte) (n int, err error) { return w.writer.Write([]byte(msg)) } -func UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { - imageJSON, err := ReadImageJSON(l) +func (i *ImagePullPolicyManager) UpdateImagePullRecord(path string, imageID string, timestamp string) error { + imageJSON, err := i.Read(path) if err != nil { return err } @@ -318,12 +316,7 @@ func UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) e } imageJSON.Image.ImageIDtoTIME[imageID] = timestamp - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - return errors.New("failed to marshal updated records: " + err.Error()) - } - - err = WriteFile(updatedJSON) + err = i.Write(imageJSON, path) if err != nil { return err } @@ -331,24 +324,8 @@ func UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) e return nil } -func (c *PullChecker) CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { - return CheckImagePullInterval(imageID, l) -} - -func (c *PullChecker) ReadImageJSON(l logging.Logger) (*ImageJSON, error) { - return ReadImageJSON(l) -} - -func (c *PullChecker) PruneOldImages(l logging.Logger, f *Fetcher) error { - return PruneOldImages(l, f) -} - -func (c *PullChecker) UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { - return UpdateImagePullRecord(l, imageID, timestamp) -} - -func CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { - imageJSON, err := ReadImageJSON(l) +func (i *ImagePullPolicyManager) CheckImagePullInterval(imageID string, path string) (bool, error) { + imageJSON, err := i.Read(path) if err != nil { return false, err } diff --git a/pkg/image/fetcher_test.go b/pkg/image/fetcher_test.go index f96cacd2c6..f20ba35ace 100644 --- a/pkg/image/fetcher_test.go +++ b/pkg/image/fetcher_test.go @@ -32,29 +32,37 @@ var imageJSON *image.ImageJSON var mockImagePullChecker = NewMockImagePullChecker(logger) type MockPullChecker struct { - *image.PullChecker - MockCheckImagePullInterval func(imageID string, l logging.Logger) (bool, error) - MockReadImageJSON func(l logging.Logger) (*image.ImageJSON, error) - MockPruneOldImages func(l logging.Logger, f *image.Fetcher) error - MockUpdateImagePullRecord func(l logging.Logger, imageID string, timestamp string) error + *image.ImagePullPolicyManager + MockCheckImagePullInterval func(imageID string, path string) (bool, error) + MockRead func(path string) (*image.ImageJSON, error) + MockPruneOldImages func(f *image.Fetcher) error + MockUpdateImagePullRecord func(path string, imageID string, timestamp string) error + MockWrite func(imageJSON *image.ImageJSON, path string) error } func NewMockImagePullChecker(logger logging.Logger) *MockPullChecker { return &MockPullChecker{ - PullChecker: image.NewPullChecker(logger), + ImagePullPolicyManager: image.NewPullPolicyManager(logger), } } -func (m *MockPullChecker) CheckImagePullInterval(imageID string, l logging.Logger) (bool, error) { +func (m *MockPullChecker) CheckImagePullInterval(imageID string, path string) (bool, error) { if m.MockCheckImagePullInterval != nil { - return m.MockCheckImagePullInterval(imageID, l) + return m.MockCheckImagePullInterval(imageID, path) } return false, nil } -func (m *MockPullChecker) ReadImageJSON(l logging.Logger) (*image.ImageJSON, error) { - if m.MockReadImageJSON != nil { - return m.MockReadImageJSON(l) +func (m *MockPullChecker) Write(imageJSON *image.ImageJSON, path string) error { + if m.MockWrite != nil { + return m.MockWrite(imageJSON, path) + } + return nil +} + +func (m *MockPullChecker) Read(path string) (*image.ImageJSON, error) { + if m.MockRead != nil { + return m.MockRead(path) } imageJSON = &image.ImageJSON{ @@ -73,18 +81,18 @@ func (m *MockPullChecker) ReadImageJSON(l logging.Logger) (*image.ImageJSON, err return imageJSON, nil } -func (m *MockPullChecker) PruneOldImages(l logging.Logger, f *image.Fetcher) error { +func (m *MockPullChecker) PruneOldImages(f *image.Fetcher) error { if m.MockPruneOldImages != nil { - return m.MockPruneOldImages(l, f) + return m.MockPruneOldImages(f) } return nil } -func (m *MockPullChecker) UpdateImagePullRecord(l logging.Logger, imageID string, timestamp string) error { +func (m *MockPullChecker) UpdateImagePullRecord(path string, imageID string, timestamp string) error { if m.MockUpdateImagePullRecord != nil { fmt.Printf("checking wheather its calling or not") - return m.MockUpdateImagePullRecord(l, imageID, timestamp) + return m.MockUpdateImagePullRecord(path, imageID, timestamp) } return nil @@ -232,7 +240,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { var outCons *color.Console outCons, output = h.MockWriterAndOutput() logger = logging.NewLogWithWriters(outCons, outCons) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logger, docker, mockImagePullChecker) @@ -440,7 +448,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns true", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) @@ -462,7 +470,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns false", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return false, nil } @@ -481,20 +489,20 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) - mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } }) it.After(func() { mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockReadImageJSON = nil + mockImagePullChecker.MockRead = nil }) it("returns an error and deletes the image record", func() { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) - imageJSON, err = mockImagePullChecker.ReadImageJSON(logger) + imageJSON, err = mockImagePullChecker.Read("") h.AssertNil(t, err) _, exists := imageJSON.Image.ImageIDtoTIME[repoName] h.AssertEq(t, exists, false) @@ -507,7 +515,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) h.AssertNil(t, img.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } @@ -524,12 +532,12 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { }, } - mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } - mockImagePullChecker.MockUpdateImagePullRecord = func(l logging.Logger, imageID string, timestamp string) error { - imageJSON, _ = mockImagePullChecker.ReadImageJSON(l) + mockImagePullChecker.MockUpdateImagePullRecord = func(path string, imageID string, timestamp string) error { + imageJSON, _ = mockImagePullChecker.Read("") imageJSON.Image.ImageIDtoTIME[repoName] = timestamp return nil } @@ -539,7 +547,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { it.After(func() { mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockReadImageJSON = nil + mockImagePullChecker.MockRead = nil mockImagePullChecker.MockUpdateImagePullRecord = nil h.DockerRmi(docker, repoName) }) @@ -550,7 +558,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertNil(t, err) - imageJSON, _ = mockImagePullChecker.ReadImageJSON(logger) + imageJSON, _ = mockImagePullChecker.Read("") afterFetch, _ := time.Parse(time.RFC3339, imageJSON.Image.ImageIDtoTIME[repoName]) fmt.Printf("after fetch: %v\n", imageJSON.Image.ImageIDtoTIME[repoName]) @@ -566,7 +574,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) @@ -593,7 +601,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns true", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) @@ -611,7 +619,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { when("there is no local image and CheckImagePullInterval returns false", func() { it.Before(func() { - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return false, nil } @@ -629,20 +637,20 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) - mockImagePullChecker.MockReadImageJSON = func(l logging.Logger) (*image.ImageJSON, error) { + mockImagePullChecker.MockRead = func(path string) (*image.ImageJSON, error) { return imageJSON, nil } }) it.After(func() { mockImagePullChecker.MockCheckImagePullInterval = nil - mockImagePullChecker.MockReadImageJSON = nil + mockImagePullChecker.MockRead = nil }) it("returns an error and deletes the image record", func() { _, err := imageFetcher.Fetch(context.TODO(), repoName, image.FetchOptions{Daemon: true, PullPolicy: image.PullWeekly}) h.AssertError(t, err, fmt.Sprintf("image '%s' does not exist on the daemon", repoName)) - imageJSON, err = mockImagePullChecker.ReadImageJSON(logger) + imageJSON, err = mockImagePullChecker.Read("") h.AssertNil(t, err) _, exists := imageJSON.Image.ImageIDtoTIME[repoName] h.AssertEq(t, exists, false) @@ -656,7 +664,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) @@ -684,7 +692,7 @@ func testFetcher(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, localImg.SetLabel(label, "2")) h.AssertNil(t, localImg.Save()) - mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, l logging.Logger) (bool, error) { + mockImagePullChecker.MockCheckImagePullInterval = func(imageID string, path string) (bool, error) { return true, nil } imageFetcher = image.NewFetcher(logging.NewLogWithWriters(&outBuf, &outBuf), docker, mockImagePullChecker) diff --git a/pkg/image/pull_policy.go b/pkg/image/pull_policy.go index 0322ee0e7e..bca560b85b 100644 --- a/pkg/image/pull_policy.go +++ b/pkg/image/pull_policy.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "os" - "os/exec" "path/filepath" "regexp" "strconv" @@ -13,6 +12,7 @@ import ( "github.com/pkg/errors" + "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/pkg/logging" ) @@ -21,12 +21,15 @@ type PullPolicy int var interval string +type ImagePullPolicyManager struct { + Logger logging.Logger +} + var ( hourly = "1h" daily = "1d" weekly = "7d" intervalRegex = regexp.MustCompile(`^(\d+d)?(\d+h)?(\d+m)?$`) - imagePath string ) const ( @@ -61,29 +64,40 @@ type ImageJSON struct { Image *ImageData `json:"image"` } +func DefaultImageJSONPath() (string, error) { + home, err := config.PackHome() + if err != nil { + return "", errors.Wrap(err, "getting pack home") + } + return filepath.Join(home, "image.json"), nil +} + var nameMap = map[string]PullPolicy{"always": PullAlways, "hourly": PullHourly, "daily": PullDaily, "weekly": PullWeekly, "never": PullNever, "if-not-present": PullIfNotPresent, "": PullAlways} // ParsePullPolicy from string with support for interval formats -func ParsePullPolicy(policy string) (PullPolicy, error) { +func ParsePullPolicy(policy string, logger logging.Logger) (PullPolicy, error) { + pullPolicyManager := NewPullPolicyManager(logger) + if val, ok := nameMap[policy]; ok { if val == PullHourly { - err := updateImageJSONDuration(hourly) + err := pullPolicyManager.updateImageJSONDuration(hourly) if err != nil { return PullAlways, err } } if val == PullDaily { - err := updateImageJSONDuration(daily) + err := pullPolicyManager.updateImageJSONDuration(daily) if err != nil { return PullAlways, err } } if val == PullWeekly { - err := updateImageJSONDuration(weekly) + err := pullPolicyManager.updateImageJSONDuration(weekly) if err != nil { return PullAlways, err } } + return val, nil } @@ -95,7 +109,7 @@ func ParsePullPolicy(policy string) (PullPolicy, error) { return PullAlways, errors.Errorf("invalid interval format: %s", intervalStr) } - err := updateImageJSONDuration(intervalStr) + err := pullPolicyManager.updateImageJSONDuration(intervalStr) if err != nil { return PullAlways, err } @@ -127,60 +141,20 @@ func (p PullPolicy) String() string { return "" } -func updateImageJSONDuration(intervalStr string) error { - imageJSON, err := ReadImageJSON(logging.NewSimpleLogger(os.Stderr)) +func (i *ImagePullPolicyManager) updateImageJSONDuration(intervalStr string) error { + path, err := DefaultImageJSONPath() if err != nil { return err } - imageJSON.Interval.PullingInterval = intervalStr - - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + imageJSON, err := i.Read(path) if err != nil { - return errors.Wrap(err, "failed to marshal updated records") - } - - return WriteFile(updatedJSON) -} - -func ReadImageJSON(l logging.Logger) (*ImageJSON, error) { - homeDir, err := os.UserHomeDir() - if err != nil { - return &ImageJSON{}, errors.Wrap(err, "failed to get home directory") - } - imagePath = filepath.Join(homeDir, ".pack", "image.json") - - // Check if the directory exists, if not, create it - dirPath := filepath.Dir(imagePath) - if _, err := os.Stat(dirPath); os.IsNotExist(err) { - l.Warnf("missing `.pack` directory under %s directory %s", homeDir, err) - l.Debugf("creating `.pack` directory under %s directory", homeDir) - if err := os.MkdirAll(dirPath, 0755); err != nil { - return &ImageJSON{}, errors.Wrap(err, "failed to create directory") - } - } - - // Check if the file exists, if not, create it with minimum JSON - if _, err := os.Stat(imagePath); os.IsNotExist(err) { - l.Warnf("missing `image.json` file under %s directory %s", dirPath, err) - l.Debugf("creating `image.json` file under %s directory", dirPath) - minimumJSON := []byte(`{"interval":{"pulling_interval":"","pruning_interval":"7d","last_prune":""},"image":{}}`) - if err := WriteFile(minimumJSON); err != nil { - return &ImageJSON{}, errors.Wrap(err, "failed to create image.json file") - } - } - - jsonData, err := os.ReadFile(imagePath) - if err != nil && !os.IsNotExist(err) { - return &ImageJSON{}, errors.Wrap(err, "failed to read image.json") + return err } - var imageJSON *ImageJSON - if err := json.Unmarshal(jsonData, &imageJSON); err != nil && !os.IsNotExist(err) { - return &ImageJSON{}, errors.Wrap(err, "failed to unmarshal image.json") - } + imageJSON.Interval.PullingInterval = intervalStr - return imageJSON, nil + return i.Write(imageJSON, path) } func parseDurationString(durationStr string) (time.Duration, error) { @@ -214,8 +188,12 @@ func parseDurationString(durationStr string) (time.Duration, error) { return time.Duration(totalMinutes) * time.Minute, nil } -func PruneOldImages(l logging.Logger, f *Fetcher) error { - imageJSON, err := ReadImageJSON(l) +func (i *ImagePullPolicyManager) PruneOldImages(f *Fetcher) error { + path, err := DefaultImageJSONPath() + if err != nil { + return err + } + imageJSON, err := i.Read(path) if err != nil { return err } @@ -263,26 +241,56 @@ func PruneOldImages(l logging.Logger, f *Fetcher) error { imageJSON.Interval.LastPrune = time.Now().Format(time.RFC3339) - updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") - if err != nil { - return errors.Wrap(err, "failed to marshal updated records") - } - - if err := WriteFile(updatedJSON); err != nil { + if err := i.Write(imageJSON, path); err != nil { return errors.Wrap(err, "failed to write updated image.json") } return nil } -func WriteFile(data []byte) error { - cmd := exec.Command("sudo", "sh", "-c", "echo '"+string(data)+"' > "+imagePath) - cmd.Stderr = os.Stderr - cmd.Stdin = os.Stdin +func (i *ImagePullPolicyManager) Read(path string) (*ImageJSON, error) { + // Check if the file exists, if not, return default values + if _, err := os.Stat(path); os.IsNotExist(err) { + return &ImageJSON{ + Interval: &Interval{ + PullingInterval: "7d", + PruningInterval: "7d", + LastPrune: "", + }, + Image: &ImageData{}, + }, nil + } - if err := cmd.Run(); err != nil { - return errors.New("failed to write file with sudo: " + err.Error()) + jsonData, err := os.ReadFile(path) + if err != nil && !os.IsNotExist(err) { + return nil, errors.Wrap(err, "failed to read image.json") + } + var imageJSON *ImageJSON + if err := json.Unmarshal(jsonData, &imageJSON); err != nil && !os.IsNotExist(err) { + return nil, errors.Wrap(err, "failed to unmarshal image.json") + } + return imageJSON, nil +} + +func (i *ImagePullPolicyManager) Write(imageJSON *ImageJSON, path string) error { + updatedJSON, err := json.MarshalIndent(imageJSON, "", " ") + if err != nil { + return errors.Wrap(err, "failed to marshal updated records") } + return WriteFile(updatedJSON, path) +} + +func WriteFile(data []byte, path string) error { + file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return errors.New("failed to open file: " + err.Error()) + } + defer file.Close() + + _, err = file.Write(data) + if err != nil { + return errors.New("failed to write data to file: " + err.Error()) + } return nil } diff --git a/pkg/image/pull_policy_test.go b/pkg/image/pull_policy_test.go index 246cf05eb6..41fde7fddf 100644 --- a/pkg/image/pull_policy_test.go +++ b/pkg/image/pull_policy_test.go @@ -17,54 +17,54 @@ func TestPullPolicy(t *testing.T) { func testPullPolicy(t *testing.T, when spec.G, it spec.S) { when("#ParsePullPolicy", func() { it("returns PullNever for never", func() { - policy, err := image.ParsePullPolicy("never") + policy, err := image.ParsePullPolicy("never", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullNever) }) it("returns PullAlways for always", func() { - policy, err := image.ParsePullPolicy("always") + policy, err := image.ParsePullPolicy("always", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullAlways) }) it("returns PullIfNotPresent for if-not-present", func() { - policy, err := image.ParsePullPolicy("if-not-present") + policy, err := image.ParsePullPolicy("if-not-present", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullIfNotPresent) }) it("returns PullHourly for hourly", func() { - policy, err := image.ParsePullPolicy("hourly") + policy, err := image.ParsePullPolicy("hourly", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullHourly) }) it("returns PullDaily for daily", func() { - policy, err := image.ParsePullPolicy("daily") + policy, err := image.ParsePullPolicy("daily", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullDaily) }) it("returns PullWeekly for weekly", func() { - policy, err := image.ParsePullPolicy("weekly") + policy, err := image.ParsePullPolicy("weekly", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullWeekly) }) it("returns PullWithInterval for interval= format", func() { - policy, err := image.ParsePullPolicy("interval=4d") + policy, err := image.ParsePullPolicy("interval=4d", logger) h.AssertNil(t, err) h.AssertEq(t, policy, image.PullWithInterval) }) it("returns error for unknown string", func() { - _, err := image.ParsePullPolicy("fake-policy-here") + _, err := image.ParsePullPolicy("fake-policy-here", logger) h.AssertError(t, err, "invalid pull policy") }) it("returns error for invalid interval format", func() { - _, err := image.ParsePullPolicy("interval=invalid") + _, err := image.ParsePullPolicy("interval=invalid", logger) h.AssertError(t, err, "invalid interval format") }) })