Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/v1alpha5/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 45 additions & 0 deletions api/v1alpha6/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
31 changes: 31 additions & 0 deletions api/v1alpha6/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
},
}
}

Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha6/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 71 additions & 2 deletions api/v1alpha7/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,20 @@ import (

var _ ctrlconversion.Convertible = &OpenStackCluster{}

var v1alpha7OpenStackClusterRestorer = conversion.RestorerFor[*OpenStackCluster]{}
func restorev1alpha7Bastion(previous **Bastion, dst **Bastion) {
if *previous != nil && *dst != nil {
restorev1alpha7MachineSpec(&(*previous).Instance, &(*dst).Instance)
}
}

var v1alpha7OpenStackClusterRestorer = conversion.RestorerFor[*OpenStackCluster]{
"bastion": conversion.HashedFieldRestorer(
func(c *OpenStackCluster) **Bastion {
return &c.Spec.Bastion
},
restorev1alpha7Bastion,
),
}

var v1alpha8OpenStackClusterRestorer = conversion.RestorerFor[*infrav1.OpenStackCluster]{
"bastion": conversion.HashedFieldRestorer(
Expand Down Expand Up @@ -67,6 +80,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) {
Expand Down Expand Up @@ -139,12 +170,23 @@ func (r *OpenStackClusterList) ConvertFrom(srcRaw ctrlconversion.Hub) error {

var _ ctrlconversion.Convertible = &OpenStackClusterTemplate{}

func restorev1alpha7ClusterTemplateSpec(previous *OpenStackClusterTemplateSpec, dst *OpenStackClusterTemplateSpec) {
restorev1alpha7Bastion(&previous.Template.Spec.Bastion, &dst.Template.Spec.Bastion)
}

func restorev1alpha8ClusterTemplateSpec(previous *infrav1.OpenStackClusterTemplateSpec, dst *infrav1.OpenStackClusterTemplateSpec) {
restorev1alpha8Bastion(&previous.Template.Spec.Bastion, &dst.Template.Spec.Bastion)
restorev1alpha8ClusterSpec(&previous.Template.Spec, &dst.Template.Spec)
}

var v1alpha7OpenStackClusterTemplateRestorer = conversion.RestorerFor[*OpenStackClusterTemplate]{}
var v1alpha7OpenStackClusterTemplateRestorer = conversion.RestorerFor[*OpenStackClusterTemplate]{
"spec": conversion.HashedFieldRestorer(
func(c *OpenStackClusterTemplate) *OpenStackClusterTemplateSpec {
return &c.Spec
},
restorev1alpha7ClusterTemplateSpec,
),
}

var v1alpha8OpenStackClusterTemplateRestorer = conversion.RestorerFor[*infrav1.OpenStackClusterTemplate]{
"spec": conversion.HashedFieldRestorer(
Expand Down Expand Up @@ -308,6 +350,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
}

Expand All @@ -332,6 +384,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
}

Expand Down
31 changes: 31 additions & 0 deletions api/v1alpha7/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,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()
}
}
},
}
}

Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha7/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 13 additions & 1 deletion api/v1alpha8/openstackmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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"`
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may have other K/V settings , maybe something like Metadata then others reuse it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lead me down an interesting rabbit hole which lead here: https://kubernetes.slack.com/archives/C05G4NJ6P6X/p1705486348670669

cc @lentzi90

I don't think so. The only other similar use I found was ValueSpecs, and the above rabbit hole lead me to believe we might want to remove that.

This struct has a 255 character limit on keys and values, which would likely not apply elsewhere.

If we did find another user for it, we could always DRY it later, as struct names don't form part of the API.

For now I think this is ok.

Copy link
Copy Markdown
Contributor

@jichenjc jichenjc Jan 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, just check in case as I think metadata KV is widely used , but I agree 255 might be a thing not widely used


// OpenStackMachineStatus defines the observed state of OpenStackMachine.
type OpenStackMachineStatus struct {
// Ready is true when the provider resource is ready.
Expand Down
21 changes: 17 additions & 4 deletions api/v1alpha8/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading