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 <[email protected]>
buildpacks · Mar 10, 2024 · fe6073a · fe6073a
1 parent 0bb8f69
commit fe6073a
Show file tree

Hide file tree

Showing 14 changed files with 172 additions and 181 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -36,15 +36,15 @@ 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
 				}
 
 				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
@@ -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
@@ -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
@@ -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
@@ -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(cfg.PullPolicy, 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
@@ -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
@@ -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,37 +316,16 @@ 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
 	}
 
 	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
 	}