Skip to content

Commit

Permalink
DAOS-13670 control: Add system name validation on startup (#12509)
Browse files Browse the repository at this point in the history
Explicitly check the configured system name and fail if
it does not meet requirements.

Signed-off-by: Michael MacDonald <mjmac.macdonald@intel.com>
  • Loading branch information
mjmac committed Jun 28, 2023
1 parent 488de2e commit 0b5cdc0
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/control/cmd/daos_agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 5 additions & 0 deletions src/control/lib/control/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
15 changes: 15 additions & 0 deletions src/control/lib/daos/system_prop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 5 additions & 0 deletions src/control/server/config/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
Expand Down
10 changes: 9 additions & 1 deletion src/tests/ftest/server/daos_server_config.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
(C) Copyright 2020-2022 Intel Corporation.
(C) Copyright 2020-2023 Intel Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent
"""
Expand Down Expand Up @@ -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]),
Expand Down

0 comments on commit 0b5cdc0

Please sign in to comment.