Skip to content

Commit

Permalink
Revert PR 1207 (bug) and Regen code (to fix imports) (#1229)
Browse files Browse the repository at this point in the history
* Revert "Refactored instance validation webhook (#1207)"

This reverts commit 366222d.

Signed-off-by: Ken Sipe <kensipe@gmail.com>
  • Loading branch information
kensipe committed Dec 27, 2019
1 parent 01808de commit 854feb2
Show file tree
Hide file tree
Showing 28 changed files with 205 additions and 162 deletions.
79 changes: 41 additions & 38 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,18 @@ import (

apiextenstionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/webhook"

"github.com/kudobuilder/kudo/pkg/apis"
"github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
"github.com/kudobuilder/kudo/pkg/controller/instance"
"github.com/kudobuilder/kudo/pkg/controller/operator"
"github.com/kudobuilder/kudo/pkg/controller/operatorversion"
"github.com/kudobuilder/kudo/pkg/util/kudo"
"github.com/kudobuilder/kudo/pkg/version"
)

Expand Down Expand Up @@ -83,6 +84,7 @@ func main() {
log.Print("Registering Components")

log.Print("setting up scheme")

if err := apis.AddToScheme(mgr.GetScheme()); err != nil {
log.Printf("unable to add APIs to scheme: %v", err)
}
Expand Down Expand Up @@ -123,61 +125,55 @@ func main() {
}

if strings.ToLower(os.Getenv("ENABLE_WEBHOOKS")) == "true" {
log.Printf("Setting up webhooks")

if err := registerWebhook("/validate", &v1beta1.Instance{}, &webhook.Admission{Handler: &v1beta1.InstanceValidator{}}, mgr); err != nil {
log.Printf("unable to create instance validation webhook: %v", err)
err = registerValidatingWebhook(&v1beta1.Instance{}, mgr)
if err != nil {
log.Printf("unable to create webhook: %v", err)
os.Exit(1)
}

// Add more webhooks below using the above registerWebhook method
}

// Start the KUDO manager
log.Print("Starting KUDO manager")
// Start the Cmd
log.Print("Starting the Cmd")
if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
log.Printf("unable to run the manager: %v", err)
os.Exit(1)
}
}

// registerWebhook method registers passed webhook using a give prefix (e.g. "/validate") and runtime object
// (e.g. v1beta1.Instance) to generate a webhook path e.g. "/validate-kudo-dev-v1beta1-instances". Webhook
// has to implement http.Handler interface (see v1beta1.InstanceValidator for an example)
func registerWebhook(prefix string, obj runtime.Object, hook http.Handler, mgr manager.Manager) error {
path, err := webhookPath(prefix, obj, mgr)
// this is a fork of a code in controller-runtime to be able to pass in our own Validator interface
// see kudo.Validator docs for more details\
//
// ideally in the future we should switch to just simply doing
// err = ctrl.NewWebhookManagedBy(mgr).
// For(&v1beta1.Instance{}).
// Complete()
//
// that internally calls this method but using their own internal Validator type
func registerValidatingWebhook(obj runtime.Object, mgr manager.Manager) error {
gvk, err := apiutil.GVKForObject(obj, mgr.GetScheme())
if err != nil {
return fmt.Errorf("failed to generate webhook path: %v", err)
return err
}
validator, ok := obj.(kudo.Validator)
if !ok {
log.Printf("skip registering a validating webhook, kudo.Validator interface is not implemented %v", gvk)

// an already handled path is likely a misconfiguration that should be fixed. failing loud and proud
if isAlreadyHandled(path, mgr) {
return fmt.Errorf("webhook path %s is already handled", path)
return nil
}
vwh := kudo.ValidatingWebhookFor(validator)
if vwh != nil {
path := generateValidatePath(gvk)

mgr.GetWebhookServer().Register(path, hook)

return nil
}

// webhookPath method generates a unique path for a given prefix and object GVK of the form:
// /validate-kudo-dev-v1beta1-instances
// Not that object version is included in the path since different object versions can have
// different webhooks. If the strategy to generate this path changes we should update init
// code and webhook setup. Right now this is in sync how controller-runtime generates these paths
func webhookPath(prefix string, obj runtime.Object, mgr manager.Manager) (string, error) {
gvk, err := apiutil.GVKForObject(obj, mgr.GetScheme())
if err != nil {
return "", err
// Checking if the path is already registered.
// If so, just skip it.
if !isAlreadyHandled(path, mgr) {
log.Printf("Registering a validating webhook for %v on path %s", gvk, path)
mgr.GetWebhookServer().Register(path, vwh)
}
}

// if the strategy to generate this path changes we should update init code and webhook setup
// right now this is in sync how controller-runtime generates these paths
return prefix + "-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" +
gvk.Version + "-" + strings.ToLower(gvk.Kind), nil
return nil
}

// isAlreadyHandled method check if there is already an admission webhook registered for a given path
func isAlreadyHandled(path string, mgr manager.Manager) bool {
if mgr.GetWebhookServer().WebhookMux == nil {
return false
Expand All @@ -188,3 +184,10 @@ func isAlreadyHandled(path string, mgr manager.Manager) bool {
}
return false
}

// if the strategy to generate this path changes we should update init code and webhook setup
// right now this is in sync how controller-runtime generates these paths
func generateValidatePath(gvk schema.GroupVersionKind) string {
return "/validate-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" +
gvk.Version + "-" + strings.ToLower(gvk.Kind)
}
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ require (
github.com/Masterminds/sprig v2.22.0+incompatible
github.com/Microsoft/go-winio v0.4.14 // indirect
github.com/containerd/containerd v1.2.9 // indirect
github.com/coreos/etcd v3.3.15+incompatible
github.com/docker/docker v1.4.2-0.20190916154449-92cc603036dd
github.com/docker/go-connections v0.4.0 // indirect
github.com/docker/go-units v0.4.0 // indirect
Expand Down
9 changes: 0 additions & 9 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,13 @@ github.com/client9/misspell v0.3.4 h1:ta993UF76GwbvJcIo3Y68y/M3WxlpEHPWIGDkJYwzJ
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/containerd/containerd v1.2.9 h1:6tyNjBmAMG47QuFPIT9LgiiexoVxC6qpTGR+eD0R0Z8=
github.com/containerd/containerd v1.2.9/go.mod h1:bC6axHOhabU15QhwfG7w5PipXdVtMXFTttgp+kVtyUA=
github.com/coreos/bbolt v1.3.1-coreos.6 h1:uTXKg9gY70s9jMAKdfljFQcuh4e/BXOM+V+d00KFj3A=
github.com/coreos/bbolt v1.3.1-coreos.6/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/etcd v3.3.15+incompatible h1:+9RjdC18gMxNQVvSiXvObLu29mOFmkgdsB4cRTlV+EE=
github.com/coreos/etcd v3.3.15+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
github.com/coreos/go-oidc v2.1.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/coreos/go-semver v0.3.0 h1:wkHLiw0WNATZnSG7epLsujiMCgPAc9xhjJ4tgnAxmfM=
github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7 h1:u9SHYsPQNyt5tgDm3YN7+9dYrpK96E5wFilTFWIDZOM=
github.com/coreos/go-systemd v0.0.0-20180511133405-39ca1b05acc7/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
Expand Down Expand Up @@ -247,17 +245,14 @@ github.com/gorilla/context v1.1.1 h1:AWwleXJkX/nhcU9bZSnZoi3h/qGYqQAGhq6zZe/aQW8
github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg=
github.com/gorilla/mux v1.6.2 h1:Pgr17XVTNXAk3q/r4CpKzC5xBM/qW1uVLV+IhRZpIIk=
github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
github.com/gorilla/websocket v1.4.0 h1:WDFjx/TMzVgy9VdMMQi2K2Emtwi2QcUQsztZ/zLaH/Q=
github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ=
github.com/gosuri/uitable v0.0.4 h1:IG2xLKRvErL3uhY6e1BylFzG+aJiwQviDDTfOKeKTpY=
github.com/gosuri/uitable v0.0.4/go.mod h1:tKR86bXuXPZazfOTG1FIzvjIdXzd0mo4Vtn16vt0PJo=
github.com/gregjones/httpcache v0.0.0-20170728041850-787624de3eb7 h1:6TSoaYExHper8PYsJu23GWVNOyYRCSnIFyxKgLSZ54w=
github.com/gregjones/httpcache v0.0.0-20170728041850-787624de3eb7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA=
github.com/grpc-ecosystem/go-grpc-middleware v0.0.0-20190222133341-cfaf5686ec79 h1:lR9ssWAqp9qL0bALxqEEkuudiP1eweOdv9jsRK3e7lE=
github.com/grpc-ecosystem/go-grpc-middleware v0.0.0-20190222133341-cfaf5686ec79/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs=
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho=
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk=
github.com/grpc-ecosystem/grpc-gateway v1.3.0 h1:HJtP6RRwj2EpPCD/mhAWzSvLL/dFTdPm1UrWwanoFos=
github.com/grpc-ecosystem/grpc-gateway v1.3.0/go.mod h1:RSKVYQBd5MCa4OVpNdGskqpgL2+G+NZTnrVHpWWfpdw=
github.com/hashicorp/golang-lru v0.5.0 h1:CL2msUPvZTLb5O648aiLNJw3hnBxN2+1Jq8rCOH9wdo=
github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
Expand All @@ -275,7 +270,6 @@ github.com/imdario/mergo v0.3.7 h1:Y+UAYTZ7gDEuOfhxKWy+dvb5dRQ6rJjFSdX2HZY1/gI=
github.com/imdario/mergo v0.3.7/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/jonboulle/clockwork v0.1.0 h1:VKV+ZcuP6l3yW9doeqz6ziZGgcynBVQO+obU0+0hcPo=
github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=
github.com/json-iterator/go v0.0.0-20180612202835-f2b4162afba3/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
github.com/json-iterator/go v1.1.6 h1:MrUvLMLTMxbqFJ9kzlvat/rYZqZnW3u4wkLzWTaFwKs=
Expand Down Expand Up @@ -411,7 +405,6 @@ github.com/sirupsen/logrus v1.4.1 h1:GL2rEmy6nsikmW0r8opw9JIRScdMF5hA8cOYLH7In1k
github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q=
github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4=
github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
github.com/soheilhy/cmux v0.1.3 h1:09wy7WZk4AqO03yH85Ex1X+Uo3vDsil3Fa9AgF8Emss=
github.com/soheilhy/cmux v0.1.3/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4kGIyLM=
github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ=
github.com/spf13/afero v1.2.2 h1:5jhuqJyZCZf2JRofRvN/nIFgIWNzPa3/Vz8mYylgbWc=
Expand All @@ -438,12 +431,10 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8 h1:ndzgwNDnKIqyCvHTXaCqh9KlOWKvBry6nuXMJmonVsE=
github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/tsenart/deadcode v0.0.0-20160724212837-210d2dc333e9 h1:vY5WqiEon0ZSTGM3ayVVi+twaHKHDFUVloaQ/wug9/c=
github.com/tsenart/deadcode v0.0.0-20160724212837-210d2dc333e9/go.mod h1:q+QjxYvZ+fpjMXqs+XEriussHjSYqeXVnAdSV1tkMYk=
github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
github.com/xiang90/probing v0.0.0-20160813154853-07dd2e8dfe18 h1:MPPkRncZLN9Kh4MEFmbnK4h3BD7AUmskWv2+EeZJCCs=
github.com/xiang90/probing v0.0.0-20160813154853-07dd2e8dfe18/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca h1:1CFlNzQhALwjS9mBAUkycX616GzgsuYUOCHA5+HSlXI=
github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca/go.mod h1:ce1O1j6UtZfjr22oyGxGLbauSBp2YVXpARAosm7dHBg=
Expand Down
73 changes: 20 additions & 53 deletions pkg/apis/kudo/v1beta1/instance_validator.go
Original file line number Diff line number Diff line change
@@ -1,75 +1,42 @@
package v1beta1

import (
"context"
"fmt"
"net/http"
"reflect"

"github.com/coreos/etcd/client"
"k8s.io/api/admission/v1beta1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

type InstanceValidator struct {
client client.Client
decoder *admission.Decoder
}

// InstanceValidator validates updates to an Instance, guarding from conflicting plan executions
func (v *InstanceValidator) Handle(ctx context.Context, req admission.Request) admission.Response {

switch req.Operation {
// we only validate Instance Updates
case v1beta1.Update:
old, new := &Instance{}, &Instance{}

// req.Object contains the updated object
if err := v.decoder.DecodeRaw(req.Object, new); err != nil {
return admission.Errored(http.StatusBadRequest, err)
}
"github.com/kudobuilder/kudo/pkg/util/kudo"
)

// req.OldObject is populated for DELETE and UPDATE requests
if err := v.decoder.DecodeRaw(req.OldObject, old); err != nil {
return admission.Errored(http.StatusBadRequest, err)
}
// this forces the instance type to implement Validator interface, we'll get compile time error if it's not true anymore
var _ kudo.Validator = &Instance{}

if err := validateUpdate(old, new); err != nil {
return admission.Denied(err.Error())
}
return admission.Allowed("")
default:
return admission.Allowed("")
}
// ValidateCreate implements kudo.Validator (slightly tweaked interface originally from controller-runtime)
// we do not enforce any rules upon creation right now
func (i *Instance) ValidateCreate(req admission.Request) error {
return nil
}

func validateUpdate(old, new *Instance) error {
// Disallow spec updates when a plan is in progress
if old.Status.AggregatedStatus.Status.IsRunning() && specChanged(old.Spec, new.Spec) {
return fmt.Errorf("cannot update Instance %s/%s right now, there's plan %s in progress", old.Namespace, old.Name, old.Status.AggregatedStatus.ActivePlanName)
// ValidateUpdate hook called when UPDATE operation is triggered and our admission webhook is triggered
// ValidateUpdate implements kudo.Validator (slightly tweaked interface originally from controller-runtime)
func (i *Instance) ValidateUpdate(old runtime.Object, req admission.Request) error {
oldInstance := old.(*Instance)
if i.Status.AggregatedStatus.Status.IsRunning() && specChanged(i.Spec, oldInstance.Spec) {
// when updating anything else than status, there shouldn't be a running plan
return fmt.Errorf("cannot update Instance %s/%s right now, there's plan %s in progress", i.Namespace, i.Name, i.Status.AggregatedStatus.ActivePlanName)
}

return nil
}

func specChanged(old InstanceSpec, new InstanceSpec) bool {
return !reflect.DeepEqual(old, new)
}

// InstanceValidator implements inject.Client.
// A client will be automatically injected.

// InjectClient injects the client.
func (v *InstanceValidator) InjectClient(c client.Client) error {
v.client = c
return nil
}

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

// InjectDecoder injects the decoder.
func (v *InstanceValidator) InjectDecoder(d *admission.Decoder) error {
v.decoder = d
// ValidateDelete hook called when DELETE operation is triggered and our admission webhook is triggered
// we don't enforce any validation on DELETE right now
// ValidateDelete implements kudo.Validator (slightly tweaked interface originally from controller-runtime)
func (i *Instance) ValidateDelete(req admission.Request) error {
return nil
}
10 changes: 7 additions & 3 deletions pkg/apis/kudo/v1beta1/instance_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -32,10 +34,12 @@ func TestValidateUpdate(t *testing.T) {
},
}

req := admission.Request{}

tests := []struct {
name string
new Instance
old Instance
i Instance
oldInstance Instance
expectedError error
}{
{"no change", runningInstance, runningInstance, nil},
Expand All @@ -52,7 +56,7 @@ func TestValidateUpdate(t *testing.T) {
}

for _, tt := range tests {
err := validateUpdate(&tt.old, &tt.new)
err := tt.i.ValidateUpdate(&tt.oldInstance, req)
assert.Equal(t, tt.expectedError, err)
}
}
5 changes: 5 additions & 0 deletions pkg/apis/kudo/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions pkg/client/clientset/versioned/clientset.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 3 additions & 4 deletions pkg/client/clientset/versioned/fake/clientset_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions pkg/client/clientset/versioned/fake/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 854feb2

Please sign in to comment.