Deprecate --local flag (HMS-3792)

README.md

@@ -56,7 +56,6 @@ sudo podman run \
     -v /var/lib/containers/storage:/var/lib/containers/storage \
     quay.io/centos-bootc/bootc-image-builder:latest \
     --type qcow2 \
-    --local \
     quay.io/centos-bootc/centos-bootc:stream9
 ```
 
@@ -127,7 +126,6 @@ Usage:
 
 Flags:
       --chown string           chown the ouput directory to match the specified UID:GID
-      --tls-verify             require HTTPS and verify certificates when contacting registries (default true)
       --type string            image type to build [qcow2, ami] (default "qcow2")
       --target-arch string     architecture to build image for (default is the native architecture)
 ```
@@ -138,7 +136,6 @@ Flags:
 |-------------------|-----------------------------------------------------------------------------------------------------------|:-------------:|
 | **--chown**       | chown the output directory to match the specified UID:GID                                                 |       ❌      |
 | **--rootfs**      | Root filesystem type. Overrides the default from the source container. Supported values: ext4, xfs, btrfs |       ❌      |
-| **--tls-verify**  | Require HTTPS and verify certificates when contacting registries                                          |    `true`     |
 | **--type**        | [Image type](#-image-types) to build                                                                      |    `qcow2`    |
 | **--target-arch** | [Target arch](#-target-architecture) to build                                                             |       ❌      |
 

bib/cmd/bootc-image-builder/image.go

@@ -47,9 +47,6 @@ type ManifestConfig struct {
 	// CPU architecture of the image
 	Architecture arch.Arch
 
-	// TLSVerify specifies whether HTTPS and a valid TLS certificate are required
-	TLSVerify bool
-
 	// The minimum size required for the root fs in order to fit the container
 	// contents
 	RootfsMinsize uint64
@@ -235,10 +232,9 @@ func manifestForDiskImage(c *ManifestConfig, rng *rand.Rand) (*manifest.Manifest
 		return nil, fmt.Errorf("pipeline: no base image defined")
 	}
 	containerSource := container.SourceSpec{
-		Source:    c.Imgref,
-		Name:      c.Imgref,
-		TLSVerify: &c.TLSVerify,
-		Local:     true,
+		Source: c.Imgref,
+		Name:   c.Imgref,
+		Local:  true,
 	}
 
 	var customizations *blueprint.Customizations
@@ -351,10 +347,9 @@ func manifestForISO(c *ManifestConfig, rng *rand.Rand) (*manifest.Manifest, erro
 	}
 
 	containerSource := container.SourceSpec{
-		Source:    c.Imgref,
-		Name:      c.Imgref,
-		TLSVerify: &c.TLSVerify,
-		Local:     true,
+		Source: c.Imgref,
+		Name:   c.Imgref,
+		Local:  true,
 	}
 
 	// The ref is not needed and will be removed from the ctor later

bib/cmd/bootc-image-builder/main.go

@@ -186,10 +186,19 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
 	imgTypes, _ := cmd.Flags().GetStringArray("type")
 	rpmCacheRoot, _ := cmd.Flags().GetString("rpmmd")
 	targetArch, _ := cmd.Flags().GetString("target-arch")
-	tlsVerify, _ := cmd.Flags().GetBool("tls-verify")
-	localStorage, _ := cmd.Flags().GetBool("local")
 	rootFs, _ := cmd.Flags().GetString("rootfs")
 
+	// If --local was given, warn in the case of --local or --local=true (true is the default), error in the case of --local=false
+	if cmd.Flags().Changed("local") {
+		localStorage, _ := cmd.Flags().GetBool("local")
+		if localStorage {
+			fmt.Fprintf(os.Stderr, "WARNING: --local is now the default behavior, you can remove it from the command line\n")
+		} else {
+			return nil, nil, fmt.Errorf(`--local=false is no longer supported, remove it and make sure to pull the container before running bib:
+	sudo podman pull %s`, imgref)
+		}
+	}
+
 	if targetArch != "" && arch.FromString(targetArch) != arch.Current() {
 		// TODO: detect if binfmt_misc for target arch is
 		// available, e.g. by mounting the binfmt_misc fs into
@@ -204,10 +213,8 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
 	}
 	// TODO: add "target-variant", see https://github.com/osbuild/bootc-image-builder/pull/139/files#r1467591868
 
-	if localStorage {
-		if err := setup.ValidateHasContainerStorageMounted(); err != nil {
-			return nil, nil, fmt.Errorf("local storage not working, did you forget -v /var/lib/containers/storage:/var/lib/containers/storage? (%w)", err)
-		}
+	if err := setup.ValidateHasContainerStorageMounted(); err != nil {
+		return nil, nil, fmt.Errorf("could not access container storage, did you forget -v /var/lib/containers/storage:/var/lib/containers/storage? (%w)", err)
 	}
 
 	imageTypes, err := imagetypes.New(imgTypes...)
@@ -220,27 +227,6 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
 		return nil, nil, fmt.Errorf("cannot read config: %w", err)
 	}
 
-	// If --local wasn't given, always pull the container.
-	// If the user mount a container storage inside bib (without --local), the code will try to pull
-	// a newer version of the container even if an older one is already present. This doesn't match
-	// how `podman run`` behaves by default, but it matches the bib's behaviour before the switch
-	// to using containers storage in all code paths happened.
-	// We might want to change this behaviour in the future to match podman.
-	if !localStorage {
-		logrus.Infof("Pulling image %s (arch=%s)\n", imgref, cntArch)
-		cmd := exec.Command("podman", "pull", "--arch", cntArch.String(), fmt.Sprintf("--tls-verify=%v", tlsVerify), imgref)
-		// podman prints progress on stderr so connect that to give
-		// better UX. But do not connect stdout as "bib manifest"
-		// needs to be purely json so any stray output there is a
-		// problem
-		cmd.Stderr = os.Stderr
-		if err := cmd.Run(); err != nil {
-			return nil, nil, fmt.Errorf("failed to pull container image: %w", util.OutputErr(err))
-		}
-	} else {
-		logrus.Debug("Using local container")
-	}
-
 	if err := setup.ValidateHasContainerTags(imgref); err != nil {
 		return nil, nil, err
 	}
@@ -302,7 +288,6 @@ func manifestFromCobra(cmd *cobra.Command, args []string) ([]byte, *mTLSConfig,
 		Config:           config,
 		ImageTypes:       imageTypes,
 		Imgref:           imgref,
-		TLSVerify:        tlsVerify,
 		RootfsMinsize:    cntSize * containerSizeToDiskSizeMultiplier,
 		DistroDefPaths:   distroDefPaths,
 		SourceInfo:       sourceinfo,
@@ -591,11 +576,17 @@ func buildCobraCmdline() (*cobra.Command, error) {
 	rootCmd.AddCommand(versionCmd)
 
 	rootCmd.AddCommand(manifestCmd)
-	manifestCmd.Flags().Bool("tls-verify", true, "require HTTPS and verify certificates when contacting registries")
+	manifestCmd.Flags().Bool("tls-verify", false, "DEPRECATED: require HTTPS and verify certificates when contacting registries")
+	if err := manifestCmd.Flags().MarkHidden("tls-verify"); err != nil {
+		return nil, fmt.Errorf("cannot hide 'tls-verify' :%w", err)
+	}
 	manifestCmd.Flags().String("rpmmd", "/rpmmd", "rpm metadata cache directory")
 	manifestCmd.Flags().String("target-arch", "", "build for the given target architecture (experimental)")
 	manifestCmd.Flags().StringArray("type", []string{"qcow2"}, fmt.Sprintf("image types to build [%s]", imagetypes.Available()))
-	manifestCmd.Flags().Bool("local", false, "use a local container rather than a container from a registry")
+	manifestCmd.Flags().Bool("local", true, "DEPRECATED: --local is now the default behavior, make sure to pull the container image before running bootc-image-builder")
+	if err := manifestCmd.Flags().MarkHidden("local"); err != nil {
+		return nil, fmt.Errorf("cannot hide 'local' :%w", err)
+	}
 	manifestCmd.Flags().String("rootfs", "", "Root filesystem type. If not given, the default configured in the source container image is used.")
 	// --config is only useful for developers who run bib outside
 	// of a container to generate a manifest. so hide it by

bib/internal/setup/setup.go

@@ -153,7 +153,9 @@ func validateCanRunTargetArch(targetArch string) error {
 func ValidateHasContainerTags(imgref string) error {
 	output, err := exec.Command("podman", "image", "inspect", imgref, "--format", "{{.Labels}}").Output()
 	if err != nil {
-		return fmt.Errorf("failed inspect image: %w", util.OutputErr(err))
+		return fmt.Errorf(`failed to inspect the image: %w
+bootc-image-builder no longer pulls images, make sure to pull it before running bootc-image-builder:
+    sudo podman pull %s`, util.OutputErr(err), imgref)
 	}
 
 	tags := string(output)

test/test_build.py

@@ -2,10 +2,8 @@
 import os
 import pathlib
 import platform
-import random
 import re
 import shutil
-import string
 import subprocess
 import tempfile
 import uuid
@@ -61,20 +59,7 @@ def image_type_fixture(shared_tmpdir, build_container, request, force_aws_upload
     In the case an image is being built from a local container, the
     function will build the required local container for the test.
     """
-    container_ref = request.param.container_ref
-
-    if request.param.local:
-        cont_tag = "localhost/cont-base-" + "".join(random.choices(string.digits, k=12))
-
-        # we are not cross-building local images (for now)
-        request.param.target_arch = ""
-
-        # copy the container into containers-storage
-        subprocess.check_call([
-            "skopeo", "copy",
-            f"docker://{container_ref}",
-            f"containers-storage:[overlay@/var/lib/containers/storage+/run/containers/storage]{cont_tag}"
-        ])
+    testutil.pull_container(request.param.container_ref, request.param.target_arch)
 
     with build_images(shared_tmpdir, build_container, request, force_aws_upload) as build_results:
         yield build_results[0]
@@ -86,6 +71,8 @@ def images_fixture(shared_tmpdir, build_container, request, force_aws_upload):
     Build one or more images inside the passed build_container and return an
     ImageBuildResult array with the resulting image path and user/password
     """
+    testutil.pull_container(request.param.container_ref, request.param.target_arch)
+
     with build_images(shared_tmpdir, build_container, request, force_aws_upload) as build_results:
         yield build_results
 
@@ -251,12 +238,9 @@ def build_images(shared_tmpdir, build_container, request, force_aws_upload):
             "-v", f"{config_json_path}:/config.json:ro",
             "-v", f"{output_path}:/output",
             "-v", "/var/tmp/osbuild-test-store:/store",  # share the cache between builds
+            "-v", "/var/lib/containers/storage:/var/lib/containers/storage",  # mount the host's containers storage
         ]
 
-        # we need to mount the host's container store
-        if tc.local:
-            cmd.extend(["-v", "/var/lib/containers/storage:/var/lib/containers/storage"])
-
         cmd.extend([
             *creds_args,
             build_container,
@@ -265,7 +249,6 @@ def build_images(shared_tmpdir, build_container, request, force_aws_upload):
             *upload_args,
             *target_arch_args,
             *tc.bib_rootfs_args(),
-            "--local" if tc.local else "--local=false",
         ])
 
         # print the build command for easier tracing

test/test_manifest.py

@@ -31,6 +31,8 @@ def find_image_size_from(manifest_str):
 
 @pytest.mark.parametrize("tc", gen_testcases("manifest"))
 def test_manifest_smoke(build_container, tc):
+    testutil.pull_container(tc.container_ref, tc.target_arch)
+
     output = subprocess.check_output([
         *testutil.podman_run_common,
         build_container,
@@ -50,6 +52,8 @@ def test_manifest_smoke(build_container, tc):
 
 @pytest.mark.parametrize("tc", gen_testcases("anaconda-iso"))
 def test_iso_manifest_smoke(build_container, tc):
+    testutil.pull_container(tc.container_ref, tc.target_arch)
+
     output = subprocess.check_output([
         *testutil.podman_run_common,
         build_container,
@@ -81,7 +85,7 @@ def test_manifest_disksize(tmp_path, build_container, tc):
         manifest_str = subprocess.check_output([
             *testutil.podman_run_common,
             build_container,
-            "manifest", "--local",
+            "manifest",
             *tc.bib_rootfs_args(),
             f"localhost/{container_tag}",
         ], encoding="utf8")
@@ -100,10 +104,11 @@ def test_manifest_local_checks_containers_storage_errors(build_container):
         "--privileged",
         "--security-opt", "label=type:unconfined_t",
         build_container,
-        "manifest", "--local", "arg-not-used",
+        "manifest", "arg-not-used",
     ], check=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf8")
     assert res.returncode == 1
-    err = 'local storage not working, did you forget -v /var/lib/containers/storage:/var/lib/containers/storage?'
+    err = 'could not access container storage, ' + \
+        'did you forget -v /var/lib/containers/storage:/var/lib/containers/storage?'
     assert err in res.stderr
 
 
@@ -118,7 +123,7 @@ def test_manifest_local_checks_containers_storage_works(tmp_path, build_containe
         subprocess.run([
             *testutil.podman_run_common,
             build_container,
-            "manifest", "--local",
+            "manifest",
             *tc.bib_rootfs_args(),
             f"localhost/{container_tag}",
         ], check=True, encoding="utf8")
@@ -138,7 +143,7 @@ def test_manifest_cross_arch_check(tmp_path, build_container):
                 *testutil.podman_run_common,
                 build_container,
                 "manifest", "--target-arch=aarch64",
-                "--local", f"localhost/{container_tag}"
+                f"localhost/{container_tag}"
             ], check=True, capture_output=True, encoding="utf8")
         assert 'image found is for unexpected architecture "x86_64"' in exc.value.stderr
 
@@ -157,6 +162,7 @@ def find_rootfs_type_from(manifest_str):
 @pytest.mark.parametrize("tc", gen_testcases("default-rootfs"))
 def test_manifest_rootfs_respected(build_container, tc):
     # TODO: derive container and fake "bootc install print-configuration"?
+    testutil.pull_container(tc.container_ref)
     output = subprocess.check_output([
         *testutil.podman_run_common,
         build_container,
@@ -196,6 +202,7 @@ def find_user_stage_from(manifest_str):
 def test_manifest_user_customizations_toml(tmp_path, build_container):
     # no need to parameterize this test, toml is the same for all containers
     container_ref = "quay.io/centos-bootc/centos-bootc:stream9"
+    testutil.pull_container(container_ref)
 
     config_toml_path = tmp_path / "config.toml"
     config_toml_path.write_text(textwrap.dedent("""\
@@ -223,6 +230,7 @@ def test_manifest_user_customizations_toml(tmp_path, build_container):
 
 def test_manifest_installer_customizations(tmp_path, build_container):
     container_ref = "quay.io/centos-bootc/centos-bootc:stream9"
+    testutil.pull_container(container_ref)
 
     config_toml_path = tmp_path / "config.toml"
     config_toml_path.write_text(textwrap.dedent("""\
@@ -256,6 +264,7 @@ def test_manifest_installer_customizations(tmp_path, build_container):
 def test_mount_ostree_error(tmpdir_factory, build_container):
     # no need to parameterize this test, toml is the same for all containers
     container_ref = "quay.io/centos-bootc/centos-bootc:stream9"
+    testutil.pull_container(container_ref)
 
     cfg = {
         "blueprint": {
@@ -304,6 +313,7 @@ def test_mount_ostree_error(tmpdir_factory, build_container):
 )
 def test_manifest_checks_build_container_is_bootc(build_container, container_ref, should_error, expected_error):
     def check_image_ref():
+        testutil.pull_container(container_ref)
         subprocess.check_output([
             *testutil.podman_run_common,
             build_container,
@@ -320,6 +330,8 @@ def check_image_ref():
 
 @pytest.mark.parametrize("tc", gen_testcases("target-arch-smoke"))
 def test_manifest_target_arch_smoke(build_container, tc):
+    testutil.pull_container(tc.container_ref, tc.target_arch)
+
     # TODO: actually build an image too
     output = subprocess.check_output([
         *testutil.podman_run_common,
@@ -372,6 +384,8 @@ def test_manifest_anaconda_module_customizations(tmpdir_factory, build_container
     config_json_path = output_path / "config.json"
     config_json_path.write_text(json.dumps(cfg), encoding="utf-8")
 
+    testutil.pull_container(tc.container_ref, tc.target_arch)
+
     output = subprocess.check_output([
         *testutil.podman_run_common,
         "-v", f"{output_path}:/output",
@@ -410,6 +424,7 @@ def find_fstab_stage_from(manifest_str):
 ])
 def test_manifest_fs_customizations(tmp_path, build_container, fscustomizations, rootfs):
     container_ref = "quay.io/centos-bootc/centos-bootc:stream9"
+    testutil.pull_container(container_ref)
 
     config = {
         "customizations": {
@@ -496,7 +511,9 @@ def assert_fs_customizations(customizations, fstype, manifest):
     ({}, "btrfs"),
 ])
 def test_manifest_fs_customizations_xarch(tmp_path, build_container, fscustomizations, rootfs):
+    target_arch = "aarch64"
     container_ref = "quay.io/centos-bootc/centos-bootc:stream9"
+    testutil.pull_container(container_ref, target_arch)
 
     config = {
         "customizations": {
@@ -512,7 +529,7 @@ def test_manifest_fs_customizations_xarch(tmp_path, build_container, fscustomiza
         "--entrypoint=/usr/bin/bootc-image-builder",
         build_container,
         f"--rootfs={rootfs}",
-        "--target-arch=aarch64",
+        f"--target-arch={target_arch}",
         "manifest", f"{container_ref}",
     ])