diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 64bae13d..cf4a5c47 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -113,7 +113,7 @@ jobs: env: T: integration DEPLOY_METHOD: chart - integration-rotation-enabled: + integration-optional-features: runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v4 @@ -126,5 +126,5 @@ jobs: env: T: integration DEPLOY_METHOD: chart - HELM_INSTALL_FLAGS_FLAGS: --set certificates.certReload.enabled=true + HELM_INSTALL_FLAGS_FLAGS: --set certificates.certReload.enabled=true, --set randomHostname=true diff --git a/admission-webhook/integration_tests/integration_test.go b/admission-webhook/integration_tests/integration_test.go index 28c476a5..79611c60 100644 --- a/admission-webhook/integration_tests/integration_test.go +++ b/admission-webhook/integration_tests/integration_test.go @@ -463,6 +463,70 @@ func TestPossibleToUpdatePodWithNewCert(t *testing.T) { assert.Equal(t, expectedCredSpec0, extractContainerCredSpecContents(t, pod3, testName3)) } +func TestPossibleHostnameRandomization(t *testing.T) { + deployMethod := os.Getenv("DEPLOY_METHOD") + if deployMethod != "chart" { + t.Skip("Non chart deployment method not supported for this test") + } + + webHookNs := os.Getenv("NAMESPACE") + webHookDeploymentName := os.Getenv("DEPLOYMENT_NAME") + webhook, err := kubeClient(t).AppsV1().Deployments(webHookNs).Get(context.Background(), webHookDeploymentName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + randomHostnameEnabled := false + for _, envVar := range webhook.Spec.Template.Spec.Containers[0].Env { + if strings.EqualFold(envVar.Name, "RANDOM_HOSTNAME") && strings.EqualFold(envVar.Value, "true") { + randomHostnameEnabled = true + } + } + + if randomHostnameEnabled { + testName1 := "happy-path-with-hostname-randomization" + credSpecTemplates1 := []string{"credspec-0"} + templates1 := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-with-gmsa"} + + testConfig1, tearDownFunc1 := integrationTestSetup(t, testName1, credSpecTemplates1, templates1) + defer tearDownFunc1() + + pod := waitForPodToComeUp(t, testConfig1.Namespace, "app="+testName1) + assert.NotEqual(t, testName1, pod.Spec.Hostname) + assert.Equal(t, 15, len(pod.Spec.Hostname)) + + testName2 := "hostnameset-no-hostname-randomization" + credSpecTemplates2 := []string{"credspec-0"} + templates2 := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-with-gmsa-hostname"} + + testConfig2, tearDownFunc2 := integrationTestSetup(t, testName2, credSpecTemplates2, templates2) + defer tearDownFunc2() + + pod = waitForPodToComeUp(t, testConfig2.Namespace, "app="+testName2) + assert.Equal(t, testName2, pod.Spec.Hostname) + + testName3 := "nogmsa-hostname-randomization" + credSpecTemplates3 := []string{"credspec-0"} + templates3 := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-without-gmsa"} + + testConfig3, tearDownFunc3 := integrationTestSetup(t, testName3, credSpecTemplates3, templates3) + defer tearDownFunc3() + pod = waitForPodToComeUp(t, testConfig3.Namespace, "app="+testName3) + + assert.Equal(t, "", pod.Spec.Hostname) + } else { + testName4 := "notenabled-hostname-randomization" + credSpecTemplates4 := []string{"credspec-0"} + templates4 := []string{"credspecs-users-rbac-role", "service-account", "sa-rbac-binding", "simple-with-gmsa"} + + testConfig4, tearDownFunc4 := integrationTestSetup(t, testName4, credSpecTemplates4, templates4) + defer tearDownFunc4() + pod := waitForPodToComeUp(t, testConfig4.Namespace, "app="+testName4) + + assert.Equal(t, "", pod.Spec.Hostname) + } +} + /* Helpers */ type testConfig struct { diff --git a/admission-webhook/integration_tests/templates/simple-with-gmsa-hostname.yml b/admission-webhook/integration_tests/templates/simple-with-gmsa-hostname.yml new file mode 100644 index 00000000..e5e415b3 --- /dev/null +++ b/admission-webhook/integration_tests/templates/simple-with-gmsa-hostname.yml @@ -0,0 +1,30 @@ +## a simple deployment with a pod-level GMSA cred spec + +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: {{ .TestName }} + name: {{ .TestName }} + namespace: {{ .Namespace }} +spec: + replicas: 1 + selector: + matchLabels: + app: {{ .TestName }} + template: + metadata: + labels: + app: {{ .TestName }} + spec: + hostname: {{ .TestName }} + serviceAccountName: {{ .ServiceAccountName }} + securityContext: + windowsOptions: + gmsaCredentialSpecName: {{ index .CredSpecNames 0 }} + containers: + - image: registry.k8s.io/pause + name: nginx +{{- range $line := .ExtraSpecLines }} + {{ $line }} +{{- end }} diff --git a/admission-webhook/integration_tests/templates/simple-without-gmsa.yml b/admission-webhook/integration_tests/templates/simple-without-gmsa.yml new file mode 100644 index 00000000..adcf1b0e --- /dev/null +++ b/admission-webhook/integration_tests/templates/simple-without-gmsa.yml @@ -0,0 +1,26 @@ +## a simple deployment with a pod-level GMSA cred spec + +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: {{ .TestName }} + name: {{ .TestName }} + namespace: {{ .Namespace }} +spec: + replicas: 1 + selector: + matchLabels: + app: {{ .TestName }} + template: + metadata: + labels: + app: {{ .TestName }} + spec: + serviceAccountName: {{ .ServiceAccountName }} + containers: + - image: registry.k8s.io/pause + name: nginx +{{- range $line := .ExtraSpecLines }} + {{ $line }} +{{- end }} diff --git a/admission-webhook/main.go b/admission-webhook/main.go index 882138d4..118db591 100644 --- a/admission-webhook/main.go +++ b/admission-webhook/main.go @@ -22,7 +22,12 @@ func main() { panic(err) } - webhook := newWebhookWithOptions(kubeClient, WithCertReload(*enableCertReload)) + randomHostname := env_bool("RANDOM_HOSTNAME") + + options := []WebhookOption{WithCertReload(*enableCertReload)} + options = append(options, WithRandomHostname(randomHostname)) + + webhook := newWebhookWithOptions(kubeClient, options...) tlsConfig := &tlsConfig{ crtPath: env("TLS_CRT"), @@ -98,6 +103,20 @@ func env_float(key string, defaultFloat float32) float32 { return defaultFloat } +func env_bool(key string) bool { + if v, found := os.LookupEnv(key); found { + // Convert string to bool + if boolValue, err := strconv.ParseBool(v); err == nil { + return boolValue + } + // throw error if unable to parse + panic(fmt.Errorf("unable to parse environment variable %s with value %s to bool", key, v)) + } + + // return bool default value: false + return false +} + func env_int(key string, defaultInt int) int { if v, found := os.LookupEnv(key); found { if i, err := strconv.Atoi(v); err == nil { diff --git a/admission-webhook/main_test.go b/admission-webhook/main_test.go index b83826a7..98d4a21a 100644 --- a/admission-webhook/main_test.go +++ b/admission-webhook/main_test.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "os" "testing" ) @@ -86,3 +87,60 @@ func Test_env_int(t *testing.T) { }) } } + +func Test_env_bool(t *testing.T) { + tests := []struct { + name string + envkey string + envval string + want bool + }{ + { + name: "Environment variable set to true", + envkey: "TEST_ENV_BOOL", + envval: "true", + want: true, + }, + { + name: "Environment variable set to false", + envkey: "TEST_ENV_BOOL", + envval: "false", + want: false, + }, + { + name: "Environment variable not set", + envkey: "TEST_ENV_BOOL", + envval: "", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envval != "" { + os.Setenv(tt.envkey, tt.envval) + } else { + os.Unsetenv(tt.envkey) + } + if got := env_bool(tt.envkey); got != tt.want { + t.Errorf("env_bool() = %v, want %v", got, tt.want) + } + }) + } + + envkey := "TEST_ENV_BOOL" + envVal := "invalid" + // Test panic + defer func() { + if r := recover(); r == nil { + t.Errorf("The code did not panic") + } else { + t.Logf("Recovered from panic: %v", r) + if r.(error).Error() != fmt.Sprintf("unable to parse environment variable %s with value %s to bool", envkey, envVal) { + t.Errorf("Unexpected panic message: %v", r) + } + } + }() + + os.Setenv(envkey, envVal) + env_bool("TEST_ENV_BOOL") +} diff --git a/admission-webhook/utils_test.go b/admission-webhook/utils_test.go index 80c6cd74..4ba14d73 100644 --- a/admission-webhook/utils_test.go +++ b/admission-webhook/utils_test.go @@ -22,7 +22,6 @@ type dummyKubeClient struct { retrieveCredSpecContentsFunc func(ctx context.Context, credSpecName string) (contents string, httpCode int, err error) } - func (dkc *dummyKubeClient) isAuthorizedToUseCredSpec(ctx context.Context, serviceAccountName, namespace, credSpecName string) (authorized bool, reason string) { if dkc.isAuthorizedToUseCredSpecFunc != nil { return dkc.isAuthorizedToUseCredSpecFunc(ctx, serviceAccountName, namespace, credSpecName) @@ -59,6 +58,14 @@ func setWindowsOptions(winOptions *corev1.WindowsSecurityContextOptions, credSpe // case a `*corev1.WindowsSecurityContextOptions` is built using that string as the name of the cred spec to use. // Same goes for the values of `containerNamesAndWindowsOptions`. func buildPod(serviceAccountName string, podWindowsOptions *corev1.WindowsSecurityContextOptions, containerNamesAndWindowsOptions map[string]*corev1.WindowsSecurityContextOptions) *corev1.Pod { + return buildPodWithHostName(serviceAccountName, nil, podWindowsOptions, containerNamesAndWindowsOptions) +} + +// buildPod builds a pod for unit tests. +// `podWindowsOptions` should be either a full `*corev1.WindowsSecurityContextOptions` or a string, in which +// case a `*corev1.WindowsSecurityContextOptions` is built using that string as the name of the cred spec to use. +// Same goes for the values of `containerNamesAndWindowsOptions`. +func buildPodWithHostName(serviceAccountName string, hostname *string, podWindowsOptions *corev1.WindowsSecurityContextOptions, containerNamesAndWindowsOptions map[string]*corev1.WindowsSecurityContextOptions) *corev1.Pod { containers := make([]corev1.Container, len(containerNamesAndWindowsOptions)) i := 0 for name, winOptions := range containerNamesAndWindowsOptions { @@ -70,10 +77,16 @@ func buildPod(serviceAccountName string, podWindowsOptions *corev1.WindowsSecuri } shuffleContainers(containers) + podSpec := corev1.PodSpec{ ServiceAccountName: serviceAccountName, Containers: containers, } + + if hostname != nil { + podSpec.Hostname = *hostname + } + if podWindowsOptions != nil { podSpec.SecurityContext = &corev1.PodSecurityContext{WindowsOptions: podWindowsOptions} } diff --git a/admission-webhook/webhook.go b/admission-webhook/webhook.go index 7c2244bd..1b727358 100644 --- a/admission-webhook/webhook.go +++ b/admission-webhook/webhook.go @@ -13,6 +13,8 @@ import ( "strings" "time" + "github.com/google/uuid" + "github.com/sirupsen/logrus" admissionV1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" @@ -48,7 +50,8 @@ type podAdmissionError struct { } type WebhookConfig struct { - EnableCertReload bool + EnableCertReload bool + EnableRandomHostName bool } type WebhookOption func(*WebhookConfig) @@ -59,12 +62,18 @@ func WithCertReload(enabled bool) WebhookOption { } } +func WithRandomHostname(enabled bool) WebhookOption { + return func(cfg *WebhookConfig) { + cfg.EnableRandomHostName = enabled + } +} + func newWebhook(client kubeClientInterface) *webhook { return newWebhookWithOptions(client) } func newWebhookWithOptions(client kubeClientInterface, options ...WebhookOption) *webhook { - config := &WebhookConfig{EnableCertReload: false} + config := &WebhookConfig{EnableCertReload: false, EnableRandomHostName: false} for _, option := range options { option(config) @@ -358,9 +367,11 @@ func compareCredSpecContents(fromResource, fromCRD string) (bool, error) { // mutateCreateRequest inlines the requested GMSA's into the pod's and containers' `WindowsSecurityOptions` structs. func (webhook *webhook) mutateCreateRequest(ctx context.Context, pod *corev1.Pod) (*admissionV1.AdmissionResponse, *podAdmissionError) { var patches []map[string]string + hasGMSA := false if err := iterateOverWindowsSecurityOptions(pod, func(windowsOptions *corev1.WindowsSecurityContextOptions, resourceKind gmsaResourceKind, resourceName string, containerIndex int) *podAdmissionError { if credSpecName := windowsOptions.GMSACredentialSpecName; credSpecName != nil { + hasGMSA = true // if the user has pre-set the GMSA's contents, we won't override it - it'll be down // to the validation endpoint to make sure the contents actually are what they should if credSpecContents := windowsOptions.GMSACredentialSpec; credSpecContents == nil { @@ -390,8 +401,23 @@ func (webhook *webhook) mutateCreateRequest(ctx context.Context, pod *corev1.Pod return nil, err } - admissionResponse := &admissionV1.AdmissionResponse{Allowed: true} + if hasGMSA && webhook.config.EnableRandomHostName { + // Pods are GMSA related, Env enabled, patch the hostname only if it is empty + hostName := pod.Spec.Hostname + if hostName == "" { + hostName = generateUUID() + patches = append(patches, map[string]string{ + "op": "add", + "path": "/spec/hostname", + "value": hostName, + }) + } else { + // Will honor the hostname set in the spec, print out a message + logrus.Warnf("hostname is set in spec and will be hornored instead of being randomized") + } + } + admissionResponse := &admissionV1.AdmissionResponse{Allowed: true} if len(patches) != 0 { patchesBytes, err := json.Marshal(patches) if err != nil { @@ -537,3 +563,11 @@ func (ln tcpKeepAliveListener) Accept() (net.Conn, error) { tc.SetKeepAlivePeriod(3 * time.Minute) return tc, nil } + +func generateUUID() string { + // Generate a new UUID + id := uuid.New() + // Convert to string and get the first 15 characters in lower case + shortUUID := strings.ToLower(id.String()[:15]) + return shortUUID +} diff --git a/admission-webhook/webhook_test.go b/admission-webhook/webhook_test.go index 5c6c354a..d1f947ab 100644 --- a/admission-webhook/webhook_test.go +++ b/admission-webhook/webhook_test.go @@ -186,7 +186,7 @@ func TestMutateCreateRequest(t *testing.T) { }, } { t.Run(testCaseName, func(t *testing.T) { - webhook := newWebhook(nil) + webhook := newWebhookWithOptions(nil, WithRandomHostname(false)) pod := buildPod(dummyServiceAccoutName, winOptionsFactory(), map[string]*corev1.WindowsSecurityContextOptions{dummyContainerName: winOptionsFactory()}) response, err := webhook.mutateCreateRequest(context.Background(), pod) @@ -194,9 +194,86 @@ func TestMutateCreateRequest(t *testing.T) { require.NotNil(t, response) assert.True(t, response.Allowed) + + assert.Nil(t, response.Patch) + }) } + for testCaseName, winOptionsFactory := range map[string]func() *corev1.WindowsSecurityContextOptions{ + "with random hostname env set and empty GMSA settings, it passes and does nothing": func() *corev1.WindowsSecurityContextOptions { + return &corev1.WindowsSecurityContextOptions{} + }, + "with random hostname env set and no GMSA settings, it passes and does nothing": func() *corev1.WindowsSecurityContextOptions { + return nil + }, + } { + t.Run(testCaseName, func(t *testing.T) { + webhook := newWebhookWithOptions(nil, WithRandomHostname(true)) + pod := buildPod(dummyServiceAccoutName, winOptionsFactory(), map[string]*corev1.WindowsSecurityContextOptions{dummyContainerName: winOptionsFactory()}) + + response, err := webhook.mutateCreateRequest(context.Background(), pod) + assert.Nil(t, err) + + require.NotNil(t, response) + assert.True(t, response.Allowed) + + assert.Nil(t, response.Patch) + + }) + } + + testCaseName, winOptionsFactory1 := "with random hostname env set and dummy GMSA settings, it passes and set random hostname", func() *corev1.WindowsSecurityContextOptions { + dummyCredSpecNameVar := dummyCredSpecName + dummyCredSpecContentsVar := dummyCredSpecContents + return &corev1.WindowsSecurityContextOptions{GMSACredentialSpecName: &dummyCredSpecNameVar, GMSACredentialSpec: &dummyCredSpecContentsVar} + } + t.Run(testCaseName, func(t *testing.T) { + webhook := newWebhookWithOptions(nil, WithRandomHostname(true)) + pod := buildPod(dummyServiceAccoutName, winOptionsFactory1(), map[string]*corev1.WindowsSecurityContextOptions{dummyContainerName: winOptionsFactory1()}) + + response, err := webhook.mutateCreateRequest(context.Background(), pod) + assert.Nil(t, err) + + require.NotNil(t, response) + assert.True(t, response.Allowed) + + var patches []map[string]string + // one more because we're adding the hostname + if err := json.Unmarshal(response.Patch, &patches); assert.Nil(t, err) && assert.Equal(t, 1, len(patches)) { + foundHostname := false + for _, patch := range patches { + if value, hasValue := patch["value"]; assert.True(t, hasValue) { + if patch["path"] == "/spec/hostname" { + foundHostname = true + assert.Equal(t, "add", patch["op"]) + assert.Equal(t, 15, len(value)) + } + } + } + assert.True(t, foundHostname) + } + }) + + testCaseName, winOptionsFactory1 = "with random hostname env set and dummy GMSA settings and hostname set in spec, it passes and do nothing", func() *corev1.WindowsSecurityContextOptions { + dummyCredSpecNameVar := dummyCredSpecName + dummyCredSpecContentsVar := dummyCredSpecContents + return &corev1.WindowsSecurityContextOptions{GMSACredentialSpecName: &dummyCredSpecNameVar, GMSACredentialSpec: &dummyCredSpecContentsVar} + } + t.Run(testCaseName, func(t *testing.T) { + webhook := newWebhookWithOptions(nil, WithRandomHostname(true)) + dummyPodNameVar := dummyPodName + pod := buildPodWithHostName(dummyServiceAccoutName, &dummyPodNameVar, winOptionsFactory1(), map[string]*corev1.WindowsSecurityContextOptions{dummyContainerName: winOptionsFactory1()}) + + response, err := webhook.mutateCreateRequest(context.Background(), pod) + assert.Nil(t, err) + + require.NotNil(t, response) + assert.True(t, response.Allowed) + + assert.Nil(t, response.Patch) + }) + kubeClientFactory := func() *dummyKubeClient { return &dummyKubeClient{ retrieveCredSpecContentsFunc: func(ctx context.Context, credSpecName string) (contents string, httpCode int, err error) { @@ -215,8 +292,8 @@ func TestMutateCreateRequest(t *testing.T) { } runWebhookValidateOrMutateTests(t, winOptionsFactory, map[string]webhookValidateOrMutateTest{ - "with a GMSA cred spec name, it passes and inlines the cred-spec's contents": func(t *testing.T, pod *corev1.Pod, optionsSelector winOptionsSelector, resourceKind gmsaResourceKind, resourceName string) { - webhook := newWebhook(kubeClientFactory()) + "with random hostname env and a GMSA cred spec name, it passes and inlines the cred-spec's contents and generate random hostname": func(t *testing.T, pod *corev1.Pod, optionsSelector winOptionsSelector, resourceKind gmsaResourceKind, resourceName string) { + webhook := newWebhookWithOptions(kubeClientFactory(), WithRandomHostname(true)) setWindowsOptions(optionsSelector(pod), dummyCredSpecName, "") @@ -269,17 +346,25 @@ func TestMutateCreateRequest(t *testing.T) { } var patches []map[string]string - if err := json.Unmarshal(response.Patch, &patches); assert.Nil(t, err) && assert.Equal(t, len(pod.Spec.Containers), len(patches)) { + // len(pod.Spec.Containers)+1 because we're adding the hostname + if err := json.Unmarshal(response.Patch, &patches); assert.Nil(t, err) && assert.Equal(t, len(pod.Spec.Containers)+1, len(patches)) { + foundHostname := false for _, patch := range patches { if value, hasValue := patch["value"]; assert.True(t, hasValue) { - if expectedPatch, present := expectedPatches[value]; assert.True(t, present, "value %s not found in expected patches", value) { + if patch["path"] == "/spec/hostname" { + foundHostname = true + assert.Equal(t, "add", patch["op"]) + assert.Equal(t, 15, len(value)) + } else if expectedPatch, present := expectedPatches[value]; assert.True(t, present, "value %s not found in expected patches", value) { assert.Equal(t, expectedPatch, patch) } } } + assert.True(t, foundHostname) } }, + // random hostname env not set in the following cases, and validated no hostname is set (implicitly) "it the cred spec's contents are already set, along with its name, it passes and doesn't overwrite the provided contents": func(t *testing.T, pod *corev1.Pod, optionsSelector winOptionsSelector, _ gmsaResourceKind, _ string) { webhook := newWebhook(kubeClientFactory()) @@ -421,8 +506,11 @@ func TestDefaultWebhookConfig(t *testing.T) { func TestSetWebhookConfig(t *testing.T) { expectedCertReload := true - webhook := newWebhookWithOptions(nil, WithCertReload(expectedCertReload)) + expectedRandomHostname := true + randomHostname := true + webhook := newWebhookWithOptions(nil, WithCertReload(expectedCertReload), WithRandomHostname(randomHostname)) assert.Equal(t, expectedCertReload, webhook.config.EnableCertReload) + assert.Equal(t, expectedRandomHostname, webhook.config.EnableRandomHostName) } func TestEqualStringPointers(t *testing.T) { diff --git a/charts/gmsa/templates/deployment.yaml b/charts/gmsa/templates/deployment.yaml index 3eebd509..f2f5d390 100644 --- a/charts/gmsa/templates/deployment.yaml +++ b/charts/gmsa/templates/deployment.yaml @@ -57,6 +57,8 @@ spec: value: "{{ .Values.burst }}" - name: QPS value: "{{ .Values.qps }}" + - name: RANDOM_HOSTNAME + value: "{{ .Values.randomHostname }}" {{- if .Values.securityContext }} securityContext: {{ toYaml .Values.securityContext | nindent 12 }} {{- end }} diff --git a/charts/gmsa/values.yaml b/charts/gmsa/values.yaml index e5bc1be6..5e9e8f50 100644 --- a/charts/gmsa/values.yaml +++ b/charts/gmsa/values.yaml @@ -51,3 +51,4 @@ securityContext: {} tolerations: [] qps: 30.0 burst: 50 +randomHostname: false