From e026382461ed06e98067db05bf2096a673329082 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Fri, 21 Jun 2024 19:36:52 -0600 Subject: [PATCH 01/22] Add mock config Signed-off-by: Miaha Cybersec --- pkg/patch/patch.go | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/pkg/patch/patch.go b/pkg/patch/patch.go index 2e9d0721..9537de7d 100644 --- a/pkg/patch/patch.go +++ b/pkg/patch/patch.go @@ -3,6 +3,7 @@ package patch import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io" @@ -11,20 +12,19 @@ import ( "strings" "time" - "github.com/containerd/containerd/platforms" - "github.com/docker/buildx/build" "github.com/docker/cli/cli/config" - log "github.com/sirupsen/logrus" - "golang.org/x/exp/slices" + + "github.com/docker/buildx/build" + "github.com/moby/buildkit/client" + "github.com/moby/buildkit/session" + "github.com/moby/buildkit/session/auth/authprovider" "golang.org/x/sync/errgroup" + "github.com/containerd/containerd/platforms" "github.com/distribution/reference" - "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/exporter/containerimage/exptypes" gwclient "github.com/moby/buildkit/frontend/gateway/client" - "github.com/moby/buildkit/session" - "github.com/moby/buildkit/session/auth/authprovider" "github.com/moby/buildkit/util/progress/progressui" "github.com/project-copacetic/copacetic/pkg/buildkit" "github.com/project-copacetic/copacetic/pkg/pkgmgr" @@ -33,6 +33,8 @@ import ( "github.com/project-copacetic/copacetic/pkg/utils" "github.com/project-copacetic/copacetic/pkg/vex" "github.com/quay/claircore/osrelease" + log "github.com/sirupsen/logrus" + "golang.org/x/exp/slices" ) const ( @@ -42,6 +44,18 @@ const ( defaultTag = "latest" ) +type Config struct { + Cmd []string `json:"Cmd"` + WorkingDir string `json:"WorkingDir"` + ExposedPorts map[string]struct{} `json:"ExposedPorts"` + Env []string `json:"Env"` + Labels map[string]string `json:"Labels"` +} + +type ImgConfig struct { + Config Config `json:"Config"` +} + // Patch command applies package updates to an OCI image given a vulnerability report. func Patch(ctx context.Context, timeout time.Duration, image, reportFile, patchedTag, workingFolder, scanner, format, output string, ignoreError bool, bkOpts buildkit.Opts) error { timeoutCtx, cancel := context.WithTimeout(ctx, timeout) @@ -140,6 +154,16 @@ func patchWithContext(ctx context.Context, ch chan error, image, reportFile, pat } defer bkClient.Close() + mockConfig := Config{ + Labels: map[string]string{"org.opencontainers.image.base.name": "test"}, + } + + imgConfig := ImgConfig{ + Config: mockConfig, + } + + configStr, _ := json.Marshal(imgConfig) + pipeR, pipeW := io.Pipe() dockerConfig := config.LoadDefaultConfigFile(os.Stderr) attachable := []session.Attachable{authprovider.NewDockerAuthProvider(dockerConfig, nil)} @@ -148,7 +172,8 @@ func patchWithContext(ctx context.Context, ch chan error, image, reportFile, pat { Type: client.ExporterDocker, Attrs: map[string]string{ - "name": patchedImageName, + "name": patchedImageName, + "containerimage.config": string(configStr), }, Output: func(_ map[string]string) (io.WriteCloser, error) { return pipeW, nil From 66f345dac933189067c7c887b5980776f748ac6b Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Mon, 24 Jun 2024 09:16:20 -0600 Subject: [PATCH 02/22] Revert "Add mock config" This reverts commit 5e3ee36cf113db853e1d8675ebbaac9186d78034. Signed-off-by: Miaha Cybersec --- pkg/patch/patch.go | 41 ++++++++--------------------------------- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/pkg/patch/patch.go b/pkg/patch/patch.go index 9537de7d..2e9d0721 100644 --- a/pkg/patch/patch.go +++ b/pkg/patch/patch.go @@ -3,7 +3,6 @@ package patch import ( "bytes" "context" - "encoding/json" "errors" "fmt" "io" @@ -12,19 +11,20 @@ import ( "strings" "time" - "github.com/docker/cli/cli/config" - + "github.com/containerd/containerd/platforms" "github.com/docker/buildx/build" - "github.com/moby/buildkit/client" - "github.com/moby/buildkit/session" - "github.com/moby/buildkit/session/auth/authprovider" + "github.com/docker/cli/cli/config" + log "github.com/sirupsen/logrus" + "golang.org/x/exp/slices" "golang.org/x/sync/errgroup" - "github.com/containerd/containerd/platforms" "github.com/distribution/reference" + "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/exporter/containerimage/exptypes" gwclient "github.com/moby/buildkit/frontend/gateway/client" + "github.com/moby/buildkit/session" + "github.com/moby/buildkit/session/auth/authprovider" "github.com/moby/buildkit/util/progress/progressui" "github.com/project-copacetic/copacetic/pkg/buildkit" "github.com/project-copacetic/copacetic/pkg/pkgmgr" @@ -33,8 +33,6 @@ import ( "github.com/project-copacetic/copacetic/pkg/utils" "github.com/project-copacetic/copacetic/pkg/vex" "github.com/quay/claircore/osrelease" - log "github.com/sirupsen/logrus" - "golang.org/x/exp/slices" ) const ( @@ -44,18 +42,6 @@ const ( defaultTag = "latest" ) -type Config struct { - Cmd []string `json:"Cmd"` - WorkingDir string `json:"WorkingDir"` - ExposedPorts map[string]struct{} `json:"ExposedPorts"` - Env []string `json:"Env"` - Labels map[string]string `json:"Labels"` -} - -type ImgConfig struct { - Config Config `json:"Config"` -} - // Patch command applies package updates to an OCI image given a vulnerability report. func Patch(ctx context.Context, timeout time.Duration, image, reportFile, patchedTag, workingFolder, scanner, format, output string, ignoreError bool, bkOpts buildkit.Opts) error { timeoutCtx, cancel := context.WithTimeout(ctx, timeout) @@ -154,16 +140,6 @@ func patchWithContext(ctx context.Context, ch chan error, image, reportFile, pat } defer bkClient.Close() - mockConfig := Config{ - Labels: map[string]string{"org.opencontainers.image.base.name": "test"}, - } - - imgConfig := ImgConfig{ - Config: mockConfig, - } - - configStr, _ := json.Marshal(imgConfig) - pipeR, pipeW := io.Pipe() dockerConfig := config.LoadDefaultConfigFile(os.Stderr) attachable := []session.Attachable{authprovider.NewDockerAuthProvider(dockerConfig, nil)} @@ -172,8 +148,7 @@ func patchWithContext(ctx context.Context, ch chan error, image, reportFile, pat { Type: client.ExporterDocker, Attrs: map[string]string{ - "name": patchedImageName, - "containerimage.config": string(configStr), + "name": patchedImageName, }, Output: func(_ map[string]string) (io.WriteCloser, error) { return pipeW, nil From c7f8c9e7108aba93e9c6f5340792244afcdf92aa Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Mon, 24 Jun 2024 12:34:16 -0600 Subject: [PATCH 03/22] refactor code to unmarshal and modify image metadata Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index 0555d694..cc090fa2 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -3,6 +3,7 @@ package buildkit import ( "bytes" "context" + "encoding/json" "github.com/containerd/containerd/platforms" "github.com/moby/buildkit/client/llb" @@ -44,6 +45,30 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, image stri config.ConfigData = configData + // Unmarshal ConfigData so we can work with the data structure + // The unmarshalled format is stored in userImageMetadata + var userImageMetadata map[string]interface{} + unmarshalErr := json.Unmarshal(config.ConfigData, &userImageMetadata) + if unmarshalErr != nil { + return nil, unmarshalErr + } + + // configMap is set specifically to the data stored within the "config" directive + configMap := userImageMetadata["config"].(map[string]interface{}) + if labels, ok := configMap["labels"]; ok { + // If labels already exists, add BaseImage metadata + labels.(map[string]string)["BaseImage"] = image + } else { + // If labels do not exist, create labels and add BaseImage metadata + labels = make(map[string]string) + configMap["Labels"] = labels + labels.(map[string]string)["BaseImage"] = image + } + + // We need to marshal the data back into a byte array and assign the modified user image metadata into config.ConfigData + configMapBytes, err := json.Marshal(userImageMetadata) + config.ConfigData = configMapBytes + // Load the target image state with the resolved image config in case environment variable settings // are necessary for running apps in the target image for updates config.ImageState, err = llb.Image(image, From b7bd8e8e92cdffe6a3e34536cdea91658851d94c Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Tue, 25 Jun 2024 09:44:43 -0600 Subject: [PATCH 04/22] refactor code for readability and maintainability Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit.go | 66 +++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index cc090fa2..e3bd78d6 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "github.com/containerd/containerd/platforms" "github.com/moby/buildkit/client/llb" @@ -43,32 +44,11 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, image stri return nil, err } - config.ConfigData = configData - - // Unmarshal ConfigData so we can work with the data structure - // The unmarshalled format is stored in userImageMetadata - var userImageMetadata map[string]interface{} - unmarshalErr := json.Unmarshal(config.ConfigData, &userImageMetadata) - if unmarshalErr != nil { - return nil, unmarshalErr - } - - // configMap is set specifically to the data stored within the "config" directive - configMap := userImageMetadata["config"].(map[string]interface{}) - if labels, ok := configMap["labels"]; ok { - // If labels already exists, add BaseImage metadata - labels.(map[string]string)["BaseImage"] = image - } else { - // If labels do not exist, create labels and add BaseImage metadata - labels = make(map[string]string) - configMap["Labels"] = labels - labels.(map[string]string)["BaseImage"] = image + config.ConfigData, err = updateImageMetadata(configData, image) + if err != nil { + return nil, err } - // We need to marshal the data back into a byte array and assign the modified user image metadata into config.ConfigData - configMapBytes, err := json.Marshal(userImageMetadata) - config.ConfigData = configMapBytes - // Load the target image state with the resolved image config in case environment variable settings // are necessary for running apps in the target image for updates config.ImageState, err = llb.Image(image, @@ -84,6 +64,44 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, image stri return &config, nil } +func updateImageMetadata(configData []byte, image string) ([]byte, error) { + var userImageMetadata map[string]interface{} + if err := json.Unmarshal(configData, &userImageMetadata); err != nil { + return nil, err + } + + configMap := userImageMetadata["config"].(map[string]interface{}) + baseImage := setupLabels(configMap, image) + + if baseImage == "" { + configMapBytes, err := json.Marshal(userImageMetadata) + if err != nil { + return nil, fmt.Errorf("failed to marshal image metadata: %v", err) + } + configData = configMapBytes + } + + // This return does not currently return the metadata of the baseImage if one is found + // If baseImage is not empty, we need to resolve the baseImage and return its metadata + return configData, nil +} + +func setupLabels(configMap map[string]interface{}, image string) string { + var baseImage string + labels := configMap["labels"] + if labels == nil { + labels = make(map[string]interface{}) + configMap["labels"] = labels + } + labelsMap := labels.(map[string]interface{}) + if _, ok := labelsMap["BaseImage"]; ok { + baseImage = labelsMap["BaseImage"].(string) + } else { + labelsMap["BaseImage"] = image + } + return baseImage +} + // Extracts the bytes of the file denoted by `path` from the state `st`. func ExtractFileFromState(ctx context.Context, c gwclient.Client, st *llb.State, path string) ([]byte, error) { // since platform is obtained from host, override it in the case of Darwin From da366b0608221db9de34b5932a1a1777dbcbbb09 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Tue, 25 Jun 2024 12:34:02 -0600 Subject: [PATCH 05/22] Refactor updateImageMetadata and setupLabels functions Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit.go | 48 ++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index e3bd78d6..e42f7227 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "fmt" "github.com/containerd/containerd/platforms" "github.com/moby/buildkit/client/llb" @@ -44,7 +43,7 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, image stri return nil, err } - config.ConfigData, err = updateImageMetadata(configData, image) + config.ConfigData, err = updateImageMetadata(ctx, c, configData, image) if err != nil { return nil, err } @@ -64,29 +63,41 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, image stri return &config, nil } -func updateImageMetadata(configData []byte, image string) ([]byte, error) { - var userImageMetadata map[string]interface{} - if err := json.Unmarshal(configData, &userImageMetadata); err != nil { - return nil, err - } - - configMap := userImageMetadata["config"].(map[string]interface{}) - baseImage := setupLabels(configMap, image) +func updateImageMetadata(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, error) { + baseImage, userImageMetadata := setupLabels(configData, image) if baseImage == "" { - configMapBytes, err := json.Marshal(userImageMetadata) + configData = userImageMetadata + } else { + _, _, baseImageConfigData, err := c.ResolveImageConfig(ctx, image, sourceresolver.Opt{ + ImageOpt: &sourceresolver.ResolveImageOpt{ + ResolveMode: llb.ResolveModePreferLocal.String(), + }, + }) if err != nil { - return nil, fmt.Errorf("failed to marshal image metadata: %v", err) + return nil, err } - configData = configMapBytes + // Pass this into setupLabels so that labels can properly be applied to an already patched image + _, baseImageWithLabels := setupLabels(baseImageConfigData, baseImage) + configData = baseImageWithLabels } - // This return does not currently return the metadata of the baseImage if one is found - // If baseImage is not empty, we need to resolve the baseImage and return its metadata return configData, nil } -func setupLabels(configMap map[string]interface{}, image string) string { +// Sets up labels for the image based on the provided configuration data and image name. +// If the labels are already present in the configuration data, it returns the value of the "BaseImage" label. +// Otherwise, it adds the "BaseImage" label with the provided image name and returns the updated configuration data. +// The updated configuration data is returned as a JSON byte slice. +func setupLabels(configData []byte, image string) (string, []byte) { + imageMetadata := make(map[string]interface{}) + err := json.Unmarshal(configData, &imageMetadata) + if err != nil { + return "", nil + } + + configMap := imageMetadata["config"].(map[string]interface{}) + var baseImage string labels := configMap["labels"] if labels == nil { @@ -99,7 +110,10 @@ func setupLabels(configMap map[string]interface{}, image string) string { } else { labelsMap["BaseImage"] = image } - return baseImage + + imageWithLabels, _ := json.Marshal(imageMetadata) + + return baseImage, imageWithLabels } // Extracts the bytes of the file denoted by `path` from the state `st`. From 20e35df732506975e44155e5f6e7d40b04172d61 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Tue, 25 Jun 2024 15:48:53 -0600 Subject: [PATCH 06/22] Add support for patched image data in BuildkitConfig Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit.go | 49 ++++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index e42f7227..91ca1347 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -13,11 +13,13 @@ import ( ) type Config struct { - ImageName string - Client gwclient.Client - ConfigData []byte - Platform *ispec.Platform - ImageState llb.State + ImageName string + Client gwclient.Client + ConfigData []byte + PatchedConfigData []byte + Platform *ispec.Platform + ImageState llb.State + PatchedImageState llb.State } type Opts struct { @@ -27,14 +29,14 @@ type Opts struct { KeyPath string } -func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, image string) (*Config, error) { +func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage string) (*Config, error) { // Initialize buildkit config for the target image config := Config{ - ImageName: image, + ImageName: userImage, } // Resolve and pull the config for the target image - _, _, configData, err := c.ResolveImageConfig(ctx, image, sourceresolver.Opt{ + _, _, configData, err := c.ResolveImageConfig(ctx, userImage, sourceresolver.Opt{ ImageOpt: &sourceresolver.ResolveImageOpt{ ResolveMode: llb.ResolveModePreferLocal.String(), }, @@ -43,14 +45,15 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, image stri return nil, err } - config.ConfigData, err = updateImageMetadata(ctx, c, configData, image) + var baseImage string + config.ConfigData, config.PatchedConfigData, baseImage, err = updateImageMetadata(ctx, c, configData, userImage) if err != nil { return nil, err } // Load the target image state with the resolved image config in case environment variable settings // are necessary for running apps in the target image for updates - config.ImageState, err = llb.Image(image, + config.ImageState, err = llb.Image(baseImage, llb.ResolveModePreferLocal, llb.WithMetaResolver(c), ).WithImageConfig(config.ConfigData) @@ -58,31 +61,47 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, image stri return nil, err } + // Only set PatchedConfigData if the user supplied a patched image + // image in this case should always refer to the patched image (if it exists) + if config.PatchedConfigData != nil { + config.PatchedImageState, err = llb.Image(userImage, + llb.ResolveModePreferLocal, + llb.WithMetaResolver(c), + ).WithImageConfig(config.PatchedConfigData) + if err != nil { + return nil, err + } + } + config.Client = c return &config, nil } -func updateImageMetadata(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, error) { +func updateImageMetadata(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, []byte, string, error) { + var patchedImageMetadata []byte baseImage, userImageMetadata := setupLabels(configData, image) if baseImage == "" { configData = userImageMetadata } else { - _, _, baseImageConfigData, err := c.ResolveImageConfig(ctx, image, sourceresolver.Opt{ + patchedImageMetadata = userImageMetadata + _, _, baseImageMetadata, err := c.ResolveImageConfig(ctx, baseImage, sourceresolver.Opt{ ImageOpt: &sourceresolver.ResolveImageOpt{ ResolveMode: llb.ResolveModePreferLocal.String(), }, }) if err != nil { - return nil, err + return nil, nil, "", err } // Pass this into setupLabels so that labels can properly be applied to an already patched image - _, baseImageWithLabels := setupLabels(baseImageConfigData, baseImage) + _, baseImageWithLabels := setupLabels(baseImageMetadata, baseImage) configData = baseImageWithLabels + + return configData, patchedImageMetadata, baseImage, nil } - return configData, nil + return configData, nil, baseImage, nil } // Sets up labels for the image based on the provided configuration data and image name. From 051ffd5332559f57a07563ba9ad1b26ef427833b Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Wed, 26 Jun 2024 11:41:49 -0600 Subject: [PATCH 07/22] Add patch retention for previously patched images, code refactoring Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit.go | 40 +++++++++++++++++----------------------- pkg/pkgmgr/apk.go | 13 +++++++++++++ pkg/pkgmgr/dpkg.go | 13 +++++++++++++ pkg/pkgmgr/rpm.go | 13 +++++++++++++ 4 files changed, 56 insertions(+), 23 deletions(-) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index 91ca1347..ed68dcdd 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -13,7 +13,6 @@ import ( ) type Config struct { - ImageName string Client gwclient.Client ConfigData []byte PatchedConfigData []byte @@ -31,9 +30,7 @@ type Opts struct { func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage string) (*Config, error) { // Initialize buildkit config for the target image - config := Config{ - ImageName: userImage, - } + config := Config{} // Resolve and pull the config for the target image _, _, configData, err := c.ResolveImageConfig(ctx, userImage, sourceresolver.Opt{ @@ -61,8 +58,9 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage return nil, err } - // Only set PatchedConfigData if the user supplied a patched image - // image in this case should always refer to the patched image (if it exists) + // Only set PatchedImageState if the user supplied a patched image + // An image is deemed to be a patched image if it contains one of two metadata values + // BaseImage or ispec.AnnotationBaseImageName if config.PatchedConfigData != nil { config.PatchedImageState, err = llb.Image(userImage, llb.ResolveModePreferLocal, @@ -79,14 +77,14 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage } func updateImageMetadata(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, []byte, string, error) { - var patchedImageMetadata []byte - baseImage, userImageMetadata := setupLabels(configData, image) + var patchedImageConfig []byte + baseImage, userImageConfig := setupLabels(image, configData) if baseImage == "" { - configData = userImageMetadata + configData = userImageConfig } else { - patchedImageMetadata = userImageMetadata - _, _, baseImageMetadata, err := c.ResolveImageConfig(ctx, baseImage, sourceresolver.Opt{ + patchedImageConfig = userImageConfig + _, _, baseImageConfig, err := c.ResolveImageConfig(ctx, baseImage, sourceresolver.Opt{ ImageOpt: &sourceresolver.ResolveImageOpt{ ResolveMode: llb.ResolveModePreferLocal.String(), }, @@ -95,27 +93,23 @@ func updateImageMetadata(ctx context.Context, c gwclient.Client, configData []by return nil, nil, "", err } // Pass this into setupLabels so that labels can properly be applied to an already patched image - _, baseImageWithLabels := setupLabels(baseImageMetadata, baseImage) + _, baseImageWithLabels := setupLabels(baseImage, baseImageConfig) configData = baseImageWithLabels - return configData, patchedImageMetadata, baseImage, nil + return configData, patchedImageConfig, baseImage, nil } - return configData, nil, baseImage, nil + return configData, nil, image, nil } -// Sets up labels for the image based on the provided configuration data and image name. -// If the labels are already present in the configuration data, it returns the value of the "BaseImage" label. -// Otherwise, it adds the "BaseImage" label with the provided image name and returns the updated configuration data. -// The updated configuration data is returned as a JSON byte slice. -func setupLabels(configData []byte, image string) (string, []byte) { - imageMetadata := make(map[string]interface{}) - err := json.Unmarshal(configData, &imageMetadata) +func setupLabels(image string, configData []byte) (string, []byte) { + imageConfig := make(map[string]interface{}) + err := json.Unmarshal(configData, &imageConfig) if err != nil { return "", nil } - configMap := imageMetadata["config"].(map[string]interface{}) + configMap := imageConfig["config"].(map[string]interface{}) var baseImage string labels := configMap["labels"] @@ -130,7 +124,7 @@ func setupLabels(configData []byte, image string) (string, []byte) { labelsMap["BaseImage"] = image } - imageWithLabels, _ := json.Marshal(imageMetadata) + imageWithLabels, _ := json.Marshal(imageConfig) return baseImage, imageWithLabels } diff --git a/pkg/pkgmgr/apk.go b/pkg/pkgmgr/apk.go index b53f94d9..207fbe43 100644 --- a/pkg/pkgmgr/apk.go +++ b/pkg/pkgmgr/apk.go @@ -207,6 +207,19 @@ func (am *apkManager) upgradePackages(ctx context.Context, updates unversioned.U // Diff the installed updates and merge that into the target image patchDiff := llb.Diff(apkUpdated, apkInstalled) patchMerge := llb.Merge([]llb.State{am.config.ImageState, patchDiff}) + + // If the image has been patched before, diff the base image and patched image to retain previous patches + var prevPatchMerge, combinedPatchMerge llb.State + if am.config.PatchedConfigData != nil { + prevPatchDiff := llb.Diff(am.config.ImageState, am.config.PatchedImageState) + prevPatchMerge = llb.Merge([]llb.State{am.config.PatchedImageState, prevPatchDiff}) + + // Merge the old patch diffs and new patch diffs + combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, patchMerge}) + + return &combinedPatchMerge, resultManifestBytes, nil + } + return &patchMerge, resultManifestBytes, nil } diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index 2784c91e..7de9537e 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -349,6 +349,19 @@ func (dm *dpkgManager) installUpdates(ctx context.Context, updates unversioned.U // Diff the installed updates and merge that into the target image patchDiff := llb.Diff(aptUpdated, aptInstalled) patchMerge := llb.Merge([]llb.State{dm.config.ImageState, patchDiff}) + + // If the image has been patched before, diff the base image and patched image to retain previous patches + var prevPatchMerge, combinedPatchMerge llb.State + if dm.config.PatchedConfigData != nil { + prevPatchDiff := llb.Diff(dm.config.ImageState, dm.config.PatchedImageState) + prevPatchMerge = llb.Merge([]llb.State{dm.config.PatchedImageState, prevPatchDiff}) + + // Merge the old patch diffs and new patch diffs + combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, patchMerge}) + + return &combinedPatchMerge, resultsBytes, nil + } + return &patchMerge, resultsBytes, nil } diff --git a/pkg/pkgmgr/rpm.go b/pkg/pkgmgr/rpm.go index 7a18cab8..5c417441 100644 --- a/pkg/pkgmgr/rpm.go +++ b/pkg/pkgmgr/rpm.go @@ -431,6 +431,19 @@ func (rm *rpmManager) installUpdates(ctx context.Context, updates unversioned.Up // Diff the installed updates and merge that into the target image patchDiff := llb.Diff(rm.config.ImageState, installed) patchMerge := llb.Merge([]llb.State{rm.config.ImageState, patchDiff}) + + // If the image has been patched before, diff the base image and patched image to retain previous patches + var prevPatchMerge, combinedPatchMerge llb.State + if rm.config.PatchedConfigData != nil { + prevPatchDiff := llb.Diff(rm.config.ImageState, rm.config.PatchedImageState) + prevPatchMerge = llb.Merge([]llb.State{rm.config.PatchedImageState, prevPatchDiff}) + + // Merge the old patch diffs and new patch diffs + combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, patchMerge}) + + return &combinedPatchMerge, resultBytes, nil + } + return &patchMerge, resultBytes, nil } From c6b6c318b2dd3144db2650e701aa1669cac2c0ab Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Wed, 26 Jun 2024 15:08:43 -0600 Subject: [PATCH 08/22] Add patch retention for previously patched images, add tests Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit_test.go | 31 +++++++++++++++++++++++++++++++ pkg/pkgmgr/dpkg.go | 13 +++++++++++++ pkg/pkgmgr/rpm.go | 13 +++++++++++++ 3 files changed, 57 insertions(+) diff --git a/pkg/buildkit/buildkit_test.go b/pkg/buildkit/buildkit_test.go index 8f9279d6..f7b601b8 100644 --- a/pkg/buildkit/buildkit_test.go +++ b/pkg/buildkit/buildkit_test.go @@ -2,6 +2,7 @@ package buildkit import ( "context" + "encoding/json" "errors" "net" "path/filepath" @@ -252,3 +253,33 @@ func TestArrayFile(t *testing.T) { }) } } + +func TestSetupLabels(t *testing.T) { + image := "test_image" + configData := []byte(`{"config": {"labels": {}}}`) + + baseImage, updatedConfigData := setupLabels(image, configData) + assert.Empty(t, baseImage) + + var updatedConfig map[string]interface{} + err := json.Unmarshal(updatedConfigData, &updatedConfig) + assert.NoError(t, err) + + labels := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) + assert.Equal(t, image, labels["BaseImage"]) +} + +func TestSetupLabelsWithBaseImage(t *testing.T) { + image := "test_image" + configData := []byte(`{"config": {"labels": {"BaseImage": "existing_base_image"}}}`) + + baseImage, updatedConfigData := setupLabels(image, configData) + assert.Equal(t, "existing_base_image", baseImage) + + var updatedConfig map[string]interface{} + err := json.Unmarshal(updatedConfigData, &updatedConfig) + assert.NoError(t, err) + + labels := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) + assert.Equal(t, "existing_base_image", labels["BaseImage"]) +} diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index 7de9537e..736e376e 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -490,6 +490,19 @@ func (dm *dpkgManager) unpackAndMergeUpdates(ctx context.Context, updates unvers // Diff unpacked packages layers from previous and merge with target statusDiff := llb.Diff(fieldsWritten, statusUpdated) merged := llb.Merge([]llb.State{dm.config.ImageState, unpackedToRoot, statusDiff}) + + // If the image has been patched before, diff the base image and patched image to retain previous patches + var prevPatchMerge, combinedPatchMerge llb.State + if dm.config.PatchedConfigData != nil { + prevPatchDiff := llb.Diff(dm.config.ImageState, dm.config.PatchedImageState) + prevPatchMerge = llb.Merge([]llb.State{dm.config.PatchedImageState, prevPatchDiff}) + + // Merge the old patch diffs and new patch diffs + combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, merged}) + + return &combinedPatchMerge, resultsBytes, nil + } + return &merged, resultsBytes, nil } diff --git a/pkg/pkgmgr/rpm.go b/pkg/pkgmgr/rpm.go index 5c417441..8a63895d 100644 --- a/pkg/pkgmgr/rpm.go +++ b/pkg/pkgmgr/rpm.go @@ -560,6 +560,19 @@ func (rm *rpmManager) unpackAndMergeUpdates(ctx context.Context, updates unversi // Diff unpacked packages layers from previous and merge with target manifestsDiff := llb.Diff(manifestsUpdated, manifestsPlaced) merged := llb.Merge([]llb.State{rm.config.ImageState, patchedRoot, manifestsDiff}) + + // If the image has been patched before, diff the base image and patched image to retain previous patches + var prevPatchMerge, combinedPatchMerge llb.State + if rm.config.PatchedConfigData != nil { + prevPatchDiff := llb.Diff(rm.config.ImageState, rm.config.PatchedImageState) + prevPatchMerge = llb.Merge([]llb.State{rm.config.PatchedImageState, prevPatchDiff}) + + // Merge the old patch diffs and new patch diffs + combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, merged}) + + return &combinedPatchMerge, resultBytes, nil + } + return &merged, resultBytes, nil } From 3dbb9c0a390b73481617b6c2553bc223361211f4 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Wed, 26 Jun 2024 16:57:12 -0600 Subject: [PATCH 09/22] Refactor tests and rename function in buildkit package Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit.go | 10 +++--- pkg/buildkit/buildkit_test.go | 64 +++++++++++++++++++++-------------- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index ed68dcdd..cae5fa2f 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -43,7 +43,7 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage } var baseImage string - config.ConfigData, config.PatchedConfigData, baseImage, err = updateImageMetadata(ctx, c, configData, userImage) + config.ConfigData, config.PatchedConfigData, baseImage, err = updateImageConfigData(ctx, c, configData, userImage) if err != nil { return nil, err } @@ -76,7 +76,7 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage return &config, nil } -func updateImageMetadata(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, []byte, string, error) { +func updateImageConfigData(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, []byte, string, error) { var patchedImageConfig []byte baseImage, userImageConfig := setupLabels(image, configData) @@ -92,7 +92,7 @@ func updateImageMetadata(ctx context.Context, c gwclient.Client, configData []by if err != nil { return nil, nil, "", err } - // Pass this into setupLabels so that labels can properly be applied to an already patched image + _, baseImageWithLabels := setupLabels(baseImage, baseImageConfig) configData = baseImageWithLabels @@ -118,8 +118,8 @@ func setupLabels(image string, configData []byte) (string, []byte) { configMap["labels"] = labels } labelsMap := labels.(map[string]interface{}) - if _, ok := labelsMap["BaseImage"]; ok { - baseImage = labelsMap["BaseImage"].(string) + if baseImageValue, ok := labelsMap["BaseImage"]; ok { + baseImage = baseImageValue.(string) } else { labelsMap["BaseImage"] = image } diff --git a/pkg/buildkit/buildkit_test.go b/pkg/buildkit/buildkit_test.go index f7b601b8..98631a15 100644 --- a/pkg/buildkit/buildkit_test.go +++ b/pkg/buildkit/buildkit_test.go @@ -255,31 +255,43 @@ func TestArrayFile(t *testing.T) { } func TestSetupLabels(t *testing.T) { - image := "test_image" - configData := []byte(`{"config": {"labels": {}}}`) - - baseImage, updatedConfigData := setupLabels(image, configData) - assert.Empty(t, baseImage) - - var updatedConfig map[string]interface{} - err := json.Unmarshal(updatedConfigData, &updatedConfig) - assert.NoError(t, err) - - labels := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) - assert.Equal(t, image, labels["BaseImage"]) -} - -func TestSetupLabelsWithBaseImage(t *testing.T) { - image := "test_image" - configData := []byte(`{"config": {"labels": {"BaseImage": "existing_base_image"}}}`) - - baseImage, updatedConfigData := setupLabels(image, configData) - assert.Equal(t, "existing_base_image", baseImage) - - var updatedConfig map[string]interface{} - err := json.Unmarshal(updatedConfigData, &updatedConfig) - assert.NoError(t, err) + tests := []struct { + testName string + configData []byte + expectBaseImg string + expectImage string + }{ + { + "No labels", + []byte(`{"config": {}}`), + "", + "test_image", + }, + { + "Labels no base", + []byte(`{"config": {"labels": {}}}`), + "", + "test_image", + }, + { + "Labels with base image", + []byte(`{"config": {"labels": {"BaseImage": "existing_base_image"}}}`), + "existing_base_image", + "existing_base_image", + }, + } + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + image := "test_image" + baseImage, updatedConfigData := setupLabels(image, test.configData) + assert.Equal(t, test.expectBaseImg, baseImage) + + var updatedConfig map[string]interface{} + err := json.Unmarshal(updatedConfigData, &updatedConfig) + assert.NoError(t, err) - labels := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) - assert.Equal(t, "existing_base_image", labels["BaseImage"]) + labels := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) + assert.Equal(t, test.expectImage, labels["BaseImage"]) + }) + } } From 8f1d79b0f644f09778c5229eb29586781473cf3e Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Thu, 27 Jun 2024 08:24:34 -0600 Subject: [PATCH 10/22] Expand test coverage Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit_test.go | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/pkg/buildkit/buildkit_test.go b/pkg/buildkit/buildkit_test.go index 98631a15..72f22f5a 100644 --- a/pkg/buildkit/buildkit_test.go +++ b/pkg/buildkit/buildkit_test.go @@ -260,38 +260,55 @@ func TestSetupLabels(t *testing.T) { configData []byte expectBaseImg string expectImage string + expectError bool }{ { "No labels", []byte(`{"config": {}}`), "", "test_image", + false, }, { "Labels no base", []byte(`{"config": {"labels": {}}}`), "", "test_image", + false, }, { "Labels with base image", []byte(`{"config": {"labels": {"BaseImage": "existing_base_image"}}}`), "existing_base_image", "existing_base_image", + false, + }, + { + "Invalid JSON", + []byte(`{"config": {"labels": {"BaseImage": "existing_base_image"}`), + "", + "", + true, }, } for _, test := range tests { t.Run(test.testName, func(t *testing.T) { image := "test_image" baseImage, updatedConfigData := setupLabels(image, test.configData) - assert.Equal(t, test.expectBaseImg, baseImage) - var updatedConfig map[string]interface{} - err := json.Unmarshal(updatedConfigData, &updatedConfig) - assert.NoError(t, err) + if test.expectError { + assert.Equal(t, "", baseImage) + assert.Nil(t, updatedConfigData) + } else { + assert.Equal(t, test.expectBaseImg, baseImage) + + var updatedConfig map[string]interface{} + err := json.Unmarshal(updatedConfigData, &updatedConfig) + assert.NoError(t, err) - labels := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) - assert.Equal(t, test.expectImage, labels["BaseImage"]) + labels := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) + assert.Equal(t, test.expectImage, labels["BaseImage"]) + } }) } } From 27ca23e1b6b4b2db7724f670153ab3f507d90b48 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Fri, 28 Jun 2024 15:24:31 -0600 Subject: [PATCH 11/22] Minor refactor to discard patch layers in all package manager Signed-off-by: Miaha Cybersec --- pkg/pkgmgr/apk.go | 23 ++++++++++++++++------- pkg/pkgmgr/dpkg.go | 46 ++++++++++++++++++++++++++++++++-------------- pkg/pkgmgr/rpm.go | 46 ++++++++++++++++++++++++++++++++-------------- 3 files changed, 80 insertions(+), 35 deletions(-) diff --git a/pkg/pkgmgr/apk.go b/pkg/pkgmgr/apk.go index 7a908ce1..cc4fd7f0 100644 --- a/pkg/pkgmgr/apk.go +++ b/pkg/pkgmgr/apk.go @@ -209,22 +209,31 @@ func (am *apkManager) upgradePackages(ctx context.Context, updates unversioned.U } } - // Diff the installed updates and merge that into the target image - patchDiff := llb.Diff(apkUpdated, apkInstalled) - patchMerge := llb.Merge([]llb.State{am.config.ImageState, patchDiff}) - // If the image has been patched before, diff the base image and patched image to retain previous patches - var prevPatchMerge, combinedPatchMerge llb.State if am.config.PatchedConfigData != nil { + // Diff the base image and new patches + patchDiff := llb.Diff(am.config.ImageState, apkInstalled) + + // Diff the base image and patched image to get previous patches prevPatchDiff := llb.Diff(am.config.ImageState, am.config.PatchedImageState) - prevPatchMerge = llb.Merge([]llb.State{am.config.PatchedImageState, prevPatchDiff}) // Merge the old patch diffs and new patch diffs - combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, patchMerge}) + prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, patchDiff}) + + // This additional diff ensures previous patch layers are discarded + // While ensuring all previous and new patches are properly applied to the base image + combinedPatch := llb.Diff(am.config.ImageState, prevAndNewPatch) + + // Merge previous and new patches into the base image + combinedPatchMerge := llb.Merge([]llb.State{am.config.ImageState, combinedPatch}) return &combinedPatchMerge, resultManifestBytes, nil } + // Diff the installed updates and merge that into the target image + patchDiff := llb.Diff(am.config.ImageState, apkInstalled) + patchMerge := llb.Merge([]llb.State{am.config.ImageState, patchDiff}) + return &patchMerge, resultManifestBytes, nil } diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index c020f66a..afd09808 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -349,22 +349,31 @@ func (dm *dpkgManager) installUpdates(ctx context.Context, updates unversioned.U return nil, nil, err } - // Diff the installed updates and merge that into the target image - patchDiff := llb.Diff(aptUpdated, aptInstalled) - patchMerge := llb.Merge([]llb.State{dm.config.ImageState, patchDiff}) - // If the image has been patched before, diff the base image and patched image to retain previous patches - var prevPatchMerge, combinedPatchMerge llb.State if dm.config.PatchedConfigData != nil { + // Diff the base image and new patches + patchDiff := llb.Diff(dm.config.ImageState, aptInstalled) + + // Diff the base image and patched image to get previous patches prevPatchDiff := llb.Diff(dm.config.ImageState, dm.config.PatchedImageState) - prevPatchMerge = llb.Merge([]llb.State{dm.config.PatchedImageState, prevPatchDiff}) // Merge the old patch diffs and new patch diffs - combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, patchMerge}) + prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, patchDiff}) + + // This additional diff ensures previous patch layers are discarded + // While ensuring all previous and new patches are properly applied to the base image + combinedPatch := llb.Diff(dm.config.ImageState, prevAndNewPatch) + + // Merge previous and new patches into the base image + combinedPatchMerge := llb.Merge([]llb.State{dm.config.ImageState, combinedPatch}) return &combinedPatchMerge, resultsBytes, nil } + // Diff the installed updates and merge that into the target image + patchDiff := llb.Diff(dm.config.ImageState, aptInstalled) + patchMerge := llb.Merge([]llb.State{dm.config.ImageState, patchDiff}) + return &patchMerge, resultsBytes, nil } @@ -504,22 +513,31 @@ func (dm *dpkgManager) unpackAndMergeUpdates(ctx context.Context, updates unvers copyStatusCmd := fmt.Sprintf(strings.ReplaceAll(copyStatusTemplate, "\n", ""), dpkgStatusFolder, dm.statusdNames) statusUpdated := fieldsWritten.Dir(resultsPath).Run(llb.Shlex(copyStatusCmd)).Root() - // Diff unpacked packages layers from previous and merge with target - statusDiff := llb.Diff(fieldsWritten, statusUpdated) - merged := llb.Merge([]llb.State{dm.config.ImageState, unpackedToRoot, statusDiff}) - // If the image has been patched before, diff the base image and patched image to retain previous patches - var prevPatchMerge, combinedPatchMerge llb.State if dm.config.PatchedConfigData != nil { + // Diff the base image and patched image to get previous patches prevPatchDiff := llb.Diff(dm.config.ImageState, dm.config.PatchedImageState) - prevPatchMerge = llb.Merge([]llb.State{dm.config.PatchedImageState, prevPatchDiff}) + + // Diff the manifests for the latest updates + manifestsDiff := llb.Diff(fieldsWritten, statusUpdated) // Merge the old patch diffs and new patch diffs - combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, merged}) + prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, manifestsDiff, unpackedToRoot}) + + // This additional diff ensures previous patch layers are discarded + // While ensuring all previous and new patches are properly applied to the base image + combinedPatch := llb.Diff(dm.config.ImageState, prevAndNewPatch) + + // Merge previous and new patches into the base image + combinedPatchMerge := llb.Merge([]llb.State{dm.config.ImageState, combinedPatch}) return &combinedPatchMerge, resultsBytes, nil } + // Diff unpacked packages layers from previous and merge with target + statusDiff := llb.Diff(fieldsWritten, statusUpdated) + merged := llb.Merge([]llb.State{dm.config.ImageState, unpackedToRoot, statusDiff}) + return &merged, resultsBytes, nil } diff --git a/pkg/pkgmgr/rpm.go b/pkg/pkgmgr/rpm.go index cfec5ab3..a8ffa7ee 100644 --- a/pkg/pkgmgr/rpm.go +++ b/pkg/pkgmgr/rpm.go @@ -433,22 +433,31 @@ func (rm *rpmManager) installUpdates(ctx context.Context, updates unversioned.Up } } - // Diff the installed updates and merge that into the target image - patchDiff := llb.Diff(rm.config.ImageState, installed) - patchMerge := llb.Merge([]llb.State{rm.config.ImageState, patchDiff}) - // If the image has been patched before, diff the base image and patched image to retain previous patches - var prevPatchMerge, combinedPatchMerge llb.State if rm.config.PatchedConfigData != nil { + // Diff the base image and new patches + patchDiff := llb.Diff(rm.config.ImageState, installed) + + // Diff the base image and patched image to get previous patches prevPatchDiff := llb.Diff(rm.config.ImageState, rm.config.PatchedImageState) - prevPatchMerge = llb.Merge([]llb.State{rm.config.PatchedImageState, prevPatchDiff}) // Merge the old patch diffs and new patch diffs - combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, patchMerge}) + prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, patchDiff}) + + // This additional diff ensures previous patch layers are discarded + // While ensuring all previous and new patches are properly applied to the base image + combinedPatch := llb.Diff(rm.config.ImageState, prevAndNewPatch) + + // Merge previous and new patches into the base image + combinedPatchMerge := llb.Merge([]llb.State{rm.config.ImageState, combinedPatch}) return &combinedPatchMerge, resultBytes, nil } + // Diff the installed updates and merge that into the target image + patchDiff := llb.Diff(rm.config.ImageState, installed) + patchMerge := llb.Merge([]llb.State{rm.config.ImageState, patchDiff}) + return &patchMerge, resultBytes, nil } @@ -575,22 +584,31 @@ func (rm *rpmManager) unpackAndMergeUpdates(ctx context.Context, updates unversi return nil, nil, err } - // Diff unpacked packages layers from previous and merge with target - manifestsDiff := llb.Diff(manifestsUpdated, manifestsPlaced) - merged := llb.Merge([]llb.State{rm.config.ImageState, patchedRoot, manifestsDiff}) - // If the image has been patched before, diff the base image and patched image to retain previous patches - var prevPatchMerge, combinedPatchMerge llb.State if rm.config.PatchedConfigData != nil { + // Diff the base image and patched image to get previous patches prevPatchDiff := llb.Diff(rm.config.ImageState, rm.config.PatchedImageState) - prevPatchMerge = llb.Merge([]llb.State{rm.config.PatchedImageState, prevPatchDiff}) + + // Diff the manifests for the latest updates + manifestsDiff := llb.Diff(manifestsUpdated, manifestsPlaced) // Merge the old patch diffs and new patch diffs - combinedPatchMerge = llb.Merge([]llb.State{prevPatchMerge, merged}) + prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, manifestsDiff, patchedRoot}) + + // This additional diff ensures previous patch layers are discarded + // While ensuring all previous and new patches are properly applied to the base image + combinedPatch := llb.Diff(rm.config.ImageState, prevAndNewPatch) + + // Merge previous and new patches into the base image + combinedPatchMerge := llb.Merge([]llb.State{rm.config.ImageState, combinedPatch}) return &combinedPatchMerge, resultBytes, nil } + // Diff unpacked packages layers from previous and merge with target + manifestsDiff := llb.Diff(manifestsUpdated, manifestsPlaced) + merged := llb.Merge([]llb.State{rm.config.ImageState, patchedRoot, manifestsDiff}) + return &merged, resultBytes, nil } From 218cf8a59293ce656d5f7c12434c36b21ea9aa39 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Tue, 2 Jul 2024 13:20:26 -0600 Subject: [PATCH 12/22] Add mock client tests and expand test coverage Signed-off-by: Miaha Cybersec --- go.mod | 1 + go.sum | 1 + mocks/mock_gwclient.go | 84 +++++++++++++++++++++++++++++++++++ pkg/buildkit/buildkit.go | 3 +- pkg/buildkit/buildkit_test.go | 59 ++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 mocks/mock_gwclient.go diff --git a/go.mod b/go.mod index 738e5869..dd67593b 100644 --- a/go.mod +++ b/go.mod @@ -130,6 +130,7 @@ require ( github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.6.0 // indirect github.com/spf13/pflag v1.0.5 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/theupdateframework/notary v0.7.0 // indirect github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c // indirect diff --git a/go.sum b/go.sum index ef17093b..948933f0 100644 --- a/go.sum +++ b/go.sum @@ -443,6 +443,7 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= diff --git a/mocks/mock_gwclient.go b/mocks/mock_gwclient.go new file mode 100644 index 00000000..72b4a0fb --- /dev/null +++ b/mocks/mock_gwclient.go @@ -0,0 +1,84 @@ +package mocks + +import ( + "context" + + "github.com/tonistiigi/fsutil/types" + + "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/client/llb/sourceresolver" + gwclient "github.com/moby/buildkit/frontend/gateway/client" + "github.com/moby/buildkit/solver/pb" + "github.com/opencontainers/go-digest" + "github.com/stretchr/testify/mock" +) + +// Mock for gwclient.Client +type MockGWClient struct { + mock.Mock +} + +func (m *MockGWClient) ResolveSourceMetadata(ctx context.Context, op *pb.SourceOp, opt sourceresolver.Opt) (*sourceresolver.MetaResponse, error) { + args := m.Called(ctx, op, opt) + return args.Get(0).(*sourceresolver.MetaResponse), args.Error(1) +} + +func (m *MockGWClient) Solve(ctx context.Context, req gwclient.SolveRequest) (*gwclient.Result, error) { + args := m.Called(ctx, req) + return args.Get(0).(*gwclient.Result), args.Error(1) +} + +func (m *MockGWClient) ResolveImageConfig(ctx context.Context, ref string, opt sourceresolver.Opt) (string, digest.Digest, []byte, error) { + args := m.Called(ctx, ref, opt) + return args.String(0), args.Get(1).(digest.Digest), args.Get(2).([]byte), args.Error(3) +} + +func (m *MockGWClient) BuildOpts() gwclient.BuildOpts { + args := m.Called() + return args.Get(0).(gwclient.BuildOpts) +} + +func (m *MockGWClient) Inputs(ctx context.Context) (map[string]llb.State, error) { + args := m.Called(ctx) + return args.Get(0).(map[string]llb.State), args.Error(1) +} + +func (m *MockGWClient) NewContainer(ctx context.Context, req gwclient.NewContainerRequest) (gwclient.Container, error) { + args := m.Called(ctx, req) + return args.Get(0).(gwclient.Container), args.Error(1) +} + +func (m *MockGWClient) Warn(ctx context.Context, dgst digest.Digest, msg string, opts gwclient.WarnOpts) error { + args := m.Called(ctx, dgst, msg, opts) + return args.Get(0).(error) +} + +// MockReference is a mock of the Reference interface +type MockReference struct { + mock.Mock +} + +func (m *MockReference) ReadFile(ctx context.Context, req gwclient.ReadRequest) ([]byte, error) { + args := m.Called(ctx, req) + return args.Get(0).([]byte), args.Error(1) +} + +func (m *MockReference) ToState() (llb.State, error) { + args := m.Called() + return args.Get(0).(llb.State), args.Error(1) +} + +func (m *MockReference) Evaluate(ctx context.Context) error { + args := m.Called(ctx) + return args.Get(0).(error) +} + +func (m *MockReference) StatFile(ctx context.Context, req gwclient.StatRequest) (*types.Stat, error) { + args := m.Called(ctx, req) + return args.Get(0).(*types.Stat), args.Error(1) +} + +func (m *MockReference) ReadDir(ctx context.Context, req gwclient.ReadDirRequest) ([]*types.Stat, error) { + args := m.Called(ctx, req) + return args.Get(0).([]*types.Stat), args.Error(1) +} diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index 8f72d55f..79567963 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -77,13 +77,12 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage } func updateImageConfigData(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, []byte, string, error) { - var patchedImageConfig []byte baseImage, userImageConfig := setupLabels(image, configData) if baseImage == "" { configData = userImageConfig } else { - patchedImageConfig = userImageConfig + patchedImageConfig := userImageConfig _, _, baseImageConfig, err := c.ResolveImageConfig(ctx, baseImage, sourceresolver.Opt{ ImageOpt: &sourceresolver.ResolveImageOpt{ ResolveMode: llb.ResolveModePreferLocal.String(), diff --git a/pkg/buildkit/buildkit_test.go b/pkg/buildkit/buildkit_test.go index 72f22f5a..58c67339 100644 --- a/pkg/buildkit/buildkit_test.go +++ b/pkg/buildkit/buildkit_test.go @@ -6,9 +6,16 @@ import ( "errors" "net" "path/filepath" + "reflect" "testing" "time" + "github.com/opencontainers/go-digest" + + "github.com/project-copacetic/copacetic/mocks" + + "github.com/stretchr/testify/mock" + controlapi "github.com/moby/buildkit/api/services/control" types "github.com/moby/buildkit/api/types" gateway "github.com/moby/buildkit/frontend/gateway/pb" @@ -312,3 +319,55 @@ func TestSetupLabels(t *testing.T) { }) } } + +func TestUpdateImageConfigData(t *testing.T) { + ctx := context.Background() + + t.Run("No base image", func(t *testing.T) { + mockClient := &mocks.MockGWClient{} + configData := []byte(`{"config": {"labels": {"com.example.label": "value"}}}`) + expectedData := []byte(`{"config": {"labels": {"com.example.label": "value"}, {"BaseImage": "myimage:latest"}}}`) + image := "myimage:latest" + + resultConfig, resultPatched, resultImage, err := updateImageConfigData(ctx, mockClient, configData, image) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if reflect.DeepEqual(expectedData, configData) { + t.Errorf("Expected config data to be %s, got %s", configData, resultConfig) + } + + if resultPatched != nil { + t.Errorf("Expected patched config to be nil, got %s", resultPatched) + } + + if resultImage != image { + t.Errorf("Expected image to be %s, got %s", image, resultImage) + } + }) + + t.Run("With base image", func(t *testing.T) { + mockClient := &mocks.MockGWClient{} + mockClient.On("ResolveImageConfig", + mock.Anything, mock.AnythingOfType("string"), mock.Anything). + Return("imageConfigString", digest.Digest("digest"), []byte(`{"config": {"labels": {"BaseImage": "rockylinux:latest"}}}`), nil) + + configData := []byte(`{"config": {"labels": {"BaseImage": "rockylinux:latest"}}}`) + image := "rockylinux:latest" + + resultConfig, _, resultImage, err := updateImageConfigData(ctx, mockClient, configData, image) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + expectedConfig := []byte(`{"config":{"labels":{"BaseImage":"rockylinux:latest"}}}`) + if !reflect.DeepEqual(resultConfig, expectedConfig) { + t.Errorf("Expected config data to be %s, got %s", expectedConfig, resultConfig) + } + + if resultImage != "rockylinux:latest" { + t.Errorf("Expected image to be baseimage:latest, got %s", resultImage) + } + }) +} From 49d6385d08d48b171b2869afc5d1fab261283fed Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Wed, 3 Jul 2024 16:13:36 -0600 Subject: [PATCH 13/22] Add error handling for type assertions Signed-off-by: Miaha Cybersec --- mocks/mock_gwclient.go | 106 +++++++++++++++++++++++++++++----- pkg/buildkit/buildkit.go | 31 +++++++--- pkg/buildkit/buildkit_test.go | 8 ++- 3 files changed, 120 insertions(+), 25 deletions(-) diff --git a/mocks/mock_gwclient.go b/mocks/mock_gwclient.go index 72b4a0fb..38ceb32a 100644 --- a/mocks/mock_gwclient.go +++ b/mocks/mock_gwclient.go @@ -2,6 +2,7 @@ package mocks import ( "context" + "fmt" "github.com/tonistiigi/fsutil/types" @@ -13,72 +14,149 @@ import ( "github.com/stretchr/testify/mock" ) -// Mock for gwclient.Client +// Mock for gwclient.Client. type MockGWClient struct { mock.Mock } func (m *MockGWClient) ResolveSourceMetadata(ctx context.Context, op *pb.SourceOp, opt sourceresolver.Opt) (*sourceresolver.MetaResponse, error) { args := m.Called(ctx, op, opt) - return args.Get(0).(*sourceresolver.MetaResponse), args.Error(1) + + metaResponse, ok := args.Get(0).(*sourceresolver.MetaResponse) + if !ok { + return nil, fmt.Errorf("type assertion to *sourceresolver.MetaResponse failed") + } + + return metaResponse, args.Error(1) } func (m *MockGWClient) Solve(ctx context.Context, req gwclient.SolveRequest) (*gwclient.Result, error) { args := m.Called(ctx, req) - return args.Get(0).(*gwclient.Result), args.Error(1) + + result, ok := args.Get(0).(*gwclient.Result) + if !ok { + return nil, fmt.Errorf("type assertion to *gwclient.Result failed") + } + + return result, args.Error(1) } func (m *MockGWClient) ResolveImageConfig(ctx context.Context, ref string, opt sourceresolver.Opt) (string, digest.Digest, []byte, error) { args := m.Called(ctx, ref, opt) - return args.String(0), args.Get(1).(digest.Digest), args.Get(2).([]byte), args.Error(3) + + digestResult, ok1 := args.Get(1).(digest.Digest) + if !ok1 { + return "", digest.Digest(""), nil, fmt.Errorf("type assertion to digest.Digest failed") + } + + byteResult, ok2 := args.Get(2).([]byte) + if !ok2 { + return "", digest.Digest(""), nil, fmt.Errorf("type assertion to []byte failed") + } + + return args.String(0), digestResult, byteResult, args.Error(3) } func (m *MockGWClient) BuildOpts() gwclient.BuildOpts { args := m.Called() - return args.Get(0).(gwclient.BuildOpts) + + buildOpts, ok := args.Get(0).(gwclient.BuildOpts) + if !ok { + panic("type assertion to gwclient.BuildOpts failed") + } + + return buildOpts } func (m *MockGWClient) Inputs(ctx context.Context) (map[string]llb.State, error) { args := m.Called(ctx) - return args.Get(0).(map[string]llb.State), args.Error(1) + + stateMap, ok := args.Get(0).(map[string]llb.State) + if !ok { + return nil, fmt.Errorf("type assertion to map[string]llb.State failed") + } + + return stateMap, args.Error(1) } func (m *MockGWClient) NewContainer(ctx context.Context, req gwclient.NewContainerRequest) (gwclient.Container, error) { args := m.Called(ctx, req) - return args.Get(0).(gwclient.Container), args.Error(1) + + container, ok := args.Get(0).(gwclient.Container) + if !ok { + return nil, fmt.Errorf("type assertion to gwclient.Container failed") + } + + return container, args.Error(1) } func (m *MockGWClient) Warn(ctx context.Context, dgst digest.Digest, msg string, opts gwclient.WarnOpts) error { args := m.Called(ctx, dgst, msg, opts) - return args.Get(0).(error) + + warnErr, ok := args.Get(0).(error) + if !ok { + return fmt.Errorf("type assertion to error failed") + } + + return warnErr } -// MockReference is a mock of the Reference interface +// MockReference is a mock of the Reference interface. type MockReference struct { mock.Mock } func (m *MockReference) ReadFile(ctx context.Context, req gwclient.ReadRequest) ([]byte, error) { args := m.Called(ctx, req) - return args.Get(0).([]byte), args.Error(1) + + byteResult, ok := args.Get(0).([]byte) + if !ok { + return nil, fmt.Errorf("type assertion to []byte failed") + } + + return byteResult, args.Error(1) } func (m *MockReference) ToState() (llb.State, error) { args := m.Called() - return args.Get(0).(llb.State), args.Error(1) + + state, ok := args.Get(0).(llb.State) + if !ok { + return state, fmt.Errorf("type assertion to llb.State failed") + } + + return state, args.Error(1) } func (m *MockReference) Evaluate(ctx context.Context) error { args := m.Called(ctx) - return args.Get(0).(error) + + evalErr, ok := args.Get(0).(error) + if !ok { + return fmt.Errorf("type assertion to error failed") + } + + return evalErr } func (m *MockReference) StatFile(ctx context.Context, req gwclient.StatRequest) (*types.Stat, error) { args := m.Called(ctx, req) - return args.Get(0).(*types.Stat), args.Error(1) + + stat, ok := args.Get(0).(*types.Stat) + if !ok { + return nil, fmt.Errorf("type assertion to *types.Stat failed") + } + + return stat, args.Error(1) } func (m *MockReference) ReadDir(ctx context.Context, req gwclient.ReadDirRequest) ([]*types.Stat, error) { args := m.Called(ctx, req) - return args.Get(0).([]*types.Stat), args.Error(1) + + statSlice, ok := args.Get(0).([]*types.Stat) + if !ok { + return nil, fmt.Errorf("type assertion to []*types.Stat failed") + } + + return statSlice, args.Error(1) } diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index 79567963..662480bb 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "github.com/containerd/containerd/platforms" "github.com/moby/buildkit/client/llb" @@ -77,7 +78,7 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage } func updateImageConfigData(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, []byte, string, error) { - baseImage, userImageConfig := setupLabels(image, configData) + baseImage, userImageConfig, _ := setupLabels(image, configData) if baseImage == "" { configData = userImageConfig @@ -92,7 +93,7 @@ func updateImageConfigData(ctx context.Context, c gwclient.Client, configData [] return nil, nil, "", err } - _, baseImageWithLabels := setupLabels(baseImage, baseImageConfig) + _, baseImageWithLabels, _ := setupLabels(baseImage, baseImageConfig) configData = baseImageWithLabels return configData, patchedImageConfig, baseImage, nil @@ -101,14 +102,18 @@ func updateImageConfigData(ctx context.Context, c gwclient.Client, configData [] return configData, nil, image, nil } -func setupLabels(image string, configData []byte) (string, []byte) { +func setupLabels(image string, configData []byte) (string, []byte, error) { imageConfig := make(map[string]interface{}) err := json.Unmarshal(configData, &imageConfig) if err != nil { - return "", nil + return "", nil, err } - configMap := imageConfig["config"].(map[string]interface{}) + configMap, ok := imageConfig["config"].(map[string]interface{}) + if !ok { + err := fmt.Errorf("type assertion to map[string]interface{} failed") + return "", nil, err + } var baseImage string labels := configMap["labels"] @@ -116,16 +121,24 @@ func setupLabels(image string, configData []byte) (string, []byte) { labels = make(map[string]interface{}) configMap["labels"] = labels } - labelsMap := labels.(map[string]interface{}) - if baseImageValue, ok := labelsMap["BaseImage"]; ok { - baseImage = baseImageValue.(string) + labelsMap, ok := labels.(map[string]interface{}) + if !ok { + err := fmt.Errorf("type assertion to map[string]interface{} failed") + return "", nil, err + } + if baseImageValue := labelsMap["BaseImage"]; baseImageValue != nil { + baseImage, ok = baseImageValue.(string) + if !ok { + err := fmt.Errorf("type assertion to string failed") + return "", nil, err + } } else { labelsMap["BaseImage"] = image } imageWithLabels, _ := json.Marshal(imageConfig) - return baseImage, imageWithLabels + return baseImage, imageWithLabels, nil } // Extracts the bytes of the file denoted by `path` from the state `st`. diff --git a/pkg/buildkit/buildkit_test.go b/pkg/buildkit/buildkit_test.go index 58c67339..69ddf5ca 100644 --- a/pkg/buildkit/buildkit_test.go +++ b/pkg/buildkit/buildkit_test.go @@ -301,7 +301,7 @@ func TestSetupLabels(t *testing.T) { for _, test := range tests { t.Run(test.testName, func(t *testing.T) { image := "test_image" - baseImage, updatedConfigData := setupLabels(image, test.configData) + baseImage, updatedConfigData, _ := setupLabels(image, test.configData) if test.expectError { assert.Equal(t, "", baseImage) @@ -313,7 +313,11 @@ func TestSetupLabels(t *testing.T) { err := json.Unmarshal(updatedConfigData, &updatedConfig) assert.NoError(t, err) - labels := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) + labels, ok := updatedConfig["config"].(map[string]interface{})["labels"].(map[string]interface{}) + if !ok { + t.Errorf("type assertion to map[string]interface{} failed") + return + } assert.Equal(t, test.expectImage, labels["BaseImage"]) } }) From 66126664812d958efaeeb980380c8aa8584729a9 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Wed, 3 Jul 2024 16:29:42 -0600 Subject: [PATCH 14/22] Revert go mod Signed-off-by: Miaha Cybersec --- go.mod | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go.mod b/go.mod index dd67593b..3de96fd8 100644 --- a/go.mod +++ b/go.mod @@ -130,7 +130,6 @@ require ( github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.6.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/theupdateframework/notary v0.7.0 // indirect github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c // indirect @@ -175,4 +174,4 @@ require ( sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect -) +) \ No newline at end of file From eb5504194bf93c536aeec75c858ccae5033ed40a Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Fri, 5 Jul 2024 10:24:15 -0600 Subject: [PATCH 15/22] Remove unused mock methods and unncessary dependency Signed-off-by: Miaha Cybersec --- go.mod | 3 ++- mocks/mock_gwclient.go | 24 ------------------------ 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/go.mod b/go.mod index 3de96fd8..dd67593b 100644 --- a/go.mod +++ b/go.mod @@ -130,6 +130,7 @@ require ( github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.6.0 // indirect github.com/spf13/pflag v1.0.5 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/theupdateframework/notary v0.7.0 // indirect github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c // indirect @@ -174,4 +175,4 @@ require ( sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect -) \ No newline at end of file +) diff --git a/mocks/mock_gwclient.go b/mocks/mock_gwclient.go index 38ceb32a..533cbda0 100644 --- a/mocks/mock_gwclient.go +++ b/mocks/mock_gwclient.go @@ -4,8 +4,6 @@ import ( "context" "fmt" - "github.com/tonistiigi/fsutil/types" - "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/client/llb/sourceresolver" gwclient "github.com/moby/buildkit/frontend/gateway/client" @@ -138,25 +136,3 @@ func (m *MockReference) Evaluate(ctx context.Context) error { return evalErr } - -func (m *MockReference) StatFile(ctx context.Context, req gwclient.StatRequest) (*types.Stat, error) { - args := m.Called(ctx, req) - - stat, ok := args.Get(0).(*types.Stat) - if !ok { - return nil, fmt.Errorf("type assertion to *types.Stat failed") - } - - return stat, args.Error(1) -} - -func (m *MockReference) ReadDir(ctx context.Context, req gwclient.ReadDirRequest) ([]*types.Stat, error) { - args := m.Called(ctx, req) - - statSlice, ok := args.Get(0).([]*types.Stat) - if !ok { - return nil, fmt.Errorf("type assertion to []*types.Stat failed") - } - - return statSlice, args.Error(1) -} From 7c1a31429b4d00bfa32403401dd9de3965278390 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Fri, 5 Jul 2024 13:00:50 -0600 Subject: [PATCH 16/22] Add mock client assertions and suppress gocritic hugeParam error Signed-off-by: Miaha Cybersec --- mocks/mock_gwclient.go | 3 +++ pkg/buildkit/buildkit_test.go | 2 ++ 2 files changed, 5 insertions(+) diff --git a/mocks/mock_gwclient.go b/mocks/mock_gwclient.go index 533cbda0..3fab007b 100644 --- a/mocks/mock_gwclient.go +++ b/mocks/mock_gwclient.go @@ -28,6 +28,7 @@ func (m *MockGWClient) ResolveSourceMetadata(ctx context.Context, op *pb.SourceO return metaResponse, args.Error(1) } +//nolint:gocritic func (m *MockGWClient) Solve(ctx context.Context, req gwclient.SolveRequest) (*gwclient.Result, error) { args := m.Called(ctx, req) @@ -77,6 +78,7 @@ func (m *MockGWClient) Inputs(ctx context.Context) (map[string]llb.State, error) return stateMap, args.Error(1) } +//nolint:gocritic func (m *MockGWClient) NewContainer(ctx context.Context, req gwclient.NewContainerRequest) (gwclient.Container, error) { args := m.Called(ctx, req) @@ -88,6 +90,7 @@ func (m *MockGWClient) NewContainer(ctx context.Context, req gwclient.NewContain return container, args.Error(1) } +//nolint:gocritic func (m *MockGWClient) Warn(ctx context.Context, dgst digest.Digest, msg string, opts gwclient.WarnOpts) error { args := m.Called(ctx, dgst, msg, opts) diff --git a/pkg/buildkit/buildkit_test.go b/pkg/buildkit/buildkit_test.go index 69ddf5ca..8071aec7 100644 --- a/pkg/buildkit/buildkit_test.go +++ b/pkg/buildkit/buildkit_test.go @@ -373,5 +373,7 @@ func TestUpdateImageConfigData(t *testing.T) { if resultImage != "rockylinux:latest" { t.Errorf("Expected image to be baseimage:latest, got %s", resultImage) } + + mockClient.AssertExpectations(t) }) } From d9b67ca2e0695d5279733637c048e3ca78c90083 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Mon, 22 Jul 2024 09:31:29 -0600 Subject: [PATCH 17/22] Refactor patch merging and apply error handling Signed-off-by: Miaha Cybersec --- pkg/buildkit/buildkit.go | 15 ++++++++++----- pkg/pkgmgr/apk.go | 18 ++++++++---------- pkg/pkgmgr/dpkg.go | 38 +++++++++++++++++++------------------- pkg/pkgmgr/rpm.go | 34 +++++++++++++++------------------- 4 files changed, 52 insertions(+), 53 deletions(-) diff --git a/pkg/buildkit/buildkit.go b/pkg/buildkit/buildkit.go index b6e5a9d6..e12e2046 100644 --- a/pkg/buildkit/buildkit.go +++ b/pkg/buildkit/buildkit.go @@ -14,6 +14,7 @@ import ( ) type Config struct { + ImageName string Client gwclient.Client ConfigData []byte PatchedConfigData []byte @@ -31,7 +32,9 @@ type Opts struct { func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage string) (*Config, error) { // Initialize buildkit config for the target image - config := Config{} + config := Config{ + ImageName: userImage, + } // Resolve and pull the config for the target image _, _, configData, err := c.ResolveImageConfig(ctx, userImage, sourceresolver.Opt{ @@ -78,7 +81,10 @@ func InitializeBuildkitConfig(ctx context.Context, c gwclient.Client, userImage } func updateImageConfigData(ctx context.Context, c gwclient.Client, configData []byte, image string) ([]byte, []byte, string, error) { - baseImage, userImageConfig, _ := setupLabels(image, configData) + baseImage, userImageConfig, err := setupLabels(image, configData) + if err != nil { + return nil, nil, "", err + } if baseImage == "" { configData = userImageConfig @@ -118,10 +124,9 @@ func setupLabels(image string, configData []byte) (string, []byte, error) { var baseImage string labels := configMap["labels"] if labels == nil { - labels = make(map[string]interface{}) - configMap["labels"] = labels + configMap["labels"] = make(map[string]interface{}) } - labelsMap, ok := labels.(map[string]interface{}) + labelsMap, ok := configMap["labels"].(map[string]interface{}) if !ok { err := fmt.Errorf("type assertion to map[string]interface{} failed") return "", nil, err diff --git a/pkg/pkgmgr/apk.go b/pkg/pkgmgr/apk.go index 275363cc..fc85f911 100644 --- a/pkg/pkgmgr/apk.go +++ b/pkg/pkgmgr/apk.go @@ -217,23 +217,21 @@ func (am *apkManager) upgradePackages(ctx context.Context, updates unversioned.U // If the image has been patched before, diff the base image and patched image to retain previous patches if am.config.PatchedConfigData != nil { - // Diff the base image and new patches - patchDiff := llb.Diff(am.config.ImageState, apkInstalled) - // Diff the base image and patched image to get previous patches prevPatchDiff := llb.Diff(am.config.ImageState, am.config.PatchedImageState) - // Merge the old patch diffs and new patch diffs - prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, patchDiff}) + // Diff the base image and new patches + newPatchDiff := llb.Diff(am.config.ImageState, apkInstalled) - // This additional diff ensures previous patch layers are discarded - // While ensuring all previous and new patches are properly applied to the base image - combinedPatch := llb.Diff(am.config.ImageState, prevAndNewPatch) + // Merging these two diffs will discard everything in the filesystem that hasn't changed + // Doing llb.Scratch ensures we can keep everything in the filesystem that has not changed + combinedPatch := llb.Merge([]llb.State{prevPatchDiff, newPatchDiff}) + squashedPatch := llb.Scratch().File(llb.Copy(combinedPatch, "/", "/")) // Merge previous and new patches into the base image - combinedPatchMerge := llb.Merge([]llb.State{am.config.ImageState, combinedPatch}) + completePatchMerge := llb.Merge([]llb.State{am.config.ImageState, squashedPatch}) - return &combinedPatchMerge, resultManifestBytes, nil + return &completePatchMerge, resultManifestBytes, nil } // Diff the installed updates and merge that into the target image diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index eef0de5a..a3d8f06b 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -83,7 +83,11 @@ func getAPTImageName(manifest *unversioned.UpdateManifest, osVersion string) str osType := Debian if manifest == nil || manifest.Metadata.OS.Type == Debian { - version = strings.Split(version, ".")[0] + "-slim" + if version > string(11) { + version = strings.Split("11", ".")[0] + "-slim" + } else { + version = strings.Split(version, ".")[0] + "-slim" + } } else { osType = manifest.Metadata.OS.Type } @@ -354,23 +358,21 @@ func (dm *dpkgManager) installUpdates(ctx context.Context, updates unversioned.U // If the image has been patched before, diff the base image and patched image to retain previous patches if dm.config.PatchedConfigData != nil { - // Diff the base image and new patches - patchDiff := llb.Diff(dm.config.ImageState, aptInstalled) - // Diff the base image and patched image to get previous patches prevPatchDiff := llb.Diff(dm.config.ImageState, dm.config.PatchedImageState) - // Merge the old patch diffs and new patch diffs - prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, patchDiff}) + // Diff the base image and new patches + newPatchDiff := llb.Diff(dm.config.ImageState, aptInstalled) - // This additional diff ensures previous patch layers are discarded - // While ensuring all previous and new patches are properly applied to the base image - combinedPatch := llb.Diff(dm.config.ImageState, prevAndNewPatch) + // Merging these two diffs will discard everything in the filesystem that hasn't changed + // Doing llb.Scratch ensures we can keep everything in the filesystem that has not changed + combinedPatch := llb.Merge([]llb.State{prevPatchDiff, newPatchDiff}) + squashedPatch := llb.Scratch().File(llb.Copy(combinedPatch, "/", "/")) // Merge previous and new patches into the base image - combinedPatchMerge := llb.Merge([]llb.State{dm.config.ImageState, combinedPatch}) + completePatchMerge := llb.Merge([]llb.State{dm.config.ImageState, squashedPatch}) - return &combinedPatchMerge, resultsBytes, nil + return &completePatchMerge, resultsBytes, nil } // Diff the installed updates and merge that into the target image @@ -529,17 +531,15 @@ func (dm *dpkgManager) unpackAndMergeUpdates(ctx context.Context, updates unvers // Diff the manifests for the latest updates manifestsDiff := llb.Diff(fieldsWritten, statusUpdated) - // Merge the old patch diffs and new patch diffs - prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, manifestsDiff, unpackedToRoot}) - - // This additional diff ensures previous patch layers are discarded - // While ensuring all previous and new patches are properly applied to the base image - combinedPatch := llb.Diff(dm.config.ImageState, prevAndNewPatch) + // Merging these two diffs will discard everything in the filesystem that hasn't changed + // Doing llb.Scratch ensures we can keep everything in the filesystem that has not changed + combinedPatch := llb.Merge([]llb.State{prevPatchDiff, manifestsDiff, unpackedToRoot}) + squashedPatch := llb.Scratch().File(llb.Copy(combinedPatch, "/", "/")) // Merge previous and new patches into the base image - combinedPatchMerge := llb.Merge([]llb.State{dm.config.ImageState, combinedPatch}) + completePatchMerge := llb.Merge([]llb.State{dm.config.ImageState, squashedPatch}) - return &combinedPatchMerge, resultsBytes, nil + return &completePatchMerge, resultsBytes, nil } // Diff unpacked packages layers from previous and merge with target diff --git a/pkg/pkgmgr/rpm.go b/pkg/pkgmgr/rpm.go index 39d8bebb..22286033 100644 --- a/pkg/pkgmgr/rpm.go +++ b/pkg/pkgmgr/rpm.go @@ -451,23 +451,21 @@ func (rm *rpmManager) installUpdates(ctx context.Context, updates unversioned.Up // If the image has been patched before, diff the base image and patched image to retain previous patches if rm.config.PatchedConfigData != nil { - // Diff the base image and new patches - patchDiff := llb.Diff(rm.config.ImageState, installed) - - // Diff the base image and patched image to get previous patches + // Diff the base image and pat[]ched image to get previous patches prevPatchDiff := llb.Diff(rm.config.ImageState, rm.config.PatchedImageState) - // Merge the old patch diffs and new patch diffs - prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, patchDiff}) + // Diff the base image and new patches + newPatchDiff := llb.Diff(rm.config.ImageState, installed) - // This additional diff ensures previous patch layers are discarded - // While ensuring all previous and new patches are properly applied to the base image - combinedPatch := llb.Diff(rm.config.ImageState, prevAndNewPatch) + // Merging these two diffs will discard everything in the filesystem that hasn't changed + // Doing llb.Scratch ensures we can keep everything in the filesystem that has not changed + combinedPatch := llb.Merge([]llb.State{prevPatchDiff, newPatchDiff}) + squashedPatch := llb.Scratch().File(llb.Copy(combinedPatch, "/", "/")) // Merge previous and new patches into the base image - combinedPatchMerge := llb.Merge([]llb.State{rm.config.ImageState, combinedPatch}) + completePatchMerge := llb.Merge([]llb.State{rm.config.ImageState, squashedPatch}) - return &combinedPatchMerge, resultBytes, nil + return &completePatchMerge, resultBytes, nil } // Diff the installed updates and merge that into the target image @@ -623,17 +621,15 @@ func (rm *rpmManager) unpackAndMergeUpdates(ctx context.Context, updates unversi // Diff the manifests for the latest updates manifestsDiff := llb.Diff(manifestsUpdated, manifestsPlaced) - // Merge the old patch diffs and new patch diffs - prevAndNewPatch := llb.Merge([]llb.State{prevPatchDiff, manifestsDiff, patchedRoot}) - - // This additional diff ensures previous patch layers are discarded - // While ensuring all previous and new patches are properly applied to the base image - combinedPatch := llb.Diff(rm.config.ImageState, prevAndNewPatch) + // Merging these two diffs will discard everything in the filesystem that hasn't changed + // Doing llb.Scratch ensures we can keep everything in the filesystem that has not changed + combinedPatch := llb.Merge([]llb.State{prevPatchDiff, manifestsDiff, patchedRoot}) + squashedPatch := llb.Scratch().File(llb.Copy(combinedPatch, "/", "/")) // Merge previous and new patches into the base image - combinedPatchMerge := llb.Merge([]llb.State{rm.config.ImageState, combinedPatch}) + completePatchMerge := llb.Merge([]llb.State{rm.config.ImageState, squashedPatch}) - return &combinedPatchMerge, resultBytes, nil + return &completePatchMerge, resultBytes, nil } // Diff unpacked packages layers from previous and merge with target From 28aa14e98f20d6caec73b25db6c6d80bd3df355b Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Mon, 22 Jul 2024 09:40:05 -0600 Subject: [PATCH 18/22] golangci-lint Signed-off-by: Miaha Cybersec --- pkg/pkgmgr/dpkg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index a3d8f06b..a2ee2eac 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -83,7 +83,7 @@ func getAPTImageName(manifest *unversioned.UpdateManifest, osVersion string) str osType := Debian if manifest == nil || manifest.Metadata.OS.Type == Debian { - if version > string(11) { + if version > fmt.Sprintf("11") { version = strings.Split("11", ".")[0] + "-slim" } else { version = strings.Split(version, ".")[0] + "-slim" From c27ac73a280707eb66c9b95213865f32af78d706 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Mon, 22 Jul 2024 09:45:03 -0600 Subject: [PATCH 19/22] golangci-lint Signed-off-by: Miaha Cybersec --- pkg/pkgmgr/dpkg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index a2ee2eac..95490cf4 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -83,7 +83,7 @@ func getAPTImageName(manifest *unversioned.UpdateManifest, osVersion string) str osType := Debian if manifest == nil || manifest.Metadata.OS.Type == Debian { - if version > fmt.Sprintf("11") { + if version > "11" { version = strings.Split("11", ".")[0] + "-slim" } else { version = strings.Split(version, ".")[0] + "-slim" From a01880e403772b4497e43a9518d3b47f1585d7e1 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Tue, 30 Jul 2024 12:49:41 -0600 Subject: [PATCH 20/22] Revert first-time patching logic Signed-off-by: Miaha Cybersec --- pkg/pkgmgr/apk.go | 2 +- pkg/pkgmgr/dpkg.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/pkgmgr/apk.go b/pkg/pkgmgr/apk.go index fc85f911..fe17954d 100644 --- a/pkg/pkgmgr/apk.go +++ b/pkg/pkgmgr/apk.go @@ -235,7 +235,7 @@ func (am *apkManager) upgradePackages(ctx context.Context, updates unversioned.U } // Diff the installed updates and merge that into the target image - patchDiff := llb.Diff(am.config.ImageState, apkInstalled) + patchDiff := llb.Diff(apkUpdated, apkInstalled) patchMerge := llb.Merge([]llb.State{am.config.ImageState, patchDiff}) return &patchMerge, resultManifestBytes, nil diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index 95490cf4..3657add0 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -376,7 +376,7 @@ func (dm *dpkgManager) installUpdates(ctx context.Context, updates unversioned.U } // Diff the installed updates and merge that into the target image - patchDiff := llb.Diff(dm.config.ImageState, aptInstalled) + patchDiff := llb.Diff(aptUpdated, aptInstalled) patchMerge := llb.Merge([]llb.State{dm.config.ImageState, patchDiff}) return &patchMerge, resultsBytes, nil From 503e24bff1e059090382a8c7483a5a6809f72ed9 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Wed, 31 Jul 2024 09:59:25 -0600 Subject: [PATCH 21/22] Revert patching logic regression in DPKG and APK Signed-off-by: Miaha Cybersec --- pkg/pkgmgr/apk.go | 2 +- pkg/pkgmgr/dpkg.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/pkgmgr/apk.go b/pkg/pkgmgr/apk.go index fe17954d..53d19fca 100644 --- a/pkg/pkgmgr/apk.go +++ b/pkg/pkgmgr/apk.go @@ -221,7 +221,7 @@ func (am *apkManager) upgradePackages(ctx context.Context, updates unversioned.U prevPatchDiff := llb.Diff(am.config.ImageState, am.config.PatchedImageState) // Diff the base image and new patches - newPatchDiff := llb.Diff(am.config.ImageState, apkInstalled) + newPatchDiff := llb.Diff(apkUpdated, apkInstalled) // Merging these two diffs will discard everything in the filesystem that hasn't changed // Doing llb.Scratch ensures we can keep everything in the filesystem that has not changed diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index 3657add0..bc160761 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -362,7 +362,7 @@ func (dm *dpkgManager) installUpdates(ctx context.Context, updates unversioned.U prevPatchDiff := llb.Diff(dm.config.ImageState, dm.config.PatchedImageState) // Diff the base image and new patches - newPatchDiff := llb.Diff(dm.config.ImageState, aptInstalled) + newPatchDiff := llb.Diff(aptUpdated, aptInstalled) // Merging these two diffs will discard everything in the filesystem that hasn't changed // Doing llb.Scratch ensures we can keep everything in the filesystem that has not changed From efa23d86b9d0d87e2109b794d28d87f23581d1f2 Mon Sep 17 00:00:00 2001 From: Miaha Cybersec Date: Fri, 2 Aug 2024 15:06:13 -0600 Subject: [PATCH 22/22] Update dpkg version handling with nil manifest Signed-off-by: Miaha Cybersec --- pkg/pkgmgr/dpkg.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/pkgmgr/dpkg.go b/pkg/pkgmgr/dpkg.go index bc160761..45901971 100644 --- a/pkg/pkgmgr/dpkg.go +++ b/pkg/pkgmgr/dpkg.go @@ -83,8 +83,8 @@ func getAPTImageName(manifest *unversioned.UpdateManifest, osVersion string) str osType := Debian if manifest == nil || manifest.Metadata.OS.Type == Debian { - if version > "11" { - version = strings.Split("11", ".")[0] + "-slim" + if version > "12" { + version = strings.Split("stable", ".")[0] + "-slim" } else { version = strings.Split(version, ".")[0] + "-slim" }