Skip to content

Commit 458b338

Browse files
lmicciniclaude
andcommitted
InstanceHA production readiness improvements
Webhook validation: - Add 11 webhook test cases covering Default(), ValidateCreate(), and ValidateUpdate() (cipher suite rejection, duplicate cloud detection, default values) - Fix webhook test suite: correct relative paths and scheme registration so envtest can find CRDs and decode InstanceHa objects - Add openStackCloud immutability check in ValidateUpdate to prevent changing the target cloud after creation Container hardening: - Add ReadOnlyRootFilesystem to the container SecurityContext - Add /tmp emptyDir volume so the Python process has a writable temp dir - Add unit test verifying the security context and /tmp volume Config template: - Add HEARTBEAT_CLIFF_THRESHOLD and K8S_API_CHECK_INTERVAL to the config template so operators can discover and tune these parameters Controller error-path tests: - Add tests for CR deletion and finalizer cleanup - Add test verifying deployment-not-ready blocks ReadyCondition - Add tests for missing openstack-config ConfigMap, openstack-config-secret, and fencing-secret (each verifies InputReadyCondition stays false) Stale Nova connection fix: - Refresh the Nova connection at the start of each poll cycle instead of reusing the one created at startup. Prevents token expiry issues where auth errors inside _process_stale_services were silently caught by inner exception handlers and never surfaced to the reconnection logic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2e80986 commit 458b338

8 files changed

Lines changed: 560 additions & 12 deletions

File tree

apis/instanceha/v1beta1/instanceha_webhook.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ func (r *InstanceHa) ValidateUpdate(ctx context.Context, c client.Client, old ru
121121
var allWarn []string
122122
basePath := field.NewPath("spec")
123123

124+
oldInstance, ok := old.(*InstanceHa)
125+
if !ok {
126+
return nil, apierrors.NewInternalError(fmt.Errorf("unable to convert existing object"))
127+
}
128+
129+
if r.Spec.OpenStackCloud != oldInstance.Spec.OpenStackCloud {
130+
allErrs = append(allErrs, field.Forbidden(
131+
basePath.Child("openStackCloud"),
132+
"openStackCloud cannot be changed after creation",
133+
))
134+
}
135+
124136
allErrs = append(allErrs, r.Spec.ValidateTopology(basePath, r.Namespace)...)
125137
allErrs = append(allErrs, r.validateUniqueOpenStackCloud(ctx, c, basePath)...)
126138
allErrs = append(allErrs, r.validateCipherSuites(basePath)...)

internal/instanceha/funcs.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ func Deployment(
186186
RunAsUser: ptr.To(instanceHaUID),
187187
RunAsGroup: ptr.To(instanceHaUID),
188188
RunAsNonRoot: ptr.To(true),
189+
ReadOnlyRootFilesystem: ptr.To(true),
189190
AllowPrivilegeEscalation: ptr.To(false),
190191
Capabilities: &corev1.Capabilities{
191192
Drop: []corev1.Capability{
@@ -279,6 +280,10 @@ func instancehaPodVolumeMounts() []corev1.VolumeMount {
279280
SubPath: "config.yaml",
280281
ReadOnly: true,
281282
},
283+
{
284+
Name: "tmp",
285+
MountPath: "/tmp",
286+
},
282287
}
283288
}
284289

@@ -336,5 +341,11 @@ func instancehaPodVolumes(
336341
},
337342
},
338343
},
344+
{
345+
Name: "tmp",
346+
VolumeSource: corev1.VolumeSource{
347+
EmptyDir: &corev1.EmptyDirVolumeSource{},
348+
},
349+
},
339350
}
340351
}

