From 097a6ee41bf0e9395f9d0ca6b1cfd5b6b15cb2b3 Mon Sep 17 00:00:00 2001 From: Austin Abro <37223396+AustinAbro321@users.noreply.github.com> Date: Tue, 21 May 2024 13:32:17 -0400 Subject: [PATCH] test: pod agent unit tests (#2526) ## Description This also changes how we the agent gets secrets from the cluster. Instead of mounting secrets in the deployment and reading from a file the agent will instead read the secret directly from the cluster. This is done for simplicity, and easier testing. Additionally, the agent will need to read from the cluster in #1974 as well so this helps prepare for that change. This only changes and tests pods to keep the scope of the PR slim. This means temporarily the agent will both read from mounted file secrets and regular kubernetes secrets depending on the hook, but this will be changed shortly in future PRs. ## Related Issue Relates to #2512 ## Checklist before merging - [ ] Test, docs, adr added or updated as needed - [ ] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/.github/CONTRIBUTING.md#developer-workflow) followed --------- Co-authored-by: Lucas Rodriguez --- packages/zarf-agent/manifests/deployment.yaml | 1 + packages/zarf-agent/manifests/role.yaml | 12 ++ .../zarf-agent/manifests/rolebinding.yaml | 13 ++ .../zarf-agent/manifests/serviceaccount.yaml | 5 + packages/zarf-agent/zarf.yaml | 3 + src/config/lang/english.go | 2 +- src/internal/agent/hooks/pods.go | 39 +++-- src/internal/agent/hooks/pods_test.go | 149 ++++++++++++++++++ src/internal/agent/hooks/utils_test.go | 70 ++++++++ .../{admission.go => admission/handler.go} | 24 +-- src/internal/agent/http/server.go | 16 +- src/pkg/cluster/zarf_test.go | 7 +- 12 files changed, 305 insertions(+), 36 deletions(-) create mode 100644 packages/zarf-agent/manifests/role.yaml create mode 100644 packages/zarf-agent/manifests/rolebinding.yaml create mode 100644 packages/zarf-agent/manifests/serviceaccount.yaml create mode 100644 src/internal/agent/hooks/pods_test.go create mode 100644 src/internal/agent/hooks/utils_test.go rename src/internal/agent/http/{admission.go => admission/handler.go} (80%) diff --git a/packages/zarf-agent/manifests/deployment.yaml b/packages/zarf-agent/manifests/deployment.yaml index 2d4767ba56..a4113812f9 100644 --- a/packages/zarf-agent/manifests/deployment.yaml +++ b/packages/zarf-agent/manifests/deployment.yaml @@ -20,6 +20,7 @@ spec: imagePullSecrets: - name: private-registry priorityClassName: system-node-critical + serviceAccountName: zarf containers: - name: server image: "###ZARF_REGISTRY###/###ZARF_CONST_AGENT_IMAGE###:###ZARF_CONST_AGENT_IMAGE_TAG###" diff --git a/packages/zarf-agent/manifests/role.yaml b/packages/zarf-agent/manifests/role.yaml new file mode 100644 index 0000000000..c310ca19b1 --- /dev/null +++ b/packages/zarf-agent/manifests/role.yaml @@ -0,0 +1,12 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: zarf-agent + namespace: zarf +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get diff --git a/packages/zarf-agent/manifests/rolebinding.yaml b/packages/zarf-agent/manifests/rolebinding.yaml new file mode 100644 index 0000000000..c46cae1b72 --- /dev/null +++ b/packages/zarf-agent/manifests/rolebinding.yaml @@ -0,0 +1,13 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: zarf-agent-binding + namespace: zarf +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: zarf-agent +subjects: +- kind: ServiceAccount + name: zarf + namespace: zarf diff --git a/packages/zarf-agent/manifests/serviceaccount.yaml b/packages/zarf-agent/manifests/serviceaccount.yaml new file mode 100644 index 0000000000..2f9b060094 --- /dev/null +++ b/packages/zarf-agent/manifests/serviceaccount.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: zarf + namespace: zarf diff --git a/packages/zarf-agent/zarf.yaml b/packages/zarf-agent/zarf.yaml index 86799ff03f..32a6c34997 100644 --- a/packages/zarf-agent/zarf.yaml +++ b/packages/zarf-agent/zarf.yaml @@ -27,6 +27,9 @@ components: - manifests/secret.yaml - manifests/deployment.yaml - manifests/webhook.yaml + - manifests/role.yaml + - manifests/rolebinding.yaml + - manifests/serviceaccount.yaml actions: onCreate: before: diff --git a/src/config/lang/english.go b/src/config/lang/english.go index b72ffa1461..c5deeffc0c 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -664,7 +664,7 @@ const ( AgentErrBadRequest = "could not read request body: %s" AgentErrBindHandler = "Unable to bind the webhook handler" AgentErrCouldNotDeserializeReq = "could not deserialize request: %s" - AgentErrGetState = "failed to load zarf state from file: %w" + AgentErrGetState = "failed to load zarf state: %w" AgentErrHostnameMatch = "failed to complete hostname matching: %w" AgentErrImageSwap = "Unable to swap the host for (%s)" AgentErrInvalidMethod = "invalid method only POST requests are allowed" diff --git a/src/internal/agent/hooks/pods.go b/src/internal/agent/hooks/pods.go index a7b6911991..cefb024df0 100644 --- a/src/internal/agent/hooks/pods.go +++ b/src/internal/agent/hooks/pods.go @@ -5,13 +5,14 @@ package hooks import ( + "context" "encoding/json" "fmt" "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/config/lang" "github.com/defenseunicorns/zarf/src/internal/agent/operations" - "github.com/defenseunicorns/zarf/src/internal/agent/state" + "github.com/defenseunicorns/zarf/src/pkg/cluster" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/transform" v1 "k8s.io/api/admission/v1" @@ -20,11 +21,15 @@ import ( ) // NewPodMutationHook creates a new instance of pods mutation hook. -func NewPodMutationHook() operations.Hook { +func NewPodMutationHook(ctx context.Context, cluster *cluster.Cluster) operations.Hook { message.Debug("hooks.NewMutationHook()") return operations.Hook{ - Create: mutatePod, - Update: mutatePod, + Create: func(r *v1.AdmissionRequest) (*operations.Result, error) { + return mutatePod(ctx, r, cluster) + }, + Update: func(r *v1.AdmissionRequest) (*operations.Result, error) { + return mutatePod(ctx, r, cluster) + }, } } @@ -34,14 +39,12 @@ func parsePod(object []byte) (*corev1.Pod, error) { if err := json.Unmarshal(object, &pod); err != nil { return nil, err } - return &pod, nil } -func mutatePod(r *v1.AdmissionRequest) (*operations.Result, error) { +func mutatePod(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (*operations.Result, error) { message.Debugf("hooks.mutatePod()(*v1.AdmissionRequest) - %#v , %s/%s: %#v", r.Kind, r.Namespace, r.Name, r.Operation) - var patchOperations []operations.PatchOperation pod, err := parsePod(r.Object.Raw) if err != nil { return &operations.Result{Msg: err.Error()}, nil @@ -51,24 +54,26 @@ func mutatePod(r *v1.AdmissionRequest) (*operations.Result, error) { // We've already played with this pod, just keep swimming 🐟 return &operations.Result{ Allowed: true, - PatchOps: patchOperations, + PatchOps: []operations.PatchOperation{}, }, nil } - // Add the zarf secret to the podspec - zarfSecret := []corev1.LocalObjectReference{{Name: config.ZarfImagePullSecretName}} - patchOperations = append(patchOperations, operations.ReplacePatchOperation("/spec/imagePullSecrets", zarfSecret)) - - zarfState, err := state.GetZarfStateFromAgentPod() + state, err := cluster.LoadZarfState(ctx) if err != nil { return nil, fmt.Errorf(lang.AgentErrGetState, err) } - containerRegistryURL := zarfState.RegistryInfo.Address + registryURL := state.RegistryInfo.Address + + var patchOperations []operations.PatchOperation + + // Add the zarf secret to the podspec + zarfSecret := []corev1.LocalObjectReference{{Name: config.ZarfImagePullSecretName}} + patchOperations = append(patchOperations, operations.ReplacePatchOperation("/spec/imagePullSecrets", zarfSecret)) // update the image host for each init container for idx, container := range pod.Spec.InitContainers { path := fmt.Sprintf("/spec/initContainers/%d/image", idx) - replacement, err := transform.ImageTransformHost(containerRegistryURL, container.Image) + replacement, err := transform.ImageTransformHost(registryURL, container.Image) if err != nil { message.Warnf(lang.AgentErrImageSwap, container.Image) continue // Continue, because we might as well attempt to mutate the other containers for this pod @@ -79,7 +84,7 @@ func mutatePod(r *v1.AdmissionRequest) (*operations.Result, error) { // update the image host for each ephemeral container for idx, container := range pod.Spec.EphemeralContainers { path := fmt.Sprintf("/spec/ephemeralContainers/%d/image", idx) - replacement, err := transform.ImageTransformHost(containerRegistryURL, container.Image) + replacement, err := transform.ImageTransformHost(registryURL, container.Image) if err != nil { message.Warnf(lang.AgentErrImageSwap, container.Image) continue // Continue, because we might as well attempt to mutate the other containers for this pod @@ -90,7 +95,7 @@ func mutatePod(r *v1.AdmissionRequest) (*operations.Result, error) { // update the image host for each normal container for idx, container := range pod.Spec.Containers { path := fmt.Sprintf("/spec/containers/%d/image", idx) - replacement, err := transform.ImageTransformHost(containerRegistryURL, container.Image) + replacement, err := transform.ImageTransformHost(registryURL, container.Image) if err != nil { message.Warnf(lang.AgentErrImageSwap, container.Image) continue // Continue, because we might as well attempt to mutate the other containers for this pod diff --git a/src/internal/agent/hooks/pods_test.go b/src/internal/agent/hooks/pods_test.go new file mode 100644 index 0000000000..eb3480b415 --- /dev/null +++ b/src/internal/agent/hooks/pods_test.go @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +package hooks + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + "github.com/defenseunicorns/zarf/src/config" + "github.com/defenseunicorns/zarf/src/internal/agent/http/admission" + "github.com/defenseunicorns/zarf/src/internal/agent/operations" + "github.com/defenseunicorns/zarf/src/types" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func createPodAdmissionRequest(t *testing.T, op v1.Operation, pod *corev1.Pod) *v1.AdmissionRequest { + t.Helper() + raw, err := json.Marshal(pod) + require.NoError(t, err) + return &v1.AdmissionRequest{ + Operation: op, + Object: runtime.RawExtension{ + Raw: raw, + }, + } +} + +func TestPodMutationWebhook(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + state := &types.ZarfState{RegistryInfo: types.RegistryInfo{Address: "127.0.0.1:31999"}} + c := createTestClientWithZarfState(ctx, t, state) + handler := admission.NewHandler().Serve(NewPodMutationHook(ctx, c)) + + tests := []struct { + name string + admissionReq *v1.AdmissionRequest + expectedPatch []operations.PatchOperation + code int + }{ + { + name: "pod with label should be mutated", + admissionReq: createPodAdmissionRequest(t, v1.Create, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"should-be": "mutated"}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "nginx"}}, + InitContainers: []corev1.Container{{Image: "busybox"}}, + EphemeralContainers: []corev1.EphemeralContainer{ + { + EphemeralContainerCommon: corev1.EphemeralContainerCommon{ + Image: "alpine", + }, + }, + }, + }, + }), + expectedPatch: []operations.PatchOperation{ + operations.ReplacePatchOperation( + "/spec/imagePullSecrets", + []corev1.LocalObjectReference{{Name: config.ZarfImagePullSecretName}}, + ), + operations.ReplacePatchOperation( + "/spec/initContainers/0/image", + "127.0.0.1:31999/library/busybox:latest-zarf-2140033595", + ), + operations.ReplacePatchOperation( + "/spec/ephemeralContainers/0/image", + "127.0.0.1:31999/library/alpine:latest-zarf-1117969859", + ), + operations.ReplacePatchOperation( + "/spec/containers/0/image", + "127.0.0.1:31999/library/nginx:latest-zarf-3793515731", + ), + operations.ReplacePatchOperation( + "/metadata/labels/zarf-agent", + "patched", + ), + }, + code: http.StatusOK, + }, + { + name: "pod with zarf-agent patched label should not be mutated", + admissionReq: createPodAdmissionRequest(t, v1.Create, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"zarf-agent": "patched"}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "nginx"}}, + }, + }), + expectedPatch: nil, + code: http.StatusOK, + }, + { + name: "pod with no labels should not error", + admissionReq: createPodAdmissionRequest(t, v1.Create, &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Labels: nil, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "nginx"}}, + }, + }), + expectedPatch: []operations.PatchOperation{ + operations.ReplacePatchOperation( + "/spec/imagePullSecrets", + []corev1.LocalObjectReference{{Name: config.ZarfImagePullSecretName}}, + ), + operations.ReplacePatchOperation( + "/spec/containers/0/image", + "127.0.0.1:31999/library/nginx:latest-zarf-3793515731", + ), + operations.AddPatchOperation( + "/metadata/labels", + map[string]string{"zarf-agent": "patched"}, + ), + }, + code: http.StatusOK, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + resp := sendAdmissionRequest(t, tt.admissionReq, handler, tt.code) + if tt.expectedPatch == nil { + require.Empty(t, string(resp.Patch)) + } else { + expectedPatchJSON, err := json.Marshal(tt.expectedPatch) + require.NoError(t, err) + require.NotNil(t, resp) + require.True(t, resp.Allowed) + require.JSONEq(t, string(expectedPatchJSON), string(resp.Patch)) + } + }) + } +} diff --git a/src/internal/agent/hooks/utils_test.go b/src/internal/agent/hooks/utils_test.go new file mode 100644 index 0000000000..909cd1f301 --- /dev/null +++ b/src/internal/agent/hooks/utils_test.go @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +package hooks + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/defenseunicorns/zarf/src/pkg/cluster" + "github.com/defenseunicorns/zarf/src/pkg/k8s" + "github.com/defenseunicorns/zarf/src/types" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/admission/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func createTestClientWithZarfState(ctx context.Context, t *testing.T, state *types.ZarfState) *cluster.Cluster { + t.Helper() + c := &cluster.Cluster{K8s: &k8s.K8s{Clientset: fake.NewSimpleClientset()}} + stateData, err := json.Marshal(state) + require.NoError(t, err) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: cluster.ZarfStateSecretName, + Namespace: cluster.ZarfNamespaceName, + }, + Data: map[string][]byte{ + cluster.ZarfStateDataKey: stateData, + }, + } + _, err = c.Clientset.CoreV1().Secrets(cluster.ZarfNamespaceName).Create(ctx, secret, metav1.CreateOptions{}) + require.NoError(t, err) + return c +} + +// sendAdmissionRequest sends an admission request to the handler and returns the response. +func sendAdmissionRequest(t *testing.T, admissionReq *v1.AdmissionRequest, handler http.HandlerFunc, code int) *v1.AdmissionResponse { + t.Helper() + + b, err := json.Marshal(&v1.AdmissionReview{ + Request: admissionReq, + }) + require.NoError(t, err) + + // Note: The URL ("/test") doesn't matter here because we are directly invoking the handler. + // The handler processes the request based on the HTTP method and body content, not the URL path. + req := httptest.NewRequest(http.MethodPost, "/test", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + handler.ServeHTTP(rr, req) + + require.Equal(t, code, rr.Code) + + var admissionReview v1.AdmissionReview + if rr.Code == http.StatusOK { + err = json.NewDecoder(rr.Body).Decode(&admissionReview) + require.NoError(t, err) + } + + return admissionReview.Response +} diff --git a/src/internal/agent/http/admission.go b/src/internal/agent/http/admission/handler.go similarity index 80% rename from src/internal/agent/http/admission.go rename to src/internal/agent/http/admission/handler.go index fbeeb3b983..b4e3109af7 100644 --- a/src/internal/agent/http/admission.go +++ b/src/internal/agent/http/admission/handler.go @@ -1,8 +1,10 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2021-Present The Zarf Authors -// Package http provides a http server for the webhook and proxy. -package http +// Package admission provides an HTTP handler for a Kubernetes admission webhook. +// It includes functionality to decode incoming admission requests, execute +// the corresponding operations, and return appropriate admission responses. +package admission import ( "encoding/json" @@ -19,20 +21,20 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" ) -// admissionHandler represents the HTTP handler for an admission webhook. -type admissionHandler struct { +// Handler represents the HTTP handler for an admission webhook. +type Handler struct { decoder runtime.Decoder } -// newAdmissionHandler returns an instance of AdmissionHandler. -func newAdmissionHandler() *admissionHandler { - return &admissionHandler{ +// NewHandler returns a new admission Handler. +func NewHandler() *Handler { + return &Handler{ decoder: serializer.NewCodecFactory(runtime.NewScheme()).UniversalDeserializer(), } } -// Serve returns a http.HandlerFunc for an admission webhook. -func (h *admissionHandler) Serve(hook operations.Hook) http.HandlerFunc { +// Serve returns an http.HandlerFunc for an admission webhook. +func (h *Handler) Serve(hook operations.Hook) http.HandlerFunc { message.Debugf("http.Serve(%#v)", hook) return func(w http.ResponseWriter, r *http.Request) { message.Debugf("http.Serve()(writer, %#v)", r.URL) @@ -67,7 +69,7 @@ func (h *admissionHandler) Serve(hook operations.Hook) http.HandlerFunc { result, err := hook.Execute(review.Request) if err != nil { - message.WarnErr(err, lang.AgentErrBindHandler) + message.Warnf("%s: %s", lang.AgentErrBindHandler, err.Error()) w.WriteHeader(http.StatusInternalServerError) return } @@ -84,7 +86,7 @@ func (h *admissionHandler) Serve(hook operations.Hook) http.HandlerFunc { }, } - // set the patch operations for mutating admission + // Set the patch operations for mutating admission if len(result.PatchOps) > 0 { jsonPatchType := v1.PatchTypeJSONPatch patchBytes, err := json.Marshal(result.PatchOps) diff --git a/src/internal/agent/http/server.go b/src/internal/agent/http/server.go index 86ff5e828f..7b07b1445d 100644 --- a/src/internal/agent/http/server.go +++ b/src/internal/agent/http/server.go @@ -5,27 +5,37 @@ package http import ( + "context" "fmt" "net/http" "time" "github.com/defenseunicorns/zarf/src/internal/agent/hooks" + "github.com/defenseunicorns/zarf/src/internal/agent/http/admission" + "github.com/defenseunicorns/zarf/src/pkg/cluster" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/prometheus/client_golang/prometheus/promhttp" ) // NewAdmissionServer creates a http.Server for the mutating webhook admission handler. func NewAdmissionServer(port string) *http.Server { - message.Debugf("http.NewServer(%s)", port) + message.Debugf("http.NewAdmissionServer(%s)", port) + + c, err := cluster.NewCluster() + if err != nil { + message.Fatalf(err, err.Error()) + } + + ctx := context.Background() // Instances hooks - podsMutation := hooks.NewPodMutationHook() + podsMutation := hooks.NewPodMutationHook(ctx, c) fluxGitRepositoryMutation := hooks.NewGitRepositoryMutationHook() argocdApplicationMutation := hooks.NewApplicationMutationHook() argocdRepositoryMutation := hooks.NewRepositoryMutationHook() // Routers - ah := newAdmissionHandler() + ah := admission.NewHandler() mux := http.NewServeMux() mux.Handle("/healthz", healthz()) mux.Handle("/mutate/pod", ah.Serve(podsMutation)) diff --git a/src/pkg/cluster/zarf_test.go b/src/pkg/cluster/zarf_test.go index b9451f3e72..0d40f58bfa 100644 --- a/src/pkg/cluster/zarf_test.go +++ b/src/pkg/cluster/zarf_test.go @@ -213,9 +213,6 @@ func TestGetDeployedPackage(t *testing.T) { for _, p := range packages { b, err := json.Marshal(p) require.NoError(t, err) - data := map[string][]byte{ - "data": b, - } secret := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: strings.Join([]string{config.ZarfPackagePrefix, p.Name}, ""), @@ -224,7 +221,9 @@ func TestGetDeployedPackage(t *testing.T) { ZarfPackageInfoLabel: p.Name, }, }, - Data: data, + Data: map[string][]byte{ + "data": b, + }, } c.Clientset.CoreV1().Secrets("zarf").Create(ctx, &secret, metav1.CreateOptions{}) actual, err := c.GetDeployedPackage(ctx, p.Name)