From ca07b658a00f406cd88c0eeffe8902f9ccff1cb2 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Fri, 2 Jun 2023 11:54:04 +0530 Subject: [PATCH 01/24] add helper functions for extend using daemon Signed-off-by: Darshan Kumar --- internal/build/docker.go | 1 + internal/build/helper.go | 90 +++++++++++++++++++++++++++ internal/build/lifecycle_execution.go | 40 ++++++++++-- pkg/client/docker.go | 1 + 4 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 internal/build/helper.go diff --git a/internal/build/docker.go b/internal/build/docker.go index 58a53f343d..064795a5e6 100644 --- a/internal/build/docker.go +++ b/internal/build/docker.go @@ -21,4 +21,5 @@ type DockerClient interface { ContainerInspect(ctx context.Context, container string) (types.ContainerJSON, error) ContainerRemove(ctx context.Context, container string, options types.ContainerRemoveOptions) error CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error + ImageBuild(ctx context.Context, buildContext io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) } diff --git a/internal/build/helper.go b/internal/build/helper.go new file mode 100644 index 0000000000..c848060b75 --- /dev/null +++ b/internal/build/helper.go @@ -0,0 +1,90 @@ +package build + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/BurntSushi/toml" + "github.com/buildpacks/lifecycle/buildpack" + "github.com/buildpacks/lifecycle/cmd" + + "github.com/buildpacks/pack/pkg/logging" +) + +const ( + DockerfileKindBuild = "build" + DockerfileKindRun = "run" +) + +type Extensions struct { + Extensions []buildpack.GroupElement +} + +func (extensions *Extensions) DockerFiles(kind string, path string, logger logging.Logger) ([]buildpack.DockerfileInfo, error) { + var dockerfiles []buildpack.DockerfileInfo + for _, ext := range extensions.Extensions { + dockerfile, err := extensions.ReadDockerFile(path, kind, ext.ID) + if err != nil { + return nil, err + } + if dockerfile != nil { + logger.Debugf("Found %s Dockerfile for extension '%s'", kind, ext.ID) + switch kind { + case DockerfileKindBuild: + // will implement later + case DockerfileKindRun: + buildpack.ValidateRunDockerfile(dockerfile, logger) + default: + return nil, fmt.Errorf("unknown dockerfile kind: %s", kind) + } + dockerfiles = append(dockerfiles, *dockerfile) + } + } + return dockerfiles, nil +} + +func (extensions *Extensions) ReadDockerFile(path string, kind string, extID string) (*buildpack.DockerfileInfo, error) { + dockerfilePath := filepath.Join(path, kind, escapeID(extID), "Dockerfile") + if _, err := os.Stat(dockerfilePath); err != nil { + return nil, nil + } + return &buildpack.DockerfileInfo{ + ExtensionID: extID, + Kind: kind, + Path: dockerfilePath, + }, nil +} + +func (extensions *Extensions) SetExtensions(path string, logger logging.Logger) error { + groupExt, err := readExtensionsGroup(path) + if err != nil { + return fmt.Errorf("reading group: %w", err) + } + for i := range groupExt { + groupExt[i].Extension = true + } + for _, groupEl := range groupExt { + if err = cmd.VerifyBuildpackAPI(groupEl.Kind(), groupEl.String(), groupEl.API, logger); err != nil { + return err + } + } + extensions.Extensions = groupExt + fmt.Println("extensions.Extensions", extensions.Extensions) + return nil +} + +func readExtensionsGroup(path string) ([]buildpack.GroupElement, error) { + var group buildpack.Group + _, err := toml.DecodeFile(filepath.Join(path, "group.toml"), &group) + for e := range group.GroupExtensions { + group.GroupExtensions[e].Extension = true + group.GroupExtensions[e].Optional = true + } + return group.GroupExtensions, err +} + +func escapeID(id string) string { + return strings.ReplaceAll(id, "/", "_") +} diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 485508aa60..52adecbcc8 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -7,10 +7,13 @@ import ( "os" "path/filepath" "strconv" + "time" + + "github.com/BurntSushi/toml" "github.com/buildpacks/pack/pkg/cache" - "github.com/BurntSushi/toml" + // "github.com/buildpacks/pack/pkg/archive" "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/auth" "github.com/google/go-containerregistry/pkg/authn" @@ -247,10 +250,17 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF } if l.platformAPI.AtLeast("0.12") && l.hasExtensionsForRun() { - group.Go(func() error { - l.logger.Info(style.Step("EXTENDING (RUN)")) - return l.ExtendRun(ctx, buildCache, phaseFactory) - }) + if l.opts.Publish { + group.Go(func() error { + l.logger.Info(style.Step("EXTENDING (RUN)")) + return l.ExtendRun(ctx, buildCache, phaseFactory) + }) + } else { + group.Go(func() error { + l.logger.Info(style.Step("EXTENDING (RUN) BY DAEMON")) + return l.ExtendRunByDaemon(ctx) + }) + } } if err := group.Wait(); err != nil { @@ -413,6 +423,10 @@ func (l *LifecycleExecution) Detect(ctx context.Context, phaseFactory PhaseFacto CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "analyzed.toml"), l.tmpDir))), If(l.hasExtensions(), WithPostContainerRunOperations( CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "generated", "build"), l.tmpDir))), + If(l.hasExtensions(), WithPostContainerRunOperations( + CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "generated", "run"), l.tmpDir))), + If(l.hasExtensions(), WithPostContainerRunOperations( + CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "group.toml"), l.tmpDir))), envOp, ) @@ -707,6 +721,22 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, ph return extend.Run(ctx) } +func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context) error { + var extensions Extensions + l.logger.Debugf("extending run image %s", l.opts.RunImage) + fmt.Println("tmpDir: ", l.tmpDir) + extensions.SetExtensions(l.tmpDir, l.logger) + dockerfiles, err := extensions.DockerFiles(DockerfileKindRun, l.tmpDir, l.logger) + if err != nil { + return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindRun, err) + } + fmt.Println("Dockerfiles: ", dockerfiles) + fmt.Println("extend: ", dockerfiles[1].Extend) + time.Sleep(10 * time.Minute) + return nil + // buildContest := archive.ReadDirAsTar(filepath.Join(l.tmpDir,"generated","run"),"/",0,0,-1,true,false) +} + func determineDefaultProcessType(platformAPI *api.Version, providedValue string) string { shouldSetForceDefault := platformAPI.Compare(api.MustParse("0.4")) >= 0 && platformAPI.Compare(api.MustParse("0.6")) < 0 diff --git a/pkg/client/docker.go b/pkg/client/docker.go index 0ae1b4880f..a825d68212 100644 --- a/pkg/client/docker.go +++ b/pkg/client/docker.go @@ -28,4 +28,5 @@ type DockerClient interface { ContainerWait(ctx context.Context, container string, condition containertypes.WaitCondition) (<-chan containertypes.WaitResponse, <-chan error) ContainerAttach(ctx context.Context, container string, options types.ContainerAttachOptions) (types.HijackedResponse, error) ContainerStart(ctx context.Context, container string, options types.ContainerStartOptions) error + ImageBuild(ctx context.Context, buildContext io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) } From abdf62c431dab3248f8ccabc33e6231bb6a8b546 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Fri, 2 Jun 2023 18:15:13 +0530 Subject: [PATCH 02/24] add buildImage by daemon Signed-off-by: Darshan Kumar --- internal/build/lifecycle_execution.go | 45 ++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 52adecbcc8..0d8b4ce2c4 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -3,19 +3,20 @@ package build import ( "context" "fmt" + "io" + "math/rand" "os" "path/filepath" "strconv" - "time" "github.com/BurntSushi/toml" "github.com/buildpacks/pack/pkg/cache" - // "github.com/buildpacks/pack/pkg/archive" "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/auth" + "github.com/docker/docker/api/types" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" "github.com/pkg/errors" @@ -24,6 +25,7 @@ import ( "github.com/buildpacks/pack/internal/builder" "github.com/buildpacks/pack/internal/paths" "github.com/buildpacks/pack/internal/style" + "github.com/buildpacks/pack/pkg/archive" "github.com/buildpacks/pack/pkg/logging" ) @@ -258,11 +260,10 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF } else { group.Go(func() error { l.logger.Info(style.Step("EXTENDING (RUN) BY DAEMON")) - return l.ExtendRunByDaemon(ctx) + return l.ExtendRunByDaemon(ctx, ¤tRunImage) }) } } - if err := group.Wait(); err != nil { return err } @@ -721,9 +722,10 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, ph return extend.Run(ctx) } -func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context) error { +func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, currentRunImage *string) error { + defaultFilterFunc := func(file string) bool { return true } var extensions Extensions - l.logger.Debugf("extending run image %s", l.opts.RunImage) + l.logger.Debugf("extending run image %s", *currentRunImage) fmt.Println("tmpDir: ", l.tmpDir) extensions.SetExtensions(l.tmpDir, l.logger) dockerfiles, err := extensions.DockerFiles(DockerfileKindRun, l.tmpDir, l.logger) @@ -732,9 +734,36 @@ func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context) error { } fmt.Println("Dockerfiles: ", dockerfiles) fmt.Println("extend: ", dockerfiles[1].Extend) - time.Sleep(10 * time.Minute) + for _, dockerfile := range dockerfiles { + if dockerfile.Extend { + fmt.Println("dockerfile: ", dockerfile) + fmt.Println("dockerfile.Path dir: ", filepath.Dir(dockerfile.Path)) + buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) + fmt.Println("buildContext: ", buildContext) + buildArguments := map[string]*string{} + if dockerfile.WithBase == "" { + buildArguments["base_image"] = currentRunImage + } + buildOptions := types.ImageBuildOptions{ + Context: buildContext, + Dockerfile: "Dockerfile", + Tags: []string{"run-image"}, + Remove: true, + BuildArgs: buildArguments, + } + response, err := l.docker.ImageBuild(ctx, buildContext, buildOptions) + if err != nil { + return err + } + defer response.Body.Close() + _, err = io.Copy(os.Stdout, response.Body) + if err != nil { + return err + } + l.logger.Debugf("build response for the extend: %v", response) + } + } return nil - // buildContest := archive.ReadDirAsTar(filepath.Join(l.tmpDir,"generated","run"),"/",0,0,-1,true,false) } func determineDefaultProcessType(platformAPI *api.Version, providedValue string) string { From 5c460c59ad9715ee1ef3373a86b5d5ec114b6573 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Mon, 12 Jun 2023 04:01:13 +0530 Subject: [PATCH 03/24] extract layers and extend the export phase Signed-off-by: Darshan Kumar --- internal/build/docker.go | 2 + internal/build/helper.go | 89 ++++++++++++++++++++++++++- internal/build/lifecycle_execution.go | 57 ++++++++++++++--- 3 files changed, 137 insertions(+), 11 deletions(-) diff --git a/internal/build/docker.go b/internal/build/docker.go index 064795a5e6..fb6717b210 100644 --- a/internal/build/docker.go +++ b/internal/build/docker.go @@ -22,4 +22,6 @@ type DockerClient interface { ContainerRemove(ctx context.Context, container string, options types.ContainerRemoveOptions) error CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error ImageBuild(ctx context.Context, buildContext io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) + ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error) + ImageSave(ctx context.Context, imageIDs []string) (io.ReadCloser, error) } diff --git a/internal/build/helper.go b/internal/build/helper.go index c848060b75..0c603d07fb 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -2,15 +2,20 @@ package build import ( "fmt" + "io" "os" "path/filepath" "strings" "github.com/BurntSushi/toml" + "github.com/buildpacks/imgutil/layout/sparse" "github.com/buildpacks/lifecycle/buildpack" "github.com/buildpacks/lifecycle/cmd" - "github.com/buildpacks/pack/pkg/logging" + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/daemon" + "golang.org/x/sync/errgroup" ) const ( @@ -88,3 +93,85 @@ func readExtensionsGroup(path string) ([]buildpack.GroupElement, error) { func escapeID(id string) string { return strings.ReplaceAll(id, "/", "_") } + +func SaveLayers(group *errgroup.Group, image v1.Image, origTopLayerHash string, dest string) error { + layoutPath, err := sparse.NewImage(dest, image) + if err != nil { + fmt.Println("sparse.NewImage err", err) + return err + } + if err = layoutPath.Save(); err != nil { + return err + } + if err != nil { + fmt.Println("sparse.NewImage err", err) + return err + } + layers, err := image.Layers() + if err != nil { + return fmt.Errorf("getting image layers: %w", err) + } + var ( + currentHash v1.Hash + needsCopying bool + ) + if origTopLayerHash == "" { + needsCopying = true + } + for _, currentLayer := range layers { + currentHash, err = currentLayer.Digest() + if err != nil { + return fmt.Errorf("getting layer hash: %w", err) + } + switch { + case needsCopying: + currentLayer := currentLayer + group.Go(func() error { + return copyLayer(currentLayer, dest) + }) + case currentHash.String() == origTopLayerHash: + needsCopying = true + continue + default: + continue + } + } + return nil +} + +func copyLayer(layer v1.Layer, toSparseImage string) error { + digest, err := layer.Digest() + if err != nil { + return err + } + f, err := os.Create(filepath.Join(toSparseImage, "blobs", digest.Algorithm, digest.Hex)) + if err != nil { + return err + } + defer f.Close() + rc, err := layer.Compressed() + if err != nil { + return err + } + defer rc.Close() + _, err = io.Copy(f, rc) + return err +} + +func topLayerHash(image *string) (string, error) { + baseRef, err := name.ParseReference(*image) + if err != nil { + return "", fmt.Errorf("failed to parse reference: %v", err) + } + baseImage, err := daemon.Image(baseRef) + if err != nil { + return "", fmt.Errorf("failed to get v1.Image: %v", err) + } + baseManifest, err := baseImage.Manifest() + if err != nil { + return "", fmt.Errorf("getting image manifest: %w", err) + } + baseLayers := baseManifest.Layers + topLayerHash := baseLayers[len(baseLayers)-1].Digest.String() + return topLayerHash, nil +} diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 0d8b4ce2c4..f87422332f 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -4,11 +4,11 @@ import ( "context" "fmt" "io" - "math/rand" "os" "path/filepath" "strconv" + "time" "github.com/BurntSushi/toml" @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/api/types" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/daemon" "github.com/pkg/errors" "golang.org/x/sync/errgroup" @@ -250,7 +251,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF return err } } - + var start time.Time if l.platformAPI.AtLeast("0.12") && l.hasExtensionsForRun() { if l.opts.Publish { group.Go(func() error { @@ -258,15 +259,18 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF return l.ExtendRun(ctx, buildCache, phaseFactory) }) } else { + start = time.Now() group.Go(func() error { l.logger.Info(style.Step("EXTENDING (RUN) BY DAEMON")) - return l.ExtendRunByDaemon(ctx, ¤tRunImage) + return l.ExtendRunByDaemon(ctx, group, ¤tRunImage) }) } } if err := group.Wait(); err != nil { return err } + duration := time.Since(start) + fmt.Println("Execution time:", duration) l.logger.Info(style.Step("EXPORTING")) return l.Export(ctx, buildCache, launchCache, phaseFactory) @@ -722,24 +726,29 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, ph return extend.Run(ctx) } -func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, currentRunImage *string) error { +func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, group *errgroup.Group, currentRunImage *string) error { defaultFilterFunc := func(file string) bool { return true } var extensions Extensions l.logger.Debugf("extending run image %s", *currentRunImage) - fmt.Println("tmpDir: ", l.tmpDir) extensions.SetExtensions(l.tmpDir, l.logger) dockerfiles, err := extensions.DockerFiles(DockerfileKindRun, l.tmpDir, l.logger) if err != nil { return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindRun, err) } - fmt.Println("Dockerfiles: ", dockerfiles) - fmt.Println("extend: ", dockerfiles[1].Extend) + nestedCtx, cancel := context.WithCancel(ctx) + defer cancel() + nestedGroup, _ := errgroup.WithContext(nestedCtx) + var origTopLayerHash string = "" + nestedGroup.Go(func() error { + origTopLayerHash, err = topLayerHash(currentRunImage) + if err != nil { + return fmt.Errorf("getting top layer hash of run image: %w", err) + } + return nil + }) for _, dockerfile := range dockerfiles { if dockerfile.Extend { - fmt.Println("dockerfile: ", dockerfile) - fmt.Println("dockerfile.Path dir: ", filepath.Dir(dockerfile.Path)) buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) - fmt.Println("buildContext: ", buildContext) buildArguments := map[string]*string{} if dockerfile.WithBase == "" { buildArguments["base_image"] = currentRunImage @@ -763,6 +772,26 @@ func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, currentRunIm l.logger.Debugf("build response for the extend: %v", response) } } + ref, err := name.ParseReference("run-image:latest") + if err != nil { + return fmt.Errorf("failed to parse reference: %v", err) + } + image, err := daemon.Image(ref) + if err != nil { + return fmt.Errorf("failed to get v1.Image: %v", err) + } + imageHash, err := image.Digest() + if err != nil { + return fmt.Errorf("getting image hash: %w", err) + } + dest := filepath.Join(l.tmpDir, "extended-new", "run", imageHash.String()) + waiterr := nestedGroup.Wait() + if waiterr != nil { + return err + } + if err = SaveLayers(group, image, origTopLayerHash, dest); err != nil { + return fmt.Errorf("copying selective image to output directory: %w", err) + } return nil } @@ -792,6 +821,12 @@ func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache } } + extDirEnv := NullOp() + if !l.opts.Publish { + l.logger.Debug("export for extend by daemon") + extDirEnv = WithEnv("CNB_EXTENDED_DIR=" + filepath.Join("/", "extended-new")) + } + if l.platformAPI.LessThan("0.7") { flags = append(flags, "-run-image", l.opts.RunImage) } @@ -828,6 +863,7 @@ func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache ), WithArgs(append([]string{l.opts.Image.String()}, l.opts.AdditionalTags...)...), WithRoot(), + WithBinds(filepath.Join(l.tmpDir, "extended-new") + ":/extended-new"), WithNetwork(l.opts.Network), cacheBindOp, WithContainerOperations(WriteStackToml(l.mountPaths.stackPath(), l.opts.Builder.Stack(), l.os)), @@ -844,6 +880,7 @@ func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache CopyOut(l.opts.Termui.ReadLayers, l.mountPaths.layersDir(), l.mountPaths.appDir()))), epochEnv, expEnv, + extDirEnv, } var export RunnerCleaner From 8710e464d91b14f9209375e5176dd6361440e849 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Sun, 25 Jun 2023 08:09:52 +0530 Subject: [PATCH 04/24] implements build image extension by daemon Signed-off-by: Darshan Kumar --- internal/build/helper.go | 26 +++++--- internal/build/lifecycle_execution.go | 86 ++++++++++++++++++++++--- internal/build/phase_config_provider.go | 6 ++ 3 files changed, 102 insertions(+), 16 deletions(-) diff --git a/internal/build/helper.go b/internal/build/helper.go index 0c603d07fb..9f8822eefd 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -6,16 +6,18 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/BurntSushi/toml" "github.com/buildpacks/imgutil/layout/sparse" "github.com/buildpacks/lifecycle/buildpack" "github.com/buildpacks/lifecycle/cmd" - "github.com/buildpacks/pack/pkg/logging" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/daemon" "golang.org/x/sync/errgroup" + + "github.com/buildpacks/pack/pkg/logging" ) const ( @@ -94,22 +96,24 @@ func escapeID(id string) string { return strings.ReplaceAll(id, "/", "_") } -func SaveLayers(group *errgroup.Group, image v1.Image, origTopLayerHash string, dest string) error { +func SaveLayers(group *errgroup.Group, image v1.Image, origTopLayerHash string, dest string) (*time.Duration, error) { + var totalSaveExecutionTime time.Duration + savetime := time.Now() layoutPath, err := sparse.NewImage(dest, image) if err != nil { fmt.Println("sparse.NewImage err", err) - return err + return nil, err } if err = layoutPath.Save(); err != nil { - return err + return nil, err } if err != nil { fmt.Println("sparse.NewImage err", err) - return err + return nil, err } layers, err := image.Layers() if err != nil { - return fmt.Errorf("getting image layers: %w", err) + return nil, fmt.Errorf("getting image layers: %w", err) } var ( currentHash v1.Hash @@ -121,12 +125,17 @@ func SaveLayers(group *errgroup.Group, image v1.Image, origTopLayerHash string, for _, currentLayer := range layers { currentHash, err = currentLayer.Digest() if err != nil { - return fmt.Errorf("getting layer hash: %w", err) + return nil, fmt.Errorf("getting layer hash: %w", err) } switch { case needsCopying: currentLayer := currentLayer + start := time.Now() group.Go(func() error { + defer func() { + duration := time.Since(start) + totalSaveExecutionTime += duration + }() return copyLayer(currentLayer, dest) }) case currentHash.String() == origTopLayerHash: @@ -136,7 +145,8 @@ func SaveLayers(group *errgroup.Group, image v1.Image, origTopLayerHash string, continue } } - return nil + totalSaveExecutionTime += time.Since(savetime) + return &totalSaveExecutionTime, nil } func copyLayer(layer v1.Layer, toSparseImage string) error { diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index f87422332f..d09f9b67c3 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -234,10 +234,20 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF group, _ := errgroup.WithContext(context.TODO()) if l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild() { - group.Go(func() error { - l.logger.Info(style.Step("EXTENDING (BUILD)")) - return l.ExtendBuild(ctx, buildCache, phaseFactory) - }) + if l.opts.Publish { + group.Go(func() error { + l.logger.Info(style.Step("EXTENDING (BUILD)")) + return l.ExtendBuild(ctx, buildCache, phaseFactory) + }) + } else { + group.Go(func() error { + l.logger.Info(style.Step("EXTENDING (BUILD) BY DAEMON")) + if err := l.ExtendBuildByDaemon(ctx, group); err != nil { + return err + } + return l.Build(ctx, phaseFactory) + }) + } } else { group.Go(func() error { l.logger.Info(style.Step("BUILDING")) @@ -262,6 +272,10 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF start = time.Now() group.Go(func() error { l.logger.Info(style.Step("EXTENDING (RUN) BY DAEMON")) + defer func() { + duration := time.Since(start) + fmt.Println("Execution time:", duration) + }() return l.ExtendRunByDaemon(ctx, group, ¤tRunImage) }) } @@ -269,8 +283,6 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF if err := group.Wait(); err != nil { return err } - duration := time.Since(start) - fmt.Println("Execution time:", duration) l.logger.Info(style.Step("EXPORTING")) return l.Export(ctx, buildCache, launchCache, phaseFactory) @@ -657,6 +669,8 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor WithNetwork(l.opts.Network), WithBinds(l.opts.Volumes...), WithFlags(flags...), + If((!l.opts.Publish && l.hasExtensionsForBuild()), WithImage("newbuilder-image:latest")), + If((!l.opts.Publish && l.hasExtensionsForBuild()), WithoutPrivilege()), ) build := phaseFactory.New(configProvider) @@ -725,27 +739,78 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, ph defer extend.Cleanup() return extend.Run(ctx) } +func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context, group *errgroup.Group) error { + builderImageName := l.opts.BuilderImage + defaultFilterFunc := func(file string) bool { return true } + var extensions Extensions + time1 := time.Now() + extensions.SetExtensions(l.tmpDir, l.logger) + fmt.Println("extensions.SetExtensions took", time.Since(time1)) + time2 := time.Now() + dockerfiles, err := extensions.DockerFiles(DockerfileKindBuild, l.tmpDir, l.logger) + if err != nil { + return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindBuild, err) + } + fmt.Println("extensions.DockerFiles took", time.Since(time2)) + dockerapplytime := time.Now() + for _, dockerfile := range dockerfiles { + buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) + buildArguments := map[string]*string{} + if dockerfile.WithBase == "" { + buildArguments["base_image"] = &builderImageName + } + buildOptions := types.ImageBuildOptions{ + Context: buildContext, + Dockerfile: "Dockerfile", + Tags: []string{"newbuilder-image"}, + Remove: true, + BuildArgs: buildArguments, + } + response, err := l.docker.ImageBuild(ctx, buildContext, buildOptions) + if err != nil { + return err + } + defer response.Body.Close() + _, err = io.Copy(os.Stdout, response.Body) + if err != nil { + return err + } + l.logger.Debugf("build response for the extend: %v", response) + } + l.logger.Debugf("docker apply time: %v", time.Since(dockerapplytime)) + + return nil +} func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, group *errgroup.Group, currentRunImage *string) error { defaultFilterFunc := func(file string) bool { return true } var extensions Extensions l.logger.Debugf("extending run image %s", *currentRunImage) + time1 := time.Now() extensions.SetExtensions(l.tmpDir, l.logger) + fmt.Println("extensions.SetExtensions took", time.Since(time1)) + time2 := time.Now() dockerfiles, err := extensions.DockerFiles(DockerfileKindRun, l.tmpDir, l.logger) if err != nil { return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindRun, err) } + fmt.Println("extensions.DockerFiles took", time.Since(time2)) nestedCtx, cancel := context.WithCancel(ctx) defer cancel() nestedGroup, _ := errgroup.WithContext(nestedCtx) - var origTopLayerHash string = "" + var origTopLayerHash = "" + time3 := time.Now() nestedGroup.Go(func() error { + defer func() { + fmt.Println("topLayerHash took", time.Since(time3)) + }() origTopLayerHash, err = topLayerHash(currentRunImage) if err != nil { return fmt.Errorf("getting top layer hash of run image: %w", err) } return nil }) + dockerapplytime := time.Now() for _, dockerfile := range dockerfiles { if dockerfile.Extend { buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) @@ -772,6 +837,8 @@ func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, group *errgr l.logger.Debugf("build response for the extend: %v", response) } } + l.logger.Debugf("docker apply time: %v", time.Since(dockerapplytime)) + time4 := time.Now() ref, err := name.ParseReference("run-image:latest") if err != nil { return fmt.Errorf("failed to parse reference: %v", err) @@ -785,13 +852,16 @@ func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, group *errgr return fmt.Errorf("getting image hash: %w", err) } dest := filepath.Join(l.tmpDir, "extended-new", "run", imageHash.String()) + fmt.Println("exporting to OCI took", time.Since(time4)) waiterr := nestedGroup.Wait() if waiterr != nil { return err } - if err = SaveLayers(group, image, origTopLayerHash, dest); err != nil { + var savetime *time.Duration + if savetime, err = SaveLayers(group, image, origTopLayerHash, dest); err != nil { return fmt.Errorf("copying selective image to output directory: %w", err) } + fmt.Println("Total Save execution time:", savetime) return nil } diff --git a/internal/build/phase_config_provider.go b/internal/build/phase_config_provider.go index 93861633ab..a6566858a2 100644 --- a/internal/build/phase_config_provider.go +++ b/internal/build/phase_config_provider.go @@ -244,6 +244,12 @@ func WithRoot() PhaseConfigProviderOperation { } } +func WithoutPrivilege() PhaseConfigProviderOperation { + return func(provider *PhaseConfigProvider) { + provider.ctrConf.User = "1000:1001" + } +} + func WithContainerOperations(operations ...ContainerOperation) PhaseConfigProviderOperation { return func(provider *PhaseConfigProvider) { provider.containerOps = append(provider.containerOps, operations...) From 88c8fbdf753e7d8702f66930126012c5bd8182ce Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Sun, 25 Jun 2023 13:04:53 +0530 Subject: [PATCH 05/24] adds execution measurement Signed-off-by: Darshan Kumar --- internal/build/lifecycle_execution.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index d09f9b67c3..47a0e10866 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -240,9 +240,14 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF return l.ExtendBuild(ctx, buildCache, phaseFactory) }) } else { + start := time.Now() + defer func() { + elapsed := time.Since(start) + l.logger.Debugf("EXTENDING (BUILD) took %s", elapsed) + }() group.Go(func() error { l.logger.Info(style.Step("EXTENDING (BUILD) BY DAEMON")) - if err := l.ExtendBuildByDaemon(ctx, group); err != nil { + if err := l.ExtendBuildByDaemon(ctx); err != nil { return err } return l.Build(ctx, phaseFactory) @@ -739,19 +744,20 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, ph defer extend.Cleanup() return extend.Run(ctx) } -func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context, group *errgroup.Group) error { +func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { + extendtime := time.Now() builderImageName := l.opts.BuilderImage defaultFilterFunc := func(file string) bool { return true } var extensions Extensions time1 := time.Now() extensions.SetExtensions(l.tmpDir, l.logger) - fmt.Println("extensions.SetExtensions took", time.Since(time1)) + l.logger.Debugf("extensions.SetExtensions for build took %s", time.Since(time1)) time2 := time.Now() dockerfiles, err := extensions.DockerFiles(DockerfileKindBuild, l.tmpDir, l.logger) if err != nil { return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindBuild, err) } - fmt.Println("extensions.DockerFiles took", time.Since(time2)) + l.logger.Debugf("extensions.DockerFiles for build took %s", time.Since(time2)) dockerapplytime := time.Now() for _, dockerfile := range dockerfiles { buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) @@ -778,6 +784,7 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context, group *err l.logger.Debugf("build response for the extend: %v", response) } l.logger.Debugf("docker apply time: %v", time.Since(dockerapplytime)) + l.logger.Debugf("Build extend time: %v", time.Since(extendtime)) return nil } From b8295c5ac15e5c0acebb5256b63e3d1bc3494120 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Sun, 6 Aug 2023 12:25:33 +0530 Subject: [PATCH 06/24] adds mockDaemon and unit test Signed-off-by: Darshan Kumar --- internal/build/docker.go | 2 + internal/build/extend_build_test.go | 138 +++++++++++ internal/build/lifecycle_execution.go | 13 +- internal/build/mockdocker/mockDockerClient.go | 233 ++++++++++++++++++ .../single/build/samples_test/Dockerfile | 24 ++ .../build-extension/single/group.toml | 11 + 6 files changed, 415 insertions(+), 6 deletions(-) create mode 100644 internal/build/extend_build_test.go create mode 100644 internal/build/mockdocker/mockDockerClient.go create mode 100644 internal/build/testdata/fake-tmp/build-extension/single/build/samples_test/Dockerfile create mode 100644 internal/build/testdata/fake-tmp/build-extension/single/group.toml diff --git a/internal/build/docker.go b/internal/build/docker.go index fb6717b210..bc0dcf3c0a 100644 --- a/internal/build/docker.go +++ b/internal/build/docker.go @@ -10,6 +10,8 @@ import ( specs "github.com/opencontainers/image-spec/specs-go/v1" ) +//go:generate mockgen -package mockdocker -destination ./mockdocker/mockDockerClient.go github.com/buildpacks/pack/internal/build DockerClient + type DockerClient interface { ImageRemove(ctx context.Context, image string, options types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error) VolumeRemove(ctx context.Context, volumeID string, force bool) error diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go new file mode 100644 index 0000000000..c7fcc242e9 --- /dev/null +++ b/internal/build/extend_build_test.go @@ -0,0 +1,138 @@ +package build_test + +import ( + "bytes" + "context" + "io" + + "path/filepath" + "strings" + "testing" + + "github.com/apex/log" + "github.com/buildpacks/pack/internal/build" + "github.com/buildpacks/pack/internal/build/fakes" + + "github.com/buildpacks/pack/pkg/archive" + "github.com/buildpacks/pack/pkg/logging" + h "github.com/buildpacks/pack/testhelpers" + "github.com/docker/docker/api/types" + "github.com/golang/mock/gomock" + "github.com/heroku/color" + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + + mockdocker "github.com/buildpacks/pack/internal/build/mockdocker" +) + +func TestBuildDockerfiles(t *testing.T) { + color.Disable(true) + defer color.Disable(false) + spec.Run(t, "buildExtendByDocker", testBuildDockerfiles, spec.Report(report.Terminal{}), spec.Sequential()) + +} + +func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { + var ( + mockDockerClient *mockdocker.MockDockerClient + mockController *gomock.Controller + lifecycle *build.LifecycleExecution + tmpDir string + + // lifecycle options + providedClearCache bool + providedPublish bool + providedBuilderImage = "some-registry.com/some-namespace/some-builder-name" + + configureDefaultTestLifecycle func(opts *build.LifecycleOptions) + lifecycleOps []func(*build.LifecycleOptions) + ) + + it.Before(func() { + var err error + mockController = gomock.NewController(t) + mockDockerClient = mockdocker.NewMockDockerClient(mockController) + h.AssertNil(t, err) + + configureDefaultTestLifecycle = func(opts *build.LifecycleOptions) { + opts.BuilderImage = providedBuilderImage + opts.ClearCache = providedClearCache + opts.Publish = providedPublish + } + + lifecycleOps = []func(*build.LifecycleOptions){configureDefaultTestLifecycle} + + }) + + when("Extend Build Image By Docker", func() { + it("should extend build image using 1 extension", func() { + // set tmp directory + tmpDir = "./testdata/fake-tmp/build-extension/single" + lifecycle = getTestLifecycleExec(t, true, tmpDir, mockDockerClient, lifecycleOps...) + expectedBuildContext := archive.ReadDirAsTar(filepath.Dir(filepath.Join(tmpDir, "fake-tmp", "build-extension", "single", "build", "samples_test", "Dockerfile")), "/", 0, 0, -1, true, false, func(file string) bool { return true }) + // Set up expected Build Args + expectedbuildArguments := map[string]*string{} + expectedbuildArguments["base_image"] = &providedBuilderImage + expectedBuildOptions := types.ImageBuildOptions{ + Context: expectedBuildContext, + Dockerfile: "Dockerfile", + Tags: []string{"newbuilder-image"}, + Remove: true, + BuildArgs: expectedbuildArguments, + } + mockResponse := types.ImageBuildResponse{ + Body: io.NopCloser(strings.NewReader("mock-build-response-body")), + OSType: "linux", + } + mockDockerClient.EXPECT().ImageBuild(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, buildContext io.ReadCloser, buildOptions types.ImageBuildOptions) { + compBuildOptions(t, expectedBuildOptions, buildOptions) + }).Return(mockResponse, nil).Times(1) + err := lifecycle.ExtendBuildByDaemon(context.Background()) + h.AssertNil(t, err) + }) + }) + +} + +func GetTestLifecycleExecErr(t *testing.T, logVerbose bool, tmpDir string, mockDockerClient *mockdocker.MockDockerClient, ops ...func(*build.LifecycleOptions)) (*build.LifecycleExecution, error) { + + var outBuf bytes.Buffer + logger := logging.NewLogWithWriters(&outBuf, &outBuf) + if logVerbose { + logger.Level = log.DebugLevel + } + + defaultBuilder, err := fakes.NewFakeBuilder() + h.AssertNil(t, err) + + opts := build.LifecycleOptions{ + AppPath: "some-app-path", + Builder: defaultBuilder, + HTTPProxy: "some-http-proxy", + HTTPSProxy: "some-https-proxy", + NoProxy: "some-no-proxy", + Termui: &fakes.FakeTermui{}, + } + + for _, op := range ops { + op(&opts) + } + + return build.NewLifecycleExecution(logger, mockDockerClient, tmpDir, opts) +} + +func getTestLifecycleExec(t *testing.T, logVerbose bool, tmpDir string, mockDockerClient *mockdocker.MockDockerClient, ops ...func(*build.LifecycleOptions)) *build.LifecycleExecution { + t.Helper() + + lifecycleExec, err := GetTestLifecycleExecErr(t, logVerbose, tmpDir, mockDockerClient, ops...) + h.AssertNil(t, err) + return lifecycleExec +} + +func compBuildOptions(t *testing.T, expectedBuildOptions types.ImageBuildOptions, actualBuildOptions types.ImageBuildOptions) { + t.Helper() + h.AssertEq(t, expectedBuildOptions.Dockerfile, actualBuildOptions.Dockerfile) + h.AssertEq(t, expectedBuildOptions.Tags, actualBuildOptions.Tags) + h.AssertEq(t, expectedBuildOptions.Remove, actualBuildOptions.Remove) + h.AssertEq(t, expectedBuildOptions.BuildArgs, actualBuildOptions.BuildArgs) +} diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 47a0e10866..c491bac6da 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -240,17 +240,16 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF return l.ExtendBuild(ctx, buildCache, phaseFactory) }) } else { - start := time.Now() - defer func() { - elapsed := time.Since(start) - l.logger.Debugf("EXTENDING (BUILD) took %s", elapsed) - }() group.Go(func() error { l.logger.Info(style.Step("EXTENDING (BUILD) BY DAEMON")) + start := time.Now() if err := l.ExtendBuildByDaemon(ctx); err != nil { return err } - return l.Build(ctx, phaseFactory) + l.Build(ctx, phaseFactory) + elapsed := time.Since(start) + l.logger.Debugf("EXTENDING (BUILD) took %s", elapsed) + return nil }) } } else { @@ -772,11 +771,13 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { Remove: true, BuildArgs: buildArguments, } + fmt.Println("buildContext: ", buildContext) response, err := l.docker.ImageBuild(ctx, buildContext, buildOptions) if err != nil { return err } defer response.Body.Close() + fmt.Println("build response for the extend: ", response.Body) _, err = io.Copy(os.Stdout, response.Body) if err != nil { return err diff --git a/internal/build/mockdocker/mockDockerClient.go b/internal/build/mockdocker/mockDockerClient.go new file mode 100644 index 0000000000..55313e7677 --- /dev/null +++ b/internal/build/mockdocker/mockDockerClient.go @@ -0,0 +1,233 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/buildpacks/pack/internal/build (interfaces: DockerClient) + +// Package mockdocker is a generated GoMock package. +package mockdocker + +import ( + context "context" + io "io" + reflect "reflect" + + types "github.com/docker/docker/api/types" + container "github.com/docker/docker/api/types/container" + network "github.com/docker/docker/api/types/network" + gomock "github.com/golang/mock/gomock" + v1 "github.com/opencontainers/image-spec/specs-go/v1" +) + +// MockDockerClient is a mock of DockerClient interface. +type MockDockerClient struct { + ctrl *gomock.Controller + recorder *MockDockerClientMockRecorder +} + +// MockDockerClientMockRecorder is the mock recorder for MockDockerClient. +type MockDockerClientMockRecorder struct { + mock *MockDockerClient +} + +// NewMockDockerClient creates a new mock instance. +func NewMockDockerClient(ctrl *gomock.Controller) *MockDockerClient { + mock := &MockDockerClient{ctrl: ctrl} + mock.recorder = &MockDockerClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockDockerClient) EXPECT() *MockDockerClientMockRecorder { + return m.recorder +} + +// ContainerAttach mocks base method. +func (m *MockDockerClient) ContainerAttach(arg0 context.Context, arg1 string, arg2 types.ContainerAttachOptions) (types.HijackedResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ContainerAttach", arg0, arg1, arg2) + ret0, _ := ret[0].(types.HijackedResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ContainerAttach indicates an expected call of ContainerAttach. +func (mr *MockDockerClientMockRecorder) ContainerAttach(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ContainerAttach", reflect.TypeOf((*MockDockerClient)(nil).ContainerAttach), arg0, arg1, arg2) +} + +// ContainerCreate mocks base method. +func (m *MockDockerClient) ContainerCreate(arg0 context.Context, arg1 *container.Config, arg2 *container.HostConfig, arg3 *network.NetworkingConfig, arg4 *v1.Platform, arg5 string) (container.CreateResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ContainerCreate", arg0, arg1, arg2, arg3, arg4, arg5) + ret0, _ := ret[0].(container.CreateResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ContainerCreate indicates an expected call of ContainerCreate. +func (mr *MockDockerClientMockRecorder) ContainerCreate(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ContainerCreate", reflect.TypeOf((*MockDockerClient)(nil).ContainerCreate), arg0, arg1, arg2, arg3, arg4, arg5) +} + +// ContainerInspect mocks base method. +func (m *MockDockerClient) ContainerInspect(arg0 context.Context, arg1 string) (types.ContainerJSON, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ContainerInspect", arg0, arg1) + ret0, _ := ret[0].(types.ContainerJSON) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ContainerInspect indicates an expected call of ContainerInspect. +func (mr *MockDockerClientMockRecorder) ContainerInspect(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ContainerInspect", reflect.TypeOf((*MockDockerClient)(nil).ContainerInspect), arg0, arg1) +} + +// ContainerRemove mocks base method. +func (m *MockDockerClient) ContainerRemove(arg0 context.Context, arg1 string, arg2 types.ContainerRemoveOptions) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ContainerRemove", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// ContainerRemove indicates an expected call of ContainerRemove. +func (mr *MockDockerClientMockRecorder) ContainerRemove(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ContainerRemove", reflect.TypeOf((*MockDockerClient)(nil).ContainerRemove), arg0, arg1, arg2) +} + +// ContainerStart mocks base method. +func (m *MockDockerClient) ContainerStart(arg0 context.Context, arg1 string, arg2 types.ContainerStartOptions) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ContainerStart", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// ContainerStart indicates an expected call of ContainerStart. +func (mr *MockDockerClientMockRecorder) ContainerStart(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ContainerStart", reflect.TypeOf((*MockDockerClient)(nil).ContainerStart), arg0, arg1, arg2) +} + +// ContainerWait mocks base method. +func (m *MockDockerClient) ContainerWait(arg0 context.Context, arg1 string, arg2 container.WaitCondition) (<-chan container.WaitResponse, <-chan error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ContainerWait", arg0, arg1, arg2) + ret0, _ := ret[0].(<-chan container.WaitResponse) + ret1, _ := ret[1].(<-chan error) + return ret0, ret1 +} + +// ContainerWait indicates an expected call of ContainerWait. +func (mr *MockDockerClientMockRecorder) ContainerWait(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ContainerWait", reflect.TypeOf((*MockDockerClient)(nil).ContainerWait), arg0, arg1, arg2) +} + +// CopyFromContainer mocks base method. +func (m *MockDockerClient) CopyFromContainer(arg0 context.Context, arg1, arg2 string) (io.ReadCloser, types.ContainerPathStat, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CopyFromContainer", arg0, arg1, arg2) + ret0, _ := ret[0].(io.ReadCloser) + ret1, _ := ret[1].(types.ContainerPathStat) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// CopyFromContainer indicates an expected call of CopyFromContainer. +func (mr *MockDockerClientMockRecorder) CopyFromContainer(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CopyFromContainer", reflect.TypeOf((*MockDockerClient)(nil).CopyFromContainer), arg0, arg1, arg2) +} + +// CopyToContainer mocks base method. +func (m *MockDockerClient) CopyToContainer(arg0 context.Context, arg1, arg2 string, arg3 io.Reader, arg4 types.CopyToContainerOptions) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CopyToContainer", arg0, arg1, arg2, arg3, arg4) + ret0, _ := ret[0].(error) + return ret0 +} + +// CopyToContainer indicates an expected call of CopyToContainer. +func (mr *MockDockerClientMockRecorder) CopyToContainer(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CopyToContainer", reflect.TypeOf((*MockDockerClient)(nil).CopyToContainer), arg0, arg1, arg2, arg3, arg4) +} + +// ImageBuild mocks base method. +func (m *MockDockerClient) ImageBuild(arg0 context.Context, arg1 io.Reader, arg2 types.ImageBuildOptions) (types.ImageBuildResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ImageBuild", arg0, arg1, arg2) + ret0, _ := ret[0].(types.ImageBuildResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ImageBuild indicates an expected call of ImageBuild. +func (mr *MockDockerClientMockRecorder) ImageBuild(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ImageBuild", reflect.TypeOf((*MockDockerClient)(nil).ImageBuild), arg0, arg1, arg2) +} + +// ImageInspectWithRaw mocks base method. +func (m *MockDockerClient) ImageInspectWithRaw(arg0 context.Context, arg1 string) (types.ImageInspect, []byte, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ImageInspectWithRaw", arg0, arg1) + ret0, _ := ret[0].(types.ImageInspect) + ret1, _ := ret[1].([]byte) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// ImageInspectWithRaw indicates an expected call of ImageInspectWithRaw. +func (mr *MockDockerClientMockRecorder) ImageInspectWithRaw(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ImageInspectWithRaw", reflect.TypeOf((*MockDockerClient)(nil).ImageInspectWithRaw), arg0, arg1) +} + +// ImageRemove mocks base method. +func (m *MockDockerClient) ImageRemove(arg0 context.Context, arg1 string, arg2 types.ImageRemoveOptions) ([]types.ImageDeleteResponseItem, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ImageRemove", arg0, arg1, arg2) + ret0, _ := ret[0].([]types.ImageDeleteResponseItem) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ImageRemove indicates an expected call of ImageRemove. +func (mr *MockDockerClientMockRecorder) ImageRemove(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ImageRemove", reflect.TypeOf((*MockDockerClient)(nil).ImageRemove), arg0, arg1, arg2) +} + +// ImageSave mocks base method. +func (m *MockDockerClient) ImageSave(arg0 context.Context, arg1 []string) (io.ReadCloser, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ImageSave", arg0, arg1) + ret0, _ := ret[0].(io.ReadCloser) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ImageSave indicates an expected call of ImageSave. +func (mr *MockDockerClientMockRecorder) ImageSave(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ImageSave", reflect.TypeOf((*MockDockerClient)(nil).ImageSave), arg0, arg1) +} + +// VolumeRemove mocks base method. +func (m *MockDockerClient) VolumeRemove(arg0 context.Context, arg1 string, arg2 bool) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "VolumeRemove", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// VolumeRemove indicates an expected call of VolumeRemove. +func (mr *MockDockerClientMockRecorder) VolumeRemove(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VolumeRemove", reflect.TypeOf((*MockDockerClient)(nil).VolumeRemove), arg0, arg1, arg2) +} diff --git a/internal/build/testdata/fake-tmp/build-extension/single/build/samples_test/Dockerfile b/internal/build/testdata/fake-tmp/build-extension/single/build/samples_test/Dockerfile new file mode 100644 index 0000000000..21e4d6eb47 --- /dev/null +++ b/internal/build/testdata/fake-tmp/build-extension/single/build/samples_test/Dockerfile @@ -0,0 +1,24 @@ +ARG base_image +FROM ${base_image} + +USER root +# Update and upgrade the Alpine packages +RUN apk update && apk upgrade + +# Install packages +RUN apk add --no-cache build-base openssl libffi-dev python3 tree + +# Print package versions +RUN gcc --version && openssl version && python3 --version + +# Remove packages +RUN apk del tree + +# Install additional packages +RUN apk add --no-cache postgresql-client git curl + +# Print package versions +RUN psql --version && git --version && curl --version + +# ENV variables +ENV ENV_VARIABLE=value diff --git a/internal/build/testdata/fake-tmp/build-extension/single/group.toml b/internal/build/testdata/fake-tmp/build-extension/single/group.toml new file mode 100644 index 0000000000..167a798713 --- /dev/null +++ b/internal/build/testdata/fake-tmp/build-extension/single/group.toml @@ -0,0 +1,11 @@ +[[group]] + id = "samples/hello-extensions" + version = "0.0.1" + api = "0.9" + homepage = "https://github.com/buildpacks/samples/tree/main/buildpacks/hello-extensions" + +[[group-extensions]] + id = "samples/test" + version = "0.0.1" + api = "0.9" + homepage = "https://github.com/buildpacks/samples/test/main/extensions/test" From 98fd87d1b56e320add305e6af97786b49a275583 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 16 Aug 2023 15:01:31 +0530 Subject: [PATCH 07/24] add tests for multiple buildextend Signed-off-by: Darshan Kumar --- internal/build/extend_build_test.go | 17 +++++++++++-- internal/build/lifecycle_execution.go | 12 ++++++---- .../multi/build/samples_test/Dockerfile | 24 +++++++++++++++++++ .../multi/build/samples_tree/Dockerfile | 5 ++++ .../fake-tmp/build-extension/multi/group.toml | 17 +++++++++++++ 5 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 internal/build/testdata/fake-tmp/build-extension/multi/build/samples_test/Dockerfile create mode 100644 internal/build/testdata/fake-tmp/build-extension/multi/build/samples_tree/Dockerfile create mode 100644 internal/build/testdata/fake-tmp/build-extension/multi/group.toml diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index c7fcc242e9..65cfa10ecf 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -43,7 +43,7 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { providedClearCache bool providedPublish bool providedBuilderImage = "some-registry.com/some-namespace/some-builder-name" - + extendedBuilderImage = "some-registry.com/some-namespace/some-builder-name-extended" configureDefaultTestLifecycle func(opts *build.LifecycleOptions) lifecycleOps []func(*build.LifecycleOptions) ) @@ -76,7 +76,7 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { expectedBuildOptions := types.ImageBuildOptions{ Context: expectedBuildContext, Dockerfile: "Dockerfile", - Tags: []string{"newbuilder-image"}, + Tags: []string{extendedBuilderImage}, Remove: true, BuildArgs: expectedbuildArguments, } @@ -90,6 +90,19 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { err := lifecycle.ExtendBuildByDaemon(context.Background()) h.AssertNil(t, err) }) + + it("should extend build image using multiple extension", func() { + // set tmp directory + tmpDir = "./testdata/fake-tmp/build-extension/multi" + lifecycle = getTestLifecycleExec(t, true, tmpDir, mockDockerClient, lifecycleOps...) + mockResponse := types.ImageBuildResponse{ + Body: io.NopCloser(strings.NewReader("mock-build-response-body")), + OSType: "linux", + } + mockDockerClient.EXPECT().ImageBuild(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockResponse, nil).Times(2) + err := lifecycle.ExtendBuildByDaemon(context.Background()) + h.AssertNil(t, err) + }) }) } diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index c491bac6da..4a893b4a5e 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -234,7 +234,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF group, _ := errgroup.WithContext(context.TODO()) if l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild() { - if l.opts.Publish { + if false { group.Go(func() error { l.logger.Info(style.Step("EXTENDING (BUILD)")) return l.ExtendBuild(ctx, buildCache, phaseFactory) @@ -673,7 +673,7 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor WithNetwork(l.opts.Network), WithBinds(l.opts.Volumes...), WithFlags(flags...), - If((!l.opts.Publish && l.hasExtensionsForBuild()), WithImage("newbuilder-image:latest")), + If((!l.opts.Publish && l.hasExtensionsForBuild()), WithImage(l.opts.BuilderImage+"-extended")), If((!l.opts.Publish && l.hasExtensionsForBuild()), WithoutPrivilege()), ) @@ -746,6 +746,7 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, ph func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { extendtime := time.Now() builderImageName := l.opts.BuilderImage + extendedBuilderImageName := l.opts.BuilderImage + "-extended" defaultFilterFunc := func(file string) bool { return true } var extensions Extensions time1 := time.Now() @@ -761,17 +762,19 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { for _, dockerfile := range dockerfiles { buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) buildArguments := map[string]*string{} + fmt.Println("builderImageName: ", builderImageName) + if dockerfile.WithBase == "" { buildArguments["base_image"] = &builderImageName } buildOptions := types.ImageBuildOptions{ Context: buildContext, Dockerfile: "Dockerfile", - Tags: []string{"newbuilder-image"}, + Tags: []string{extendedBuilderImageName}, Remove: true, BuildArgs: buildArguments, } - fmt.Println("buildContext: ", buildContext) + fmt.Println("buildContext: ", buildContext) response, err := l.docker.ImageBuild(ctx, buildContext, buildOptions) if err != nil { return err @@ -783,6 +786,7 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { return err } l.logger.Debugf("build response for the extend: %v", response) + builderImageName = l.opts.BuilderImage+"-extended" } l.logger.Debugf("docker apply time: %v", time.Since(dockerapplytime)) l.logger.Debugf("Build extend time: %v", time.Since(extendtime)) diff --git a/internal/build/testdata/fake-tmp/build-extension/multi/build/samples_test/Dockerfile b/internal/build/testdata/fake-tmp/build-extension/multi/build/samples_test/Dockerfile new file mode 100644 index 0000000000..f2e3fd88b5 --- /dev/null +++ b/internal/build/testdata/fake-tmp/build-extension/multi/build/samples_test/Dockerfile @@ -0,0 +1,24 @@ +ARG base_image +FROM ${base_image} + +USER root +# Update and upgrade the packages +RUN apk update && apk upgrade + +# Install packages +RUN apk add --no-cache build-base openssl libffi-dev python3 tree + +# Print package versions +RUN gcc --version && openssl version && python3 --version + +# Remove packages +RUN apk del tree + +# Install additional packages +RUN apk add --no-cache postgresql-client git curl + +# Print package versions +RUN psql --version && git --version && curl --version + +# ENV variables +ENV ENV_VARIABLE=value diff --git a/internal/build/testdata/fake-tmp/build-extension/multi/build/samples_tree/Dockerfile b/internal/build/testdata/fake-tmp/build-extension/multi/build/samples_tree/Dockerfile new file mode 100644 index 0000000000..3554737318 --- /dev/null +++ b/internal/build/testdata/fake-tmp/build-extension/multi/build/samples_tree/Dockerfile @@ -0,0 +1,5 @@ +ARG base_image +FROM ${base_image} + +USER root +RUN apk update && apk add tree diff --git a/internal/build/testdata/fake-tmp/build-extension/multi/group.toml b/internal/build/testdata/fake-tmp/build-extension/multi/group.toml new file mode 100644 index 0000000000..77bed0fb9e --- /dev/null +++ b/internal/build/testdata/fake-tmp/build-extension/multi/group.toml @@ -0,0 +1,17 @@ +[[group]] + id = "samples/hello-extensions" + version = "0.0.1" + api = "0.9" + homepage = "https://github.com/buildpacks/samples/tree/main/buildpacks/hello-extensions" + +[[group-extensions]] + id = "samples/tree" + version = "0.0.1" + api = "0.9" + homepage = "https://github.com/buildpacks/samples/tree/main/extensions/tree" + +[[group-extensions]] + id = "samples/test" + version = "0.0.1" + api = "0.9" + homepage = "https://github.com/buildpacks/samples/test/main/extensions/test" From 4ad4631dbd08d386e9aa08f8cb777d730f24a923 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Thu, 17 Aug 2023 16:31:09 +0530 Subject: [PATCH 08/24] adds functionality to fetch args from config Signed-off-by: Darshan Kumar --- internal/build/extend_build_test.go | 37 ++++++++++------- internal/build/helper.go | 53 +++++++++++++++++++++---- internal/build/lifecycle_execution.go | 32 ++++++++++----- internal/build/phase_config_provider.go | 5 ++- 4 files changed, 94 insertions(+), 33 deletions(-) diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index 65cfa10ecf..35f7be5acc 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -6,22 +6,25 @@ import ( "io" "path/filepath" + "strconv" "strings" "testing" "github.com/apex/log" + "github.com/buildpacks/pack/internal/build" "github.com/buildpacks/pack/internal/build/fakes" - "github.com/buildpacks/pack/pkg/archive" - "github.com/buildpacks/pack/pkg/logging" - h "github.com/buildpacks/pack/testhelpers" "github.com/docker/docker/api/types" "github.com/golang/mock/gomock" "github.com/heroku/color" "github.com/sclevine/spec" "github.com/sclevine/spec/report" + "github.com/buildpacks/pack/pkg/archive" + "github.com/buildpacks/pack/pkg/logging" + h "github.com/buildpacks/pack/testhelpers" + mockdocker "github.com/buildpacks/pack/internal/build/mockdocker" ) @@ -29,9 +32,13 @@ func TestBuildDockerfiles(t *testing.T) { color.Disable(true) defer color.Disable(false) spec.Run(t, "buildExtendByDocker", testBuildDockerfiles, spec.Report(report.Terminal{}), spec.Sequential()) - } +const ( + argUserID = "user_id" + argGroupID = "group_id" +) + func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { var ( mockDockerClient *mockdocker.MockDockerClient @@ -40,10 +47,10 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { tmpDir string // lifecycle options - providedClearCache bool - providedPublish bool - providedBuilderImage = "some-registry.com/some-namespace/some-builder-name" - extendedBuilderImage = "some-registry.com/some-namespace/some-builder-name-extended" + providedClearCache bool + providedPublish bool + providedBuilderImage = "some-registry.com/some-namespace/some-builder-name" + extendedBuilderImage = "some-registry.com/some-namespace/some-builder-name-extended" configureDefaultTestLifecycle func(opts *build.LifecycleOptions) lifecycleOps []func(*build.LifecycleOptions) ) @@ -61,7 +68,6 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { } lifecycleOps = []func(*build.LifecycleOptions){configureDefaultTestLifecycle} - }) when("Extend Build Image By Docker", func() { @@ -69,10 +75,15 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { // set tmp directory tmpDir = "./testdata/fake-tmp/build-extension/single" lifecycle = getTestLifecycleExec(t, true, tmpDir, mockDockerClient, lifecycleOps...) + expectedBuilder := lifecycle.Builder() expectedBuildContext := archive.ReadDirAsTar(filepath.Dir(filepath.Join(tmpDir, "fake-tmp", "build-extension", "single", "build", "samples_test", "Dockerfile")), "/", 0, 0, -1, true, false, func(file string) bool { return true }) // Set up expected Build Args + UID := strconv.Itoa(expectedBuilder.UID()) + GID := strconv.Itoa(expectedBuilder.GID()) expectedbuildArguments := map[string]*string{} expectedbuildArguments["base_image"] = &providedBuilderImage + expectedbuildArguments[argUserID] = &UID + expectedbuildArguments[argGroupID] = &GID expectedBuildOptions := types.ImageBuildOptions{ Context: expectedBuildContext, Dockerfile: "Dockerfile", @@ -104,17 +115,13 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) }) - } - func GetTestLifecycleExecErr(t *testing.T, logVerbose bool, tmpDir string, mockDockerClient *mockdocker.MockDockerClient, ops ...func(*build.LifecycleOptions)) (*build.LifecycleExecution, error) { - var outBuf bytes.Buffer logger := logging.NewLogWithWriters(&outBuf, &outBuf) if logVerbose { logger.Level = log.DebugLevel } - defaultBuilder, err := fakes.NewFakeBuilder() h.AssertNil(t, err) @@ -147,5 +154,7 @@ func compBuildOptions(t *testing.T, expectedBuildOptions types.ImageBuildOptions h.AssertEq(t, expectedBuildOptions.Dockerfile, actualBuildOptions.Dockerfile) h.AssertEq(t, expectedBuildOptions.Tags, actualBuildOptions.Tags) h.AssertEq(t, expectedBuildOptions.Remove, actualBuildOptions.Remove) - h.AssertEq(t, expectedBuildOptions.BuildArgs, actualBuildOptions.BuildArgs) + h.AssertEq(t, expectedBuildOptions.BuildArgs["base_image"], actualBuildOptions.BuildArgs["base_image"]) + h.AssertEq(t, expectedBuildOptions.BuildArgs[argUserID], actualBuildOptions.BuildArgs[argUserID]) + h.AssertEq(t, expectedBuildOptions.BuildArgs[argGroupID], actualBuildOptions.BuildArgs[argGroupID]) } diff --git a/internal/build/helper.go b/internal/build/helper.go index 9f8822eefd..4c2fc60737 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -29,8 +29,27 @@ type Extensions struct { Extensions []buildpack.GroupElement } -func (extensions *Extensions) DockerFiles(kind string, path string, logger logging.Logger) ([]buildpack.DockerfileInfo, error) { - var dockerfiles []buildpack.DockerfileInfo +type DockerfileInfo struct { + Info *buildpack.DockerfileInfo + Args []Arg +} + +type Arg struct { + Name string `toml:"name"` + Value string `toml:"value"` +} + +type Config struct { + Build BuildConfig `toml:"build"` + Run BuildConfig `toml:"run"` +} + +type BuildConfig struct { + Args []Arg `toml:"args"` +} + +func (extensions *Extensions) DockerFiles(kind string, path string, logger logging.Logger) ([]DockerfileInfo, error) { + var dockerfiles []DockerfileInfo for _, ext := range extensions.Extensions { dockerfile, err := extensions.ReadDockerFile(path, kind, ext.ID) if err != nil { @@ -42,7 +61,7 @@ func (extensions *Extensions) DockerFiles(kind string, path string, logger loggi case DockerfileKindBuild: // will implement later case DockerfileKindRun: - buildpack.ValidateRunDockerfile(dockerfile, logger) + buildpack.ValidateRunDockerfile(dockerfile.Info, logger) default: return nil, fmt.Errorf("unknown dockerfile kind: %s", kind) } @@ -52,15 +71,33 @@ func (extensions *Extensions) DockerFiles(kind string, path string, logger loggi return dockerfiles, nil } -func (extensions *Extensions) ReadDockerFile(path string, kind string, extID string) (*buildpack.DockerfileInfo, error) { +func (extensions *Extensions) ReadDockerFile(path string, kind string, extID string) (*DockerfileInfo, error) { dockerfilePath := filepath.Join(path, kind, escapeID(extID), "Dockerfile") if _, err := os.Stat(dockerfilePath); err != nil { return nil, nil } - return &buildpack.DockerfileInfo{ - ExtensionID: extID, - Kind: kind, - Path: dockerfilePath, + configPath := filepath.Join(path, kind, escapeID(extID), "extend-config.toml") + var config Config + _, err := toml.DecodeFile(configPath, &config) + if err != nil { + if !os.IsNotExist(err) { + return nil, err + } + } + + var args []Arg + if kind == buildpack.DockerfileKindBuild { + args = config.Build.Args + } else { + args = config.Run.Args + } + return &DockerfileInfo{ + Info: &buildpack.DockerfileInfo{ + ExtensionID: extID, + Kind: kind, + Path: dockerfilePath, + }, + Args: args, }, nil } diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 4a893b4a5e..6ea48436be 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -20,6 +20,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/daemon" + "github.com/google/uuid" "github.com/pkg/errors" "golang.org/x/sync/errgroup" @@ -674,7 +675,7 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor WithBinds(l.opts.Volumes...), WithFlags(flags...), If((!l.opts.Publish && l.hasExtensionsForBuild()), WithImage(l.opts.BuilderImage+"-extended")), - If((!l.opts.Publish && l.hasExtensionsForBuild()), WithoutPrivilege()), + If((!l.opts.Publish && l.hasExtensionsForBuild()), WithoutPrivilege(l.opts.Builder.UID(), l.opts.Builder.GID())), ) build := phaseFactory.New(configProvider) @@ -743,6 +744,13 @@ func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, ph defer extend.Cleanup() return extend.Run(ctx) } + +const ( + argBuildID = "build_id" + argUserID = "user_id" + argGroupID = "group_id" +) + func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { extendtime := time.Now() builderImageName := l.opts.BuilderImage @@ -760,12 +768,18 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { l.logger.Debugf("extensions.DockerFiles for build took %s", time.Since(time2)) dockerapplytime := time.Now() for _, dockerfile := range dockerfiles { - buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) + dockerfile.Args = append([]Arg{ + {Name: argBuildID, Value: uuid.New().String()}, + {Name: argUserID, Value: strconv.Itoa(l.opts.Builder.UID())}, + {Name: argGroupID, Value: strconv.Itoa(l.opts.Builder.GID())}, + }, dockerfile.Args...) + buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Info.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) buildArguments := map[string]*string{} fmt.Println("builderImageName: ", builderImageName) - - if dockerfile.WithBase == "" { - buildArguments["base_image"] = &builderImageName + buildArguments["base_image"] = &builderImageName + for i := range dockerfile.Args { + arg := &dockerfile.Args[i] + buildArguments[arg.Name] = &arg.Value } buildOptions := types.ImageBuildOptions{ Context: buildContext, @@ -786,7 +800,7 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { return err } l.logger.Debugf("build response for the extend: %v", response) - builderImageName = l.opts.BuilderImage+"-extended" + builderImageName = l.opts.BuilderImage + "-extended" } l.logger.Debugf("docker apply time: %v", time.Since(dockerapplytime)) l.logger.Debugf("Build extend time: %v", time.Since(extendtime)) @@ -824,10 +838,10 @@ func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, group *errgr }) dockerapplytime := time.Now() for _, dockerfile := range dockerfiles { - if dockerfile.Extend { - buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) + if dockerfile.Info.Extend { + buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Info.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) buildArguments := map[string]*string{} - if dockerfile.WithBase == "" { + if dockerfile.Info.WithBase == "" { buildArguments["base_image"] = currentRunImage } buildOptions := types.ImageBuildOptions{ diff --git a/internal/build/phase_config_provider.go b/internal/build/phase_config_provider.go index a6566858a2..ae402826ec 100644 --- a/internal/build/phase_config_provider.go +++ b/internal/build/phase_config_provider.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "os" + "strconv" "strings" "github.com/docker/docker/api/types/container" @@ -244,9 +245,9 @@ func WithRoot() PhaseConfigProviderOperation { } } -func WithoutPrivilege() PhaseConfigProviderOperation { +func WithoutPrivilege(uid int, gid int) PhaseConfigProviderOperation { return func(provider *PhaseConfigProvider) { - provider.ctrConf.User = "1000:1001" + provider.ctrConf.User = strconv.Itoa(uid) + ":" + strconv.Itoa(gid) } } From 94662041de96c7ddd2d017db1e955a9b23b259dd Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 23 Aug 2023 18:43:33 +0530 Subject: [PATCH 09/24] remove run extend Signed-off-by: Darshan Kumar --- internal/build/helper.go | 97 ------------------- internal/build/lifecycle_execution.go | 119 ++---------------------- internal/build/phase_config_provider.go | 2 +- 3 files changed, 7 insertions(+), 211 deletions(-) diff --git a/internal/build/helper.go b/internal/build/helper.go index 4c2fc60737..56f359b1de 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -2,20 +2,13 @@ package build import ( "fmt" - "io" "os" "path/filepath" "strings" - "time" "github.com/BurntSushi/toml" - "github.com/buildpacks/imgutil/layout/sparse" "github.com/buildpacks/lifecycle/buildpack" "github.com/buildpacks/lifecycle/cmd" - "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/daemon" - "golang.org/x/sync/errgroup" "github.com/buildpacks/pack/pkg/logging" ) @@ -132,93 +125,3 @@ func readExtensionsGroup(path string) ([]buildpack.GroupElement, error) { func escapeID(id string) string { return strings.ReplaceAll(id, "/", "_") } - -func SaveLayers(group *errgroup.Group, image v1.Image, origTopLayerHash string, dest string) (*time.Duration, error) { - var totalSaveExecutionTime time.Duration - savetime := time.Now() - layoutPath, err := sparse.NewImage(dest, image) - if err != nil { - fmt.Println("sparse.NewImage err", err) - return nil, err - } - if err = layoutPath.Save(); err != nil { - return nil, err - } - if err != nil { - fmt.Println("sparse.NewImage err", err) - return nil, err - } - layers, err := image.Layers() - if err != nil { - return nil, fmt.Errorf("getting image layers: %w", err) - } - var ( - currentHash v1.Hash - needsCopying bool - ) - if origTopLayerHash == "" { - needsCopying = true - } - for _, currentLayer := range layers { - currentHash, err = currentLayer.Digest() - if err != nil { - return nil, fmt.Errorf("getting layer hash: %w", err) - } - switch { - case needsCopying: - currentLayer := currentLayer - start := time.Now() - group.Go(func() error { - defer func() { - duration := time.Since(start) - totalSaveExecutionTime += duration - }() - return copyLayer(currentLayer, dest) - }) - case currentHash.String() == origTopLayerHash: - needsCopying = true - continue - default: - continue - } - } - totalSaveExecutionTime += time.Since(savetime) - return &totalSaveExecutionTime, nil -} - -func copyLayer(layer v1.Layer, toSparseImage string) error { - digest, err := layer.Digest() - if err != nil { - return err - } - f, err := os.Create(filepath.Join(toSparseImage, "blobs", digest.Algorithm, digest.Hex)) - if err != nil { - return err - } - defer f.Close() - rc, err := layer.Compressed() - if err != nil { - return err - } - defer rc.Close() - _, err = io.Copy(f, rc) - return err -} - -func topLayerHash(image *string) (string, error) { - baseRef, err := name.ParseReference(*image) - if err != nil { - return "", fmt.Errorf("failed to parse reference: %v", err) - } - baseImage, err := daemon.Image(baseRef) - if err != nil { - return "", fmt.Errorf("failed to get v1.Image: %v", err) - } - baseManifest, err := baseImage.Manifest() - if err != nil { - return "", fmt.Errorf("getting image manifest: %w", err) - } - baseLayers := baseManifest.Layers - topLayerHash := baseLayers[len(baseLayers)-1].Digest.String() - return topLayerHash, nil -} diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 6ea48436be..bf204d038f 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -19,7 +19,6 @@ import ( "github.com/docker/docker/api/types" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/v1/daemon" "github.com/google/uuid" "github.com/pkg/errors" "golang.org/x/sync/errgroup" @@ -266,24 +265,11 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF return err } } - var start time.Time if l.platformAPI.AtLeast("0.12") && l.hasExtensionsForRun() { - if l.opts.Publish { - group.Go(func() error { - l.logger.Info(style.Step("EXTENDING (RUN)")) - return l.ExtendRun(ctx, buildCache, phaseFactory) - }) - } else { - start = time.Now() - group.Go(func() error { - l.logger.Info(style.Step("EXTENDING (RUN) BY DAEMON")) - defer func() { - duration := time.Since(start) - fmt.Println("Execution time:", duration) - }() - return l.ExtendRunByDaemon(ctx, group, ¤tRunImage) - }) - } + group.Go(func() error { + l.logger.Info(style.Step("EXTENDING (RUN)")) + return l.ExtendRun(ctx, buildCache, phaseFactory) + }) } if err := group.Wait(); err != nil { return err @@ -445,8 +431,6 @@ func (l *LifecycleExecution) Detect(ctx context.Context, phaseFactory PhaseFacto CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "analyzed.toml"), l.tmpDir))), If(l.hasExtensions(), WithPostContainerRunOperations( CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "generated", "build"), l.tmpDir))), - If(l.hasExtensions(), WithPostContainerRunOperations( - CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "generated", "run"), l.tmpDir))), If(l.hasExtensions(), WithPostContainerRunOperations( CopyOutToMaybe(filepath.Join(l.mountPaths.layersDir(), "group.toml"), l.tmpDir))), envOp, @@ -675,7 +659,7 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor WithBinds(l.opts.Volumes...), WithFlags(flags...), If((!l.opts.Publish && l.hasExtensionsForBuild()), WithImage(l.opts.BuilderImage+"-extended")), - If((!l.opts.Publish && l.hasExtensionsForBuild()), WithoutPrivilege(l.opts.Builder.UID(), l.opts.Builder.GID())), + If((!l.opts.Publish && l.hasExtensionsForBuild()), WithUser(l.opts.Builder.UID(), l.opts.Builder.GID())), ) build := phaseFactory.New(configProvider) @@ -795,7 +779,7 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { } defer response.Body.Close() fmt.Println("build response for the extend: ", response.Body) - _, err = io.Copy(os.Stdout, response.Body) + _, err = io.Copy(l.logger.Writer(), response.Body) if err != nil { return err } @@ -808,89 +792,6 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { return nil } -func (l *LifecycleExecution) ExtendRunByDaemon(ctx context.Context, group *errgroup.Group, currentRunImage *string) error { - defaultFilterFunc := func(file string) bool { return true } - var extensions Extensions - l.logger.Debugf("extending run image %s", *currentRunImage) - time1 := time.Now() - extensions.SetExtensions(l.tmpDir, l.logger) - fmt.Println("extensions.SetExtensions took", time.Since(time1)) - time2 := time.Now() - dockerfiles, err := extensions.DockerFiles(DockerfileKindRun, l.tmpDir, l.logger) - if err != nil { - return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindRun, err) - } - fmt.Println("extensions.DockerFiles took", time.Since(time2)) - nestedCtx, cancel := context.WithCancel(ctx) - defer cancel() - nestedGroup, _ := errgroup.WithContext(nestedCtx) - var origTopLayerHash = "" - time3 := time.Now() - nestedGroup.Go(func() error { - defer func() { - fmt.Println("topLayerHash took", time.Since(time3)) - }() - origTopLayerHash, err = topLayerHash(currentRunImage) - if err != nil { - return fmt.Errorf("getting top layer hash of run image: %w", err) - } - return nil - }) - dockerapplytime := time.Now() - for _, dockerfile := range dockerfiles { - if dockerfile.Info.Extend { - buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Info.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) - buildArguments := map[string]*string{} - if dockerfile.Info.WithBase == "" { - buildArguments["base_image"] = currentRunImage - } - buildOptions := types.ImageBuildOptions{ - Context: buildContext, - Dockerfile: "Dockerfile", - Tags: []string{"run-image"}, - Remove: true, - BuildArgs: buildArguments, - } - response, err := l.docker.ImageBuild(ctx, buildContext, buildOptions) - if err != nil { - return err - } - defer response.Body.Close() - _, err = io.Copy(os.Stdout, response.Body) - if err != nil { - return err - } - l.logger.Debugf("build response for the extend: %v", response) - } - } - l.logger.Debugf("docker apply time: %v", time.Since(dockerapplytime)) - time4 := time.Now() - ref, err := name.ParseReference("run-image:latest") - if err != nil { - return fmt.Errorf("failed to parse reference: %v", err) - } - image, err := daemon.Image(ref) - if err != nil { - return fmt.Errorf("failed to get v1.Image: %v", err) - } - imageHash, err := image.Digest() - if err != nil { - return fmt.Errorf("getting image hash: %w", err) - } - dest := filepath.Join(l.tmpDir, "extended-new", "run", imageHash.String()) - fmt.Println("exporting to OCI took", time.Since(time4)) - waiterr := nestedGroup.Wait() - if waiterr != nil { - return err - } - var savetime *time.Duration - if savetime, err = SaveLayers(group, image, origTopLayerHash, dest); err != nil { - return fmt.Errorf("copying selective image to output directory: %w", err) - } - fmt.Println("Total Save execution time:", savetime) - return nil -} - func determineDefaultProcessType(platformAPI *api.Version, providedValue string) string { shouldSetForceDefault := platformAPI.Compare(api.MustParse("0.4")) >= 0 && platformAPI.Compare(api.MustParse("0.6")) < 0 @@ -917,12 +818,6 @@ func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache } } - extDirEnv := NullOp() - if !l.opts.Publish { - l.logger.Debug("export for extend by daemon") - extDirEnv = WithEnv("CNB_EXTENDED_DIR=" + filepath.Join("/", "extended-new")) - } - if l.platformAPI.LessThan("0.7") { flags = append(flags, "-run-image", l.opts.RunImage) } @@ -959,7 +854,6 @@ func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache ), WithArgs(append([]string{l.opts.Image.String()}, l.opts.AdditionalTags...)...), WithRoot(), - WithBinds(filepath.Join(l.tmpDir, "extended-new") + ":/extended-new"), WithNetwork(l.opts.Network), cacheBindOp, WithContainerOperations(WriteStackToml(l.mountPaths.stackPath(), l.opts.Builder.Stack(), l.os)), @@ -976,7 +870,6 @@ func (l *LifecycleExecution) Export(ctx context.Context, buildCache, launchCache CopyOut(l.opts.Termui.ReadLayers, l.mountPaths.layersDir(), l.mountPaths.appDir()))), epochEnv, expEnv, - extDirEnv, } var export RunnerCleaner diff --git a/internal/build/phase_config_provider.go b/internal/build/phase_config_provider.go index ae402826ec..a63f7903b5 100644 --- a/internal/build/phase_config_provider.go +++ b/internal/build/phase_config_provider.go @@ -245,7 +245,7 @@ func WithRoot() PhaseConfigProviderOperation { } } -func WithoutPrivilege(uid int, gid int) PhaseConfigProviderOperation { +func WithUser(uid int, gid int) PhaseConfigProviderOperation { return func(provider *PhaseConfigProvider) { provider.ctrConf.User = strconv.Itoa(uid) + ":" + strconv.Itoa(gid) } From 0e2fce6bc8cd76240a72c06db48311f959c8ee45 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 23 Aug 2023 19:11:58 +0530 Subject: [PATCH 10/24] remove profiling and add comments Signed-off-by: Darshan Kumar --- internal/build/lifecycle_execution.go | 57 ++++++++++++--------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index bf204d038f..86f8f8ddb8 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -8,7 +8,6 @@ import ( "os" "path/filepath" "strconv" - "time" "github.com/BurntSushi/toml" @@ -234,24 +233,18 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF group, _ := errgroup.WithContext(context.TODO()) if l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild() { - if false { - group.Go(func() error { - l.logger.Info(style.Step("EXTENDING (BUILD)")) - return l.ExtendBuild(ctx, buildCache, phaseFactory) - }) - } else { - group.Go(func() error { - l.logger.Info(style.Step("EXTENDING (BUILD) BY DAEMON")) - start := time.Now() - if err := l.ExtendBuildByDaemon(ctx); err != nil { - return err - } - l.Build(ctx, phaseFactory) - elapsed := time.Since(start) - l.logger.Debugf("EXTENDING (BUILD) took %s", elapsed) - return nil - }) - } + /* + [RFC #0105] - As decided, Pack should support build image extension with Docker #1623. We removed the previous implementation that was using kaniko in the extend lifecycle phase and shifted the implementation to use docker daemon to extend the build Image. As pack already has access to a daemon, it can apply the dockerfiles directly, saving the extended build base image in the daemon. Thus it will not need to use the extender phase of lifecycle. Additionally it dropped the requirement that the image being extended must be published to a registry. This implementation resulted us to have build Extension Improved by 87.3578% wrt kaniko implementation with caching and 20.5567% wrt kaniko implementation without caching. + + */ + group.Go(func() error { + l.logger.Info(style.Step("EXTENDING (BUILD) BY DAEMON")) + if err := l.ExtendBuildByDaemon(ctx); err != nil { + return err + } + l.Build(ctx, phaseFactory) + return nil + }) } else { group.Go(func() error { l.logger.Info(style.Step("BUILDING")) @@ -697,6 +690,10 @@ func (l *LifecycleExecution) ExtendBuild(ctx context.Context, buildCache Cache, return extend.Run(ctx) } +/* + Note: - Run Image Extension by docker daemon was much worse than kaniko because of saving layers on disk. +*/ + func (l *LifecycleExecution) ExtendRun(ctx context.Context, buildCache Cache, phaseFactory PhaseFactory) error { flags := []string{"-app", l.mountPaths.appDir(), "-kind", "run"} @@ -735,22 +732,26 @@ const ( argGroupID = "group_id" ) +/* + This implementation of ExtendBuildByDaemon is based on the RFC #0105 which uses docker daemon to extend the build Image instead of kaniko. + * Parsing the `group.toml` from the temp directory of buildpack and set the extensions. + * Reading the dockerfiles that were generated during the `generate` phase and also parsing the Arguments given by the user from + `extend-config.toml`. + * Using ImageBuild method of docker API client to extend the Image and save it to the daemon. + * Invoking Build phase of lifecycle by creating a container from the extended Image and dropping the privileges. + +*/ + func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { - extendtime := time.Now() builderImageName := l.opts.BuilderImage extendedBuilderImageName := l.opts.BuilderImage + "-extended" defaultFilterFunc := func(file string) bool { return true } var extensions Extensions - time1 := time.Now() extensions.SetExtensions(l.tmpDir, l.logger) - l.logger.Debugf("extensions.SetExtensions for build took %s", time.Since(time1)) - time2 := time.Now() dockerfiles, err := extensions.DockerFiles(DockerfileKindBuild, l.tmpDir, l.logger) if err != nil { return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindBuild, err) } - l.logger.Debugf("extensions.DockerFiles for build took %s", time.Since(time2)) - dockerapplytime := time.Now() for _, dockerfile := range dockerfiles { dockerfile.Args = append([]Arg{ {Name: argBuildID, Value: uuid.New().String()}, @@ -759,7 +760,6 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { }, dockerfile.Args...) buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Info.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) buildArguments := map[string]*string{} - fmt.Println("builderImageName: ", builderImageName) buildArguments["base_image"] = &builderImageName for i := range dockerfile.Args { arg := &dockerfile.Args[i] @@ -772,22 +772,17 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { Remove: true, BuildArgs: buildArguments, } - fmt.Println("buildContext: ", buildContext) response, err := l.docker.ImageBuild(ctx, buildContext, buildOptions) if err != nil { return err } defer response.Body.Close() - fmt.Println("build response for the extend: ", response.Body) _, err = io.Copy(l.logger.Writer(), response.Body) if err != nil { return err } - l.logger.Debugf("build response for the extend: %v", response) builderImageName = l.opts.BuilderImage + "-extended" } - l.logger.Debugf("docker apply time: %v", time.Since(dockerapplytime)) - l.logger.Debugf("Build extend time: %v", time.Since(extendtime)) return nil } From 888d1a82ec0dd4b83e28bb7f17374650bc8f0370 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Sat, 26 Aug 2023 09:51:49 +0530 Subject: [PATCH 11/24] fix logger for daemon extend Signed-off-by: Darshan Kumar --- internal/build/extend_build_test.go | 9 ++- internal/build/lifecycle_execution.go | 106 +++++++++++++------------- 2 files changed, 62 insertions(+), 53 deletions(-) diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index 35f7be5acc..42c90a0c8c 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -6,6 +6,7 @@ import ( "io" "path/filepath" + "runtime" "strconv" "strings" "testing" @@ -71,10 +72,14 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { }) when("Extend Build Image By Docker", func() { + if runtime.GOOS == "windows" { + t.Skip("Skipping test on Windows OS") + } + it("should extend build image using 1 extension", func() { // set tmp directory - tmpDir = "./testdata/fake-tmp/build-extension/single" - lifecycle = getTestLifecycleExec(t, true, tmpDir, mockDockerClient, lifecycleOps...) + filepath.Join() + lifecycle = getTestLifecycleExec(t, true, filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single"), mockDockerClient, lifecycleOps...) expectedBuilder := lifecycle.Builder() expectedBuildContext := archive.ReadDirAsTar(filepath.Dir(filepath.Join(tmpDir, "fake-tmp", "build-extension", "single", "build", "samples_test", "Dockerfile")), "/", 0, 0, -1, true, false, func(file string) bool { return true }) // Set up expected Build Args diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 23e4d8dfc1..cb2ea68ae8 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -683,8 +683,8 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor WithNetwork(l.opts.Network), WithBinds(l.opts.Volumes...), WithFlags(flags...), - If((!l.opts.Publish && l.hasExtensionsForBuild()), WithImage(l.opts.BuilderImage+"-extended")), - If((!l.opts.Publish && l.hasExtensionsForBuild()), WithUser(l.opts.Builder.UID(), l.opts.Builder.GID())), + If((l.hasExtensionsForBuild()), WithImage(l.opts.BuilderImage+"-extended")), + If((l.hasExtensionsForBuild()), WithUser(l.opts.Builder.UID(), l.opts.Builder.GID())), ) build := phaseFactory.New(configProvider) @@ -692,54 +692,6 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor return build.Run(ctx) } -func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error { - flags := []string{"-app", l.mountPaths.appDir()} - - configProvider := NewPhaseConfigProvider( - "extender", - l, - WithLogPrefix("extender (build)"), - WithArgs(l.withLogLevel()...), - WithBinds(l.opts.Volumes...), - WithEnv("CNB_EXPERIMENTAL_MODE=warn"), - WithFlags(flags...), - WithNetwork(l.opts.Network), - WithRoot(), - WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())), - ) - - extend := phaseFactory.New(configProvider) - defer extend.Cleanup() - return extend.Run(ctx) -} - -/* - Note: - Run Image Extension by docker daemon was much worse than kaniko because of saving layers on disk. -*/ - -func (l *LifecycleExecution) ExtendRun(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error { - flags := []string{"-app", l.mountPaths.appDir(), "-kind", "run"} - - configProvider := NewPhaseConfigProvider( - "extender", - l, - WithLogPrefix("extender (run)"), - WithArgs(l.withLogLevel()...), - WithBinds(l.opts.Volumes...), - WithEnv("CNB_EXPERIMENTAL_MODE=warn"), - WithFlags(flags...), - WithNetwork(l.opts.Network), - WithRoot(), - WithImage(l.runImageAfterExtensions()), - WithBinds(fmt.Sprintf("%s:%s", filepath.Join(l.tmpDir, "cnb"), l.mountPaths.cnbDir())), - WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())), - ) - - extend := phaseFactory.New(configProvider) - defer extend.Cleanup() - return extend.Run(ctx) -} - const ( argBuildID = "build_id" argUserID = "user_id" @@ -791,7 +743,7 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { return err } defer response.Body.Close() - _, err = io.Copy(l.logger.Writer(), response.Body) + _, err = io.Copy(logging.NewPrefixWriter(logging.GetWriterForLevel(l.logger, logging.InfoLevel), "extender (build)"), response.Body) if err != nil { return err } @@ -801,6 +753,58 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { return nil } +/* + Deprecated: Check RFC #0105 for the new implementation of ExtendBuild using docker daemon #1623. +*/ + +func (l *LifecycleExecution) ExtendBuild(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error { + flags := []string{"-app", l.mountPaths.appDir()} + + configProvider := NewPhaseConfigProvider( + "extender", + l, + WithLogPrefix("extender (build)"), + WithArgs(l.withLogLevel()...), + WithBinds(l.opts.Volumes...), + WithEnv("CNB_EXPERIMENTAL_MODE=warn"), + WithFlags(flags...), + WithNetwork(l.opts.Network), + WithRoot(), + WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())), + ) + + extend := phaseFactory.New(configProvider) + defer extend.Cleanup() + return extend.Run(ctx) +} + +/* + Note: - Run Image Extension by docker daemon was much worse than kaniko because of saving layers on disk. +*/ + +func (l *LifecycleExecution) ExtendRun(ctx context.Context, kanikoCache Cache, phaseFactory PhaseFactory) error { + flags := []string{"-app", l.mountPaths.appDir(), "-kind", "run"} + + configProvider := NewPhaseConfigProvider( + "extender", + l, + WithLogPrefix("extender (run)"), + WithArgs(l.withLogLevel()...), + WithBinds(l.opts.Volumes...), + WithEnv("CNB_EXPERIMENTAL_MODE=warn"), + WithFlags(flags...), + WithNetwork(l.opts.Network), + WithRoot(), + WithImage(l.runImageAfterExtensions()), + WithBinds(fmt.Sprintf("%s:%s", filepath.Join(l.tmpDir, "cnb"), l.mountPaths.cnbDir())), + WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())), + ) + + extend := phaseFactory.New(configProvider) + defer extend.Cleanup() + return extend.Run(ctx) +} + func determineDefaultProcessType(platformAPI *api.Version, providedValue string) string { shouldSetForceDefault := platformAPI.Compare(api.MustParse("0.4")) >= 0 && platformAPI.Compare(api.MustParse("0.6")) < 0 From f935dd23c0caa0d5c0bc3504c5c36613ebe90cdf Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Sat, 26 Aug 2023 10:33:37 +0530 Subject: [PATCH 12/24] fix tests Signed-off-by: Darshan Kumar --- internal/build/lifecycle_execution_test.go | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index 8dc2d49047..deb6dab5ac 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -584,20 +584,8 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { platformAPI = api.MustParse("0.10") it("runs the extender (build)", func() { - err := lifecycle.Run(context.Background(), func(execution *build.LifecycleExecution) build.PhaseFactory { - return fakePhaseFactory - }) - h.AssertNil(t, err) - - h.AssertEq(t, len(fakePhaseFactory.NewCalledWithProvider), 5) - - var found bool - for _, entry := range fakePhaseFactory.NewCalledWithProvider { - if entry.Name() == "extender" { - found = true - } - } - h.AssertEq(t, found, true) + // Add tests from mock docker daemon + testBuildDockerfiles(t, when, it) }) }) }) From 38d0278a54b8c3a08149fed099c41bfd05f3902a Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 30 Aug 2023 09:09:15 +0530 Subject: [PATCH 13/24] add tarwriter for single file and include workspace in context Signed-off-by: Darshan Kumar --- internal/build/extend_build_test.go | 18 +++++++++----- internal/build/helper.go | 32 ++++++++++++++++++++++++ internal/build/lifecycle_execution.go | 7 +++--- pkg/archive/archive.go | 35 ++++++++++++++++++++++++--- 4 files changed, 79 insertions(+), 13 deletions(-) diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index 42c90a0c8c..5663a30f46 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -13,6 +13,8 @@ import ( "github.com/apex/log" + "github.com/buildpacks/lifecycle/buildpack" + "github.com/buildpacks/pack/internal/build" "github.com/buildpacks/pack/internal/build/fakes" @@ -22,7 +24,6 @@ import ( "github.com/sclevine/spec" "github.com/sclevine/spec/report" - "github.com/buildpacks/pack/pkg/archive" "github.com/buildpacks/pack/pkg/logging" h "github.com/buildpacks/pack/testhelpers" @@ -78,10 +79,15 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { it("should extend build image using 1 extension", func() { // set tmp directory - filepath.Join() - lifecycle = getTestLifecycleExec(t, true, filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single"), mockDockerClient, lifecycleOps...) + tmpDir = filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single") + lifecycle = getTestLifecycleExec(t, true, tmpDir, mockDockerClient, lifecycleOps...) + dockerfile := build.DockerfileInfo{ + Info: &buildpack.DockerfileInfo{ + Path: filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single", "build", "samples_test", "Dockerfile"), + }, + } expectedBuilder := lifecycle.Builder() - expectedBuildContext := archive.ReadDirAsTar(filepath.Dir(filepath.Join(tmpDir, "fake-tmp", "build-extension", "single", "build", "samples_test", "Dockerfile")), "/", 0, 0, -1, true, false, func(file string) bool { return true }) + expectedBuildContext, _ := dockerfile.CreateBuildContext(lifecycle.AppPath()) // Set up expected Build Args UID := strconv.Itoa(expectedBuilder.UID()) GID := strconv.Itoa(expectedBuilder.GID()) @@ -100,7 +106,7 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { Body: io.NopCloser(strings.NewReader("mock-build-response-body")), OSType: "linux", } - mockDockerClient.EXPECT().ImageBuild(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, buildContext io.ReadCloser, buildOptions types.ImageBuildOptions) { + mockDockerClient.EXPECT().ImageBuild(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, buildContext io.Reader, buildOptions types.ImageBuildOptions) { compBuildOptions(t, expectedBuildOptions, buildOptions) }).Return(mockResponse, nil).Times(1) err := lifecycle.ExtendBuildByDaemon(context.Background()) @@ -109,7 +115,7 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { it("should extend build image using multiple extension", func() { // set tmp directory - tmpDir = "./testdata/fake-tmp/build-extension/multi" + tmpDir = filepath.Join(".", "testdata", "fake-tmp", "build-extension", "multi") lifecycle = getTestLifecycleExec(t, true, tmpDir, mockDockerClient, lifecycleOps...) mockResponse := types.ImageBuildResponse{ Body: io.NopCloser(strings.NewReader("mock-build-response-body")), diff --git a/internal/build/helper.go b/internal/build/helper.go index 56f359b1de..23b0cb079e 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -1,7 +1,10 @@ package build import ( + "archive/tar" + "bytes" "fmt" + "io" "os" "path/filepath" "strings" @@ -10,6 +13,8 @@ import ( "github.com/buildpacks/lifecycle/buildpack" "github.com/buildpacks/lifecycle/cmd" + "github.com/buildpacks/pack/pkg/archive" + "github.com/buildpacks/pack/pkg/logging" ) @@ -125,3 +130,30 @@ func readExtensionsGroup(path string) ([]buildpack.GroupElement, error) { func escapeID(id string) string { return strings.ReplaceAll(id, "/", "_") } + +func (dockerfile *DockerfileInfo) CreateBuildContext(path string) (io.Reader, error) { + defaultFilterFunc := func(file string) bool { return true } + buf := new(bytes.Buffer) + tarWriter := tar.NewWriter(buf) + var completeErr error + + defer func() { + if err := tarWriter.Close(); err != nil { + fmt.Println("Error closing tar writer:", err) + completeErr = archive.AggregateError(completeErr, err) + } + }() + if err := archive.WriteDirToTar(tarWriter, path, "/workspace", 0, 0, -1, true, false, defaultFilterFunc); err != nil { + tarWriter.Close() + fmt.Println("Error adding workspace:", err) + completeErr = archive.AggregateError(completeErr, err) + } + + if err := archive.WriteFileToTar(tarWriter, dockerfile.Info.Path, filepath.Join(".", "Dockerfile"), 0, 0, -1, true); err != nil { + tarWriter.Close() + fmt.Println("Error adding dockerfile:", err) + completeErr = archive.AggregateError(completeErr, err) + } + + return buf, completeErr +} diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index cb2ea68ae8..40b714ca1c 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -24,7 +24,6 @@ import ( "github.com/buildpacks/pack/internal/builder" "github.com/buildpacks/pack/internal/paths" "github.com/buildpacks/pack/internal/style" - "github.com/buildpacks/pack/pkg/archive" "github.com/buildpacks/pack/pkg/cache" "github.com/buildpacks/pack/pkg/logging" ) @@ -711,7 +710,6 @@ const ( func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { builderImageName := l.opts.BuilderImage extendedBuilderImageName := l.opts.BuilderImage + "-extended" - defaultFilterFunc := func(file string) bool { return true } var extensions Extensions extensions.SetExtensions(l.tmpDir, l.logger) dockerfiles, err := extensions.DockerFiles(DockerfileKindBuild, l.tmpDir, l.logger) @@ -724,13 +722,16 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { {Name: argUserID, Value: strconv.Itoa(l.opts.Builder.UID())}, {Name: argGroupID, Value: strconv.Itoa(l.opts.Builder.GID())}, }, dockerfile.Args...) - buildContext := archive.ReadDirAsTar(filepath.Dir(dockerfile.Info.Path), "/", 0, 0, -1, true, false, defaultFilterFunc) buildArguments := map[string]*string{} buildArguments["base_image"] = &builderImageName for i := range dockerfile.Args { arg := &dockerfile.Args[i] buildArguments[arg.Name] = &arg.Value } + buildContext, err := dockerfile.CreateBuildContext(l.opts.AppPath) + if err != nil { + return err + } buildOptions := types.ImageBuildOptions{ Context: buildContext, Dockerfile: "Dockerfile", diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index 9affb3a3cc..e8d3082582 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -78,7 +78,7 @@ func GenerateTarWithWriter(genFn func(TarWriter) error, twf TarWriterFactory) io err := genFn(tw) closeErr := tw.Close() - closeErr = aggregateError(closeErr, pw.CloseWithError(err)) + closeErr = AggregateError(closeErr, pw.CloseWithError(err)) errChan <- closeErr }() @@ -94,12 +94,12 @@ func GenerateTarWithWriter(genFn func(TarWriter) error, twf TarWriterFactory) io // closing the reader ensures that if anything attempts // further reading it doesn't block waiting for content if err := pr.Close(); err != nil { - completeErr = aggregateError(completeErr, err) + completeErr = AggregateError(completeErr, err) } // wait until everything closes properly if err := <-errChan; err != nil { - completeErr = aggregateError(completeErr, err) + completeErr = AggregateError(completeErr, err) } closed = true @@ -107,7 +107,7 @@ func GenerateTarWithWriter(genFn func(TarWriter) error, twf TarWriterFactory) io }) } -func aggregateError(base, addition error) error { +func AggregateError(base, addition error) error { if addition == nil { return base } @@ -319,6 +319,33 @@ func WriteZipToTar(tw TarWriter, srcZip, basePath string, uid, gid int, mode int return nil } +func WriteFileToTar(tw TarWriter, srcFile, destPath string, uid, gid int, mode int64, normalizeModTime bool) error { + f, err := os.Open(srcFile) + if err != nil { + return err + } + defer f.Close() + + fi, err := f.Stat() + if err != nil { + return err + } + + header, err := tar.FileInfoHeader(fi, "") + if err != nil { + return err + } + + header.Name = destPath + err = writeHeader(header, uid, gid, mode, normalizeModTime, tw) + if err != nil { + return err + } + + _, err = io.Copy(tw, f) + return err +} + // NormalizeHeader normalizes a tar.Header // // Normalizes the following: From aa78c43fc9ae44c0536ed14e371ae95691b83974 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 30 Aug 2023 09:56:12 +0530 Subject: [PATCH 14/24] skip test for windows Signed-off-by: Darshan Kumar --- internal/build/extend_build_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index 5663a30f46..391a50e2d7 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -58,6 +58,7 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { ) it.Before(func() { + h.SkipIf(t, runtime.GOOS == "windows", "extensions not supported on windows") var err error mockController = gomock.NewController(t) mockDockerClient = mockdocker.NewMockDockerClient(mockController) @@ -73,10 +74,6 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { }) when("Extend Build Image By Docker", func() { - if runtime.GOOS == "windows" { - t.Skip("Skipping test on Windows OS") - } - it("should extend build image using 1 extension", func() { // set tmp directory tmpDir = filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single") From 57f88a21dba17e7902798fbfd10d29c8b7b8436f Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 30 Aug 2023 10:19:14 +0530 Subject: [PATCH 15/24] remove redundant logs Signed-off-by: Darshan Kumar --- internal/build/helper.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/build/helper.go b/internal/build/helper.go index 23b0cb079e..73e3f8957b 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -57,7 +57,7 @@ func (extensions *Extensions) DockerFiles(kind string, path string, logger loggi logger.Debugf("Found %s Dockerfile for extension '%s'", kind, ext.ID) switch kind { case DockerfileKindBuild: - // will implement later + break case DockerfileKindRun: buildpack.ValidateRunDockerfile(dockerfile.Info, logger) default: @@ -113,7 +113,6 @@ func (extensions *Extensions) SetExtensions(path string, logger logging.Logger) } } extensions.Extensions = groupExt - fmt.Println("extensions.Extensions", extensions.Extensions) return nil } From 72c6fdef37b84d304189dfcd7ace390907473073 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Sun, 8 Oct 2023 06:38:27 +0530 Subject: [PATCH 16/24] add tests for helper functions Signed-off-by: Darshan Kumar --- internal/build/helper_test.go | 174 ++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 internal/build/helper_test.go diff --git a/internal/build/helper_test.go b/internal/build/helper_test.go new file mode 100644 index 0000000000..6308d69981 --- /dev/null +++ b/internal/build/helper_test.go @@ -0,0 +1,174 @@ +package build_test + +import ( + "archive/tar" + "bytes" + "io" + "path/filepath" + "testing" + + "github.com/buildpacks/lifecycle/buildpack" + "github.com/heroku/color" + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + + "github.com/buildpacks/pack/internal/build" + "github.com/buildpacks/pack/pkg/logging" + h "github.com/buildpacks/pack/testhelpers" +) + +func TestHelper(t *testing.T) { + color.Disable(true) + defer color.Disable(false) + spec.Run(t, "helperForBuildExtension", testHelper, spec.Report(report.Terminal{}), spec.Sequential()) +} + +func testHelper(t *testing.T, when spec.G, it spec.S) { + var ( + tmpDir string + extensions build.Extensions + logger *logging.LogWithWriters + ) + it.Before(func() { + var outBuf bytes.Buffer + logger = logging.NewLogWithWriters(&outBuf, &outBuf) + }) + when("set-extensions", func() { + it("should set single extension", func() { + tmpDir = filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single") + expectedExtension := build.Extensions{ + Extensions: []buildpack.GroupElement{ + { + ID: "samples/test", + Version: "0.0.1", + API: "0.9", + Homepage: "https://github.com/buildpacks/samples/test/main/extensions/test", + }, + }, + } + extensions.SetExtensions(tmpDir, logger) + h.AssertEq(t, extensions.Extensions[0].ID, expectedExtension.Extensions[0].ID) + h.AssertEq(t, extensions.Extensions[0].Version, expectedExtension.Extensions[0].Version) + h.AssertEq(t, extensions.Extensions[0].API, expectedExtension.Extensions[0].API) + h.AssertEq(t, extensions.Extensions[0].Homepage, expectedExtension.Extensions[0].Homepage) + }) + it("should set multiple extensions", func() { + tmpDir = filepath.Join(".", "testdata", "fake-tmp", "build-extension", "multi") + expectedExtension := build.Extensions{ + Extensions: []buildpack.GroupElement{ + { + ID: "samples/tree", + Version: "0.0.1", + API: "0.9", + Homepage: "https://github.com/buildpacks/samples/tree/main/extensions/tree", + }, + { + ID: "samples/test", + Version: "0.0.1", + API: "0.9", + Homepage: "https://github.com/buildpacks/samples/test/main/extensions/test", + }, + }} + extensions.SetExtensions(tmpDir, logger) + h.AssertEq(t, extensions.Extensions[0].ID, expectedExtension.Extensions[0].ID) + h.AssertEq(t, extensions.Extensions[0].Version, expectedExtension.Extensions[0].Version) + h.AssertEq(t, extensions.Extensions[0].API, expectedExtension.Extensions[0].API) + h.AssertEq(t, extensions.Extensions[0].Homepage, expectedExtension.Extensions[0].Homepage) + h.AssertEq(t, extensions.Extensions[1].ID, expectedExtension.Extensions[1].ID) + h.AssertEq(t, extensions.Extensions[1].Version, expectedExtension.Extensions[1].Version) + h.AssertEq(t, extensions.Extensions[1].API, expectedExtension.Extensions[1].API) + h.AssertEq(t, extensions.Extensions[1].Homepage, expectedExtension.Extensions[1].Homepage) + }) + }) + + when("set dockerfiles", func() { + it("should set dockerfiles for single extension", func() { + tmpDir = filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single") + expectedDockerfile := build.DockerfileInfo{ + Info: &buildpack.DockerfileInfo{ + ExtensionID: "samples/test", + Kind: build.DockerfileKindBuild, + Path: filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single", "build", "samples_test", "Dockerfile"), + }, + } + extensions.SetExtensions(tmpDir, logger) + dockerfiles, err := extensions.DockerFiles(build.DockerfileKindBuild, tmpDir, logger) + h.AssertNil(t, err) + h.AssertEq(t, dockerfiles[0].Info.ExtensionID, expectedDockerfile.Info.ExtensionID) + h.AssertEq(t, dockerfiles[0].Info.Kind, expectedDockerfile.Info.Kind) + h.AssertEq(t, dockerfiles[0].Info.Path, expectedDockerfile.Info.Path) + }) + it("should set dockerfiles for multiple extensions", func() { + tmpDir = filepath.Join(".", "testdata", "fake-tmp", "build-extension", "multi") + expectedDockerfiles := []build.DockerfileInfo{ + { + Info: &buildpack.DockerfileInfo{ + ExtensionID: "samples/tree", + Kind: build.DockerfileKindBuild, + Path: filepath.Join(".", "testdata", "fake-tmp", "build-extension", "multi", "build", "samples_tree", "Dockerfile"), + }, + }, + { + Info: &buildpack.DockerfileInfo{ + ExtensionID: "samples/test", + Kind: build.DockerfileKindBuild, + Path: filepath.Join(".", "testdata", "fake-tmp", "build-extension", "multi", "build", "samples_test", "Dockerfile"), + }, + }, + } + extensions.SetExtensions(tmpDir, logger) + dockerfiles, err := extensions.DockerFiles(build.DockerfileKindBuild, tmpDir, logger) + h.AssertNil(t, err) + h.AssertEq(t, dockerfiles[0].Info.ExtensionID, expectedDockerfiles[0].Info.ExtensionID) + h.AssertEq(t, dockerfiles[0].Info.Kind, expectedDockerfiles[0].Info.Kind) + h.AssertEq(t, dockerfiles[0].Info.Path, expectedDockerfiles[0].Info.Path) + h.AssertEq(t, dockerfiles[1].Info.ExtensionID, expectedDockerfiles[1].Info.ExtensionID) + h.AssertEq(t, dockerfiles[1].Info.Kind, expectedDockerfiles[1].Info.Kind) + h.AssertEq(t, dockerfiles[1].Info.Path, expectedDockerfiles[1].Info.Path) + }) + }) + + when("create build context", func() { + it("should create build context", func() { + tmpDir = filepath.Join(".", "testdata", "fake-tmp", "build-extension", "single") + extensions.SetExtensions(tmpDir, logger) + dockerfiles, err := extensions.DockerFiles(build.DockerfileKindBuild, tmpDir, logger) + h.AssertNil(t, err) + buildContext, err := dockerfiles[0].CreateBuildContext(tmpDir) + h.AssertNil(t, err) + tr := tar.NewReader(buildContext) + checkDirectoryInTar(t, tr, "/workspace/build") + checkFileInTar(t, tr, "Dockerfile") + }) + }) +} + +func checkDirectoryInTar(t *testing.T, tr *tar.Reader, directoryName string) { + for { + header, err := tr.Next() + if err == io.EOF { + t.Fatalf("directory %s not found", directoryName) + } + if err != nil { + t.Fatal(err) + } + if header.Name == directoryName && header.Typeflag == tar.TypeDir { + return + } + } +} + +func checkFileInTar(t *testing.T, tr *tar.Reader, fileName string) { + for { + header, err := tr.Next() + if err == io.EOF { + t.Fatalf("file %s not found", fileName) + } + if err != nil { + t.Fatal(err) + } + if header.Name == fileName && header.Typeflag == tar.TypeReg { + return + } + } +} From 3ca6ad194bb86104b01eea77d098dcd4e3737192 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Sat, 16 Dec 2023 02:00:52 +0530 Subject: [PATCH 17/24] adds test for file-to-tar and fixes loggers Signed-off-by: Darshan Kumar --- internal/build/extend_build_test.go | 3 +- internal/build/helper.go | 8 ++--- internal/build/helper_test.go | 2 +- internal/build/lifecycle_execution.go | 6 +++- pkg/archive/archive_test.go | 47 +++++++++++++++++++++++++++ pkg/archive/testdata/file-to-tar.txt | 1 + 6 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 pkg/archive/testdata/file-to-tar.txt diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index 391a50e2d7..0e3cbc768d 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -84,7 +84,8 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { }, } expectedBuilder := lifecycle.Builder() - expectedBuildContext, _ := dockerfile.CreateBuildContext(lifecycle.AppPath()) + expectedBuildContext, _ := dockerfile. + CreateBuildContext(lifecycle.AppPath(), lifecycle.GetLogger()) // Set up expected Build Args UID := strconv.Itoa(expectedBuilder.UID()) GID := strconv.Itoa(expectedBuilder.GID()) diff --git a/internal/build/helper.go b/internal/build/helper.go index 73e3f8957b..e4afcef87d 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -130,7 +130,7 @@ func escapeID(id string) string { return strings.ReplaceAll(id, "/", "_") } -func (dockerfile *DockerfileInfo) CreateBuildContext(path string) (io.Reader, error) { +func (dockerfile *DockerfileInfo) CreateBuildContext(path string, logger logging.Logger) (io.Reader, error) { defaultFilterFunc := func(file string) bool { return true } buf := new(bytes.Buffer) tarWriter := tar.NewWriter(buf) @@ -138,19 +138,19 @@ func (dockerfile *DockerfileInfo) CreateBuildContext(path string) (io.Reader, er defer func() { if err := tarWriter.Close(); err != nil { - fmt.Println("Error closing tar writer:", err) + logger.Errorf("Error closing tar writer: %s", err) completeErr = archive.AggregateError(completeErr, err) } }() if err := archive.WriteDirToTar(tarWriter, path, "/workspace", 0, 0, -1, true, false, defaultFilterFunc); err != nil { tarWriter.Close() - fmt.Println("Error adding workspace:", err) + logger.Errorf("Error adding workspace: %s", err) completeErr = archive.AggregateError(completeErr, err) } if err := archive.WriteFileToTar(tarWriter, dockerfile.Info.Path, filepath.Join(".", "Dockerfile"), 0, 0, -1, true); err != nil { tarWriter.Close() - fmt.Println("Error adding dockerfile:", err) + logger.Errorf("Error adding dockerfile: %s", err) completeErr = archive.AggregateError(completeErr, err) } diff --git a/internal/build/helper_test.go b/internal/build/helper_test.go index 6308d69981..b684654086 100644 --- a/internal/build/helper_test.go +++ b/internal/build/helper_test.go @@ -134,7 +134,7 @@ func testHelper(t *testing.T, when spec.G, it spec.S) { extensions.SetExtensions(tmpDir, logger) dockerfiles, err := extensions.DockerFiles(build.DockerfileKindBuild, tmpDir, logger) h.AssertNil(t, err) - buildContext, err := dockerfiles[0].CreateBuildContext(tmpDir) + buildContext, err := dockerfiles[0].CreateBuildContext(tmpDir, logger) h.AssertNil(t, err) tr := tar.NewReader(buildContext) checkDirectoryInTar(t, tr, "/workspace/build") diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 40340fb251..421229f89a 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -743,7 +743,7 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { arg := &dockerfile.Args[i] buildArguments[arg.Name] = &arg.Value } - buildContext, err := dockerfile.CreateBuildContext(l.opts.AppPath) + buildContext, err := dockerfile.CreateBuildContext(l.opts.AppPath, l.logger) if err != nil { return err } @@ -1007,6 +1007,10 @@ func (l *LifecycleExecution) appendLayoutOperations(opts []PhaseConfigProviderOp return opts, nil } +func (l *LifecycleExecution) GetLogger() logging.Logger { + return l.logger +} + func withLayoutOperation() PhaseConfigProviderOperation { layoutDir := filepath.Join(paths.RootDir, "layout-repo") return WithEnv("CNB_USE_LAYOUT=true", "CNB_LAYOUT_DIR="+layoutDir, "CNB_EXPERIMENTAL_MODE=warn") diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go index 6eeaa54312..4f33a4d702 100644 --- a/pkg/archive/archive_test.go +++ b/pkg/archive/archive_test.go @@ -593,6 +593,53 @@ func testArchive(t *testing.T, when spec.G, it spec.S) { }) }) + when("#WrtieFileToTar", func() { + var src string + it.Before(func() { + src = filepath.Join("testdata", "file-to-tar.txt") + }) + + when("mode is set to 0777", func() { + it("writes a tar to the dest dir with 0777", func() { + fh, err := os.Create(filepath.Join(tmpDir, "some.tar")) + h.AssertNil(t, err) + + tw := tar.NewWriter(fh) + + err = archive.WriteFileToTar(tw, src, "/nested/dir/dir-in-archive/file-to-tar.txt", 1234, 2345, 0777, true) + h.AssertNil(t, err) + h.AssertNil(t, tw.Close()) + h.AssertNil(t, fh.Close()) + + file, err := os.Open(filepath.Join(tmpDir, "some.tar")) + h.AssertNil(t, err) + defer file.Close() + + tr := tar.NewReader(file) + + verify := h.NewTarVerifier(t, tr, 1234, 2345) + verify.NextFile("/nested/dir/dir-in-archive/file-to-tar.txt", "Hi I love CNB!", 0777) + }) + }) + + when("normalize mod time is false", func() { + it("does not normalize mod times", func() { + tarFile := filepath.Join(tmpDir, "some.tar") + fh, err := os.Create(tarFile) + h.AssertNil(t, err) + + tw := tar.NewWriter(fh) + + err = archive.WriteFileToTar(tw, src, "/foo/file-to-tar.txt", 1234, 2345, 0777, false) + h.AssertNil(t, err) + h.AssertNil(t, tw.Close()) + h.AssertNil(t, fh.Close()) + + h.AssertOnTarEntry(t, tarFile, "/foo/file-to-tar.txt", h.DoesNotHaveModTime(archive.NormalizedDateTime)) + }) + }) + }) + when("#IsZip", func() { when("file is a zip file", func() { it("returns true", func() { diff --git a/pkg/archive/testdata/file-to-tar.txt b/pkg/archive/testdata/file-to-tar.txt new file mode 100644 index 0000000000..4145eed435 --- /dev/null +++ b/pkg/archive/testdata/file-to-tar.txt @@ -0,0 +1 @@ +Hi I love CNB! \ No newline at end of file From e401acdcb1d41114e5d55e4ea61a5bc71b892c16 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Sat, 16 Dec 2023 02:16:20 +0530 Subject: [PATCH 18/24] fixes linting of imports Signed-off-by: Darshan Kumar --- internal/build/extend_build_test.go | 11 +++-------- internal/build/helper.go | 1 - internal/build/lifecycle_execution.go | 1 - pkg/archive/archive_test.go | 3 +-- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index 0e3cbc768d..8fd65fca45 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "io" - "path/filepath" "runtime" "strconv" @@ -12,22 +11,18 @@ import ( "testing" "github.com/apex/log" - "github.com/buildpacks/lifecycle/buildpack" - - "github.com/buildpacks/pack/internal/build" - "github.com/buildpacks/pack/internal/build/fakes" - "github.com/docker/docker/api/types" "github.com/golang/mock/gomock" "github.com/heroku/color" "github.com/sclevine/spec" "github.com/sclevine/spec/report" + "github.com/buildpacks/pack/internal/build" + "github.com/buildpacks/pack/internal/build/fakes" + mockdocker "github.com/buildpacks/pack/internal/build/mockdocker" "github.com/buildpacks/pack/pkg/logging" h "github.com/buildpacks/pack/testhelpers" - - mockdocker "github.com/buildpacks/pack/internal/build/mockdocker" ) func TestBuildDockerfiles(t *testing.T) { diff --git a/internal/build/helper.go b/internal/build/helper.go index e4afcef87d..9547d144cb 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -14,7 +14,6 @@ import ( "github.com/buildpacks/lifecycle/cmd" "github.com/buildpacks/pack/pkg/archive" - "github.com/buildpacks/pack/pkg/logging" ) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 421229f89a..1869363d9b 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -10,7 +10,6 @@ import ( "strconv" "github.com/BurntSushi/toml" - "github.com/buildpacks/lifecycle/api" "github.com/buildpacks/lifecycle/auth" "github.com/buildpacks/lifecycle/platform/files" diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go index 4f33a4d702..3ec53c94c8 100644 --- a/pkg/archive/archive_test.go +++ b/pkg/archive/archive_test.go @@ -9,9 +9,8 @@ import ( "strings" "testing" - "github.com/pkg/errors" - "github.com/heroku/color" + "github.com/pkg/errors" "github.com/sclevine/spec" "github.com/sclevine/spec/report" From 147699b49b16f19793fac426577004059b85430a Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 27 Dec 2023 00:42:36 +0530 Subject: [PATCH 19/24] adds warnings over UID and GID Signed-off-by: Darshan Kumar --- internal/build/helper.go | 13 +++++++++++++ internal/build/lifecycle_execution.go | 20 ++++++++++++++++++-- internal/build/lifecycle_executor.go | 1 + 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/internal/build/helper.go b/internal/build/helper.go index 9547d144cb..ecb3bcdfa0 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -12,6 +12,7 @@ import ( "github.com/BurntSushi/toml" "github.com/buildpacks/lifecycle/buildpack" "github.com/buildpacks/lifecycle/cmd" + "github.com/docker/docker/api/types" "github.com/buildpacks/pack/pkg/archive" "github.com/buildpacks/pack/pkg/logging" @@ -155,3 +156,15 @@ func (dockerfile *DockerfileInfo) CreateBuildContext(path string, logger logging return buf, completeErr } + +func userFrom(imageInfo types.ImageInspect) (string, string) { + user := strings.Split(imageInfo.Config.User, ":") + if len(user) < 2 { + return imageInfo.Config.User, "" + } + return user[0], user[1] +} + +func isRoot(userID string) bool { + return userID == "0" || userID == "root" +} diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index 52b7c15ba6..da778aea29 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -702,7 +702,7 @@ func (l *LifecycleExecution) Build(ctx context.Context, phaseFactory PhaseFactor WithBinds(l.opts.Volumes...), WithFlags(flags...), If((l.hasExtensionsForBuild()), WithImage(l.opts.BuilderImage+"-extended")), - If((l.hasExtensionsForBuild()), WithUser(l.opts.Builder.UID(), l.opts.Builder.GID())), + If((l.hasExtensionsForBuild() && l.opts.isExtendedBuilderImageRoot), WithUser(l.opts.Builder.UID(), l.opts.Builder.GID())), ) build := phaseFactory.New(configProvider) @@ -731,6 +731,8 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { extendedBuilderImageName := l.opts.BuilderImage + "-extended" var extensions Extensions extensions.SetExtensions(l.tmpDir, l.logger) + origuserID := strconv.Itoa(l.opts.Builder.UID()) + origgroupID := strconv.Itoa(l.opts.Builder.GID()) dockerfiles, err := extensions.DockerFiles(DockerfileKindBuild, l.tmpDir, l.logger) if err != nil { return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindBuild, err) @@ -769,7 +771,21 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { } builderImageName = l.opts.BuilderImage + "-extended" } - + extendedBuilderImageInfo, _, err := l.docker.ImageInspectWithRaw(ctx, extendedBuilderImageName) + if err != nil { + return fmt.Errorf("inspecting extended builder image: %w", err) + } + userID, groupID := userFrom(extendedBuilderImageInfo) + if userID != origuserID { + l.logger.Warnf("User ID changed from %s to %s", origuserID, userID) + } + if groupID != origgroupID && groupID != "" { + l.logger.Warnf("Group ID changed from %s to %s", origgroupID, groupID) + } + if isRoot(userID) { + l.logger.Warnf("Final extension left user as root thus forcing the user to be the original user %s and original group %s", origuserID, origgroupID) + l.opts.isExtendedBuilderImageRoot = true + } return nil } diff --git a/internal/build/lifecycle_executor.go b/internal/build/lifecycle_executor.go index 704c31b9a0..34cff09c66 100644 --- a/internal/build/lifecycle_executor.go +++ b/internal/build/lifecycle_executor.go @@ -70,6 +70,7 @@ type LifecycleOptions struct { Image name.Reference Builder Builder BuilderImage string // differs from Builder.Name() and Builder.Image().Name() in that it includes the registry context + isExtendedBuilderImageRoot bool LifecycleImage string LifecycleApis []string // optional - populated only if custom lifecycle image is downloaded, from that lifecycle's container's Labels. RunImage string From 1311893b3681c3c5e46f2344e826bcc8afdffcdf Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 27 Dec 2023 01:01:32 +0530 Subject: [PATCH 20/24] adds note in help and phase to notifiy the nomenclature Signed-off-by: Darshan Kumar --- internal/build/lifecycle_execution.go | 1 + internal/commands/build.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index da778aea29..d9d8523ec4 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -271,6 +271,7 @@ func (l *LifecycleExecution) Run(ctx context.Context, phaseFactoryCreator PhaseF */ group.Go(func() error { l.logger.Info(style.Step("EXTENDING (BUILD) BY DAEMON")) + l.logger.Info(style.Warn("WARNING: Extended build image is saved in the docker daemon as -extended")) if err := l.ExtendBuildByDaemon(ctx); err != nil { return err } diff --git a/internal/commands/build.go b/internal/commands/build.go index 3984da8503..739ca3cb6e 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -69,7 +69,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob "requires an image name, which will be generated from the source code. Build defaults to the current directory, " + "but you can use `--path` to specify another source code directory. Build requires a `builder`, which can either " + "be provided directly to build using `--builder`, or can be set using the `set-default-builder` command. For more " + - "on how to use `pack build`, see: https://buildpacks.io/docs/app-developer-guide/build-an-app/.", + "on how to use `pack build`, see: https://buildpacks.io/docs/app-developer-guide/build-an-app/.\n\nNote: Pack uses the nomeclature of -extended to refer to a builder image that has been extended using an extension.", RunE: logError(logger, func(cmd *cobra.Command, args []string) error { inputImageName := client.ParseInputImageReference(args[0]) if err := validateBuildFlags(&flags, cfg, inputImageName, logger); err != nil { From 3e64a541410c3f9aa1810e059e9f81dfac0907b4 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 27 Dec 2023 01:08:45 +0530 Subject: [PATCH 21/24] remove kaniko caching for build extend Signed-off-by: Darshan Kumar --- internal/build/lifecycle_execution.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index d9d8523ec4..e3793a90c8 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -487,16 +487,11 @@ func (l *LifecycleExecution) Restore(ctx context.Context, buildCache Cache, kani // for kaniko kanikoCacheBindOp := NullOp() - if (l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild()) || - l.platformAPI.AtLeast("0.12") { - if l.hasExtensionsForBuild() { - flags = append(flags, "-build-image", l.opts.BuilderImage) - registryImages = append(registryImages, l.opts.BuilderImage) - } + if l.platformAPI.AtLeast("0.12") { if l.runImageChanged() || l.hasExtensionsForRun() { registryImages = append(registryImages, l.runImageAfterExtensions()) } - if l.hasExtensionsForBuild() || l.hasExtensionsForRun() { + if l.hasExtensionsForRun() { kanikoCacheBindOp = WithBinds(fmt.Sprintf("%s:%s", kanikoCache.Name(), l.mountPaths.kanikoCacheDir())) } } From bb255dc33f3c490aab1c8d7c865eb29f11baedcb Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 27 Dec 2023 01:54:44 +0530 Subject: [PATCH 22/24] update subsequent changes in UID and GID to config Signed-off-by: Darshan Kumar --- internal/build/lifecycle_execution.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/internal/build/lifecycle_execution.go b/internal/build/lifecycle_execution.go index e3793a90c8..49745b9879 100644 --- a/internal/build/lifecycle_execution.go +++ b/internal/build/lifecycle_execution.go @@ -729,6 +729,9 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { extensions.SetExtensions(l.tmpDir, l.logger) origuserID := strconv.Itoa(l.opts.Builder.UID()) origgroupID := strconv.Itoa(l.opts.Builder.GID()) + intermediateUserID := origuserID + intermediateGroupID := origgroupID + var extendedBuilderImageInfo types.ImageInspect dockerfiles, err := extensions.DockerFiles(DockerfileKindBuild, l.tmpDir, l.logger) if err != nil { return fmt.Errorf("getting %s.Dockerfiles: %w", DockerfileKindBuild, err) @@ -736,8 +739,8 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { for _, dockerfile := range dockerfiles { dockerfile.Args = append([]Arg{ {Name: argBuildID, Value: uuid.New().String()}, - {Name: argUserID, Value: strconv.Itoa(l.opts.Builder.UID())}, - {Name: argGroupID, Value: strconv.Itoa(l.opts.Builder.GID())}, + {Name: argUserID, Value: intermediateUserID}, + {Name: argGroupID, Value: intermediateGroupID}, }, dockerfile.Args...) buildArguments := map[string]*string{} buildArguments["base_image"] = &builderImageName @@ -766,17 +769,25 @@ func (l *LifecycleExecution) ExtendBuildByDaemon(ctx context.Context) error { return err } builderImageName = l.opts.BuilderImage + "-extended" - } - extendedBuilderImageInfo, _, err := l.docker.ImageInspectWithRaw(ctx, extendedBuilderImageName) - if err != nil { - return fmt.Errorf("inspecting extended builder image: %w", err) + extendedBuilderImageInfo, _, err = l.docker.ImageInspectWithRaw(ctx, extendedBuilderImageName) + if err != nil { + return fmt.Errorf("inspecting extended builder image: %w", err) + } + userID, groupID := userFrom(extendedBuilderImageInfo) + if isRoot(userID) { + l.logger.Warnf("Extension from %s changed the user ID from %s to %s; this must not be the final user ID (a following extension must reset the user).", dockerfile.Info.Path, intermediateUserID, userID) + } + intermediateUserID = userID + if groupID != "" { + intermediateGroupID = groupID + } } userID, groupID := userFrom(extendedBuilderImageInfo) if userID != origuserID { - l.logger.Warnf("User ID changed from %s to %s", origuserID, userID) + l.logger.Warnf("Final User ID changed from %s to %s", origuserID, userID) } if groupID != origgroupID && groupID != "" { - l.logger.Warnf("Group ID changed from %s to %s", origgroupID, groupID) + l.logger.Warnf("Final Group ID changed from %s to %s", origgroupID, groupID) } if isRoot(userID) { l.logger.Warnf("Final extension left user as root thus forcing the user to be the original user %s and original group %s", origuserID, origgroupID) From c560e9feae977a982e2636d056564848d07e5755 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 27 Dec 2023 02:31:19 +0530 Subject: [PATCH 23/24] fix tests Signed-off-by: Darshan Kumar --- internal/build/extend_build_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/build/extend_build_test.go b/internal/build/extend_build_test.go index 8fd65fca45..8d0c833b37 100644 --- a/internal/build/extend_build_test.go +++ b/internal/build/extend_build_test.go @@ -13,6 +13,7 @@ import ( "github.com/apex/log" "github.com/buildpacks/lifecycle/buildpack" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" "github.com/golang/mock/gomock" "github.com/heroku/color" "github.com/sclevine/spec" @@ -102,6 +103,11 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { mockDockerClient.EXPECT().ImageBuild(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ context.Context, buildContext io.Reader, buildOptions types.ImageBuildOptions) { compBuildOptions(t, expectedBuildOptions, buildOptions) }).Return(mockResponse, nil).Times(1) + mockDockerClient.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return(types.ImageInspect{ + Config: &container.Config{ + User: "root", + }, + }, nil, nil).Times(1) err := lifecycle.ExtendBuildByDaemon(context.Background()) h.AssertNil(t, err) }) @@ -115,6 +121,10 @@ func testBuildDockerfiles(t *testing.T, when spec.G, it spec.S) { OSType: "linux", } mockDockerClient.EXPECT().ImageBuild(gomock.Any(), gomock.Any(), gomock.Any()).Return(mockResponse, nil).Times(2) + mockDockerClient.EXPECT().ImageInspectWithRaw(gomock.Any(), gomock.Any()).Return(types.ImageInspect{ + Config: &container.Config{ + User: "root", + }}, nil, nil).Times(2) err := lifecycle.ExtendBuildByDaemon(context.Background()) h.AssertNil(t, err) }) From 7cf7e08d3849d0d42ee8ee99caeba80b18c8af43 Mon Sep 17 00:00:00 2001 From: Darshan Kumar Date: Wed, 27 Dec 2023 02:41:19 +0530 Subject: [PATCH 24/24] remove redundant tests Signed-off-by: Darshan Kumar --- internal/build/lifecycle_execution_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/internal/build/lifecycle_execution_test.go b/internal/build/lifecycle_execution_test.go index ffb5081193..189eefb682 100644 --- a/internal/build/lifecycle_execution_test.go +++ b/internal/build/lifecycle_execution_test.go @@ -1728,14 +1728,6 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { platformAPI = api.MustParse("0.12") providedOrderExt = dist.Order{dist.OrderEntry{Group: []dist.ModuleRef{ /* don't care */ }}} - when("for build", func() { - extensionsForBuild = true - - it("configures the phase with registry access", func() { - h.AssertSliceContains(t, configProvider.ContainerConfig().Env, "CNB_REGISTRY_AUTH={}") - }) - }) - when("for run", func() { extensionsForRun = true @@ -1810,16 +1802,6 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) { h.AssertSliceNotContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") }) }) - - when("platform >= 0.10", func() { - platformAPI = api.MustParse("0.10") - - it("provides -build-image and /kaniko bind", func() { - h.AssertSliceContainsInOrder(t, configProvider.ContainerConfig().Cmd, "-build-image", providedBuilderImage) - h.AssertSliceContains(t, configProvider.ContainerConfig().Env, "CNB_REGISTRY_AUTH={}") - h.AssertSliceContains(t, configProvider.HostConfig().Binds, "some-kaniko-cache:/kaniko") - }) - }) }) when("not present in /generated/build", func() {