VolumeType: implement API, controller and tests

Author: mandre

api/v1alpha1/volumetype_types.go

@@ -29,13 +29,13 @@ type VolumeTypeResourceSpec struct {
 	// +optional
 	Description *string `json:"description,omitempty"`
 
-	// TODO(scaffolding): Add more types.
-	// To see what is supported, you can take inspiration from the CreateOpts stucture from
-	// github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumetypes
-	//
-	// Until you have implemented mutability for the field, you must add a CEL validation
-	// preventing the field being modified:
-	// `// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="<fieldname> is immutable"`
+	// extraSpecs is a map of key-value pairs that define extra specifications for the volume type.
+	// +optional
+	ExtraSpecs map[string]string `json:"extraSpecs,omitempty"`
+
+	// isPublic indicates whether the volume type is public.
+	// +optional
+	IsPublic *bool `json:"isPublic,omitempty"`
 }
 
 // VolumeTypeFilter defines an existing resource by its properties
@@ -51,9 +51,9 @@ type VolumeTypeFilter struct {
 	// +optional
 	Description *string `json:"description,omitempty"`
 
-	// TODO(scaffolding): Add more types.
-	// To see what is supported, you can take inspiration from the ListOpts stucture from
-	// github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumetypes
+	// isPublic indicates whether the VolumeType is public.
+	// +optional
+	IsPublic *bool `json:"isPublic,omitempty"`
 }
 
 // VolumeTypeResourceStatus represents the observed state of the resource.
@@ -68,7 +68,11 @@ type VolumeTypeResourceStatus struct {
 	// +optional
 	Description string `json:"description,omitempty"`
 
-	// TODO(scaffolding): Add more types.
-	// To see what is supported, you can take inspiration from the VolumeType stucture from
-	// github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumetypes
+	// extraSpecs is a map of key-value pairs that define extra specifications for the volume type.
+	// +optional
+	ExtraSpecs map[string]string `json:"extraSpecs"`
+
+	// isPublic indicates whether the VolumeType is public.
+	// +optional
+	IsPublic *bool `json:"isPublic"`
 }

api/v1alpha1/zz_generated.deepcopy.go

@@ -96,6 +96,10 @@ spec:
                         maxLength: 255
                         minLength: 1
                         type: string
+                      isPublic:
+                        description: isPublic indicates whether the VolumeType is
+                          public.
+                        type: boolean
                       name:
                         description: name of the existing resource
                         maxLength: 255
@@ -155,6 +159,15 @@ spec:
                     maxLength: 255
                     minLength: 1
                     type: string
+                  extraSpecs:
+                    additionalProperties:
+                      type: string
+                    description: extraSpecs is a map of key-value pairs that define
+                      extra specifications for the volume type.
+                    type: object
+                  isPublic:
+                    description: isPublic indicates whether the volume type is public.
+                    type: boolean
                   name:
                     description: |-
                       name will be the name of the created resource. If not specified, the
@@ -271,6 +284,15 @@ spec:
                       resource.
                     maxLength: 1024
                     type: string
+                  extraSpecs:
+                    additionalProperties:
+                      type: string
+                    description: extraSpecs is a map of key-value pairs that define
+                      extra specifications for the volume type.
+                    type: object
+                  isPublic:
+                    description: isPublic indicates whether the VolumeType is public.
+                    type: boolean
                   name:
                     description: name is a Human-readable name for the resource. Might
                       not be unique.

cmd/models-schema/zz_generated.openapi.go

@@ -5,10 +5,12 @@ metadata:
   name: volumetype-sample
 spec:
   cloudCredentialsRef:
-    # TODO(scaffolding): Use openstack-admin if the resouce needs admin credentials to be created
-    cloudName: openstack
+    cloudName: openstack-admin
     secretName: openstack-clouds
   managementPolicy: managed
   resource:
     description: Sample VolumeType
-    # TODO(scaffolding): Add all fields the resource supports
+    isPublic: false
+    extraSpecs:
+      spec1: "foo"
+      spec2: "bar"

config/crd/bases/openstack.k-orc.cloud_volumetypes.yaml

@@ -70,30 +70,72 @@ func (actuator volumetypeActuator) ListOSResourcesForAdoption(ctx context.Contex
 		return nil, false
 	}
 
-	// TODO(scaffolding) If you need to filter resources on fields that the List() function
-	// of gophercloud does not support, it's possible to perform client-side filtering.
-	// Check osclients.ResourceFilter
+	var filters []osclients.ResourceFilter[osResourceT]
+
+	// NOTE: The API doesn't allow filtering by name or description, we'll have to do it client-side.
+	filters = append(filters,
+		func(f *volumetypes.VolumeType) bool {
+			name := getResourceName(orcObject)
+			// Compare non-pointer values
+			return f.Name == name
+		},
+	)
+	if resourceSpec.Description != nil {
+		filters = append(filters, func(f *volumetypes.VolumeType) bool {
+			return f.Description == *resourceSpec.Description
+		})
+	}
+
+	isPublic := volumetypes.VisibilityDefault
+	if resourceSpec.IsPublic != nil {
+		if *resourceSpec.IsPublic {
+			isPublic = volumetypes.VisibilityPublic
+		} else {
+			isPublic = volumetypes.VisibilityPrivate
+		}
+	}
 
 	listOpts := volumetypes.ListOpts{
-		Name:        getResourceName(orcObject),
-		Description: ptr.Deref(resourceSpec.Description, ""),
+		IsPublic: isPublic,
 	}
 
