Make the deployment and the jobs run in a non-root environment

.github/workflows/release-chart.yaml

@@ -4,9 +4,9 @@ on:
   push:
     branches:
       - main
-# TODO: add this back
-#    paths:
-#      - 'helm/**'
+      - release/legacy-csr
+    paths:
+      - 'charts/**'
 
 jobs:
   release:
@@ -14,8 +14,6 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v1
-        with:
-          ref: main
 
       - name: Configure Git
         run: |
@@ -48,8 +46,6 @@ jobs:
 
       - name: Run chart-releaser
         if: steps.helm_version_checker.outcome == 'success'
-        uses: helm/chart-releaser-action@master
-        with:
-          charts_dir: 'helm'
+        uses: helm/chart-releaser-action@v1.2.1
         env:
           CR_TOKEN: '${{ secrets.GITHUB_TOKEN }}'

README.md

@@ -40,7 +40,7 @@ Of note is also a fact that the chart runs a lookup to the connected cluster to
     make build-image
     helm upgrade -i imagepullsecret-injector \
         -n imagepullsecret-injector \
-        helm/imagepullsecret-injector
+        charts/imagepullsecret-injector
     ```
     Alternatively, you can use the pre-built, publicly available helm chart and docker images:
     ```bash

VERSION

@@ -1 +1 @@
-0.0.10
+0.0.12

build/Dockerfile

@@ -7,4 +7,7 @@ RUN make build
 FROM alpine:3.13.4 as base
 COPY --from=builder /go/src/github.com/ysoftdevs/imagepullsecret-injector/build/_output/bin/imagepullsecret-injector /usr/local/bin/imagepullsecret-injector
 
-ENTRYPOINT ["imagepullsecret-injector"]
+RUN addgroup -S imagepullsecret-injector-group && adduser -S imagepullsecret-injector-user -G imagepullsecret-injector-group
+USER imagepullsecret-injector-user
+
+ENTRYPOINT ["imagepullsecret-injector"]

build/Dockerfile.cert-generator

@@ -1,6 +1,9 @@
 FROM alpine:3.13.4
 
-RUN apk add bash curl openssl \
+RUN addgroup -S imagepullsecret-injector-group && adduser -S imagepullsecret-injector-user -G imagepullsecret-injector-group \
+    && apk add bash curl openssl jq \
     && curl -LO "https://dl.k8s.io/release/$(curl -L -s https://dl.k8s.io/release/stable.txt)/bin/linux/amd64/kubectl" \
     && chmod 755 ./kubectl \
     && mv ./kubectl /usr/bin/kubectl
+
+USER imagepullsecret-injector-user

charts/imagepullsecret-injector/Chart.yaml

@@ -15,9 +15,9 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.0.18
+version: 0.0.21
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
-appVersion: 0.0.10
+appVersion: 0.0.12

charts/imagepullsecret-injector/scripts/create-signed-cert.sh

@@ -83,23 +83,27 @@ echo "Deleting old CertificateSigningRequests"
 kubectl delete csr ${csrName} 2>/dev/null || true
 
 echo "Creating new CertificateSigningRequests"
-# create  server cert/key CSR and  send to k8s API
-cat <<EOF | kubectl create -f -
-apiVersion: certificates.k8s.io/v1
-kind: CertificateSigningRequest
-metadata:
-  name: ${csrName}
-  namespace: ${namespace}
-spec:
-  signerName: kubernetes.io/kubelet-serving
-  groups:
-  - system:authenticated
-  request: $(< "${tmpdir}"/server.csr base64 | tr -d '\n')
-  usages:
-  - digital signature
-  - key encipherment
-  - server auth
-EOF
+# create server cert/key CSR and  send to k8s API
+jq -n --arg request "$(< "${tmpdir}"/server.csr base64)" \
+  --arg namespace "$namespace" \
+  --arg csrName "$csrName" '{
+    apiVersion: "certificates.k8s.io/v1",
+    kind: "CertificateSigningRequest",
+    metadata: {
+      name: $csrName,
+      namespace: $namespace
+    },
+    spec: {
+      signerName: "kubernetes.io/kubelet-serving",
+      groups: ["system:authenticated"],
+      request: $request,
+      usages: [
+        "digital signature",
+        "key encipherment",
+        "server auth"
+      ]
+    }
+  }' | kubectl create -f -
 
 # verify CSR has been created
 while true; do

