Consolidate the definition and consumption of --sign* CLI options

cmd/podman/artifact/push.go

@@ -19,13 +19,12 @@ import (
 // CLI-only fields into the API types.
 type pushOptionsWrapper struct {
 	entities.ArtifactPushOptions
-	TLSVerifyCLI               bool // CLI only
-	CredentialsCLI             string
-	SignPassphraseFileCLI      string
-	SignBySigstoreParamFileCLI string
-	EncryptionKeys             []string
-	EncryptLayers              []int
-	DigestFile                 string
+	TLSVerifyCLI   bool // CLI only
+	CredentialsCLI string
+	signing        common.SigningCLIOnlyOptions
+	EncryptionKeys []string
+	EncryptLayers  []int
+	DigestFile     string
 }
 
 var (
@@ -87,21 +86,7 @@ func pushFlags(cmd *cobra.Command) {
 	flags.String(retryDelayFlagName, registry.RetryDelayDefault(), "delay between retries in case of push failures")
 	_ = cmd.RegisterFlagCompletionFunc(retryDelayFlagName, completion.AutocompleteNone)
 
-	signByFlagName := "sign-by"
-	flags.StringVar(&pushOptions.SignBy, signByFlagName, "", "Add a signature at the destination using the specified key")
-	_ = cmd.RegisterFlagCompletionFunc(signByFlagName, completion.AutocompleteNone)
-
-	signBySigstoreFlagName := "sign-by-sigstore"
-	flags.StringVar(&pushOptions.SignBySigstoreParamFileCLI, signBySigstoreFlagName, "", "Sign the image using a sigstore parameter file at `PATH`")
-	_ = cmd.RegisterFlagCompletionFunc(signBySigstoreFlagName, completion.AutocompleteDefault)
-
-	signBySigstorePrivateKeyFlagName := "sign-by-sigstore-private-key"
-	flags.StringVar(&pushOptions.SignBySigstorePrivateKeyFile, signBySigstorePrivateKeyFlagName, "", "Sign the image using a sigstore private key at `PATH`")
-	_ = cmd.RegisterFlagCompletionFunc(signBySigstorePrivateKeyFlagName, completion.AutocompleteDefault)
-
-	signPassphraseFileFlagName := "sign-passphrase-file"
-	flags.StringVar(&pushOptions.SignPassphraseFileCLI, signPassphraseFileFlagName, "", "Read a passphrase for signing an image from `PATH`")
-	_ = cmd.RegisterFlagCompletionFunc(signPassphraseFileFlagName, completion.AutocompleteDefault)
+	common.DefineSigningFlags(cmd, &pushOptions.signing, &pushOptions.ArtifactPushOptions.ImagePushOptions)
 
 	flags.BoolVar(&pushOptions.TLSVerifyCLI, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries")
 
@@ -130,10 +115,6 @@ func pushFlags(cmd *cobra.Command) {
 		_ = flags.MarkHidden("cert-dir")
 		_ = flags.MarkHidden("compress")
 		_ = flags.MarkHidden("quiet")
-		_ = flags.MarkHidden(signByFlagName)
-		_ = flags.MarkHidden(signBySigstoreFlagName)
-		_ = flags.MarkHidden(signBySigstorePrivateKeyFlagName)
-		_ = flags.MarkHidden(signPassphraseFileFlagName)
 	} else {
 		signaturePolicyFlagName := "signature-policy"
 		flags.StringVar(&pushOptions.SignaturePolicy, signaturePolicyFlagName, "", "Path to a signature-policy file")
@@ -173,8 +154,7 @@ func artifactPush(cmd *cobra.Command, args []string) error {
 		pushOptions.Writer = os.Stderr
 	}
 
-	signingCleanup, err := common.PrepareSigning(&pushOptions.ImagePushOptions,
-		pushOptions.SignPassphraseFileCLI, pushOptions.SignBySigstoreParamFileCLI)
+	signingCleanup, err := common.PrepareSigning(&pushOptions.ImagePushOptions, &pushOptions.signing)
 	if err != nil {
 		return err
 	}

cmd/podman/common/sign.go

@@ -4,30 +4,66 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/containers/common/pkg/completion"
 	"github.com/containers/common/pkg/ssh"
 	"github.com/containers/image/v5/pkg/cli"
 	"github.com/containers/image/v5/pkg/cli/sigstore"
 	"github.com/containers/image/v5/signature/signer"
+	"github.com/containers/podman/v5/cmd/podman/registry"
 	"github.com/containers/podman/v5/pkg/domain/entities"
+	"github.com/spf13/cobra"
 )
 
-// PrepareSigning updates pushOpts.Signers, pushOpts.SignPassphrase and SignSigstorePrivateKeyPassphrase based on a --sign-passphrase-file
-// value signPassphraseFile and a --sign-by-sigsstore value signBySigstoreParamFile, and validates pushOpts.Sign* consistency.
+// SigningCLIOnlyOptions contains signing-related CLI options.
+// Some other options are defined in entities.ImagePushOptions.
+type SigningCLIOnlyOptions struct {
+	signPassphraseFile      string
+	signBySigstoreParamFile string
+}
+
+func DefineSigningFlags(cmd *cobra.Command, cliOpts *SigningCLIOnlyOptions, pushOpts *entities.ImagePushOptions) {
+	flags := cmd.Flags()
+
+	signByFlagName := "sign-by"
+	flags.StringVar(&pushOpts.SignBy, signByFlagName, "", "Add a signature at the destination using the specified key")
+	_ = cmd.RegisterFlagCompletionFunc(signByFlagName, completion.AutocompleteNone)
+
+	signBySigstoreFlagName := "sign-by-sigstore"
+	flags.StringVar(&cliOpts.signBySigstoreParamFile, signBySigstoreFlagName, "", "Sign the image using a sigstore parameter file at `PATH`")
+	_ = cmd.RegisterFlagCompletionFunc(signBySigstoreFlagName, completion.AutocompleteDefault)
+
+	signBySigstorePrivateKeyFlagName := "sign-by-sigstore-private-key"
+	flags.StringVar(&pushOpts.SignBySigstorePrivateKeyFile, signBySigstorePrivateKeyFlagName, "", "Sign the image using a sigstore private key at `PATH`")
+	_ = cmd.RegisterFlagCompletionFunc(signBySigstorePrivateKeyFlagName, completion.AutocompleteDefault)
+
+	signPassphraseFileFlagName := "sign-passphrase-file"
+	flags.StringVar(&cliOpts.signPassphraseFile, signPassphraseFileFlagName, "", "Read a passphrase for signing an image from `PATH`")
+	_ = cmd.RegisterFlagCompletionFunc(signPassphraseFileFlagName, completion.AutocompleteDefault)
+
+	if registry.IsRemote() {
+		_ = flags.MarkHidden(signByFlagName)
+		_ = flags.MarkHidden(signBySigstoreFlagName)
+		_ = flags.MarkHidden(signBySigstorePrivateKeyFlagName)
+		_ = flags.MarkHidden(signPassphraseFileFlagName)
+	}
+}
+
+// PrepareSigning updates pushOpts.Signers, pushOpts.SignPassphrase and SignSigstorePrivateKeyPassphrase based on cliOpts,
+// and validates pushOpts.Sign* consistency.
 // It may interactively prompt for a passphrase if one is required and wasn’t provided otherwise;
 // or it may interactively trigger an OIDC authentication, using standard input/output, or even open a web browser.
 // Returns a cleanup callback on success, which must be called when done.
-func PrepareSigning(pushOpts *entities.ImagePushOptions,
-	signPassphraseFile, signBySigstoreParamFile string) (func(), error) {
+func PrepareSigning(pushOpts *entities.ImagePushOptions, cliOpts *SigningCLIOnlyOptions) (func(), error) {
 	// c/common/libimage.Image does allow creating both simple signing and sigstore signatures simultaneously,
 	// with independent passphrases, but that would make the CLI probably too confusing.
 	// For now, use the passphrase with either, but only one of them.
-	if signPassphraseFile != "" && pushOpts.SignBy != "" && pushOpts.SignBySigstorePrivateKeyFile != "" {
+	if cliOpts.signPassphraseFile != "" && pushOpts.SignBy != "" && pushOpts.SignBySigstorePrivateKeyFile != "" {
 		return nil, fmt.Errorf("only one of --sign-by and sign-by-sigstore-private-key can be used with --sign-passphrase-file")
 	}
 
 	var passphrase string
-	if signPassphraseFile != "" {
-		p, err := cli.ReadPassphraseFile(signPassphraseFile)
+	if cliOpts.signPassphraseFile != "" {
+		p, err := cli.ReadPassphraseFile(cliOpts.signPassphraseFile)
 		if err != nil {
 			return nil, err
 		}
@@ -39,8 +75,8 @@ func PrepareSigning(pushOpts *entities.ImagePushOptions,
 	pushOpts.SignPassphrase = passphrase
 	pushOpts.SignSigstorePrivateKeyPassphrase = []byte(passphrase)
 	cleanup := signingCleanup{}
-	if signBySigstoreParamFile != "" {
-		signer, err := sigstore.NewSignerFromParameterFile(signBySigstoreParamFile, &sigstore.Options{
+	if cliOpts.signBySigstoreParamFile != "" {
+		signer, err := sigstore.NewSignerFromParameterFile(cliOpts.signBySigstoreParamFile, &sigstore.Options{
 			PrivateKeyPassphrasePrompt: cli.ReadPassphraseFile,
 			Stdin:                      os.Stdin,
 			Stdout:                     os.Stdout,

cmd/podman/images/push.go

@@ -19,13 +19,12 @@ import (
 // CLI-only fields into the API types.
 type pushOptionsWrapper struct {
 	entities.ImagePushOptions
-	TLSVerifyCLI               bool // CLI only
-	CredentialsCLI             string
-	SignPassphraseFileCLI      string
-	SignBySigstoreParamFileCLI string
-	EncryptionKeys             []string
-	EncryptLayers              []int
-	DigestFile                 string
+	TLSVerifyCLI   bool // CLI only
+	CredentialsCLI string
+	signing        common.SigningCLIOnlyOptions
+	EncryptionKeys []string
+	EncryptLayers  []int
+	DigestFile     string
 }
 
 var (
@@ -118,21 +117,7 @@ func pushFlags(cmd *cobra.Command) {
 	flags.String(retryDelayFlagName, registry.RetryDelayDefault(), "delay between retries in case of push failures")
 	_ = cmd.RegisterFlagCompletionFunc(retryDelayFlagName, completion.AutocompleteNone)
 
-	signByFlagName := "sign-by"
-	flags.StringVar(&pushOptions.SignBy, signByFlagName, "", "Add a signature at the destination using the specified key")
-	_ = cmd.RegisterFlagCompletionFunc(signByFlagName, completion.AutocompleteNone)
-
-	signBySigstoreFlagName := "sign-by-sigstore"
-	flags.StringVar(&pushOptions.SignBySigstoreParamFileCLI, signBySigstoreFlagName, "", "Sign the image using a sigstore parameter file at `PATH`")
-	_ = cmd.RegisterFlagCompletionFunc(signBySigstoreFlagName, completion.AutocompleteDefault)
-
-	signBySigstorePrivateKeyFlagName := "sign-by-sigstore-private-key"
-	flags.StringVar(&pushOptions.SignBySigstorePrivateKeyFile, signBySigstorePrivateKeyFlagName, "", "Sign the image using a sigstore private key at `PATH`")
-	_ = cmd.RegisterFlagCompletionFunc(signBySigstorePrivateKeyFlagName, completion.AutocompleteDefault)
-
-	signPassphraseFileFlagName := "sign-passphrase-file"
-	flags.StringVar(&pushOptions.SignPassphraseFileCLI, signPassphraseFileFlagName, "", "Read a passphrase for signing an image from `PATH`")
-	_ = cmd.RegisterFlagCompletionFunc(signPassphraseFileFlagName, completion.AutocompleteDefault)
+	common.DefineSigningFlags(cmd, &pushOptions.signing, &pushOptions.ImagePushOptions)
 
 	flags.BoolVar(&pushOptions.TLSVerifyCLI, "tls-verify", true, "Require HTTPS and verify certificates when contacting registries")
 
@@ -156,10 +141,6 @@ func pushFlags(cmd *cobra.Command) {
 		_ = flags.MarkHidden("cert-dir")
 		_ = flags.MarkHidden("compress")
 		_ = flags.MarkHidden("quiet")
-		_ = flags.MarkHidden(signByFlagName)
-		_ = flags.MarkHidden(signBySigstoreFlagName)
-		_ = flags.MarkHidden(signBySigstorePrivateKeyFlagName)
-		_ = flags.MarkHidden(signPassphraseFileFlagName)
 		_ = flags.MarkHidden(encryptionKeysFlagName)
 		_ = flags.MarkHidden(encryptLayersFlagName)
 	} else {
@@ -201,8 +182,7 @@ func imagePush(cmd *cobra.Command, args []string) error {
 		pushOptions.Writer = os.Stderr
 	}
 
-	signingCleanup, err := common.PrepareSigning(&pushOptions.ImagePushOptions,
-		pushOptions.SignPassphraseFileCLI, pushOptions.SignBySigstoreParamFileCLI)
+	signingCleanup, err := common.PrepareSigning(&pushOptions.ImagePushOptions, &pushOptions.signing)
 	if err != nil {
 		return err
 	}

cmd/podman/manifest/push.go

@@ -21,11 +21,10 @@ import (
 type manifestPushOptsWrapper struct {
 	entities.ImagePushOptions
 
-	TLSVerifyCLI, Insecure     bool // CLI only
-	CredentialsCLI             string
-	SignBySigstoreParamFileCLI string
-	SignPassphraseFileCLI      string
-	DigestFile                 string
+	TLSVerifyCLI, Insecure bool // CLI only
+	CredentialsCLI         string
+	signing                common.SigningCLIOnlyOptions
+	DigestFile             string
 }
 
 var (
@@ -81,21 +80,7 @@ func init() {
 
 	flags.BoolVarP(&manifestPushOpts.RemoveSignatures, "remove-signatures", "", false, "don't copy signatures when pushing images")
 
-	signByFlagName := "sign-by"
-	flags.StringVar(&manifestPushOpts.SignBy, signByFlagName, "", "sign the image using a GPG key with the specified `FINGERPRINT`")
-	_ = pushCmd.RegisterFlagCompletionFunc(signByFlagName, completion.AutocompleteNone)
-
-	signBySigstoreFlagName := "sign-by-sigstore"
-	flags.StringVar(&manifestPushOpts.SignBySigstoreParamFileCLI, signBySigstoreFlagName, "", "Sign the image using a sigstore parameter file at `PATH`")
-	_ = pushCmd.RegisterFlagCompletionFunc(signBySigstoreFlagName, completion.AutocompleteDefault)
-
-	signBySigstorePrivateKeyFlagName := "sign-by-sigstore-private-key"
-	flags.StringVar(&manifestPushOpts.SignBySigstorePrivateKeyFile, signBySigstorePrivateKeyFlagName, "", "Sign the image using a sigstore private key at `PATH`")
-	_ = pushCmd.RegisterFlagCompletionFunc(signBySigstorePrivateKeyFlagName, completion.AutocompleteDefault)
-
-	signPassphraseFileFlagName := "sign-passphrase-file"
-	flags.StringVar(&manifestPushOpts.SignPassphraseFileCLI, signPassphraseFileFlagName, "", "Read a passphrase for signing an image from `PATH`")
-	_ = pushCmd.RegisterFlagCompletionFunc(signPassphraseFileFlagName, completion.AutocompleteDefault)
+	common.DefineSigningFlags(pushCmd, &manifestPushOpts.signing, &manifestPushOpts.ImagePushOptions)
 
 	flags.BoolVar(&manifestPushOpts.TLSVerifyCLI, "tls-verify", true, "require HTTPS and verify certificates when accessing the registry")
 	flags.BoolVar(&manifestPushOpts.Insecure, "insecure", false, "neither require HTTPS nor verify certificates when accessing the registry")
@@ -113,10 +98,6 @@ func init() {
 
 	if registry.IsRemote() {
 		_ = flags.MarkHidden("cert-dir")
-		_ = flags.MarkHidden(signByFlagName)
-		_ = flags.MarkHidden(signBySigstoreFlagName)
-		_ = flags.MarkHidden(signBySigstorePrivateKeyFlagName)
-		_ = flags.MarkHidden(signPassphraseFileFlagName)
 	}
 }
 
@@ -148,8 +129,7 @@ func push(cmd *cobra.Command, args []string) error {
 		manifestPushOpts.Writer = os.Stderr
 	}
 
-	signingCleanup, err := common.PrepareSigning(&manifestPushOpts.ImagePushOptions,
-		manifestPushOpts.SignPassphraseFileCLI, manifestPushOpts.SignBySigstoreParamFileCLI)
+	signingCleanup, err := common.PrepareSigning(&manifestPushOpts.ImagePushOptions, &manifestPushOpts.signing)
 	if err != nil {
 		return err
 	}

pkg/domain/entities/artifact.go

@@ -81,13 +81,9 @@ type ArtifactPullOptions struct {
 
 type ArtifactPushOptions struct {
 	ImagePushOptions
-	CredentialsCLI             string
-	DigestFile                 string
-	EncryptLayers              []int
-	EncryptionKeys             []string
-	SignBySigstoreParamFileCLI string
-	SignPassphraseFileCLI      string
-	TLSVerifyCLI               bool // CLI only
+	DigestFile     string
+	EncryptLayers  []int
+	EncryptionKeys []string
 }
 
 type ArtifactRemoveOptions struct {

pkg/domain/infra/abi/artifact.go

@@ -181,9 +181,8 @@ func (ir *ImageEngine) ArtifactPush(ctx context.Context, name string, opts entit
 		Architecture:                     "",
 		OS:                               "",
 		Variant:                          "",
-		Username:                         "",
-		Password:                         "",
-		Credentials:                      opts.CredentialsCLI,
+		Username:                         opts.Username,
+		Password:                         opts.Password,
 		IdentityToken:                    "",
 		Writer:                           opts.Writer,
 	}

test/e2e/artifact_test.go

@@ -6,9 +6,11 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	. "github.com/containers/podman/v5/test/utils"
+	"github.com/containers/podman/v5/utils"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -158,6 +160,47 @@ var _ = Describe("Podman artifact", func() {
 		Expect(a.Name).To(Equal(artifact1Name))
 	})
 
+	It("podman artifact push with authorization", func() {
+		portNo, err := utils.GetRandomPort()
+		Expect(err).ToNot(HaveOccurred())
+		port := strconv.Itoa(portNo)
+
+		lock := GetPortLock(port)
+		defer lock.Unlock()
+
+		artifact1File, err := createArtifactFile(1024)
+		Expect(err).ToNot(HaveOccurred())
+		artifact1Name := fmt.Sprintf("localhost:%s/test/artifact1", port)
+		podmanTest.PodmanExitCleanly("artifact", "add", artifact1Name, artifact1File)
+
+		authPath := filepath.Join(podmanTest.TempDir, "auth")
+		err = os.Mkdir(authPath, os.ModePerm)
+		Expect(err).ToNot(HaveOccurred())
+		htpasswd := SystemExec("htpasswd", []string{"-Bbn", "podmantest", "test"})
+		htpasswd.WaitWithDefaultTimeout()
+		Expect(htpasswd).Should(ExitCleanly())
+
+		f, err := os.Create(filepath.Join(authPath, "htpasswd"))
+		Expect(err).ToNot(HaveOccurred())
+		defer f.Close()
+		_, err = f.WriteString(htpasswd.OutputToString())
+		Expect(err).ToNot(HaveOccurred())
+		err = f.Sync()
+		Expect(err).ToNot(HaveOccurred())
+
+		podmanTest.PodmanExitCleanly("run", "-d", "-p", port+":5000", "--name", "artifact-creds-registry", "-v",
+			strings.Join([]string{authPath, "/auth", "z"}, ":"), "-e", "REGISTRY_AUTH=htpasswd", "-e",
+			"REGISTRY_AUTH_HTPASSWD_REALM=Registry Realm", "-e", "REGISTRY_AUTH_HTPASSWD_PATH=/auth/htpasswd",
+			REGISTRY_IMAGE)
+		Expect(WaitContainerReady(podmanTest, "artifact-creds-registry", "listening on", 20, 1)).To(BeTrue(), "registry container ready")
+
+		push := podmanTest.Podman([]string{"artifact", "push", "--tls-verify=false", "--creds=podmantest:wrongpasswd", artifact1Name})
+		push.WaitWithDefaultTimeout()
+		Expect(push).To(ExitWithError(125, "/artifact1: authentication required"))
+
+		podmanTest.PodmanExitCleanly("artifact", "push", "-q", "--tls-verify=false", "--creds=podmantest:test", artifact1Name)
+	})
+
 	It("podman artifact remove", func() {
 		// Trying to remove an image that does not exist should fail
 		rmFail := podmanTest.Podman([]string{"artifact", "rm", "foobar"})