Skip to content

Commit b3a1963

Browse files
committed
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 0342835 commit b3a1963

File tree

5 files changed

+50
-10
lines changed

5 files changed

+50
-10
lines changed

pkg/crc/machine/bundle/metadata.go

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

241-
func GetBundleNameFromURI(bundleURI string) string {
242-
// the URI is expected to have been validated by validation.ValidateBundlePath first
241+
func GetBundleNameFromURI(bundleURI string) (string, error) {
243242
switch {
244243
case strings.HasPrefix(bundleURI, "docker://"):
245244
imageAndTag := strings.Split(path.Base(bundleURI), ":")
246-
return constants.BundleForPreset(image.GetPresetName(imageAndTag[0]), imageAndTag[1])
245+
if len(imageAndTag) < 2 {
246+
return "", fmt.Errorf("No tag found in bundle URI")
247+
}
248+
return constants.BundleForPreset(image.GetPresetName(imageAndTag[0]), imageAndTag[1]), nil
247249
case strings.HasPrefix(bundleURI, "http://"), strings.HasPrefix(bundleURI, "https://"):
248-
return path.Base(bundleURI)
250+
return path.Base(bundleURI), nil
249251
default:
250252
// local path
251-
return filepath.Base(bundleURI)
253+
return filepath.Base(bundleURI), nil
252254
}
253255
}
254256

pkg/crc/machine/bundle/metadata_test.go

+25-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func jsonForBundleWithVersion(version, name string) string {
5555
{
5656
"name": "crc.qcow2",
5757
"format": "qcow2",
58-
"size": "9",
58+
"size": "9",
5959
"sha256sum": "245a0e5acd4f09000a9a5f37d731082ed1cf3fdcad1b5320cbe9b153c9fd82a4"
6060
}
6161
],
@@ -289,3 +289,27 @@ func TestGetBundleInfoFromNameInvalid(t *testing.T) {
289289
_, err = GetBundleInfoFromName("crc_nanoshift_libvirt_4.16.7_amd64_232.crcbundle")
290290
assert.Error(t, err)
291291
}
292+
293+
func TestGetBundleNameFromURI(t *testing.T) {
294+
// URI with no tag
295+
bundleName, err := GetBundleNameFromURI("docker://quay.io/crcont/openshift-bundle")
296+
assert.Equal(t, "", bundleName)
297+
assert.Error(t, err)
298+
299+
expectedBundleName := "crc_libvirt_4.17.3_amd64.crcbundle"
300+
301+
// URI with tag
302+
bundleName, err = GetBundleNameFromURI("docker://quay.io/crcont/openshift-bundle:4.17.3")
303+
assert.Nil(t, err)
304+
assert.Equal(t, expectedBundleName, bundleName)
305+
306+
// HTTPs
307+
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")
308+
assert.Nil(t, err)
309+
assert.Equal(t, bundleName, expectedBundleName, bundleName)
310+
311+
// Local file
312+
bundleName, err = GetBundleNameFromURI("/home/user/Downloads/crc_libvirt_4.17.3_amd64.crcbundle")
313+
assert.Nil(t, err)
314+
assert.Equal(t, bundleName, expectedBundleName, bundleName)
315+
}

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(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
@@ -86,7 +86,11 @@ var genericCleanupChecks = []Check{
8686
func checkBundleExtracted(bundlePath string) func() error {
8787
return func() error {
8888
logging.Infof("Checking if %s exists", bundlePath)
89-
bundleName := bundle.GetBundleNameFromURI(bundlePath)
89+
bundleName, err := bundle.GetBundleNameFromURI(bundlePath)
90+
if err != nil {
91+
logging.Debugf("error getting bundle name from path %s: %v", bundlePath, err)
92+
return err
93+
}
9094
if _, err := bundle.Get(bundleName); err != nil {
9195
logging.Debugf("error getting bundle info for %s: %v", bundleName, err)
9296
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)