Skip to content

Commit 564b6bd

Browse files
mdboothmdbooth
committed
Move CloudName into IdentityRef and make cluster IdentityRef required
This change came from attempting to write validation markers for CloudName and IdentityRef in both the machine and cluster specs. Firstly I noticed that IdentityRef was marked optional in the cluster spec, but it is certainly required: the cluster cannot be provisioned without cloud credentials. I made IdentityRef required in the cluster spec. In contrast, IdentityRef is genuinely optional in the machine spec because, if not specified, we will use the credentials defined in the cluster spec. CloudName on the machine spec is also marked optional. However, it is required if IdentityRef was specified. This is because it refers to the same object as IdentityRef. The most sensible way to to represent this in the API is to put it in the IdentityRef. This means that if IdentityRef is provided, it must be provided completely, including CloudName.
1 parent 9e00969 commit 564b6bd

29 files changed

+458
-401
lines changed

‎api/v1alpha5/conversion.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,9 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in *i
215215
out.AllowAllInClusterTraffic = in.ManagedSecurityGroups.AllowAllInClusterTraffic
216216
}
217217

218+
out.CloudName = in.IdentityRef.CloudName
219+
out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name}
220+
218221
return nil
219222
}
220223

@@ -258,6 +261,11 @@ func Convert_v1alpha5_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O
258261
}
259262
}
260263

264+
out.IdentityRef.CloudName = in.CloudName
265+
if in.IdentityRef != nil {
266+
out.IdentityRef.Name = in.IdentityRef.Name
267+
}
268+
261269
return nil
262270
}
263271

@@ -321,6 +329,16 @@ func Convert_v1alpha5_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *O
321329
}
322330
out.Image = imageFilter
323331

332+
if in.IdentityRef != nil {
333+
out.IdentityRef = &infrav1.OpenStackIdentityReference{Name: in.IdentityRef.Name}
334+
}
335+
if in.CloudName != "" {
336+
if out.IdentityRef == nil {
337+
out.IdentityRef = &infrav1.OpenStackIdentityReference{}
338+
}
339+
out.IdentityRef.CloudName = in.CloudName
340+
}
341+
324342
return nil
325343
}
326344

@@ -606,6 +624,11 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec(in *i
606624
out.ImageUUID = in.Image.ID
607625
}
608626

627+
if in.IdentityRef != nil {
628+
out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name}
629+
out.CloudName = in.IdentityRef.CloudName
630+
}
631+
609632
return nil
610633
}
611634

