Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reintroduce 366222d (#1207) #1232

Merged
merged 2 commits into from
Dec 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 38 additions & 41 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,17 @@ 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 @@ -84,7 +83,6 @@ 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 @@ -125,55 +123,61 @@ func main() {
}

if strings.ToLower(os.Getenv("ENABLE_WEBHOOKS")) == "true" {
err = registerValidatingWebhook(&v1beta1.Instance{}, mgr)
if err != nil {
log.Printf("unable to create webhook: %v", err)
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)
os.Exit(1)
}

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

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

// 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())
// 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)
if err != nil {
return err
return fmt.Errorf("failed to generate webhook path: %v", err)
}
validator, ok := obj.(kudo.Validator)
if !ok {
log.Printf("skip registering a validating webhook, kudo.Validator interface is not implemented %v", gvk)

return nil
// 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)
}
vwh := kudo.ValidatingWebhookFor(validator)
if vwh != nil {
path := generateValidatePath(gvk)

// 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)
}
}
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
}

// 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
}

// 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 @@ -184,10 +188,3 @@ 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: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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: 9 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,15 @@ 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 @@ -245,14 +247,17 @@ 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 @@ -270,6 +275,7 @@ 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 @@ -405,6 +411,7 @@ 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 @@ -431,10 +438,12 @@ 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
76 changes: 56 additions & 20 deletions pkg/apis/kudo/v1beta1/instance_validator.go
Original file line number Diff line number Diff line change
@@ -1,42 +1,78 @@
package v1beta1

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

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/api/admission/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/kudobuilder/kudo/pkg/util/kudo"
)

// 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{}
// +k8s:deepcopy-gen=false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual change


// 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
// InstanceValidator holds the data for the instance validator hook
type InstanceValidator struct {
client client.Client
decoder *admission.Decoder
}

// 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)
// 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)
}

// 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)
}

if err := validateUpdate(old, new); err != nil {
return admission.Denied(err.Error())
}
return admission.Allowed("")
default:
return admission.Allowed("")
}
}

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)
}

return nil
}

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

// 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 {
// 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
return nil
}
10 changes: 3 additions & 7 deletions pkg/apis/kudo/v1beta1/instance_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ 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 @@ -34,12 +32,10 @@ func TestValidateUpdate(t *testing.T) {
},
}

req := admission.Request{}

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

for _, tt := range tests {
err := tt.i.ValidateUpdate(&tt.oldInstance, req)
err := validateUpdate(&tt.old, &tt.new)
assert.Equal(t, tt.expectedError, err)
}
}
Loading