Add management config validation

The management configuration does not have central validation, leading
to late validation errors. Additionally, there is an effort to provide
better config validation on a command-by-command basis.

This change adds central validation to the management configuration that
will be used several ways:

  1. For quicker validation today
  2. For a new CLI command that will be introduced in a future change
     that enables imperative modification of the management config
  3. Possibly in future efforts to provide command-by-command config
     validation by calling the exported Validate() function

Change-Id: I19eafddc818e8d478b9afd053d4ab387c7ad38b3
Signed-off-by: Drew Walters <andrew.walters@att.com>

pkg/config/config.go

@@ -915,15 +915,6 @@ func (c *Config) Purge() error {
 	return os.Remove(c.loadedConfigPath)
 }
 
-// Management Configuration functions
-func (m *ManagementConfiguration) String() string {
-	yamlData, err := yaml.Marshal(&m)
-	if err != nil {
-		return ""
-	}
-	return string(yamlData)
-}
-
 // DecodeAuthInfo returns authInfo with credentials decoded
 func DecodeAuthInfo(authinfo *clientcmdapi.AuthInfo) (*clientcmdapi.AuthInfo, error) {
 	password := authinfo.Password

pkg/config/constants.go

@@ -57,7 +57,7 @@ const (
 	// Modules
 	AirshipDefaultBootstrapImage = "quay.io/airshipit/isogen:latest-debian_stable"
 	AirshipDefaultIsoURL         = "http://localhost:8099/debian-custom.iso"
-	AirshipDefaultRemoteType     = redfish.ClientType
+	AirshipDefaultManagementType = redfish.ClientType
 )
 
 // Default values for remote operations

pkg/config/errors.go

@@ -17,6 +17,9 @@ package config
 import (
 	"fmt"
 	"strings"
+
+	"opendev.org/airship/airshipctl/pkg/remote/redfish"
+	redfishdell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell"
 )
 
 // ErrIncompatibleAuthOptions is returned when incompatible
@@ -178,3 +181,14 @@ type ErrDecodingCredentials struct {
 func (e ErrDecodingCredentials) Error() string {
 	return fmt.Sprintf("Error decoding credentials. String '%s' cannot not be decoded", e.Given)
 }
+
+// ErrUnknownManagementType describes a situation in which an unknown management type is listed in the airshipctl
+// config.
+type ErrUnknownManagementType struct {
+	Type string
+}
+
+func (e ErrUnknownManagementType) Error() string {
+	return fmt.Sprintf("Unknown management type '%s'. Known types include '%s' and '%s'.", e.Type,
+		redfish.ClientType, redfishdell.ClientType)
+}

pkg/config/management.go

@@ -0,0 +1,95 @@
+/*
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+package config
+
+import (
+	"sigs.k8s.io/yaml"
+
+	"opendev.org/airship/airshipctl/pkg/remote/redfish"
+	redfishdell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell"
+)
+
+const (
+	insecureDefaultValue = false
+	useProxyDefaultValue = false
+)
+
+// ManagementConfiguration defines configuration data for all remote systems within a context.
+type ManagementConfiguration struct {
+	// Insecure indicates whether the SSL certificate should be checked on remote management requests.
+	Insecure bool `json:"insecure,omitempty"`
+
+	// SystemActionRetries is the number of attempts to poll a host for a status.
+	SystemActionRetries int `json:"systemActionRetries,omitempty"`
+
+	// SystemRebootDelay is the number of seconds to wait between power actions (e.g. shutdown, startup).
+	SystemRebootDelay int `json:"systemRebootDelay,omitempty"`
+
+	// Type the type of out-of-band management that will be used for baremetal orchestration, e.g. redfish.
+	Type string `json:"type"`
+
+	// UseProxy indicates whether airshipctl should transmit remote management requests through a proxy server when
+	// one is configured in an environment.
+	UseProxy bool `json:"useproxy,omitempty"`
+}
+
+// SetType is a helper function that sets and validates the management type.
+func (m *ManagementConfiguration) SetType(managementType string) error {
+	prev := m.Type
+	m.Type = managementType
+
+	if err := m.Validate(); err != nil {
+		m.Type = prev
+		return err
+	}
+
+	return nil
+}
+
+// String converts a management configuration to a human-readable string.
+func (m *ManagementConfiguration) String() string {
+	yamlData, err := yaml.Marshal(&m)
+	if err != nil {
+		return ""
+	}
+
+	return string(yamlData)
+}
+
+// Validate validates that a management configuration is valid. Currently, this only checks the value of the management
+// type as the other fields have appropriate zero values and may be omitted.
+func (m *ManagementConfiguration) Validate() error {
+	switch m.Type {
+	case redfish.ClientType:
+		m.Type = redfish.ClientType
+	case redfishdell.ClientType:
+		m.Type = redfishdell.ClientType
+	default:
+		return ErrUnknownManagementType{Type: m.Type}
+	}
+
+	return nil
+}
+
+// NewManagementConfiguration returns a management configuration with default values.
+func NewManagementConfiguration() *ManagementConfiguration {
+	return &ManagementConfiguration{
+		Insecure:            insecureDefaultValue,
+		SystemActionRetries: DefaultSystemActionRetries,
+		SystemRebootDelay:   DefaultSystemRebootDelay,
+		Type:                AirshipDefaultManagementType,
+		UseProxy:            useProxyDefaultValue,
+	}
+}

pkg/config/management_test.go

@@ -0,0 +1,70 @@
+/*
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+package config_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"opendev.org/airship/airshipctl/pkg/config"
+	redfishdell "opendev.org/airship/airshipctl/pkg/remote/redfish/vendors/dell"
+)
+
+func TestNewManagementConfiguration(t *testing.T) {
+	cfg := config.NewManagementConfiguration()
+	assert.Equal(t, cfg.Type, config.AirshipDefaultManagementType)
+}
+
+func TestSetType(t *testing.T) {
+	cfg := config.NewManagementConfiguration()
+
+	err := cfg.SetType(redfishdell.ClientType)
+	require.NoError(t, err)
+
+	assert.Equal(t, cfg.Type, redfishdell.ClientType)
+}
+
+func TestSetTypeInvalid(t *testing.T) {
+	cfg := config.NewManagementConfiguration()
+
+	err := cfg.SetType("invalid")
+	require.Error(t, err)
+
+	assert.Equal(t, cfg.Type, config.AirshipDefaultManagementType)
+}
+func TestValidateDefault(t *testing.T) {
+	cfg := config.NewManagementConfiguration()
+
+	err := cfg.Validate()
+	assert.NoError(t, err)
+}
+
+func TestValidateRedfishDell(t *testing.T) {
+	cfg := config.NewManagementConfiguration()
+	cfg.Type = redfishdell.ClientType
+
+	err := cfg.Validate()
+	assert.NoError(t, err)
+}
+
+func TestValidateInvalidManagementType(t *testing.T) {
+	cfg := config.NewManagementConfiguration()
+	cfg.Type = "invalid"
+
+	err := cfg.Validate()
+	assert.Error(t, err)
+}

pkg/config/types.go

@@ -68,19 +68,3 @@ type Config struct {
 	// Private instance of Kube Config content as an object
 	kubeConfig *kubeconfig.Config
 }
-
-// ManagementConfiguration defines configuration data for all remote systems within a context.
-type ManagementConfiguration struct {
-	// Insecure indicates whether the SSL certificate should be checked on remote management requests.
-	Insecure bool `json:"insecure,omitempty"`
-	// Type indicates the type of out-of-band management that will be used for baremetal orchestration, e.g.
-	// redfish.
-	Type string `json:"type"`
-	// UseProxy indicates whether airshipctl should transmit remote management requests through a proxy server when
-	// one is configured in an environment.
-	UseProxy bool `json:"useproxy,omitempty"`
-	// Number of attempts to reach host during reboot process and ejecting virtual media
-	SystemActionRetries int `json:"systemActionRetries,omitempty"`
-	// Number of seconds to wait after reboot if host isn't available
-	SystemRebootDelay int `json:"systemRebootDelay,omitempty"`
-}

pkg/config/utils.go

@@ -18,8 +18,6 @@ package config
 
 import (
 	"encoding/base64"
-
-	"opendev.org/airship/airshipctl/pkg/remote/redfish"
 )
 
 const (
@@ -58,13 +56,7 @@ func NewConfig() *Config {
 		},
 		CurrentContext: AirshipDefaultContext,
 		ManagementConfiguration: map[string]*ManagementConfiguration{
-			AirshipDefaultManagementConfiguration: {
+			AirshipDefaultManagementConfiguration: NewManagementConfiguration(),
-				Type:                redfish.ClientType,
-				Insecure:            true,
-				UseProxy:            false,
-				SystemActionRetries: DefaultSystemActionRetries,
-				SystemRebootDelay:   DefaultSystemRebootDelay,
-			},
 		},
 		Manifests: map[string]*Manifest{
 			AirshipDefaultManifest: {

pkg/remote/management.go

@@ -122,6 +122,10 @@ func NewManager(settings *environment.AirshipCTLSettings, phase string, hosts ..
 		return nil, err
 	}
 
+	if err = managementCfg.Validate(); err != nil {
+		return nil, err
+	}
+
 	entrypoint, err := settings.Config.CurrentContextEntryPoint(phase)
 	if err != nil {
 		return nil, err