charts/imagepullsecret-injector/templates/certificate-gen/cronjob-certificate-gen.yaml

@@ -1,7 +1,7 @@
 apiVersion: batch/v1beta1
 kind: CronJob
 metadata:
-  name: "{{ .Release.Name }}"
+  name: "{{ .Release.Name }}-cert-gen-cron-job"
   labels:
     {{- include "imagepullsecret-injector.labels" . | nindent 4 }}
 spec:

charts/imagepullsecret-injector/templates/certificate-gen/job-certificate-gen.yaml

@@ -1,7 +1,7 @@
 apiVersion: batch/v1
 kind: Job
 metadata:
-  name: "{{ .Release.Name }}"
+  name: "{{ .Release.Name }}-cert-gen-job"
   labels:
     {{- include "imagepullsecret-injector.labels" . | nindent 4 }}
 spec:

charts/imagepullsecret-injector/templates/rbac.yaml

@@ -25,6 +25,7 @@ rules:
       - create
       - get
       - delete
+      - update
   - apiGroups:
       - ""
     resources:

cmd/main.go

@@ -8,40 +8,11 @@ import (
 	"net/http"
 	"os"
 	"os/signal"
-	"strconv"
 	"syscall"
 
 	"github.com/golang/glog"
 )
 
-func LookupStringEnv(envName string, defVal string) string {
-	if envVal, exists := os.LookupEnv(envName); exists {
-		return envVal
-	}
-
-	return defVal
-}
-
-func LookupBoolEnv(envName string, defVal bool) bool {
-	if envVal, exists := os.LookupEnv(envName); exists {
-		if boolVal, err := strconv.ParseBool(envVal); err == nil {
-			return boolVal
-		}
-	}
-
-	return defVal
-}
-
-func LookupIntEnv(envName string, defVal int) int {
-	if envVal, exists := os.LookupEnv(envName); exists {
-		if intVal, err := strconv.Atoi(envVal); err == nil {
-			return intVal
-		}
-	}
-
-	return defVal
-}
-
 func main() {
 	parameters := DefaultParametersObject()
 

cmd/utils.go

@@ -0,0 +1,55 @@
+package main
+
+import (
+	"io/ioutil"
+	"os"
+	"strconv"
+	"strings"
+)
+
+// LookupStringEnv either returns the the value of the env variable, or the provided default value, if the env doesn't exist
+func LookupStringEnv(envName string, defVal string) string {
+	if envVal, exists := os.LookupEnv(envName); exists {
+		return envVal
+	}
+
+	return defVal
+}
+
+// LookupBoolEnv either returns the the value of the env variable, or the provided default value, if the env doesn't exist
+func LookupBoolEnv(envName string, defVal bool) bool {
+	if envVal, exists := os.LookupEnv(envName); exists {
+		if boolVal, err := strconv.ParseBool(envVal); err == nil {
+			return boolVal
+		}
+	}
+
+	return defVal
+}
+
+// LookupIntEnv either returns the the value of the env variable, or the provided default value, if the env doesn't exist
+func LookupIntEnv(envName string, defVal int) int {
+	if envVal, exists := os.LookupEnv(envName); exists {
+		if intVal, err := strconv.Atoi(envVal); err == nil {
+			return intVal
+		}
+	}
+
+	return defVal
+}
+
+func getCurrentNamespace() string {
+	// Check whether we have overridden the namespace
+	if ns, ok := os.LookupEnv("POD_NAMESPACE"); ok {
+		return ns
+	}
+
+	// Fall back to the namespace associated with the service account token, if available (this should exist if running in a K8S pod)
+	if data, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil {
+		if ns := strings.TrimSpace(string(data)); len(ns) > 0 {
+			return ns
+		}
+	}
+
+	return "default"
+}

cmd/webhook.go

@@ -6,7 +6,6 @@ import (
 	"io/ioutil"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"net/http"
-	"os"
 	"strings"
 
 	"github.com/golang/glog"
@@ -33,11 +32,11 @@ type WebhookServer struct {
 	client *kubernetes.Clientset
 }
 
-// Webhook Server parameters
+// WhSvrParameters represents all configuration options available though cmd parameters or env variables
 type WhSvrParameters struct {
-	port                           int    // webhook server port
-	certFile                       string // path to the x509 certificate for https
-	keyFile                        string // path to the x509 private key matching `CertFile`
+	port                           int
+	certFile                       string
+	keyFile                        string
 	excludeNamespaces              string
 	serviceAccounts                string
 	allServiceAccounts             bool
@@ -58,6 +57,7 @@ var (
 	}
 )
 
+// NewWebhookServer constructor for WebhookServer
 func NewWebhookServer(parameters *WhSvrParameters, server *http.Server) (*WebhookServer, error) {
 	config, err := rest.InClusterConfig()
 	if err != nil {
@@ -78,6 +78,7 @@ func NewWebhookServer(parameters *WhSvrParameters, server *http.Server) (*Webhoo
 
 }
 
+// DefaultParametersObject returns a parameters object with the default values
 func DefaultParametersObject() WhSvrParameters {
 	return WhSvrParameters{
 		port:                           8443,
@@ -128,23 +129,7 @@ func addImagePullSecret(target, added []corev1.LocalObjectReference, basePath st
 	return patch
 }
 
-func getCurrentNamespace() string {
-	// This way assumes you've set the POD_NAMESPACE environment variable using the downward API.
-	// This check has to be done first for backwards compatibility with the way InClusterConfig was originally set up
-	if ns, ok := os.LookupEnv("POD_NAMESPACE"); ok {
-		return ns
-	}
-
-	// Fall back to the namespace associated with the service account token, if available
-	if data, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil {
-		if ns := strings.TrimSpace(string(data)); len(ns) > 0 {
-			return ns
-		}
-	}
-
-	return "default"
-}
-
+// ensureSecrets looks up the source secret and makes sure the namespace the patched SA is in contains it too
 func (whsvr *WebhookServer) ensureSecrets(ar *v1beta1.AdmissionReview) error {
 	glog.Infof("Ensuring existing secrets")
 	targetNamespace := ar.Request.Namespace
@@ -199,6 +184,7 @@ func (whsvr *WebhookServer) ensureSecrets(ar *v1beta1.AdmissionReview) error {
 	return nil
 }
 
+// shouldMutate goes through all filters and determines whether the incoming SA matches them
 func (whsvr *WebhookServer) shouldMutate(sa corev1.ServiceAccount) bool {
 	for _, excludedNamespace := range strings.Split(whsvr.config.excludeNamespaces, ",") {
 		if sa.Namespace == excludedNamespace {
@@ -219,6 +205,7 @@ func (whsvr *WebhookServer) shouldMutate(sa corev1.ServiceAccount) bool {
 	return false
 }
 
+// mutateServiceAccount contains the whole mutation logic
 func (whsvr *WebhookServer) mutateServiceAccount(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse {
 	req := ar.Request
 	glog.Infof("Unmarshalling the ServiceAccount object from request")
@@ -291,8 +278,8 @@ func (whsvr *WebhookServer) mutateServiceAccount(ar *v1beta1.AdmissionReview) *v
 	}
 }
 
-// Serve method for webhook server
-func (whsvr *WebhookServer) serve(w http.ResponseWriter, r *http.Request) {
+func parseIncomingRequest(r *http.Request) (v1beta1.AdmissionReview, *errors.StatusError) {
+	var ar v1beta1.AdmissionReview
 	var body []byte
 	if r.Body != nil {
 		if data, err := ioutil.ReadAll(r.Body); err == nil {
@@ -300,50 +287,69 @@ func (whsvr *WebhookServer) serve(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 	if len(body) == 0 {
-		glog.Error("empty body")
-		http.Error(w, "empty body", http.StatusBadRequest)
-		return
+		glog.Error("Empty body")
+		return ar, errors.NewBadRequest("Empty body")
 	}
 
 	// verify the content type is accurate
 	contentType := r.Header.Get("Content-Type")
 	if contentType != "application/json" {
 		glog.Errorf("Content-Type=%s, expect application/json", contentType)
-		http.Error(w, "invalid Content-Type, expect `application/json`", http.StatusUnsupportedMediaType)
-		return
+		err := &errors.StatusError{ErrStatus: metav1.Status{
+			Status:  metav1.StatusFailure,
+			Message: fmt.Sprintf("Content-Type=%s, expect application/json", contentType),
+			Reason:  metav1.StatusReasonUnsupportedMediaType,
+			Code:    http.StatusUnsupportedMediaType,
+		}}
+		return ar, err
 	}
 
-	var admissionResponse *v1beta1.AdmissionResponse
-	ar := v1beta1.AdmissionReview{}
 	if _, _, err := deserializer.Decode(body, nil, &ar); err != nil {
-		glog.Errorf("Can't decode body: %v", err)
-		admissionResponse = &v1beta1.AdmissionResponse{
-			Result: &metav1.Status{
-				Message: err.Error(),
-			},
-		}
-	} else {
-		admissionResponse = whsvr.mutateServiceAccount(&ar)
+		glog.Error("Could not parse the request body")
+		return ar, errors.NewBadRequest(fmt.Sprintf("Could not parse the request body: %+v", err))
 	}
 
-	admissionReview := v1beta1.AdmissionReview{
-		TypeMeta: metav1.TypeMeta{Kind: "AdmissionReview", APIVersion: "admission.k8s.io/v1"},
-	}
-	if admissionResponse != nil {
-		admissionReview.Response = admissionResponse
-		if ar.Request != nil {
-			admissionReview.Response.UID = ar.Request.UID
-		}
-	}
+	return ar, nil
+}
 
+func (whsvr *WebhookServer) sendResponse(w http.ResponseWriter, admissionReview v1beta1.AdmissionReview) error {
 	resp, err := json.Marshal(admissionReview)
 	if err != nil {
 		glog.Errorf("Can't encode response: %v", err)
 		http.Error(w, fmt.Sprintf("could not encode response: %v", err), http.StatusInternalServerError)
+		return err
 	}
-	glog.Infof("Ready to write reponse ...")
+	glog.Infof("Writing response")
 	if _, err := w.Write(resp); err != nil {
 		glog.Errorf("Can't write response: %v", err)
 		http.Error(w, fmt.Sprintf("could not write response: %v", err), http.StatusInternalServerError)
+		return err
+	}
+
+	return nil
+}
+
+// serve parses the raw incoming request, calls the mutation logic and sends the proper response
+func (whsvr *WebhookServer) serve(w http.ResponseWriter, r *http.Request) {
+	admissionReviewIn, statusErr := parseIncomingRequest(r)
+	if statusErr != nil {
+		http.Error(w, statusErr.ErrStatus.Message, int(statusErr.ErrStatus.Code))
+		return
+	}
+
+	admissionResponse := whsvr.mutateServiceAccount(&admissionReviewIn)
+
+	admissionReviewOut := v1beta1.AdmissionReview{
+		TypeMeta: metav1.TypeMeta{Kind: "AdmissionReview", APIVersion: "admission.k8s.io/v1"},
+	}
+	if admissionResponse != nil {
+		admissionReviewOut.Response = admissionResponse
+		if admissionReviewIn.Request != nil {
+			admissionReviewOut.Response.UID = admissionReviewIn.Request.UID
+		}
+	}
+
+	if err := whsvr.sendResponse(w, admissionReviewOut); err != nil {
+		glog.Errorf("Could not send response %v", err)
 	}
 }