From c63c31796fb462b65328bf21d2ac2674fba1b754 Mon Sep 17 00:00:00 2001
From: Nicolas Vinuesa <vinuesa.nico@gmail.com>
Date: Mon, 26 Aug 2024 17:32:40 +0200
Subject: [PATCH] fix(config): allow un-setting ha-space controller config

Before this patch it was impossible to un-set the juju-ha-space
controller config value because:
- first, the empty string was rejected because it does not match the
  regexp on the names package for space names.
- second, because on the state layer we check that the provided space
  exists, and an empty string space never does.

The first reason was fixed on `controller/config.go` by simply adding
the "not empty string" check before validating the space name.

For the second reason, a workaround had to be added: if the passed space
name is the empty string, then we don't check for its existence. This is
not good because in the `UpdateControllerConfig()` method we already
have the `removeAttrs` argument but on the only usage of this method (on
the client facades) we pass a nil value as `removeAttrs`.
---
 controller/config.go      |  6 +++++-
 controller/config_test.go |  5 +++++
 state/controller.go       |  9 +++++++++
 state/controller_test.go  | 23 +++++++++++++++++++++++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/controller/config.go b/controller/config.go
index 79b013efbfd..d695f95462d 100755
--- a/controller/config.go
+++ b/controller/config.go
@@ -1250,7 +1250,11 @@ func (c Config) validateSpaceConfig(key, topic string) error {
 		return nil
 	}
 	if v, ok := val.(string); ok {
-		if !names.IsValidSpace(v) {
+		// NOTE(nvinuesa): We also check for the case where the passed
+		// value is the empty string, this is needed to un-set the
+		// controller config value and not added in the regexp to
+		// validate the space.
+		if !names.IsValidSpace(v) && v != "" {
 			return errors.NotValidf("%s space name %q", topic, val)
 		}
 	} else {
diff --git a/controller/config_test.go b/controller/config_test.go
index 2654f1c8068..cdb06cfbda3 100755
--- a/controller/config_test.go
+++ b/controller/config_test.go
@@ -129,6 +129,11 @@ var newConfigTests = []struct {
 		controller.JujuManagementSpace: "\n",
 	},
 	expectError: `juju mgmt space name "\\n" not valid`,
+}, {
+	about: "HA space name - empty string OK",
+	config: controller.Config{
+		controller.JujuHASpace: "",
+	},
 }, {
 	about: "invalid HA space name - number",
 	config: controller.Config{
diff --git a/state/controller.go b/state/controller.go
index a5a4465f1c4..242fd384b70 100644
--- a/state/controller.go
+++ b/state/controller.go
@@ -172,6 +172,15 @@ func checkUpdateControllerConfig(name string) error {
 // checkSpaceIsAvailableToAllControllers checks if each controller machine has
 // at least one address in the input space. If not, an error is returned.
 func (st *State) checkSpaceIsAvailableToAllControllers(spaceName string) error {
+	// TODO(nvinuesa): We should not be checking if the space is empty
+	// at this point, instead we should be using the `removeAttrs`
+	// attribute on `UpdateControllerConfig()` to un-set the spaces.
+	// Unfortunately this is not the case and therefore we must check for
+	// the empty string (un-setting the space value) as a workaround.
+	if spaceName == "" {
+		return nil
+	}
+
 	controllerIds, err := st.ControllerIds()
 	if err != nil {
 		return errors.Annotate(err, "cannot get controller info")
diff --git a/state/controller_test.go b/state/controller_test.go
index 6cfb09ee500..9ef664378db 100644
--- a/state/controller_test.go
+++ b/state/controller_test.go
@@ -185,6 +185,29 @@ func (s *ControllerSuite) TestRemovingUnknownName(c *gc.C) {
 	c.Assert(err, gc.ErrorMatches, `unknown controller config setting "dr-worm"`)
 }
 
+func (s *ControllerSuite) TestUpdateControllerConfigAcceptEmptyStringSpace(c *gc.C) {
+	sp, err := s.State.AddSpace("ha-space", "", nil, false)
+	c.Assert(err, jc.ErrorIsNil)
+
+	m, err := s.State.AddMachine(state.UbuntuBase("12.10"), state.JobManageModel, state.JobHostUnits)
+	c.Assert(err, jc.ErrorIsNil)
+
+	addr := network.NewSpaceAddress("192.168.9.9")
+	addr.SpaceID = sp.Id()
+
+	c.Assert(m.SetProviderAddresses(addr), jc.ErrorIsNil)
+
+	err = s.State.UpdateControllerConfig(map[string]interface{}{
+		controller.JujuHASpace: "ha-space",
+	}, nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	err = s.State.UpdateControllerConfig(map[string]interface{}{
+		controller.JujuHASpace: "",
+	}, nil)
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 func (s *ControllerSuite) TestUpdateControllerConfigRejectsSpaceWithoutAddresses(c *gc.C) {
 	_, err := s.State.AddSpace("mgmt-space", "", nil, false)
 	c.Assert(err, jc.ErrorIsNil)