Skip to content

Commit ca48654

Browse files
dulekdulek
committed
ImageFilter - add validations, switch to pointers
This commit changes `ID` and `Name` of `ImageFilter` to pointers which should only affect go marshalling. Other than that it adds CEL validation of the ImageFilter, so that Name or Tags can only be set when ID is unset. Conversions are updated accordingly to make sure we only set Name when ID is unset. Moreover validation is added that ID has to be UUID. It's not enforced in conversions, as non-UUID IDs would produce clusters or machines that would not work properly.
1 parent 654d714 commit ca48654

20 files changed

+132
-73
lines changed

‎api/v1alpha5/conversion.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -358,11 +358,10 @@ func Convert_v1alpha5_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *O
358358
}
359359

360360
imageFilter := infrav1.ImageFilter{}
361-
if in.Image != "" {
362-
imageFilter.Name = in.Image
363-
}
364361
if in.ImageUUID != "" {
365-
imageFilter.ID = in.ImageUUID
362+
imageFilter.ID = &in.ImageUUID
363+
} else if in.Image != "" { // Only add name when ID is not set, in v1beta1 it's not possible to set both.
364+
imageFilter.Name = &in.Image
366365
}
367366
out.Image = imageFilter
368367

@@ -643,12 +642,12 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in *i
643642
out.ServerGroupID = in.ServerGroup.ID
644643
}
645644

646-
if in.Image.Name != "" {
647-
out.Image = in.Image.Name
645+
if in.Image.Name != nil && *in.Image.Name != "" {
646+
out.Image = *in.Image.Name
648647
}
649648

650-
if in.Image.ID != "" {
651-
out.ImageUUID = in.Image.ID
649+
if in.Image.ID != nil && *in.Image.ID != "" {
650+
out.ImageUUID = *in.Image.ID
652651
}
653652

654653
if in.IdentityRef != nil {

‎api/v1alpha6/openstackmachine_conversion.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,11 @@ func restorev1alpha6MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa
152152
restorev1alpha6SecurityGroupFilter(&previous.SecurityGroups[i].Filter, &dst.SecurityGroups[i].Filter)
153153
}
154154
}
155+
156+
// Conversion to v1beta1 removes Image when ImageUUID is set
157+
if dst.Image == "" && previous.Image != "" {
158+
dst.Image = previous.Image
159+
}
155160
}
156161

157162
func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infrav1.OpenStackMachineSpec) {
@@ -258,11 +263,10 @@ func Convert_v1alpha6_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *O
258263
}
259264

260265
imageFilter := infrav1.ImageFilter{}
261-
if in.Image != "" {
262-
imageFilter.Name = in.Image
263-
}
264266
if in.ImageUUID != "" {
265-
imageFilter.ID = in.ImageUUID
267+
imageFilter.ID = &in.ImageUUID
268+
} else if in.Image != "" { // Only add name when ID is not set, in v1beta1 it's not possible to set both.
269+
imageFilter.Name = &in.Image
266270
}
267271
out.Image = imageFilter
268272

@@ -306,12 +310,12 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in *i
306310
out.ServerGroupID = in.ServerGroup.ID
307311
}
308312

309-
if in.Image.Name != "" {
310-
out.Image = in.Image.Name
313+
if in.Image.Name != nil && *in.Image.Name != "" {
314+
out.Image = *in.Image.Name
311315
}
312316

313-
if in.Image.ID != "" {
314-
out.ImageUUID = in.Image.ID
317+
if in.Image.ID != nil && *in.Image.ID != "" {
318+
out.ImageUUID = *in.Image.ID
315319
}
316320

317321
if len(in.ServerMetadata) > 0 {

‎api/v1alpha7/openstackmachine_conversion.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,11 @@ func restorev1alpha7MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa
126126
restorev1alpha7SecurityGroupFilter(&previous.SecurityGroups[i], &dst.SecurityGroups[i])
127127
}
128128
}
129+
130+
// Conversion to v1beta1 removes Image when ImageUUID is set
131+
if dst.Image == "" && previous.Image != "" {
132+
dst.Image = previous.Image
133+
}
129134
}
130135

131136
func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infrav1.OpenStackMachineSpec) {
@@ -152,11 +157,10 @@ func Convert_v1alpha7_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *O
152157
}
153158

154159
imageFilter := infrav1.ImageFilter{}
155-
if in.Image != "" {
156-
imageFilter.Name = in.Image
157-
}
158160
if in.ImageUUID != "" {
159-
imageFilter.ID = in.ImageUUID
161+
imageFilter.ID = &in.ImageUUID
162+
} else if in.Image != "" { // Only add name when ID is not set, in v1beta1 it's not possible to set both.
163+
imageFilter.Name = &in.Image
160164
}
161165
out.Image = imageFilter
162166

@@ -197,12 +201,12 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha7_OpenStackMachineSpec(in *i
197201
out.ServerGroupID = in.ServerGroup.ID
198202
}
199203