internal/instanceha/funcs_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,55 @@ func TestDeploymentDisabledEnvVar(t *testing.T) {
7070
})
7171
}
7272
}
73+
74+
func TestDeploymentSecurityContext(t *testing.T) {
75+
g := NewWithT(t)
76+
77+
instance := &instancehav1.InstanceHa{
78+
ObjectMeta: metav1.ObjectMeta{
79+
Name: "test-instanceha",
80+
Namespace: "test-namespace",
81+
},
82+
Spec: instancehav1.InstanceHaSpec{
83+
ContainerImage: "test-image:latest",
84+
OpenStackCloud: "default",
85+
OpenStackConfigMap: "openstack-config",
86+
OpenStackConfigSecret: "openstack-config-secret",
87+
FencingSecret: "fencing-secret",
88+
InstanceHaConfigMap: "instanceha-config",
89+
InstanceHaKdumpPort: 7410,
90+
Disabled: "False",
91+
},
92+
}
93+
94+
labels := map[string]string{"app": "instanceha"}
95+
annotations := map[string]string{}
96+
97+
dep := Deployment(instance, labels, annotations, "default", "hash123", "test-image:latest", nil, "", "", nil)
98+
99+
container := dep.Spec.Template.Spec.Containers[0]
100+
101+
g.Expect(container.SecurityContext.ReadOnlyRootFilesystem).NotTo(BeNil())
102+
g.Expect(*container.SecurityContext.ReadOnlyRootFilesystem).To(BeTrue())
103+
104+
// Verify /tmp emptyDir volume exists
105+
var tmpVolumeFound bool
106+
for _, vol := range dep.Spec.Template.Spec.Volumes {
107+
if vol.Name == "tmp" {
108+
tmpVolumeFound = true
109+
g.Expect(vol.VolumeSource.EmptyDir).NotTo(BeNil())
110+
break
111+
}
112+
}
113+
g.Expect(tmpVolumeFound).To(BeTrue(), "/tmp emptyDir volume should exist")
114+
115+
// Verify /tmp mount exists
116+
var tmpMountFound bool
117+
for _, mount := range container.VolumeMounts {
118+
if mount.Name == "tmp" && mount.MountPath == "/tmp" {
119+
tmpMountFound = true
120+
break
121+
}
122+
}
123+
g.Expect(tmpMountFound).To(BeTrue(), "/tmp volume mount should exist")
124+
}
Lines changed: 279 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,279 @@
1+
/*
2+
Copyright 2025.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v1beta1
18+
19+
import (
20+
. "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports
21+
. "github.com/onsi/gomega" //revive:disable:dot-imports
22+
instancehav1beta1 "github.com/openstack-k8s-operators/infra-operator/apis/instanceha/v1beta1"
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
)
25+
26+
var _ = Describe("InstanceHa webhook", func() {
27+
Context("Default method", func() {
28+
It("should set all defaults on an empty spec", func() {
29+
instanceha := &instancehav1beta1.InstanceHa{
30+
ObjectMeta: metav1.ObjectMeta{
31+
Name: "test-instanceha-defaults",
32+
Namespace: "default",
33+
},
34+
Spec: instancehav1beta1.InstanceHaSpec{},
35+
}
36+
37+
instanceha.Default()
38+
39+
Expect(instanceha.Spec.OpenStackCloud).To(Equal(instancehav1beta1.OpenStackCloud))
40+
Expect(instanceha.Spec.OpenStackConfigMap).To(Equal("openstack-config"))
41+
Expect(instanceha.Spec.OpenStackConfigSecret).To(Equal("openstack-config-secret"))
42+
Expect(instanceha.Spec.FencingSecret).To(Equal("fencing-secret"))
43+
Expect(instanceha.Spec.InstanceHaConfigMap).To(Equal("instanceha-config"))
44+
Expect(instanceha.Spec.InstanceHaKdumpPort).To(Equal(int32(7410)))
45+
Expect(instanceha.Spec.InstanceHaHeartbeatPort).To(Equal(int32(7411)))
46+
Expect(instanceha.Spec.MetricsTLS.MinTLSVersion).To(Equal("1.2"))
47+
Expect(instanceha.Spec.MetricsTLS.CipherSuites).To(Equal("HIGH:!aNULL:!MD5:!RC4:!3DES:!kRSA"))
48+
})
49+
50+
It("should not override explicitly set values", func() {
51+
instanceha := &instancehav1beta1.InstanceHa{
52+
ObjectMeta: metav1.ObjectMeta{
53+
Name: "test-instanceha-explicit",
54+
Namespace: "default",
55+
},
56+
Spec: instancehav1beta1.InstanceHaSpec{
57+
OpenStackCloud: "my-cloud",
58+
OpenStackConfigMap: "my-config",
59+
OpenStackConfigSecret: "my-secret",
60+
FencingSecret: "my-fencing",
61+
InstanceHaConfigMap: "my-instanceha-config",
62+
InstanceHaKdumpPort: 8000,
63+
InstanceHaHeartbeatPort: 9000,
64+
MetricsTLS: instancehav1beta1.InstanceHaMetricsTLS{
65+
MinTLSVersion: "1.3",
66+
CipherSuites: "HIGH",
67+
},
68+
},
69+
}
70+
71+
instanceha.Default()
72+
73+
Expect(instanceha.Spec.OpenStackCloud).To(Equal("my-cloud"))
74+
Expect(instanceha.Spec.OpenStackConfigMap).To(Equal("my-config"))
75+
Expect(instanceha.Spec.OpenStackConfigSecret).To(Equal("my-secret"))
76+
Expect(instanceha.Spec.FencingSecret).To(Equal("my-fencing"))
77+
Expect(instanceha.Spec.InstanceHaConfigMap).To(Equal("my-instanceha-config"))
78+
Expect(instanceha.Spec.InstanceHaKdumpPort).To(Equal(int32(8000)))
79+
Expect(instanceha.Spec.InstanceHaHeartbeatPort).To(Equal(int32(9000)))
80+
Expect(instanceha.Spec.MetricsTLS.MinTLSVersion).To(Equal("1.3"))
81+
Expect(instanceha.Spec.MetricsTLS.CipherSuites).To(Equal("HIGH"))
82+
})
83+
})
84+
85+
Context("ValidateCreate", func() {
86+
It("should accept a valid InstanceHa", func() {
87+
instanceha := &instancehav1beta1.InstanceHa{
88+
ObjectMeta: metav1.ObjectMeta{
89+
Name: "test-create-valid",
90+
Namespace: "default",
91+
},
92+
Spec: instancehav1beta1.InstanceHaSpec{
93+
OpenStackCloud: "default",
94+
MetricsTLS: instancehav1beta1.InstanceHaMetricsTLS{
95+
CipherSuites: "HIGH:!aNULL:!MD5",
96+
},
97+
},
98+
}
99+
100+
warnings, err := instanceha.ValidateCreate(ctx, k8sClient)
101+
Expect(warnings).To(BeEmpty())
102+
Expect(err).NotTo(HaveOccurred())
103+
})
104+
105+
It("should reject NULL cipher suite without ! prefix", func() {
106+
instanceha := &instancehav1beta1.InstanceHa{
107+
ObjectMeta: metav1.ObjectMeta{
108+
Name: "test-create-null-cipher",
109+
Namespace: "default",
110+
},
111+
Spec: instancehav1beta1.InstanceHaSpec{
112+
OpenStackCloud: "default",
113+
MetricsTLS: instancehav1beta1.InstanceHaMetricsTLS{
114+
CipherSuites: "HIGH:NULL",
115+
},
116+
},
117+
}
118+
119+
_, err := instanceha.ValidateCreate(ctx, k8sClient)
120+
Expect(err).To(HaveOccurred())
121+
Expect(err.Error()).To(ContainSubstring("NULL"))
122+
})
123+
124+
It("should reject ALL cipher suite", func() {
125+
instanceha := &instancehav1beta1.InstanceHa{
126+
ObjectMeta: metav1.ObjectMeta{
127+
Name: "test-create-all-cipher",
128+
Namespace: "default",
129+
},
130+
Spec: instancehav1beta1.InstanceHaSpec{
131+
OpenStackCloud: "default",
132+
MetricsTLS: instancehav1beta1.InstanceHaMetricsTLS{
133+
CipherSuites: "ALL",
134+
},
135+
},
136+
}
137+
138+
_, err := instanceha.ValidateCreate(ctx, k8sClient)
139+
Expect(err).To(HaveOccurred())
140+
Expect(err.Error()).To(ContainSubstring("ALL"))
141+
})
142+
143+
It("should reject @SECLEVEL=0 cipher suite", func() {
144+
instanceha := &instancehav1beta1.InstanceHa{
145+
ObjectMeta: metav1.ObjectMeta{
146+
Name: "test-create-seclevel0",
147+
Namespace: "default",
148+
},
149+
Spec: instancehav1beta1.InstanceHaSpec{
150+
OpenStackCloud: "default",
151+
MetricsTLS: instancehav1beta1.InstanceHaMetricsTLS{
152+
CipherSuites: "HIGH:@SECLEVEL=0",
153+
},
154+
},
155+
}
156+
157+
_, err := instanceha.ValidateCreate(ctx, k8sClient)
158+
Expect(err).To(HaveOccurred())
159+
Expect(err.Error()).To(ContainSubstring("SECLEVEL=0"))
160+
})
161+
162+
It("should accept cipher suite with !NULL (negated)", func() {
163+
instanceha := &instancehav1beta1.InstanceHa{
164+
ObjectMeta: metav1.ObjectMeta{
165+
Name: "test-create-negated-null",
166+
Namespace: "default",
167+
},
168+
Spec: instancehav1beta1.InstanceHaSpec{
169+
OpenStackCloud: "default",
170+
MetricsTLS: instancehav1beta1.InstanceHaMetricsTLS{
171+
CipherSuites: "HIGH:!NULL:!aNULL",
172+
},
173+
},
174+
}
175+
176+
warnings, err := instanceha.ValidateCreate(ctx, k8sClient)
177+
Expect(warnings).To(BeEmpty())
178+
Expect(err).NotTo(HaveOccurred())
179+
})
180+
181+
It("should reject duplicate OpenStackCloud in the same namespace", func() {
182+
existing := &instancehav1beta1.InstanceHa{
183+
ObjectMeta: metav1.ObjectMeta{
184+
Name: "test-existing",
185+
Namespace: "default",
186+
},
187+
Spec: instancehav1beta1.InstanceHaSpec{
188+
ContainerImage: "test:latest",
189+
OpenStackCloud: "shared-cloud",
190+
Disabled: "False",
191+
},
192+
}
193+
Expect(k8sClient.Create(ctx, existing)).To(Succeed())
194+
DeferCleanup(func() {
195+
Expect(k8sClient.Delete(ctx, existing)).To(Succeed())
196+
})
197+
198+
duplicate := &instancehav1beta1.InstanceHa{
199+
ObjectMeta: metav1.ObjectMeta{
200+
Name: "test-duplicate",
201+
Namespace: "default",
202+
},
203+
Spec: instancehav1beta1.InstanceHaSpec{
204+
OpenStackCloud: "shared-cloud",
205+
},
206+
}
207+
208+
_, err := duplicate.ValidateCreate(ctx, k8sClient)
209+
Expect(err).To(HaveOccurred())
210+
Expect(err.Error()).To(ContainSubstring("already manages OpenStackCloud"))
211+
})
212+
})
213+
214+
Context("ValidateUpdate", func() {
215+
It("should reject changing openStackCloud", func() {
216+
oldInstance := &instancehav1beta1.InstanceHa{
217+
ObjectMeta: metav1.ObjectMeta{
218+
Name: "test-update-immutable",
219+
Namespace: "default",
220+
},
221+
Spec: instancehav1beta1.InstanceHaSpec{
222+
OpenStackCloud: "original-cloud",
223+
},
224+
}
225+
226+
newInstance := oldInstance.DeepCopy()
227+
newInstance.Spec.OpenStackCloud = "different-cloud"
228+
229+
_, err := newInstance.ValidateUpdate(ctx, k8sClient, oldInstance)
230+
Expect(err).To(HaveOccurred())
231+
Expect(err.Error()).To(ContainSubstring("cannot be changed after creation"))
232+
})
233+
234+
It("should accept update when openStackCloud is unchanged", func() {
235+
oldInstance := &instancehav1beta1.InstanceHa{
236+
ObjectMeta: metav1.ObjectMeta{
237+
Name: "test-update-valid",
238+
Namespace: "default",
239+
},
240+
Spec: instancehav1beta1.InstanceHaSpec{
241+
OpenStackCloud: "my-cloud",
242+
Disabled: "False",
243+
MetricsTLS: instancehav1beta1.InstanceHaMetricsTLS{
244+
CipherSuites: "HIGH:!aNULL",
245+
},
246+
},
247+
}
248+
249+
newInstance := oldInstance.DeepCopy()
250+
newInstance.Spec.Disabled = "True"
251+
252+
warnings, err := newInstance.ValidateUpdate(ctx, k8sClient, oldInstance)
253+
Expect(warnings).To(BeEmpty())
254+
Expect(err).NotTo(HaveOccurred())
255+
})
256+
257+
It("should reject unsafe cipher suites on update", func() {
258+
oldInstance := &instancehav1beta1.InstanceHa{
259+
ObjectMeta: metav1.ObjectMeta{
260+
Name: "test-update-cipher",
261+
Namespace: "default",
262+
},
263+
Spec: instancehav1beta1.InstanceHaSpec{
264+
OpenStackCloud: "my-cloud",
265+
MetricsTLS: instancehav1beta1.InstanceHaMetricsTLS{
266+
CipherSuites: "HIGH:!aNULL",
267+
},
268+
},
269+
}
270+
271+
newInstance := oldInstance.DeepCopy()
272+
newInstance.Spec.MetricsTLS.CipherSuites = "ALL"
273+
274+
_, err := newInstance.ValidateUpdate(ctx, k8sClient, oldInstance)
275+
Expect(err).To(HaveOccurred())
276+
Expect(err.Error()).To(ContainSubstring("ALL"))
277+
})
278+
})
279+
})

0 commit comments

Comments
 (0)