From 6c146e5199b94fd5dcb3e119310e1552952d86f6 Mon Sep 17 00:00:00 2001 From: Easton Crupper <65553218+ecrupper@users.noreply.github.com> Date: Tue, 4 Apr 2023 14:13:55 -0600 Subject: [PATCH] enhance(auth): implement registration flow (#452) * initial work * more work * backwards compatibility with comments * adjust middleware to only use validate-token for server tokens * more work * rename auth token channel to RegisterToken * rename token channel and update api comments * updating some comments and not loggin token * fix docker compose * fix local replace * token expiration func has two return vals * docker compose no register * name register token middleware file correctly * update swagger for register --- api/register.go | 70 +++++++++++++++++++++++ cmd/vela-worker/flags.go | 1 + cmd/vela-worker/operate.go | 72 ++++++++++++++++++------ cmd/vela-worker/register.go | 20 +++---- cmd/vela-worker/run.go | 7 +++ cmd/vela-worker/server.go | 1 + cmd/vela-worker/validate.go | 5 -- cmd/vela-worker/worker.go | 12 ++-- docker-compose.yml | 10 +++- router/middleware/register_token.go | 18 ++++++ router/middleware/register_token_test.go | 48 ++++++++++++++++ router/middleware/server_test.go | 46 +++++++++++++++ router/router.go | 3 + router/router_test.go | 6 ++ 14 files changed, 280 insertions(+), 39 deletions(-) create mode 100644 api/register.go create mode 100644 router/middleware/register_token.go create mode 100644 router/middleware/register_token_test.go create mode 100644 router/middleware/server_test.go diff --git a/api/register.go b/api/register.go new file mode 100644 index 00000000..33829dff --- /dev/null +++ b/api/register.go @@ -0,0 +1,70 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package api + +import ( + "net/http" + + "github.com/gin-gonic/gin" + "github.com/go-vela/worker/router/middleware/token" +) + +// swagger:operation POST /register system Register +// +// Fill registration token channel in worker to continue operation +// +// --- +// produces: +// - application/json +// parameters: +// security: +// - ApiKeyAuth: [] +// responses: +// '200': +// description: Successfully passed token to worker +// schema: +// type: string +// '500': +// description: Unable to pass token to worker +// schema: +// "$ref": "#/definitions/Error" + +// Register will pass the token given in the request header to the register token +// channel of the worker. This will unblock operation if the worker has not been +// registered and the provided registration token is valid. +func Register(c *gin.Context) { + // extract the register token channel that was packed into gin context + v, ok := c.Get("register-token") + if !ok { + c.JSON(http.StatusInternalServerError, "no register token channel in the context") + return + } + + // make sure we configured the channel properly + rChan, ok := v.(chan string) + if !ok { + c.JSON(http.StatusInternalServerError, "register token channel in the context is the wrong type") + return + } + + // if token is present in the channel, deny registration + // this will likely never happen as the channel is offloaded immediately + if len(rChan) > 0 { + c.JSON(http.StatusOK, "worker already registered") + return + } + + // retrieve auth token from header + token, err := token.Retrieve(c.Request) + if err != nil { + c.JSON(http.StatusInternalServerError, err) + return + } + + // write registration token to auth token channel + rChan <- token + + c.JSON(http.StatusOK, "successfully passed token to worker") +} diff --git a/cmd/vela-worker/flags.go b/cmd/vela-worker/flags.go index bce7284c..23d7b025 100644 --- a/cmd/vela-worker/flags.go +++ b/cmd/vela-worker/flags.go @@ -74,6 +74,7 @@ func flags() []cli.Flag { EnvVars: []string{"WORKER_SERVER_SECRET", "VELA_SERVER_SECRET", "SERVER_SECRET"}, Name: "server.secret", Usage: "secret used for server <-> worker communication", + Value: "", }, &cli.StringFlag{ EnvVars: []string{"WORKER_SERVER_CERT", "VELA_SERVER_CERT", "SERVER_CERT"}, diff --git a/cmd/vela-worker/operate.go b/cmd/vela-worker/operate.go index bbbe2e7a..8a8909a5 100644 --- a/cmd/vela-worker/operate.go +++ b/cmd/vela-worker/operate.go @@ -22,12 +22,6 @@ import ( func (w *Worker) operate(ctx context.Context) error { var err error - // setup the vela client with the server - w.VelaClient, err = setupClient(w.Config.Server, w.Config.Server.Secret) - if err != nil { - return err - } - // create the errgroup for managing operator subprocesses // // https://pkg.go.dev/golang.org/x/sync/errgroup?tab=doc#Group @@ -40,9 +34,17 @@ func (w *Worker) operate(ctx context.Context) error { registryWorker.SetAddress(w.Config.API.Address.String()) registryWorker.SetRoutes(w.Config.Queue.Routes) registryWorker.SetActive(true) - registryWorker.SetLastCheckedIn(time.Now().UTC().Unix()) registryWorker.SetBuildLimit(int64(w.Config.Build.Limit)) + // pull registration token from configuration if provided; wait if not + token := <-w.RegisterToken + + // setup the vela client with the token + w.VelaClient, err = setupClient(w.Config.Server, token) + if err != nil { + return err + } + // spawn goroutine for phoning home executors.Go(func() error { for { @@ -51,19 +53,51 @@ func (w *Worker) operate(ctx context.Context) error { logrus.Info("Completed looping on worker registration") return nil default: - // set checking time to now and call the server - registryWorker.SetLastCheckedIn(time.Now().UTC().Unix()) + // check in attempt loop + for { + // register or update the worker + //nolint:contextcheck // ignore passing context + w.CheckedIn, token, err = w.checkIn(registryWorker) + // check in failed + if err != nil { + // check if token is expired + expired, err := w.VelaClient.Authentication.IsTokenAuthExpired() + if err != nil { + logrus.Error("unable to check token expiration") + return err + } + + // token has expired + if expired && len(w.Config.Server.Secret) == 0 { + // wait on new registration token, return to check in attempt + token = <-w.RegisterToken + + // setup the vela client with the token + w.VelaClient, err = setupClient(w.Config.Server, token) + if err != nil { + return err + } + + continue + } + + // check in failed, token is still valid, retry + logrus.Errorf("unable to check-in worker %s on the server: %v", registryWorker.GetHostname(), err) + logrus.Info("retrying...") + + time.Sleep(5 * time.Second) + + continue + } - // register or update the worker - //nolint:contextcheck // ignore passing context - err = w.checkIn(registryWorker) - if err != nil { - logrus.Error(err) + // successful check in breaks the loop + break } - // if unable to update the worker, log the error but allow the worker to continue running + // setup the vela client with the token + w.VelaClient, err = setupClient(w.Config.Server, token) if err != nil { - logrus.Errorf("unable to update worker %s on the server: %v", registryWorker.GetHostname(), err) + return err } // sleep for the configured time @@ -99,6 +133,12 @@ func (w *Worker) operate(ctx context.Context) error { executors.Go(func() error { // create an infinite loop to poll for builds for { + // do not pull from queue unless worker is checked in with server + if !w.CheckedIn { + time.Sleep(5 * time.Second) + logrus.Info("worker not checked in, skipping queue read") + continue + } select { case <-gctx.Done(): logrus.WithFields(logrus.Fields{ diff --git a/cmd/vela-worker/register.go b/cmd/vela-worker/register.go index 5de52721..e922c745 100644 --- a/cmd/vela-worker/register.go +++ b/cmd/vela-worker/register.go @@ -13,7 +13,7 @@ import ( ) // checkIn is a helper function to phone home to the server. -func (w *Worker) checkIn(config *library.Worker) error { +func (w *Worker) checkIn(config *library.Worker) (bool, string, error) { // check to see if the worker already exists in the database logrus.Infof("retrieving worker %s from the server", config.GetHostname()) @@ -21,37 +21,37 @@ func (w *Worker) checkIn(config *library.Worker) error { if err != nil { respErr := fmt.Errorf("unable to retrieve worker %s from the server: %w", config.GetHostname(), err) if resp == nil { - return respErr + return false, "", respErr } // if we receive a 404 the worker needs to be registered if resp.StatusCode == http.StatusNotFound { return w.register(config) } - return respErr + return false, "", respErr } // if we were able to GET the worker, update it logrus.Infof("checking worker %s into the server", config.GetHostname()) - _, _, err = w.VelaClient.Worker.Update(config.GetHostname(), config) + tkn, _, err := w.VelaClient.Worker.RefreshAuth(config.GetHostname()) if err != nil { - return fmt.Errorf("unable to update worker %s on the server: %w", config.GetHostname(), err) + return false, "", fmt.Errorf("unable to refresh auth for worker %s on the server: %w", config.GetHostname(), err) } - return nil + return true, tkn.GetToken(), nil } // register is a helper function to register the worker with the server. -func (w *Worker) register(config *library.Worker) error { +func (w *Worker) register(config *library.Worker) (bool, string, error) { logrus.Infof("worker %s not found, registering it with the server", config.GetHostname()) - _, _, err := w.VelaClient.Worker.Add(config) + tkn, _, err := w.VelaClient.Worker.Add(config) if err != nil { // log the error instead of returning so the operation doesn't block worker deployment - return fmt.Errorf("unable to register worker %s with the server: %w", config.GetHostname(), err) + return false, "", fmt.Errorf("unable to register worker %s with the server: %w", config.GetHostname(), err) } // successfully added the worker so return nil - return nil + return true, tkn.GetToken(), nil } diff --git a/cmd/vela-worker/run.go b/cmd/vela-worker/run.go index 222aed07..10cf5c02 100644 --- a/cmd/vela-worker/run.go +++ b/cmd/vela-worker/run.go @@ -135,6 +135,8 @@ func run(c *cli.Context) error { TLSMinVersion: c.String("server.tls-min-version"), }, Executors: make(map[int]executor.Engine), + + RegisterToken: make(chan string, 1), } // set the worker address if no flag was provided @@ -142,6 +144,11 @@ func run(c *cli.Context) error { w.Config.API.Address, _ = url.Parse(fmt.Sprintf("http://%s", hostname)) } + // if server secret is provided, use as register token on start up + if len(c.String("server.secret")) > 0 { + w.RegisterToken <- c.String("server.secret") + } + // validate the worker err = w.Validate() if err != nil { diff --git a/cmd/vela-worker/server.go b/cmd/vela-worker/server.go index d9810659..7961fe73 100644 --- a/cmd/vela-worker/server.go +++ b/cmd/vela-worker/server.go @@ -33,6 +33,7 @@ func (w *Worker) server() (http.Handler, *tls.Config) { middleware.ServerAddress(w.Config.Server.Address), middleware.Executors(w.Executors), middleware.Logger(logrus.StandardLogger(), time.RFC3339, true), + middleware.RegisterToken(w.RegisterToken), ) // log a message indicating the start of serving traffic diff --git a/cmd/vela-worker/validate.go b/cmd/vela-worker/validate.go index 84db82b1..82d1b79a 100644 --- a/cmd/vela-worker/validate.go +++ b/cmd/vela-worker/validate.go @@ -52,11 +52,6 @@ func (w *Worker) Validate() error { return fmt.Errorf("no worker server address provided") } - // verify a server secret was provided - if len(w.Config.Server.Secret) == 0 { - return fmt.Errorf("no worker server secret provided") - } - // verify an executor driver was provided if len(w.Config.Executor.Driver) == 0 { return fmt.Errorf("no worker executor driver provided") diff --git a/cmd/vela-worker/worker.go b/cmd/vela-worker/worker.go index 1d21e75c..895fdd19 100644 --- a/cmd/vela-worker/worker.go +++ b/cmd/vela-worker/worker.go @@ -62,10 +62,12 @@ type ( // Worker represents all configuration and // system processes for the worker. Worker struct { - Config *Config - Executors map[int]executor.Engine - Queue queue.Service - Runtime runtime.Engine - VelaClient *vela.Client + Config *Config + Executors map[int]executor.Engine + Queue queue.Service + Runtime runtime.Engine + VelaClient *vela.Client + RegisterToken chan string + CheckedIn bool } ) diff --git a/docker-compose.yml b/docker-compose.yml index d0758630..8fde3ef5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -30,9 +30,10 @@ services: VELA_RUNTIME_PRIVILEGED_IMAGES: 'target/vela-docker' VELA_EXECUTOR_ENFORCE_TRUSTED_REPOS: 'true' VELA_SERVER_ADDR: 'http://server:8080' + # comment the line below to use registration flow VELA_SERVER_SECRET: 'zB7mrKDTZqNeNTD8z47yG4DHywspAh' WORKER_ADDR: 'http://worker:8080' - WORKER_CHECK_IN: 5m + WORKER_CHECK_IN: 2m restart: always ports: - "8081:8080" @@ -68,9 +69,12 @@ services: VELA_ADDR: 'http://localhost:8080' VELA_WEBUI_ADDR: 'http://localhost:8888' VELA_LOG_LEVEL: trace + # comment the line below to use registration flow VELA_SECRET: 'zB7mrKDTZqNeNTD8z47yG4DHywspAh' - VELA_REFRESH_TOKEN_DURATION: 90m - VELA_ACCESS_TOKEN_DURATION: 60m + VELA_SERVER_PRIVATE_KEY: 'F534FF2A080E45F38E05DC70752E6787' + VELA_USER_REFRESH_TOKEN_DURATION: 90m + VELA_USER_ACCESS_TOKEN_DURATION: 60m + VELA_WORKER_AUTH_TOKEN_DURATION: 3m VELA_DISABLE_WEBHOOK_VALIDATION: 'true' VELA_ENABLE_SECURE_COOKIE: 'false' VELA_REPO_ALLOWLIST: '*' diff --git a/router/middleware/register_token.go b/router/middleware/register_token.go new file mode 100644 index 00000000..177fd3f2 --- /dev/null +++ b/router/middleware/register_token.go @@ -0,0 +1,18 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package middleware + +import ( + "github.com/gin-gonic/gin" +) + +// RegisterToken is a middleware function that attaches the +// auth-token channel to the context of every http.Request. +func RegisterToken(r chan string) gin.HandlerFunc { + return func(c *gin.Context) { + c.Set("register-token", r) + c.Next() + } +} diff --git a/router/middleware/register_token_test.go b/router/middleware/register_token_test.go new file mode 100644 index 00000000..afe8241d --- /dev/null +++ b/router/middleware/register_token_test.go @@ -0,0 +1,48 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package middleware + +import ( + "net/http" + "net/http/httptest" + "reflect" + "testing" + + "github.com/gin-gonic/gin" +) + +func TestMiddleware_RegisterToken(t *testing.T) { + // setup types + want := make(chan string, 1) + got := make(chan string, 1) + + want <- "foo" + + // setup context + gin.SetMode(gin.TestMode) + + resp := httptest.NewRecorder() + context, engine := gin.CreateTestContext(resp) + context.Request, _ = http.NewRequest(http.MethodGet, "/health", nil) + + // setup mock server + engine.Use(RegisterToken(want)) + engine.GET("/health", func(c *gin.Context) { + got = c.Value("register-token").(chan string) + + c.Status(http.StatusOK) + }) + + // run test + engine.ServeHTTP(context.Writer, context.Request) + + if resp.Code != http.StatusOK { + t.Errorf("RegisterToken returned %v, want %v", resp.Code, http.StatusOK) + } + + if !reflect.DeepEqual(got, want) { + t.Errorf("RegisterToken is %v, want foo", got) + } +} diff --git a/router/middleware/server_test.go b/router/middleware/server_test.go new file mode 100644 index 00000000..a1221287 --- /dev/null +++ b/router/middleware/server_test.go @@ -0,0 +1,46 @@ +// Copyright (c) 2023 Target Brands, Inc. All rights reserved. +// +// Use of this source code is governed by the LICENSE file in this repository. + +package middleware + +import ( + "net/http" + "net/http/httptest" + "reflect" + "testing" + + "github.com/gin-gonic/gin" +) + +func TestMiddleware_ServerAddress(t *testing.T) { + // setup types + got := "" + want := "foobar" + + // setup context + gin.SetMode(gin.TestMode) + + resp := httptest.NewRecorder() + context, engine := gin.CreateTestContext(resp) + context.Request, _ = http.NewRequest(http.MethodGet, "/health", nil) + + // setup mock server + engine.Use(ServerAddress(want)) + engine.GET("/health", func(c *gin.Context) { + got = c.Value("server-address").(string) + + c.Status(http.StatusOK) + }) + + // run test + engine.ServeHTTP(context.Writer, context.Request) + + if resp.Code != http.StatusOK { + t.Errorf("ServerAddress returned %v, want %v", resp.Code, http.StatusOK) + } + + if !reflect.DeepEqual(got, want) { + t.Errorf("ServerAddress is %v, want %v", got, want) + } +} diff --git a/router/router.go b/router/router.go index 14b3955c..7de750f4 100644 --- a/router/router.go +++ b/router/router.go @@ -107,5 +107,8 @@ func Load(options ...gin.HandlerFunc) *gin.Engine { ExecutorHandlers(baseAPI) } + // endpoint for passing a new registration token to the deadloop running operate.go + r.POST("/register", api.Register) + return r } diff --git a/router/router_test.go b/router/router_test.go index 549369a9..128d91fd 100644 --- a/router/router_test.go +++ b/router/router_test.go @@ -41,6 +41,12 @@ func TestRouter_Load(t *testing.T) { Handler: "github.com/go-vela/worker/api.Shutdown", HandlerFunc: api.Shutdown, }, + { + Method: "POST", + Path: "/api/v1/register", + Handler: "github.com/go-vela/worker/api.Register", + HandlerFunc: api.Register, + }, { Method: "GET", Path: "/api/v1/executors",