diff --git a/crd.yaml b/crd.yaml index 7dfa814..1d2041b 100644 --- a/crd.yaml +++ b/crd.yaml @@ -6,12 +6,19 @@ spec: group: vaultwebhook.uswitch.com versions: - name: v1alpha1 + # Each version can be enabled/disabled by Served flag. served: true + # One and only one version must be marked as the storage version. storage: true schema: openAPIV3Schema: + type: object + description: |- + A MutatingAdmissionController that will add the vault-creds container to your pod + for you when your pod is created (assuming that vault webhook is enabled on your namespace properties: spec: + type: object properties: database: type: string @@ -20,7 +27,29 @@ spec: outputPath: type: string outputFile: + type: string + serviceAccount: type: string + container: + description: Specification of the container that will be created as part of this binding. + type: object + properties: + lifecycle: + description: Specification of the lifecycle hooks of the container. https://kubernetes.io/docs/concepts/containers/container-lifecycle-hooks/ + type: object + properties: + preStop: + description: This hook is called immediately before a container is terminated due to an API request or management event such as a liveness/startup probe failure, preemption, resource contention and others + type: object + properties: + exec: + description: Executes a specific command, inside the cgroups and namespaces of the Container. + type: object + properties: + command: + type: array + items: + type: string names: kind: DatabaseCredentialBinding plural: databasecredentialbindings diff --git a/go.mod b/go.mod index 7723702..8d81a1b 100644 --- a/go.mod +++ b/go.mod @@ -71,4 +71,4 @@ require ( sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect sigs.k8s.io/yaml v1.3.0 // indirect -) +) \ No newline at end of file diff --git a/go.sum b/go.sum index a783a80..4aa2499 100644 --- a/go.sum +++ b/go.sum @@ -274,4 +274,4 @@ sigs.k8s.io/structured-merge-diff/v4 v4.2.3 h1:PRbqxJClWWYMNV1dhaG4NsibJbArud9kF sigs.k8s.io/structured-merge-diff/v4 v4.2.3/go.mod h1:qjx8mGObPmV2aSZepjQjbmb2ihdVs8cGKBraizNC69E= sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc= sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo= -sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= +sigs.k8s.io/yaml v1.3.0/go.mod h1:GeOyir5tyXNByN85N/dRIT9es5UQNerPYEKK56eTBm8= \ No newline at end of file diff --git a/pkg/apis/vaultwebhook.uswitch.com/v1alpha1/types.go b/pkg/apis/vaultwebhook.uswitch.com/v1alpha1/types.go index 38a4816..4e26ca3 100644 --- a/pkg/apis/vaultwebhook.uswitch.com/v1alpha1/types.go +++ b/pkg/apis/vaultwebhook.uswitch.com/v1alpha1/types.go @@ -1,6 +1,7 @@ package v1alpha1 import ( + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -15,11 +16,13 @@ type DatabaseCredentialBinding struct { } type DatabaseCredentialBindingSpec struct { - Database string `json:"database"` - Role string `json:"role"` - OutputPath string `json:"outputPath"` - OutputFile string `json:"outputFile"` - ServiceAccount string `json:"serviceAccount"` + Database string `json:"database"` + Role string `json:"role"` + OutputPath string `json:"outputPath"` + OutputFile string `json:"outputFile"` + ServiceAccount string `json:"serviceAccount"` + Container Container `json:"container,omitempty"` + // InitContainer Container `json:"initcontainer,omitempty"` // TODO: Fix support for initcontainer's Lifecycle hooks ( Go dep to be updated ) } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -30,3 +33,23 @@ type DatabaseCredentialBindingList struct { Items []DatabaseCredentialBinding `json:"items"` } + +type Container struct { + Lifecycle corev1.Lifecycle `json:"lifecycle,omitempty"` +} + +/* +https://pkg.go.dev/k8s.io/api/core/v1#LifecycleHandler +Check if Container.Lifecycle.PreStop is valid. This is to avoid mishandling incomplete inputs like the below: + + { "Lifecycle": { + "PostStart": null, + "PreStop": { + "Exec": null, # <----- Missing Command!! + "HTTPGet": null,"TCPSocket": null}}} +*/ +func (c Container) HasValidPreStop() bool { + return c.Lifecycle.PreStop != nil && + c.Lifecycle.PreStop.Exec != nil && + len(c.Lifecycle.PreStop.Exec.Command) > 0 +} diff --git a/vault.go b/vault.go index 3a2948d..acc9544 100644 --- a/vault.go +++ b/vault.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/uswitch/vault-webhook/pkg/apis/vaultwebhook.uswitch.com/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" ) @@ -24,6 +25,9 @@ func addVault(pod *corev1.Pod, namespace string, databases []database) (patch [] initContainers := []corev1.Container{} for _, databaseInfo := range databases { + vaultContainerSpec := databaseInfo.vaultContainer + //initVaultContainerSpec := databaseInfo.initVaultContainer // TODO: Fix support for initcontainer's Lifecycle hooks ( Go dep to be updated ) + database := databaseInfo.database role := databaseInfo.role serviceAccount := pod.Spec.ServiceAccountName @@ -104,6 +108,10 @@ func addVault(pod *corev1.Pod, namespace string, databases []database) (patch [] initContainer := vaultContainer + // Configure Lifecycle Hooks if spec exists + // initContainer = addLifecycleHook(initContainer, initVaultContainerSpec) // TODO: Fix support for initcontainer's Lifecycle hooks ( Go dep to be updated ) + vaultContainer = addLifecycleHook(vaultContainer, vaultContainerSpec) + jobLikeOwnerReferencesKinds := map[string]bool{"Job": true, "Workflow": true} if len(pod.ObjectMeta.OwnerReferences) != 0 { ownerKind := pod.ObjectMeta.OwnerReferences[0].Kind @@ -112,6 +120,7 @@ func addVault(pod *corev1.Pod, namespace string, databases []database) (patch [] } } + // Append the new Vault container spec into the Pod Spec generated by the client Deployment/Daemonset/etc pod.Spec.Containers = append(pod.Spec.Containers, vaultContainer) initContainer.Args = append(initContainer.Args, "--init") @@ -197,3 +206,26 @@ func appendVolumeMountIfMissing(slice []corev1.VolumeMount, v corev1.VolumeMount } return append(slice, v) } + +// Conditionally set Lifecycle if it exists in containerSpec +func addLifecycleHook(container corev1.Container, containerSpec v1alpha1.Container) corev1.Container { + + // Check DatabaseCredentialBindingSpec.Container.Lifecycle is not empty + emptyLifecycle := corev1.Lifecycle{} + if containerSpec.Lifecycle != emptyLifecycle { + + // Check for a complete PreStop hook + if containerSpec.HasValidPreStop() { + container.Lifecycle = &containerSpec.Lifecycle + } + + } + // TODO: Fix support for initcontainer's Lifecycle hooks ( Go dep to be updated ) + // Init containers must have RestartPolicy=Always to be able to support Lifecycle hooks + // if isInit { + // restartPolicy := corev1.ContainerRestartPolicyAlways + // container.RestartPolicy = &restartPolicy + // } + //} + return container +} diff --git a/vault_test.go b/vault_test.go index d534e74..e8b505e 100644 --- a/vault_test.go +++ b/vault_test.go @@ -1,10 +1,12 @@ package main import ( + "fmt" "strings" "testing" - "k8s.io/api/core/v1" + "github.com/uswitch/vault-webhook/pkg/apis/vaultwebhook.uswitch.com/v1alpha1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -183,3 +185,68 @@ func TestVaultJobMode(t *testing.T) { }) } } + +// Can we add a preStop hook to the vault container? +func TestAddLifecyclePreStopHook(t *testing.T) { + + // Define test cases + var tests = []struct { + scenario string + lifecycleObj v1alpha1.Container + answer bool + }{ + { + scenario: "Test passing a complete lifecyle config", + lifecycleObj: v1alpha1.Container{ + Lifecycle: v1.Lifecycle{ + PreStop: &v1.LifecycleHandler{ + Exec: &v1.ExecAction{ + Command: []string{"echo", "hello"}, + }, + }, + }, + }, + answer: true, + }, + { + scenario: "Test passing an incomplete lifecycle config", + lifecycleObj: v1alpha1.Container{ + Lifecycle: v1.Lifecycle{ + PreStop: &v1.LifecycleHandler{ + Exec: nil, + }, + }, + }, + answer: false, + }, + { + // v1alpha1.Container{}, comes from corev1.Container{} and this ALWAYS have a c.Lifecycle object. The latter, always has pointers to PostStart and PreStop handlers ( but no further down the struct since they are pointers ) + // if our dcb input does not specify a container object, the received input will look like this: {Lifecycle:{PostStart:nil PreStop:nil}} + scenario: "Test passing no lifecycle config", + lifecycleObj: v1alpha1.Container{ + Lifecycle: v1.Lifecycle{ + PreStop: nil, + }, + }, + answer: false, + }, + } + + // Run tests + for _, tt := range tests { + // t.Run enables running "subtests", one for each table entry. These are shown separately when executing `go test -v`. + vaultContainer := v1.Container{} // Define a Vault sidecar Container + testname := fmt.Sprintf("%v", tt.scenario) + t.Run(testname, func(t *testing.T) { + ans := addLifecycleHook(vaultContainer, tt.lifecycleObj) + + //log.Printf("%+v", ans) + isValid := ans.Lifecycle != nil && ans.Lifecycle.PreStop != nil && ans.Lifecycle.PreStop.Exec != nil && len(ans.Lifecycle.PreStop.Exec.Command) > 0 + + if isValid != tt.answer { + t.Errorf("got %v, want %v", isValid, tt.answer) + } + }) + } + +} diff --git a/webhook.go b/webhook.go index 731e85e..60f71d6 100644 --- a/webhook.go +++ b/webhook.go @@ -38,10 +38,12 @@ type patchOperation struct { } type database struct { - database string - role string - outputPath string - outputFile string + database string + role string + outputPath string + outputFile string + vaultContainer v1alpha1.Container + // initVaultContainer v1alpha1.Container // TODO: Fix support for initcontainer's Lifecycle hooks ( Go dep to be updated ) } func (srv webHookServer) serve(w http.ResponseWriter, r *http.Request) { @@ -99,6 +101,7 @@ func (srv webHookServer) serve(w http.ResponseWriter, r *http.Request) { } +// This handles the admission review sent by k8s and mutates the pod func (srv webHookServer) mutate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse { req := ar.Request @@ -121,7 +124,9 @@ func (srv webHookServer) mutate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionR log.Infof("AdmissionReview for Kind=%v, Namespace=%v Name=%v UID=%v patchOperation=%v UserInfo=%v", ownerKind, req.Namespace, ownerName, req.UID, req.Operation, req.UserInfo) + // A list of ALL the bindings. binds, err := srv.bindings.List() + log.Infof("[mutate] List of all bindings: %+v", binds) if err != nil { return &v1beta1.AdmissionResponse{ Result: &metav1.Status{ @@ -130,6 +135,7 @@ func (srv webHookServer) mutate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionR } } + // Filter out the bindings that are not in the target namespace filteredBindings := filterBindings(binds, req.Namespace) if len(filteredBindings) == 0 { log.Infof("Skipping mutation for %s/%s, no database credential bindings in namespace", req.Namespace, ownerName) @@ -138,6 +144,7 @@ func (srv webHookServer) mutate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionR } } + // Identify bindings with ServiceAccount field matching the pod's ServiceAccountName databases := matchBindings(filteredBindings, pod.Spec.ServiceAccountName) if len(databases) == 0 { log.Infof("Skipping mutation for %s/%s due to policy check", req.Namespace, ownerName) @@ -166,6 +173,7 @@ func (srv webHookServer) mutate(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionR } } +// For all the bindings, we need to find the ones in the target namespace func filterBindings(bindings []v1alpha1.DatabaseCredentialBinding, namespace string) []v1alpha1.DatabaseCredentialBinding { filteredBindings := []v1alpha1.DatabaseCredentialBinding{} for _, binding := range bindings { @@ -176,6 +184,12 @@ func filterBindings(bindings []v1alpha1.DatabaseCredentialBinding, namespace str return filteredBindings } +/* + For all the bindings in the namespace, check which one has a ServiceeAccount that matches the pod's ServiceAccount + - We could have multiple database specifications to be attached to a single pod. + - This means that we could also have different VaultContainer specs for each DatabaseCredentialBinding. + - As a consequence, to keep things consistent and easy to follow, we are appending into the `database` slice. +*/ func matchBindings(bindings []v1alpha1.DatabaseCredentialBinding, serviceAccount string) []database { matchedBindings := []database{} for _, binding := range bindings { @@ -184,7 +198,17 @@ func matchBindings(bindings []v1alpha1.DatabaseCredentialBinding, serviceAccount if output == "" { output = "/etc/database" } - matchedBindings = appendIfMissing(matchedBindings, database{role: binding.Spec.Role, database: binding.Spec.Database, outputPath: output, outputFile: binding.Spec.OutputFile}) + log.Infof("[matchBindings] Printing content of Container: %+v", binding.Spec.Container) + //log.Infof("[matchBindings] Printing content of InitContainer: %+v", binding.Spec.InitContainer) + + matchedBindings = appendIfMissing(matchedBindings, database{ + role: binding.Spec.Role, + database: binding.Spec.Database, + outputPath: output, + outputFile: binding.Spec.OutputFile, + vaultContainer: binding.Spec.Container, + //initVaultContainer: binding.Spec.InitContainer, // TODO: Fix support for initcontainer's Lifecycle hooks ( Go dep to be updated ) + }) } } return matchedBindings @@ -192,7 +216,11 @@ func matchBindings(bindings []v1alpha1.DatabaseCredentialBinding, serviceAccount func appendIfMissing(slice []database, d database) []database { for _, ele := range slice { - if ele == d { + // No need to compare the Container and InitContainer fields. + if ele.role == d.role && + ele.database == d.database && + ele.outputPath == d.outputPath && + ele.outputFile == d.outputFile { return slice } }