Skip to content

Commit

Permalink
Move validation web hook to a subpackage
Browse files Browse the repository at this point in the history 
To add validation logic based on manifest rendering,
we need to decouple the validation web hook from the api package
to avoid loop dependencies

Signed-off-by: amaslennikov <[email protected]>
  • Loading branch information
@almaslennikov
almaslennikov committed Dec 6, 2023
1 parent f1840c5 commit 5c67766
Show file tree
Hide file tree
Showing 7 changed files with 224 additions and 193 deletions.
22 changes: 14 additions & 8 deletions api/v1alpha1/hostdevicenetwork_webhook.go → ...a1/validator/hostdevicenetwork_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1
package validator

import (
"regexp"
Expand All @@ -27,24 +27,30 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/Mellanox/network-operator/api/v1alpha1"
)

// log is for logging in this package.
var hostDeviceNetworkLog = logf.Log.WithName("hostdevicenetwork-resource")

func (w *HostDeviceNetwork) SetupWebhookWithManager(mgr ctrl.Manager) error {
type hostDeviceNetworkWrapper struct {
v1alpha1.HostDeviceNetwork
}

func SetupHostDeviceNetworkWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
For(w).
For(&hostDeviceNetworkWrapper{}).
Complete()
}

//nolint:lll
//+kubebuilder:webhook:path=/validate-mellanox-com-v1alpha1-hostdevicenetwork,mutating=false,failurePolicy=fail,sideEffects=None,groups=mellanox.com,resources=hostdevicenetworks,verbs=create;update,versions=v1alpha1,name=vhostdevicenetwork.kb.io,admissionReviewVersions=v1

