Merge pull request #155 from zylxjtu/dev

 Create random hostname for GMSA

Author: k8s-ci-robot

.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
 

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 {

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 }}

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 }}

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 {

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")
+}

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}
 	}

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
+}