Merge pull request #23066 from Luap99/connection-setup

remote: fix incorrect CONTAINER_CONNECTION parsing

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,19 +191,22 @@ 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")
 
 	switch {
 	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 {
-			podmanConfig.URI = registry.DefaultAPIAddress()
+			return con.Name
 		}
+		podmanConfig.URI = registry.DefaultAPIAddress()
 	}
-	return nil
+	return ""
 }
 
 func persistentPreRunE(cmd *cobra.Command, args []string) error {
@@ -463,17 +472,16 @@ func stdOutHook() {
 }
 
 func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
-	if err := setupRemoteConnection(podmanConfig); err != nil {
-		return
-	}
+	connectionName := setupRemoteConnection(podmanConfig)
+
 	lFlags := cmd.Flags()
 
 	sshFlagName := "ssh"
 	lFlags.StringVar(&podmanConfig.SSHMode, sshFlagName, string(ssh.GolangMode), "define the ssh mode")
 	_ = 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