Skip to content

Commit

Permalink
Remove controller-runtime dependency from api package (#1483)
Browse files Browse the repository at this point in the history
Because of webhook codes, we have a dependency on
contoller-runtime in our api package. Api packages should
be as clean as possible. Also, we are hitting an issue*
in test when we try to consume our api package because of
this dependency.

* kubernetes-sigs/controller-runtime#878

Signed-off-by: Erkan Erol <eerol@redhat.com>
  • Loading branch information
Erkan Erol committed Aug 26, 2021
1 parent e71a532 commit 997d753
Show file tree
Hide file tree
Showing 10 changed files with 479 additions and 358 deletions.
24 changes: 13 additions & 11 deletions cmd/hyperconverged-cluster-webhook/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,32 @@ package main
import (
"context"
"fmt"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
kubevirtv1 "kubevirt.io/client-go/api/v1"
"os"

"github.com/kubevirt/hyperconverged-cluster-operator/cmd/cmdcommon"
"github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"

"github.com/kubevirt/hyperconverged-cluster-operator/pkg/webhooks"
kubevirtv1 "kubevirt.io/client-go/api/v1"

"sigs.k8s.io/controller-runtime/pkg/client/config"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/manager/signals"

networkaddons "github.com/kubevirt/cluster-network-addons-operator/pkg/apis"
hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis/hco/v1beta1"
hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
vmimportv1beta1 "github.com/kubevirt/vm-import-operator/pkg/apis/v2v/v1beta1"
"github.com/kubevirt/hyperconverged-cluster-operator/cmd/cmdcommon"
"github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis"

openshiftconfigv1 "github.com/openshift/api/config/v1"
corev1 "k8s.io/api/core/v1"
apiruntime "k8s.io/apimachinery/pkg/runtime"
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
logf "sigs.k8s.io/controller-runtime/pkg/log"

networkaddons "github.com/kubevirt/cluster-network-addons-operator/pkg/apis"
hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
vmimportv1beta1 "github.com/kubevirt/vm-import-operator/pkg/apis/v2v/v1beta1"
cdiv1beta1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1beta1"
sspv1beta1 "kubevirt.io/ssp-operator/api/v1beta1"
logf "sigs.k8s.io/controller-runtime/pkg/log"
)

// Change below variables to serve metrics on different host or port.
Expand Down Expand Up @@ -97,8 +100,7 @@ func main() {

// CreateServiceMonitors will automatically create the prometheus-operator ServiceMonitor resources
// necessary to configure Prometheus to scrape metrics from this operator.
hwHandler := &webhooks.WebhookHandler{}
if err = (&hcov1beta1.HyperConverged{}).SetupWebhookWithManager(ctx, mgr, hwHandler, ci.IsOpenshift()); err != nil {
if err = webhooks.SetupWebhookWithManager(ctx, mgr, ci.IsOpenshift()); err != nil {
logger.Error(err, "unable to create webhook", "webhook", "HyperConverged")
eventEmitter.EmitEvent(nil, corev1.EventTypeWarning, "InitError", "Unable to create webhook")
os.Exit(1)
Expand Down
163 changes: 5 additions & 158 deletions pkg/apis/hco/v1beta1/hyperconverged_webhook.go
Original file line number Diff line number Diff line change
@@ -1,98 +1,28 @@
package v1beta1

import (
"context"
"fmt"
"net/http"
"os"
"path/filepath"

"github.com/go-logr/logr"
admissionv1 "k8s.io/api/admission/v1"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
)

const (
webHookCertDirEnv = "WEBHOOK_CERT_DIR"
DefaultWebhookCertDir = "/apiserver.local.config/certificates"

WebhookCertName = "apiserver.crt"
WebhookKeyName = "apiserver.key"
)

var (
hcolog = logf.Log.WithName("hyperconverged-resource")
)

func GetWebhookCertDir() string {
webhookCertDir := os.Getenv(webHookCertDirEnv)
if webhookCertDir != "" {
return webhookCertDir
}
whHandler ValidatorWebhookHandler

return DefaultWebhookCertDir
}
_ webhook.Validator = &HyperConverged{}
)

type WebhookHandlerIfs interface {
Init(logger logr.Logger, cli client.Client, namespace string, isOpenshift bool)
type ValidatorWebhookHandler interface {
ValidateCreate(hc *HyperConverged) error
ValidateUpdate(requested *HyperConverged, exists *HyperConverged) error
ValidateDelete(hc *HyperConverged) error
HandleMutatingNsDelete(ns *corev1.Namespace, dryRun bool) (bool, error)
}

var whHandler WebhookHandlerIfs

func (r *HyperConverged) SetupWebhookWithManager(ctx context.Context, mgr ctrl.Manager, handler WebhookHandlerIfs, isOpenshift bool) error {
operatorNsEnv, nserr := hcoutil.GetOperatorNamespaceFromEnv()
if nserr != nil {
hcolog.Error(nserr, "failed to get operator namespace from the environment")
return nserr
}

// Make sure the certificates are mounted, this should be handled by the OLM
func SetValidatorWebhookHandler(handler ValidatorWebhookHandler) {
whHandler = handler
whHandler.Init(hcolog, mgr.GetClient(), operatorNsEnv, isOpenshift)

webhookCertDir := GetWebhookCertDir()
certs := []string{filepath.Join(webhookCertDir, WebhookCertName), filepath.Join(webhookCertDir, WebhookKeyName)}
for _, fname := range certs {
if _, err := os.Stat(fname); err != nil {
hcolog.Error(err, "CSV certificates were not found, skipping webhook initialization")
return err
}
}

if err := allowWatchAllNamespaces(ctx, mgr); err != nil {
return err
}

bldr := ctrl.NewWebhookManagedBy(mgr).For(r)

srv := mgr.GetWebhookServer()
srv.CertDir = GetWebhookCertDir()
srv.CertName = WebhookCertName
srv.KeyName = WebhookKeyName
srv.Port = hcoutil.WebhookPort
srv.Register(hcoutil.HCONSWebhookPath, &webhook.Admission{Handler: &nsMutator{}})

return bldr.Complete()
}

var (
_ webhook.Validator = &HyperConverged{}
)

func (r *HyperConverged) ValidateCreate() error {
return whHandler.ValidateCreate(r)
}
Expand All @@ -109,86 +39,3 @@ func (r *HyperConverged) ValidateUpdate(old runtime.Object) error {
func (r *HyperConverged) ValidateDelete() error {
return whHandler.ValidateDelete(r)
}

// nsMutator mutates Ns requests
type nsMutator struct {
decoder *admission.Decoder
}

func (a *nsMutator) Handle(_ context.Context, req admission.Request) admission.Response {
hcolog.Info("reaching nsMutator.Handle")

if req.Operation == admissionv1.Delete {
return a.handleNsDelete(req)
}

// ignoring other operations
return admission.Allowed("ignoring other operations")

}

func (a *nsMutator) handleNsDelete(req admission.Request) admission.Response {
ns := &corev1.Namespace{}

// In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346
// OldObject contains the object being deleted
err := a.decoder.DecodeRaw(req.OldObject, ns)
if err != nil {
hcolog.Error(err, "failed decoding namespace object")
return admission.Errored(http.StatusBadRequest, err)
}

admitted, err := whHandler.HandleMutatingNsDelete(ns, *req.DryRun)
if err != nil {
return admission.Errored(http.StatusInternalServerError, err)
}

if admitted {
return admission.Allowed("the namespace doesn't contain HyperConverged CR, admitting its deletion")
}
return admission.Denied("HyperConverged CR is still present, please remove it before deleting the containing namespace")
}

// The OLM limits the webhook scope to the namespaces that are defined in the OperatorGroup
// by setting namespaceSelector in the ValidatingWebhookConfiguration. We would like our webhook to intercept
// requests from all namespaces, and fail them if they're not in the correct namespace for HCO (for CREATE).
// Luckily the OLM does not watch and reconcile the ValidatingWebhookConfiguration so we can simply reset the
// namespaceSelector
func allowWatchAllNamespaces(ctx context.Context, mgr ctrl.Manager) error {
vwcList := &admissionregistrationv1.ValidatingWebhookConfigurationList{}
err := mgr.GetAPIReader().List(ctx, vwcList, client.MatchingLabels{"olm.webhook-description-generate-name": hcoutil.HcoValidatingWebhook})
if err != nil {
hcolog.Error(err, "A validating webhook for the HCO was not found")
return err
}

for _, vwc := range vwcList.Items {
update := false

for i, wh := range vwc.Webhooks {
if wh.Name == hcoutil.HcoValidatingWebhook {
vwc.Webhooks[i].NamespaceSelector = &metav1.LabelSelector{MatchLabels: map[string]string{}}
update = true
}
}

if update {
hcolog.Info("Removing namespace scope from webhook", "webhook", vwc.Name)
err = mgr.GetClient().Update(ctx, &vwc)
if err != nil {
hcolog.Error(err, "Failed updating webhook", "webhook", vwc.Name)
return err
}
}
}
return nil
}

// nsMutator implements admission.DecoderInjector.
// A decoder will be automatically injected.

// InjectDecoder injects the decoder.
func (a *nsMutator) InjectDecoder(d *admission.Decoder) error {
a.decoder = d
return nil
}
6 changes: 3 additions & 3 deletions pkg/components/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -1158,11 +1158,11 @@ func InjectVolumesForWebHookCerts(deploy *appsv1.Deployment) {
Items: []corev1.KeyToPath{
{
Key: "tls.crt",
Path: hcov1beta1.WebhookCertName,
Path: hcoutil.WebhookCertName,
},
{
Key: "tls.key",
Path: hcov1beta1.WebhookKeyName,
Path: hcoutil.WebhookKeyName,
},
},
},
Expand All @@ -1174,7 +1174,7 @@ func InjectVolumesForWebHookCerts(deploy *appsv1.Deployment) {
deploy.Spec.Template.Spec.Containers[index].VolumeMounts = append(container.VolumeMounts,
corev1.VolumeMount{
Name: "apiservice-cert",
MountPath: hcov1beta1.DefaultWebhookCertDir,
MountPath: hcoutil.DefaultWebhookCertDir,
})
}
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/util/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ const (
DefaulterWebhookPath = "/mutate-hco-kubevirt-io-v1beta1-hyperconverged"
WebhookPort = 4343

WebhookCertName = "apiserver.crt"
WebhookKeyName = "apiserver.key"
DefaultWebhookCertDir = "/apiserver.local.config/certificates"

CliDownloadsServerPort = 8080
)

Expand Down
120 changes: 120 additions & 0 deletions pkg/webhooks/mutator/mutator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package mutator

import (
"context"
"net/http"

admissionv1 "k8s.io/api/admission/v1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis/hco/v1beta1"
hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
)

const (
ignoreOperationMessage = "ignoring other operations"
admittingDeletionMessage = "the namespace doesn't contain HyperConverged CR, admitting its deletion"
deniedDeletionMessage = "HyperConverged CR is still present, please remove it before deleting the containing namespace"
)

var (
logger = logf.Log.WithName("mutator")

_ admission.Handler = &NsMutator{}
)

// NsMutator mutates Ns requests
type NsMutator struct {
decoder *admission.Decoder
cli client.Client
namespace string
}

func NewNsMutator(cli client.Client, namespace string) *NsMutator {
return &NsMutator{
cli: cli,
namespace: namespace,
}
}

func (nm *NsMutator) Handle(_ context.Context, req admission.Request) admission.Response {
logger.Info("reaching NsMutator.Handle")

if req.Operation == admissionv1.Delete {
return nm.handleNsDelete(req)
}

// ignoring other operations
return admission.Allowed(ignoreOperationMessage)

}

func (nm *NsMutator) handleNsDelete(req admission.Request) admission.Response {
ns := &corev1.Namespace{}

// In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346
// OldObject contains the object being deleted
err := nm.decoder.DecodeRaw(req.OldObject, ns)
if err != nil {
logger.Error(err, "failed decoding namespace object")
return admission.Errored(http.StatusBadRequest, err)
}

admitted, err := nm.handleMutatingNsDelete(ns)
if err != nil {
return admission.Errored(http.StatusInternalServerError, err)
}

if admitted {
return admission.Allowed(admittingDeletionMessage)
}

return admission.Denied(deniedDeletionMessage)
}

// NsMutator implements admission.DecoderInjector.
// A decoder will be automatically injected.

// InjectDecoder injects the decoder.
func (nm *NsMutator) InjectDecoder(d *admission.Decoder) error {
nm.decoder = d
return nil
}

func (nm *NsMutator) handleMutatingNsDelete(ns *corev1.Namespace) (bool, error) {
logger.Info("validating namespace deletion", "name", ns.Name)

if ns.Name != nm.namespace {
logger.Info("ignoring request for a different namespace")
return true, nil
}

ctx := context.TODO()
hco := &v1beta1.HyperConverged{
ObjectMeta: metav1.ObjectMeta{
Name: hcoutil.HyperConvergedName,
Namespace: nm.namespace,
},
}

// Block the deletion if the namespace with a clear error message
// if HCO CR is still there

found := &v1beta1.HyperConverged{}
err := nm.cli.Get(ctx, client.ObjectKeyFromObject(hco), found)
if err != nil {
if apierrors.IsNotFound(err) {
logger.Info("HCO CR doesn't not exist, allow namespace deletion")
return true, nil
}
logger.Error(err, "failed getting HyperConverged CR")
return false, err
}
logger.Info("HCO CR still exists, forbid namespace deletion")
return false, nil
}
Loading

0 comments on commit 997d753

Please sign in to comment.