From 0b5cdc07a409c525b03dd632626d4fd8753982c0 Mon Sep 17 00:00:00 2001
From: Michael MacDonald <mjmac.macdonald@intel.com>
Date: Wed, 28 Jun 2023 17:22:43 -0400
Subject: [PATCH] DAOS-13670 control: Add system name validation on startup
 (#12509)

Explicitly check the configured system name and fail if
it does not meet requirements.

Signed-off-by: Michael MacDonald <mjmac.macdonald@intel.com>
---
 src/control/cmd/daos_agent/config.go         |  6 ++++++
 src/control/lib/control/config.go            |  5 +++++
 src/control/lib/daos/system_prop.go          | 15 +++++++++++++++
 src/control/server/config/server.go          |  5 +++++
 src/tests/ftest/server/daos_server_config.py | 10 +++++++++-
 5 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/control/cmd/daos_agent/config.go b/src/control/cmd/daos_agent/config.go
index 39bd41989e8..3a6f7a14368 100644
--- a/src/control/cmd/daos_agent/config.go
+++ b/src/control/cmd/daos_agent/config.go
@@ -16,6 +16,7 @@ import (
 
 	"github.com/daos-stack/daos/src/control/build"
 	"github.com/daos-stack/daos/src/control/common"
+	"github.com/daos-stack/daos/src/control/lib/daos"
 	"github.com/daos-stack/daos/src/control/security"
 )
 
@@ -83,6 +84,11 @@ func LoadConfig(cfgPath string) (*Config, error) {
 	if err := yaml.UnmarshalStrict(data, cfg); err != nil {
 		return nil, err
 	}
+
+	if !daos.SystemNameIsValid(cfg.SystemName) {
+		return nil, fmt.Errorf("invalid system name: %q", cfg.SystemName)
+	}
+
 	return cfg, nil
 }
 
diff --git a/src/control/lib/control/config.go b/src/control/lib/control/config.go
index 5798282d711..bb293f05cb6 100644
--- a/src/control/lib/control/config.go
+++ b/src/control/lib/control/config.go
@@ -15,6 +15,7 @@ import (
 	"gopkg.in/yaml.v2"
 
 	"github.com/daos-stack/daos/src/control/build"
+	"github.com/daos-stack/daos/src/control/lib/daos"
 	"github.com/daos-stack/daos/src/control/security"
 )
 
@@ -93,5 +94,9 @@ func LoadConfig(cfgPath string) (*Config, error) {
 	}
 	cfg.Path = cfgPath
 
+	if !daos.SystemNameIsValid(cfg.SystemName) {
+		return nil, fmt.Errorf("invalid system name: %q", cfg.SystemName)
+	}
+
 	return cfg, nil
 }
diff --git a/src/control/lib/daos/system_prop.go b/src/control/lib/daos/system_prop.go
index 09f334d578e..ee0d4a336ff 100644
--- a/src/control/lib/daos/system_prop.go
+++ b/src/control/lib/daos/system_prop.go
@@ -18,6 +18,21 @@ import (
 	"github.com/daos-stack/daos/src/control/common"
 )
 
+/*
+#include <daos_types.h>
+*/
+import "C"
+
+// SystemNameIsValid returns true if the given name is valid for a DAOS system.
+func SystemNameIsValid(name string) bool {
+	// NB: So far, this seems to be the only constraint on system names.
+	if name == "" || len(name) > C.DAOS_SYS_NAME_MAX {
+		return false
+	}
+
+	return true
+}
+
 // BoolPropVal is a boolean property value.
 type BoolPropVal bool
 
diff --git a/src/control/server/config/server.go b/src/control/server/config/server.go
index c7e8c8006fc..0414069fd1a 100644
--- a/src/control/server/config/server.go
+++ b/src/control/server/config/server.go
@@ -22,6 +22,7 @@ import (
 	"github.com/daos-stack/daos/src/control/build"
 	"github.com/daos-stack/daos/src/control/common"
 	"github.com/daos-stack/daos/src/control/fault"
+	"github.com/daos-stack/daos/src/control/lib/daos"
 	"github.com/daos-stack/daos/src/control/logging"
 	"github.com/daos-stack/daos/src/control/security"
 	"github.com/daos-stack/daos/src/control/server/engine"
@@ -354,6 +355,10 @@ func (cfg *Server) Load() error {
 			cfg.Path)
 	}
 
+	if !daos.SystemNameIsValid(cfg.SystemName) {
+		return errors.Errorf("invalid system name: %q", cfg.SystemName)
+	}
+
 	// Update server config based on legacy parameters.
 	if err := updateFromLegacyParams(cfg); err != nil {
 		return errors.Wrap(err, "updating config from legacy parameters")
diff --git a/src/tests/ftest/server/daos_server_config.py b/src/tests/ftest/server/daos_server_config.py
index 63f27895153..b5383793cec 100644
--- a/src/tests/ftest/server/daos_server_config.py
+++ b/src/tests/ftest/server/daos_server_config.py
@@ -1,5 +1,5 @@
 """
-  (C) Copyright 2020-2022 Intel Corporation.
+  (C) Copyright 2020-2023 Intel Corporation.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 """
@@ -46,6 +46,14 @@ def test_daos_server_config_basic(self):
         # Get the input to verify
         c_val = self.params.get("config_val", "/run/server_config_val/*/")
 
+        if c_val[0] == "name":
+            # Set the dmg system name to match the server in order to avoid
+            # mismatch failures that aren't part of this test
+            self.assertTrue(
+                self.server_managers[-1].dmg.set_config_value(c_val[0], c_val[1]),
+                "Error setting the '{}' config file parameter to '{}'".format(
+                    c_val[0], c_val[1]))
+
         # Identify the attribute and modify its value to test value
         self.assertTrue(
             self.server_managers[0].set_config_value(c_val[0], c_val[1]),