RegisteredLimit Actuator mutability

On-behalf-of: SAP nils.gondermann@sap.com

Author: gndrmnn

api/v1alpha1/registeredlimit_types.go

@@ -26,21 +26,18 @@ type RegisteredLimitResourceSpec struct {
 
 	// serviceRef is a reference to the ORC Service which this resource is associated with.
 	// +required
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="serviceRef is immutable"
 	ServiceRef KubernetesNameRef `json:"serviceRef,omitempty"`
 
 	// resourceName is name of the resource to be limited.
 	// +kubebuilder:validation:MinLength:=1
 	// +kubebuilder:validation:MaxLength:=255
 	// +required
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="resourceName is immutable"
 	ResourceName string `json:"resourceName,omitempty"`
 
 	// defaultLimit is limit of the specified resource in the given context.
 	// +kubebuilder:validation:Minimum=-1
 	// +kubebuilder:validation:Maximum=2147483647
 	// +required
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="defaultLimit is immutable"
 	DefaultLimit *int32 `json:"defaultLimit,omitempty"`
 }
 

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

@@ -162,9 +162,6 @@ spec:
                     maximum: 2147483647
                     minimum: -1
                     type: integer
-                    x-kubernetes-validations:
-                    - message: defaultLimit is immutable
-                      rule: self == oldSelf
                   description:
                     description: description is a human-readable description for the
                       resource.
@@ -176,18 +173,12 @@ spec:
                     maxLength: 255
                     minLength: 1
                     type: string
-                    x-kubernetes-validations:
-                    - message: resourceName is immutable
-                      rule: self == oldSelf
                   serviceRef:
                     description: serviceRef is a reference to the ORC Service which
                       this resource is associated with.
                     maxLength: 253
                     minLength: 1
                     type: string
-                    x-kubernetes-validations:
-                    - message: serviceRef is immutable
-                      rule: self == oldSelf
                 required:
                 - defaultLimit
                 - resourceName

internal/controllers/registeredlimit/actuator.go

@@ -190,8 +190,9 @@ func (actuator registeredlimitActuator) updateResource(ctx context.Context, obj
 	updateOpts := registeredlimits.UpdateOpts{}
 
 	handleDescriptionUpdate(&updateOpts, resource, osResource)
-
-	// TODO(scaffolding): add handler for all fields supporting mutability
+	handleResourceNameUpdate(&updateOpts, resource, osResource)
+	handleDefaultLimitUpdate(&updateOpts, resource, osResource)
+	handleServiceUpdate(&updateOpts, resource, osResource)
 
 	needsUpdate, err := needsUpdate(updateOpts)
 	if err != nil {
@@ -236,6 +237,27 @@ func handleDescriptionUpdate(updateOpts *registeredlimits.UpdateOpts, resource *
 	}
 }
 
+func handleResourceNameUpdate(updateOpts *registeredlimits.UpdateOpts, resource *resourceSpecT, osResource *osResourceT) {
+	resourceName := resource.ResourceName
+	if osResource.ResourceName != resourceName {
+		updateOpts.ResourceName = resourceName
+	}
+}
+
+func handleDefaultLimitUpdate(updateOpts *registeredlimits.UpdateOpts, resource *resourceSpecT, osResource *osResourceT) {
+	defaultLimit := int(resource.DefaultLimit)
+	if osResource.DefaultLimit != defaultLimit {
+		updateOpts.DefaultLimit = &defaultLimit
+	}
+}
+
+func handleServiceUpdate(updateOpts *registeredlimits.UpdateOpts, resource *resourceSpecT, osResource *osResourceT) {
+	serviceRef := resource.ServiceRef
+	if osResource.ServiceID != string(serviceRef) {
+		updateOpts.ServiceID = string(serviceRef)
+	}
+}
+
 func (actuator registeredlimitActuator) GetResourceReconcilers(ctx context.Context, orcObject orcObjectPT, osResource *osResourceT, controller interfaces.ResourceController) ([]resourceReconciler, progress.ReconcileStatus) {
 	return []resourceReconciler{
 		actuator.updateResource,

internal/controllers/registeredlimit/actuator_test.go

@@ -52,6 +52,60 @@ func TestNeedsUpdate(t *testing.T) {
 	}
 }
 
+func TestHandleResourceNameUpdate(t *testing.T) {
+	testCases := []struct {
+		name          string
+		newValue      *string
+		existingValue string
+		expectChange  bool
+	}{
+		{name: "Identical", newValue: ptr.To("same-resourceName"), existingValue: "same-resourceName", expectChange: false},
+		{name: "Different", newValue: ptr.To("new-resourceName"), existingValue: "same-resourceName", expectChange: true},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			resource := &orcv1alpha1.RegisteredLimitResourceSpec{ResourceName: ptr.Deref(tt.newValue, "")}
+			osResource := &osResourceT{ResourceName: tt.existingValue}
+
+			updateOpts := registeredlimits.UpdateOpts{}
+			handleResourceNameUpdate(&updateOpts, resource, osResource)
+
+			got, _ := needsUpdate(updateOpts)
+			if got != tt.expectChange {
+				t.Errorf("Expected change: %v, got: %v", tt.expectChange, got)
+			}
+		})
+	}
+}
+
+func TestHandleDefaultLimitUpdate(t *testing.T) {
+	testCases := []struct {
+		name          string
+		newValue      int
+		existingValue int
+		expectChange  bool
+	}{
+		{name: "Identical", newValue: 1, existingValue: 1, expectChange: false},
+		{name: "Different", newValue: 2, existingValue: 1, expectChange: true},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			resource := &orcv1alpha1.RegisteredLimitResourceSpec{DefaultLimit: int32(tt.newValue)}
+			osResource := &osResourceT{DefaultLimit: tt.existingValue}
+
+			updateOpts := registeredlimits.UpdateOpts{}
+			handleDefaultLimitUpdate(&updateOpts, resource, osResource)
+
+			got, _ := needsUpdate(updateOpts)
+			if got != tt.expectChange {
+				t.Errorf("Expected change: %v, got: %v", tt.expectChange, got)
+			}
+		})
+	}
+}
+
 func TestHandleDescriptionUpdate(t *testing.T) {
 	ptrToDescription := ptr.To[string]
 	testCases := []struct {