var _ webhook.Validator = &HostDeviceNetwork{}
var _ webhook.Validator = &hostDeviceNetworkWrapper{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (w *HostDeviceNetwork) ValidateCreate() (admission.Warnings, error) {
func (w *hostDeviceNetworkWrapper) ValidateCreate() (admission.Warnings, error) {
if skipValidations {
nicClusterPolicyLog.Info("skipping CR validation")
return nil, nil
Expand All @@ -56,7 +62,7 @@ func (w *HostDeviceNetwork) ValidateCreate() (admission.Warnings, error) {
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (w *HostDeviceNetwork) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
func (w *hostDeviceNetworkWrapper) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
if skipValidations {
nicClusterPolicyLog.Info("skipping CR validation")
return nil, nil
Expand All @@ -68,7 +74,7 @@ func (w *HostDeviceNetwork) ValidateUpdate(_ runtime.Object) (admission.Warnings
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (w *HostDeviceNetwork) ValidateDelete() (admission.Warnings, error) {
func (w *hostDeviceNetworkWrapper) ValidateDelete() (admission.Warnings, error) {
if skipValidations {
nicClusterPolicyLog.Info("skipping CR validation")
return nil, nil
Expand All @@ -85,7 +91,7 @@ We are validating here HostDeviceNetwork:
- ResourceName must be valid for k8s
*/

func (w *HostDeviceNetwork) validateHostDeviceNetwork() error {
func (w *hostDeviceNetworkWrapper) validateHostDeviceNetwork() error {
resourceName := w.Spec.ResourceName
if !isValidHostDeviceNetworkResourceName(resourceName) {
var allErrs field.ErrorList
Expand Down
16 changes: 9 additions & 7 deletions ...1alpha1/hostdevicenetwork_webhook_test.go → ...lidator/hostdevicenetwork_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,36 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package v1alpha1 //nolint:dupl
package validator //nolint:dupl

import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/Mellanox/network-operator/api/v1alpha1"
)

//nolint:dupl
var _ = Describe("Validate", func() {
Context("HostDeviceNetwork tests", func() {
It("Valid ResourceName", func() {
hostDeviceNetwork := HostDeviceNetwork{
hostDeviceNetwork := hostDeviceNetworkWrapper{HostDeviceNetwork: v1alpha1.HostDeviceNetwork{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: HostDeviceNetworkSpec{
Spec: v1alpha1.HostDeviceNetworkSpec{
ResourceName: "hostdev",
},
}
}}
_, err := hostDeviceNetwork.ValidateCreate()
Expect(err).NotTo(HaveOccurred())
})
It("Invalid ResourceName", func() {
hostDeviceNetwork := HostDeviceNetwork{
hostDeviceNetwork := hostDeviceNetworkWrapper{HostDeviceNetwork: v1alpha1.HostDeviceNetwork{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: HostDeviceNetworkSpec{
Spec: v1alpha1.HostDeviceNetworkSpec{
ResourceName: "hostdev!!",
},
}
}}
_, err := hostDeviceNetwork.ValidateCreate()
Expect(err.Error()).To(ContainSubstring("Invalid Resource name"))
})
Expand Down
68 changes: 45 additions & 23 deletions api/v1alpha1/nicclusterpolicy_webhook.go → ...ha1/validator/nicclusterpolicy_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1
package validator

import (
"encoding/json"
Expand All @@ -37,6 +37,8 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/Mellanox/network-operator/api/v1alpha1"
)

const (
Expand All @@ -52,21 +54,37 @@ var schemaValidators *schemaValidator

var skipValidations = false

func (w *NicClusterPolicy) SetupWebhookWithManager(mgr ctrl.Manager) error {
type nicClusterPolicyWrapper struct {
v1alpha1.NicClusterPolicy
}

type devicePluginSpecWrapper struct {
v1alpha1.DevicePluginSpec
}

type ibKubernetesSpecWrapper struct {
v1alpha1.IBKubernetesSpec
}

type ofedDriverSpecWrapper struct {
v1alpha1.OFEDDriverSpec
}

func SetupNicClusterPolicyWebhookWithManager(mgr ctrl.Manager) error {
nicClusterPolicyLog.Info("Nic cluster policy webhook admission controller")
InitSchemaValidator("./webhook-schemas")
return ctrl.NewWebhookManagedBy(mgr).
For(w).
For(&nicClusterPolicyWrapper{}).
Complete()
}

//nolint:lll
//+kubebuilder:webhook:path=/validate-mellanox-com-v1alpha1-nicclusterpolicy,mutating=false,failurePolicy=fail,sideEffects=None,groups=mellanox.com,resources=nicclusterpolicies,verbs=create;update,versions=v1alpha1,name=vnicclusterpolicy.kb.io,admissionReviewVersions=v1

var _ webhook.Validator = &NicClusterPolicy{}
var _ webhook.Validator = &nicClusterPolicyWrapper{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (w *NicClusterPolicy) ValidateCreate() (admission.Warnings, error) {
func (w *nicClusterPolicyWrapper) ValidateCreate() (admission.Warnings, error) {
if skipValidations {
nicClusterPolicyLog.Info("skipping CR validation")
return nil, nil
Expand All @@ -77,7 +95,7 @@ func (w *NicClusterPolicy) ValidateCreate() (admission.Warnings, error) {
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (w *NicClusterPolicy) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
func (w *nicClusterPolicyWrapper) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
if skipValidations {
nicClusterPolicyLog.Info("skipping CR validation")
return nil, nil
Expand All @@ -88,7 +106,7 @@ func (w *NicClusterPolicy) ValidateUpdate(_ runtime.Object) (admission.Warnings,
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (w *NicClusterPolicy) ValidateDelete() (admission.Warnings, error) {
func (w *nicClusterPolicyWrapper) ValidateDelete() (admission.Warnings, error) {
if skipValidations {
nicClusterPolicyLog.Info("skipping CR validation")
return nil, nil
Expand Down Expand Up @@ -117,34 +135,38 @@ We are validating here NicClusterPolicy:
4.3. At least one of the supported selectors exists.
4.4. All selectors are strings.
*/
func (w *NicClusterPolicy) validateNicClusterPolicy() error {
func (w *nicClusterPolicyWrapper) validateNicClusterPolicy() error {
var allErrs field.ErrorList
// Validate Repository
allErrs = w.validateRepositories(allErrs)
allErrs = w.validateContainerResources(allErrs)
// Validate IBKubernetes
ibKubernetes := w.Spec.IBKubernetes
if ibKubernetes != nil {
allErrs = append(allErrs, ibKubernetes.validate(field.NewPath("spec").Child("ibKubernetes"))...)
wrapper := ibKubernetesSpecWrapper{IBKubernetesSpec: *w.Spec.IBKubernetes}
allErrs = append(allErrs, wrapper.validate(field.NewPath("spec").Child("ibKubernetes"))...)
}
// Validate OFEDDriverSpec
ofedDriver := w.Spec.OFEDDriver
if ofedDriver != nil {
wrapper := ofedDriverSpecWrapper{OFEDDriverSpec: *w.Spec.OFEDDriver}
ofedDriverFieldPath := field.NewPath("spec").Child("ofedDriver")
allErrs = append(append(allErrs,
ofedDriver.validateVersion(ofedDriverFieldPath)...),
ofedDriver.validateSafeLoad(ofedDriverFieldPath)...)
wrapper.validateVersion(ofedDriverFieldPath)...),
wrapper.validateSafeLoad(ofedDriverFieldPath)...)
}
// Validate RdmaSharedDevicePlugin
rdmaSharedDevicePlugin := w.Spec.RdmaSharedDevicePlugin
if rdmaSharedDevicePlugin != nil {
allErrs = append(allErrs, w.Spec.RdmaSharedDevicePlugin.validateRdmaSharedDevicePlugin(
wrapper := devicePluginSpecWrapper{DevicePluginSpec: *w.Spec.RdmaSharedDevicePlugin}
allErrs = append(allErrs, wrapper.validateRdmaSharedDevicePlugin(
field.NewPath("spec").Child("rdmaSharedDevicePlugin"))...)
}
// Validate SriovDevicePlugin
sriovNetworkDevicePlugin := w.Spec.SriovDevicePlugin
if sriovNetworkDevicePlugin != nil {
allErrs = append(allErrs, w.Spec.SriovDevicePlugin.validateSriovNetworkDevicePlugin(
wrapper := devicePluginSpecWrapper{DevicePluginSpec: *w.Spec.SriovDevicePlugin}
allErrs = append(allErrs, wrapper.validateSriovNetworkDevicePlugin(
field.NewPath("spec").Child("sriovNetworkDevicePlugin"))...)
}

Expand All @@ -155,7 +177,7 @@ func (w *NicClusterPolicy) validateNicClusterPolicy() error {
schema.GroupKind{Group: "mellanox.com", Kind: "NicClusterPolicy"},
w.Name, allErrs)
}
func (dp *DevicePluginSpec) validateSriovNetworkDevicePlugin(fldPath *field.Path) field.ErrorList {
func (dp *devicePluginSpecWrapper) validateSriovNetworkDevicePlugin(fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
var sriovNetworkDevicePluginConfigJSON map[string]interface{}
sriovNetworkDevicePluginConfig := *dp.Config
Expand Down Expand Up @@ -245,7 +267,7 @@ func (dp *DevicePluginSpec) validateSriovNetworkDevicePlugin(fldPath *field.Path
}

func validateResourceNamePrefix(resource map[string]interface{},
allErrs field.ErrorList, fldPath *field.Path, dp *DevicePluginSpec) (bool, field.ErrorList) {
allErrs field.ErrorList, fldPath *field.Path, dp *devicePluginSpecWrapper) (bool, field.ErrorList) {
resourceName := resource["resourceName"].(string)
if !isValidSriovNetworkDevicePluginResourceName(resourceName) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("Config"), dp.Config,
Expand All @@ -266,7 +288,7 @@ func validateResourceNamePrefix(resource map[string]interface{},
return true, allErrs
}

func (dp *DevicePluginSpec) validateRdmaSharedDevicePlugin(fldPath *field.Path) field.ErrorList {
func (dp *devicePluginSpecWrapper) validateRdmaSharedDevicePlugin(fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
var rdmaSharedDevicePluginConfigJSON map[string]interface{}
rdmaSharedDevicePluginConfig := *dp.Config
Expand Down Expand Up @@ -321,7 +343,7 @@ func (dp *DevicePluginSpec) validateRdmaSharedDevicePlugin(fldPath *field.Path)
}

// validate is a helper function to perform validation for IBKubernetesSpec.
func (ibk *IBKubernetesSpec) validate(fldPath *field.Path) field.ErrorList {
func (ibk *ibKubernetesSpecWrapper) validate(fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList

if !isValidPKeyGUID(ibk.PKeyGUIDPoolRangeStart) || !isValidPKeyGUID(ibk.PKeyGUIDPoolRangeEnd) {
Expand Down Expand Up @@ -362,7 +384,7 @@ func isValidPKeyRange(startGUID, endGUID string) bool {
return endGUIDIntValue.Cmp(startGUIDIntValue) > 0
}

func (ofedSpec *OFEDDriverSpec) validateVersion(fldPath *field.Path) field.ErrorList {
func (ofedSpec *ofedDriverSpecWrapper) validateVersion(fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

// Perform version validation logic here
Expand All @@ -373,7 +395,7 @@ func (ofedSpec *OFEDDriverSpec) validateVersion(fldPath *field.Path) field.Error
return allErrs
}

func (ofedSpec *OFEDDriverSpec) validateSafeLoad(fldPath *field.Path) field.ErrorList {
func (ofedSpec *ofedDriverSpecWrapper) validateSafeLoad(fldPath *field.Path) field.ErrorList {
upgradePolicy := ofedSpec.OfedUpgradePolicy
if upgradePolicy == nil {
return nil
Expand All @@ -391,7 +413,7 @@ func (ofedSpec *OFEDDriverSpec) validateSafeLoad(fldPath *field.Path) field.Erro
return allErrs
}

func (w *NicClusterPolicy) validateRepositories(allErrs field.ErrorList) field.ErrorList {
func (w *nicClusterPolicyWrapper) validateRepositories(allErrs field.ErrorList) field.ErrorList {
fp := field.NewPath("spec")
if w.Spec.OFEDDriver != nil {
allErrs = validateRepository(w.Spec.OFEDDriver.ImageSpec.Repository, allErrs, fp, "nicFeatureDiscovery")
Expand Down Expand Up @@ -439,7 +461,7 @@ func validateRepository(repo string, allErrs field.ErrorList, fp *field.Path, ch
return allErrs
}

func (w *NicClusterPolicy) validateContainerResources(allErrs field.ErrorList) field.ErrorList {
func (w *nicClusterPolicyWrapper) validateContainerResources(allErrs field.ErrorList) field.ErrorList {
fp := field.NewPath("spec")

states := map[string]interface{}{
Expand Down Expand Up @@ -484,15 +506,15 @@ func validateContainerResourcesIfNotNil(resource interface{}, allErrs field.Erro
containerResources := imageSpec.FieldByName("ContainerResources")
if containerResources.IsValid() && !containerResources.IsNil() {
return validateResourceRequirements(
containerResources.Interface().([]ResourceRequirements), allErrs, fp, fieldName)
containerResources.Interface().([]v1alpha1.ResourceRequirements), allErrs, fp, fieldName)
}
}
}

return allErrs
}

func validateResourceRequirements(resources []ResourceRequirements, allErrs field.ErrorList,
func validateResourceRequirements(resources []v1alpha1.ResourceRequirements, allErrs field.ErrorList,
fp *field.Path, child string) field.ErrorList {
if len(resources) == 0 {
allErrs = append(allErrs, field.Invalid(fp.Child(child).Child("containerResources"),
Expand Down
Loading

0 comments on commit 5c67766

Please sign in to comment.