From face0f4b8c17295bfd28c79e23fb0aad5a551b53 Mon Sep 17 00:00:00 2001 From: A-725-K Date: Thu, 21 Nov 2024 11:34:00 +0100 Subject: [PATCH] fix: Download bundle without specifying tag 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 --- pkg/crc/machine/bundle/metadata.go | 12 ++++--- pkg/crc/machine/bundle/metadata_test.go | 34 +++++++++++++++++++- pkg/crc/machine/start.go | 6 +++- pkg/crc/preflight/preflight_checks_common.go | 6 +++- pkg/crc/validation/validation.go | 10 ++++-- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/pkg/crc/machine/bundle/metadata.go b/pkg/crc/machine/bundle/metadata.go index f3cb117866..40f6c4f38f 100644 --- a/pkg/crc/machine/bundle/metadata.go +++ b/pkg/crc/machine/bundle/metadata.go @@ -243,17 +243,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 } } diff --git a/pkg/crc/machine/bundle/metadata_test.go b/pkg/crc/machine/bundle/metadata_test.go index eff5d772eb..796e729a0b 100644 --- a/pkg/crc/machine/bundle/metadata_test.go +++ b/pkg/crc/machine/bundle/metadata_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "path/filepath" + "runtime" "strings" "testing" "unicode" @@ -55,7 +56,7 @@ func jsonForBundleWithVersion(version, name string) string { { "name": "crc.qcow2", "format": "qcow2", - "size": "9", + "size": "9", "sha256sum": "245a0e5acd4f09000a9a5f37d731082ed1cf3fdcad1b5320cbe9b153c9fd82a4" } ], @@ -311,3 +312,34 @@ func TestGetFQDN(t *testing.T) { }) } } + +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) +} diff --git a/pkg/crc/machine/start.go b/pkg/crc/machine/start.go index a77665f924..17eacb2c33 100644 --- a/pkg/crc/machine/start.go +++ b/pkg/crc/machine/start.go @@ -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") diff --git a/pkg/crc/preflight/preflight_checks_common.go b/pkg/crc/preflight/preflight_checks_common.go index 50cf05ee95..3c401ae89c 100644 --- a/pkg/crc/preflight/preflight_checks_common.go +++ b/pkg/crc/preflight/preflight_checks_common.go @@ -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 diff --git a/pkg/crc/validation/validation.go b/pkg/crc/validation/validation.go index 3e8ee9496b..c330c4b4e3 100644 --- a/pkg/crc/validation/validation.go +++ b/pkg/crc/validation/validation.go @@ -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) {