Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Saving bundles as an intermediate ocitar #612

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/imgpkg/bundle/contents.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type Contents struct {
paths []string
excludedPaths []string
preservePermissions bool
ociTarPath string
}

//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . ImagesMetadataWriter
Expand All @@ -38,8 +39,8 @@ type ImagesMetadataWriter interface {
}

// NewContents creates Contents struct
func NewContents(paths []string, excludedPaths []string, preservePermissions bool) Contents {
return Contents{paths: paths, excludedPaths: excludedPaths, preservePermissions: preservePermissions}
func NewContents(paths []string, excludedPaths []string, preservePermissions bool, ociTarPath string) Contents {
return Contents{paths: paths, excludedPaths: excludedPaths, preservePermissions: preservePermissions, ociTarPath: ociTarPath}
}

// Push the contents of the bundle to the registry as an OCI Image
Expand All @@ -54,7 +55,7 @@ func (b Contents) Push(uploadRef regname.Tag, labels map[string]string, registry
}
labels[BundleConfigLabel] = "true"

return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions).Push(uploadRef, labels, registry, logger)
return plainimage.NewContents(b.paths, b.excludedPaths, b.preservePermissions, b.ociTarPath).Push(uploadRef, labels, registry, logger)
}

// PresentsAsBundle checks if the provided folders have the needed structure to be a bundle
Expand Down
4 changes: 2 additions & 2 deletions pkg/imgpkg/bundle/contents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ images:
fakeRegistry.ImageReturns(bundleImg, nil)

