diff --git a/pkg/asset/agent/installconfig.go b/pkg/asset/agent/installconfig.go index 3bccd4d3f3b..3b52267dbd6 100644 --- a/pkg/asset/agent/installconfig.go +++ b/pkg/asset/agent/installconfig.go @@ -14,6 +14,7 @@ import ( "github.com/openshift/installer/pkg/types/baremetal" baremetaldefaults "github.com/openshift/installer/pkg/types/baremetal/defaults" "github.com/openshift/installer/pkg/types/none" + "github.com/openshift/installer/pkg/types/validation" "github.com/openshift/installer/pkg/types/vsphere" ) @@ -24,10 +25,12 @@ const ( // OptionalInstallConfig is an InstallConfig where the default is empty, rather // than generated from running the survey. type OptionalInstallConfig struct { - installconfig.InstallConfig + installconfig.AssetBase Supplied bool } +var _ asset.WritableAsset = (*OptionalInstallConfig)(nil) + // Dependencies returns all of the dependencies directly needed by an // InstallConfig asset. func (a *OptionalInstallConfig) Dependencies() []asset.Asset { @@ -45,21 +48,24 @@ func (a *OptionalInstallConfig) Generate(parents asset.Parents) error { // Load returns the installconfig from disk. func (a *OptionalInstallConfig) Load(f asset.FileFetcher) (bool, error) { - - var foundValidatedDefaultInstallConfig bool - - foundValidatedDefaultInstallConfig, err := a.InstallConfig.Load(f) - if foundValidatedDefaultInstallConfig && err == nil { + found, err := a.LoadFromFile(f) + if found && err == nil { a.Supplied = true if err := a.validateInstallConfig(a.Config).ToAggregate(); err != nil { return false, errors.Wrapf(err, "invalid install-config configuration") } + if err := a.RecordFile(); err != nil { + return false, err + } } - return foundValidatedDefaultInstallConfig, err + return found, err } func (a *OptionalInstallConfig) validateInstallConfig(installConfig *types.InstallConfig) field.ErrorList { var allErrs field.ErrorList + if err := validation.ValidateInstallConfig(a.Config, true); err != nil { + allErrs = append(allErrs, err...) + } if err := a.validateSupportedPlatforms(installConfig); err != nil { allErrs = append(allErrs, err...) diff --git a/pkg/asset/agent/installconfig_test.go b/pkg/asset/agent/installconfig_test.go index 591b1071839..b65f2eae354 100644 --- a/pkg/asset/agent/installconfig_test.go +++ b/pkg/asset/agent/installconfig_test.go @@ -86,7 +86,7 @@ platform: pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}" `, expectedFound: false, - expectedError: "failed to create install config: invalid \"install-config.yaml\" file: [platform.baremetal.apiVIPs: Required value: must specify at least one VIP for the API, platform.baremetal.apiVIPs: Required value: must specify VIP for API, when VIP for ingress is set]", + expectedError: "invalid install-config configuration: [platform.baremetal.apiVIPs: Required value: must specify at least one VIP for the API, platform.baremetal.apiVIPs: Required value: must specify VIP for API, when VIP for ingress is set]", }, { name: "Required values not set for vsphere platform", @@ -102,7 +102,7 @@ platform: pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}" `, expectedFound: false, - expectedError: `failed to create install config: invalid "install-config.yaml" file: [platform.vsphere.apiVIPs: Invalid value: "192.168.122.10": IP expected to be in one of the machine networks: 10.0.0.0/16, platform.vsphere.ingressVIPs: Required value: must specify VIP for ingress, when VIP for API is set, platform.vsphere.vcenters.server: Required value: must be the domain name or IP address of the vCenter, platform.vsphere.vcenters.username: Required value: must specify the username, platform.vsphere.vcenters.password: Required value: must specify the password, platform.vsphere.vcenters.datacenters: Required value: must specify at least one datacenter, platform.vsphere.failureDomains.server: Required value: must specify a vCenter server, platform.vsphere.failureDomains.topology.datacenter: Required value: must specify a datacenter, platform.vsphere.failureDomains.topology.datastore: Required value: must specify a datastore, platform.vsphere.failureDomains.topology.computeCluster: Required value: must specify a computeCluster, platform.vsphere.failureDomains.topology.resourcePool: Invalid value: "//Resources": full path of resource pool must be provided in format //host//...]`, + expectedError: `invalid install-config configuration: [platform.vsphere.apiVIPs: Invalid value: "192.168.122.10": IP expected to be in one of the machine networks: 10.0.0.0/16, platform.vsphere.ingressVIPs: Required value: must specify VIP for ingress, when VIP for API is set]`, }, { name: "invalid configuration for none platform for sno", diff --git a/pkg/asset/agent/manifests/util_test.go b/pkg/asset/agent/manifests/util_test.go index bed24e708a4..bd65cbfe9f9 100644 --- a/pkg/asset/agent/manifests/util_test.go +++ b/pkg/asset/agent/manifests/util_test.go @@ -72,7 +72,7 @@ func getValidOptionalInstallConfig() *agent.OptionalInstallConfig { _, machineNetCidr, _ := net.ParseCIDR("10.10.11.0/24") return &agent.OptionalInstallConfig{ - InstallConfig: installconfig.InstallConfig{ + AssetBase: installconfig.AssetBase{ Config: &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "ocp-edge-cluster-0", @@ -133,7 +133,7 @@ func getValidOptionalInstallConfigDualStack() *agent.OptionalInstallConfig { _, machineNetCidrIPv6, _ := net.ParseCIDR("2001:db8:5dd8:c956::/64") return &agent.OptionalInstallConfig{ - InstallConfig: installconfig.InstallConfig{ + AssetBase: installconfig.AssetBase{ Config: &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "ocp-edge-cluster-0", @@ -202,7 +202,7 @@ func getValidOptionalInstallConfigDualStackDualVIPs() *agent.OptionalInstallConf // getProxyValidOptionalInstallConfig returns a valid optional install config for proxied installation func getProxyValidOptionalInstallConfig() *agent.OptionalInstallConfig { validIC := getValidOptionalInstallConfig() - validIC.InstallConfig.Config.Proxy = &types.Proxy{ + validIC.Config.Proxy = &types.Proxy{ HTTPProxy: "http://10.10.10.11:80", HTTPSProxy: "http://my-lab-proxy.org:443", NoProxy: "internal.com", diff --git a/pkg/asset/agent/mirror/cabundle_test.go b/pkg/asset/agent/mirror/cabundle_test.go index 61b9d1ec0b7..8e0e8b02789 100644 --- a/pkg/asset/agent/mirror/cabundle_test.go +++ b/pkg/asset/agent/mirror/cabundle_test.go @@ -35,7 +35,7 @@ func TestCaBundle_Generate(t *testing.T) { dependencies: []asset.Asset{ &agent.OptionalInstallConfig{ Supplied: true, - InstallConfig: installconfig.InstallConfig{ + AssetBase: installconfig.AssetBase{ Config: &types.InstallConfig{ ObjectMeta: v1.ObjectMeta{ Namespace: "cluster-0", @@ -50,7 +50,7 @@ func TestCaBundle_Generate(t *testing.T) { dependencies: []asset.Asset{ &agent.OptionalInstallConfig{ Supplied: true, - InstallConfig: installconfig.InstallConfig{ + AssetBase: installconfig.AssetBase{ Config: &types.InstallConfig{ ObjectMeta: v1.ObjectMeta{ Namespace: "cluster-0", diff --git a/pkg/asset/agent/mirror/registriesconf_test.go b/pkg/asset/agent/mirror/registriesconf_test.go index 92240662fad..0c289d15018 100644 --- a/pkg/asset/agent/mirror/registriesconf_test.go +++ b/pkg/asset/agent/mirror/registriesconf_test.go @@ -37,7 +37,7 @@ func TestRegistriesConf_Generate(t *testing.T) { dependencies: []asset.Asset{ &agent.OptionalInstallConfig{ Supplied: true, - InstallConfig: installconfig.InstallConfig{ + AssetBase: installconfig.AssetBase{ Config: &types.InstallConfig{ ObjectMeta: v1.ObjectMeta{ Namespace: "cluster-0", @@ -56,7 +56,7 @@ func TestRegistriesConf_Generate(t *testing.T) { dependencies: []asset.Asset{ &agent.OptionalInstallConfig{ Supplied: true, - InstallConfig: installconfig.InstallConfig{ + AssetBase: installconfig.AssetBase{ Config: &types.InstallConfig{ ObjectMeta: v1.ObjectMeta{ Namespace: "cluster-0", @@ -89,7 +89,7 @@ func TestRegistriesConf_Generate(t *testing.T) { dependencies: []asset.Asset{ &agent.OptionalInstallConfig{ Supplied: true, - InstallConfig: installconfig.InstallConfig{ + AssetBase: installconfig.AssetBase{ Config: &types.InstallConfig{ ObjectMeta: v1.ObjectMeta{ Namespace: "cluster-0", diff --git a/pkg/asset/ignition/machine/master_ignition_customizations_test.go b/pkg/asset/ignition/machine/master_ignition_customizations_test.go index 69dd6983167..b8f8377a0fe 100644 --- a/pkg/asset/ignition/machine/master_ignition_customizations_test.go +++ b/pkg/asset/ignition/machine/master_ignition_customizations_test.go @@ -35,8 +35,8 @@ func TestMasterIgnitionCustomizationsGenerate(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - installConfig := &installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installConfig := installconfig.MakeAsset( + &types.InstallConfig{ Networking: &types.Networking{ ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("10.0.1.0/24")}, }, @@ -45,8 +45,7 @@ func TestMasterIgnitionCustomizationsGenerate(t *testing.T) { Region: "us-east", }, }, - }, - } + }) rootCA := &tls.RootCA{} err := rootCA.Generate(nil) diff --git a/pkg/asset/ignition/machine/master_test.go b/pkg/asset/ignition/machine/master_test.go index d547f42ec2d..ac6b84de624 100644 --- a/pkg/asset/ignition/machine/master_test.go +++ b/pkg/asset/ignition/machine/master_test.go @@ -17,8 +17,8 @@ import ( // TestMasterGenerate tests generating the master asset. func TestMasterGenerate(t *testing.T) { - installConfig := &installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installConfig := installconfig.MakeAsset( + &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", }, @@ -35,8 +35,7 @@ func TestMasterGenerate(t *testing.T) { Name: "master", Replicas: pointer.Int64Ptr(3), }, - }, - } + }) rootCA := &tls.RootCA{} err := rootCA.Generate(nil) diff --git a/pkg/asset/ignition/machine/worker_ignition_customizations_test.go b/pkg/asset/ignition/machine/worker_ignition_customizations_test.go index 595faff8be6..599567b0682 100644 --- a/pkg/asset/ignition/machine/worker_ignition_customizations_test.go +++ b/pkg/asset/ignition/machine/worker_ignition_customizations_test.go @@ -35,8 +35,8 @@ func TestWorkerIgnitionCustomizationsGenerate(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - installConfig := &installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installConfig := installconfig.MakeAsset( + &types.InstallConfig{ Networking: &types.Networking{ ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("10.0.1.0/24")}, }, @@ -45,8 +45,7 @@ func TestWorkerIgnitionCustomizationsGenerate(t *testing.T) { Region: "us-east", }, }, - }, - } + }) rootCA := &tls.RootCA{} err := rootCA.Generate(nil) diff --git a/pkg/asset/ignition/machine/worker_test.go b/pkg/asset/ignition/machine/worker_test.go index 91e0833511f..a623fb8b7b1 100644 --- a/pkg/asset/ignition/machine/worker_test.go +++ b/pkg/asset/ignition/machine/worker_test.go @@ -15,8 +15,8 @@ import ( // TestWorkerGenerate tests generating the worker asset. func TestWorkerGenerate(t *testing.T) { - installConfig := &installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installConfig := installconfig.MakeAsset( + &types.InstallConfig{ Networking: &types.Networking{ ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("10.0.1.0/24")}, }, @@ -25,8 +25,7 @@ func TestWorkerGenerate(t *testing.T) { Region: "us-east", }, }, - }, - } + }) rootCA := &tls.RootCA{} err := rootCA.Generate(nil) diff --git a/pkg/asset/installconfig/installconfig.go b/pkg/asset/installconfig/installconfig.go index b92b27e0f4e..9049b1cc3cf 100644 --- a/pkg/asset/installconfig/installconfig.go +++ b/pkg/asset/installconfig/installconfig.go @@ -2,12 +2,8 @@ package installconfig import ( "context" - "os" - "strings" - "github.com/ghodss/yaml" "github.com/pkg/errors" - "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" @@ -23,7 +19,6 @@ import ( icpowervs "github.com/openshift/installer/pkg/asset/installconfig/powervs" icvsphere "github.com/openshift/installer/pkg/asset/installconfig/vsphere" "github.com/openshift/installer/pkg/types" - "github.com/openshift/installer/pkg/types/conversion" "github.com/openshift/installer/pkg/types/defaults" "github.com/openshift/installer/pkg/types/validation" ) @@ -34,8 +29,7 @@ const ( // InstallConfig generates the install-config.yaml file. type InstallConfig struct { - Config *types.InstallConfig `json:"config"` - File *asset.File `json:"file"` + AssetBase AWS *aws.Metadata `json:"aws,omitempty"` Azure *icazure.Metadata `json:"azure,omitempty"` IBMCloud *icibmcloud.Metadata `json:"ibmcloud,omitempty"` @@ -45,6 +39,15 @@ type InstallConfig struct { var _ asset.WritableAsset = (*InstallConfig)(nil) +// MakeAsset returns an InstallConfig asset containing a given InstallConfig CR. +func MakeAsset(config *types.InstallConfig) *InstallConfig { + return &InstallConfig{ + AssetBase: AssetBase{ + Config: config, + }, + } +} + // Dependencies returns all of the dependencies directly needed by an // InstallConfig asset. func (a *InstallConfig) Dependencies() []asset.Asset { @@ -104,63 +107,24 @@ func (a *InstallConfig) Generate(parents asset.Parents) error { a.Config.PowerVS = platform.PowerVS a.Config.Nutanix = platform.Nutanix - return a.finish("") -} - -// Name returns the human-friendly name of the asset. -func (a *InstallConfig) Name() string { - return "Install Config" -} + defaults.SetInstallConfigDefaults(a.Config) -// Files returns the files generated by the asset. -func (a *InstallConfig) Files() []*asset.File { - if a.File != nil { - return []*asset.File{a.File} - } - return []*asset.File{} + return a.finish("") } // Load returns the installconfig from disk. func (a *InstallConfig) Load(f asset.FileFetcher) (found bool, err error) { - file, err := f.FetchByName(installConfigFilename) - if err != nil { - if os.IsNotExist(err) { - return false, nil - } - return false, errors.Wrap(err, asset.InstallConfigError) - } - - config := &types.InstallConfig{} - if err := yaml.UnmarshalStrict(file.Data, config, yaml.DisallowUnknownFields); err != nil { - err = errors.Wrapf(err, "failed to unmarshal %s", installConfigFilename) - if !strings.Contains(err.Error(), "unknown field") { - return false, errors.Wrap(err, asset.InstallConfigError) - } - err = errors.Wrapf(err, "failed to parse first occurence of unknown field") - logrus.Warnf(err.Error()) - logrus.Info("Attempting to unmarshal while ignoring unknown keys because strict unmarshaling failed") - if err = yaml.UnmarshalStrict(file.Data, config); err != nil { - err = errors.Wrapf(err, "failed to unmarshal %s", installConfigFilename) + found, err = a.LoadFromFile(f) + if found && err == nil { + if err := a.finish(installConfigFilename); err != nil { return false, errors.Wrap(err, asset.InstallConfigError) } } - a.Config = config - - // Upconvert any deprecated fields - if err := conversion.ConvertInstallConfig(a.Config); err != nil { - return false, errors.Wrap(errors.Wrap(err, "failed to upconvert install config"), asset.InstallConfigError) - } - err = a.finish(installConfigFilename) - if err != nil { - return false, errors.Wrap(err, asset.InstallConfigError) - } - return true, nil + return found, err } func (a *InstallConfig) finish(filename string) error { - defaults.SetInstallConfigDefaults(a.Config) - if a.Config.AWS != nil { a.AWS = aws.NewMetadata(a.Config.Platform.AWS.Region, a.Config.Platform.AWS.Subnets, a.Config.AWS.ServiceEndpoints) } @@ -177,7 +141,7 @@ func (a *InstallConfig) finish(filename string) error { a.PowerVS = icpowervs.NewMetadata(a.Config.BaseDomain) } - if err := validation.ValidateInstallConfig(a.Config).ToAggregate(); err != nil { + if err := validation.ValidateInstallConfig(a.Config, false).ToAggregate(); err != nil { if filename == "" { return errors.Wrap(err, "invalid install config") } @@ -188,17 +152,12 @@ func (a *InstallConfig) finish(filename string) error { return err } - data, err := yaml.Marshal(a.Config) - if err != nil { - return errors.Wrap(err, "failed to Marshal InstallConfig") - } - a.File = &asset.File{ - Filename: installConfigFilename, - Data: data, - } - return nil + return a.RecordFile() } +// platformValidation runs validations that require connecting to the +// underlying platform. In some cases, platforms also duplicate validations +// that have already been checked by validation.ValidateInstallConfig(). func (a *InstallConfig) platformValidation() error { if a.Config.Platform.AlibabaCloud != nil { client, err := a.AlibabaCloud.Client() diff --git a/pkg/asset/installconfig/installconfigbase.go b/pkg/asset/installconfig/installconfigbase.go new file mode 100644 index 00000000000..1eeab893daf --- /dev/null +++ b/pkg/asset/installconfig/installconfigbase.go @@ -0,0 +1,84 @@ +package installconfig + +import ( + "os" + "strings" + + "github.com/ghodss/yaml" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/conversion" + "github.com/openshift/installer/pkg/types/defaults" +) + +// AssetBase is the base structure for the separate InstallConfig assets used +// in the agent-based and IPI/UPI installation methods. +type AssetBase struct { + Config *types.InstallConfig `json:"config"` + File *asset.File `json:"file"` +} + +// Files returns the files generated by the asset. +func (a *AssetBase) Files() []*asset.File { + if a.File != nil { + return []*asset.File{a.File} + } + return []*asset.File{} +} + +// Name returns the human-friendly name of the asset. +func (a *AssetBase) Name() string { + return "Install Config" +} + +// LoadFromFile returns the installconfig from disk. +func (a *AssetBase) LoadFromFile(f asset.FileFetcher) (found bool, err error) { + file, err := f.FetchByName(installConfigFilename) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, errors.Wrap(err, asset.InstallConfigError) + } + + config := &types.InstallConfig{} + if err := yaml.UnmarshalStrict(file.Data, config, yaml.DisallowUnknownFields); err != nil { + err = errors.Wrapf(err, "failed to unmarshal %s", installConfigFilename) + if !strings.Contains(err.Error(), "unknown field") { + return false, errors.Wrap(err, asset.InstallConfigError) + } + err = errors.Wrapf(err, "failed to parse first occurrence of unknown field") + logrus.Warnf(err.Error()) + logrus.Info("Attempting to unmarshal while ignoring unknown keys because strict unmarshaling failed") + if err = yaml.UnmarshalStrict(file.Data, config); err != nil { + err = errors.Wrapf(err, "failed to unmarshal %s", installConfigFilename) + return false, errors.Wrap(err, asset.InstallConfigError) + } + } + a.Config = config + + // Upconvert any deprecated fields + if err := conversion.ConvertInstallConfig(a.Config); err != nil { + return false, errors.Wrap(errors.Wrap(err, "failed to upconvert install config"), asset.InstallConfigError) + } + + defaults.SetInstallConfigDefaults(a.Config) + + return true, nil +} + +// RecordFile generates the asset manifest file from the config CR. +func (a *AssetBase) RecordFile() error { + data, err := yaml.Marshal(a.Config) + if err != nil { + return errors.Wrap(err, "failed to Marshal InstallConfig") + } + a.File = &asset.File{ + Filename: installConfigFilename, + Data: data, + } + return nil +} diff --git a/pkg/asset/installconfig/vsphere/validation.go b/pkg/asset/installconfig/vsphere/validation.go index 2292facd377..06c90574c07 100644 --- a/pkg/asset/installconfig/vsphere/validation.go +++ b/pkg/asset/installconfig/vsphere/validation.go @@ -59,7 +59,7 @@ func Validate(ic *types.InstallConfig) error { if ic.Platform.VSphere == nil { return errors.New(field.Required(field.NewPath("platform", "vsphere"), "vSphere validation requires a vSphere platform configuration").Error()) } - return validation.ValidatePlatform(ic.Platform.VSphere, field.NewPath("platform").Child("vsphere"), ic).ToAggregate() + return validation.ValidatePlatform(ic.Platform.VSphere, false, field.NewPath("platform").Child("vsphere"), ic).ToAggregate() } func getVCenterClient(failureDomain vsphere.FailureDomain, ic *types.InstallConfig) (*validationContext, ClientLogout, error) { diff --git a/pkg/asset/logging/filelogging_test.go b/pkg/asset/logging/filelogging_test.go index ad0e76905ea..7436c31995e 100644 --- a/pkg/asset/logging/filelogging_test.go +++ b/pkg/asset/logging/filelogging_test.go @@ -35,8 +35,10 @@ func TestLogFilesChanged(t *testing.T) { name: "test asset with one file", assets: []asset.WritableAsset{ &installconfig.InstallConfig{ - File: &asset.File{ - Filename: "a.yaml", + AssetBase: installconfig.AssetBase{ + File: &asset.File{ + Filename: "a.yaml", + }, }, }, }, diff --git a/pkg/asset/machines/master_test.go b/pkg/asset/machines/master_test.go index 85e147ca4ba..299b36f2a6a 100644 --- a/pkg/asset/machines/master_test.go +++ b/pkg/asset/machines/master_test.go @@ -134,8 +134,8 @@ spec: UUID: "test-uuid", InfraID: "test-infra-id", }, - &installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installconfig.MakeAsset( + &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", }, @@ -156,8 +156,7 @@ spec: }, }, }, - }, - }, + }), (*rhcos.Image)(pointer.StringPtr("test-image")), (*rhcos.Release)(pointer.StringPtr("412.86.202208101040-0")), &machine.Master{ @@ -185,8 +184,8 @@ spec: func TestControlPlaneIsNotModified(t *testing.T) { parents := asset.Parents{} - installConfig := installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installConfig := installconfig.MakeAsset( + &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", }, @@ -209,15 +208,14 @@ func TestControlPlaneIsNotModified(t *testing.T) { }, }, }, - }, - } + }) parents.Add( &installconfig.ClusterID{ UUID: "test-uuid", InfraID: "test-infra-id", }, - &installConfig, + installConfig, (*rhcos.Image)(pointer.StringPtr("test-image")), (*rhcos.Release)(pointer.StringPtr("412.86.202208101040-0")), &machine.Master{ @@ -239,8 +237,8 @@ func TestControlPlaneIsNotModified(t *testing.T) { func TestBaremetalGeneratedAssetFiles(t *testing.T) { parents := asset.Parents{} - installConfig := installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installConfig := installconfig.MakeAsset( + &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", }, @@ -279,15 +277,14 @@ func TestBaremetalGeneratedAssetFiles(t *testing.T) { Platform: types.MachinePoolPlatform{}, }, }, - }, - } + }) parents.Add( &installconfig.ClusterID{ UUID: "test-uuid", InfraID: "test-infra-id", }, - &installConfig, + installConfig, (*rhcos.Image)(pointer.StringPtr("test-image")), (*rhcos.Release)(pointer.StringPtr("412.86.202208101040-0")), &machine.Master{ diff --git a/pkg/asset/machines/worker_test.go b/pkg/asset/machines/worker_test.go index 35654bbdafb..4abb12eec7b 100644 --- a/pkg/asset/machines/worker_test.go +++ b/pkg/asset/machines/worker_test.go @@ -128,8 +128,8 @@ spec: UUID: "test-uuid", InfraID: "test-infra-id", }, - &installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installconfig.MakeAsset( + &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", }, @@ -152,8 +152,7 @@ spec: }, }, }, - }, - }, + }), (*rhcos.Image)(pointer.StringPtr("test-image")), (*rhcos.Release)(pointer.StringPtr("412.86.202208101040-0")), &machine.Worker{ @@ -181,8 +180,8 @@ spec: func TestComputeIsNotModified(t *testing.T) { parents := asset.Parents{} - installConfig := installconfig.InstallConfig{ - Config: &types.InstallConfig{ + installConfig := installconfig.MakeAsset( + &types.InstallConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "test-cluster", }, @@ -208,15 +207,14 @@ func TestComputeIsNotModified(t *testing.T) { }, }, }, - }, - } + }) parents.Add( &installconfig.ClusterID{ UUID: "test-uuid", InfraID: "test-infra-id", }, - &installConfig, + installConfig, (*rhcos.Image)(pointer.StringPtr("test-image")), (*rhcos.Release)(pointer.StringPtr("412.86.202208101040-0")), &machine.Worker{ diff --git a/pkg/asset/manifests/infrastructure_test.go b/pkg/asset/manifests/infrastructure_test.go index 6d17f41c843..d0cd13c5d60 100644 --- a/pkg/asset/manifests/infrastructure_test.go +++ b/pkg/asset/manifests/infrastructure_test.go @@ -58,7 +58,7 @@ func TestGenerateInfrastructure(t *testing.T) { UUID: "test-uuid", InfraID: "test-infra-id", }, - &installconfig.InstallConfig{Config: tc.installConfig}, + installconfig.MakeAsset(tc.installConfig), &CloudProviderConfig{}, &AdditionalTrustBundleConfig{}, ) diff --git a/pkg/asset/manifests/ingress_test.go b/pkg/asset/manifests/ingress_test.go index fddbe5f8c78..b4f23196871 100644 --- a/pkg/asset/manifests/ingress_test.go +++ b/pkg/asset/manifests/ingress_test.go @@ -158,10 +158,10 @@ func TestGenerateIngerssDefaultPlacement(t *testing.T) { UUID: "test-uuid", InfraID: "test-infra-id", }, - &installconfig.InstallConfig{ - Config: installConfigFromTopologies(t, tc.installConfigBuildOptions, + installconfig.MakeAsset( + installConfigFromTopologies(t, tc.installConfigBuildOptions, tc.controlPlaneTopology, tc.infrastructureTopology), - }, + ), ) ingressAsset := &Ingress{} err := ingressAsset.Generate(parents) diff --git a/pkg/types/baremetal/validation/platform.go b/pkg/types/baremetal/validation/platform.go index b6c2e66a33f..b3669ed0279 100644 --- a/pkg/types/baremetal/validation/platform.go +++ b/pkg/types/baremetal/validation/platform.go @@ -382,7 +382,7 @@ func validateProvisioningNetworkDisabledSupported(hosts []*baremetal.Host, fldPa } // ValidatePlatform checks that the specified platform is valid. -func ValidatePlatform(p *baremetal.Platform, n *types.Networking, fldPath *field.Path, c *types.InstallConfig) field.ErrorList { +func ValidatePlatform(p *baremetal.Platform, agentBasedInstallation bool, n *types.Networking, fldPath *field.Path, c *types.InstallConfig) field.ErrorList { allErrs := field.ErrorList{} provisioningNetwork := sets.NewString(string(baremetal.ManagedProvisioningNetwork), @@ -405,8 +405,6 @@ func ValidatePlatform(p *baremetal.Platform, n *types.Networking, fldPath *field } } - agentBasedInstallation := validate.IsAgentBasedInstallation() - if !agentBasedInstallation && p.Hosts == nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("hosts"), p.Hosts, "bare metal hosts are missing")) } diff --git a/pkg/types/baremetal/validation/platform_test.go b/pkg/types/baremetal/validation/platform_test.go index bcf22d9069a..d7f31aed7ef 100644 --- a/pkg/types/baremetal/validation/platform_test.go +++ b/pkg/types/baremetal/validation/platform_test.go @@ -348,7 +348,7 @@ interfaces: tc.config.BareMetal = tc.platform } - err := ValidatePlatform(tc.config.BareMetal, network(), field.NewPath("baremetal"), tc.config).ToAggregate() + err := ValidatePlatform(tc.config.BareMetal, false, network(), field.NewPath("baremetal"), tc.config).ToAggregate() if tc.expected == "" { assert.NoError(t, err) diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index bb0e32c20fe..d88ad85496b 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -54,7 +54,7 @@ import ( var pluginsUsingHostPrefix = sets.NewString(string(operv1.NetworkTypeOpenShiftSDN), string(operv1.NetworkTypeOVNKubernetes)) // ValidateInstallConfig checks that the specified install config is valid. -func ValidateInstallConfig(c *types.InstallConfig) field.ErrorList { +func ValidateInstallConfig(c *types.InstallConfig, usingAgentMethod bool) field.ErrorList { allErrs := field.ErrorList{} if c.TypeMeta.APIVersion == "" { return field.ErrorList{field.Required(field.NewPath("apiVersion"), "install-config version required")} @@ -114,7 +114,7 @@ func ValidateInstallConfig(c *types.InstallConfig) field.ErrorList { } else { allErrs = append(allErrs, field.Required(field.NewPath("networking"), "networking is required")) } - allErrs = append(allErrs, validatePlatform(&c.Platform, field.NewPath("platform"), c.Networking, c)...) + allErrs = append(allErrs, validatePlatform(&c.Platform, usingAgentMethod, field.NewPath("platform"), c.Networking, c)...) if c.ControlPlane != nil { allErrs = append(allErrs, validateControlPlane(&c.Platform, c.ControlPlane, field.NewPath("controlPlane"))...) } else { @@ -673,7 +673,7 @@ func ValidateIPinMachineCIDR(vip string, n *types.Networking) error { return fmt.Errorf("IP expected to be in one of the machine networks: %s", strings.Join(networks, ",")) } -func validatePlatform(platform *types.Platform, fldPath *field.Path, network *types.Networking, c *types.InstallConfig) field.ErrorList { +func validatePlatform(platform *types.Platform, usingAgentMethod bool, fldPath *field.Path, network *types.Networking, c *types.InstallConfig) field.ErrorList { allErrs := field.ErrorList{} activePlatform := platform.Name() platforms := make([]string, len(types.PlatformNames)) @@ -722,12 +722,12 @@ func validatePlatform(platform *types.Platform, fldPath *field.Path, network *ty } if platform.VSphere != nil { validate(vsphere.Name, platform.VSphere, func(f *field.Path) field.ErrorList { - return vspherevalidation.ValidatePlatform(platform.VSphere, f, c) + return vspherevalidation.ValidatePlatform(platform.VSphere, usingAgentMethod, f, c) }) } if platform.BareMetal != nil { validate(baremetal.Name, platform.BareMetal, func(f *field.Path) field.ErrorList { - return baremetalvalidation.ValidatePlatform(platform.BareMetal, network, f, c) + return baremetalvalidation.ValidatePlatform(platform.BareMetal, usingAgentMethod, network, f, c) }) } if platform.Ovirt != nil { diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index 9aa962c0ab3..74511661b4b 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -2040,7 +2040,7 @@ func TestValidateInstallConfig(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := ValidateInstallConfig(tc.installConfig).ToAggregate() + err := ValidateInstallConfig(tc.installConfig, false).ToAggregate() if tc.expectedError == "" { assert.NoError(t, err) } else { diff --git a/pkg/types/vsphere/validation/platform.go b/pkg/types/vsphere/validation/platform.go index a2f452e23e2..54dbd115521 100644 --- a/pkg/types/vsphere/validation/platform.go +++ b/pkg/types/vsphere/validation/platform.go @@ -16,7 +16,7 @@ import ( ) // ValidatePlatform checks that the specified platform is valid. -func ValidatePlatform(p *vsphere.Platform, fldPath *field.Path, c *types.InstallConfig) field.ErrorList { +func ValidatePlatform(p *vsphere.Platform, agentBasedInstallation bool, fldPath *field.Path, c *types.InstallConfig) field.ErrorList { isLegacyUpi := false // This is to cover existing UPI non-zonal case // where neither network or cluster is required. @@ -33,7 +33,7 @@ func ValidatePlatform(p *vsphere.Platform, fldPath *field.Path, c *types.Install allErrs = append(allErrs, validateDiskType(p, fldPath)...) } - if !validate.IsAgentBasedInstallation() { + if !agentBasedInstallation { if len(p.VCenters) == 0 { return append(allErrs, field.Required(fldPath.Child("vcenters"), "must be defined")) } diff --git a/pkg/types/vsphere/validation/platform_test.go b/pkg/types/vsphere/validation/platform_test.go index 6994d323363..7b4aede648f 100644 --- a/pkg/types/vsphere/validation/platform_test.go +++ b/pkg/types/vsphere/validation/platform_test.go @@ -288,7 +288,7 @@ func TestValidatePlatform(t *testing.T) { tc.config = installConfig().build() tc.config.VSphere = tc.platform } - err := ValidatePlatform(tc.platform, field.NewPath("test-path"), tc.config).ToAggregate() + err := ValidatePlatform(tc.platform, false, field.NewPath("test-path"), tc.config).ToAggregate() if tc.expectedError == "" { assert.NoError(t, err) } else { diff --git a/pkg/validate/method.go b/pkg/validate/method.go deleted file mode 100644 index ba7cd9d32c7..00000000000 --- a/pkg/validate/method.go +++ /dev/null @@ -1,16 +0,0 @@ -package validate - -import "os" - -// IsAgentBasedInstallation determines whether we are using the 'agent' -// subcommand to install. -func IsAgentBasedInstallation() bool { - if len(os.Args) > 1 { - for _, arg := range os.Args { - if arg == "agent" { - return true - } - } - } - return false -}