Can admission logic be placed in a separate package when using scaffolded webhooks?

[LCaparelli]

Type of question

How to implement a specific feature

Question

Can admission logic be placed in a separate package when using scaffolded webhooks?

What did you do?

I'm implementing webhook-based validation for the single CR my operator manages. For that, I simply ran:

operator-sdk create webhook --group apps.m88i.io --version v1alpha1 --kind Nexus --defaulting --programmatic-validation

It generated the code as expected, so far so good. I decided it would be nice to keep all logic related to default setting and validation in its own package (namely pkg/admission), so I wrote that and imported this package for doing the heavy lifting in the scaffolded methods present in api/v1alpha1/nexus_webhook.go.

What did you expect to see?

I would expect to be able to use a separate package for the logic. Until now, our api/v1alpha1 package contained no logic other than the auto-generated deep copy logic and we would like to keep it that way.

What did you see instead? Under which circumstances?

This runs into an import cycle. To validate the CR we actually need to import its package (v1alpha1) into our separate admission package, but the webhook methods import the admission package to actually perform the necessary operations.

The webhook cannot be moved to this separate package as the receiver of the scaffolded methods is our CR, defined in v1alpha1. Is keeping all validation and default setting logic in v1alpha1 instead the only way to implement this?

Environment

Operator type:

/language go

Kubernetes cluster type: any

$ operator-sdk version

operator-sdk version: "v1.0.1", commit: "4169b318b578156ed56530f373d328276d040a1b", kubernetes version: "v1.18.2", go version: "go1.13.15 linux/amd64", GOOS: "linux", GOARCH: "amd64"

$ go version (if language is Go)

go version go1.14.2 linux/amd64

$ kubectl version

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.2", GitCommit:"52c56ce7a8272c798dbc29846288d7cd9fbae032", GitTreeState:"clean", BuildDate:"2020-04-16T11:56:40Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}

Additional context

[LCaparelli]

We have since reconsidered our decision to keep validation logic in a separate package and made peace with keeping it in the same package as the CR struct definition. For now we only manage one CR, but if we ever expand on it we'd rather organizing the validation code on a per-CR basis, instead of keeping the code for all CRs in a single package.

Although we no longer wish to go forward with the design mentioned on the issue's description, we're still curious if this is possible at all, so I'm keeping it open for now. :-)

[estroz]

@LCaparelli loosely speaking it isn't really possible because of Go's import and type system; additionally controller-runtime's webhook.Validator interface needs to be implemented on the type it's validating, so that your main() function can registered webhooks with the manager. You could set up some sort of interface in pkg/admission that Nexus implements as well as webhook.Validator such that you can get relevant fields to validate:

// In pkg/admission/admission.go

type Validatable interface {
    GetSomeField() string
    ...
}

func ValidateCreate(v Validatable) error {
    if v.GetSomeField() != "validvalue" {
        ...
    }
    ...
}

func ValidateUpdate(v Validatable) error {
    ...
}

// In nexus_types.go

import (
    "github.com/my/module/pkg/admission"
)

type Nexus struct {
    ...
}

var _ admission.Validatable = Nexus{}

func (k Nexus) GetSomeField() string {
    return k.Spec.SomeField
}

// In nexus_webhooks.go

import (
    "github.com/my/module/pkg/admission"
)

var _ webhook.Validator = &Nexus{}

func (k *Nexus) ValidateCreate() error {
    nexuslog.Info("validate create", "name", r.Name)

    return admission.ValidateCreate(r)
}

[jberkhahn]

We're going to close this out. If this comes up again it's probably a good idea to open this upstream in controller-runtime as this is more an issue with CR then OperatorSDK.

[LCaparelli]

Thank you for the responses @estroz @jberkhahn :-)