Skip to content

Commit 1789572

Browse files
fmountfmount
committed
Complete webhook validation for topology references
This patch is a follow up of [1] in the validation webhook area. It enhances the validation workflow and it aims to complete the pattern by distributing the ValidateTopology logic across the API instances. It is based on [2] and the key improvements include: - Distributed validation: it provides a ValidateTopology function across all API instances, ensuring consistent validation throughout the system - Optimized validation calls: it refines how validation is triggered and reduce code duplication; it still requires a basic struct refactor to fully de-duplicate the existing functions, but it can be done as part of a follow up This patch leverages the centralized topology validator introduced in infra-operator [3]. Jira: https://issues.redhat.com/browse/OSPRH-14626 [1] openstack-k8s-operators#924 [2] openstack-k8s-operators/nova-operator#936 [3] openstack-k8s-operators/infra-operator#356 Signed-off-by: Francesco Pantano <[email protected]>
1 parent 6fab322 commit 1789572

13 files changed

+142
-47
lines changed

‎api/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ require (
66
github.com/go-logr/logr v1.4.2
77
github.com/google/uuid v1.6.0
88
github.com/onsi/gomega v1.34.1
9-
github.com/openstack-k8s-operators/infra-operator/apis v0.6.0
9+
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250319162810-463dd75a4cc4
1010
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250315090821-34e570d2d5fb
1111
k8s.io/api v0.29.15
1212
k8s.io/apimachinery v0.29.15

‎api/go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k=
7878
github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY=
7979
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 h1:J1wuGhVxpsHykZBa6Beb1gQ96Ptej9AE/BvwCBiRj1E=
8080
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
81-
github.com/openstack-k8s-operators/infra-operator/apis v0.6.0 h1:28i9Yc3UAdQK8VNzk0ubwq4n+qLuhD0nk6/7iHgD9us=
82-
github.com/openstack-k8s-operators/infra-operator/apis v0.6.0/go.mod h1:JgcmYJyyMKfArK8ulZnbls0L01qt8Dq6s5LH8TZH63A=
81+
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250319162810-463dd75a4cc4 h1:wb2zsr9x9LantNLN/9dmYM42c3yLq3QuzsoOlGpDUxM=
82+
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250319162810-463dd75a4cc4/go.mod h1:n5DV/lGE9DHryAJ+JLJSgUXI2QHTj+aN4KoeSNC3PfU=
8383
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250315090821-34e570d2d5fb h1:UAFYEHnbyhO0+yymquFmIqxc9QGji9mzreuYrDS1Ev4=
8484
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250315090821-34e570d2d5fb/go.mod h1:1CtBP0MQffdjE6buOv5jP2rB3+h7WH0a11lcyrpmxOk=
8585
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=

‎api/v1beta1/ovncontroller_types.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20+
"k8s.io/apimachinery/pkg/util/validation/field"
2021
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
2122
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
2223

@@ -208,3 +209,15 @@ func (instance *OVNController) GetLastAppliedTopology() *topologyv1.TopoRef {
208209
func (instance *OVNController) SetLastAppliedTopology(topologyRef *topologyv1.TopoRef) {
209210
instance.Status.LastAppliedTopology = topologyRef
210211
}
212+
213+
// ValidateTopology -
214+
func (instance *OVNControllerSpecCore) ValidateTopology(
215+
basePath *field.Path,
216+
namespace string,
217+
) field.ErrorList {
218+
var allErrs field.ErrorList
219+
allErrs = append(allErrs, topologyv1.ValidateTopologyRef(
220+
instance.TopologyRef,
221+
*basePath.Child("topologyRef"), namespace)...)
222+
return allErrs
223+
}

‎api/v1beta1/ovncontroller_webhook.go‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package v1beta1
1818

1919
import (
20-
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
2120
"k8s.io/apimachinery/pkg/runtime"
2221
"k8s.io/apimachinery/pkg/util/validation/field"
2322
ctrl "sigs.k8s.io/controller-runtime"
@@ -106,11 +105,8 @@ func (r *OVNControllerSpecCore) ValidateCreate(basePath *field.Path, namespace s
106105

107106
// When a TopologyRef CR is referenced, fail if a different Namespace is
108107
// referenced because is not supported
109-
if r.TopologyRef != nil {
110-
if err := topologyv1.ValidateTopologyNamespace(r.TopologyRef.Namespace, *basePath, namespace); err != nil {
111-
errors = append(errors, err)
112-
}
113-
}
108+
errors = append(errors, r.ValidateTopology(basePath, namespace)...)
109+
114110
return errors
115111
}
116112

@@ -123,11 +119,8 @@ func (r *OVNController) ValidateUpdate(old runtime.Object) (admission.Warnings,
123119

124120
// When a TopologyRef CR is referenced, fail if a different Namespace is
125121
// referenced because is not supported
126-
if r.Spec.TopologyRef != nil {
127-
if err := topologyv1.ValidateTopologyNamespace(r.Spec.TopologyRef.Namespace, *basePath, r.Namespace); err != nil {
128-
errors = append(errors, err)
129-
}
130-
}
122+
errors = append(errors, r.Spec.ValidateTopology(basePath, r.Namespace)...)
123+
131124
if len(errors) != 0 {
132125
return nil, apierrors.NewInvalid(
133126
schema.GroupKind{Group: "ovn.openstack.org", Kind: "OVNController"},
@@ -141,16 +134,13 @@ func (r OVNControllerSpec) ValidateUpdate(old OVNControllerSpec, basePath *field
141134
}
142135

143136
func (r *OVNControllerSpecCore) ValidateUpdate(old OVNControllerSpec, basePath *field.Path, namespace string) field.ErrorList {
144-
errors := field.ErrorList{}
137+
allErrs := field.ErrorList{}
145138

146139
// When a TopologyRef CR is referenced, fail if a different Namespace is
147140
// referenced because is not supported
148-
if r.TopologyRef != nil {
149-
if err := topologyv1.ValidateTopologyNamespace(r.TopologyRef.Namespace, *basePath, namespace); err != nil {
150-
errors = append(errors, err)
151-
}
152-
}
153-
return errors
141+
allErrs = append(allErrs, r.ValidateTopology(basePath, namespace)...)
142+
143+
return allErrs
154144
}
155145

156146
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type

‎api/v1beta1/ovndbcluster_types.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"github.com/openstack-k8s-operators/lib-common/modules/common/service"
2424
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
2525
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
26+
"k8s.io/apimachinery/pkg/util/validation/field"
2627

2728
corev1 "k8s.io/api/core/v1"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -244,3 +245,15 @@ func (instance *OVNDBCluster) GetLastAppliedTopology() *topologyv1.TopoRef {
244245
func (instance *OVNDBCluster) SetLastAppliedTopology(topologyRef *topologyv1.TopoRef) {
245246
instance.Status.LastAppliedTopology = topologyRef
246247
}
248+
249+
// ValidateTopology -
250+
func (instance *OVNDBClusterSpecCore) ValidateTopology(
251+
basePath *field.Path,
252+
namespace string,
253+
) field.ErrorList {
254+
var allErrs field.ErrorList
255+
allErrs = append(allErrs, topologyv1.ValidateTopologyRef(
256+
instance.TopologyRef,
257+
*basePath.Child("topologyRef"), namespace)...)
258+
return allErrs
259+
}

‎api/v1beta1/ovndbcluster_webhook.go‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
logf "sigs.k8s.io/controller-runtime/pkg/log"
2929
"sigs.k8s.io/controller-runtime/pkg/webhook"
3030
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
31-
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
3231
"k8s.io/apimachinery/pkg/util/validation/field"
3332
apierrors "k8s.io/apimachinery/pkg/api/errors"
3433
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -100,11 +99,8 @@ func (r *OVNDBCluster) ValidateCreate() (admission.Warnings, error) {
10099

101100
// When a TopologyRef CR is referenced, fail if a different Namespace is
102101
// referenced because is not supported
103-
if r.Spec.TopologyRef != nil {
104-
if err := topologyv1.ValidateTopologyNamespace(r.Spec.TopologyRef.Namespace, *basePath, r.Namespace); err != nil {
105-
errors = append(errors, err)
106-
}
107-
}
102+
errors = append(errors, r.Spec.ValidateTopology(basePath, r.Namespace)...)
103+
108104
if len(errors) != 0 {
109105
return nil, apierrors.NewInvalid(
110106
schema.GroupKind{Group: "ovn.openstack.org", Kind: "OVNDBCluster"},
@@ -122,11 +118,8 @@ func (r *OVNDBCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, e
122118

123119
// When a TopologyRef CR is referenced, fail if a different Namespace is
124120
// referenced because is not supported
125-
if r.Spec.TopologyRef != nil {
126-
if err := topologyv1.ValidateTopologyNamespace(r.Spec.TopologyRef.Namespace, *basePath, r.Namespace); err != nil {
127-
errors = append(errors, err)
128-
}
129-
}
121+
errors = append(errors, r.Spec.ValidateTopology(basePath, r.Namespace)...)
122+
130123
if len(errors) != 0 {
131124
return nil, apierrors.NewInvalid(
132125
schema.GroupKind{Group: "ovn.openstack.org", Kind: "OVNDBCluster"},

‎api/v1beta1/ovnnorthd_types.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
2121
"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
2222
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
23+
"k8s.io/apimachinery/pkg/util/validation/field"
2324

2425
corev1 "k8s.io/api/core/v1"
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -163,3 +164,15 @@ func (instance *OVNNorthd) GetLastAppliedTopology() *topologyv1.TopoRef {
163164
func (instance *OVNNorthd) SetLastAppliedTopology(topologyRef *topologyv1.TopoRef) {
164165
instance.Status.LastAppliedTopology = topologyRef
165166
}
167+
168+
// ValidateTopology -
169+
func (instance *OVNNorthdSpecCore) ValidateTopology(
170+
basePath *field.Path,
171+
namespace string,
172+
) field.ErrorList {
173+
var allErrs field.ErrorList
174+
allErrs = append(allErrs, topologyv1.ValidateTopologyRef(
175+
instance.TopologyRef,
176+
*basePath.Child("topologyRef"), namespace)...)
177+
return allErrs
178+
}

‎api/v1beta1/ovnnorthd_webhook.go‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
logf "sigs.k8s.io/controller-runtime/pkg/log"
2929
"sigs.k8s.io/controller-runtime/pkg/webhook"
3030
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
31-
topologyv1 "github.com/openstack-k8s-operators/infra-operator/apis/topology/v1beta1"
3231
"k8s.io/apimachinery/pkg/util/validation/field"
3332
apierrors "k8s.io/apimachinery/pkg/api/errors"
3433
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -95,11 +94,8 @@ func (r *OVNNorthd) ValidateCreate() (admission.Warnings, error) {
9594

9695
// When a TopologyRef CR is referenced, fail if a different Namespace is
9796
// referenced because is not supported
98-
if r.Spec.TopologyRef != nil {
99-
if err := topologyv1.ValidateTopologyNamespace(r.Spec.TopologyRef.Namespace, *basePath, r.Namespace); err != nil {
100-
errors = append(errors, err)
101-
}
102-
}
97+
errors = append(errors, r.Spec.ValidateTopology(basePath, r.Namespace)...)
98+
10399
if len(errors) != 0 {
104100
return nil, apierrors.NewInvalid(
105101
schema.GroupKind{Group: "ovn.openstack.org", Kind: "OVNNorthd"},
@@ -117,11 +113,8 @@ func (r *OVNNorthd) ValidateUpdate(old runtime.Object) (admission.Warnings, erro
117113

118114
// When a TopologyRef CR is referenced, fail if a different Namespace is
119115
// referenced because is not supported
120-
if r.Spec.TopologyRef != nil {
121-
if err := topologyv1.ValidateTopologyNamespace(r.Spec.TopologyRef.Namespace, *basePath, r.Namespace); err != nil {
122-
errors = append(errors, err)
123-
}
124-
}
116+
errors = append(errors, r.Spec.ValidateTopology(basePath, r.Namespace)...)
117+
125118
if len(errors) != 0 {
126119
return nil, apierrors.NewInvalid(
127120
schema.GroupKind{Group: "ovn.openstack.org", Kind: "OVNNorthd"},

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ require (
88
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.7.5
99
github.com/onsi/ginkgo/v2 v2.20.1
1010
github.com/onsi/gomega v1.34.1
11-
github.com/openstack-k8s-operators/infra-operator/apis v0.6.0
11+
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250319162810-463dd75a4cc4
1212
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250315090821-34e570d2d5fb
1313
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20250315090821-34e570d2d5fb
1414
github.com/openstack-k8s-operators/ovn-operator/api v0.0.0-20230418071801-b5843d9e05fb

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k=
7676
github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY=
7777
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094 h1:J1wuGhVxpsHykZBa6Beb1gQ96Ptej9AE/BvwCBiRj1E=
7878
github.com/openshift/api v0.0.0-20240830023148-b7d0481c9094/go.mod h1:CxgbWAlvu2iQB0UmKTtRu1YfepRg1/vJ64n2DlIEVz4=
79-
github.com/openstack-k8s-operators/infra-operator/apis v0.6.0 h1:28i9Yc3UAdQK8VNzk0ubwq4n+qLuhD0nk6/7iHgD9us=
80-
github.com/openstack-k8s-operators/infra-operator/apis v0.6.0/go.mod h1:JgcmYJyyMKfArK8ulZnbls0L01qt8Dq6s5LH8TZH63A=
79+
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250319162810-463dd75a4cc4 h1:wb2zsr9x9LantNLN/9dmYM42c3yLq3QuzsoOlGpDUxM=
80+
github.com/openstack-k8s-operators/infra-operator/apis v0.6.1-0.20250319162810-463dd75a4cc4/go.mod h1:n5DV/lGE9DHryAJ+JLJSgUXI2QHTj+aN4KoeSNC3PfU=
8181
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250315090821-34e570d2d5fb h1:UAFYEHnbyhO0+yymquFmIqxc9QGji9mzreuYrDS1Ev4=
8282
github.com/openstack-k8s-operators/lib-common/modules/common v0.6.1-0.20250315090821-34e570d2d5fb/go.mod h1:1CtBP0MQffdjE6buOv5jP2rB3+h7WH0a11lcyrpmxOk=
8383
github.com/openstack-k8s-operators/lib-common/modules/test v0.6.1-0.20250315090821-34e570d2d5fb h1:7ADU3ZLE0l9/3A5l0jzAZt9XywzchosoY/m7VlFWu3I=

0 commit comments

Comments
 (0)