14807: Pass token to service handlers explicitly, not via context.
authorTom Clegg <tclegg@veritasgenetics.com>
Fri, 22 Mar 2019 18:44:03 +0000 (14:44 -0400)
committerTom Clegg <tclegg@veritasgenetics.com>
Fri, 22 Mar 2019 18:44:03 +0000 (14:44 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

lib/controller/cmd.go
lib/controller/handler_test.go
lib/controller/proxy.go
lib/dispatchcloud/cmd.go
lib/service/cmd.go
lib/service/token.go [deleted file]

index c1d4657ba47b7801b63bad0222e4a2df71f7881d..f0268091bedb58f412d4e93ba675481d99f5e3ef 100644 (file)
@@ -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}
 }
index dfe60d90a5f3119909658149b1017f3b782515f3..96110ea85859b05b362f849475a9d77c91919752 100644 (file)
@@ -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) {
index c01c152352e6b8f101179bf38add3b0574a00c5d..c0b94c2b5f76d604e738c2d9bc43d3a01f8bf5dc 100644 (file)
@@ -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
index 82205c74262458d39ac4c25c9a6327815a34c332..f8143ac8ce5f50a06a4bdd39ed363bc6699c5563 100644 (file)
@@ -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
index e56f52eec5d2460dfb9de761ec65d0ef09b63bd2..e23791b30a214d659390b3fe987ca247dde569e4 100644 (file)
@@ -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 (file)
index 5070ae5..0000000
+++ /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)
-}