Skip to content
This repository was archived by the owner on Jul 2, 2025. It is now read-only.

Commit 4e87275

Browse files
committed
fix tag validation
1 parent 81fec4f commit 4e87275

File tree

7 files changed

+83
-67
lines changed

7 files changed

+83
-67
lines changed

pkg/vsphere/apis/provider_spec.go

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@ const (
3535
type VsphereProviderSpec struct {
3636
V1 *VsphereProviderSpec1 `json:"v1,omitempty"`
3737
V2 *VsphereProviderSpec2 `json:"v2,omitempty"`
38+
39+
// SSHKeys is an optional array of ssh public keys to deploy to VM (may already be included in UserData)
40+
// +optional
41+
SSHKeys []string `json:"sshKeys,omitempty"`
42+
// Tags to be placed on the VM
43+
// +optional
44+
Tags map[string]string `json:"tags,omitempty"`
3845
}
3946

4047
// VsphereProviderSpec1 contains the fields of
@@ -101,13 +108,6 @@ type VsphereProviderSpec1 struct {
101108
// Customization is an experimental option to add a CustomizationSpec
102109
// +optional
103110
Customization string `json:"customization,omitempty"`
104-
105-
// SSHKeys is an optional array of ssh public keys to deploy to VM (may already be included in UserData)
106-
// +optional
107-
SSHKeys []string `json:"sshKeys,omitempty"`
108-
// Tags to be placed on the VM
109-
// +optional
110-
Tags map[string]string `json:"tags,omitempty"`
111111
}
112112

113113
// SpecVersion returns spec version
@@ -154,13 +154,6 @@ type VsphereProviderSpec2 struct {
154154
// e.g. sched.swap.vmxSwapEnabled=false to disable the VMX process swap file
155155
// +optional
156156
//ExtraConfig map[string]string `json:"extraConfig,omitempty"`
157-
158-
// SSHKeys is an optional array of ssh public keys to deploy to VM (may already be included in UserData)
159-
// +optional
160-
SSHKeys []string `json:"sshKeys,omitempty"`
161-
// Tags to be placed on the VM
162-
// +optional
163-
Tags map[string]string `json:"tags,omitempty"`
164157
}
165158

166159
// SpecVersion returns spec version

pkg/vsphere/apis/validation/validation.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,20 @@ import (
2626
)
2727

2828
// ValidateVsphereProviderSpec1 validates Vsphere provider spec
29-
func ValidateVsphereProviderSpec1(spec *api.VsphereProviderSpec1, secrets *corev1.Secret) []error {
29+
func ValidateVsphereProviderSpec1(spec *api.VsphereProviderSpec, secrets *corev1.Secret) []error {
3030
var allErrs []error
3131

32-
if "" == spec.Datastore && "" == spec.DatastoreCluster {
32+
v1 := spec.V1
33+
if "" == v1.Datastore && "" == v1.DatastoreCluster {
3334
allErrs = append(allErrs, fmt.Errorf("either datastoreCluster or datastore field is required"))
3435
}
35-
if "" == spec.TemplateVM {
36+
if "" == v1.TemplateVM {
3637
allErrs = append(allErrs, fmt.Errorf("templateVM is a required field"))
3738
}
38-
if "" == spec.ComputeCluster && "" == spec.ResourcePool && "" == spec.HostSystem {
39+
if "" == v1.ComputeCluster && "" == v1.ResourcePool && "" == v1.HostSystem {
3940
allErrs = append(allErrs, fmt.Errorf("either computeCluster or resourcePool or hostSystem field is required"))
4041
}
41-
if "" == spec.Network {
42+
if "" == v1.Network {
4243
allErrs = append(allErrs, fmt.Errorf("network is a required field"))
4344
}
4445

@@ -78,22 +79,23 @@ func validateSecrets(secret *corev1.Secret) []error {
7879
}
7980

8081
// ValidateVsphereProviderSpec2 validates Vsphere provider spec2
81-
func ValidateVsphereProviderSpec2(spec *api.VsphereProviderSpec2, secrets *corev1.Secret) []error {
82+
func ValidateVsphereProviderSpec2(spec *api.VsphereProviderSpec, secrets *corev1.Secret) []error {
8283
var allErrs []error
8384

84-
if "" == spec.Namespace {
85+
v2 := spec.V2
86+
if "" == v2.Namespace {
8587
allErrs = append(allErrs, fmt.Errorf("namespace is a required field"))
8688
}
87-
if "" == spec.ImageName {
89+
if "" == v2.ImageName {
8890
allErrs = append(allErrs, fmt.Errorf("imageName is a required field"))
8991
}
90-
if "" == spec.NetworkType {
92+
if "" == v2.NetworkType {
9193
allErrs = append(allErrs, fmt.Errorf("networkType is a required field"))
9294
}
93-
if "" == spec.NetworkName {
95+
if "" == v2.NetworkName {
9496
allErrs = append(allErrs, fmt.Errorf("networkName is a required field"))
9597
}
96-
if "" == spec.ClassName {
98+
if "" == v2.ClassName {
9799
allErrs = append(allErrs, fmt.Errorf("className is a required field"))
98100
}
99101

pkg/vsphere/internal/plugin_spi.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ func NewPluginSPISwitch() *PluginSPISwitch {
4545
// CreateMachine creates a VM by cloning from a template
4646
func (spi *PluginSPISwitch) CreateMachine(ctx context.Context, machineName string, providerSpec *api.VsphereProviderSpec, secrets *corev1.Secret) (string, error) {
4747
if providerSpec.V1 != nil {
48-
return spi.spec1.CreateMachine(ctx, machineName, providerSpec.V1, secrets)
48+
return spi.spec1.CreateMachine(ctx, machineName, providerSpec, secrets)
4949
}
5050
if providerSpec.V2 != nil {
51-
return spi.spec2.CreateMachine(ctx, machineName, providerSpec.V2, secrets)
51+
return spi.spec2.CreateMachine(ctx, machineName, providerSpec, secrets)
5252
}
5353
return "", fmt.Errorf("invalid spec version")
5454
}
@@ -90,10 +90,10 @@ func (spi *PluginSPISwitch) GetMachineStatus(ctx context.Context, machineName st
9090
// ListMachines lists all VMs in the DC or folder
9191
func (spi *PluginSPISwitch) ListMachines(ctx context.Context, providerSpec *api.VsphereProviderSpec, secrets *corev1.Secret) (map[string]string, error) {
9292
if providerSpec.V1 != nil {
93-
return spi.spec1.ListMachines(ctx, providerSpec.V1, secrets)
93+
return spi.spec1.ListMachines(ctx, providerSpec, secrets)
9494
}
9595
if providerSpec.V2 != nil {
96-
return spi.spec2.ListMachines(ctx, providerSpec.V2, secrets)
96+
return spi.spec2.ListMachines(ctx, providerSpec, secrets)
9797
}
9898
return nil, fmt.Errorf("invalid spec version")
9999
}

pkg/vsphere/internal/vmomi/clone.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ type clone struct {
5050
name string
5151
userData string
5252
spec *api.VsphereProviderSpec1
53+
sshKeys []string
54+
tags map[string]string
5355

5456
NetworkFlag *flags.NetworkFlag
5557

@@ -66,8 +68,8 @@ type clone struct {
6668
Clone *object.VirtualMachine
6769
}
6870

69-
func newClone(machineName string, spec *api.VsphereProviderSpec1, userData string) *clone {
70-
return &clone{name: machineName, spec: spec, userData: userData}
71+
func newClone(machineName string, spec *api.VsphereProviderSpec1, sshKeys []string, tags map[string]string, userData string) *clone {
72+
return &clone{name: machineName, spec: spec, sshKeys: sshKeys, tags: tags, userData: userData}
7173
}
7274

7375
func (cmd *clone) Run(ctx context.Context, client *govmomi.Client) error {
@@ -165,9 +167,9 @@ func (cmd *clone) Run(ctx context.Context, client *govmomi.Client) error {
165167
}
166168
klog.V(4).Infof("Template guestId: %s, used guestId: %s", props.Config.GuestId, guestID)
167169

168-
sshkeys := make([]string, len(cmd.spec.SSHKeys))
169-
for i := range cmd.spec.SSHKeys {
170-
sshkeys[i] = strings.TrimSpace(cmd.spec.SSHKeys[i])
170+
sshkeys := make([]string, len(cmd.sshKeys))
171+
for i := range cmd.sshKeys {
172+
sshkeys[i] = strings.TrimSpace(cmd.sshKeys[i])
171173
}
172174
vapp := cmd.spec.VApp
173175
if vapp == nil {
@@ -282,13 +284,13 @@ func (cmd *clone) Run(ctx context.Context, client *govmomi.Client) error {
282284
return errors.Wrap(err, "reconfiguring VM failed")
283285
}
284286

285-
if len(cmd.spec.Tags) > 0 {
287+
if len(cmd.tags) > 0 {
286288
manager, err := object.GetCustomFieldsManager(client.Client)
287289
if err != nil {
288290
return errors.Wrap(err, "Set tags: GetCustomFieldsManager failed")
289291
}
290292

291-
for k, v := range cmd.spec.Tags {
293+
for k, v := range cmd.tags {
292294
key, err := manager.FindKey(ctx, k)
293295
if err != nil {
294296
if err != object.ErrKeyNameNotFound {

pkg/vsphere/internal/vmomi/plugin_spi_impl.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,25 @@ type PluginSPIImpl struct{}
4141
const providerPrefix = "vsphere://"
4242

4343
// CreateMachine creates a VM by cloning from a template
44-
func (spi *PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, providerSpec *api.VsphereProviderSpec1, secrets *corev1.Secret) (string, error) {
44+
func (spi *PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, providerSpec *api.VsphereProviderSpec, secrets *corev1.Secret) (string, error) {
4545
client, err := createVsphereClient(ctx, secrets)
4646
if err != nil {
4747
return "", err
4848
}
4949
defer client.Logout(ctx)
5050

51-
cmd := newClone(machineName, providerSpec, string(secrets.Data["userData"]))
51+
v1 := providerSpec.V1
52+
if v1 == nil {
53+
return "", fmt.Errorf("missing v1")
54+
}
55+
56+
cmd := newClone(machineName, v1, providerSpec.SSHKeys, providerSpec.Tags, string(secrets.Data["userData"]))
5257
err = cmd.Run(ctx, client)
5358
if err != nil {
5459
return "", err
5560
}
5661
machineID := cmd.Clone.UUID(ctx)
57-
providerID := spi.encodeProviderID(providerSpec.Region, machineID)
62+
providerID := spi.encodeProviderID(v1.Region, machineID)
5863
return providerID, nil
5964
}
6065

@@ -135,13 +140,18 @@ func (spi *PluginSPIImpl) GetMachineStatus(ctx context.Context, machineName stri
135140
}
136141

137142
// ListMachines lists all VMs in the DC or folder
138-
func (spi *PluginSPIImpl) ListMachines(ctx context.Context, providerSpec *api.VsphereProviderSpec1, secrets *corev1.Secret) (map[string]string, error) {
143+
func (spi *PluginSPIImpl) ListMachines(ctx context.Context, providerSpec *api.VsphereProviderSpec, secrets *corev1.Secret) (map[string]string, error) {
139144
client, err := createVsphereClient(ctx, secrets)
140145
if err != nil {
141146
return nil, err
142147
}
143148
defer client.Logout(ctx)
144149

150+
v1 := providerSpec.V1
151+
if v1 == nil {
152+
return nil, fmt.Errorf("missing v1")
153+
}
154+
145155
machineList := map[string]string{}
146156

147157
relevantTags, _ := tags.NewRelevantTags(providerSpec.Tags)
@@ -158,18 +168,18 @@ func (spi *PluginSPIImpl) ListMachines(ctx context.Context, providerSpec *api.Vs
158168

159169
if relevantTags.Matches(customValues) {
160170
uuid := vm.UUID(ctx)
161-
providerID := spi.encodeProviderID(providerSpec.Region, uuid)
171+
providerID := spi.encodeProviderID(v1.Region, uuid)
162172
machineList[providerID] = obj.Name
163173
}
164174
return nil
165175
}
166176

167-
err = visitVirtualMachines(ctx, client, providerSpec, visitor)
177+
err = visitVirtualMachines(ctx, client, v1, visitor)
168178
if err != nil {
169179
return nil, err
170180
}
171181

172-
klog.V(2).Infof("List machines request for dc %s, folder %s found %d machines", providerSpec.Datacenter, providerSpec.Folder, len(machineList))
182+
klog.V(2).Infof("List machines request for dc %s, folder %s found %d machines", v1.Datacenter, v1.Folder, len(machineList))
173183

174184
return machineList, nil
175185
}

pkg/vsphere/internal/vmop/plugin_spi_impl.go

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func init() {
6666
}
6767

6868
// CreateMachine creates a VM by cloning from a template
69-
func (spi *PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, providerSpec *api.VsphereProviderSpec2, secrets *corev1.Secret) (string, error) {
69+
func (spi *PluginSPIImpl) CreateMachine(ctx context.Context, machineName string, providerSpec *api.VsphereProviderSpec, secrets *corev1.Secret) (string, error) {
7070
client, err := createVsphereKubernetesClient(ctx, secrets)
7171
if err != nil {
7272
return "", fmt.Errorf("creating vsphere k8s client failed: %w", err)
@@ -77,10 +77,14 @@ func (spi *PluginSPIImpl) CreateMachine(ctx context.Context, machineName string,
7777
return "", fmt.Errorf("adding ssh keys to userdata failed: %w", err)
7878
}
7979

80+
v2 := providerSpec.V2
81+
if v2 == nil {
82+
return "", fmt.Errorf("missing v2")
83+
}
8084
configMap := &corev1.ConfigMap{
8185
ObjectMeta: metav1.ObjectMeta{
8286
Name: configMapName(machineName),
83-
Namespace: providerSpec.Namespace,
87+
Namespace: v2.Namespace,
8488
},
8589
}
8690
_, err = controllerutil.CreateOrUpdate(ctx, client, configMap, func() error {
@@ -101,18 +105,18 @@ func (spi *PluginSPIImpl) CreateMachine(ctx context.Context, machineName string,
101105
return "", fmt.Errorf("missing relevant tags")
102106
}
103107

104-
vm := createEmptyVirtualMachine(machineName, providerSpec.Namespace)
105-
vm.Spec.ClassName = providerSpec.ClassName
108+
vm := createEmptyVirtualMachine(machineName, v2.Namespace)
109+
vm.Spec.ClassName = v2.ClassName
106110
vm.Spec.NetworkInterfaces = []vmopapi.VirtualMachineNetworkInterface{
107-
{NetworkType: providerSpec.NetworkType, NetworkName: providerSpec.NetworkName},
111+
{NetworkType: v2.NetworkType, NetworkName: v2.NetworkName},
108112
}
109-
if providerSpec.StorageClass != nil {
110-
vm.Spec.StorageClass = *providerSpec.StorageClass
113+
if v2.StorageClass != nil {
114+
vm.Spec.StorageClass = *v2.StorageClass
111115
}
112-
if providerSpec.ResourcePolicyName != nil {
113-
vm.Spec.ResourcePolicyName = *providerSpec.ResourcePolicyName
116+
if v2.ResourcePolicyName != nil {
117+
vm.Spec.ResourcePolicyName = *v2.ResourcePolicyName
114118
}
115-
vm.Spec.ImageName = providerSpec.ImageName
119+
vm.Spec.ImageName = v2.ImageName
116120
vm.Spec.VmMetadata = &vmopapi.VirtualMachineMetadata{
117121
ConfigMapName: configMap.Name,
118122
Transport: "ExtraConfig",
@@ -141,7 +145,7 @@ func (spi *PluginSPIImpl) CreateMachine(ctx context.Context, machineName string,
141145
return "", fmt.Errorf("timeout on vm create of virtual machine %s. phase=%s", machineName, vm.Status.Phase)
142146
}
143147

144-
providerID := spi.encodeProviderID(providerSpec.Namespace, vm.Status.InstanceUUID)
148+
providerID := spi.encodeProviderID(v2.Namespace, vm.Status.InstanceUUID)
145149
return providerID, nil
146150
}
147151

@@ -229,12 +233,17 @@ func (spi *PluginSPIImpl) GetMachineStatus(ctx context.Context, machineName stri
229233
}
230234

231235
// ListMachines lists all VMs in the DC or folder
232-
func (spi *PluginSPIImpl) ListMachines(ctx context.Context, providerSpec *api.VsphereProviderSpec2, secrets *corev1.Secret) (map[string]string, error) {
236+
func (spi *PluginSPIImpl) ListMachines(ctx context.Context, providerSpec *api.VsphereProviderSpec, secrets *corev1.Secret) (map[string]string, error) {
233237
client, err := createVsphereKubernetesClient(ctx, secrets)
234238
if err != nil {
235239
return nil, fmt.Errorf("creating vsphere k8s client failed: %w", err)
236240
}
237241

242+
v2 := providerSpec.V2
243+
if v2 == nil {
244+
return nil, fmt.Errorf("missing v2")
245+
}
246+
238247
machineList := map[string]string{}
239248
relevantTags, _ := tags.NewRelevantTags(providerSpec.Tags)
240249
if relevantTags == nil {
@@ -244,20 +253,20 @@ func (spi *PluginSPIImpl) ListMachines(ctx context.Context, providerSpec *api.Vs
244253
vms := &vmopapi.VirtualMachineList{}
245254
labels := relevantTags.GetLabels()
246255
labels[api.LabelMCMVSphere] = "true"
247-
err = client.List(ctx, vms, ctrlClient.InNamespace(providerSpec.Namespace), ctrlClient.MatchingLabels(labels))
256+
err = client.List(ctx, vms, ctrlClient.InNamespace(v2.Namespace), ctrlClient.MatchingLabels(labels))
248257
if err != nil {
249-
return nil, fmt.Errorf("listing virtual machines in namespace %s failed: %w", providerSpec.Namespace, err)
258+
return nil, fmt.Errorf("listing virtual machines in namespace %s failed: %w", v2.Namespace, err)
250259
}
251260

252261
for _, vm := range vms.Items {
253262
machineName := vm.Name
254263
if vm.Status.InstanceUUID != "" {
255-
providerID := spi.encodeProviderID(providerSpec.Namespace, vm.Status.InstanceUUID)
264+
providerID := spi.encodeProviderID(v2.Namespace, vm.Status.InstanceUUID)
256265
machineList[providerID] = machineName
257266
}
258267
}
259268

260-
klog.V(2).Infof("List machines request for namespace %s found %d machines", providerSpec.Namespace, len(machineList))
269+
klog.V(2).Infof("List machines request for namespace %s found %d machines", v2.Namespace, len(machineList))
261270

262271
return machineList, nil
263272
}

pkg/vsphere/machine_server_util.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,33 +43,33 @@ func decodeProviderSpecAndSecret(machineClass *v1alpha1.MachineClass, secret *co
4343
}
4444

4545
if providerSpec.V1 != nil {
46-
err := validateSpec1(providerSpec.V1, secret)
46+
err := validateSpecV1(providerSpec, secret)
4747
return providerSpec, err
4848
}
4949

5050
if providerSpec.V2 != nil {
51-
err := validateSpec2(providerSpec.V2, secret)
51+
err := validateSpecV2(providerSpec, secret)
5252
return providerSpec, err
5353
}
5454

5555
return nil, fmt.Errorf("invalid providerSpec")
5656
}
5757

58-
// validateSpec1 validates api.VsphereProviderSpec1
59-
func validateSpec1(spec1 *api.VsphereProviderSpec1, secret *corev1.Secret) error {
58+
// validateSpecV1 validates api.VsphereProviderSpec1
59+
func validateSpecV1(spec *api.VsphereProviderSpec, secret *corev1.Secret) error {
6060
//Validate the Spec and Secrets
61-
ValidationErr := validation.ValidateVsphereProviderSpec1(spec1, secret)
61+
ValidationErr := validation.ValidateVsphereProviderSpec1(spec, secret)
6262
if ValidationErr != nil {
6363
err := fmt.Errorf("Error while validating ProviderSpec V1 %v", ValidationErr)
6464
return status.Error(codes.Internal, err.Error())
6565
}
6666
return nil
6767
}
6868

69-
// validateSpec2 validates api.VsphereProviderSpec2
70-
func validateSpec2(spec2 *api.VsphereProviderSpec2, secret *corev1.Secret) error {
69+
// validateSpecV2 validates api.VsphereProviderSpec2
70+
func validateSpecV2(spec *api.VsphereProviderSpec, secret *corev1.Secret) error {
7171
//Validate the Spec and Secrets
72-
ValidationErr := validation.ValidateVsphereProviderSpec2(spec2, secret)
72+
ValidationErr := validation.ValidateVsphereProviderSpec2(spec, secret)
7373
if ValidationErr != nil {
7474
err := fmt.Errorf("Error while validating ProviderSpec V2 %v", ValidationErr)
7575
return status.Error(codes.Internal, err.Error())

0 commit comments

Comments
 (0)