Remove serviceaccount for game server container

This mounts an emptydir over the service account token
that is automatically mounted in the container that runs
the game server binary.

Since this is exposed to the outside world, removing the serviceaccount
token removes authentication against the rest of the Kubernetes cluster
if it ever gets compromised.

Closes googleforgames#150

pkg/apis/stable/v1alpha1/gameserver.go

@@ -354,6 +354,19 @@ func (gs *GameServer) FindGameServerContainer() (int, corev1.Container, error) {
 	return -1, corev1.Container{}, errors.Errorf("Could not find a container named %s", gs.Spec.Container)
 }
 
+// ApplyToPodGameServerContainer applies func(v1.Container) to the pod's gameserver container
+func (gs *GameServer) ApplyToPodGameServerContainer(pod *corev1.Pod, f func(corev1.Container) corev1.Container) *corev1.Pod {
+	for i, c := range pod.Spec.Containers {
+		if c.Name == gs.Spec.Container {
+			c = f(c)
+			pod.Spec.Containers[i] = c
+			break
+		}
+	}
+
+	return pod
+}
+
 // Pod creates a new Pod from the PodTemplateSpec
 // attached to the GameServer resource
 func (gs *GameServer) Pod(sidecars ...corev1.Container) (*corev1.Pod, error) {
@@ -364,8 +377,13 @@ func (gs *GameServer) Pod(sidecars ...corev1.Container) (*corev1.Pod, error) {
 
 	gs.podObjectMeta(pod)
 
+	// if the service account is not set, then you are in the "opinionated"
+	// mode. If the user sets the service account, we assume they know what they are
+	// doing, and don't disable the gameserver container.
+	// TODO: write tests for this.
 	if pod.Spec.ServiceAccountName == "" {
 		pod.Spec.ServiceAccountName = SidecarServiceAccountName
+		gs.disableServiceAccount(pod)
 	}
 
 	i, gsContainer, err := gs.FindGameServerContainer()
@@ -457,6 +475,21 @@ func (gs *GameServer) podScheduling(pod *corev1.Pod) {
 	}
 }
 
+// disableServiceAccount disables the service account for the gameserver container
+// TODO: move tests
+func (gs *GameServer) disableServiceAccount(pod *corev1.Pod) {
+	// gameservers don't get access to the k8s api.
+	emptyVol := corev1.Volume{Name: "empty", VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}
+	pod.Spec.Volumes = append(pod.Spec.Volumes, emptyVol)
+	mount := corev1.VolumeMount{MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", Name: emptyVol.Name, ReadOnly: true}
+
+	gs.ApplyToPodGameServerContainer(pod, func(c corev1.Container) corev1.Container {
+		c.VolumeMounts = append(c.VolumeMounts, mount)
+
+		return c
+	})
+}
+
 // HasPortPolicy checks if there is a port with a given
 // PortPolicy
 func (gs *GameServer) HasPortPolicy(policy PortPolicy) bool {

pkg/apis/stable/v1alpha1/gameserver_test.go

@@ -401,6 +401,28 @@ func TestGameServerPodScheduling(t *testing.T) {
 	})
 }
 
+func TestGameServerDisableServiceAccount(t *testing.T) {
+	t.Parallel()
+
+	gs := &GameServer{ObjectMeta: metav1.ObjectMeta{Name: "gameserver", UID: "1234"}, Spec: GameServerSpec{
+		Template: corev1.PodTemplateSpec{
+			Spec: corev1.PodSpec{
+				Containers: []corev1.Container{{Name: "container", Image: "container/image"}},
+			},
+		}}}
+
+	gs.ApplyDefaults()
+	pod, err := gs.Pod()
+	assert.NoError(t, err)
+	assert.Len(t, pod.Spec.Containers, 1)
+	assert.Empty(t, pod.Spec.Containers[0].VolumeMounts)
+
+	gs.disableServiceAccount(pod)
+	assert.Len(t, pod.Spec.Containers, 1)
+	assert.Len(t, pod.Spec.Containers[0].VolumeMounts, 1)
+	assert.Equal(t, "/var/run/secrets/kubernetes.io/serviceaccount", pod.Spec.Containers[0].VolumeMounts[0].MountPath)
+}
+
 func TestGameServerCountPorts(t *testing.T) {
 	fixture := &GameServer{Spec: GameServerSpec{Ports: []GameServerPort{
 		{PortPolicy: Dynamic},
@@ -425,7 +447,7 @@ func TestGameServerPatch(t *testing.T) {
 
 	assert.Contains(t, string(patch), `{"op":"replace","path":"/spec/container","value":"bear"}`)
 }
-func TestGetDevAddress(t *testing.T) {
+func TestGameServerGetDevAddress(t *testing.T) {
 	devGs := &GameServer{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        "dev-game",
@@ -451,3 +473,35 @@ func TestGetDevAddress(t *testing.T) {
 	assert.False(t, isDev, "dev-game should NOT have a dev-address")
 	assert.Equal(t, "", devAddress, "dev-address IP address should be 127.1.1.1")
 }
+
+func TestGameServerApplyToPodGameServerContainer(t *testing.T) {
+	t.Parallel()
+
+	name := "mycontainer"
+	gs := &GameServer{
+		Spec: GameServerSpec{
+			Container: name,
+			Template: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{Name: name, Image: "foo/mycontainer"},
+						{Name: "notmycontainer", Image: "foo/notmycontainer"},
+					},
+				},
+			},
+		},
+	}
+
+	p1 := &corev1.Pod{Spec: *gs.Spec.Template.Spec.DeepCopy()}
+
+	p2 := gs.ApplyToPodGameServerContainer(p1, func(c corev1.Container) corev1.Container {
+		//  easy thing to change and test for
+		c.TTY = true
+
+		return c
+	})
+
+	assert.Len(t, p2.Spec.Containers, 2)
+	assert.True(t, p2.Spec.Containers[0].TTY)
+	assert.False(t, p2.Spec.Containers[1].TTY)
+}

pkg/gameservers/controller.go

@@ -595,27 +595,27 @@ func (c *Controller) sidecar(gs *v1alpha1.GameServer) corev1.Container {
 
 // addGameServerHealthCheck adds the http health check to the GameServer container
 func (c *Controller) addGameServerHealthCheck(gs *v1alpha1.GameServer, pod *corev1.Pod) {
-	if !gs.Spec.Health.Disabled {
-		for i, c := range pod.Spec.Containers {
-			if c.Name == gs.Spec.Container {
-				if c.LivenessProbe == nil {
-					c.LivenessProbe = &corev1.Probe{
-						Handler: corev1.Handler{
-							HTTPGet: &corev1.HTTPGetAction{
-								Path: "/gshealthz",
-								Port: intstr.FromInt(8080),
-							},
-						},
-						InitialDelaySeconds: gs.Spec.Health.InitialDelaySeconds,
-						PeriodSeconds:       gs.Spec.Health.PeriodSeconds,
-						FailureThreshold:    gs.Spec.Health.FailureThreshold,
-					}
-					pod.Spec.Containers[i] = c
-				}
-				break
+	if gs.Spec.Health.Disabled {
+		return
+	}
+
+	gs.ApplyToPodGameServerContainer(pod, func(c corev1.Container) corev1.Container {
+		if c.LivenessProbe == nil {
+			c.LivenessProbe = &corev1.Probe{
+				Handler: corev1.Handler{
+					HTTPGet: &corev1.HTTPGetAction{
+						Path: "/gshealthz",
+						Port: intstr.FromInt(8080),
+					},
+				},
+				InitialDelaySeconds: gs.Spec.Health.InitialDelaySeconds,
+				PeriodSeconds:       gs.Spec.Health.PeriodSeconds,
+				FailureThreshold:    gs.Spec.Health.FailureThreshold,
 			}
 		}
-	}
+
+		return c
+	})
 }
 
 // syncGameServerStartingState looks for a pod that has been scheduled for this GameServer

site/content/en/docs/Advanced/service-accounts.md

@@ -0,0 +1,46 @@
+---
+title: "GameServer Pod Service Accounts"
+linkTitle: "Service Accounts"
+date: 2019-03-14T04:30:37Z
+publishDate: 2019-04-01
+description: >
+  RBAC permissions and service accounts for the `GameServer` Pod. 
+---
+
+## Default Settings
+
+By default, Agones sets up service accounts and sets them appropriately for the `Pods` that are created for `GameServers`.
+
+Since Agones provides `GameServer` `Pods` with a sidecar container that needs access to Agones Custom Resource Definitions,
+`Pods` are configured with a service account with extra RBAC permissions to ensure that it can read and modify the resources it needs.
+
+Since service accounts apply to all containers in a `Pod`, Agones will automatically overwrite the mounted key for the 
+service account in the container that is running the dedicate game server in the backing `Pod`. This is done 
+since game server containers are exposed publicly, and generally dom't require the extra permissions to access aspects 
+of the Kubernetes API.
+
+## Bringing your own Service Account
+
+If needed, you can provide your own service account on the `Pod` specification in the `GameServer` configuration.
+
+For example:
+
+```yaml
+apiVersion: "stable.agones.dev/v1alpha1"
+kind: GameServer
+metadata:
+  generateName: "simple-udp-"
+spec:
+  ports:
+  - name: default
+    containerPort: 7654
+  template:
+    spec:
+      serviceAccountName: my-special-service-account # a custom service account
+      containers:
+      - name: simple-udp
+        image: gcr.io/agones-images/udp-server:0.7
+```
+
+If a service account is configured, the mounted key is not overwritten, as it assumed that you want to have full control
+of the service account and underlying RBAC permissions.