-	return actuator.osClient.ListVolumeTypes(ctx, listOpts), true
+	return actuator.listOSResources(ctx, filters, listOpts), true
 }
 
 func (actuator volumetypeActuator) ListOSResourcesForImport(ctx context.Context, obj orcObjectPT, filter filterT) (iter.Seq2[*osResourceT, error], progress.ReconcileStatus) {
-	// TODO(scaffolding) If you need to filter resources on fields that the List() function
-	// of gophercloud does not support, it's possible to perform client-side filtering.
-	// Check osclients.ResourceFilter
+	var filters []osclients.ResourceFilter[osResourceT]
+
+	// NOTE: The API doesn't allow filtering by name or description, we'll have to do it client-side.
+	if filter.Name != nil {
+		filters = append(filters, func(f *volumetypes.VolumeType) bool {
+			return f.Name == string(*filter.Name)
+		})
+	}
+	if filter.Description != nil {
+		filters = append(filters, func(f *volumetypes.VolumeType) bool {
+			return f.Description == *filter.Description
+		})
+	}
+
+	isPublic := volumetypes.VisibilityDefault
+	if filter.IsPublic != nil {
+		if *filter.IsPublic {
+			isPublic = volumetypes.VisibilityPublic
+		} else {
+			isPublic = volumetypes.VisibilityPrivate
+		}
+	}
 
 	listOpts := volumetypes.ListOpts{
-		Name:        string(ptr.Deref(filter.Name, "")),
-		Description: string(ptr.Deref(filter.Description, "")),
-		// TODO(scaffolding): Add more import filters
+		IsPublic: isPublic,
 	}
 
-	return actuator.osClient.ListVolumeTypes(ctx, listOpts), nil
+	return actuator.listOSResources(ctx, filters, listOpts), nil
+}
+
+func (actuator volumetypeActuator) listOSResources(ctx context.Context, filters []osclients.ResourceFilter[osResourceT], listOpts volumetypes.ListOptsBuilder) iter.Seq2[*volumetypes.VolumeType, error] {
+	volumetypes := actuator.osClient.ListVolumeTypes(ctx, listOpts)
+	return osclients.Filter(volumetypes, filters...)
 }
 
 func (actuator volumetypeActuator) CreateResource(ctx context.Context, obj orcObjectPT) (*osResourceT, progress.ReconcileStatus) {
@@ -107,7 +149,8 @@ func (actuator volumetypeActuator) CreateResource(ctx context.Context, obj orcOb
 	createOpts := volumetypes.CreateOpts{
 		Name:        getResourceName(obj),
 		Description: ptr.Deref(resource.Description, ""),
-		// TODO(scaffolding): Add more fields
+		IsPublic:    resource.IsPublic,
+		ExtraSpecs:  resource.ExtraSpecs,
 	}
 
 	osResource, err := actuator.osClient.CreateVolumeType(ctx, createOpts)
@@ -139,6 +182,7 @@ func (actuator volumetypeActuator) updateResource(ctx context.Context, obj orcOb
 
 	handleNameUpdate(&updateOpts, obj, osResource)
 	handleDescriptionUpdate(&updateOpts, resource, osResource)
+	handleIsPublicUpdate(&updateOpts, resource, osResource)
 
 	needsUpdate, err := needsUpdate(updateOpts)
 	if err != nil {
@@ -192,6 +236,14 @@ func handleDescriptionUpdate(updateOpts *volumetypes.UpdateOpts, resource *resou
 	}
 }
 
+func handleIsPublicUpdate(updateOpts *volumetypes.UpdateOpts, resource *resourceSpecT, osResource *osResourceT) {
+	// Default is true
+	isPublic := ptr.Deref(resource.IsPublic, true)
+	if osResource.IsPublic != isPublic {
+		updateOpts.IsPublic = &isPublic
+	}
+}
+
 func (actuator volumetypeActuator) GetResourceReconcilers(ctx context.Context, orcObject orcObjectPT, osResource *osResourceT, controller interfaces.ResourceController) ([]resourceReconciler, progress.ReconcileStatus) {
 	return []resourceReconciler{
 		actuator.updateResource,

config/samples/openstack_v1alpha1_volumetype.yaml

@@ -117,3 +117,30 @@ func TestHandleDescriptionUpdate(t *testing.T) {
 
 	}
 }
+
+func TestHandleIsPublicUpdate(t *testing.T) {
+	ptrToBool := ptr.To[bool]
+	testCases := []struct {
+		name          string
+		newValue      *bool
+		existingValue bool
+		expectChange  bool
+	}{
+		{name: "Identical", newValue: ptrToBool(true), existingValue: true, expectChange: false},
+		{name: "Different", newValue: ptrToBool(true), existingValue: false, expectChange: true},
+		{name: "No value provided, existing is set", newValue: nil, existingValue: false, expectChange: true},
+		{name: "No value provided, existing is default", newValue: nil, existingValue: true, expectChange: false},
+	}
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			resource := &orcv1alpha1.VolumeTypeResourceSpec{IsPublic: tt.newValue}
+			osResource := &volumetypes.VolumeType{IsPublic: tt.existingValue}
+			updateOpts := volumetypes.UpdateOpts{}
+			handleIsPublicUpdate(&updateOpts, resource, osResource)
+			got, _ := needsUpdate(updateOpts)
+			if got != tt.expectChange {
+				t.Errorf("Expected change: %v, got: %v", tt.expectChange, got)
+			}
+		})
+	}
+}