From a00fa808108fc7749b2f438f7334992e5320118c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dulko?= Date: Tue, 12 Mar 2024 14:15:33 +0100 Subject: [PATCH 1/2] 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. --- api/v1alpha5/conversion.go | 15 +++++++------- api/v1alpha6/openstackmachine_conversion.go | 20 +++++++++++-------- api/v1alpha7/openstackmachine_conversion.go | 20 +++++++++++-------- api/v1beta1/openstackmachine_types.go | 3 ++- api/v1beta1/types.go | 13 +++++++++--- api/v1beta1/zz_generated.deepcopy.go | 10 ++++++++++ ...re.cluster.x-k8s.io_openstackclusters.yaml | 12 +++++++++-- ...er.x-k8s.io_openstackclustertemplates.yaml | 12 +++++++++-- ...re.cluster.x-k8s.io_openstackmachines.yaml | 10 ++++++++-- ...er.x-k8s.io_openstackmachinetemplates.yaml | 12 +++++++++-- .../openstackmachine_controller_test.go | 2 +- docs/book/src/api/v1beta1/api.md | 6 +++++- .../topics/crd-changes/v1alpha7-to-v1beta1.md | 2 +- pkg/cloud/services/compute/instance.go | 4 ++-- pkg/cloud/services/compute/instance_test.go | 14 ++++++------- .../compute/referenced_resources_test.go | 9 +++++---- pkg/utils/filterconvert/convert.go | 16 +++++++++------ pkg/webhooks/openstackcluster_webhook_test.go | 8 ++++---- .../openstackmachinetemplate_webhook_test.go | 16 +++++++-------- test/e2e/suites/e2e/e2e_test.go | 4 ++-- 20 files changed, 136 insertions(+), 72 deletions(-) diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go index cd66e62bd3..1ba726f9c8 100644 --- a/api/v1alpha5/conversion.go +++ b/api/v1alpha5/conversion.go @@ -358,11 +358,10 @@ func Convert_v1alpha5_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *O } imageFilter := infrav1.ImageFilter{} - if in.Image != "" { - imageFilter.Name = in.Image - } if in.ImageUUID != "" { - imageFilter.ID = in.ImageUUID + imageFilter.ID = &in.ImageUUID + } else if in.Image != "" { // Only add name when ID is not set, in v1beta1 it's not possible to set both. + imageFilter.Name = &in.Image } out.Image = imageFilter @@ -643,12 +642,12 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in *i out.ServerGroupID = in.ServerGroup.ID } - if in.Image.Name != "" { - out.Image = in.Image.Name + if in.Image.Name != nil && *in.Image.Name != "" { + out.Image = *in.Image.Name } - if in.Image.ID != "" { - out.ImageUUID = in.Image.ID + if in.Image.ID != nil && *in.Image.ID != "" { + out.ImageUUID = *in.Image.ID } if in.IdentityRef != nil { diff --git a/api/v1alpha6/openstackmachine_conversion.go b/api/v1alpha6/openstackmachine_conversion.go index 6720f99f90..108db09757 100644 --- a/api/v1alpha6/openstackmachine_conversion.go +++ b/api/v1alpha6/openstackmachine_conversion.go @@ -152,6 +152,11 @@ func restorev1alpha6MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa restorev1alpha6SecurityGroupFilter(&previous.SecurityGroups[i].Filter, &dst.SecurityGroups[i].Filter) } } + + // Conversion to v1beta1 removes Image when ImageUUID is set + if dst.Image == "" && previous.Image != "" { + dst.Image = previous.Image + } } func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infrav1.OpenStackMachineSpec) { @@ -258,11 +263,10 @@ func Convert_v1alpha6_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *O } imageFilter := infrav1.ImageFilter{} - if in.Image != "" { - imageFilter.Name = in.Image - } if in.ImageUUID != "" { - imageFilter.ID = in.ImageUUID + imageFilter.ID = &in.ImageUUID + } else if in.Image != "" { // Only add name when ID is not set, in v1beta1 it's not possible to set both. + imageFilter.Name = &in.Image } out.Image = imageFilter @@ -306,12 +310,12 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in *i out.ServerGroupID = in.ServerGroup.ID } - if in.Image.Name != "" { - out.Image = in.Image.Name + if in.Image.Name != nil && *in.Image.Name != "" { + out.Image = *in.Image.Name } - if in.Image.ID != "" { - out.ImageUUID = in.Image.ID + if in.Image.ID != nil && *in.Image.ID != "" { + out.ImageUUID = *in.Image.ID } if len(in.ServerMetadata) > 0 { diff --git a/api/v1alpha7/openstackmachine_conversion.go b/api/v1alpha7/openstackmachine_conversion.go index 575f8ad40b..0939b1c2f6 100644 --- a/api/v1alpha7/openstackmachine_conversion.go +++ b/api/v1alpha7/openstackmachine_conversion.go @@ -126,6 +126,11 @@ func restorev1alpha7MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa restorev1alpha7SecurityGroupFilter(&previous.SecurityGroups[i], &dst.SecurityGroups[i]) } } + + // Conversion to v1beta1 removes Image when ImageUUID is set + if dst.Image == "" && previous.Image != "" { + dst.Image = previous.Image + } } func restorev1beta1MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infrav1.OpenStackMachineSpec) { @@ -152,11 +157,10 @@ func Convert_v1alpha7_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *O } imageFilter := infrav1.ImageFilter{} - if in.Image != "" { - imageFilter.Name = in.Image - } if in.ImageUUID != "" { - imageFilter.ID = in.ImageUUID + imageFilter.ID = &in.ImageUUID + } else if in.Image != "" { // Only add name when ID is not set, in v1beta1 it's not possible to set both. + imageFilter.Name = &in.Image } out.Image = imageFilter @@ -197,12 +201,12 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha7_OpenStackMachineSpec(in *i out.ServerGroupID = in.ServerGroup.ID } - if in.Image.Name != "" { - out.Image = in.Image.Name + if in.Image.Name != nil && *in.Image.Name != "" { + out.Image = *in.Image.Name } - if in.Image.ID != "" { - out.ImageUUID = in.Image.ID + if in.Image.ID != nil && *in.Image.ID != "" { + out.ImageUUID = *in.Image.ID } if len(in.ServerMetadata) > 0 { diff --git a/api/v1beta1/openstackmachine_types.go b/api/v1beta1/openstackmachine_types.go index 9e36fa9dd8..743468399a 100644 --- a/api/v1beta1/openstackmachine_types.go +++ b/api/v1beta1/openstackmachine_types.go @@ -43,7 +43,8 @@ type OpenStackMachineSpec struct { // The image to use for your server instance. // If the rootVolume is specified, this will be used when creating the root volume. - Image ImageFilter `json:"image,omitempty"` + // +required + Image ImageFilter `json:"image"` // The ssh key to inject in the instance SSHKeyName string `json:"sshKeyName,omitempty"` diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 4dccc85c7a..94625b20db 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -28,12 +28,19 @@ type OpenStackMachineTemplateResource struct { Spec OpenStackMachineSpec `json:"spec"` } +// ImageFilter describes the data needed to identify which image to use. If ID is provided it is required that all other fields are unset. +// +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" type ImageFilter struct { - // The ID of the desired image. If this is provided, the other filters will be ignored. - ID string `json:"id,omitempty"` + // The ID of the desired image. If ID is provided, the other filters cannot be provided. Must be in UUID format. + // +kubebuilder:validation:Format:=uuid + // +optional + ID optional.String `json:"id,omitempty"` // 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. - Name string `json:"name,omitempty"` + // +optional + Name optional.String `json:"name,omitempty"` // 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. + // +listType=set + // +optional Tags []string `json:"tags,omitempty"` } diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index abd378418e..2fe078f3dd 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -305,6 +305,16 @@ func (in *FixedIP) DeepCopy() *FixedIP { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ImageFilter) DeepCopyInto(out *ImageFilter) { *out = *in + if in.ID != nil { + in, out := &in.ID, &out.ID + *out = new(string) + **out = **in + } + if in.Name != nil { + in, out := &in.Name, &out.Name + *out = new(string) + **out = **in + } if in.Tags != nil { in, out := &in.Tags, &out.Tags *out = make([]string, len(*in)) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index 2e31f55442..50fca991d1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5022,8 +5022,10 @@ spec: If the rootVolume is specified, this will be used when creating the root volume. properties: id: - description: The ID of the desired image. If this is provided, - the other filters will be ignored. + description: The ID of the desired image. If ID is provided, + the other filters cannot be provided. Must be in UUID + format. + format: uuid type: string name: description: The name of the desired image. If specified, @@ -5037,7 +5039,12 @@ spec: items: type: string type: array + x-kubernetes-list-type: set type: object + x-kubernetes-validations: + - message: when ID is set you cannot set other options + rule: (has(self.id) && !has(self.name) && !has(self.tags)) + || !has(self.id) instanceID: description: InstanceID is the OpenStack instance ID for this machine. @@ -5540,6 +5547,7 @@ spec: type: boolean required: - flavor + - image type: object type: object controlPlaneAvailabilityZones: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index 5dc38e4098..acb1556d63 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2447,8 +2447,10 @@ spec: If the rootVolume is specified, this will be used when creating the root volume. properties: id: - description: The ID of the desired image. If this - is provided, the other filters will be ignored. + description: The ID of the desired image. If ID + is provided, the other filters cannot be provided. + Must be in UUID format. + format: uuid type: string name: description: The name of the desired image. If @@ -2464,7 +2466,12 @@ spec: items: type: string type: array + x-kubernetes-list-type: set type: object + x-kubernetes-validations: + - message: when ID is set you cannot set other options + rule: (has(self.id) && !has(self.name) && !has(self.tags)) + || !has(self.id) instanceID: description: InstanceID is the OpenStack instance ID for this machine. @@ -2972,6 +2979,7 @@ spec: type: boolean required: - flavor + - image type: object type: object controlPlaneAvailabilityZones: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml index b47a3fe347..475ff16f1b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1810,8 +1810,9 @@ spec: If the rootVolume is specified, this will be used when creating the root volume. properties: id: - description: The ID of the desired image. If this is provided, - the other filters will be ignored. + description: The ID of the desired image. If ID is provided, the + other filters cannot be provided. Must be in UUID format. + format: uuid type: string name: description: The name of the desired image. If specified, the @@ -1825,7 +1826,11 @@ spec: items: type: string type: array + x-kubernetes-list-type: set type: object + x-kubernetes-validations: + - message: when ID is set you cannot set other options + rule: (has(self.id) && !has(self.name) && !has(self.tags)) || !has(self.id) instanceID: description: InstanceID is the OpenStack instance ID for this machine. type: string @@ -2324,6 +2329,7 @@ spec: type: boolean required: - flavor + - image type: object status: description: OpenStackMachineStatus defines the observed state of OpenStackMachine. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml index 975c907650..6c982124ae 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -1483,8 +1483,10 @@ spec: If the rootVolume is specified, this will be used when creating the root volume. properties: id: - description: The ID of the desired image. If this is provided, - the other filters will be ignored. + description: The ID of the desired image. If ID is provided, + the other filters cannot be provided. Must be in UUID + format. + format: uuid type: string name: description: The name of the desired image. If specified, @@ -1498,7 +1500,12 @@ spec: items: type: string type: array + x-kubernetes-list-type: set type: object + x-kubernetes-validations: + - message: when ID is set you cannot set other options + rule: (has(self.id) && !has(self.name) && !has(self.tags)) + || !has(self.id) instanceID: description: InstanceID is the OpenStack instance ID for this machine. @@ -2001,6 +2008,7 @@ spec: type: boolean required: - flavor + - image type: object required: - spec diff --git a/controllers/openstackmachine_controller_test.go b/controllers/openstackmachine_controller_test.go index a03120ce5e..648b1abc0d 100644 --- a/controllers/openstackmachine_controller_test.go +++ b/controllers/openstackmachine_controller_test.go @@ -83,7 +83,7 @@ func getDefaultOpenStackMachine() *infrav1.OpenStackMachine { // FloatingIP is only used by the cluster controller for the Bastion // TODO: Test Networks, Ports, Subnet, and Trunk separately Flavor: flavorName, - Image: infrav1.ImageFilter{ID: imageUUID}, + Image: infrav1.ImageFilter{ID: pointer.String(imageUUID)}, SSHKeyName: sshKeyName, Tags: []string{"test-tag"}, ServerMetadata: []infrav1.ServerMetadata{ diff --git a/docs/book/src/api/v1beta1/api.md b/docs/book/src/api/v1beta1/api.md index ce660d5e0d..5ac1613daf 100644 --- a/docs/book/src/api/v1beta1/api.md +++ b/docs/book/src/api/v1beta1/api.md @@ -1589,6 +1589,7 @@ address in any subnet of the port’s network.

OpenStackMachineSpec)

