Fix farm update to check for connections

Fix farm update to verify a connection exists before
removing or adding it.
Also verify that the farm we want to update exists.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>

cmd/podman/farm/update.go

@@ -8,6 +8,7 @@ import (
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v4/cmd/podman/common"
 	"github.com/containers/podman/v4/cmd/podman/registry"
+	"github.com/containers/podman/v4/pkg/util"
 	"github.com/spf13/cobra"
 )
 
@@ -68,6 +69,10 @@ func farmUpdate(cmd *cobra.Command, args []string) error {
 		return errors.New("no farms are created at this time, there is nothing to update")
 	}
 
+	if _, ok := cfg.Farms.List[farmName]; !ok {
+		return fmt.Errorf("cannot update farm, %q farm doesn't exist", farmName)
+	}
+
 	if defChanged {
 		// Change the default to the given farm if --default=true
 		if updateOpts.Default {
@@ -85,12 +90,21 @@ func farmUpdate(cmd *cobra.Command, args []string) error {
 		}
 
 		for _, cRemove := range updateOpts.Remove {
-			delete(cMap, cRemove)
+			connections := cfg.Farms.List[farmName]
+			if util.StringInSlice(cRemove, connections) {
+				delete(cMap, cRemove)
+			} else {
+				return fmt.Errorf("cannot remove from farm, %q is not a connection in the farm", cRemove)
+			}
 		}
 
 		for _, cAdd := range updateOpts.Add {
-			if _, ok := cMap[cAdd]; !ok {
-				cMap[cAdd] = 0
+			if _, ok := cfg.Engine.ServiceDestinations[cAdd]; ok {
+				if _, ok := cMap[cAdd]; !ok {
+					cMap[cAdd] = 0
+				}
+			} else {
+				return fmt.Errorf("cannot add to farm, %q is not a system connection", cAdd)
 			}
 		}
 

test/e2e/farm_test.go

@@ -156,7 +156,80 @@ var _ = Describe("podman farm", func() {
 			Expect(cfg.Farms.Default).Should(BeEmpty())
 		})
 
-		It("remove existing farms", func() {
+		It("update farm with non-existing connections", func() {
+			// create farm with multiple system connections
+			cmd := []string{"farm", "create", "farm1", "QA", "QB"}
+			session := podmanTest.Podman(cmd)
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(ExitCleanly())
+			Expect(session.Out.Contents()).Should(ContainSubstring("Farm \"farm1\" created"))
+
+			// create farm with only one system connection
+			cmd = []string{"farm", "create", "farm2", "QA"}
+			session = podmanTest.Podman(cmd)
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(ExitCleanly())
+			Expect(session.Out.Contents()).Should(ContainSubstring("Farm \"farm2\" created"))
+
+			cfg, err := config.ReadCustomConfig()
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(cfg.Farms.Default).Should(Equal("farm1"))
+			Expect(cfg.Farms.List).Should(HaveKeyWithValue("farm1", []string{"QA", "QB"}))
+			Expect(cfg.Farms.List).Should(HaveKeyWithValue("farm2", []string{"QA"}))
+
+			// update farm1 to add no-node connection to it
+			cmd = []string{"farm", "update", "--add", "no-node", "farm1"}
+			session = podmanTest.Podman(cmd)
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(ExitWithError())
+
+			// update farm2 to remove node not in farm connections from it
+			cmd = []string{"farm", "update", "--remove", "QB", "farm2"}
+			session = podmanTest.Podman(cmd)
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(ExitWithError())
+
+			// read config again to ensure that nothing has changed
+			cfg, err = config.ReadCustomConfig()
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(cfg.Farms.Default).Should(Equal("farm1"))
+			Expect(cfg.Farms.List).Should(HaveKeyWithValue("farm1", []string{"QA", "QB"}))
+			Expect(cfg.Farms.List).Should(HaveKeyWithValue("farm2", []string{"QA"}))
+		})
+
+		It("update non-existent farm", func() {
+			// create farm with multiple system connections
+			cmd := []string{"farm", "create", "farm1", "QA", "QB"}
+			session := podmanTest.Podman(cmd)
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(ExitCleanly())
+			Expect(session.Out.Contents()).Should(ContainSubstring("Farm \"farm1\" created"))
+
+			cfg, err := config.ReadCustomConfig()
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(cfg.Farms.Default).Should(Equal("farm1"))
+			Expect(cfg.Farms.List).Should(HaveKeyWithValue("farm1", []string{"QA", "QB"}))
+
+			// update non-existent farm to add QA connection to it
+			cmd = []string{"farm", "update", "--add", "no-node", "non-existent"}
+			session = podmanTest.Podman(cmd)
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(ExitWithError())
+
+			// update non-existent farm to default
+			cmd = []string{"farm", "update", "--default", "non-existent"}
+			session = podmanTest.Podman(cmd)
+			session.WaitWithDefaultTimeout()
+			Expect(session).Should(ExitWithError())
+
+			// read config again and ensure nothing has changed
+			cfg, err = config.ReadCustomConfig()
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(cfg.Farms.Default).Should(Equal("farm1"))
+			Expect(cfg.Farms.List).Should(HaveKeyWithValue("farm1", []string{"QA", "QB"}))
+		})
+
+		It("remove farms", func() {
 			// create farm with multiple system connections
 			cmd := []string{"farm", "create", "farm1", "QA", "QB"}
 			session := podmanTest.Podman(cmd)