Farm build should read server registries.conf

Fix the way we set skipTLSVerify on the client side to ensure that the push stage in farm build takes into account the configuration in the farm node's registries.conf when the user hasn't set it on the client side. Signed-off-by: Urvashi Mohnani <[email protected]>
containers · Jan 31, 2024 · a06685a · a06685a
1 parent 1a8cb15
commit a06685a
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 12 deletions.
diff --git a/cmd/podman/farm/build.go b/cmd/podman/farm/build.go
@@ -109,11 +109,17 @@ func build(cmd *cobra.Command, args []string) error {
 		return err
 	}
 	opts.IIDFile = iidFile
-	tlsVerify, err := cmd.Flags().GetBool("tls-verify")
-	if err != nil {
-		return err
+	// only set tls-verify if it has been changed by the user
+	// if it hasn't we will read the registries.conf on the farm
+	// nodes for further configuration
+	if changed := cmd.Flags().Changed("tls-verify"); changed {
+		tlsVerify, err := cmd.Flags().GetBool("tls-verify")
+		if err != nil {
+			return err
+		}
+		skipTLSVerify := !tlsVerify
+		opts.SkipTLSVerify = &skipTLSVerify
 	}
-	opts.SkipTLSVerify = !tlsVerify
 
 	localEngine := registry.ImageEngine()
 	ctx := registry.Context()

diff --git a/pkg/domain/entities/types/types.go b/pkg/domain/entities/types/types.go
@@ -56,7 +56,7 @@ type FarmBuildOptions struct {
 	// Authfile is the path to the file holding registry credentials
 	Authfile string
 	// SkipTLSVerify skips tls verification when set to true
-	SkipTLSVerify bool
+	SkipTLSVerify *bool
 }
 
 // BuildOptions describe the options for building container images.

diff --git a/pkg/farm/list_builder.go b/pkg/farm/list_builder.go
@@ -17,7 +17,7 @@ type listBuilderOptions struct {
 	cleanup       bool
 	iidFile       string
 	authfile      string
-	skipTLSVerify bool
+	skipTLSVerify *bool
 }
 
 type listLocal struct {
@@ -39,13 +39,19 @@ func newManifestListBuilder(listName string, localEngine entities.ImageEngine, o
 // Build retrieves images from the build reports and assembles them into a
 // manifest list in local container storage.
 func (l *listLocal) build(ctx context.Context, images map[entities.BuildReport]entities.ImageEngine) (string, error) {
+	// Set skipTLSVerify based on whether it was changed by the caller
+	skipTLSVerify := types.OptionalBoolUndefined
+	if l.options.skipTLSVerify != nil {
+		skipTLSVerify = types.NewOptionalBool(*l.options.skipTLSVerify)
+	}
+
 	exists, err := l.localEngine.ManifestExists(ctx, l.listName)
 	if err != nil {
 		return "", err
 	}
 	// Create list if it doesn't exist
 	if !exists.Value {
-		_, err = l.localEngine.ManifestCreate(ctx, l.listName, []string{}, entities.ManifestCreateOptions{SkipTLSVerify: types.NewOptionalBool(l.options.skipTLSVerify)})
+		_, err = l.localEngine.ManifestCreate(ctx, l.listName, []string{}, entities.ManifestCreateOptions{SkipTLSVerify: skipTLSVerify})
 		if err != nil {
 			return "", fmt.Errorf("creating manifest list %q: %w", l.listName, err)
 		}
@@ -63,7 +69,7 @@ func (l *listLocal) build(ctx context.Context, images map[entities.BuildReport]e
 			logrus.Infof("pushing image %s", image.ID)
 			defer logrus.Infof("pushed image %s", image.ID)
 			// Push the image to the registry
-			report, err := engine.Push(ctx, image.ID, l.listName+docker.UnknownDigestSuffix, entities.ImagePushOptions{Authfile: l.options.authfile, Quiet: false, SkipTLSVerify: types.NewOptionalBool(l.options.skipTLSVerify)})
+			report, err := engine.Push(ctx, image.ID, l.listName+docker.UnknownDigestSuffix, entities.ImagePushOptions{Authfile: l.options.authfile, Quiet: false, SkipTLSVerify: skipTLSVerify})
 			if err != nil {
 				return fmt.Errorf("pushing image %q to registry: %w", image, err)
 			}
@@ -111,11 +117,11 @@ func (l *listLocal) build(ctx context.Context, images map[entities.BuildReport]e
 	}
 
 	// Add the images to the list
-	listID, err := l.localEngine.ManifestAdd(ctx, l.listName, refs, entities.ManifestAddOptions{Authfile: l.options.authfile, SkipTLSVerify: types.NewOptionalBool(l.options.skipTLSVerify)})
+	listID, err := l.localEngine.ManifestAdd(ctx, l.listName, refs, entities.ManifestAddOptions{Authfile: l.options.authfile, SkipTLSVerify: skipTLSVerify})
 	if err != nil {
 		return "", fmt.Errorf("adding images %q to list: %w", refs, err)
 	}
-	_, err = l.localEngine.ManifestPush(ctx, l.listName, l.listName, entities.ImagePushOptions{Authfile: l.options.authfile, SkipTLSVerify: types.NewOptionalBool(l.options.skipTLSVerify)})
+	_, err = l.localEngine.ManifestPush(ctx, l.listName, l.listName, entities.ImagePushOptions{Authfile: l.options.authfile, SkipTLSVerify: skipTLSVerify})
 	if err != nil {
 		return "", err
 	}

diff --git a/test/farm/001-farm.bats b/test/farm/001-farm.bats
@@ -85,10 +85,37 @@ load helpers.bash
     run_podman image prune -f
 }
 
+@test "farm - build on farm node only with registries.conf" {
+    cat >$PODMAN_TMPDIR/registries.conf <<EOF
+[[registry]]
+location="$REGISTRY"
+insecure=true
+EOF
+
+    iname="test-image-4"
+    CONTAINERS_REGISTRIES_CONF="$PODMAN_TMPDIR/registries.conf" run_podman farm build --authfile $AUTHFILE -t $REGISTRY/$iname $FARM_TMPDIR
+    assert "$output" =~ "Farm \"$FARMNAME\" ready"
+
+    # get the system architecture
+    CONTAINERS_REGISTRIES_CONF="$PODMAN_TMPDIR/registries.conf" run_podman info --format '{{.Host.Arch}}'
+    ARCH=$output
+    # inspect manifest list built and saved
+    CONTAINERS_REGISTRIES_CONF="$PODMAN_TMPDIR/registries.conf" run_podman manifest inspect $iname
+    assert "$output" =~ $ARCH
+
+    echo "# skopeo inspect ..."
+    run skopeo inspect "$@" --tls-verify=false --authfile $AUTHFILE docker://$REGISTRY/$iname
+    echo "$output"
+    is "$status" "0" "skopeo inspect - exit status"
+
+    run_podman manifest rm $iname
+    run_podman image prune -f
+}
+
 # Test out podman-remote
 
 @test "farm - build on farm node only (podman-remote)" {
-    iname="test-image-4"
+    iname="test-image-5"
     run_podman --remote farm build --authfile $AUTHFILE --tls-verify=false -t $REGISTRY/$iname $FARM_TMPDIR
     assert "$output" =~ "Farm \"$FARMNAME\" ready"
 

diff --git a/test/farm/setup_suite.bash b/test/farm/setup_suite.bash
@@ -36,7 +36,7 @@ function setup_suite(){
     run_podman system connection add --identity $sshkey test-node $ROOTLESS_USER@localhost
     run_podman farm create $FARMNAME test-node
 
-     export PODMAN_LOGIN_WORKDIR=$(mktemp -d --tmpdir=${BATS_TMPDIR:-${TMPDIR:-/tmp}} podman-bats-registry.XXXXXX)
+    export PODMAN_LOGIN_WORKDIR=$(mktemp -d --tmpdir=${BATS_TMPDIR:-${TMPDIR:-/tmp}} podman-bats-registry.XXXXXX)
 
     export PODMAN_LOGIN_USER="user$(random_string 4)"
     export PODMAN_LOGIN_PASS="pw$(random_string 15)"