Skip to content

Commit face0f4

Browse files
A-725-Kpraveenkumar
authored andcommitted
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
1 parent 2c5e735 commit face0f4

File tree

5 files changed

+58
-10
lines changed

5 files changed

+58
-10
lines changed

pkg/crc/machine/bundle/metadata.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -243,17 +243,19 @@ func GetCustomBundleName(bundleFilename string) string {
243243
return fmt.Sprintf("%s_%d%s", baseName, time.Now().Unix(), bundleExtension)
244244
}
245245

246-
func GetBundleNameFromURI(bundleURI string) string {
247-
// the URI is expected to have been validated by validation.ValidateBundlePath first
246+
func GetBundleNameFromURI(bundleURI string) (string, error) {
248247
switch {
249248
case strings.HasPrefix(bundleURI, "docker://"):
250249
imageAndTag := strings.Split(path.Base(bundleURI), ":")
251-
return constants.BundleForPreset(image.GetPresetName(imageAndTag[0]), imageAndTag[1])
250+
if len(imageAndTag) < 2 {
251+
return "", fmt.Errorf("No tag found in bundle URI")
252+
}
253+
return constants.BundleForPreset(image.GetPresetName(imageAndTag[0]), imageAndTag[1]), nil
252254
case strings.HasPrefix(bundleURI, "http://"), strings.HasPrefix(bundleURI, "https://"):
253-
return path.Base(bundleURI)
255+
return path.Base(bundleURI), nil
254256
default:
255257
// local path
256-
return filepath.Base(bundleURI)
258+
return filepath.Base(bundleURI), nil
257259
}
258260
}
259261

pkg/crc/machine/bundle/metadata_test.go

+33-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"fmt"
66
"path/filepath"
7+
"runtime"
78
"strings"
89
"testing"
910
"unicode"
@@ -55,7 +56,7 @@ func jsonForBundleWithVersion(version, name string) string {
5556
{
5657
"name": "crc.qcow2",
5758
"format": "qcow2",
58-
"size": "9",
59+
"size": "9",
5960
"sha256sum": "245a0e5acd4f09000a9a5f37d731082ed1cf3fdcad1b5320cbe9b153c9fd82a4"
6061
}
6162
],
@@ -311,3 +312,34 @@ func TestGetFQDN(t *testing.T) {
311312
})
312313
}
313314
}
315+
316+
func TestGetBundleNameFromURI(t *testing.T) {
317+
// URI with no tag
318+
bundleName, err := GetBundleNameFromURI("docker://quay.io/crcont/openshift-bundle")
319+
assert.Equal(t, "", bundleName)
320+
assert.Error(t, err)
321+
322+
// URI with tag
323+
bundleName, err = GetBundleNameFromURI("docker://quay.io/crcont/openshift-bundle:4.17.3")
324+
assert.Nil(t, err)
325+
var osVirt string
326+
switch runtime.GOOS {
327+
case "darwin":
328+
osVirt = "vfkit"
329+
case "linux":
330+
osVirt = "libvirt"
331+
case "windows":
332+
osVirt = "hyperv"
333+
}
334+
assert.Equal(t, fmt.Sprintf("crc_%s_4.17.3_%s.crcbundle", osVirt, runtime.GOARCH), bundleName)
335+
336+
// HTTPs
337+
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")
338+
assert.Nil(t, err)
339+
assert.Equal(t, "crc_libvirt_4.17.3_amd64.crcbundle", bundleName)
340+
341+
// Local file
342+
bundleName, err = GetBundleNameFromURI("/home/user/Downloads/crc_libvirt_4.17.3_amd64.crcbundle")
343+
assert.Nil(t, err)
344+
assert.Equal(t, "crc_libvirt_4.17.3_amd64.crcbundle", bundleName)
345+
}

pkg/crc/machine/start.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,11 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
278278
return nil, errors.Wrap(err, "Cannot determine if VM exists")
279279
}
280280

281-
bundleName := bundle.GetBundleNameWithoutExtension(bundle.GetBundleNameFromURI(startConfig.BundlePath))
281+
bundleNameFromURI, err := bundle.GetBundleNameFromURI(startConfig.BundlePath)
282+
if err != nil {
283+
return nil, errors.Wrap(err, "Error getting bundle name")
284+
}
285+
bundleName := bundle.GetBundleNameWithoutExtension(bundleNameFromURI)
282286
crcBundleMetadata, err := getCrcBundleInfo(ctx, startConfig.Preset, bundleName, startConfig.BundlePath, startConfig.EnableBundleQuayFallback)
283287
if err != nil {
284288
return nil, errors.Wrap(err, "Error getting bundle metadata")

pkg/crc/preflight/preflight_checks_common.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ var genericCleanupChecks = []Check{
8787
func checkBundleExtracted(bundlePath string) func() error {
8888
return func() error {
8989
logging.Infof("Checking if %s exists", bundlePath)
90-
bundleName := bundle.GetBundleNameFromURI(bundlePath)
90+
bundleName, err := bundle.GetBundleNameFromURI(bundlePath)
91+
if err != nil {
92+
logging.Debugf("error getting bundle name from path %s: %v", bundlePath, err)
93+
return err
94+
}
9195
if _, err := bundle.Get(bundleName); err != nil {
9296
logging.Debugf("error getting bundle info for %s: %v", bundleName, err)
9397
return err

pkg/crc/validation/validation.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,19 @@ func ValidateBundlePath(bundlePath string, preset crcpreset.Preset) error {
8383
}
8484
}
8585

86-
userProvidedBundle := bundle.GetBundleNameFromURI(bundlePath)
86+
userProvidedBundle, err := bundle.GetBundleNameFromURI(bundlePath)
87+
if err != nil {
88+
return err
89+
}
8790
bundleMismatchWarning(userProvidedBundle, preset)
8891
return nil
8992
}
9093

9194
func ValidateBundle(bundlePath string, preset crcpreset.Preset) error {
92-
bundleName := bundle.GetBundleNameFromURI(bundlePath)
95+
bundleName, err := bundle.GetBundleNameFromURI(bundlePath)
96+
if err != nil {
97+
return err
98+
}
9399
bundleMetadata, err := bundle.Get(bundleName)
94100
if err != nil {
95101
if bundlePath == constants.GetDefaultBundlePath(preset) {

0 commit comments

Comments
 (0)