-
Notifications
You must be signed in to change notification settings - Fork 816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MutatingWebHookConfiguration for GameServer creation & Validation. #95
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
#!/usr/bin/env bash | ||
|
||
echo "Generating certs..." | ||
echo "Email should be: [email protected]" | ||
echo "Common Name should be: agones-controller-service.agones-system.svc" | ||
openssl genrsa -out server.key 2048 | ||
openssl req -new -x509 -sha256 -key server.key -out server.crt -days 3650 | ||
|
||
echo "caBundle:" | ||
base64 -w 0 server.crt | ||
|
||
echo "done" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
-----BEGIN CERTIFICATE----- | ||
MIIEKTCCAxGgAwIBAgIJAOJP6410wvGSMA0GCSqGSIb3DQEBCwUAMIGqMQswCQYD | ||
VQQGEwJVUzETMBEGA1UECAwKU29tZS1TdGF0ZTEPMA0GA1UECgwGQWdvbmVzMQ8w | ||
DQYDVQQLDAZBZ29uZXMxNDAyBgNVBAMMK2Fnb25lcy1jb250cm9sbGVyLXNlcnZp | ||
Y2UuYWdvbmVzLXN5c3RlbS5zdmMxLjAsBgkqhkiG9w0BCQEWH2Fnb25lcy1kaXNj | ||
dXNzQGdvb2dsZWdyb3Vwcy5jb20wHhcNMTgwMjE0MDQ0NDQ2WhcNMjgwMjEyMDQ0 | ||
NDQ2WjCBqjELMAkGA1UEBhMCVVMxEzARBgNVBAgMClNvbWUtU3RhdGUxDzANBgNV | ||
BAoMBkFnb25lczEPMA0GA1UECwwGQWdvbmVzMTQwMgYDVQQDDCthZ29uZXMtY29u | ||
dHJvbGxlci1zZXJ2aWNlLmFnb25lcy1zeXN0ZW0uc3ZjMS4wLAYJKoZIhvcNAQkB | ||
Fh9hZ29uZXMtZGlzY3Vzc0Bnb29nbGVncm91cHMuY29tMIIBIjANBgkqhkiG9w0B | ||
AQEFAAOCAQ8AMIIBCgKCAQEAzgVT90ejxNnwCo/OjMD26fU4drkSefwdQgwibifa | ||
l8rk6Y0VvOIV1H+lRowe06mWNuR5FOXFA0fXlvxCKKYVQpSPEK2YSyh/aSnJQL6q | ||
o8eAYTJBkOYLB5CbzIziWeoQfOYN8MlEn8bXJdieJhHH8UnyjtyoTlxzhZmX+pft | ||
hUdea3VkzO21n4+QE3RX5f132FTFcuqXOUA/pi8ccANGc3zjLeZJvA9odPEhGf7g | ||
C8eyA68SVcsh+Pjz0lw95APvlMv1jmqURFWcESTLaQ0Fx5KwRyV0ziZmUvAAF2Zx | ||
DZhHUcoFPHAwTl75NAhnHpMlLNp5L7tcVdyT8B2GRs+slQIDAQABo1AwTjAdBgNV | ||
HQ4EFgQUgv1nTPaQJSN3Lqm5jIjW4xHmdG0wHwYDVR0jBBgwFoAUgv1nTPaQJSN3 | ||
Lqm5jIjW4xHmdG0wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAHKEC | ||
ktEjYNUCA+mysz4orW7pRUvhBHDVSgsY6eEVRLzf/1yIZE0u656kpK6OT7MhJGlU | ||
KwGSSoUBBzVgUsZjDm7Pgbk4iYzn4M1xLzbLQBr3Mc5zXHefPvbimhD55cLzpVFu | ||
VQmBmZV2NjU5DuSdRndlcPaNcg/uOcvZK4KY1KCBA3EoAQIkpzHX2iUMoxiRvZVN | ||
NEwgFTtIGBYn0HfL/vgOsH8fVrMUkuG2vhGdeXBpZiq/BZJbZeN2rCf2gaX1QIv0 | ||
EKbcuFqM98WT5ZVZRtX1Y3Rwevg4mxYJXCuH6FF9W9/Sz294Fy2OBKB8JAVaEx9n | ||
1/i6fIffGnHTXWHsVQ== | ||
-----END CERTIFICATE----- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
-----BEGIN RSA PRIVATE KEY----- | ||
MIIEpAIBAAKCAQEAzgVT90ejxNnwCo/OjMD26fU4drkSefwdQgwibifal8rk6Y0V | ||
vOIV1H+lRowe06mWNuR5FOXFA0fXlvxCKKYVQpSPEK2YSyh/aSnJQL6qo8eAYTJB | ||
kOYLB5CbzIziWeoQfOYN8MlEn8bXJdieJhHH8UnyjtyoTlxzhZmX+pfthUdea3Vk | ||
zO21n4+QE3RX5f132FTFcuqXOUA/pi8ccANGc3zjLeZJvA9odPEhGf7gC8eyA68S | ||
Vcsh+Pjz0lw95APvlMv1jmqURFWcESTLaQ0Fx5KwRyV0ziZmUvAAF2ZxDZhHUcoF | ||
PHAwTl75NAhnHpMlLNp5L7tcVdyT8B2GRs+slQIDAQABAoIBAEoU5GKQ4jTQ4V4K | ||
5Az8/kyWnx0h46D1pVewoVjW/+WBUdshnmVzLsJgu/+oNxWJb7iBY4C+Np+9X6qt | ||
PuT7A74TSXaH1bGA+H/KRNIBPb7y6BkLR0RhVCn+N+fP6TzHy/H9j5m75e9GQusa | ||
/5NU5X7ARnZUpji3SdsKpfm4U/KOV+p2jWSPX7HOBu/KYa1jveCt6JMPQ36KBXkR | ||
MlVCkADcvAAGuObpa/sm0MA63+ihdeSYhkEXKqxH4Az3PVDLwP80T8f5VqwWYmnq | ||
L/Bg6HnV5GHnlTXA+WepNbHkokN9G0H6m0Itj4al3bGTB4jeBCJZp5FHW5obJ4qP | ||
WkcfXcECgYEA8UIZvdNSsSpRZVTe4hhtXncOptFTdoVKIzTuUVKfKe9OB5i32B9u | ||
4hDNpBDqkgEktg4R8/cn57z6ZUdUCcobjWQbJLXE5wp7Htf7jCGnL2QmIcp20rbF | ||
HrE2lHr0oNr3MLUruBvArVB3LLGmOEgn3NQw9aJwg/542EDS7x5v3XkCgYEA2pwG | ||
HiGYXTJfYNg/+SmDuSDWFe4iMPTRw1TT85uGE9UErYPQd8OqYQQOgIAT7xp1z0oq | ||
6pmsjPO6HZng9oHKeAO/JPeSuXE+bJ8HYKp7W929F+LGJoqitOLPMm/9OovYDCvh | ||
6+qDY5LNRHdAeTqwwI2yugf4YnBjY6zIfBp5LP0CgYEAyrrv5JqSbzuPQGZMEJPU | ||
O8Ax+K4Hw52HygPtizqxcsybtjh3rE3loGPcWdS5OE1rquwx299BkjMz+i0xCjTi | ||
aDLJuFRiDH+7LBT0VTHmSiWPAXAf3zskc4EYyzZzIEQ/2Zc0ELaJd1oZet4hPkQr | ||
8x3/sjl48QHCTH5UggkCmYkCgYAfK/pPV5kDSQiCpbNRkxLeVglQ7Tjg5Df482KZ | ||
rQaMU2asW0xhl3v3A34R4rF0+b/sw/WkqC8LlkFmsSd73vwA6v/ZhJfea4BsOqzx | ||
or2eVtr8yfBZVJFo26KR3ZgtPf2blrJLUpBTpX4xkhOWdcD4Y/wlPLe1SbNSZjPc | ||
RmYa/QKBgQC6a7zNASP0A0qMPhqyJZlbg4F8WMRtgHFfsy/iLwdiEUGuChQWULkc | ||
hzPWpjD0zwrSxZtyhSL0b6THnqTiin23OvtINlViZmos5mWW0VeGsbHnJuRa3THy | ||
EjSkSP4mR7tJlJokboZ+lYL7gABHn2ERnDsqX8oEU6QpDP1rZhYSzg== | ||
-----END RSA PRIVATE KEY----- |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,13 +18,16 @@ package main | |
import ( | ||
"strings" | ||
"time" | ||
"os" | ||
"path/filepath" | ||
|
||
"agones.dev/agones/pkg" | ||
"agones.dev/agones/pkg/client/clientset/versioned" | ||
"agones.dev/agones/pkg/client/informers/externalversions" | ||
"agones.dev/agones/pkg/gameservers" | ||
"agones.dev/agones/pkg/util/runtime" | ||
"agones.dev/agones/pkg/util/signals" | ||
"agones.dev/agones/pkg/util/webhooks" | ||
"github.com/sirupsen/logrus" | ||
"github.com/spf13/pflag" | ||
"github.com/spf13/viper" | ||
|
@@ -39,6 +42,8 @@ const ( | |
pullSidecarFlag = "always-pull-sidecar" | ||
minPortFlag = "min-port" | ||
maxPortFlag = "max-port" | ||
certFileFlag = "cert-file" | ||
keyFileFlag = "key-file" | ||
) | ||
|
||
func init() { | ||
|
@@ -47,30 +52,46 @@ func init() { | |
|
||
// main starts the operator for the gameserver CRD | ||
func main() { | ||
exec, err := os.Executable() | ||
if err != nil { | ||
logrus.WithError(err).Fatal("Could not get executable path") | ||
} | ||
|
||
base := filepath.Dir(exec) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a cool trick. Haven't see this before. 👍 |
||
viper.SetDefault(sidecarFlag, "gcr.io/agones-images/agones-sdk:"+pkg.Version) | ||
viper.SetDefault(pullSidecarFlag, false) | ||
viper.SetDefault(certFileFlag, filepath.Join(base, "certs/server.crt")) | ||
viper.SetDefault(keyFileFlag, filepath.Join(base, "certs/server.key")) | ||
|
||
pflag.String(sidecarFlag, viper.GetString(sidecarFlag), "Flag to overwrite the GameServer sidecar image that is used. Can also use SIDECAR env variable") | ||
pflag.Bool(pullSidecarFlag, viper.GetBool(pullSidecarFlag), "For development purposes, set the sidecar image to have a ImagePullPolicy of Always. Can also use ALWAYS_PULL_SIDECAR env variable") | ||
pflag.Int32(minPortFlag, 0, "Required. The minimum port that that a GameServer can be allocated to. Can also use MIN_PORT env variable.") | ||
pflag.Int32(maxPortFlag, 0, "Required. The maximum port that that a GameServer can be allocated to. Can also use MAX_PORT env variable") | ||
pflag.String(keyFileFlag, viper.GetString(keyFileFlag), "Optional. Path to the key file") | ||
pflag.String(certFileFlag, viper.GetString(certFileFlag), "Optional. Path to the crt file") | ||
pflag.Parse() | ||
|
||
viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) | ||
runtime.Must(viper.BindEnv(sidecarFlag)) | ||
runtime.Must(viper.BindEnv(pullSidecarFlag)) | ||
runtime.Must(viper.BindEnv(minPortFlag)) | ||
runtime.Must(viper.BindEnv(maxPortFlag)) | ||
runtime.Must(viper.BindEnv(keyFileFlag)) | ||
runtime.Must(viper.BindEnv(certFileFlag)) | ||
runtime.Must(viper.BindPFlags(pflag.CommandLine)) | ||
|
||
minPort := int32(viper.GetInt64(minPortFlag)) | ||
maxPort := int32(viper.GetInt64(maxPortFlag)) | ||
sidecarImage := viper.GetString(sidecarFlag) | ||
alwaysPullSidecar := viper.GetBool(pullSidecarFlag) | ||
keyFile := viper.GetString(keyFileFlag) | ||
certFile := viper.GetString(certFileFlag) | ||
|
||
logrus.WithField(sidecarFlag, sidecarImage). | ||
WithField("minPort", minPort). | ||
WithField("maxPort", maxPort). | ||
WithField(keyFileFlag, keyFile). | ||
WithField(certFileFlag, certFile). | ||
WithField("alwaysPullSidecarImage", alwaysPullSidecar). | ||
WithField("Version", pkg.Version).Info("starting gameServer operator...") | ||
|
||
|
@@ -102,13 +123,21 @@ func main() { | |
|
||
agonesInformerFactory := externalversions.NewSharedInformerFactory(agonesClient, 30*time.Second) | ||
kubeInformationFactory := informers.NewSharedInformerFactory(kubeClient, 30*time.Second) | ||
c := gameservers.NewController(minPort, maxPort, sidecarImage, alwaysPullSidecar, kubeClient, kubeInformationFactory, extClient, agonesClient, agonesInformerFactory) | ||
|
||
wh := webhooks.NewWebHook(certFile, keyFile) | ||
c := gameservers.NewController(wh, minPort, maxPort, sidecarImage, alwaysPullSidecar, kubeClient, kubeInformationFactory, extClient, agonesClient, agonesInformerFactory) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessarily an issue for this CL, but is there precedent in Go to support the Builder paradigm? Whenever I have to look at adding a param to this constructor it makes me nervous, and a ControllerBuilder might be a way to alleviate potential problems here. If there is support for this I will create an issue for it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could always do functional options. It's become a standard pattern. Something like:
where the last argument to the constructor is variadic. See here for more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would say for Go, functional options are the more prevalent pattern: It's an interesting question. I decided not to go that route, since default values are set in the @enocom what do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a straightforward refactor. Once we hit the 0.1 milestone, I would be interested in revisiting it, especially once I understand Agones better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it's less critical if we don't expect users to be calling this function directly, if all their config are provided as k8s type config. If it was a public API I'd definitely push for it, but with this I could see it either way. Would make it less error prone in the future though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. stupid question : Does go recommend a line return for long line of code ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting question - from https://golang.org/doc/effective_go.html
That being said, it is fairly common practice to wrap on long lines. |
||
|
||
stop := signals.NewStopChannel() | ||
|
||
kubeInformationFactory.Start(stop) | ||
agonesInformerFactory.Start(stop) | ||
|
||
go func() { | ||
if err := wh.Run(stop); err != nil { // nolint: vetshadow | ||
logrus.WithError(err).Fatal("could not run webhook server") | ||
} | ||
}() | ||
|
||
err = c.Run(2, stop) | ||
if err != nil { | ||
logrus.WithError(err).Fatal("Could not run gameserver controller") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small thing, but probably worth fixing. Git wants newlines on all files (see here), but not all editors insert them by default. Do we want to include an
.editorconfig
file in this project? See here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that in
Gopkg.toml
as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to do this as a separate PR? Sounds like you have much more experience with this than I do.
But this sounds like a great idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sure. I'll pick it up. 👍