Skip to content

Commit

Permalink
Added a new 'disableTLS' flag and changed 'disableMTLS' to only disab… (
Browse files Browse the repository at this point in the history 
#1777)

* Added a new 'disableTLS' flag and changed 'disableMTLS' to only disable client auth.

* Conditionally disabled cert dir watching/reading on disableMTLS.

* Added some keepalive parameters for health checks.

* Changed logic to only have mTLS in the case of TLS.

* Added a comment for why the keepalive options are necessary.

Co-authored-by: Nikhil Athreya <nathreya@google.com>
  • Loading branch information
@devloop0 @nathreya-google
devloop0 and nathreya-google authored Sep 2, 2020
1 parent 70d3e26 commit a1ce52c
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 41 deletions.
118 changes: 77 additions & 41 deletions cmd/allocator/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/keepalive"
"google.golang.org/grpc/status"
"gopkg.in/fsnotify.v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -88,24 +89,14 @@ func main() {
return err
})

h := newServiceHandler(kubeClient, agonesClient, health, conf.MTLSDisabled)
h := newServiceHandler(kubeClient, agonesClient, health, conf.MTLSDisabled, conf.TLSDisabled)

listener, err := net.Listen("tcp", fmt.Sprintf(":%s", sslPort))
if err != nil {
logger.WithError(err).Fatalf("failed to listen on TCP port %s", sslPort)
}

if !h.mTLSDisabled {
// creates a new file watcher for client certificate folder
watcher, err := fsnotify.NewWatcher()
if err != nil {
logger.WithError(err).Fatal("could not create watcher for client certs")
}
defer watcher.Close() // nolint: errcheck
if err := watcher.Add(certDir); err != nil {
logger.WithError(err).Fatalf("cannot watch folder %s for secret changes", certDir)
}

if !h.tlsDisabled {
watcherTLS, err := fsnotify.NewWatcher()
if err != nil {
logger.WithError(err).Fatal("could not create watcher for tls certs")
Expand All @@ -130,23 +121,47 @@ func main() {
h.tlsMutex.Unlock()
}
logger.Infof("Tls directory change event %v", event)
case event := <-watcher.Events:
h.certMutex.Lock()
caCertPool, err := getCACertPool(certDir)
if err != nil {
logger.WithError(err).Error("could not load CA certs; keeping old ones")
} else {
h.caCertPool = caCertPool
}
logger.Infof("Certificate directory change event %v", event)
h.certMutex.Unlock()

// watch for errors
case err := <-watcher.Errors:
logger.WithError(err).Error("error watching for certificate directory")
// watch for errors
case err := <-watcherTLS.Errors:
logger.WithError(err).Error("error watching for TLS directory")
}
}
}()

if !h.mTLSDisabled {
// creates a new file watcher for client certificate folder
watcher, err := fsnotify.NewWatcher()
if err != nil {
logger.WithError(err).Fatal("could not create watcher for client certs")
}
defer watcher.Close() // nolint: errcheck
if err := watcher.Add(certDir); err != nil {
logger.WithError(err).Fatalf("cannot watch folder %s for secret changes", certDir)
}

go func() {
for {
select {
// watch for events
case event := <-watcher.Events:
h.certMutex.Lock()
caCertPool, err := getCACertPool(certDir)
if err != nil {
logger.WithError(err).Error("could not load CA certs; keeping old ones")
} else {
h.caCertPool = caCertPool
}
logger.Infof("Certificate directory change event %v", event)
h.certMutex.Unlock()

// watch for errors
case err := <-watcher.Errors:
logger.WithError(err).Error("error watching for certificate directory")
}
}
}()
}
}

opts := h.getServerOptions()
Expand All @@ -167,7 +182,7 @@ func main() {
logger.WithError(err).Fatal("allocation service crashed")
}

