Don't allow managed workers to be deleted (#3254)

This also adds checks to ensure the tag cannot be added to workers via
the API.
pull/3256/head
Jeff Mitchell 3 years ago committed by GitHub
parent 84b96f89a7
commit 3d5bed9ce8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -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
}

@ -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 ""
}

@ -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 {

Loading…
Cancel
Save