🐛 Fix panic when OpenStackCluster.Status.Network is nil in HCP scenarios

controllers/openstackcluster_controller.go

@@ -499,7 +499,10 @@ func (r *OpenStackClusterReconciler) reconcileBastionServer(ctx context.Context,
 	}
 
 	// If the bastion is found but the spec has changed, we need to delete it and reconcile.
-	bastionServerSpec := bastionToOpenStackServerSpec(openStackCluster)
+	bastionServerSpec, err := bastionToOpenStackServerSpec(openStackCluster)
+	if err != nil {
+		return nil, true, err
+	}
 	if !bastionNotFound && server != nil && !apiequality.Semantic.DeepEqual(bastionServerSpec, &server.Spec) {
 		scope.Logger().Info("Bastion spec has changed, re-creating the OpenStackServer object")
 		if err := r.deleteBastion(ctx, scope, cluster, openStackCluster); err != nil {
@@ -545,7 +548,10 @@ func (r *OpenStackClusterReconciler) getBastionServer(ctx context.Context, openS
 // createBastionServer creates the OpenStackServer object for the bastion server.
 // It returns the OpenStackServer object and an error if any.
 func (r *OpenStackClusterReconciler) createBastionServer(ctx context.Context, openStackCluster *infrav1.OpenStackCluster, cluster *clusterv1.Cluster) (*infrav1alpha1.OpenStackServer, error) {
-	bastionServerSpec := bastionToOpenStackServerSpec(openStackCluster)
+	bastionServerSpec, err := bastionToOpenStackServerSpec(openStackCluster)
+	if err != nil {
+		return nil, err
+	}
 	bastionServer := &infrav1alpha1.OpenStackServer{
 		ObjectMeta: metav1.ObjectMeta{
 			Labels: map[string]string{
@@ -573,7 +579,7 @@ func (r *OpenStackClusterReconciler) createBastionServer(ctx context.Context, op
 
 // bastionToOpenStackServerSpec converts the OpenStackMachineSpec for the bastion to an OpenStackServerSpec.
 // It returns the OpenStackServerSpec and an error if any.
-func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) *infrav1alpha1.OpenStackServerSpec {
+func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) (*infrav1alpha1.OpenStackServerSpec, error) {
 	bastion := openStackCluster.Spec.Bastion
 	if bastion == nil {
 		bastion = &infrav1.Bastion{}
@@ -588,9 +594,12 @@ func bastionToOpenStackServerSpec(openStackCluster *infrav1.OpenStackCluster) *i
 	if bastion.AvailabilityZone != nil {
 		az = *bastion.AvailabilityZone
 	}
-	openStackServerSpec := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster.Status.Network.ID)
+	openStackServerSpec, err := openStackMachineSpecToOpenStackServerSpec(bastion.Spec, openStackCluster.Spec.IdentityRef, compute.InstanceTags(bastion.Spec, openStackCluster), az, nil, getBastionSecurityGroupID(openStackCluster), openStackCluster.Status.Network)
+	if err != nil {
+		return nil, err
+	}
 
-	return openStackServerSpec
+	return openStackServerSpec, nil
 }
 
 func bastionName(clusterResourceName string) string {

controllers/openstackmachine_controller.go

@@ -480,7 +480,18 @@ func (r *OpenStackMachineReconciler) getMachineServer(ctx context.Context, openS
 
 // openStackMachineSpecToOpenStackServerSpec converts an OpenStackMachineSpec to an OpenStackServerSpec.
 // It returns the OpenStackServerSpec object and an error if there is any.
-func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, defaultNetworkID string) *infrav1alpha1.OpenStackServerSpec {
+func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.OpenStackMachineSpec, identityRef infrav1.OpenStackIdentityReference, tags []string, failureDomain string, userDataRef *corev1.LocalObjectReference, defaultSecGroup *string, clusterNetwork *infrav1.NetworkStatusWithSubnets) (*infrav1alpha1.OpenStackServerSpec, error) {
+	// Determine default network ID if the cluster status exposes one.
+	var defaultNetworkID string
+	if clusterNetwork != nil {
+		defaultNetworkID = clusterNetwork.ID
+	}
+
+	// If no cluster network is available AND the machine spec did not define any ports with a network, we cannot choose a network.
+	if defaultNetworkID == "" && len(openStackMachineSpec.Ports) == 0 {
+		return nil, capoerrors.Terminal(infrav1.InvalidMachineSpecReason, "no network configured: cluster network is missing and machine spec does not define ports with a network")
+	}
+
 	openStackServerSpec := &infrav1alpha1.OpenStackServerSpec{
 		AdditionalBlockDevices:            openStackMachineSpec.AdditionalBlockDevices,
 		ConfigDrive:                       openStackMachineSpec.ConfigDrive,
@@ -522,7 +533,8 @@ func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.Ope
 		serverPorts = make([]infrav1.PortOpts, 1)
 	}
 	for i := range serverPorts {
-		if serverPorts[i].Network == nil {
+		// Only inject the default network when we actually have an ID.
+		if serverPorts[i].Network == nil && defaultNetworkID != "" {
 			serverPorts[i].Network = &infrav1.NetworkParam{
 				ID: &defaultNetworkID,
 			}
@@ -540,7 +552,7 @@ func openStackMachineSpecToOpenStackServerSpec(openStackMachineSpec *infrav1.Ope
 	}
 	openStackServerSpec.Ports = serverPorts
 
-	return openStackServerSpec
+	return openStackServerSpec, nil
 }
 
 // reconcileMachineServer reconciles the OpenStackServer object for the OpenStackMachine.
@@ -589,7 +601,10 @@ func (r *OpenStackMachineReconciler) getOrCreateMachineServer(ctx context.Contex
 			}
 			return openStackCluster.Spec.IdentityRef
 		}()
-		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network.ID)
+		machineServerSpec, err := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network)
+		if err != nil {
+			return nil, err
+		}
 		machineServer = &infrav1alpha1.OpenStackServer{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{

controllers/openstackmachine_controller_test.go

@@ -95,9 +95,11 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 	tags := []string{"tag1", "tag2"}
 	userData := &corev1.LocalObjectReference{Name: "server-data-secret"}
 	tests := []struct {
-		name string
-		spec *infrav1.OpenStackMachineSpec
-		want *infrav1alpha1.OpenStackServerSpec
+		name           string
+		spec           *infrav1.OpenStackMachineSpec
+		clusterNetwork *infrav1.NetworkStatusWithSubnets
+		want           *infrav1alpha1.OpenStackServerSpec
+		wantErr        bool
 	}{
 		{
 			name: "Test a minimum OpenStackMachineSpec to OpenStackServerSpec conversion",
@@ -106,6 +108,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 				Image:      image,
 				SSHKeyName: sshKeyName,
 			},
+			clusterNetwork: openStackCluster.Status.Network,
 			want: &infrav1alpha1.OpenStackServerSpec{
 				Flavor:      ptr.To(flavorName),
 				IdentityRef: identityRef,
@@ -128,6 +131,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 					},
 				},
 			},
+			clusterNetwork: openStackCluster.Status.Network,
 			want: &infrav1alpha1.OpenStackServerSpec{
 				Flavor:      ptr.To(flavorName),
 				IdentityRef: identityRef,
@@ -146,6 +150,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 				Image:      image,
 				SSHKeyName: sshKeyName,
 			},
+			clusterNetwork: openStackCluster.Status.Network,
 			want: &infrav1alpha1.OpenStackServerSpec{
 				Flavor:      ptr.To(flavorName),
 				FlavorID:    ptr.To(flavorUUID),
@@ -164,6 +169,7 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 				Image:      image,
 				SSHKeyName: sshKeyName,
 			},
+			clusterNetwork: openStackCluster.Status.Network,
 			want: &infrav1alpha1.OpenStackServerSpec{
 				FlavorID:    ptr.To(flavorUUID),
 				IdentityRef: identityRef,
@@ -174,12 +180,90 @@ func TestOpenStackMachineSpecToOpenStackServerSpec(t *testing.T) {
 				UserDataRef: userData,
 			},
 		},
+		{
+			name: "Cluster network nil, machine defines port network and overrides SG",
+			spec: &infrav1.OpenStackMachineSpec{
+				Ports: []infrav1.PortOpts{{
+					Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+				}},
+				SecurityGroups: []infrav1.SecurityGroupParam{{ID: ptr.To(extraSecurityGroupUUID)}},
+			},
+			clusterNetwork: nil,
+			want: &infrav1alpha1.OpenStackServerSpec{
+				IdentityRef: identityRef,
+				Ports: []infrav1.PortOpts{{
+					Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+					SecurityGroups: []infrav1.SecurityGroupParam{
+						{ID: ptr.To(workerSecurityGroupUUID)},
+						{ID: ptr.To(extraSecurityGroupUUID)},
+					},
+				}},
+				Tags:        tags,
+				UserDataRef: userData,
+			},
+		},
+		{
+			name: "Cluster network nil, machine defines port network and falls back to cluster SG",
+			spec: &infrav1.OpenStackMachineSpec{
+				Ports: []infrav1.PortOpts{{
+					Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+				}},
+			},
+			clusterNetwork: nil,
+			want: &infrav1alpha1.OpenStackServerSpec{
+				IdentityRef: identityRef,
+				Ports: []infrav1.PortOpts{{
+					Network:        &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+					SecurityGroups: []infrav1.SecurityGroupParam{{ID: ptr.To(workerSecurityGroupUUID)}},
+				}},
+				Tags:        tags,
+				UserDataRef: userData,
+			},
+		},
+		{
+			name: "Error case: no cluster network and no machine ports",
+			spec: &infrav1.OpenStackMachineSpec{
+				Flavor:     ptr.To(flavorName),
+				Image:      image,
+				SSHKeyName: sshKeyName,
+				// No ports defined
+			},
+			clusterNetwork: nil,
+			want:           nil,
+			wantErr:        true,
+		},
+		{
+			name: "Empty cluster network ID, machine defines explicit ports",
+			spec: &infrav1.OpenStackMachineSpec{
+				Flavor: ptr.To(flavorName),
+				Image:  image,
+				Ports: []infrav1.PortOpts{{
+					Network: &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+				}},
+			},
+			clusterNetwork: &infrav1.NetworkStatusWithSubnets{NetworkStatus: infrav1.NetworkStatus{ID: ""}},
+			want: &infrav1alpha1.OpenStackServerSpec{
+				Flavor:      ptr.To(flavorName),
+				IdentityRef: identityRef,
+				Image:       image,
+				Ports: []infrav1.PortOpts{{
+					Network:        &infrav1.NetworkParam{ID: ptr.To(networkUUID)},
+					SecurityGroups: []infrav1.SecurityGroupParam{{ID: ptr.To(workerSecurityGroupUUID)}},
+				}},
+				Tags:        tags,
+				UserDataRef: userData,
+			},
+		},
 	}
 	for i := range tests {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
-			spec := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, openStackCluster.Status.Network.ID)
-			if !reflect.DeepEqual(spec, tt.want) {
+			spec, err := openStackMachineSpecToOpenStackServerSpec(tt.spec, identityRef, tags, "", userData, &openStackCluster.Status.WorkerSecurityGroup.ID, tt.clusterNetwork)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("openStackMachineSpecToOpenStackServerSpec() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !tt.wantErr && !reflect.DeepEqual(spec, tt.want) {
 				t.Errorf("openStackMachineSpecToOpenStackServerSpec() got = %+v, want %+v", spec, tt.want)
 			}
 		})

docs/book/src/SUMMARY.md

@@ -5,6 +5,7 @@
 - [Configuration](clusteropenstack/configuration.md)
 - [Topics](./topics/index.md)
     - [external cloud provider](./topics/external-cloud-provider.md)
+    - [hosted control plane](./topics/hosted-control-plane.md)
     - [move from bootstrap](./topics/mover.md)
     - [trouble shooting](./topics/troubleshooting.md)
     - [CRD Changes](./topics/crd-changes/index.md)