200-
if in.Image.Name != "" {
201-
out.Image = in.Image.Name
204+
if in.Image.Name != nil && *in.Image.Name != "" {
205+
out.Image = *in.Image.Name
202206
}
203207

204-
if in.Image.ID != "" {
205-
out.ImageUUID = in.Image.ID
208+
if in.Image.ID != nil && *in.Image.ID != "" {
209+
out.ImageUUID = *in.Image.ID
206210
}
207211

208212
if len(in.ServerMetadata) > 0 {

‎api/v1beta1/openstackmachine_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type OpenStackMachineSpec struct {
4343

4444
// The image to use for your server instance.
4545
// If the rootVolume is specified, this will be used when creating the root volume.
46-
Image ImageFilter `json:"image,omitempty"`
46+
Image ImageFilter `json:"image"`
4747

4848
// The ssh key to inject in the instance
4949
SSHKeyName string `json:"sshKeyName,omitempty"`

‎api/v1beta1/types.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,19 @@ type OpenStackMachineTemplateResource struct {
2828
Spec OpenStackMachineSpec `json:"spec"`
2929
}
3030

31+
// ImageFilter describes the data needed to identify which image to use.
32+
// +kubebuilder:validation:XValidation:rule="(has(self.id) && !has(self.name) && !has(self.tags)) || !has(self.id)",message="when ID is set you cannot set other options"
3133
type ImageFilter struct {
32-
// The ID of the desired image. If this is provided, the other filters will be ignored.
33-
ID string `json:"id,omitempty"`
34+
// The ID of the desired image. If ID is provided, the other filters cannot be provided.
35+
// +kubebuilder:validation:Format:=uuid
36+
// +optional
37+
ID optional.String `json:"id,omitempty"`
3438
// The name of the desired image. If specified, the combination of name and tags must return a single matching image or an error will be raised.
35-
Name string `json:"name,omitempty"`
39+
// +optional
40+
Name optional.String `json:"name,omitempty"`
3641
// The tags associated with the desired image. If specified, the combination of name and tags must return a single matching image or an error will be raised.
42+
// +listType=set
43+
// +optional
3744
Tags []string `json:"tags,omitempty"`
3845
}
3946

‎api/v1beta1/zz_generated.deepcopy.go

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5022,8 +5022,9 @@ spec:
50225022
If the rootVolume is specified, this will be used when creating the root volume.
50235023
properties:
50245024
id:
5025-
description: The ID of the desired image. If this is provided,
5026-
the other filters will be ignored.
5025+
description: The ID of the desired image. If ID is provided,
5026+
the other filters cannot be provided.
5027+
format: uuid
50275028
type: string
50285029
name:
50295030
description: The name of the desired image. If specified,
@@ -5037,7 +5038,12 @@ spec:
50375038
items:
50385039
type: string
50395040
type: array
5041+
x-kubernetes-list-type: set
50405042
type: object
5043+
x-kubernetes-validations:
5044+
- message: when ID is set you cannot set other options
5045+
rule: (has(self.id) && !has(self.name) && !has(self.tags))
5046+
|| !has(self.id)
50415047
instanceID:
50425048
description: InstanceID is the OpenStack instance ID for this
50435049
machine.
@@ -5540,6 +5546,7 @@ spec:
55405546
type: boolean
55415547
required:
55425548
- flavor
5549+
- image
55435550
type: object
55445551
type: object
55455552
controlPlaneAvailabilityZones:

‎config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2447,8 +2447,9 @@ spec:
24472447
If the rootVolume is specified, this will be used when creating the root volume.
24482448
properties:
24492449
id:
2450-
description: The ID of the desired image. If this
2451-
is provided, the other filters will be ignored.
2450+
description: The ID of the desired image. If ID
2451+
is provided, the other filters cannot be provided.
2452+
format: uuid
24522453
type: string
24532454
name:
24542455
description: The name of the desired image. If
@@ -2464,7 +2465,12 @@ spec:
24642465
items:
24652466
type: string
24662467
type: array
2468+
x-kubernetes-list-type: set
24672469
type: object
2470+
x-kubernetes-validations:
2471+
- message: when ID is set you cannot set other options
2472+
rule: (has(self.id) && !has(self.name) && !has(self.tags))
2473+
|| !has(self.id)
24682474
instanceID:
24692475
description: InstanceID is the OpenStack instance
24702476
ID for this machine.
@@ -2972,6 +2978,7 @@ spec:
29722978
type: boolean
29732979
required:
29742980
- flavor
2981+
- image
29752982
type: object
29762983
type: object
29772984
controlPlaneAvailabilityZones:

‎config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,8 +1810,9 @@ spec:
18101810
If the rootVolume is specified, this will be used when creating the root volume.
18111811
properties:
18121812
id:
1813-
description: The ID of the desired image. If this is provided,
1814-
the other filters will be ignored.
1813+
description: The ID of the desired image. If ID is provided, the
1814+
other filters cannot be provided.
1815+
format: uuid
18151816
type: string
18161817
name:
18171818
description: The name of the desired image. If specified, the
@@ -1825,7 +1826,11 @@ spec:
18251826
items:
18261827
type: string
18271828
type: array
1829+
x-kubernetes-list-type: set
18281830
type: object
1831+
x-kubernetes-validations:
1832+
- message: when ID is set you cannot set other options
1833+
rule: (has(self.id) && !has(self.name) && !has(self.tags)) || !has(self.id)
18291834
instanceID:
18301835
description: InstanceID is the OpenStack instance ID for this machine.
18311836
type: string
@@ -2324,6 +2329,7 @@ spec:
23242329
type: boolean
23252330
required:
23262331
- flavor
2332+
- image
23272333
type: object
23282334
status:
23292335
description: OpenStackMachineStatus defines the observed state of OpenStackMachine.

‎config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,8 +1483,9 @@ spec:
14831483
If the rootVolume is specified, this will be used when creating the root volume.
14841484
properties:
14851485
id:
1486-
description: The ID of the desired image. If this is provided,
1487-
the other filters will be ignored.
1486+
description: The ID of the desired image. If ID is provided,
1487+
the other filters cannot be provided.
1488+
format: uuid
14881489
type: string
14891490
name:
14901491
description: The name of the desired image. If specified,
@@ -1498,7 +1499,12 @@ spec:
14981499
items:
14991500
type: string
15001501
type: array
1502+
x-kubernetes-list-type: set
15011503
type: object
1504+
x-kubernetes-validations:
1505+
- message: when ID is set you cannot set other options
1506+
rule: (has(self.id) && !has(self.name) && !has(self.tags))
1507+
|| !has(self.id)
15021508
instanceID:
15031509
description: InstanceID is the OpenStack instance ID for this
15041510
machine.
@@ -2001,6 +2007,7 @@ spec:
20012007
type: boolean
20022008
required:
20032009
- flavor
2010+
- image
20042011
type: object
20052012
required:
20062013
- spec

‎controllers/openstackmachine_controller_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func getDefaultOpenStackMachine() *infrav1.OpenStackMachine {
8383
// FloatingIP is only used by the cluster controller for the Bastion
8484
// TODO: Test Networks, Ports, Subnet, and Trunk separately
8585
Flavor: flavorName,
86-
Image: infrav1.ImageFilter{ID: imageUUID},
86+
Image: infrav1.ImageFilter{ID: pointer.String(imageUUID)},
8787
SSHKeyName: sshKeyName,
8888
Tags: []string{"test-tag"},
8989
ServerMetadata: []infrav1.ServerMetadata{

‎docs/book/src/api/v1beta1/api.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1589,6 +1589,7 @@ address in any subnet of the port&rsquo;s network.</p>
15891589
<a href="#infrastructure.cluster.x-k8s.io/v1beta1.OpenStackMachineSpec">OpenStackMachineSpec</a>)
15901590
</p>
15911591
<p>
1592+
<p>ImageFilter describes the data needed to identify which image to use.</p>
15921593
</p>
15931594
<table>
15941595
<thead>
@@ -1606,7 +1607,8 @@ string
16061607
</em>
16071608
</td>
16081609
<td>
1609-
<p>The ID of the desired image. If this is provided, the other filters will be ignored.</p>
1610+
<em>(Optional)</em>
1611+
<p>The ID of the desired image. If ID is provided, the other filters cannot be provided.</p>
16101612
</td>
16111613
</tr>
16121614
<tr>
@@ -1617,6 +1619,7 @@ string
16171619
</em>
16181620
</td>
16191621
<td>
1622+
<em>(Optional)</em>
16201623
<p>The name of the desired image. If specified, the combination of name and tags must return a single matching image or an error will be raised.</p>
16211624
</td>
16221625
</tr>
@@ -1628,6 +1631,7 @@ string
16281631
</em>
16291632
</td>
16301633
<td>
1634+
<em>(Optional)</em>
16311635
<p>The tags associated with the desired image. If specified, the combination of name and tags must return a single matching image or an error will be raised.</p>
16321636
</td>
16331637
</tr>

‎docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ If empty object or null is provided, Machine will not be added to any server gro
9191
#### Change to image
9292

9393
The field `image` is now an `ImageFilter` object rather than a string name.
94-
The `ImageFilter` object allows selection of an image by name, by ID or by tags.
94+
The `ImageFilter` object allows selection of an image by ID or by name and tags. If ID is set, no other fields can be set in the object.
9595

9696
```yaml
9797
image: "test-image"

‎pkg/cloud/services/compute/instance.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,8 @@ func applyServerGroupID(opts servers.CreateOptsBuilder, serverGroupID string) se
333333

334334
// Helper function for getting image ID from name, ID, or tags.
335335
func (s *Service) GetImageID(image infrav1.ImageFilter) (string, error) {
336-
if image.ID != "" {
337-
return image.ID, nil
336+
if image.ID != nil {
337+
return *image.ID, nil
338338
}
339339

340340
listOpts := filterconvert.ImageFilterToListOpts(&image)

0 commit comments

Comments
 (0)