remote: fix incorrect CONTAINER_CONNECTION parsing

When a user specifies a invalid connection in CONTAINER_CONNECTION then
podman should return a proper error saying so. Currently it ignored the
error and in rootFlags() just exited early with defining any flags. This
caused a panic then when trying to use the flags later.

In order to address this first store the connection error in the
PodmanConfig struct and not abort right away during flag setup. This is
important as the user might have specified a flag with a valid remote
connection. As such we check all flags and only when none were given we
return the connection error.

Also while at it I noticed that the default connection reported via
podman --help was wrong as it only used the old containers.conf field
for it and did not consider the podman-connections.json default.

New regression tests have been added to make sure it behaves correctly.

This fixes the problem reported in the PR #22997.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

cmd/podman/root.go

@@ -175,6 +175,12 @@ func readRemoteCliFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig)
 		}
 	case host.Changed:
 		podmanConfig.URI = host.Value.String()
+	default:
+		// No cli options set, in case CONTAINER_CONNECTION was set to something
+		// invalid this contains the error, see setupRemoteConnection().
+		// Important so that we can show a proper useful error message but still
+		// allow the cli overwrites (https://github.com/containers/podman/pull/22997).
+		return podmanConfig.ConnectionError
 	}
 	return nil
 }
@@ -185,7 +191,8 @@ func readRemoteCliFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig)
 // 2. Env variables (CONTAINER_HOST and CONTAINER_CONNECTION);
 // 3. ActiveService from containers.conf;
 // 4. RemoteURI;
-func setupRemoteConnection(podmanConfig *entities.PodmanConfig) error {
+// Returns the name of the default connection if any.
+func setupRemoteConnection(podmanConfig *entities.PodmanConfig) string {
 	conf := podmanConfig.ContainersConfDefaultsRO
 	connEnv, hostEnv, sshkeyEnv := os.Getenv("CONTAINER_CONNECTION"), os.Getenv("CONTAINER_HOST"), os.Getenv("CONTAINER_SSHKEY")
 
@@ -193,11 +200,13 @@ func setupRemoteConnection(podmanConfig *entities.PodmanConfig) error {
 	case connEnv != "":
 		con, err := conf.GetConnection(connEnv, false)
 		if err != nil {
-			return err
+			podmanConfig.ConnectionError = err
+			return connEnv
 		}
 		podmanConfig.URI = con.URI
 		podmanConfig.Identity = con.Identity
 		podmanConfig.MachineMode = con.IsMachine
+		return con.Name
 	case hostEnv != "":
 		if sshkeyEnv != "" {
 			podmanConfig.Identity = sshkeyEnv
@@ -209,11 +218,11 @@ func setupRemoteConnection(podmanConfig *entities.PodmanConfig) error {
 			podmanConfig.URI = con.URI
 			podmanConfig.Identity = con.Identity
 			podmanConfig.MachineMode = con.IsMachine
-		} else {
+			return con.Name
-			podmanConfig.URI = registry.DefaultAPIAddress()
 		}
+		podmanConfig.URI = registry.DefaultAPIAddress()
 	}
-	return nil
+	return ""
 }
 
 func persistentPreRunE(cmd *cobra.Command, args []string) error {
@@ -463,9 +472,8 @@ func stdOutHook() {
 }
 
 func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
-	if err := setupRemoteConnection(podmanConfig); err != nil {
+	connectionName := setupRemoteConnection(podmanConfig)
-		return
+
-	}
 	lFlags := cmd.Flags()
 
 	sshFlagName := "ssh"
@@ -473,7 +481,7 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
 	_ = cmd.RegisterFlagCompletionFunc(sshFlagName, common.AutocompleteSSH)
 
 	connectionFlagName := "connection"
-	lFlags.StringP(connectionFlagName, "c", podmanConfig.ContainersConfDefaultsRO.Engine.ActiveService, "Connection to use for remote Podman service")
+	lFlags.StringP(connectionFlagName, "c", connectionName, "Connection to use for remote Podman service (CONTAINER_CONNECTION)")
 	_ = cmd.RegisterFlagCompletionFunc(connectionFlagName, common.AutocompleteSystemConnections)
 
 	urlFlagName := "url"

pkg/domain/entities/engine.go

@@ -48,6 +48,7 @@ type PodmanConfig struct {
 	Trace                    bool     // Hidden: Trace execution
 	URI                      string   // URI to RESTful API Service
 	FarmNodeName             string   // Name of farm node
+	ConnectionError          error    // Error when looking up the connection in setupRemoteConnection()
 
 	Runroot        string
 	ImageStore     string

test/system/272-system-connection.bats

@@ -230,6 +230,18 @@ $c2[ ]\+tcp://localhost:54321[ ]\+true[ ]\+true" \
     CONTAINER_HOST=foo://124.com _run_podman_remote 125 --remote ps
     assert "$output" =~ "foo" "test env variable CONTAINER_HOST wrt config"
 
+    # There was a bug where this would panic instead of returning a proper error (#22997)
+    CONTAINER_CONNECTION=invalid-env _run_podman_remote 125 --remote ps
+    assert "$output" =~ "read cli flags: connection \"invalid-env\" not found" "connection error from  env"
+
+    # Check again with cli overwrite to ensure correct connection name in error is reported
+    CONTAINER_CONNECTION=invalid-env _run_podman_remote 125 --connection=invalid-cli ps
+    assert "$output" =~ "read cli flags: connection \"invalid-cli\" not found" "connection error from --connection cli"
+
+    # Invalid env is fine if valid connection is given via cli
+    CONTAINER_CONNECTION=invalid-env _run_podman_remote 125 --connection=cli-override ps
+    assert "$output" =~ "/run/user/cli-override/podman/podman.sock" "no CONTAINER_CONNECTION connection error with valid --connection cli"
+
     # Clean up
     run_podman system connection rm defaultconnection
     run_podman system connection rm env-override