From e4b51619430c53f6da603dc8f9b5ece1cb33449f Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 22 Mar 2019 14:44:03 -0400 Subject: [PATCH] 14807: Pass token to service handlers explicitly, not via context. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/controller/cmd.go | 2 +- lib/controller/handler_test.go | 2 +- lib/controller/proxy.go | 1 + lib/dispatchcloud/cmd.go | 4 ++-- lib/service/cmd.go | 24 ++++++++++++------------ lib/service/token.go | 30 ------------------------------ 6 files changed, 17 insertions(+), 46 deletions(-) delete mode 100644 lib/service/token.go diff --git a/lib/controller/cmd.go b/lib/controller/cmd.go index c1d4657ba4..f0268091be 100644 --- a/lib/controller/cmd.go +++ b/lib/controller/cmd.go @@ -14,6 +14,6 @@ import ( var Command cmd.Handler = service.Command(arvados.ServiceNameController, newHandler) -func newHandler(_ context.Context, cluster *arvados.Cluster, np *arvados.NodeProfile) service.Handler { +func newHandler(_ context.Context, cluster *arvados.Cluster, np *arvados.NodeProfile, _ string) service.Handler { return &Handler{Cluster: cluster, NodeProfile: np} } diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go index dfe60d90a5..96110ea858 100644 --- a/lib/controller/handler_test.go +++ b/lib/controller/handler_test.go @@ -50,7 +50,7 @@ func (s *HandlerSuite) SetUpTest(c *check.C) { }, } node := s.cluster.NodeProfiles["*"] - s.handler = newHandler(s.ctx, s.cluster, &node) + s.handler = newHandler(s.ctx, s.cluster, &node, "") } func (s *HandlerSuite) TearDownTest(c *check.C) { diff --git a/lib/controller/proxy.go b/lib/controller/proxy.go index c01c152352..c0b94c2b5f 100644 --- a/lib/controller/proxy.go +++ b/lib/controller/proxy.go @@ -32,6 +32,7 @@ var dropHeaders = map[string]bool{ "Keep-Alive": true, "Proxy-Authenticate": true, "Proxy-Authorization": true, + // this line makes gofmt 1.10 and 1.11 agree "TE": true, "Trailer": true, "Transfer-Encoding": true, // *-Encoding headers interfer with Go's automatic compression/decompression diff --git a/lib/dispatchcloud/cmd.go b/lib/dispatchcloud/cmd.go index 82205c7426..f8143ac8ce 100644 --- a/lib/dispatchcloud/cmd.go +++ b/lib/dispatchcloud/cmd.go @@ -14,11 +14,11 @@ import ( var Command cmd.Handler = service.Command(arvados.ServiceNameDispatchCloud, newHandler) -func newHandler(ctx context.Context, cluster *arvados.Cluster, _ *arvados.NodeProfile) service.Handler { +func newHandler(ctx context.Context, cluster *arvados.Cluster, _ *arvados.NodeProfile, token string) service.Handler { d := &dispatcher{ Cluster: cluster, Context: ctx, - AuthToken: service.Token(ctx), + AuthToken: token, } go d.Start() return d diff --git a/lib/service/cmd.go b/lib/service/cmd.go index e56f52eec5..e23791b30a 100644 --- a/lib/service/cmd.go +++ b/lib/service/cmd.go @@ -26,7 +26,7 @@ type Handler interface { CheckHealth() error } -type NewHandlerFunc func(context.Context, *arvados.Cluster, *arvados.NodeProfile) Handler +type NewHandlerFunc func(_ context.Context, _ *arvados.Cluster, _ *arvados.NodeProfile, token string) Handler type command struct { newHandler NewHandlerFunc @@ -79,16 +79,6 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout }) ctx := ctxlog.Context(context.Background(), log) - // Currently all components use SystemRootToken if configured, - // otherwise ARVADOS_API_TOKEN. In future, per-process tokens - // will be generated/obtained here. - token := cluster.SystemRootToken - if token == "" { - log.Warn("SystemRootToken missing from cluster config, falling back to ARVADOS_API_TOKEN environment variable") - token = os.Getenv("ARVADOS_API_TOKEN") - } - ctx = tokenContext(ctx, token) - profileName := *nodeProfile if profileName == "" { profileName = os.Getenv("ARVADOS_NODE_PROFILE") @@ -102,7 +92,17 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout err = fmt.Errorf("configuration does not enable the %s service on this host", c.svcName) return 1 } - handler := c.newHandler(ctx, cluster, profile) + + // Currently all components use SystemRootToken if configured, + // otherwise ARVADOS_API_TOKEN. In future, per-process tokens + // will be generated/obtained here. + token := cluster.SystemRootToken + if token == "" { + log.Warn("SystemRootToken missing from cluster config, falling back to ARVADOS_API_TOKEN environment variable") + token = os.Getenv("ARVADOS_API_TOKEN") + } + + handler := c.newHandler(ctx, cluster, profile, token) if err = handler.CheckHealth(); err != nil { return 1 } diff --git a/lib/service/token.go b/lib/service/token.go deleted file mode 100644 index 5070ae564e..0000000000 --- a/lib/service/token.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (C) The Arvados Authors. All rights reserved. -// -// SPDX-License-Identifier: Apache-2.0 - -package service - -import ( - "context" -) - -type contextKey string - -var contextKeyServiceToken contextKey = "serviceToken" - -// Token returns the privileged system token suitable for the given -// service context. -// -// It only works on contexts that were generated by Command() and -// passed to a Handler. For other contexts it returns the empty -// string. -func Token(ctx context.Context) string { - t, _ := ctx.Value(contextKeyServiceToken).(string) - return t -} - -// tokenContext returns a child context with the given token attached -// so it can be retrieved by Token(). -func tokenContext(ctx context.Context, t string) context.Context { - return context.WithValue(ctx, contextKeyServiceToken, t) -} -- 2.39.5