AGENT-161: Install-config validations

pkg/asset/agent/installconfig.go

@@ -1,10 +1,28 @@
 package agent
 
 import (
+	"fmt"
+	"os"
+	"strings"
+
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/asset/installconfig"
+	"github.com/openshift/installer/pkg/types"
+	"github.com/openshift/installer/pkg/types/baremetal"
+	"github.com/openshift/installer/pkg/types/none"
+	"github.com/openshift/installer/pkg/types/vsphere"
+	"github.com/pkg/errors"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"sigs.k8s.io/yaml"
+)
+
+const (
+	installConfigFilename = "install-config.yaml"
 )
 
+// supportedPlatforms lists the supported platforms for agent installer
+var supportedPlatforms = []string{baremetal.Name, vsphere.Name, none.Name}
+
 // OptionalInstallConfig is an InstallConfig where the default is empty, rather
 // than generated from running the survey.
 type OptionalInstallConfig struct {
@@ -29,9 +47,129 @@ func (a *OptionalInstallConfig) Generate(parents asset.Parents) error {
 
 // Load returns the installconfig from disk.
 func (a *OptionalInstallConfig) Load(f asset.FileFetcher) (bool, error) {
-	found, err := a.InstallConfig.Load(f)
+
+	var found bool
+
+	// First load the provided install config to early validate
+	// as per agent installer specific requirements
+	// Detailed generic validations of install config are
+	// done by pkg/asset/installconfig/installconfig.go
+	installConfig, err := a.loadEarly(f)
+	if err != nil {
+		return found, err
+	}
+
+	if err := a.validateInstallConfig(installConfig).ToAggregate(); err != nil {
+		return found, errors.Wrapf(err, "invalid install-config configuration")
+	}
+
+	found, err = a.InstallConfig.Load(f)
 	if found && err == nil {
 		a.Supplied = true
 	}
 	return found, err
 }
+
+// loadEarly loads the install config from the disk
+// to be able to validate early for agent installer
+func (a *OptionalInstallConfig) loadEarly(f asset.FileFetcher) (*types.InstallConfig, error) {
+
+	file, err := f.FetchByName(installConfigFilename)
+	config := &types.InstallConfig{}
+	if err != nil {
+		if os.IsNotExist(err) {
+			return config, nil
+		}
+		return config, errors.Wrap(err, asset.InstallConfigError)
+	}
+
+	if err := yaml.UnmarshalStrict(file.Data, config, yaml.DisallowUnknownFields); err != nil {
+		if strings.Contains(err.Error(), "unknown field") {
+			err = errors.Wrapf(err, "failed to parse first occurence of unknown field")
+		}
+		err = errors.Wrapf(err, "failed to unmarshal %s", installConfigFilename)
+		return config, errors.Wrap(err, asset.InstallConfigError)
+	}
+	return config, nil
+}
+
+func (a *OptionalInstallConfig) validateInstallConfig(installConfig *types.InstallConfig) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	if err := a.validateSupportedPlatforms(installConfig); err != nil {
+		allErrs = append(allErrs, err...)
+	}
+
+	if err := a.validateVIPsAreSet(installConfig); err != nil {
+		allErrs = append(allErrs, err...)
+	}
+
+	if err := a.validateSNOConfiguration(installConfig); err != nil {
+		allErrs = append(allErrs, err...)
+	}
+
+	return allErrs
+}
+
+func (a *OptionalInstallConfig) validateSupportedPlatforms(installConfig *types.InstallConfig) field.ErrorList {
+	var allErrs field.ErrorList
+
+	fieldPath := field.NewPath("Platform")
+
+	if installConfig.Platform.Name() != baremetal.Name &&
+		installConfig.Platform.Name() != vsphere.Name &&
+		installConfig.Platform.Name() != none.Name {
+		allErrs = append(allErrs, field.NotSupported(fieldPath, installConfig.Platform.Name(), supportedPlatforms))
+	}
+	return allErrs
+}
+
+func (a *OptionalInstallConfig) validateVIPsAreSet(installConfig *types.InstallConfig) field.ErrorList {
+	var allErrs field.ErrorList
+	var fieldPath *field.Path
+
+	if installConfig.Platform.Name() == baremetal.Name {
+		if installConfig.Platform.BareMetal.APIVIP == "" {
+			fieldPath = field.NewPath("Platform", "Baremetal", "ApiVip")
+			allErrs = append(allErrs, field.Required(fieldPath, fmt.Sprintf("ApiVip must be set for %s platform", baremetal.Name)))
+		}
+		if installConfig.Platform.BareMetal.IngressVIP == "" {
+			fieldPath = field.NewPath("Platform", "Baremetal", "IngressVip")
+			allErrs = append(allErrs, field.Required(fieldPath, fmt.Sprintf("IngressVip must be set for %s platform", baremetal.Name)))
+		}
+	}
+
+	if installConfig.Platform.Name() == vsphere.Name {
+		if installConfig.Platform.VSphere.APIVIP == "" {
+			fieldPath = field.NewPath("Platform", "VSphere", "ApiVip")
+			allErrs = append(allErrs, field.Required(fieldPath, fmt.Sprintf("ApiVip must be set for %s platform", vsphere.Name)))
+		}
+		if installConfig.Platform.VSphere.IngressVIP == "" {
+			fieldPath = field.NewPath("Platform", "VSphere", "IngressVip")
+			allErrs = append(allErrs, field.Required(fieldPath, fmt.Sprintf("IngressVip must be set for %s platform", vsphere.Name)))
+		}
+	}
+	return allErrs
+}
+
+func (a *OptionalInstallConfig) validateSNOConfiguration(installConfig *types.InstallConfig) field.ErrorList {
+	var allErrs field.ErrorList
+	var fieldPath *field.Path
+
+	if installConfig.Platform.Name() == none.Name {
+		if *installConfig.ControlPlane.Replicas != 1 {
+			fieldPath = field.NewPath("ControlPlane", "Replicas")
+			allErrs = append(allErrs, field.Required(fieldPath, fmt.Sprintf("control plane replicas must be 1 for %s platform. Found %v", none.Name, *installConfig.ControlPlane.Replicas)))
+		}
+
+		var workers int
+		for _, worker := range installConfig.Compute {
+			workers = workers + int(*worker.Replicas)
+		}
+		if workers != 0 {
+			fieldPath = field.NewPath("Compute", "Replicas")
+			allErrs = append(allErrs, field.Required(fieldPath, fmt.Sprintf("total number of worker replicas must be 0 for %s platform. Found %v", none.Name, workers)))
+		}
+	}
+	return allErrs
+}

pkg/asset/agent/installconfig_test.go

@@ -0,0 +1,193 @@
+package agent
+
+import (
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
+
+	"github.com/openshift/installer/pkg/asset"
+	"github.com/openshift/installer/pkg/asset/mock"
+	"github.com/openshift/installer/pkg/ipnet"
+	"github.com/openshift/installer/pkg/types"
+	"github.com/openshift/installer/pkg/types/none"
+)
+
+func TestInstallConfigLoad(t *testing.T) {
+	cases := []struct {
+		name           string
+		data           string
+		fetchError     error
+		expectedFound  bool
+		expectedError  string
+		expectedConfig *types.InstallConfig
+	}{
+		{
+			name: "unsupported platform",
+			data: `
+apiVersion: v1
+metadata:
+    name: test-cluster
+baseDomain: test-domain
+platform:
+  aws:
+    region: us-east-1
+pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
+`,
+			expectedFound: false,
+			expectedError: `invalid install-config configuration: Platform: Unsupported value: "aws": supported values: "baremetal", "vsphere", "none"`,
+		},
+		{
+			name: "apiVip not set for baremetal platform",
+			data: `
+apiVersion: v1
+metadata:
+  name: test-cluster
+baseDomain: test-domain
+platform:
+  baremetal:
+    hosts:
+      - name: host1
+        bootMACAddress: 52:54:01:xx:zz:z1
+    ingressVip: 192.168.122.11
+pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
+`,
+			expectedFound: false,
+			expectedError: "invalid install-config configuration: Platform.Baremetal.ApiVip: Required value: ApiVip must be set for baremetal platform",
+		},
+		{
+			name: "ingressVip not set for vsphere platform",
+			data: `
+apiVersion: v1
+metadata:
+  name: test-cluster
+baseDomain: test-domain
+platform:
+  vsphere:
+    apiVip: 192.168.122.10
+pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
+`,
+			expectedFound: false,
+			expectedError: "invalid install-config configuration: Platform.VSphere.IngressVip: Required value: IngressVip must be set for vsphere platform",
+		},
+		{
+			name: "invalid configuration for none platform for sno",
+			data: `
+apiVersion: v1
+metadata:
+  name: test-cluster
+baseDomain: test-domain
+compute:
+  - architecture: amd64
+    hyperthreading: Enabled
+    name: worker
+    platform: {}
+    replicas: 2
+controlPlane:
+  architecture: amd64
+  hyperthreading: Enabled
+  name: master
+  platform: {}
+  replicas: 3
+platform:
+  none : {}
+pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
+`,
+			expectedFound: false,
+			expectedError: "invalid install-config configuration: [ControlPlane.Replicas: Required value: control plane replicas must be 1 for none platform. Found 3, Compute.Replicas: Required value: total number of worker replicas must be 0 for none platform. Found 2]",
+		},
+		{
+			name: "valid configuration for none platform for sno",
+			data: `
+apiVersion: v1
+metadata:
+  name: test-cluster
+baseDomain: test-domain
+compute:
+  - architecture: amd64
+    hyperthreading: Enabled
+    name: worker
+    platform: {}
+    replicas: 0
+controlPlane:
+  architecture: amd64
+  hyperthreading: Enabled
+  name: master
+  platform: {}
+  replicas: 1
+platform:
+  none : {}
+pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
+`,
+			expectedFound: true,
+			expectedConfig: &types.InstallConfig{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: types.InstallConfigVersion,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-cluster",
+				},
+				BaseDomain: "test-domain",
+				Networking: &types.Networking{
+					MachineNetwork: []types.MachineNetworkEntry{
+						{CIDR: *ipnet.MustParseCIDR("10.0.0.0/16")},
+					},
+					NetworkType:    "OpenShiftSDN",
+					ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("172.30.0.0/16")},
+					ClusterNetwork: []types.ClusterNetworkEntry{
+						{
+							CIDR:       *ipnet.MustParseCIDR("10.128.0.0/14"),
+							HostPrefix: 23,
+						},
+					},
+				},
+				ControlPlane: &types.MachinePool{
+					Name:           "master",
+					Replicas:       pointer.Int64Ptr(1),
+					Hyperthreading: types.HyperthreadingEnabled,
+					Architecture:   types.ArchitectureAMD64,
+				},
+				Compute: []types.MachinePool{
+					{
+						Name:           "worker",
+						Replicas:       pointer.Int64Ptr(0),
+						Hyperthreading: types.HyperthreadingEnabled,
+						Architecture:   types.ArchitectureAMD64,
+					},
+				},
+				Platform:   types.Platform{None: &none.Platform{}},
+				PullSecret: `{"auths":{"example.com":{"auth":"authorization value"}}}`,
+				Publish:    types.ExternalPublishingStrategy,
+			},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			mockCtrl := gomock.NewController(t)
+			defer mockCtrl.Finish()
+
+			fileFetcher := mock.NewMockFileFetcher(mockCtrl)
+			fileFetcher.EXPECT().FetchByName("install-config.yaml").
+				Return(
+					&asset.File{
+						Filename: "install-config.yaml",
+						Data:     []byte(tc.data)},
+					tc.fetchError,
+				).MaxTimes(2)
+
+			asset := &OptionalInstallConfig{}
+			found, err := asset.Load(fileFetcher)
+			assert.Equal(t, tc.expectedFound, found, "unexpected found value returned from Load")
+			if tc.expectedError != "" {
+				assert.Equal(t, tc.expectedError, err.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+			if tc.expectedFound {
+				assert.Equal(t, tc.expectedConfig, asset.Config, "unexpected Config in InstallConfig")
+			}
+		})
+	}
+}