try to fix tenant webhooks

Author: QcFe

operators/cmd/tenant-operator/main.go

@@ -150,8 +150,14 @@ func main() {
 	if *enableWH {
 		hookServer := mgr.GetWebhookServer()
 		webhookBypassGroupsList := strings.Split(webhookBypassGroups, ",")
-		hookServer.Register(ValidatingWebhookPath, tenantwh.MakeTenantValidator(mgr.GetClient(), webhookBypassGroupsList))
-		hookServer.Register(MutatingWebhookPath, tenantwh.MakeTenantMutator(mgr.GetClient(), webhookBypassGroupsList, targetLabelKey, targetLabelValue, baseWorkspacesList))
+		hookServer.Register(
+			ValidatingWebhookPath,
+			tenantwh.MakeTenantValidator(mgr.GetClient(), webhookBypassGroupsList, mgr.GetScheme()),
+		)
+		hookServer.Register(
+			MutatingWebhookPath,
+			tenantwh.MakeTenantMutator(mgr.GetClient(), webhookBypassGroupsList, targetLabelKey, targetLabelValue, baseWorkspacesList, mgr.GetScheme()),
+		)
 	} else {
 		log.Info("Webhook set up: operation skipped")
 	}

operators/pkg/tenantwh/common.go

@@ -16,6 +16,7 @@ package tenantwh
 
 import (
 	"context"
+	"errors"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -38,14 +39,11 @@ func (twh *TenantWebhook) CheckWebhookOverride(req *admission.Request) bool {
 	return utils.MatchOneInStringSlices(twh.BypassGroups, req.UserInfo.Groups)
 }
 
-// InjectDecoder injects the decoder - this method is used by controller runtime.
-func (twh *TenantWebhook) InjectDecoder(d *admission.Decoder) error {
-	twh.decoder = d
-	return nil
-}
-
 // DecodeTenant decodes the tenant from the incoming request.
 func (twh *TenantWebhook) DecodeTenant(obj runtime.RawExtension) (tenant *clv1alpha2.Tenant, err error) {
+	if twh.decoder == nil {
+		return nil, errors.New("missing decoder")
+	}
 	tenant = &clv1alpha2.Tenant{}
 	err = twh.decoder.DecodeRaw(obj, tenant)
 	return

operators/pkg/tenantwh/mutating.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 
 	admissionv1 "k8s.io/api/admission/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -38,10 +39,14 @@ type TenantMutator struct {
 }
 
 // MakeTenantMutator creates a new webhook handler suitable for controller runtime based on TenantMutator.
-func MakeTenantMutator(c client.Client, webhookBypassGroups []string, opSelectorKey, opSelectorValue string, baseWorkspaces []string) *webhook.Admission {
+func MakeTenantMutator(c client.Client, webhookBypassGroups []string, opSelectorKey, opSelectorValue string, baseWorkspaces []string, scheme *runtime.Scheme) *webhook.Admission {
 	return &webhook.Admission{Handler: &TenantMutator{
 		opSelectorKey, opSelectorValue, baseWorkspaces,
-		TenantWebhook{Client: c, BypassGroups: webhookBypassGroups},
+		TenantWebhook{
+			Client:       c,
+			BypassGroups: webhookBypassGroups,
+			decoder:      admission.NewDecoder(scheme),
+		},
 	}}
 }
 

operators/pkg/tenantwh/mutating_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package tenantwh_test
+package tenantwh
 
 import (
 	"net/http"
@@ -26,12 +26,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	clv1alpha2 "github.com/netgroup-polito/CrownLabs/operators/api/v1alpha2"
-	"github.com/netgroup-polito/CrownLabs/operators/pkg/tenantwh"
 )
 
 var _ = Describe("Mutating webhook", func() {
 	var (
-		mutatingWH *tenantwh.TenantMutator
+		mutatingWH *TenantMutator
 		request    admission.Request
 
 		opSelectorKey   = "crownlabs.polito.it/op-sel"
@@ -53,8 +52,8 @@ var _ = Describe("Mutating webhook", func() {
 
 	JustBeforeEach(func() {
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
-		mutatingWH = tenantwh.MakeTenantMutator(fakeClient, bypassGroups, opSelectorKey, opSelectorValue, baseWorkspaces).Handler.(*tenantwh.TenantMutator)
-		Expect(mutatingWH.InjectDecoder(decoder)).To(Succeed())
+		mutatingWH = MakeTenantMutator(fakeClient, bypassGroups, opSelectorKey, opSelectorValue, baseWorkspaces, scheme).Handler.(*TenantMutator)
+		Expect(mutatingWH.decoder).NotTo(BeNil())
 	})
 
 	Describe("The TenantMutator.Handle method", func() {

operators/pkg/tenantwh/tenantwh_suite_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package tenantwh_test
+package tenantwh
 
 import (
 	"context"
@@ -30,9 +30,8 @@ import (
 )
 
 var (
-	scheme  *runtime.Scheme
-	decoder *admission.Decoder
-	ctx     = context.Background()
+	scheme *runtime.Scheme
+	ctx    = context.Background()
 
 	bypassGroups   = []string{"admins"}
 	testTenantName = "test-tenant"
@@ -43,7 +42,6 @@ var _ = BeforeSuite(func() {
 	scheme = runtime.NewScheme()
 	Expect(clv1alpha1.AddToScheme(scheme)).To(Succeed())
 	Expect(clv1alpha2.AddToScheme(scheme)).To(Succeed())
-	decoder = admission.NewDecoder(scheme)
 })
 
 func TestTenantWebHooks(t *testing.T) {

operators/pkg/tenantwh/validating.go

@@ -25,6 +25,7 @@ import (
 
 	admissionv1 "k8s.io/api/admission/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"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -42,10 +43,11 @@ const LastLoginToleration = time.Hour * 24
 type TenantValidator struct{ TenantWebhook }
 
 // MakeTenantValidator creates a new webhook handler suitable for controller runtime based on TenantValidator.
-func MakeTenantValidator(c client.Client, webhookBypassGroups []string) *webhook.Admission {
+func MakeTenantValidator(c client.Client, webhookBypassGroups []string, scheme *runtime.Scheme) *webhook.Admission {
 	return &webhook.Admission{Handler: &TenantValidator{TenantWebhook{
 		Client:       c,
 		BypassGroups: webhookBypassGroups,
+		decoder:      admission.NewDecoder(scheme),
 	}}}
 }
 

operators/pkg/tenantwh/validating_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package tenantwh_test
+package tenantwh
 
 import (
 	"net/http"
@@ -27,7 +27,6 @@ import (
 
 	clv1alpha1 "github.com/netgroup-polito/CrownLabs/operators/api/v1alpha1"
 	clv1alpha2 "github.com/netgroup-polito/CrownLabs/operators/api/v1alpha2"
-	"github.com/netgroup-polito/CrownLabs/operators/pkg/tenantwh"
 )
 
 var _ = Describe("Validating webhook", func() {
@@ -47,7 +46,7 @@ var _ = Describe("Validating webhook", func() {
 	}
 
 	var (
-		validatingWH *tenantwh.TenantValidator
+		validatingWH *TenantValidator
 		request      admission.Request
 		response     admission.Response
 		manager      *clv1alpha2.Tenant
@@ -111,8 +110,9 @@ var _ = Describe("Validating webhook", func() {
 			workspaceIM,
 		).Build()
 
-		validatingWH = tenantwh.MakeTenantValidator(fakeClient, bypassGroups).Handler.(*tenantwh.TenantValidator)
-		Expect(validatingWH.InjectDecoder(decoder)).To(Succeed())
+		validatingWH = MakeTenantValidator(fakeClient, bypassGroups, scheme).Handler.(*TenantValidator)
+
+		Expect(validatingWH.decoder).NotTo(BeNil())
 	})
 
 	Describe("The TenantValidator.Handle method", func() {
@@ -185,7 +185,7 @@ var _ = Describe("Validating webhook", func() {
 			BeforeEach(func() {
 				newTenant = &clv1alpha2.Tenant{Spec: clv1alpha2.TenantSpec{
 					LastLogin: metav1.Time{
-						Time: time.Now().Add(tenantwh.LastLoginToleration / 2),
+						Time: time.Now().Add(LastLoginToleration / 2),
 					},
 				}}
 			})
@@ -198,7 +198,7 @@ var _ = Describe("Validating webhook", func() {
 			BeforeEach(func() {
 				newTenant = &clv1alpha2.Tenant{Spec: clv1alpha2.TenantSpec{
 					LastLogin: metav1.Time{
-						Time: time.Now().Add(tenantwh.LastLoginToleration + time.Millisecond),
+						Time: time.Now().Add(LastLoginToleration + time.Millisecond),
 					},
 				}}
 			})
@@ -372,7 +372,7 @@ var _ = Describe("Validating webhook", func() {
 		WhenBody := func(cwdc CalcWsDiffCase) func() {
 			return func() {
 				var actuals []string
-				result := tenantwh.CalculateWorkspacesDiff(
+				result := CalculateWorkspacesDiff(
 					&clv1alpha2.Tenant{Spec: clv1alpha2.TenantSpec{Workspaces: cwdc.a}},
 					&clv1alpha2.Tenant{Spec: clv1alpha2.TenantSpec{Workspaces: cwdc.b}},
 				)