Skip to content

Commit efb0bf7

Browse files
abaysclaudecursoragent
committed
Prevent port 0 from being persisted or causing phantom conflicts
When AssignProvisionServerPort fails (port range exhausted), port remains at the Go zero value (0). Previously this could be persisted because omitempty caused the field to be absent from JSON, bypassing the CRD Minimum=6190 schema validation. Subsequent creates would then hit a confusing "port 0 is already in use" error from validateCr. Fix by: - Removing omitempty from the Port JSON tag so schema validation rejects port 0 after a failed defaulter - Skipping port==0 entries in GetExistingProvServerPorts so unassigned resources don't pollute the port map - Adding an early return in validateCr with a clear error message when port is 0 - Removing the out-of-range port from the sample CR Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 4ecec66 commit efb0bf7

5 files changed

Lines changed: 66 additions & 3 deletions

api/v1beta1/openstackprovisionserver.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ func GetExistingProvServerPorts(
2525
}
2626

2727
for _, provServer := range provServerList.Items {
28+
if provServer.Spec.Port == 0 {
29+
continue
30+
}
2831
namespacedName := types.NamespacedName{
2932
Namespace: provServer.Namespace,
3033
Name: provServer.Name}

api/v1beta1/openstackprovisionserver_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type OpenStackProvisionServerSpec struct {
5757
// +kubebuilder:validation:Optional
5858
// +kubebuilder:validation:Minimum=6190
5959
// +kubebuilder:validation:Maximum=6220
60-
Port int32 `json:"port,omitempty"`
60+
Port int32 `json:"port"`
6161
// +kubebuilder:validation:Optional
6262
// Interface - An optional interface to use instead of the cluster's default provisioning interface (if any)
6363
Interface string `json:"interface,omitempty"`

api/v1beta1/openstackprovisionserver_webhook.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ func (r *OpenStackProvisionServer) ValidateUpdate(old runtime.Object) (admission
7272
}
7373

7474
func (r *OpenStackProvisionServer) validateCr() error {
75-
// TODO: Create a specific context here instead of passing TODO()?
75+
if r.Spec.Port == 0 {
76+
return fmt.Errorf("no available ports in range %d-%d", ProvisionServerPortStart, ProvisionServerPortEnd)
77+
}
78+
7679
existingPorts, err := GetExistingProvServerPorts(context.TODO(), webhookClient, r)
7780
if err != nil {
7881
return err

config/samples/baremetal_v1beta1_openstackprovisionserver.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,4 @@ metadata:
44
name: openstackprovisionserver
55
spec:
66
interface: enp1s0
7-
port: 8080
87
osImage: edpm-hardened-uefi.qcow2

tests/functional/openstackprovisionserver_webhook_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ package functional
1717

1818
import (
1919
"errors"
20+
"fmt"
2021

2122
. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
2223
. "github.com/onsi/gomega" //revive:disable:dot-imports
24+
baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1"
2325
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
2426
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2527
"k8s.io/apimachinery/pkg/types"
@@ -127,6 +129,23 @@ var _ = Describe("OpenStackProvisionServer Webhook", func() {
127129
})
128130
})
129131

132+
When("Creating ProvisionServer with port 0 (auto-assign)", func() {
133+
It("should auto-assign a valid port via the defaulting webhook", func() {
134+
spec := map[string]interface{}{
135+
"osImage": "edpm-hardened-uefi.qcow2",
136+
"osContainerImageUrl": "quay.io/podified-antelope-centos9/edpm-hardened-uefi:current-podified",
137+
"apacheImageUrl": "registry.redhat.io/ubi9/httpd-24:latest",
138+
"agentImageUrl": "quay.io/openstack-k8s-operators/openstack-baremetal-operator-agent:latest",
139+
"port": int64(0),
140+
}
141+
DeferCleanup(th.DeleteInstance, CreateProvisionServer(provisionServerName, spec))
142+
143+
instance := GetProvisionServerDirect(provisionServerName)
144+
Expect(instance.Spec.Port).Should(BeNumerically(">=", baremetalv1.ProvisionServerPortStart))
145+
Expect(instance.Spec.Port).Should(BeNumerically("<=", baremetalv1.ProvisionServerPortEnd))
146+
})
147+
})
148+
130149
When("Creating ProvisionServer with duplicate port", func() {
131150
var provisionServer2Name types.NamespacedName
132151

@@ -204,4 +223,43 @@ var _ = Describe("OpenStackProvisionServer Webhook", func() {
204223
Expect(instance.Spec.AgentImageURL).ShouldNot(BeEmpty())
205224
})
206225
})
226+
227+
When("Port range is exhausted", func() {
228+
BeforeEach(func() {
229+
spec := map[string]interface{}{
230+
"osImage": "edpm-hardened-uefi.qcow2",
231+
"osContainerImageUrl": "quay.io/podified-antelope-centos9/edpm-hardened-uefi:current-podified",
232+
"apacheImageUrl": "registry.redhat.io/ubi9/httpd-24:latest",
233+
"agentImageUrl": "quay.io/openstack-k8s-operators/openstack-baremetal-operator-agent:latest",
234+
}
235+
portCount := baremetalv1.ProvisionServerPortEnd - baremetalv1.ProvisionServerPortStart + 1
236+
for i := 0; i < int(portCount); i++ {
237+
name := types.NamespacedName{
238+
Name: fmt.Sprintf("provserver-exhaust-%d", i),
239+
Namespace: namespace,
240+
}
241+
DeferCleanup(th.DeleteInstance, CreateProvisionServer(name, spec))
242+
}
243+
})
244+
245+
It("should reject creation when no ports are available", func() {
246+
raw := map[string]interface{}{
247+
"apiVersion": "baremetal.openstack.org/v1beta1",
248+
"kind": "OpenStackProvisionServer",
249+
"metadata": map[string]interface{}{
250+
"name": "provserver-over-limit",
251+
"namespace": namespace,
252+
},
253+
"spec": map[string]interface{}{
254+
"osImage": "edpm-hardened-uefi.qcow2",
255+
"osContainerImageUrl": "quay.io/podified-antelope-centos9/edpm-hardened-uefi:current-podified",
256+
"apacheImageUrl": "registry.redhat.io/ubi9/httpd-24:latest",
257+
"agentImageUrl": "quay.io/openstack-k8s-operators/openstack-baremetal-operator-agent:latest",
258+
},
259+
}
260+
unstructuredObj := &unstructured.Unstructured{Object: raw}
261+
err := k8sClient.Create(ctx, unstructuredObj)
262+
Expect(err).Should(HaveOccurred())
263+
})
264+
})
207265
})

0 commit comments

Comments
 (0)