GameServer definition validation

[markmandel]

Currently there is no validation on a GameServer

Once Kubernetes 1.9 is available on GKE, this can be implemented in the master branch.

We will likely need a combination of CRD validation, as well as webhook validation on creation and mutation.

List of validations

• Container should refer to an existing container in the PodTemplateSpec, optional if only one container is defined otherwise mandatory.
• PortPolicy should be either static or dynamic, optional, defaults to dynamic
• ContainerPort should be a valid tcp/udp port 0-65535, required.
• HostPort should be a valid port 0-65535, should only be passed if PortPolicy == static
• Protocol should be TCP or UDP only, optional - defaults to UDP
• A valid PodTemplate....
• health
  • disabled: boolean
  • PeriodSeconds, InitialDelaySeconds, FailureThreshold valid positive integer (max 2**31) - all optional as they have defaults and/or not required if health is disabled

Research

• http://blog.kubernetes.io/2018/01/introducing-client-go-version-6.html
• CRD Validation configuration
  • https://kubernetes.io/docs/tasks/access-kubernetes-api/extend-api-custom-resource-definitions/#validation
  • https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#schemaObject
  • https://tools.ietf.org/html/draft-wright-json-schema-validation-00
• Webhooks
  • https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.9/#validatingwebhookconfiguration-v1beta1-admissionregistration
• http://blog.kubernetes.io/2018/01/extensible-admission-is-beta.html
• https://kubernetes.io/docs/admin/extensible-admission-controllers/

[cyriltovena]

So we should add validations inside the install.yaml at the CRD level using OpenAPI spec ?

List of required validations :

• Container should refer to an existing container in the PodTemplateSpec, optional if only one container is defined otherwise mandatory.
• PortPolicy should be either static or dynamic, optional ?
• ContainerPort should be a valid udp port 0-65535, required.
• HostPort should be a valid port 0-65535, should only be passed if PortPolicy == dynamic
• Protocol should be TCP or UDP only, required.
• A valid PodTemplate....
• health if enabled.
  • PeriodSeconds, InitialDelaySeconds, FailureThreshold valid positive integer (max 2**31)

Anything else ?

Also I see that you listed webhooks and admission controller, I guess you want to use the validation one for complex scenarios ?

[markmandel]

You got most of it - updated the description above to be the canonical reference.

Regarding the validation webhook - I'd be surprised if the OpenAPI spec can cover all of the above scenarios - e.g. "Container should refer to an existing container in the PodTemplateSpec, optional if only one container is defined otherwise mandatory.", so leaning on the validation webhook seems like the appropriate response.

These features are only available on 1.9 - I started a bit of work on this branch - but I'm currently blocked by #57 (although I could turn off RBAC if need be, at least for development).

That being said, if you wanted to start on the OpenSpec part of this, we test it and keep it in reserve until the support for 1.9 is fully integrated.

[cyriltovena]

I wanted to also tackle the validation webhook, but I could not find a documentation on how to self host your own api in the api-server which seems the best option.

They mostly documented what to use and how to setup the webhook.

[markmandel]

@Kuqd yeah, it's not well documented.

My task for next week is to build a basic MutatingWebhookConfiguration to set the default value on the GameServer when it first gets created (I need to work through some of the undocumented features with some members from sig-apimachinery) - but once I've got that done, that will setup the infrastructure for this ticket and also #70 .

[cyriltovena]

found some documentation https://github.com/openshift/generic-admission-server if that can help

[markmandel]

I've got (the receiving) of a basic MutatingWebhookConfiguration in this gameserver-mutate-webhook - which should be the same structure as the ValidatingWebhookConfiguration

I'm in the process of making a small library to make adding new webhook configurations simpler without having to build as much of the plumbing yourself - it's getting there.

I'm not sure how to candle the ca certs, but for now - keeping things simple and keeping them in the repo.

[markmandel]

Kubernetes 1.9 is now merged - so we can at least now start on the CRD/yaml part of the validation.

[cyriltovena]

@markmandel Do you expect OpenAPI to also validate the PodSpecTemplate ? if yes I guess the bare minimun ? Which is one container with a name and image.

[markmandel]

You ask a good question. I'm leaning towards "no" just for simplicity sake. The template should be required though. WDYT?

If the pod template is bad, the GameServer will go into an Error state, and have an event attached with a message, so if there is an issue, it will be caught by the system.
https://github.com/googleprivate/agones/blob/master/pkg/gameservers/controller.go#L456

As an aside - I swear I found a github repo witth all of the standard Kubernetes resources as OpenAPI specs, but I can't find it now.

[cyriltovena]

I might be able to reference it if you do ! At least I could try.

[markmandel]

Ah this was what I found - https://github.com/garethr/kubernetes-json-schema

Not sure if it's actually helpful. It seems to reference existing OpenAPI schemas, but I'm not sure where they are. I'm having a bit of an ask around to see what I can find.

I wonder if there is a way to reference the existing schema?

[cyriltovena]

it's there https://raw.githubusercontent.com/kubernetes/kubernetes/master/api/openapi-spec/swagger.json
but I tried and go this

The CustomResourceDefinition "gameservers.stable.agones.dev" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[template].$ref: Forbidden: $ref is not supported

[markmandel]

Erk 😞

I'll ping some people on my end, see if I can find any information - but sounds like we're going to skip the PodTemplate for now 😢

[cyriltovena]

I can copy the most important part which is the containers part right ?

[cyriltovena]

I'm having issue with arrays.
Never mind dam yaml tabulation.

[markmandel]

Sounds reasonable to me 👍

[markmandel]

Just found out - refs are coming, just not there yet -
kubernetes/kubernetes#54579

[markmandel]

@Kuqd for organisation sake, should we assign this ticket to you? (are you working on the webhook side of things as well?)

[cyriltovena]

Yes assign it to me.

My plan was to add more validation in the mutating webhook like you started is it ok ?

[markmandel]

Yep - was just chatting with @dzlier-gcp about who was working on what, so just wanted to make sure.

👍