func newServiceHandler(kubeClient kubernetes.Interface, agonesClient versioned.Interface, health healthcheck.Handler, mTLSDisabled bool) *serviceHandler {
func newServiceHandler(kubeClient kubernetes.Interface, agonesClient versioned.Interface, health healthcheck.Handler, mTLSDisabled bool, tlsDisabled bool) *serviceHandler {
defaultResync := 30 * time.Second
agonesInformerFactory := externalversions.NewSharedInformerFactory(agonesClient, defaultResync)
kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, defaultResync)
Expand All @@ -185,6 +200,7 @@ func newServiceHandler(kubeClient kubernetes.Interface, agonesClient versioned.I
return allocator.Allocate(gsa, stop)
},
mTLSDisabled: mTLSDisabled,
tlsDisabled: tlsDisabled,
}

kubeInformerFactory.Start(stop)
Expand All @@ -193,22 +209,24 @@ func newServiceHandler(kubeClient kubernetes.Interface, agonesClient versioned.I
logger.WithError(err).Fatal("starting allocator failed.")
}

if !h.mTLSDisabled {
caCertPool, err := getCACertPool(certDir)
if err != nil {
logger.WithError(err).Fatal("could not load CA certs.")
}
h.certMutex.Lock()
h.caCertPool = caCertPool
h.certMutex.Unlock()

if !h.tlsDisabled {
tlsCert, err := readTLSCert()
if err != nil {
logger.WithError(err).Fatal("could not load TLS certs.")
}
h.tlsMutex.Lock()
h.tlsCert = tlsCert
h.tlsMutex.Unlock()

if !h.mTLSDisabled {
caCertPool, err := getCACertPool(certDir)
if err != nil {
logger.WithError(err).Fatal("could not load CA certs.")
}
h.certMutex.Lock()
h.caCertPool = caCertPool
h.certMutex.Unlock()
}
}

return &h
Expand All @@ -225,17 +243,34 @@ func readTLSCert() (*tls.Certificate, error) {
// getServerOptions returns a list of GRPC server options.
// Current options are TLS certs and opencensus stats handler.
func (h *serviceHandler) getServerOptions() []grpc.ServerOption {
if h.mTLSDisabled {
if h.tlsDisabled {
return []grpc.ServerOption{grpc.StatsHandler(&ocgrpc.ServerHandler{})}
}

cfg := &tls.Config{
GetCertificate: h.getTLSCert,
ClientAuth: tls.RequireAnyClientCert,
VerifyPeerCertificate: h.verifyClientCertificate,
GetCertificate: h.getTLSCert,
}

if !h.mTLSDisabled {
cfg.ClientAuth = tls.RequireAnyClientCert
cfg.VerifyPeerCertificate = h.verifyClientCertificate
}

// Add options for creds and OpenCensus stats handler to enable stats and tracing.
// The keepalive options are useful for efficiency purposes (keeping a single connection alive
// instead of constantly recreating connections), when placing the Agones allocator behind load balancers.
return []grpc.ServerOption{
grpc.Creds(credentials.NewTLS(cfg)),
grpc.StatsHandler(&ocgrpc.ServerHandler{}),
grpc.KeepaliveEnforcementPolicy(keepalive.EnforcementPolicy{
MinTime: 1 * time.Minute,
PermitWithoutStream: true,
}),
grpc.KeepaliveParams(keepalive.ServerParameters{
MaxConnectionIdle: 5 * time.Minute,
Timeout: 10 * time.Minute,
}),
}
// Add options for creds and OpenCensus stats handler to enable stats and tracing.
return []grpc.ServerOption{grpc.Creds(credentials.NewTLS(cfg)), grpc.StatsHandler(&ocgrpc.ServerHandler{})}
}

func (h *serviceHandler) getTLSCert(ch *tls.ClientHelloInfo) (*tls.Certificate, error) {
Expand Down Expand Up @@ -333,6 +368,7 @@ type serviceHandler struct {
tlsCert *tls.Certificate

mTLSDisabled bool
tlsDisabled bool
}

// Allocate implements the Allocate gRPC method definition
Expand Down
6 changes: 6 additions & 0 deletions cmd/allocator/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@ const (
projectIDFlag = "gcp-project-id"
stackdriverLabels = "stackdriver-labels"
mTLSDisabledFlag = "disable-mtls"
tlsDisabledFlag = "disable-tls"
)

func init() {
registerMetricViews()
}

type config struct {
TLSDisabled bool
MTLSDisabled bool
PrometheusMetrics bool
Stackdriver bool
Expand All @@ -54,12 +56,14 @@ func parseEnvFlags() config {
viper.SetDefault(projectIDFlag, "")
viper.SetDefault(stackdriverLabels, "")
viper.SetDefault(mTLSDisabledFlag, false)
viper.SetDefault(tlsDisabledFlag, false)

pflag.Bool(enablePrometheusMetricsFlag, viper.GetBool(enablePrometheusMetricsFlag), "Flag to activate metrics of Agones. Can also use PROMETHEUS_EXPORTER env variable.")
pflag.Bool(enableStackdriverMetricsFlag, viper.GetBool(enableStackdriverMetricsFlag), "Flag to activate stackdriver monitoring metrics for Agones. Can also use STACKDRIVER_EXPORTER env variable.")
pflag.String(projectIDFlag, viper.GetString(projectIDFlag), "GCP ProjectID used for Stackdriver, if not specified ProjectID from Application Default Credentials would be used. Can also use GCP_PROJECT_ID env variable.")
pflag.String(stackdriverLabels, viper.GetString(stackdriverLabels), "A set of default labels to add to all stackdriver metrics generated. By default metadata are automatically added using Kubernetes API and GCP metadata enpoint.")
pflag.Bool(mTLSDisabledFlag, viper.GetBool(mTLSDisabledFlag), "Flag to enable/disable mTLS in the allocator.")
pflag.Bool(tlsDisabledFlag, viper.GetBool(tlsDisabledFlag), "Flag to enable/disable TLS in the allocator.")
runtime.FeaturesBindFlags()
pflag.Parse()

Expand All @@ -69,6 +73,7 @@ func parseEnvFlags() config {
runtime.Must(viper.BindEnv(projectIDFlag))
runtime.Must(viper.BindEnv(stackdriverLabels))
runtime.Must(viper.BindEnv(mTLSDisabledFlag))
runtime.Must(viper.BindEnv(tlsDisabledFlag))
runtime.Must(viper.BindPFlags(pflag.CommandLine))
runtime.Must(runtime.FeaturesBindEnv())

Expand All @@ -80,6 +85,7 @@ func parseEnvFlags() config {
GCPProjectID: viper.GetString(projectIDFlag),
StackdriverLabels: viper.GetString(stackdriverLabels),
MTLSDisabled: viper.GetBool(mTLSDisabledFlag),
TLSDisabled: viper.GetBool(tlsDisabledFlag),
}
}

Expand Down
2 changes: 2 additions & 0 deletions install/helm/agones/templates/service/allocation.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ spec:
value: {{ .Values.agones.metrics.stackdriverLabels | quote }}
- name: DISABLE_MTLS
value: {{ .Values.agones.allocator.disableMTLS | quote }}
- name: DISABLE_TLS
value: {{ .Values.agones.allocator.disableTLS | quote }}
- name: POD_NAME
valueFrom:
fieldRef:
Expand Down
1 change: 1 addition & 0 deletions install/helm/agones/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ agones:
generateTLS: true
generateClientTLS: true
disableMTLS: false
disableTLS: false
image:
registry: gcr.io/agones-images
tag: 1.9.0-dev
Expand Down
2 changes: 2 additions & 0 deletions install/yaml/install.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,8 @@ spec:
value: ""
- name: DISABLE_MTLS
value: "false"
- name: DISABLE_TLS
value: "false"
- name: POD_NAME
valueFrom:
fieldRef:
Expand Down

0 comments on commit a1ce52c

Please sign in to comment.