Skip to content

Commit

Permalink
cache filters and add initialize decoder on webhooks
Browse files Browse the repository at this point in the history
  • Loading branch information
jaydeokar committed Aug 10, 2023
1 parent ec35332 commit ba8963a
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 54 deletions.
3 changes: 0 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ delete:
docker-buildx: check-env test
docker buildx build --platform=$(PLATFORM) -t $(IMAGE)-$(GOARCH) --build-arg BASE_IMAGE=$(BASE_IMAGE) --build-arg BUILD_IMAGE=$(BUILD_IMAGE) --build-arg $(GOARCH) --load .

image:
KO_DOCKER_REPO=${REPO} ko build --bare main.go

# Build the docker image
docker-build: check-env test
docker build --build-arg BASE_IMAGE=$(BASE_IMAGE) --build-arg BUILD_IMAGE=$(BUILD_IMAGE) . -t ${IMAGE}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ require (
k8s.io/api v0.27.2
k8s.io/apimachinery v0.27.2
k8s.io/client-go v0.27.2
sigs.k8s.io/controller-runtime v0.15.0
sigs.k8s.io/controller-runtime v0.15.1
)

require (
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f h1:2kWPakN3i/k81b0gvD5C5F
k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f/go.mod h1:byini6yhqGC14c3ebc/QwanvYwhuMWF6yz2F8uwW8eg=
k8s.io/utils v0.0.0-20230209194617-a36077c30491 h1:r0BAOLElQnnFhE/ApUsg3iHdVYYPBjNSSOMowRZxxsY=
k8s.io/utils v0.0.0-20230209194617-a36077c30491/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
sigs.k8s.io/controller-runtime v0.15.0 h1:ML+5Adt3qZnMSYxZ7gAverBLNPSMQEibtzAgp0UPojU=
sigs.k8s.io/controller-runtime v0.15.0/go.mod h1:7ngYvp1MLT+9GeZ+6lH3LOlcHkp/+tzA/fmHa4iq9kk=
sigs.k8s.io/controller-runtime v0.15.1 h1:9UvgKD4ZJGcj24vefUFgZFP3xej/3igL9BsOUTb/+4c=
sigs.k8s.io/controller-runtime v0.15.1/go.mod h1:7ngYvp1MLT+9GeZ+6lH3LOlcHkp/+tzA/fmHa4iq9kk=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 h1:PRbqxJClWWYMNV1dhaG4NsibJbArud9kFxnAMREiWFE=
Expand Down
46 changes: 27 additions & 19 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ import (
"github.com/go-logr/zapr"
zapRaw "go.uber.org/zap"
"go.uber.org/zap/zapcore"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
Expand All @@ -52,7 +54,9 @@ import (
"k8s.io/client-go/tools/leaderelection/resourcelock"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
// +kubebuilder:scaffold:imports
)

Expand All @@ -63,6 +67,7 @@ var (
)

func init() {
// TODO: We should restrict this even more to use only the schemas the controller really needs to have.
_ = clientgoscheme.AddToScheme(scheme)

_ = corev1.AddToScheme(scheme)
Expand Down Expand Up @@ -217,22 +222,25 @@ func main() {
LeaderElectionNamespace: config.LeaderElectionNamespace,
LeaderElectionResourceLock: leaderElectionSource,
HealthProbeBindAddress: ":61779", // the liveness endpoint is default to "/healthz"
Cache: cache.Options{
// ByObject: map[client.Object]cache.ByObject{
// &corev1.ConfigMap{}: {Field: fields.Set{
// "metadata.name": config.VpcCniConfigMapName,
// "metadata.namespace": config.KubeSystemNamespace,
// }.AsSelector()},
// &appsv1.Deployment{}: {Field: fields.Set{
// "metadata.name": config.OldVPCControllerDeploymentName,
// "metadata.namespace": config.KubeSystemNamespace,
// }.AsSelector()},
// &appsv1.DaemonSet{}: {Field: fields.Set{
// "metadata.name": config.VpcCNIDaemonSetName,
// "metadata.namespace": config.KubeSystemNamespace,
// }.AsSelector(),
// },
// },
// ConfigMaps - WATCH only the ConfigMap that VPC RC consumes
// Deployments - WATCH only the old VPC Controller deployment
// Daemonsets - WATCH only the VPC CNI
Cache: cache.Options{
ByObject: map[client.Object]cache.ByObject{
&corev1.ConfigMap{}: {Field: fields.Set{
"metadata.name": config.VpcCniConfigMapName,
"metadata.namespace": config.KubeSystemNamespace,
}.AsSelector()},
&appsv1.Deployment{}: {Field: fields.Set{
"metadata.name": config.OldVPCControllerDeploymentName,
"metadata.namespace": config.KubeSystemNamespace,
}.AsSelector()},
&appsv1.DaemonSet{}: {Field: fields.Set{
"metadata.name": config.VpcCNIDaemonSetName,
"metadata.namespace": config.KubeSystemNamespace,
}.AsSelector(),
},
},
},
})
if err != nil {
Expand Down Expand Up @@ -378,19 +386,19 @@ func main() {

setupLog.Info("registering webhooks to the webhook server")
podMutationWebhook := webhookcore.NewPodMutationWebHook(
sgpAPI, ctrl.Log.WithName("resource mutating webhook"), controllerConditions, healthzHandler)
sgpAPI, ctrl.Log.WithName("resource mutating webhook"), controllerConditions, admission.NewDecoder(mgr.GetScheme()), healthzHandler)
webhookServer.Register("/mutate-v1-pod", &webhook.Admission{
Handler: podMutationWebhook,
})

nodeValidateWebhook := webhookcore.NewNodeUpdateWebhook(
controllerConditions, ctrl.Log.WithName("node validating webhook"), healthzHandler)
controllerConditions, ctrl.Log.WithName("node validating webhook"), admission.NewDecoder(mgr.GetScheme()), healthzHandler)
webhookServer.Register("/validate-v1-node", &webhook.Admission{
Handler: nodeValidateWebhook})

// Validating webhook for pod.
annotationValidator := webhookcore.NewAnnotationValidator(
controllerConditions, ctrl.Log.WithName("annotation validating webhook"), healthzHandler)
controllerConditions, ctrl.Log.WithName("annotation validating webhook"), admission.NewDecoder(mgr.GetScheme()), healthzHandler)
webhookServer.Register("/validate-v1-pod", &webhook.Admission{
Handler: annotationValidator})

Expand Down
2 changes: 1 addition & 1 deletion test/integration/perpodsg/perpodsg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ var _ = Describe("Branch ENI Pods", func() {
By("removing the has-trunk-attached label from the node")
err = frameWork.NodeManager.RemoveLabels(targetedNodes,
map[string]string{config.HasTrunkAttachedLabel: "true"})
Expect(err).To(HaveOccurred())
Expect(err).ToNot(HaveOccurred())

firstPod := podTemplate.DeepCopy()
By("creating a Pod on the un-managed node and verifying it fails")
Expand Down
9 changes: 2 additions & 7 deletions webhooks/core/annotation_validation_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@ type AnnotationValidator struct {
Checker healthz.Checker
}

func NewAnnotationValidator(condition condition.Conditions, log logr.Logger, healthzHandler *rcHealthz.HealthzHandler) *AnnotationValidator {
func NewAnnotationValidator(condition condition.Conditions, log logr.Logger, d *admission.Decoder, healthzHandler *rcHealthz.HealthzHandler) *AnnotationValidator {
annotationValidator := &AnnotationValidator{
Condition: condition,
Log: log,
decoder: d,
}

// add health check on subpath for pod annotation validating webhook
Expand Down Expand Up @@ -152,9 +153,3 @@ func (a *AnnotationValidator) getAnnotationKeysToBeValidated() []string {
}
return annotationsToValidate
}

// InjectDecoder injects the decoder.
func (a *AnnotationValidator) InjectDecoder(d *admission.Decoder) error {
a.decoder = d
return nil
}
8 changes: 0 additions & 8 deletions webhooks/core/annotation_validation_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,6 @@ type MockAnnotationWebHook struct {
MockCondition *mock_condition.MockConditions
}

func TestAnnotationValidator_InjectDecoder(t *testing.T) {
a := AnnotationValidator{}
decoder := &admission.Decoder{}
a.InjectDecoder(decoder)

assert.Equal(t, decoder, a.decoder)
}

func TestAnnotationValidator_Handle(t *testing.T) {
schema := runtime.NewScheme()
err := clientgoscheme.AddToScheme(schema)
Expand Down
9 changes: 2 additions & 7 deletions webhooks/core/node_update_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ type NodeUpdateWebhook struct {
Checker healthz.Checker
}

func NewNodeUpdateWebhook(condition condition.Conditions, log logr.Logger, healthzHandler *rcHealthz.HealthzHandler) *NodeUpdateWebhook {
func NewNodeUpdateWebhook(condition condition.Conditions, log logr.Logger, d *admission.Decoder, healthzHandler *rcHealthz.HealthzHandler) *NodeUpdateWebhook {
nodeUpdateWebhook := &NodeUpdateWebhook{
Condition: condition,
Log: log,
decoder: d,
}

// add health check on subpath for node validation webhook
Expand Down Expand Up @@ -92,9 +93,3 @@ func (a *NodeUpdateWebhook) Handle(_ context.Context, req admission.Request) adm
// If all validation check succeed, allow admission
return admission.Allowed("")
}

// InjectDecoder injects the decoder.
func (a *NodeUpdateWebhook) InjectDecoder(d *admission.Decoder) error {
a.decoder = d
return nil
}
8 changes: 2 additions & 6 deletions webhooks/core/pod_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,14 @@ func NewPodMutationWebHook(
sgpAPI utils.SecurityGroupForPodsAPI,
log logr.Logger,
condition condition.Conditions,
d *admission.Decoder,
healthzHandler *rcHealthz.HealthzHandler,
) *PodMutationWebHook {
podWebhook := &PodMutationWebHook{
SGPAPI: sgpAPI,
Log: log,
Condition: condition,
decoder: d,
}
// add health check on subpath for pod mutation webhook
healthzHandler.AddControllersHealthCheckers(
Expand Down Expand Up @@ -245,9 +247,3 @@ func hasWindowsNodeSelector(pod *corev1.Pod) bool {
}
return true
}

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

0 comments on commit ba8963a

Please sign in to comment.