From 30a18fc02d157b205891c08aed3a3f251c5f33cc Mon Sep 17 00:00:00 2001
From: Paul Holzinger <pholzing@redhat.com>
Date: Fri, 16 Feb 2024 14:58:17 +0100
Subject: [PATCH] pkg/machine: make only one AddConnection() call

This function has to read/write the connections file as such it should
only ever be called once otherwise we read/write the same file twice
which makes no sense. Also cleanup the fucntion a bit and make it
private as there are no external callers.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 pkg/machine/connection/add.go        | 22 +++++++-------
 pkg/machine/connection/connection.go | 43 ++++++++++++++++------------
 2 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/pkg/machine/connection/add.go b/pkg/machine/connection/add.go
index fe0ed7f3fa..c87550065f 100644
--- a/pkg/machine/connection/add.go
+++ b/pkg/machine/connection/add.go
@@ -2,7 +2,6 @@ package connection
 
 import (
 	"fmt"
-	"net/url"
 	"strconv"
 
 	"github.com/containers/podman/v5/pkg/machine/define"
@@ -18,19 +17,22 @@ func AddSSHConnectionsToPodmanSocket(uid, port int, identityPath, name, remoteUs
 	uri := makeSSHURL(LocalhostIP, fmt.Sprintf("/run/user/%d/podman/podman.sock", uid), strconv.Itoa(port), remoteUsername)
 	uriRoot := makeSSHURL(LocalhostIP, "/run/podman/podman.sock", strconv.Itoa(port), "root")
 
-	uris := []*url.URL{uri, uriRoot}
-	names := []string{name, name + "-root"}
+	cons := []connection{
+		{
+			name: name,
+			uri:  uri,
+		},
+		{
+			name: name + "-root",
+			uri:  uriRoot,
+		},
+	}
 
 	// The first connection defined when connections is empty will become the default
 	// regardless of IsDefault, so order according to rootful
 	if opts.Rootful {
-		uris[0], names[0], uris[1], names[1] = uris[1], names[1], uris[0], names[0]
+		cons[0], cons[1] = cons[1], cons[0]
 	}
 
-	for i := 0; i < 2; i++ {
-		if err := AddConnection(uris[i], names[i], identityPath, opts.IsDefault && i == 0); err != nil {
-			return err
-		}
-	}
-	return nil
+	return addConnection(cons, identityPath, opts.IsDefault)
 }
diff --git a/pkg/machine/connection/connection.go b/pkg/machine/connection/connection.go
index 94dc26569c..4c3394a8d2 100644
--- a/pkg/machine/connection/connection.go
+++ b/pkg/machine/connection/connection.go
@@ -15,33 +15,40 @@ import (
 
 const LocalhostIP = "127.0.0.1"
 
-func AddConnection(uri *url.URL, name, identity string, isDefault bool) error {
+type connection struct {
+	name string
+	uri  *url.URL
+}
+
+func addConnection(cons []connection, identity string, isDefault bool) error {
 	if len(identity) < 1 {
 		return errors.New("identity must be defined")
 	}
 
 	return config.EditConnectionConfig(func(cfg *config.ConnectionsFile) error {
-		if _, ok := cfg.Connection.Connections[name]; ok {
-			return errors.New("cannot overwrite connection")
-		}
+		for i, con := range cons {
+			if _, ok := cfg.Connection.Connections[con.name]; ok {
+				return fmt.Errorf("cannot overwrite connection %q", con.name)
+			}
 
-		dst := config.Destination{
-			URI:       uri.String(),
-			IsMachine: true,
-			Identity:  identity,
-		}
+			dst := config.Destination{
+				URI:       con.uri.String(),
+				IsMachine: true,
+				Identity:  identity,
+			}
 
-		if isDefault {
-			cfg.Connection.Default = name
-		}
+			if isDefault && i == 0 {
+				cfg.Connection.Default = con.name
+			}
 
-		if cfg.Connection.Connections == nil {
-			cfg.Connection.Connections = map[string]config.Destination{
-				name: dst,
+			if cfg.Connection.Connections == nil {
+				cfg.Connection.Connections = map[string]config.Destination{
+					con.name: dst,
+				}
+				cfg.Connection.Default = con.name
+			} else {
+				cfg.Connection.Connections[con.name] = dst
 			}
-			cfg.Connection.Default = name
-		} else {
-			cfg.Connection.Connections[name] = dst
 		}
 
 		return nil