torbendury / kube-networkpolicy-denier Goto Github PK

The repositories' state is still quite cluttered.

The whole code should be kept in a well maintained state automatically, best achieved via pre-commit hooks that prevent unclean code from being pushed at all.

Due to the server not catching and processing signals like SIGINT or SIGKILL i.e. in a goroutine, Pods are being killed the hard way by Kubernetes after the terminationGracefulPeriod.

This is a bug and not intended.

While the Pod is being shut down, requests to the admission controller might fail and users might see an unexpected error returned by the API server which was not able to reach the admission controller.

Currently, the server does not implement any timeout handling that goes above the default config of net/http.

TimeoutHandler handler might be a good choice to make use of the composability of net/http handlers in combination with context cancellation, see also this post.

The current handling is a bug and not intended. Bad actors might make use of this fact in a quite unrealistic scenario of overloading the admission controller.

However, the ValidatingWebhookConfiguration itself has a timeout set to 10 seconds which would at least not leave users blank with error messages.

(Also, I did a quick test and the server on my local machine runs about 20k req/s without errors as a single instance and I guess users with a load of >20k req/s on a single instance have way different problems than a small admission controller).

There should be documentation for users which informs about

• Container versioning
• Helm Chart versioning
• API stability
• Valid Kubernetes versions

Right now, the Docker image is around 25MB in size (~12MB compressed).

This is quite nice because it comes with full debugging abilities - because it runs in a full-fledged Alpine Linux environment.

However, this is intended to run as a system component from administrator POV and the resource overhead should be kept as minimal as possible.

Thus, offering an image from scratch should be the optimal solution and be worked towards upon.

A nice post I read about it is here.

The deny message is currently hardcoded into the validation handler.

It should rather be customizable optionally so administrators can "talk" to the users of the Kubernetes clusters the way they want to.

Practically, the health check endpoint is being called way more often than the controller is actually being called for admission.

Right now, the health check being called causes a line of log to be emitted.

The overhead caused by the health check should be kept to a minimum since a) it costs unnecessary resources to emit the log, b) the log is spammed and needs to be filtered to see actual workload being done and c) saving or even streaming and processing logs can be expensive.

Thus, health check request logging should be able to turned off.

Before releasing a new version, the code, resulting Docker image and the Helm Chart is tested manually with make local by building the image, spinning up a Minikube and installing the Helm Chart.

When the deployment is

• running correctly,
• answering health checks and
• denying NetworkPolicies,
  the test has succeeded and the new version can be released.

The manual process described above takes about 1 minute of time but must be done before being sure that the software state is releasable.

The task is to search for a possible automation of this (e.g. via GitHub Actions or at least as a pre-commit hook).

The application is "mature" enough to be refactored. At the moment, the main.go is lots of spaghetti which can definitely be improved and prepared for future enhancements.

Currently, the Deployment of the controller contains a readinessProbe and a livenessProbe. A startup probe should be added which enables faster startup and thus faster rolling upgrades, shutdowns, ... of the controller.

• Decrease the failureThreshold so controller failed states are acknowledged ealier
• Decrease initialDelaySeconds so the controller reaches a working (and traffic receiving state) earlier

The Kubernetes documentation can be used for this.

Currently, the Helm releases on GitHub only contain the description of the Helm Chart itself.

A new release should contain a summary of the last few changes, e.g. by posting the commit messages into the release description.