Convert ServerMetadata from a map to a list

While we're changing it, we also add validation that keys and values don't exceed 255 characters.
kubernetes-sigs · Jan 18, 2024 · 980b91b · 980b91b
1 parent f90ddda
commit 980b91b
Show file tree

Hide file tree

Showing 16 changed files with 300 additions and 26 deletions.
diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go
diff --git a/api/v1alpha6/conversion.go b/api/v1alpha6/conversion.go
@@ -44,6 +44,24 @@ func restorev1alpha6MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMa
 	// FloatingIP is removed from v1alpha7 with no replacement, so can't be
 	// losslessly converted. Restore the previously stored value on down-conversion.
 	dst.FloatingIP = previous.FloatingIP
+
+	// Conversion to v1alpha8 truncates keys and values to 255 characters
+	for k, v := range previous.ServerMetadata {
+		kd := k
+		if len(k) > 255 {
+			kd = k[:255]
+		}
+
+		vd := v
+		if len(v) > 255 {
+			vd = v[:255]
+		}
+
+		if kd != k || vd != v {
+			delete(dst.ServerMetadata, kd)
+			dst.ServerMetadata[k] = v
+		}
+	}
 }
 
 func restorev1alpha6ClusterStatus(previous *OpenStackClusterStatus, dst *OpenStackClusterStatus) {
@@ -425,6 +443,23 @@ func Convert_v1alpha6_OpenStackMachineSpec_To_v1alpha8_OpenStackMachineSpec(in *
 	}
 	out.Image = imageFilter
 
+	if len(in.ServerMetadata) > 0 {
+		serverMetadata := make([]infrav1.ServerMetadata, 0, len(in.ServerMetadata))
+		for k, v := range in.ServerMetadata {
+			// Truncate key and value to 255 characters if required, as this
+			// was not validated prior to v1alpha8
+			if len(k) > 255 {
+				k = k[:255]
+			}
+			if len(v) > 255 {
+				v = v[:255]
+			}
+
+			serverMetadata = append(serverMetadata, infrav1.ServerMetadata{Key: k, Value: v})
+		}
+		out.ServerMetadata = serverMetadata
+	}
+
 	return nil
 }
 
@@ -776,6 +811,16 @@ func Convert_v1alpha8_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in *
 		out.ImageUUID = in.Image.ID
 	}
 
+	if len(in.ServerMetadata) > 0 {
+		serverMetadata := make(map[string]string, len(in.ServerMetadata))
+		for i := range in.ServerMetadata {
+			key := in.ServerMetadata[i].Key
+			value := in.ServerMetadata[i].Value
+			serverMetadata[key] = value
+		}
+		out.ServerMetadata = serverMetadata
+	}
+
 	return nil
 }
 

diff --git a/api/v1alpha6/conversion_test.go b/api/v1alpha6/conversion_test.go
@@ -101,6 +101,37 @@ func TestFuzzyConversion(t *testing.T) {
 					spec.Subnets = append(spec.Subnets, subnet)
 				}
 			},
+
+			func(spec *OpenStackMachineSpec, c fuzz.Continue) {
+				c.FuzzNoCustom(spec)
+
+				// RandString() generates strings up to 20
+				// characters long. To exercise truncation of
+				// long server metadata keys and values we need
+				// the possibility of strings > 255 chars.
+				genLongString := func() string {
+					var ret string
+					for len(ret) < 255 {
+						ret += c.RandString()
+					}
+					return ret
+				}
+
+				// Existing server metadata keys will be short. Add a random number of long ones.
+				for c.RandBool() {
+					if spec.ServerMetadata == nil {
+						spec.ServerMetadata = map[string]string{}
+					}
+					spec.ServerMetadata[genLongString()] = c.RandString()
+				}
+
+				// Randomly make some server metadata values long.
+				for k := range spec.ServerMetadata {
+					if c.RandBool() {
+						spec.ServerMetadata[k] = genLongString()
+					}
+				}
+			},
 		}
 	}
 

diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go
diff --git a/api/v1alpha7/conversion.go b/api/v1alpha7/conversion.go
@@ -67,6 +67,24 @@ var v1alpha8OpenStackClusterRestorer = conversion.RestorerFor[*infrav1.OpenStack
 
 func restorev1alpha7MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMachineSpec) {
 	dst.FloatingIP = previous.FloatingIP
+
+	// Conversion to v1alpha8 truncates keys and values to 255 characters
+	for k, v := range previous.ServerMetadata {
+		kd := k
+		if len(k) > 255 {
+			kd = k[:255]
+		}
+
+		vd := v
+		if len(v) > 255 {
+			vd = v[:255]
+		}
+
+		if kd != k || vd != v {
+			delete(dst.ServerMetadata, kd)
+			dst.ServerMetadata[k] = v
+		}
+	}
 }
 
 func restorev1alpha8MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infrav1.OpenStackMachineSpec) {
@@ -308,6 +326,16 @@ func Convert_v1alpha8_OpenStackMachineSpec_To_v1alpha7_OpenStackMachineSpec(in *
 		out.ImageUUID = in.Image.ID
 	}
 
+	if len(in.ServerMetadata) > 0 {
+		serverMetadata := make(map[string]string, len(in.ServerMetadata))
+		for i := range in.ServerMetadata {
+			key := in.ServerMetadata[i].Key
+			value := in.ServerMetadata[i].Value
+			serverMetadata[key] = value
+		}
+		out.ServerMetadata = serverMetadata
+	}
+
 	return nil
 }
 
