From fe0abc9d16333a666516f26496d453ae75c4b90f Mon Sep 17 00:00:00 2001 From: Mark Mandel Date: Fri, 22 Feb 2019 18:11:35 +1100 Subject: [PATCH] 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 #150 --- pkg/apis/stable/v1alpha1/gameserver.go | 31 ++++++++++ pkg/apis/stable/v1alpha1/gameserver_test.go | 56 ++++++++++++++++++- pkg/gameservers/controller.go | 38 ++++++------- .../en/docs/Advanced/service-accounts.md | 46 +++++++++++++++ 4 files changed, 151 insertions(+), 20 deletions(-) create mode 100644 site/content/en/docs/Advanced/service-accounts.md diff --git a/pkg/apis/stable/v1alpha1/gameserver.go b/pkg/apis/stable/v1alpha1/gameserver.go index d3042f88f8..cc5e864ed4 100644 --- a/pkg/apis/stable/v1alpha1/gameserver.go +++ b/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,12 @@ 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. if pod.Spec.ServiceAccountName == "" { pod.Spec.ServiceAccountName = SidecarServiceAccountName + gs.disableServiceAccount(pod) } i, gsContainer, err := gs.FindGameServerContainer() @@ -457,6 +474,20 @@ func (gs *GameServer) podScheduling(pod *corev1.Pod) { } } +// disableServiceAccount disables the service account for the gameserver container +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 { diff --git a/pkg/apis/stable/v1alpha1/gameserver_test.go b/pkg/apis/stable/v1alpha1/gameserver_test.go index fd5ba98407..caaaae3438 100644 --- a/pkg/apis/stable/v1alpha1/gameserver_test.go +++ b/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) +} diff --git a/pkg/gameservers/controller.go b/pkg/gameservers/controller.go index d1f4594ee4..2af88034b7 100644 --- a/pkg/gameservers/controller.go +++ b/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 diff --git a/site/content/en/docs/Advanced/service-accounts.md b/site/content/en/docs/Advanced/service-accounts.md new file mode 100644 index 0000000000..c3fb24ab71 --- /dev/null +++ b/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. \ No newline at end of file