+

ImageFilter describes the data needed to identify which image to use. If ID is provided it is required that all other fields are unset.

@@ -1606,7 +1607,8 @@ string @@ -1617,6 +1619,7 @@ string @@ -1628,6 +1631,7 @@ string diff --git a/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md b/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md index 7c98118078..3776822cf7 100644 --- a/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md +++ b/docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md @@ -91,7 +91,7 @@ If empty object or null is provided, Machine will not be added to any server gro #### Change to image The field `image` is now an `ImageFilter` object rather than a string name. -The `ImageFilter` object allows selection of an image by name, by ID or by tags. +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. ```yaml image: "test-image" diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index 9981ebeabb..eebfd9278d 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -333,8 +333,8 @@ func applyServerGroupID(opts servers.CreateOptsBuilder, serverGroupID string) se // Helper function for getting image ID from name, ID, or tags. func (s *Service) GetImageID(image infrav1.ImageFilter) (string, error) { - if image.ID != "" { - return image.ID, nil + if image.ID != nil { + return *image.ID, nil } listOpts := filterconvert.ImageFilterToListOpts(&image) diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index bc5b01def5..e715171556 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -41,8 +41,8 @@ import ( ) func TestService_getImageID(t *testing.T) { - const imageID = "ce96e584-7ebc-46d6-9e55-987d72e3806c" - const imageName = "test-image" + imageID := "ce96e584-7ebc-46d6-9e55-987d72e3806c" + imageName := "test-image" imageTags := []string{"test-tag"} tests := []struct { @@ -54,14 +54,14 @@ func TestService_getImageID(t *testing.T) { }{ { testName: "Return image ID when ID given", - image: infrav1.ImageFilter{ID: imageID}, + image: infrav1.ImageFilter{ID: &imageID}, want: imageID, expect: func(m *mock.MockImageClientMockRecorder) {}, wantErr: false, }, { testName: "Return image ID when name given", - image: infrav1.ImageFilter{Name: imageName}, + image: infrav1.ImageFilter{Name: &imageName}, want: imageID, expect: func(m *mock.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: imageName}).Return( @@ -83,7 +83,7 @@ func TestService_getImageID(t *testing.T) { }, { testName: "Return no results", - image: infrav1.ImageFilter{Name: imageName}, + image: infrav1.ImageFilter{Name: &imageName}, expect: func(m *mock.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: imageName}).Return( []images.Image{}, @@ -94,7 +94,7 @@ func TestService_getImageID(t *testing.T) { }, { testName: "Return multiple results", - image: infrav1.ImageFilter{Name: imageName}, + image: infrav1.ImageFilter{Name: &imageName}, expect: func(m *mock.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( []images.Image{ @@ -107,7 +107,7 @@ func TestService_getImageID(t *testing.T) { }, { testName: "OpenStack returns error", - image: infrav1.ImageFilter{Name: imageName}, + image: infrav1.ImageFilter{Name: &imageName}, expect: func(m *mock.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( nil, diff --git a/pkg/cloud/services/compute/referenced_resources_test.go b/pkg/cloud/services/compute/referenced_resources_test.go index a62ce19e2f..1dd50788b6 100644 --- a/pkg/cloud/services/compute/referenced_resources_test.go +++ b/pkg/cloud/services/compute/referenced_resources_test.go @@ -26,6 +26,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions" . "github.com/onsi/gomega" + "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" @@ -35,7 +36,7 @@ import ( func Test_ResolveReferencedMachineResources(t *testing.T) { constFalse := false const serverGroupID1 = "ce96e584-7ebc-46d6-9e55-987d72e3806c" - const imageID1 = "de96e584-7ebc-46d6-9e55-987d72e3806c" + imageID1 := "de96e584-7ebc-46d6-9e55-987d72e3806c" minimumReferences := &infrav1.ReferencedMachineResources{ ImageID: imageID1, @@ -56,7 +57,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { { testName: "Resources ID passed", serverGroupFilter: &infrav1.ServerGroupFilter{ID: serverGroupID1}, - imageFilter: &infrav1.ImageFilter{ID: imageID1}, + imageFilter: &infrav1.ImageFilter{ID: &imageID1}, expectComputeMock: func(m *mock.MockComputeClientMockRecorder) {}, expectImageMock: func(m *mock.MockImageClientMockRecorder) {}, expectNetworkMock: func(m *mock.MockNetworkClientMockRecorder) {}, @@ -96,7 +97,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { }, { testName: "Image by Name not found", - imageFilter: &infrav1.ImageFilter{Name: "test-image"}, + imageFilter: &infrav1.ImageFilter{Name: pointer.String("test-image")}, expectComputeMock: func(m *mock.MockComputeClientMockRecorder) {}, expectImageMock: func(m *mock.MockImageClientMockRecorder) { m.ListImages(images.ListOpts{Name: "test-image"}).Return( @@ -157,7 +158,7 @@ func Test_ResolveReferencedMachineResources(t *testing.T) { tt.expectNetworkMock(mockScopeFactory.NetworkClient.EXPECT()) // Set defaults for required fields - imageFilter := &infrav1.ImageFilter{ID: imageID1} + imageFilter := &infrav1.ImageFilter{ID: pointer.String(imageID1)} if tt.imageFilter != nil { imageFilter = tt.imageFilter } diff --git a/pkg/utils/filterconvert/convert.go b/pkg/utils/filterconvert/convert.go index a4ba88e44f..a1574cc49f 100644 --- a/pkg/utils/filterconvert/convert.go +++ b/pkg/utils/filterconvert/convert.go @@ -95,13 +95,17 @@ func RouterFilterToListOpts(routerFilter *infrav1.RouterFilter) routers.ListOpts } } -func ImageFilterToListOpts(imageFilter *infrav1.ImageFilter) images.ListOpts { +func ImageFilterToListOpts(imageFilter *infrav1.ImageFilter) (listOpts images.ListOpts) { if imageFilter == nil { - return images.ListOpts{} + return } - return images.ListOpts{ - ID: imageFilter.ID, - Name: imageFilter.Name, - Tags: imageFilter.Tags, + + if imageFilter.Name != nil && *imageFilter.Name != "" { + listOpts.Name = *imageFilter.Name + } + + if len(imageFilter.Tags) > 0 { + listOpts.Tags = imageFilter.Tags } + return } diff --git a/pkg/webhooks/openstackcluster_webhook_test.go b/pkg/webhooks/openstackcluster_webhook_test.go index 1d2a4cd6ad..dcead6be4f 100644 --- a/pkg/webhooks/openstackcluster_webhook_test.go +++ b/pkg/webhooks/openstackcluster_webhook_test.go @@ -85,7 +85,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, Bastion: &infrav1.Bastion{ Instance: infrav1.OpenStackMachineSpec{ - Image: infrav1.ImageFilter{Name: "foobar"}, + Image: infrav1.ImageFilter{Name: pointer.String("foobar")}, Flavor: "minimal", }, Enabled: true, @@ -105,7 +105,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, Bastion: &infrav1.Bastion{ Instance: infrav1.OpenStackMachineSpec{ - Image: infrav1.ImageFilter{Name: "foobarbaz"}, + Image: infrav1.ImageFilter{Name: pointer.String("foobarbaz")}, Flavor: "medium", }, Enabled: true, @@ -454,7 +454,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Enabled: true, Instance: infrav1.OpenStackMachineSpec{ Flavor: "m1.small", - Image: infrav1.ImageFilter{Name: "ubuntu"}, + Image: infrav1.ImageFilter{Name: pointer.String("ubuntu")}, }, }, }, @@ -481,7 +481,7 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { Enabled: false, Instance: infrav1.OpenStackMachineSpec{ Flavor: "m1.small", - Image: infrav1.ImageFilter{Name: "ubuntu"}, + Image: infrav1.ImageFilter{Name: pointer.String("ubuntu")}, }, }, }, diff --git a/pkg/webhooks/openstackmachinetemplate_webhook_test.go b/pkg/webhooks/openstackmachinetemplate_webhook_test.go index b6d30791c6..899cf509f1 100644 --- a/pkg/webhooks/openstackmachinetemplate_webhook_test.go +++ b/pkg/webhooks/openstackmachinetemplate_webhook_test.go @@ -47,7 +47,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Template: infrav1.OpenStackMachineTemplateResource{ Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", - Image: infrav1.ImageFilter{Name: "bar"}, + Image: infrav1.ImageFilter{Name: pointer.String("bar")}, }, }, }, @@ -57,7 +57,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Template: infrav1.OpenStackMachineTemplateResource{ Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", - Image: infrav1.ImageFilter{Name: "NewImage"}, + Image: infrav1.ImageFilter{Name: pointer.String("NewImage")}, }, }, }, @@ -72,7 +72,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Template: infrav1.OpenStackMachineTemplateResource{ Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", - Image: infrav1.ImageFilter{Name: "bar"}, + Image: infrav1.ImageFilter{Name: pointer.String("bar")}, }, }, }, @@ -85,7 +85,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Template: infrav1.OpenStackMachineTemplateResource{ Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", - Image: infrav1.ImageFilter{Name: "bar"}, + Image: infrav1.ImageFilter{Name: pointer.String("bar")}, }, }, }, @@ -102,7 +102,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Template: infrav1.OpenStackMachineTemplateResource{ Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", - Image: infrav1.ImageFilter{Name: "bar"}, + Image: infrav1.ImageFilter{Name: pointer.String("bar")}, }, }, }, @@ -112,7 +112,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Template: infrav1.OpenStackMachineTemplateResource{ Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", - Image: infrav1.ImageFilter{Name: "NewImage"}, + Image: infrav1.ImageFilter{Name: pointer.String("NewImage")}, }, }, }, @@ -127,7 +127,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Template: infrav1.OpenStackMachineTemplateResource{ Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", - Image: infrav1.ImageFilter{Name: "bar"}, + Image: infrav1.ImageFilter{Name: pointer.String("bar")}, }, }, }, @@ -142,7 +142,7 @@ func TestOpenStackMachineTemplate_ValidateUpdate(t *testing.T) { Template: infrav1.OpenStackMachineTemplateResource{ Spec: infrav1.OpenStackMachineSpec{ Flavor: "foo", - Image: infrav1.ImageFilter{Name: "NewImage"}, + Image: infrav1.ImageFilter{Name: pointer.String("NewImage")}, }, }, }, diff --git a/test/e2e/suites/e2e/e2e_test.go b/test/e2e/suites/e2e/e2e_test.go index c184f96011..e57c5bdd99 100644 --- a/test/e2e/suites/e2e/e2e_test.go +++ b/test/e2e/suites/e2e/e2e_test.go @@ -861,7 +861,7 @@ func makeOpenStackMachineTemplate(namespace, clusterName, name string) *infrav1. Spec: infrav1.OpenStackMachineSpec{ Flavor: e2eCtx.E2EConfig.GetVariable(shared.OpenStackNodeMachineFlavor), Image: infrav1.ImageFilter{ - Name: e2eCtx.E2EConfig.GetVariable(shared.OpenStackImageName), + Name: pointer.String(e2eCtx.E2EConfig.GetVariable(shared.OpenStackImageName)), }, SSHKeyName: shared.DefaultSSHKeyPairName, IdentityRef: &infrav1.OpenStackIdentityReference{ @@ -885,7 +885,7 @@ func makeOpenStackMachineTemplateWithPortOptions(namespace, clusterName, name st Spec: infrav1.OpenStackMachineSpec{ Flavor: e2eCtx.E2EConfig.GetVariable(shared.OpenStackNodeMachineFlavor), Image: infrav1.ImageFilter{ - Name: e2eCtx.E2EConfig.GetVariable(shared.OpenStackImageName), + Name: pointer.String(e2eCtx.E2EConfig.GetVariable(shared.OpenStackImageName)), }, SSHKeyName: shared.DefaultSSHKeyPairName, IdentityRef: &infrav1.OpenStackIdentityReference{ From dc3959e0dd803c30361cbf6697293db6090dad66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Dulko?= Date: Tue, 12 Mar 2024 16:02:43 +0100 Subject: [PATCH 2/2] Add ImageFilter API validations This adds tests related to kubebuilder validations of ImageFilter. --- ...neutronfilters_test.go => filters_test.go} | 48 ++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) rename test/e2e/suites/apivalidations/{neutronfilters_test.go => filters_test.go} (79%) diff --git a/test/e2e/suites/apivalidations/neutronfilters_test.go b/test/e2e/suites/apivalidations/filters_test.go similarity index 79% rename from test/e2e/suites/apivalidations/neutronfilters_test.go rename to test/e2e/suites/apivalidations/filters_test.go index 59fb80e9fd..b4e256cff8 100644 --- a/test/e2e/suites/apivalidations/neutronfilters_test.go +++ b/test/e2e/suites/apivalidations/filters_test.go @@ -20,11 +20,12 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" ) -var _ = Describe("Neutron filter API validations", func() { +var _ = Describe("Filter API validations", func() { var ( namespace *corev1.Namespace cluster *infrav1.OpenStackCluster @@ -173,4 +174,49 @@ var _ = Describe("Neutron filter API validations", func() { {Tags: []infrav1.NeutronTag{"foo", ""}}, }), ) + + const imageUUID = "5a78f794-cdc3-48d2-8d9f-0fd472fdd743" + + It("should not allow both ID and Name of ImageFilter to be set", func() { + By("Creating a machine") + machine.Spec.Image = infrav1.ImageFilter{ + ID: pointer.String(imageUUID), + Name: pointer.String("bar"), + } + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "OpenStackMachine creation should fail") + }) + + It("should not allow both ID and Tags of ImageFilter to be set", func() { + By("Creating a machine") + machine.Spec.Image = infrav1.ImageFilter{ + ID: pointer.String(imageUUID), + Tags: []string{"bar", "baz"}, + } + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "OpenStackMachine creation should fail") + }) + + It("should allow UUID ID of ImageFilter to be set", func() { + By("Creating a machine") + machine.Spec.Image = infrav1.ImageFilter{ + ID: pointer.String(imageUUID), + } + Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "OpenStackMachine creation should succeed") + }) + + It("should not allow non-UUID ID of ImageFilter to be set", func() { + By("Creating a machine") + machine.Spec.Image = infrav1.ImageFilter{ + ID: pointer.String("foo"), + } + Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "OpenStackMachine creation should fail") + }) + + It("should allow Name and Tags of ImageFilter to be set", func() { + By("Creating a machine") + machine.Spec.Image = infrav1.ImageFilter{ + Name: pointer.String("bar"), + Tags: []string{"bar", "baz"}, + } + Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "OpenStackMachine creation should succeed") + }) })
-

The ID of the desired image. If this is provided, the other filters will be ignored.

+(Optional) +

The ID of the desired image. If ID is provided, the other filters cannot be provided. Must be in UUID format.
+(Optional)

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.
+(Optional)

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.