@@ -332,6 +360,23 @@ func Convert_v1alpha7_OpenStackMachineSpec_To_v1alpha8_OpenStackMachineSpec(in *
 	}
 	out.Image = imageFilter
 
+	if len(in.ServerMetadata) > 0 {
+		serverMetadata := make([]infrav1.ServerMetadata, 0, len(in.ServerMetadata))
+		for k, v := range in.ServerMetadata {
+			// Truncate key and value to 255 characters if required, as this
+			// was not validated prior to v1alpha8
+			if len(k) > 255 {
+				k = k[:255]
+			}
+			if len(v) > 255 {
+				v = v[:255]
+			}
+
+			serverMetadata = append(serverMetadata, infrav1.ServerMetadata{Key: k, Value: v})
+		}
+		out.ServerMetadata = serverMetadata
+	}
+
 	return nil
 }
 

diff --git a/api/v1alpha7/conversion_test.go b/api/v1alpha7/conversion_test.go
@@ -57,6 +57,37 @@ func TestFuzzyConversion(t *testing.T) {
 
 	fuzzerFuncs := func(_ runtimeserializer.CodecFactory) []interface{} {
 		return []interface{}{
+			func(spec *OpenStackMachineSpec, c fuzz.Continue) {
+				c.FuzzNoCustom(spec)
+
+				// RandString() generates strings up to 20
+				// characters long. To exercise truncation of
+				// long server metadata keys and values we need
+				// the possibility of strings > 255 chars.
+				genLongString := func() string {
+					var ret string
+					for len(ret) < 255 {
+						ret += c.RandString()
+					}
+					return ret
+				}
+
+				// Existing server metadata keys will be short. Add a random number of long ones.
+				for c.RandBool() {
+					if spec.ServerMetadata == nil {
+						spec.ServerMetadata = map[string]string{}
+					}
+					spec.ServerMetadata[genLongString()] = c.RandString()
+				}
+
+				// Randomly make some server metadata values long.
+				for k := range spec.ServerMetadata {
+					if c.RandBool() {
+						spec.ServerMetadata[k] = genLongString()
+					}
+				}
+			},
+
 			func(spec *infrav1.OpenStackClusterSpec, c fuzz.Continue) {
 				c.FuzzNoCustom(spec)
 

diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go
diff --git a/api/v1alpha8/openstackmachine_types.go b/api/v1alpha8/openstackmachine_types.go
@@ -68,7 +68,9 @@ type OpenStackMachineSpec struct {
 	Tags []string `json:"tags,omitempty"`
 
 	// Metadata mapping. Allows you to create a map of key value pairs to add to the server instance.
-	ServerMetadata map[string]string `json:"serverMetadata,omitempty"`
+	// +listType=map
+	// +listMapKey=key
+	ServerMetadata []ServerMetadata `json:"serverMetadata,omitempty"`
 
 	// Config Drive support
 	ConfigDrive *bool `json:"configDrive,omitempty"`
@@ -91,6 +93,16 @@ type OpenStackMachineSpec struct {
 	IdentityRef *OpenStackIdentityReference `json:"identityRef,omitempty"`
 }
 
+type ServerMetadata struct {
+	// Key is the server metadata key
+	// kubebuilder:validation:MaxLength:=255
+	Key string `json:"key"`
+
+	// Value is the server metadata value
+	// kubebuilder:validation:MaxLength:=255
+	Value string `json:"value"`
+}
+
 // OpenStackMachineStatus defines the observed state of OpenStackMachine.
 type OpenStackMachineStatus struct {
 	// Ready is true when the provider resource is ready.

diff --git a/api/v1alpha8/zz_generated.deepcopy.go b/api/v1alpha8/zz_generated.deepcopy.go
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml
@@ -5269,11 +5269,28 @@ spec:
                             type: string
                         type: object
                       serverMetadata:
-                        additionalProperties:
-                          type: string
                         description: Metadata mapping. Allows you to create a map
                           of key value pairs to add to the server instance.
-                        type: object
+                        items:
+                          properties:
+                            key:
+                              description: |-
+                                Key is the server metadata key
+                                kubebuilder:validation:MaxLength:=255
+                              type: string
+                            value:
+                              description: |-
+                                Value is the server metadata value
+                                kubebuilder:validation:MaxLength:=255
+                              type: string
+                          required:
+                          - key
+                          - value
+                          type: object
+                        type: array
+                        x-kubernetes-list-map-keys:
+                        - key
+                        x-kubernetes-list-type: map
                       sshKeyName:
                         description: The ssh key to inject in the instance
                         type: string

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml
@@ -2703,11 +2703,28 @@ spec:
                                     type: string
                                 type: object
                               serverMetadata:
-                                additionalProperties:
-                                  type: string
                                 description: Metadata mapping. Allows you to create
                                   a map of key value pairs to add to the server instance.
-                                type: object
+                                items:
+                                  properties:
+                                    key:
+                                      description: |-
+                                        Key is the server metadata key
+                                        kubebuilder:validation:MaxLength:=255
+                                      type: string
+                                    value:
+                                      description: |-
+                                        Value is the server metadata value
+                                        kubebuilder:validation:MaxLength:=255
+                                      type: string
+                                  required:
+                                  - key
+                                  - value
+                                  type: object
+                                type: array
+                                x-kubernetes-list-map-keys:
+                                - key
+                                x-kubernetes-list-type: map
                               sshKeyName:
                                 description: The ssh key to inject in the instance
                                 type: string