diff --git a/internal/daemon/common/worker_list.go b/internal/daemon/common/worker_list.go index 6763991d09..0a348eb0b3 100644 --- a/internal/daemon/common/worker_list.go +++ b/internal/daemon/common/worker_list.go @@ -81,3 +81,20 @@ func (w WorkerList) Filtered(eval *bexpr.Evaluator) (WorkerList, error) { } return ret, nil } + +// SeparateManagedWorkers divides the incoming workers into managed and +// unmanaged workers, respectively +func SeparateManagedWorkers(workers WorkerList) (managedWorkers, nonManagedWorkers WorkerList) { + // Build a set of managed and unmanaged workers + managedWorkers = make([]*server.Worker, 0, len(workers)) + nonManagedWorkers = make([]*server.Worker, 0, len(workers)) + for _, worker := range workers { + tags := worker.CanonicalTags() + if len(tags[ManagedWorkerTag]) != 0 { + managedWorkers = append(managedWorkers, worker) + } else { + nonManagedWorkers = append(nonManagedWorkers, worker) + } + } + return managedWorkers, nonManagedWorkers +} diff --git a/internal/daemon/controller/handlers/workers/worker_service.go b/internal/daemon/controller/handlers/workers/worker_service.go index d372a02ba8..f211b07d8b 100644 --- a/internal/daemon/controller/handlers/workers/worker_service.go +++ b/internal/daemon/controller/handlers/workers/worker_service.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/hashicorp/boundary/globals" + wl "github.com/hashicorp/boundary/internal/daemon/common" "github.com/hashicorp/boundary/internal/daemon/controller/auth" "github.com/hashicorp/boundary/internal/daemon/controller/common" "github.com/hashicorp/boundary/internal/daemon/controller/common/scopeids" @@ -317,7 +318,18 @@ func (s Service) DeleteWorker(ctx context.Context, req *pbs.DeleteWorkerRequest) if authResults.Error != nil { return nil, authResults.Error } - _, err := s.deleteFromRepo(ctx, req.GetId()) + + w, err := s.getFromRepo(ctx, req.GetId()) + if err != nil { + return nil, err + } + + managed, _ := wl.SeparateManagedWorkers(wl.WorkerList{w}) + if len(managed) == 1 { + return nil, handlers.InvalidArgumentErrorf("Error in provided request.", map[string]string{"id": "Managed workers cannot be deleted."}) + } + + _, err = s.deleteFromRepo(ctx, w.GetPublicId()) if err != nil { return nil, err } @@ -1012,6 +1024,8 @@ func validateStringForDb(str string) string { return "must be non-empty." case len(str) > 512: return "must be within 512 characters." + case str == wl.ManagedWorkerTag: + return "cannot be the managed worker tag." default: return "" } diff --git a/internal/daemon/controller/handlers/workers/worker_service_test.go b/internal/daemon/controller/handlers/workers/worker_service_test.go index 28bcd03ee4..77d68d54ce 100644 --- a/internal/daemon/controller/handlers/workers/worker_service_test.go +++ b/internal/daemon/controller/handlers/workers/worker_service_test.go @@ -12,6 +12,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/boundary/globals" + wl "github.com/hashicorp/boundary/internal/daemon/common" "github.com/hashicorp/boundary/internal/daemon/controller/auth" "github.com/hashicorp/boundary/internal/daemon/controller/common" "github.com/hashicorp/boundary/internal/daemon/controller/handlers" @@ -410,25 +411,33 @@ func TestDelete(t *testing.T) { s, err := NewService(ctx, repoFn, iamRepoFn, workerAuthRepoFn, nil) require.NoError(t, err, "Error when getting new worker service.") - w := server.TestKmsWorker(t, conn, wrap) + wUnmanaged := server.TestKmsWorker(t, conn, wrap, server.WithWorkerTags(&server.Tag{ + Key: "foo", + Value: "bar", + })) + wManaged := server.TestKmsWorker(t, conn, wrap, server.WithWorkerTags(&server.Tag{ + Key: wl.ManagedWorkerTag, + Value: "bar", + })) cases := []struct { - name string - scopeId string - req *pbs.DeleteWorkerRequest - res *pbs.DeleteWorkerResponse - err error + name string + scopeId string + req *pbs.DeleteWorkerRequest + res *pbs.DeleteWorkerResponse + err error + errContains string }{ { name: "Delete an Existing Worker", - scopeId: w.GetScopeId(), + scopeId: wUnmanaged.GetScopeId(), req: &pbs.DeleteWorkerRequest{ - Id: w.GetPublicId(), + Id: wUnmanaged.GetPublicId(), }, }, { name: "Delete bad worker id", - scopeId: w.GetScopeId(), + scopeId: wUnmanaged.GetScopeId(), req: &pbs.DeleteWorkerRequest{ Id: globals.WorkerPrefix + "_doesntexis", }, @@ -436,12 +445,21 @@ func TestDelete(t *testing.T) { }, { name: "Bad Worker Id formatting", - scopeId: w.GetScopeId(), + scopeId: wUnmanaged.GetScopeId(), req: &pbs.DeleteWorkerRequest{ Id: "bad_format", }, err: handlers.ApiErrorWithCode(codes.InvalidArgument), }, + { + name: "Cannot delete managed worker", + scopeId: wManaged.GetScopeId(), + req: &pbs.DeleteWorkerRequest{ + Id: wManaged.GetPublicId(), + }, + err: handlers.ApiErrorWithCode(codes.InvalidArgument), + errContains: "Managed workers cannot be deleted", + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -451,6 +469,10 @@ func TestDelete(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "DeleteWorker(%+v) got error %v, wanted %v", tc.req, gErr, tc.err) } + if tc.errContains != "" { + require.Error(gErr) + assert.Contains(gErr.Error(), tc.errContains) + } assert.EqualValuesf(tc.res, got, "DeleteWorker(%+v) got response %q, wanted %q", tc.req, got, tc.res) }) } @@ -1927,6 +1949,18 @@ func TestService_AddWorkerTags(t *testing.T) { }(), wantErrContains: "Tag values must be strings.", }, + { + name: "invalid-managed-tag-value", + req: func() *pbs.AddWorkerTagsRequest { + worker := server.TestKmsWorker(t, conn, wrapper) + return &pbs.AddWorkerTagsRequest{ + Id: worker.PublicId, + Version: worker.Version, + ApiTags: map[string]*structpb.ListValue{wl.ManagedWorkerTag: {Values: []*structpb.Value{structpb.NewStringValue("value2")}}}, + } + }(), + wantErrContains: "Tag keys cannot be the managed worker tag.", + }, { name: "mixed-invalid-tags", req: func() *pbs.AddWorkerTagsRequest { @@ -2078,6 +2112,18 @@ func TestService_SetWorkerTags(t *testing.T) { }(), wantErrContains: "Tag values must be strings.", }, + { + name: "invalid-managed-tag-value", + req: func() *pbs.SetWorkerTagsRequest { + worker := server.TestKmsWorker(t, conn, wrapper) + return &pbs.SetWorkerTagsRequest{ + Id: worker.PublicId, + Version: worker.Version, + ApiTags: map[string]*structpb.ListValue{wl.ManagedWorkerTag: {Values: []*structpb.Value{structpb.NewStringValue("value2")}}}, + } + }(), + wantErrContains: "Tag keys cannot be the managed worker tag.", + }, { name: "mixed-invalid-tags", req: func() *pbs.SetWorkerTagsRequest {