t.Run("push is successful", func(t *testing.T) {
subject := bundle.NewContents([]string{bundleDir}, nil, false)
subject := bundle.NewContents([]string{bundleDir}, nil, false, "")
imgTag, err := name.NewTag("my.registry.io/new-bundle:tag")
if err != nil {
t.Fatalf("failed to read tag: %s", err)
Expand Down Expand Up @@ -72,7 +72,7 @@ images:
fakeRegistry.ImageReturns(bundleImg, nil)

t.Run("push is successful", func(t *testing.T) {
subject := bundle.NewContents([]string{bundleDir}, nil, false)
subject := bundle.NewContents([]string{bundleDir}, nil, false, "")
imgTag, err := name.NewTag("my.registry.io/new-bundle:tag")
if err != nil {
t.Fatalf("failed to read tag: %s", err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/imgpkg/bundle/locations_configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (r LocationsConfigs) Save(reg ImagesMetadataWriter, bundleRef name.Digest,

r.ui.Tracef("Pushing image\n")

_, err = plainimage.NewContents([]string{tmpDir}, nil, false).Push(locRef, nil, reg.CloneWithLogger(util.NewNoopProgressBar()), logger)
_, err = plainimage.NewContents([]string{tmpDir}, nil, false, "").Push(locRef, nil, reg.CloneWithLogger(util.NewNoopProgressBar()), logger)
if err != nil {
// Immutable tag errors within registries are not standardized.
// Assume word "immutable" would be present in most cases.
Expand Down
6 changes: 6 additions & 0 deletions pkg/imgpkg/cmd/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type CopyOptions struct {
ui ui.UI

ImageFlags ImageFlags
OciFlags OciFlags
BundleFlags BundleFlags
LockInputFlags LockInputFlags
LockOutputFlags LockOutputFlags
Expand Down Expand Up @@ -72,6 +73,7 @@ func NewCopyCmd(o *CopyOptions) *cobra.Command {
}

o.ImageFlags.SetCopy(cmd)
o.OciFlags.Set(cmd)
o.BundleFlags.SetCopy(cmd)
o.LockInputFlags.Set(cmd)
o.LockOutputFlags.SetOnCopy(cmd)
Expand Down Expand Up @@ -125,6 +127,7 @@ func (c *CopyOptions) Run() error {

repoSrc := CopyRepoSrc{
ImageFlags: c.ImageFlags,
OciFlags: c.OciFlags,
BundleFlags: c.BundleFlags,
LockInputFlags: c.LockInputFlags,
TarFlags: c.TarFlags,
Expand Down Expand Up @@ -261,6 +264,9 @@ func (c *CopyOptions) hasOneSrc() bool {
seen = true
}
}
if c.OciFlags.IsOci() {
seen = true
}
return seen
}

Expand Down
80 changes: 56 additions & 24 deletions pkg/imgpkg/cmd/copy_repo_src.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ package cmd

import (
"fmt"
"os"

ctlbundle "carvel.dev/imgpkg/pkg/imgpkg/bundle"
"carvel.dev/imgpkg/pkg/imgpkg/image"
"carvel.dev/imgpkg/pkg/imgpkg/imageset"
ctlimgset "carvel.dev/imgpkg/pkg/imgpkg/imageset"
"carvel.dev/imgpkg/pkg/imgpkg/imagetar"
Expand All @@ -23,6 +25,7 @@ type SignatureRetriever interface {

type CopyRepoSrc struct {
ImageFlags ImageFlags
OciFlags OciFlags
BundleFlags BundleFlags
LockInputFlags LockInputFlags
TarFlags TarFlags
Expand Down Expand Up @@ -60,49 +63,73 @@ func (c CopyRepoSrc) CopyToTar(dstPath string, resume bool) error {
func (c CopyRepoSrc) CopyToRepo(repo string) (*ctlimgset.ProcessedImages, error) {
c.logger.Tracef("CopyToRepo(%s)\n", repo)

var tempDir string
var processedImages *ctlimgset.ProcessedImages
importRepo, err := regname.NewRepository(repo)
if err != nil {
return nil, fmt.Errorf("Building import repository ref: %s", err)
}

if c.TarFlags.IsSrc() {
if c.TarFlags.IsSrc() || c.OciFlags.IsOci() {
if c.TarFlags.IsDst() {
return nil, fmt.Errorf("Cannot use tar source (--tar) with tar destination (--to-tar)")
}

processedImages, err = c.tarImageSet.Import(c.TarFlags.TarSrc, importRepo, c.registry)
if c.OciFlags.IsOci() {
tempDir, err := os.MkdirTemp("", "imgpkg-oci-extract-")
if err != nil {
return nil, err
}
err = image.ExtractOciTarGz(c.OciFlags.OcitoReg, tempDir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let us make sure we clean up after ourselves

Suggested change
err = image.ExtractOciTarGz(c.OciFlags.OcitoReg, tempDir)
defer os.RemoveAll(tempDir)
err = image.ExtractOciTarGz(c.OciFlags.OcitoReg, tempDir)

if err != nil {
return nil, fmt.Errorf("Extracting OCI tar: %s", err)
}
processedImages, err = c.tarImageSet.Import(tempDir, importRepo, c.registry, true)
if err != nil {
return nil, fmt.Errorf("Importing OCI tar: %s", err)
}
Comment on lines +86 to +89
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is similar to the else case, maybe we can move this outside the if statement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else case is calling the import function with false as a parameter while here its true which in turns calls the readoci tar different to read tar depending on the bool in the import function.
Screenshot 2024-05-25 at 1 45 45 PM


} else {
processedImages, err = c.tarImageSet.Import(c.TarFlags.TarSrc, importRepo, c.registry, false)
if err != nil {
return nil, err
}
}

if err != nil {
return nil, err
}

var parentBundle *ctlbundle.Bundle
foundRootBundle := false
for _, processedImage := range processedImages.All() {
if processedImage.ImageIndex != nil {
continue
}
// This is added to not read the lockfile and change the ref for oci-flag. Will be removed once we add an inflate option to copy the refs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this commented out because if we copy from oci-tar to the repository, we do not have all the images? If that is the case, we should change the comment to highlight something like:

// When copying images from an oci-tar to a repository, imgpkg will not try to access the origin repositories and will only copy the OCI Image to the registry, similar to the behavior we currently have on the `imgpkg push` command.

if !c.OciFlags.IsOci() {
var parentBundle *ctlbundle.Bundle
foundRootBundle := false
for _, processedImage := range processedImages.All() {
if processedImage.ImageIndex != nil {
continue
}

if _, ok := processedImage.Labels[rootBundleLabelKey]; ok {
if foundRootBundle {
panic("Internal inconsistency: expected only 1 root bundle")
if _, ok := processedImage.Labels[rootBundleLabelKey]; ok {
if foundRootBundle {
panic("Internal inconsistency: expected only 1 root bundle")
}
foundRootBundle = true
pImage := plainimage.NewFetchedPlainImageWithTag(processedImage.DigestRef, processedImage.Tag, processedImage.Image)
lockReader := ctlbundle.NewImagesLockReader()
parentBundle = ctlbundle.NewBundle(pImage, c.registry, lockReader, ctlbundle.NewFetcherFromProcessedImages(processedImages.All(), c.registry, lockReader))
}
foundRootBundle = true
pImage := plainimage.NewFetchedPlainImageWithTag(processedImage.DigestRef, processedImage.Tag, processedImage.Image)
lockReader := ctlbundle.NewImagesLockReader()
parentBundle = ctlbundle.NewBundle(pImage, c.registry, lockReader, ctlbundle.NewFetcherFromProcessedImages(processedImages.All(), c.registry, lockReader))
}
}

if foundRootBundle {
bundles, _, err := parentBundle.AllImagesLockRefs(c.Concurrency, c.logger)
if err != nil {
return nil, err
}
if foundRootBundle {
bundles, _, err := parentBundle.AllImagesLockRefs(c.Concurrency, c.logger)
if err != nil {
return nil, err
}

for _, bundle := range bundles {
if err := bundle.NoteCopy(processedImages, c.registry, c.logger); err != nil {
return nil, fmt.Errorf("Creating copy information for bundle %s: %s", bundle.DigestRef(), err)
for _, bundle := range bundles {
if err := bundle.NoteCopy(processedImages, c.registry, c.logger); err != nil {
return nil, fmt.Errorf("Creating copy information for bundle %s: %s", bundle.DigestRef(), err)
}
}
}
}
Expand Down Expand Up @@ -133,6 +160,11 @@ func (c CopyRepoSrc) CopyToRepo(repo string) (*ctlimgset.ProcessedImages, error)
return nil, fmt.Errorf("Tagging images: %s", err)
}

err = os.RemoveAll(tempDir)
if err != nil {
fmt.Println("Error cleaning up temporary directory:", err)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a prior comment that I think it would be better to use than this part of the code. Because if anything fails in the meanwhile we will leave the folder back

Suggested change
err = os.RemoveAll(tempDir)
if err != nil {
fmt.Println("Error cleaning up temporary directory:", err)
}

return processedImages, nil
}

Expand Down
23 changes: 23 additions & 0 deletions pkg/imgpkg/cmd/oci_flags.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2023 The Carvel Authors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Copyright 2023 The Carvel Authors.
// Copyright 2024 The Carvel Authors.

// SPDX-License-Identifier: Apache-2.0

package cmd

import (
"github.com/spf13/cobra"
)

// OciFlags is a struct that holds the flags for the OCI tar file.
type OciFlags struct {
OcitoReg string
OciTar string
}

// Set sets the flags for the OCI tar file.
func (o *OciFlags) Set(cmd *cobra.Command) {
cmd.Flags().StringVar(&o.OciTar, "to-oci-tar", "", "Set OciTarPath to be saved to disk (example: /path/file.tar)")
cmd.Flags().StringVar(&o.OcitoReg, "oci-tar", "", "Give path to OCI tar file (example: /path/file.tar)")
}

// IsOci returns true if the OCI tar file is set.
func (o OciFlags) IsOci() bool { return o.OcitoReg != "" }
11 changes: 8 additions & 3 deletions pkg/imgpkg/cmd/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type PushOptions struct {
ui ui.UI

ImageFlags ImageFlags
OciFlags OciFlags
BundleFlags BundleFlags
LockOutputFlags LockOutputFlags
FileFlags FileFlags
Expand All @@ -40,10 +41,14 @@ func NewPushCmd(o *PushOptions) *cobra.Command {
# Push bundle repo/app1-config with contents of config/ directory
imgpkg push -b repo/app1-config -f config/

#Push bundle saving the tar as OCI tar
imgpkg push -b repo/app1-config -f config/ --to-oci-tar /path/to/file.tar

# Push image repo/app1-config with contents from multiple locations
imgpkg push -i repo/app1-config -f config/ -f additional-config.yml`,
}
o.ImageFlags.Set(cmd)
o.OciFlags.Set(cmd)
o.BundleFlags.Set(cmd)
o.LockOutputFlags.SetOnPush(cmd)
o.FileFlags.Set(cmd)
Expand Down Expand Up @@ -104,7 +109,7 @@ func (po *PushOptions) pushBundle(registry registry.Registry) (string, error) {
}

logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui))
imageURL, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger)
imageURL, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions, po.OciFlags.OciTar).Push(uploadRef, po.LabelFlags.Labels, registry, logger)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -140,7 +145,7 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) {
return "", fmt.Errorf("Parsing '%s': %s", po.ImageFlags.Image, err)
}

isBundle, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).PresentsAsBundle()
isBundle, err := bundle.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions, po.OciFlags.OciTar).PresentsAsBundle()
if err != nil {
return "", err
}
Expand All @@ -149,7 +154,7 @@ func (po *PushOptions) pushImage(registry registry.Registry) (string, error) {
}

logger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(po.ui))
return plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions).Push(uploadRef, po.LabelFlags.Labels, registry, logger)
return plainimage.NewContents(po.FileFlags.Files, po.FileFlags.ExcludedFilePaths, po.FileFlags.PreservePermissions, po.OciFlags.OciTar).Push(uploadRef, po.LabelFlags.Labels, registry, logger)
}

// validateFlags checks if the provided flags are valid
Expand Down
Loading
Loading