Skip to content

Commit

Permalink
fix: Download bundle without specifying tag
Browse files Browse the repository at this point in the history
The "latest" tag is not supported, therefore it is necessary to stop
with an error in case it is not provided when using the -b flag command
line flag.

Add unit tests to reproduce the bug and verify the fix.

Closes issue #4470
  • Loading branch information
A-725-K committed Nov 25, 2024
1 parent 41695ca commit 90b4ca7
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 10 deletions.
12 changes: 7 additions & 5 deletions pkg/crc/machine/bundle/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,19 @@ func GetCustomBundleName(bundleFilename string) string {
return fmt.Sprintf("%s_%d%s", baseName, time.Now().Unix(), bundleExtension)
}

func GetBundleNameFromURI(bundleURI string) string {
// the URI is expected to have been validated by validation.ValidateBundlePath first
func GetBundleNameFromURI(bundleURI string) (string, error) {
switch {
case strings.HasPrefix(bundleURI, "docker://"):
imageAndTag := strings.Split(path.Base(bundleURI), ":")
return constants.BundleForPreset(image.GetPresetName(imageAndTag[0]), imageAndTag[1])
if len(imageAndTag) < 2 {
return "", fmt.Errorf("No tag found in bundle URI")
}
return constants.BundleForPreset(image.GetPresetName(imageAndTag[0]), imageAndTag[1]), nil
case strings.HasPrefix(bundleURI, "http://"), strings.HasPrefix(bundleURI, "https://"):
return path.Base(bundleURI)
return path.Base(bundleURI), nil
default:
// local path
return filepath.Base(bundleURI)
return filepath.Base(bundleURI), nil
}
}

Expand Down
34 changes: 33 additions & 1 deletion pkg/crc/machine/bundle/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"path/filepath"
"runtime"
"strings"
"testing"
"unicode"
Expand Down Expand Up @@ -55,7 +56,7 @@ func jsonForBundleWithVersion(version, name string) string {
{
"name": "crc.qcow2",
"format": "qcow2",
"size": "9",
"size": "9",
"sha256sum": "245a0e5acd4f09000a9a5f37d731082ed1cf3fdcad1b5320cbe9b153c9fd82a4"
}
],
Expand Down Expand Up @@ -289,3 +290,34 @@ func TestGetBundleInfoFromNameInvalid(t *testing.T) {
_, err = GetBundleInfoFromName("crc_nanoshift_libvirt_4.16.7_amd64_232.crcbundle")
assert.Error(t, err)
}

func TestGetBundleNameFromURI(t *testing.T) {
// URI with no tag
bundleName, err := GetBundleNameFromURI("docker://quay.io/crcont/openshift-bundle")
assert.Equal(t, "", bundleName)
assert.Error(t, err)

// URI with tag
bundleName, err = GetBundleNameFromURI("docker://quay.io/crcont/openshift-bundle:4.17.3")
assert.Nil(t, err)
var osVirt string
switch runtime.GOOS {
case "darwin":
osVirt = "vfkit"
case "linux":
osVirt = "libvirt"
case "windows":
osVirt = "hyperv"
}
assert.Equal(t, fmt.Sprintf("crc_%s_4.17.3_%s.crcbundle", osVirt, runtime.GOARCH), bundleName)

// HTTPs
bundleName, err = GetBundleNameFromURI("https://developers.redhat.com/content-gateway/file/pub/openshift-v4/clients/crc/bundles/openshift/4.17.3/crc_libvirt_4.17.3_amd64.crcbundle")
assert.Nil(t, err)
assert.Equal(t, "crc_libvirt_4.17.3_amd64.crcbundle", bundleName)

// Local file
bundleName, err = GetBundleNameFromURI("/home/user/Downloads/crc_libvirt_4.17.3_amd64.crcbundle")
assert.Nil(t, err)
assert.Equal(t, "crc_libvirt_4.17.3_amd64.crcbundle", bundleName)
}
6 changes: 5 additions & 1 deletion pkg/crc/machine/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,11 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
return nil, errors.Wrap(err, "Cannot determine if VM exists")
}

bundleName := bundle.GetBundleNameWithoutExtension(bundle.GetBundleNameFromURI(startConfig.BundlePath))
bundleNameFromURI, err := bundle.GetBundleNameFromURI(startConfig.BundlePath)
if err != nil {
return nil, errors.Wrap(err, "Error getting bundle name")
}
bundleName := bundle.GetBundleNameWithoutExtension(bundleNameFromURI)
crcBundleMetadata, err := getCrcBundleInfo(ctx, startConfig.Preset, bundleName, startConfig.BundlePath, startConfig.EnableBundleQuayFallback)
if err != nil {
return nil, errors.Wrap(err, "Error getting bundle metadata")
Expand Down
6 changes: 5 additions & 1 deletion pkg/crc/preflight/preflight_checks_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ var genericCleanupChecks = []Check{
func checkBundleExtracted(bundlePath string) func() error {
return func() error {
logging.Infof("Checking if %s exists", bundlePath)
bundleName := bundle.GetBundleNameFromURI(bundlePath)
bundleName, err := bundle.GetBundleNameFromURI(bundlePath)
if err != nil {
logging.Debugf("error getting bundle name from path %s: %v", bundlePath, err)
return err
}
if _, err := bundle.Get(bundleName); err != nil {
logging.Debugf("error getting bundle info for %s: %v", bundleName, err)
return err
Expand Down
10 changes: 8 additions & 2 deletions pkg/crc/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,19 @@ func ValidateBundlePath(bundlePath string, preset crcpreset.Preset) error {
}
}

userProvidedBundle := bundle.GetBundleNameFromURI(bundlePath)
userProvidedBundle, err := bundle.GetBundleNameFromURI(bundlePath)
if err != nil {
return err
}
bundleMismatchWarning(userProvidedBundle, preset)
return nil
}

func ValidateBundle(bundlePath string, preset crcpreset.Preset) error {
bundleName := bundle.GetBundleNameFromURI(bundlePath)
bundleName, err := bundle.GetBundleNameFromURI(bundlePath)
if err != nil {
return err
}
bundleMetadata, err := bundle.Get(bundleName)
if err != nil {
if bundlePath == constants.GetDefaultBundlePath(preset) {
Expand Down

0 comments on commit 90b4ca7

Please sign in to comment.