diff --git a/internal/cmd/commands/dev/dev.go b/internal/cmd/commands/dev/dev.go index 8081efa49..fe8b9b8bc 100644 --- a/internal/cmd/commands/dev/dev.go +++ b/internal/cmd/commands/dev/dev.go @@ -587,6 +587,8 @@ func (c *Command) Run(args []string) int { c.UI.Error("Worker Auth KMS not found after parsing KMS blocks") return base.CommandUserError } + c.Config.Worker.Name = "" + c.Config.Worker.Description = "" c.InfoKeys = append(c.InfoKeys, "[Worker-Auth] AEAD Key Bytes") c.Info["[Worker-Auth] AEAD Key Bytes"] = c.Config.DevWorkerAuthKey } diff --git a/internal/cmd/commands/server/server.go b/internal/cmd/commands/server/server.go index 9a7a07897..cf819ae9a 100644 --- a/internal/cmd/commands/server/server.go +++ b/internal/cmd/commands/server/server.go @@ -197,6 +197,10 @@ func (c *Command) Run(args []string) int { c.UI.Error("No worker auth KMS specified and no worker auth storage path specified.") return base.CommandUserError } + if c.Config.Worker.Name != "" || c.Config.Worker.Description != "" { + c.UI.Error("Worker config cannot contain name or description when using PKI-based worker authentication; it must be set via the API.") + return base.CommandUserError + } default: if c.Config.Worker.Name == "" { c.UI.Error("Worker is using KMS auth but has no name set. It must be the unique name of this instance.") diff --git a/internal/daemon/cluster/handlers/worker_service.go b/internal/daemon/cluster/handlers/worker_service.go index 3b2810754..581182341 100644 --- a/internal/daemon/cluster/handlers/worker_service.go +++ b/internal/daemon/cluster/handlers/worker_service.go @@ -92,6 +92,7 @@ func (ws *workerServiceServer) Status(ctx context.Context, req *pbs.StatusReques wConf := servers.NewWorker(scope.Global.String(), servers.WithName(wStat.GetName()), + servers.WithDescription(wStat.GetDescription()), servers.WithAddress(wStat.GetAddress()), servers.WithWorkerTags(workerTags...)) opts := []servers.Option{servers.WithUpdateTags(req.GetUpdateTags())} diff --git a/internal/daemon/worker/status.go b/internal/daemon/worker/status.go index aaeffe710..9d312a6d7 100644 --- a/internal/daemon/worker/status.go +++ b/internal/daemon/worker/status.go @@ -157,10 +157,11 @@ func (w *Worker) sendWorkerStatus(cancelCtx context.Context) { result, err := client.Status(statusCtx, &pbs.StatusRequest{ Jobs: activeJobs, WorkerStatus: &servers.ServerWorkerStatus{ - Name: w.conf.RawConfig.Worker.Name, - Address: w.conf.RawConfig.Worker.PublicAddr, - Tags: tags, - KeyId: keyId, + Name: w.conf.RawConfig.Worker.Name, + Description: w.conf.RawConfig.Worker.Description, + Address: w.conf.RawConfig.Worker.PublicAddr, + Tags: tags, + KeyId: keyId, }, UpdateTags: w.updateTags.Load(), }) diff --git a/internal/proto/controller/servers/v1/servers.proto b/internal/proto/controller/servers/v1/servers.proto index 33b06d696..2b2ac63cd 100644 --- a/internal/proto/controller/servers/v1/servers.proto +++ b/internal/proto/controller/servers/v1/servers.proto @@ -18,6 +18,9 @@ message ServerWorkerStatus { // Name of the resource (optional) string name = 20; // @gotags: `class:"public"` + // Description of the resource (optional) + string description = 25; // @gotags: `class:"public"` + // Address for the worker. This must be set. string address = 30; // @gotags: `class:"public"` diff --git a/internal/servers/repository_worker.go b/internal/servers/repository_worker.go index 06e1fe1b7..e652bc317 100644 --- a/internal/servers/repository_worker.go +++ b/internal/servers/repository_worker.go @@ -270,11 +270,36 @@ func (r *Repository) UpsertWorkerStatus(ctx context.Context, worker *Worker, opt worker := worker.clone() worker.PublicId = workerId switch { + case opts.withKeyId != "": + // This case goes first in case we want to relax the restriction + // around both name and key ID being supplied to account for + // e.g. config processing bugs. In that case, a key ID being + // supplied should be a clear indicator that we are working with + // a PKI worker, and a lack of one a clear indication we are + // working with a KMS worker. + // + // Note: unlike in the below case, this purposefully leaves out + // "description" since we want description changes for PKI-based + // workers to come via API only. We can't really guard on this + // in the DB so we need to be sure to not include it here. + n, err := w.Update(ctx, worker, []string{"address"}, nil) + if err != nil { + return errors.Wrap(ctx, err, op, errors.WithMsg("unable to update status of pki worker")) + } + switch n { + case 0: + return errors.New(ctx, errors.RecordNotFound, op, fmt.Sprintf("failed to find worker with key id %q", opts.withKeyId)) + case 1: + break + default: + return errors.New(ctx, errors.MultipleRecords, op, fmt.Sprintf("multiple records found when updating worker with id %q", worker.GetPublicId())) + } + case worker.GetName() != "": worker.Type = KmsWorkerType.String() workerCreateConflict := &db.OnConflict{ Target: db.Columns{"public_id"}, - Action: append(db.SetColumns([]string{"address"}), + Action: append(db.SetColumns([]string{"description", "address"}), db.SetColumnValues(map[string]interface{}{"last_status_time": "now()"})...), } var withRowsAffected int64 @@ -289,19 +314,6 @@ func (r *Repository) UpsertWorkerStatus(ctx context.Context, worker *Worker, opt case withRowsAffected == 0: return errors.New(ctx, errors.NotUnique, op, "error updating worker") } - case opts.withKeyId != "": - n, err := w.Update(ctx, worker, []string{"address"}, nil) - if err != nil { - return errors.Wrap(ctx, err, op, errors.WithMsg("unable to update status of pki worker")) - } - switch n { - case 0: - return errors.New(ctx, errors.RecordNotFound, op, fmt.Sprintf("failed to find worker with key id %q", opts.withKeyId)) - case 1: - break - default: - return errors.New(ctx, errors.MultipleRecords, op, fmt.Sprintf("multiple records found when updating worker with id %q", worker.GetPublicId())) - } } // If we've been told to update tags, we need to clean out old @@ -309,7 +321,9 @@ func (r *Repository) UpsertWorkerStatus(ctx context.Context, worker *Worker, opt // delete all tags for the given worker, then add the new ones // we've been sent. if opts.withUpdateTags { - setWorkerTags(ctx, w, worker.GetPublicId(), ConfigurationTagSource, worker.inputTags) + if err := setWorkerTags(ctx, w, worker.GetPublicId(), ConfigurationTagSource, worker.inputTags); err != nil { + return errors.Wrap(ctx, err, op, errors.WithMsg("error setting worker tags")) + } } wAgg := &workerAggregate{PublicId: worker.GetPublicId()} diff --git a/internal/servers/repository_worker_test.go b/internal/servers/repository_worker_test.go index 4bcfe1468..147947339 100644 --- a/internal/servers/repository_worker_test.go +++ b/internal/servers/repository_worker_test.go @@ -265,44 +265,53 @@ func TestUpsertWorkerStatus(t *testing.T) { ctx := context.Background() - t.Run("create an initial kms worker", func(t *testing.T) { + t.Run("create an initial kms worker and update status", func(t *testing.T) { wStatus1 := servers.NewWorker(scope.Global.String(), - servers.WithAddress("address"), servers.WithName("config_name1")) + servers.WithAddress("address"), servers.WithName("config_name1"), + servers.WithDescription("kms_description1")) worker, err := repo.UpsertWorkerStatus(ctx, wStatus1) require.NoError(t, err) assert.True(t, strings.HasPrefix(worker.GetPublicId(), "w_")) assert.Equal(t, wStatus1.GetAddress(), worker.GetAddress()) assert.Equal(t, "config_name1", worker.Name) + assert.Equal(t, "kms_description1", worker.Description) assert.Equal(t, worker.GetLastStatusTime().AsTime(), worker.GetUpdateTime().AsTime()) assert.Equal(t, uint32(1), worker.Version) assert.Equal(t, "address", worker.GetAddress()) - assert.Empty(t, worker.Description) // update again and see updated last status time - worker, err = repo.UpsertWorkerStatus(ctx, wStatus1) + wStatus2 := servers.NewWorker(scope.Global.String(), + servers.WithAddress("new_address"), servers.WithName("config_name1"), + servers.WithDescription("kms_description2")) + worker, err = repo.UpsertWorkerStatus(ctx, wStatus2) require.NoError(t, err) assert.Greater(t, worker.GetLastStatusTime().AsTime(), worker.GetCreateTime().AsTime()) + assert.Equal(t, "config_name1", worker.Name) + assert.Equal(t, "kms_description2", worker.Description) + assert.Equal(t, worker.GetLastStatusTime().AsTime(), worker.GetUpdateTime().AsTime()) + assert.Equal(t, uint32(2), worker.Version) + assert.Equal(t, "new_address", worker.GetAddress()) }) // Setup and use a pki worker var pkiWorkerKeyId string - pkiWorker := servers.TestPkiWorker(t, conn, wrapper, servers.WithName("pki"), servers.WithTestPkiWorkerAuthorizedKeyId(&pkiWorkerKeyId)) + pkiWorker := servers.TestPkiWorker(t, conn, wrapper, servers.WithName("pki"), servers.WithDescription("pki_description1"), servers.WithTestPkiWorkerAuthorizedKeyId(&pkiWorkerKeyId)) t.Run("update status for pki worker", func(t *testing.T) { wStatus1 := servers.NewWorker(scope.Global.String(), - servers.WithAddress("pki_address")) + servers.WithAddress("pki_address"), servers.WithDescription("pki_description2")) worker, err := repo.UpsertWorkerStatus(ctx, wStatus1, servers.WithKeyId(pkiWorkerKeyId)) require.NoError(t, err) assert.True(t, strings.HasPrefix(worker.GetPublicId(), "w_")) assert.Equal(t, wStatus1.GetAddress(), worker.GetAddress()) assert.Equal(t, "pki", worker.Name) + assert.Equal(t, "pki_description1", worker.Description) // PKI workers don't update description via status assert.Greater(t, worker.GetLastStatusTime().AsTime(), worker.GetCreateTime().AsTime()) assert.Equal(t, worker.GetLastStatusTime().AsTime(), worker.GetUpdateTime().AsTime()) assert.Equal(t, uint32(1), worker.Version) assert.Equal(t, "pki_address", worker.GetAddress()) - assert.Empty(t, worker.Description) }) failureCases := []struct { @@ -334,7 +343,7 @@ func TestUpsertWorkerStatus(t *testing.T) { }, }, { - name: "cant specifying public id", + name: "cant specify public id", repo: repo, status: func() *servers.Worker { w := servers.NewWorker(scope.Global.String(), diff --git a/internal/servers/servers.pb.go b/internal/servers/servers.pb.go index 842ba9bd4..edf1bd10d 100644 --- a/internal/servers/servers.pb.go +++ b/internal/servers/servers.pb.go @@ -86,6 +86,8 @@ type ServerWorkerStatus struct { PublicId string `protobuf:"bytes,10,opt,name=public_id,json=publicId,proto3" json:"public_id,omitempty" class:"public"` // @gotags: `class:"public"` // Name of the resource (optional) Name string `protobuf:"bytes,20,opt,name=name,proto3" json:"name,omitempty" class:"public"` // @gotags: `class:"public"` + // Description of the resource (optional) + Description string `protobuf:"bytes,25,opt,name=description,proto3" json:"description,omitempty" class:"public"` // @gotags: `class:"public"` // Address for the worker. This must be set. Address string `protobuf:"bytes,30,opt,name=address,proto3" json:"address,omitempty" class:"public"` // @gotags: `class:"public"` // Tags for workers @@ -140,6 +142,13 @@ func (x *ServerWorkerStatus) GetName() string { return "" } +func (x *ServerWorkerStatus) GetDescription() string { + if x != nil { + return x.Description + } + return "" +} + func (x *ServerWorkerStatus) GetAddress() string { if x != nil { return x.Address @@ -171,21 +180,23 @@ var file_controller_servers_v1_servers_proto_rawDesc = []byte{ 0x54, 0x61, 0x67, 0x50, 0x61, 0x69, 0x72, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x22, - 0xaa, 0x01, 0x0a, 0x12, 0x53, 0x65, 0x72, 0x76, 0x65, 0x72, 0x57, 0x6f, 0x72, 0x6b, 0x65, 0x72, + 0xcc, 0x01, 0x0a, 0x12, 0x53, 0x65, 0x72, 0x76, 0x65, 0x72, 0x57, 0x6f, 0x72, 0x6b, 0x65, 0x72, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x1b, 0x0a, 0x09, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x5f, 0x69, 0x64, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x49, 0x64, 0x12, 0x12, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x14, 0x20, 0x01, 0x28, - 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x18, 0x0a, 0x07, 0x61, 0x64, 0x64, 0x72, 0x65, - 0x73, 0x73, 0x18, 0x1e, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x61, 0x64, 0x64, 0x72, 0x65, 0x73, - 0x73, 0x12, 0x32, 0x0a, 0x04, 0x74, 0x61, 0x67, 0x73, 0x18, 0x28, 0x20, 0x03, 0x28, 0x0b, 0x32, - 0x1e, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x73, 0x65, 0x72, - 0x76, 0x65, 0x72, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x54, 0x61, 0x67, 0x50, 0x61, 0x69, 0x72, 0x52, - 0x04, 0x74, 0x61, 0x67, 0x73, 0x12, 0x15, 0x0a, 0x06, 0x6b, 0x65, 0x79, 0x5f, 0x69, 0x64, 0x18, - 0x32, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x6b, 0x65, 0x79, 0x49, 0x64, 0x42, 0x38, 0x5a, 0x36, - 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, - 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, 0x69, 0x6e, - 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x3b, 0x73, - 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x09, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x20, 0x0a, 0x0b, 0x64, 0x65, 0x73, 0x63, 0x72, + 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x18, 0x19, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0b, 0x64, 0x65, + 0x73, 0x63, 0x72, 0x69, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x18, 0x0a, 0x07, 0x61, 0x64, 0x64, + 0x72, 0x65, 0x73, 0x73, 0x18, 0x1e, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x61, 0x64, 0x64, 0x72, + 0x65, 0x73, 0x73, 0x12, 0x32, 0x0a, 0x04, 0x74, 0x61, 0x67, 0x73, 0x18, 0x28, 0x20, 0x03, 0x28, + 0x0b, 0x32, 0x1e, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x73, + 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x2e, 0x76, 0x31, 0x2e, 0x54, 0x61, 0x67, 0x50, 0x61, 0x69, + 0x72, 0x52, 0x04, 0x74, 0x61, 0x67, 0x73, 0x12, 0x15, 0x0a, 0x06, 0x6b, 0x65, 0x79, 0x5f, 0x69, + 0x64, 0x18, 0x32, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x6b, 0x65, 0x79, 0x49, 0x64, 0x42, 0x38, + 0x5a, 0x36, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, + 0x68, 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, + 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, + 0x3b, 0x73, 0x65, 0x72, 0x76, 0x65, 0x72, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var (