@@ -690,3 +713,8 @@ func Convert_v1alpha5_SecurityGroup_To_v1beta1_SecurityGroupStatus(in *SecurityG
690713
func Convert_v1alpha5_OpenStackIdentityReference_To_v1beta1_OpenStackIdentityReference(in *OpenStackIdentityReference, out *infrav1.OpenStackIdentityReference, s conversion.Scope) error {
691714
return autoConvert_v1alpha5_OpenStackIdentityReference_To_v1beta1_OpenStackIdentityReference(in, out, s)
692715
}
716+
717+
func Convert_v1beta1_OpenStackIdentityReference_To_v1alpha5_OpenStackIdentityReference(in *infrav1.OpenStackIdentityReference, out *OpenStackIdentityReference, _ conversion.Scope) error {
718+
out.Name = in.Name
719+
return nil
720+
}

‎api/v1alpha5/conversion_test.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,12 @@ func TestConvertFrom(t *testing.T) {
4646
Spec: infrav1.OpenStackClusterSpec{},
4747
},
4848
want: &OpenStackCluster{
49-
Spec: OpenStackClusterSpec{},
49+
Spec: OpenStackClusterSpec{
50+
IdentityRef: &OpenStackIdentityReference{},
51+
},
5052
ObjectMeta: metav1.ObjectMeta{
5153
Annotations: map[string]string{
52-
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"apiServerLoadBalancer\":{},\"cloudName\":\"\",\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"managedSecurityGroups\":null,\"network\":{}},\"status\":{\"ready\":false}}",
54+
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"apiServerLoadBalancer\":{},\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"identityRef\":{\"cloudName\":\"\",\"name\":\"\"},\"managedSecurityGroups\":null,\"network\":{}},\"status\":{\"ready\":false}}",
5355
},
5456
},
5557
},
@@ -61,10 +63,16 @@ func TestConvertFrom(t *testing.T) {
6163
Spec: infrav1.OpenStackClusterTemplateSpec{},
6264
},
6365
want: &OpenStackClusterTemplate{
64-
Spec: OpenStackClusterTemplateSpec{},
66+
Spec: OpenStackClusterTemplateSpec{
67+
Template: OpenStackClusterTemplateResource{
68+
Spec: OpenStackClusterSpec{
69+
IdentityRef: &OpenStackIdentityReference{},
70+
},
71+
},
72+
},
6573
ObjectMeta: metav1.ObjectMeta{
6674
Annotations: map[string]string{
67-
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"apiServerLoadBalancer\":{},\"cloudName\":\"\",\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"managedSecurityGroups\":null,\"network\":{}}}}}",
75+
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"apiServerLoadBalancer\":{},\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"identityRef\":{\"cloudName\":\"\",\"name\":\"\"},\"managedSecurityGroups\":null,\"network\":{}}}}}",
6876
},
6977
},
7078
},
@@ -79,7 +87,7 @@ func TestConvertFrom(t *testing.T) {
7987
Spec: OpenStackMachineSpec{},
8088
ObjectMeta: metav1.ObjectMeta{
8189
Annotations: map[string]string{
82-
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"cloudName\":\"\",\"flavor\":\"\",\"image\":{}},\"status\":{\"dependentResources\":{},\"ready\":false,\"referencedResources\":{}}}",
90+
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"flavor\":\"\",\"image\":{}},\"status\":{\"dependentResources\":{},\"ready\":false,\"referencedResources\":{}}}",
8391
},
8492
},
8593
},
@@ -94,7 +102,7 @@ func TestConvertFrom(t *testing.T) {
94102
Spec: OpenStackMachineTemplateSpec{},
95103
ObjectMeta: metav1.ObjectMeta{
96104
Annotations: map[string]string{
97-
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"cloudName\":\"\",\"flavor\":\"\",\"image\":{}}}}}",
105+
"cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"flavor\":\"\",\"image\":{}}}}}",
98106
},
99107
},
100108
},

‎api/v1alpha5/zz_generated.conversion.go‎

Lines changed: 10 additions & 32 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1alpha6/conversion.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,16 @@ func Convert_v1alpha6_OpenStackMachineSpec_To_v1beta1_OpenStackMachineSpec(in *O
529529
out.ServerMetadata = serverMetadata
530530
}
531531

532+
if in.IdentityRef != nil {
533+
out.IdentityRef = &infrav1.OpenStackIdentityReference{Name: in.IdentityRef.Name}
534+
}
535+
if in.CloudName != "" {
536+
if out.IdentityRef == nil {
537+
out.IdentityRef = &infrav1.OpenStackIdentityReference{}
538+
}
539+
out.IdentityRef.CloudName = in.CloudName
540+
}
541+
532542
return nil
533543
}
534544

@@ -624,6 +634,9 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in *i
624634
out.AllowAllInClusterTraffic = in.ManagedSecurityGroups.AllowAllInClusterTraffic
625635
}
626636

637+
out.CloudName = in.IdentityRef.CloudName
638+
out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name}
639+
627640
return nil
628641
}
629642

@@ -667,6 +680,11 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1beta1_OpenStackClusterSpec(in *O
667680
}
668681
}
669682

683+
out.IdentityRef.CloudName = in.CloudName
684+
if in.IdentityRef != nil {
685+
out.IdentityRef.Name = in.IdentityRef.Name
686+
}
687+
670688
return nil
671689
}
672690

@@ -973,6 +991,11 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in *i
973991
out.ServerMetadata = serverMetadata
974992
}
975993

994+
if in.IdentityRef != nil {
995+
out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name}
996+
out.CloudName = in.IdentityRef.CloudName
997+
}
998+
976999
return nil
9771000
}
9781001

@@ -1069,3 +1092,8 @@ func Convert_v1alpha6_SecurityGroup_To_v1beta1_SecurityGroupStatus(in *SecurityG
10691092
func Convert_v1alpha6_OpenStackIdentityReference_To_v1beta1_OpenStackIdentityReference(in *OpenStackIdentityReference, out *infrav1.OpenStackIdentityReference, s apiconversion.Scope) error {
10701093
return autoConvert_v1alpha6_OpenStackIdentityReference_To_v1beta1_OpenStackIdentityReference(in, out, s)
10711094
}
1095+
1096+
func Convert_v1beta1_OpenStackIdentityReference_To_v1alpha6_OpenStackIdentityReference(in *infrav1.OpenStackIdentityReference, out *OpenStackIdentityReference, _ apiconversion.Scope) error {
1097+
out.Name = in.Name
1098+
return nil
1099+
}

0 commit comments

Comments
 (0)