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

[CONTP-356] feat(admission server): implement ValidatingAdmissionWebhook #28512

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Next Next commit
chore(admission server): make controller mutating specific
Signed-off-by: Wassim DHIF <wassim.dhif@datadoghq.com>
  • Loading branch information
wdhif committed Sep 25, 2024
commit 1308982e043a9a0805693726192db023c99bedc2
12 changes: 6 additions & 6 deletions cmd/cluster-agent/admission/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ type MutateRequest struct {
APIClient kubernetes.Interface
}

// WebhookFunc is the function that runs the webhook logic
type WebhookFunc func(request *MutateRequest) ([]byte, error)
// MutatingWebhookFunc is the function that runs the mutating webhook logic
type MutatingWebhookFunc func(request *MutateRequest) ([]byte, error)

// Server TODO <container-integrations>
type Server struct {
Expand Down Expand Up @@ -94,7 +94,7 @@ func (s *Server) initDecoder() {

// Register adds an admission webhook handler.
// Register must be called to register the desired webhook handlers before calling Run.
func (s *Server) Register(uri string, webhookName string, f WebhookFunc, dc dynamic.Interface, apiClient kubernetes.Interface) {
func (s *Server) Register(uri string, webhookName string, f MutatingWebhookFunc, dc dynamic.Interface, apiClient kubernetes.Interface) {
s.mux.HandleFunc(uri, func(w http.ResponseWriter, r *http.Request) {
s.mutateHandler(w, r, webhookName, f, dc, apiClient)
})
Expand Down Expand Up @@ -138,12 +138,12 @@ func (s *Server) Run(mainCtx context.Context, client kubernetes.Interface) error

// mutateHandler contains the main logic responsible for handling mutation requests.
// It supports both v1 and v1beta1 requests.
func (s *Server) mutateHandler(w http.ResponseWriter, r *http.Request, webhookName string, mutateFunc WebhookFunc, dc dynamic.Interface, apiClient kubernetes.Interface) {
metrics.WebhooksReceived.Inc(webhookName)
func (s *Server) mutateHandler(w http.ResponseWriter, r *http.Request, webhookName string, mutateFunc MutatingWebhookFunc, dc dynamic.Interface, apiClient kubernetes.Interface) {
metrics.MutatingWebhooksReceived.Inc(webhookName)

start := time.Now()
defer func() {
metrics.WebhooksResponseDuration.Observe(time.Since(start).Seconds(), webhookName)
metrics.MutatingWebhooksResponseDuration.Observe(time.Since(start).Seconds(), webhookName)
}()

if r.Method != http.MethodPost {
Expand Down
4 changes: 2 additions & 2 deletions cmd/cluster-agent/subcommands/start/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,15 +469,15 @@ func start(log log.Component,
StopCh: stopCh,
}

webhooks, err := admissionpkg.StartControllers(admissionCtx, wmeta, pa)
mutatingWebhooks, err := admissionpkg.StartControllers(admissionCtx, wmeta, pa)
if err != nil {
pkglog.Errorf("Could not start admission controller: %v", err)
} else {
// Webhook and secret controllers are started successfully
// Setup the k8s admission webhook server
server := admissioncmd.NewServer()

for _, webhookConf := range webhooks {
for _, webhookConf := range mutatingWebhooks {
server.Register(webhookConf.Endpoint(), webhookConf.Name(), webhookConf.MutateFunc(), apiCl.DynamicCl, apiCl.Cl)
}

Expand Down
26 changes: 13 additions & 13 deletions pkg/clusteragent/admission/controllers/webhook/controller_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
// Controller is an interface implemented by ControllerV1 and ControllerV1beta1.
type Controller interface {
Run(stopCh <-chan struct{})
EnabledWebhooks() []MutatingWebhook
EnabledMutatingWebhooks() []MutatingWebhook
}

// NewController returns the adequate implementation of the Controller interface.
Expand Down Expand Up @@ -76,7 +76,7 @@ type MutatingWebhook interface {
// should be invoked
LabelSelectors(useNamespaceSelector bool) (namespaceSelector *metav1.LabelSelector, objectSelector *metav1.LabelSelector)
// MutateFunc returns the function that mutates the resources
MutateFunc() admission.WebhookFunc
MutateFunc() admission.MutatingWebhookFunc
}

// mutatingWebhooks returns the list of mutating webhooks. Notice that the order
Expand Down Expand Up @@ -119,19 +119,19 @@ func mutatingWebhooks(wmeta workloadmeta.Component, pa workload.PodPatcher) []Mu
// It contains the shared fields and provides shared methods.
// For the nolint:structcheck see https://github.com/golangci/golangci-lint/issues/537
type controllerBase struct {
clientSet kubernetes.Interface //nolint:structcheck
config Config
secretsLister corelisters.SecretLister
secretsSynced cache.InformerSynced //nolint:structcheck
webhooksSynced cache.InformerSynced //nolint:structcheck
queue workqueue.RateLimitingInterface
isLeaderFunc func() bool
isLeaderNotif <-chan struct{}
mutatingWebhooks []MutatingWebhook
clientSet kubernetes.Interface //nolint:structcheck
config Config
secretsLister corelisters.SecretLister
secretsSynced cache.InformerSynced //nolint:structcheck
mutatingWebhooksSynced cache.InformerSynced //nolint:structcheck
queue workqueue.RateLimitingInterface
isLeaderFunc func() bool
isLeaderNotif <-chan struct{}
mutatingWebhooks []MutatingWebhook
}

// EnabledWebhooks returns the list of enabled webhooks.
func (c *controllerBase) EnabledWebhooks() []MutatingWebhook {
// EnabledMutatingWebhooks returns the list of enabled mutating webhooks.
func (c *controllerBase) EnabledMutatingWebhooks() []MutatingWebhook {
var res []MutatingWebhook

for _, webhook := range c.mutatingWebhooks {
Expand Down
56 changes: 28 additions & 28 deletions pkg/clusteragent/admission/controllers/webhook/controller_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ import (
// It uses the admissionregistration/v1 API.
type ControllerV1 struct {
controllerBase
webhooksLister admissionlisters.MutatingWebhookConfigurationLister
webhookTemplates []admiv1.MutatingWebhook
mutatingWebhooksLister admissionlisters.MutatingWebhookConfigurationLister
mutatingWebhookTemplates []admiv1.MutatingWebhook
}

// NewControllerV1 returns a new Webhook Controller using admissionregistration/v1.
func NewControllerV1(
client kubernetes.Interface,
secretInformer coreinformers.SecretInformer,
webhookInformer admissioninformers.MutatingWebhookConfigurationInformer,
MutatingWebhookInformer admissioninformers.MutatingWebhookConfigurationInformer,
isLeaderFunc func() bool,
isLeaderNotif <-chan struct{},
config Config,
Expand All @@ -55,8 +55,8 @@ func NewControllerV1(
controller.config = config
controller.secretsLister = secretInformer.Lister()
controller.secretsSynced = secretInformer.Informer().HasSynced
controller.webhooksLister = webhookInformer.Lister()
controller.webhooksSynced = webhookInformer.Informer().HasSynced
controller.mutatingWebhooksLister = MutatingWebhookInformer.Lister()
controller.mutatingWebhooksSynced = MutatingWebhookInformer.Informer().HasSynced
controller.queue = workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "webhooks")
controller.isLeaderFunc = isLeaderFunc
controller.isLeaderNotif = isLeaderNotif
Expand All @@ -71,9 +71,9 @@ func NewControllerV1(
log.Errorf("cannot add event handler to secret informer: %v", err)
}

if _, err := webhookInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
if _, err := MutatingWebhookInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: controller.handleWebhook,
UpdateFunc: controller.handleWebhookUpdate,
UpdateFunc: controller.handleMutatingWebhookUpdate,
DeleteFunc: controller.handleWebhook,
}); err != nil {
log.Errorf("cannot add event handler to webhook informer: %v", err)
Expand All @@ -90,7 +90,7 @@ func (c *ControllerV1) Run(stopCh <-chan struct{}) {
log.Infof("Starting webhook controller for secret %s/%s and webhook %s - Using admissionregistration/v1", c.config.getSecretNs(), c.config.getSecretName(), c.config.getWebhookName())
defer log.Infof("Stopping webhook controller for secret %s/%s and webhook %s", c.config.getSecretNs(), c.config.getSecretName(), c.config.getWebhookName())

if ok := cache.WaitForCacheSync(stopCh, c.secretsSynced, c.webhooksSynced); !ok {
if ok := cache.WaitForCacheSync(stopCh, c.secretsSynced, c.mutatingWebhooksSynced); !ok {
return
}

Expand All @@ -109,9 +109,9 @@ func (c *ControllerV1) run() {
}
}

// handleWebhookUpdate handles the new Webhook reported in update events.
// handleMutatingWebhookUpdate handles the new Webhook reported in update events.
// It can be a callback function for update events.
func (c *ControllerV1) handleWebhookUpdate(oldObj, newObj interface{}) {
func (c *ControllerV1) handleMutatingWebhookUpdate(oldObj, newObj interface{}) {
if !c.isLeaderFunc() {
return
}
Expand Down Expand Up @@ -142,51 +142,51 @@ func (c *ControllerV1) reconcile() error {
return err
}

webhook, err := c.webhooksLister.Get(c.config.getWebhookName())
mutatingWebhook, err := c.mutatingWebhooksLister.Get(c.config.getWebhookName())
Copy link
Member

Choose a reason for hiding this comment

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

This err overrides the one for the validating webhooks lister. So, if there was an error for those, we won't return it. Is this intentional?

if err != nil {
if errors.IsNotFound(err) {
log.Infof("Webhook %s was not found, creating it", c.config.getWebhookName())
return c.createWebhook(secret)
log.Infof("Mutating Webhook %s was not found, creating it", c.config.getWebhookName())
return c.createMutatingWebhook(secret)
}

return err
}

log.Debugf("The Webhook %s was found, updating it", c.config.getWebhookName())
log.Debugf("Mutating Webhook %s was found, updating it", c.config.getWebhookName())

return c.updateWebhook(secret, webhook)
return c.updateMutatingWebhook(secret, mutatingWebhook)
}

// createWebhook creates a new MutatingWebhookConfiguration object.
func (c *ControllerV1) createWebhook(secret *corev1.Secret) error {
// createMutatingWebhook creates a new MutatingWebhookConfiguration object.
func (c *ControllerV1) createMutatingWebhook(secret *corev1.Secret) error {
webhook := &admiv1.MutatingWebhookConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: c.config.getWebhookName(),
},
Webhooks: c.newWebhooks(secret),
Webhooks: c.newMutatingWebhooks(secret),
}

_, err := c.clientSet.AdmissionregistrationV1().MutatingWebhookConfigurations().Create(context.TODO(), webhook, metav1.CreateOptions{})
if errors.IsAlreadyExists(err) {
log.Infof("Webhook %s already exists", webhook.GetName())
log.Infof("Mutating Webhook %s already exists", webhook.GetName())
return nil
}

return err
}

// updateWebhook stores a new configuration in the MutatingWebhookConfiguration object.
func (c *ControllerV1) updateWebhook(secret *corev1.Secret, webhook *admiv1.MutatingWebhookConfiguration) error {
// updateMutatingWebhook stores a new configuration in the MutatingWebhookConfiguration object.
func (c *ControllerV1) updateMutatingWebhook(secret *corev1.Secret, webhook *admiv1.MutatingWebhookConfiguration) error {
webhook = webhook.DeepCopy()
webhook.Webhooks = c.newWebhooks(secret)
webhook.Webhooks = c.newMutatingWebhooks(secret)
_, err := c.clientSet.AdmissionregistrationV1().MutatingWebhookConfigurations().Update(context.TODO(), webhook, metav1.UpdateOptions{})
return err
}

// newWebhooks generates MutatingWebhook objects from config templates with updated CABundle from Secret.
func (c *ControllerV1) newWebhooks(secret *corev1.Secret) []admiv1.MutatingWebhook {
// newMutatingWebhooks generates MutatingWebhook objects from config templates with updated CABundle from Secret.
func (c *ControllerV1) newMutatingWebhooks(secret *corev1.Secret) []admiv1.MutatingWebhook {
webhooks := []admiv1.MutatingWebhook{}
for _, tpl := range c.webhookTemplates {
for _, tpl := range c.mutatingWebhookTemplates {
tpl.ClientConfig.CABundle = certificate.GetCABundle(secret.Data)
webhooks = append(webhooks, tpl)
}
Expand All @@ -206,7 +206,7 @@ func (c *ControllerV1) generateTemplates() {

webhooks = append(
webhooks,
c.getWebhookSkeleton(
c.getMutatingWebhookSkeleton(
webhook.Name(),
webhook.Endpoint(),
webhook.Operations(),
Expand All @@ -217,10 +217,10 @@ func (c *ControllerV1) generateTemplates() {
)
}

c.webhookTemplates = webhooks
c.mutatingWebhookTemplates = webhooks
}

func (c *ControllerV1) getWebhookSkeleton(nameSuffix, path string, operations []admiv1.OperationType, resources []string, namespaceSelector, objectSelector *metav1.LabelSelector) admiv1.MutatingWebhook {
func (c *ControllerV1) getMutatingWebhookSkeleton(nameSuffix, path string, operations []admiv1.OperationType, resources []string, namespaceSelector, objectSelector *metav1.LabelSelector) admiv1.MutatingWebhook {
matchPolicy := admiv1.Exact
sideEffects := admiv1.SideEffectClassNone
port := c.config.getServicePort()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestSecretNotFoundV1(t *testing.T) {
defer close(stopCh)
c := f.run(stopCh)

_, err := c.webhooksLister.Get(v1Cfg.getWebhookName())
_, err := c.mutatingWebhooksLister.Get(v1Cfg.getWebhookName())
if !errors.IsNotFound(err) {
t.Fatal("Webhook shouldn't be created")
}
Expand Down Expand Up @@ -80,7 +80,7 @@ func TestCreateWebhookV1(t *testing.T) {

var webhook *admiv1.MutatingWebhookConfiguration
require.Eventually(t, func() bool {
webhook, err = c.webhooksLister.Get(v1Cfg.getWebhookName())
webhook, err = c.mutatingWebhooksLister.Get(v1Cfg.getWebhookName())
return err == nil
}, waitFor, tick)

Expand Down Expand Up @@ -129,7 +129,7 @@ func TestUpdateOutdatedWebhookV1(t *testing.T) {

var newWebhook *admiv1.MutatingWebhookConfiguration
require.Eventually(t, func() bool {
newWebhook, err = c.webhooksLister.Get(v1Cfg.getWebhookName())
newWebhook, err = c.mutatingWebhooksLister.Get(v1Cfg.getWebhookName())
return err == nil && !reflect.DeepEqual(webhook, newWebhook)
}, waitFor, tick)

Expand Down Expand Up @@ -965,7 +965,7 @@ func TestGenerateTemplatesV1(t *testing.T) {
c.mutatingWebhooks = mutatingWebhooks(wmeta, nil)
c.generateTemplates()

assert.EqualValues(t, tt.want(), c.webhookTemplates)
assert.EqualValues(t, tt.want(), c.mutatingWebhookTemplates)
})
}
}
Expand Down
Loading