CyberCoveLLC/threshold
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refact(target): Remove old TargetType (#1618)

Browse Source 
Now that subtypes are registered via sub-packages, this TargetType is
not needed and should not be used. This removes it and updates the
target.Repository and targets.Service to use the updated code.

This was missed in eac433f7ff.
This commit is contained in:
Timothy Messier authored and GitHub committed 2021-10-20 08:35:33 -04:00
1 parent 769416c2ff
commit b5fbf9608e
8 files changed
+49 -390
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/servers/controller/handlers/targets/target_service.go
+3 -5
View File
@@ -1474,7 +1474,7 @@ func toProto(ctx context.Context, in target.Target, hostSources []target.HostSou
out.ScopeId = in.GetScopeId()
}
if outputFields.Has(globals.TypeField) {
out.Type = target.TcpTargetType.String()
out.Type = in.GetType().String()
}
if outputFields.Has(globals.DescriptionField) && in.GetDescription() != "" {
out.Description = wrapperspb.String(in.GetDescription())
@@ -1626,11 +1626,9 @@ func validateCreateRequest(req *pbs.CreateTargetRequest) error {
badFields["attributes.default_port"] = "This optional field cannot be set to 0."
}
}
switch req.GetItem().GetType() {
case target.TcpTargetType.String():
case "":
if req.GetItem().GetType() == "" {
badFields[globals.TypeField] = "This is a required field."
default:
} else if target.SubtypeFromType(req.GetItem().GetType()) == "" {
badFields[globals.TypeField] = "Unknown type provided."
}
if filter := req.GetItem().GetWorkerFilter(); filter != nil {
internal/servers/controller/handlers/targets/target_service_test.go
+11 -11
View File
@@ -117,7 +117,7 @@ func TestGet(t *testing.T) {
CreatedTime: tar.CreateTime.GetTimestamp(),
UpdatedTime: tar.UpdateTime.GetTimestamp(),
Scope: &scopes.ScopeInfo{Id: proj.GetPublicId(), Type: scope.Project.String(), ParentScopeId: o.GetPublicId()},
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
HostSetIds: []string{hs[0].GetPublicId(), hs[1].GetPublicId()},
HostSourceIds: []string{hs[0].GetPublicId(), hs[1].GetPublicId()},
Attributes: new(structpb.Struct),
@@ -212,7 +212,7 @@ func TestList(t *testing.T) {
CreatedTime: tar.GetCreateTime().GetTimestamp(),
UpdatedTime: tar.GetUpdateTime().GetTimestamp(),
Version: tar.GetVersion(),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: new(structpb.Struct),
SessionMaxSeconds: wrapperspb.UInt32(28800),
SessionConnectionLimit: wrapperspb.Int32(1),
@@ -228,7 +228,7 @@ func TestList(t *testing.T) {
CreatedTime: tar.GetCreateTime().GetTimestamp(),
UpdatedTime: tar.GetUpdateTime().GetTimestamp(),
Version: tar.GetVersion(),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: new(structpb.Struct),
SessionMaxSeconds: wrapperspb.UInt32(28800),
SessionConnectionLimit: wrapperspb.Int32(1),
@@ -421,7 +421,7 @@ func TestCreate(t *testing.T) {
ScopeId: proj.GetPublicId(),
Name: wrapperspb.String("name"),
Description: wrapperspb.String("desc"),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
"default_port": structpb.NewNumberValue(2),
}},
@@ -434,7 +434,7 @@ func TestCreate(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: proj.GetPublicId(), Type: scope.Project.String(), ParentScopeId: org.GetPublicId()},
Name: wrapperspb.String("name"),
Description: wrapperspb.String("desc"),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
"default_port": structpb.NewNumberValue(2),
}},
@@ -450,7 +450,7 @@ func TestCreate(t *testing.T) {
req: &pbs.CreateTargetRequest{Item: &pb.Target{
Name: wrapperspb.String("name"),
Description: wrapperspb.String("desc"),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
"default_port": structpb.NewNumberValue(0),
}},
@@ -623,7 +623,7 @@ func TestUpdate(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: proj.GetPublicId(), Type: scope.Project.String(), ParentScopeId: org.GetPublicId()},
Name: wrapperspb.String("name"),
Description: wrapperspb.String("desc"),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
"default_port": structpb.NewNumberValue(2),
}},
@@ -658,7 +658,7 @@ func TestUpdate(t *testing.T) {
Name: wrapperspb.String("name"),
Description: wrapperspb.String("desc"),
CreatedTime: tar.GetCreateTime().GetTimestamp(),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
"default_port": structpb.NewNumberValue(2),
}},
@@ -745,7 +745,7 @@ func TestUpdate(t *testing.T) {
Scope: &scopes.ScopeInfo{Id: proj.GetPublicId(), Type: scope.Project.String(), ParentScopeId: org.GetPublicId()},
Name: wrapperspb.String("default"),
CreatedTime: tar.GetCreateTime().GetTimestamp(),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
"default_port": structpb.NewNumberValue(2),
}},
@@ -778,7 +778,7 @@ func TestUpdate(t *testing.T) {
Name: wrapperspb.String("updated"),
Description: wrapperspb.String("default"),
CreatedTime: tar.GetCreateTime().GetTimestamp(),
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
"default_port": structpb.NewNumberValue(2),
}},
@@ -814,7 +814,7 @@ func TestUpdate(t *testing.T) {
Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
"default_port": structpb.NewNumberValue(2),
}},
Type: target.TcpTargetType.String(),
Type: tcp.Subtype.String(),
HostSetIds: hsIds,
HostSets: hostSets,
HostSourceIds: hostSourceIds,
internal/target/options.go
+10 -6
View File
@@ -1,6 +1,10 @@
package target
import "time"
import (
"time"
"github.com/hashicorp/boundary/internal/types/subtypes"
)
// GetOpts - iterate the inbound Options and return a struct
func GetOpts(opt ...Option) options {
@@ -24,7 +28,7 @@ type options struct {
WithScopeIds []string
WithScopeName string
WithUserId string
WithTargetType *TargetType
WithType subtypes.Subtype
WithHostSources []string
WithCredentialSources []string
WithSessionMaxSeconds uint32
@@ -43,7 +47,7 @@ func getDefaultOptions() options {
WithScopeIds: nil,
WithScopeName: "",
WithUserId: "",
WithTargetType: nil,
WithType: "",
WithHostSources: nil,
WithCredentialSources: nil,
WithSessionMaxSeconds: uint32((8 * time.Hour).Seconds()),
@@ -111,10 +115,10 @@ func WithUserId(userId string) Option {
}
}
// WithTargetType provides an option to search by a target type
func WithTargetType(t TargetType) Option {
// WithType provides an option to search by a target type
func WithType(t subtypes.Subtype) Option {
return func(o *options) {
o.WithTargetType = &t
o.WithType = t
}
}
internal/target/options_test.go
+6 -4
View File
@@ -3,6 +3,7 @@ package target
import (
"testing"
"github.com/hashicorp/boundary/internal/types/subtypes"
"github.com/stretchr/testify/assert"
)
@@ -82,12 +83,13 @@ func Test_GetOpts(t *testing.T) {
testOpts.WithPublicId = "testId"
assert.Equal(opts, testOpts)
})
t.Run("WithTargetType", func(t *testing.T) {
t.Run("WithType", func(t *testing.T) {
subtype := subtypes.Subtype("testtype")
assert := assert.New(t)
opts := GetOpts(WithTargetType(TcpTargetType))
opts := GetOpts(WithType(subtype))
testOpts := getDefaultOptions()
target := TcpTargetType
testOpts.WithTargetType = &target
target := subtype
testOpts.WithType = target
assert.Equal(opts, testOpts)
})
t.Run("WithHostSources", func(t *testing.T) {
internal/target/repository.go
+3 -3
View File
@@ -138,7 +138,7 @@ func (r *Repository) LookupTarget(ctx context.Context, publicIdOrName string, op
return subtype, hostSources, credSources, nil
}
// ListTargets in targets in a scope. Supports the WithScopeId, WithLimit, WithTargetType options.
// ListTargets in targets in a scope. Supports the WithScopeId, WithLimit, WithType options.
func (r *Repository) ListTargets(ctx context.Context, opt ...Option) ([]Target, error) {
const op = "target.(Repository).ListTargets"
opts := GetOpts(opt...)
@@ -151,8 +151,8 @@ func (r *Repository) ListTargets(ctx context.Context, opt ...Option) ([]Target,
if len(opts.WithScopeIds) != 0 {
where, args = append(where, "scope_id in (?)"), append(args, opts.WithScopeIds)
}
if opts.WithTargetType != nil {
where, args = append(where, "type = ?"), append(args, opts.WithTargetType.String())
if opts.WithType != "" {
where, args = append(where, "type = ?"), append(args, opts.WithType.String())
}
var foundTargets []*targetView
internal/target/repository_test.go
+15 -344
View File
@@ -1,11 +1,10 @@
package target_test
package target
import (
"testing"
"github.com/hashicorp/boundary/internal/db"
"github.com/hashicorp/boundary/internal/kms"
"github.com/hashicorp/boundary/internal/target"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -22,9 +21,9 @@ func TestNewRepository(t *testing.T) {
kms *kms.Kms
}
tests := []struct {
name string
args args
// want *target.Repository
name string
args args
want *Repository
wantErr bool
wantErrString string
}{
@@ -35,12 +34,12 @@ func TestNewRepository(t *testing.T) {
w: rw,
kms: testKms,
},
// want: &target.Repository{
// reader: rw,
// writer: rw,
// kms: testKms,
// defaultLimit: db.DefaultLimit,
// },
want: &Repository{
reader: rw,
writer: rw,
kms: testKms,
defaultLimit: db.DefaultLimit,
},
wantErr: false,
},
{
@@ -50,7 +49,7 @@ func TestNewRepository(t *testing.T) {
w: rw,
kms: nil,
},
// want: nil,
want: nil,
wantErr: true,
wantErrString: "target.NewRepository: nil kms: parameter violation: error #100",
},
@@ -61,7 +60,7 @@ func TestNewRepository(t *testing.T) {
w: nil,
kms: testKms,
},
// want: nil,
want: nil,
wantErr: true,
wantErrString: "target.NewRepository: nil writer: parameter violation: error #100",
},
@@ -72,7 +71,7 @@ func TestNewRepository(t *testing.T) {
w: rw,
kms: testKms,
},
// want: nil,
want: nil,
wantErr: true,
wantErrString: "target.NewRepository: nil reader: parameter violation: error #100",
},
@@ -80,7 +79,7 @@ func TestNewRepository(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert, require := assert.New(t), require.New(t)
got, err := target.NewRepository(tt.args.r, tt.args.w, tt.args.kms)
got, err := NewRepository(tt.args.r, tt.args.w, tt.args.kms)
if tt.wantErr {
require.Error(err)
assert.Equal(tt.wantErrString, err.Error())
@@ -88,335 +87,7 @@ func TestNewRepository(t *testing.T) {
}
require.NoError(err)
assert.NotNil(got)
// assert.Equal(tt.want, got)
assert.Equal(tt.want, got)
})
}
}
// func TestRepository_LookupTarget(t *testing.T) {
// t.Parallel()
// conn, _ := db.TestSetup(t, "postgres")
// wrapper := db.TestWrapper(t)
// testKms := kms.TestKms(t, conn, wrapper)
// iamRepo := iam.TestRepo(t, conn, wrapper)
// _, proj := iam.TestScopes(t, iamRepo)
// proj.Name = "project-name"
// _, _, err := iamRepo.UpdateScope(context.Background(), proj, 1, []string{"name"})
// require.NoError(t, err)
// rw := db.New(conn)
// repo, err := target.NewRepository(rw, rw, testKms)
// require.NoError(t, err)
// tgt := target.TestTcpTarget(t, conn, proj.PublicId, "target-name")
//
// tests := []struct {
// testName string
// id string
// name string
// scopeId string
// scopeName string
// wantErr bool
// }{
// {
// testName: "id",
// id: tgt.PublicId,
// wantErr: false,
// },
// {
// testName: "name only",
// name: tgt.Name,
// wantErr: true,
// },
// {
// testName: "scope id only",
// scopeId: proj.PublicId,
// wantErr: true,
// },
// {
// testName: "scope name only",
// scopeName: proj.Name,
// wantErr: true,
// },
// {
// testName: "scope name and id",
// scopeId: proj.PublicId,
// scopeName: proj.Name,
// wantErr: true,
// },
// {
// testName: "everything",
// name: tgt.Name,
// scopeId: proj.PublicId,
// scopeName: proj.Name,
// wantErr: true,
// },
// {
// testName: "name and scope name",
// name: tgt.Name,
// scopeName: proj.Name,
// wantErr: false,
// },
// {
// testName: "name and scope id",
// name: tgt.Name,
// scopeId: proj.PublicId,
// wantErr: false,
// },
// {
// testName: "id and name",
// id: tgt.PublicId,
// name: tgt.Name,
// scopeId: proj.PublicId,
// wantErr: true,
// },
// {
// testName: "id and scope name",
// id: tgt.PublicId,
// scopeName: proj.Name,
// wantErr: true,
// },
// {
// testName: "id and scope id",
// id: tgt.PublicId,
// scopeId: proj.PublicId,
// wantErr: true,
// },
// }
// for _, tt := range tests {
// t.Run(tt.testName, func(t *testing.T) {
// assert, require := assert.New(t), require.New(t)
// id := tt.id
// if tt.name != "" && tt.id == "" {
// id = tt.name
// }
// var opts []target.Option
// if tt.name != "" {
// opts = append(opts, target.WithName(tt.name))
// }
// if tt.scopeId != "" {
// opts = append(opts, target.WithScopeId(tt.scopeId))
// }
// if tt.scopeName != "" {
// opts = append(opts, target.WithScopeName(tt.scopeName))
// }
// got, _, _, err := repo.LookupTarget(context.Background(), id, opts...)
// if tt.wantErr {
// require.Error(err)
// return
// }
// require.NoError(err)
// assert.Equal(tgt.PublicId, got.GetPublicId())
// })
// }
// }
//
// func TestRepository_ListTargets(t *testing.T) {
// t.Parallel()
// conn, _ := db.TestSetup(t, "postgres")
// const testLimit = 10
// wrapper := db.TestWrapper(t)
// testKms := kms.TestKms(t, conn, wrapper)
// iamRepo := iam.TestRepo(t, conn, wrapper)
// _, proj := iam.TestScopes(t, iamRepo)
// rw := db.New(conn)
// repo, err := target.NewRepository(rw, rw, testKms, target.WithLimit(testLimit))
// require.NoError(t, err)
//
// type args struct {
// opt []target.Option
// }
// tests := []struct {
// name string
// createCnt int
// createScopeId string
// createScopeId2 string
// grantUserId string
// args args
// wantCnt int
// wantErr bool
// }{
// {
// name: "tcp-target",
// createCnt: 5,
// createScopeId: proj.PublicId,
// args: args{
// opt: []target.Option{target.WithTargetType(target.TcpTargetType), target.WithScopeIds([]string{proj.PublicId})},
// },
// wantCnt: 5,
// wantErr: false,
// },
// {
// name: "no-limit",
// createCnt: testLimit + 1,
// createScopeId: proj.PublicId,
// args: args{
// opt: []target.Option{target.WithLimit(-1), target.WithScopeIds([]string{proj.PublicId})},
// },
// wantCnt: testLimit + 1,
// wantErr: false,
// },
// {
// name: "default-limit",
// createCnt: testLimit + 1,
// createScopeId: proj.PublicId,
// args: args{
// opt: []target.Option{target.WithScopeIds([]string{proj.PublicId})},
// },
// wantCnt: testLimit,
// wantErr: false,
// },
// {
// name: "custom-limit",
// createCnt: testLimit + 1,
// createScopeId: proj.PublicId,
// args: args{
// opt: []target.Option{target.WithLimit(3), target.WithScopeIds([]string{proj.PublicId})},
// },
// wantCnt: 3,
// wantErr: false,
// },
// {
// name: "bad-org",
// createCnt: 1,
// createScopeId: proj.PublicId,
// args: args{
// opt: []target.Option{target.WithScopeIds([]string{"bad-id"})},
// },
// wantCnt: 0,
// wantErr: false,
// },
// }
// for _, tt := range tests {
// t.Run(tt.name, func(t *testing.T) {
// assert, require := assert.New(t), require.New(t)
// require.NoError(conn.Where("1=1").Delete(target.NewTestTcpTarget("")).Error)
// testGroups := []*target.TcpTarget{}
// for i := 0; i < tt.createCnt; i++ {
// switch {
// case tt.createScopeId2 != "" && i%2 == 0:
// testGroups = append(testGroups, target.TestTcpTarget(t, conn, tt.createScopeId2, strconv.Itoa(i)))
// default:
// testGroups = append(testGroups, target.TestTcpTarget(t, conn, tt.createScopeId, strconv.Itoa(i)))
// }
// }
// assert.Equal(tt.createCnt, len(testGroups))
// got, err := repo.ListTargets(context.Background(), tt.args.opt...)
// if tt.wantErr {
// require.Error(err)
// return
// }
// require.NoError(err)
// assert.Equal(tt.wantCnt, len(got))
// })
// }
// }
//
// func TestRepository_ListRoles_Multiple_Scopes(t *testing.T) {
// t.Parallel()
// conn, _ := db.TestSetup(t, "postgres")
// wrapper := db.TestWrapper(t)
// testKms := kms.TestKms(t, conn, wrapper)
// iamRepo := iam.TestRepo(t, conn, wrapper)
// _, proj1 := iam.TestScopes(t, iamRepo)
// _, proj2 := iam.TestScopes(t, iamRepo)
// rw := db.New(conn)
// repo, err := target.NewRepository(rw, rw, testKms)
// require.NoError(t, err)
//
// require.NoError(t, conn.Where("1=1").Delete(target.NewTestTcpTarget("")).Error)
//
// const numPerScope = 10
// var total int
// for i := 0; i < numPerScope; i++ {
// target.TestTcpTarget(t, conn, proj1.GetPublicId(), fmt.Sprintf("proj1-%d", i))
// total++
// target.TestTcpTarget(t, conn, proj2.GetPublicId(), fmt.Sprintf("proj2-%d", i))
// total++
// }
//
// got, err := repo.ListTargets(context.Background(), target.WithScopeIds([]string{"global", proj1.GetPublicId(), proj2.GetPublicId()}))
// require.NoError(t, err)
// assert.Equal(t, total, len(got))
// }
//
// func TestRepository_DeleteTarget(t *testing.T) {
// t.Parallel()
// conn, _ := db.TestSetup(t, "postgres")
// rw := db.New(conn)
// wrapper := db.TestWrapper(t)
// testKms := kms.TestKms(t, conn, wrapper)
// iamRepo := iam.TestRepo(t, conn, wrapper)
// _, proj := iam.TestScopes(t, iamRepo)
// repo, err := target.NewRepository(rw, rw, testKms)
// require.NoError(t, err)
//
// type args struct {
// target target.Target
// opt []target.Option
// }
// tests := []struct {
// name string
// args args
// wantRowsDeleted int
// wantErr bool
// wantErrMsg string
// }{
// {
// name: "valid",
// args: args{
// target: target.TestTcpTarget(t, conn, proj.PublicId, "valid"),
// },
// wantRowsDeleted: 1,
// wantErr: false,
// },
// {
// name: "no-public-id",
// args: args{
// target: func() target.Target {
// tar, _ := target.NewTcpTarget(proj.PublicId)
// return tar
// }(),
// },
// wantRowsDeleted: 0,
// wantErr: true,
// wantErrMsg: "target.(Repository).DeleteTarget: missing public id: parameter violation: error #100",
// },
// {
// name: "not-found",
// args: args{
// target: func() target.Target {
// id, err := target.NewTcpTargetId()
// require.NoError(t, err)
// tar, _ := target.NewTcpTarget(proj.PublicId)
// tar.PublicId = id
// return tar
// }(),
// },
// wantRowsDeleted: 0,
// wantErr: true,
// wantErrMsg: "db.LookupById: record not found, search issue: error #1100",
// },
// }
// for _, tt := range tests {
// t.Run(tt.name, func(t *testing.T) {
// assert := assert.New(t)
// deletedRows, err := repo.DeleteTarget(context.Background(), tt.args.target.GetPublicId(), tt.args.opt...)
// if tt.wantErr {
// assert.Error(err)
// assert.Equal(0, deletedRows)
// assert.Contains(err.Error(), tt.wantErrMsg)
// err = db.TestVerifyOplog(t, rw, tt.args.target.GetPublicId(), db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))
// assert.Error(err)
// assert.True(errors.IsNotFoundError(err))
// return
// }
// assert.NoError(err)
// assert.Equal(tt.wantRowsDeleted, deletedRows)
// foundGroup, _, _, err := repo.LookupTarget(context.Background(), tt.args.target.GetPublicId())
// assert.NoError(err)
// assert.Nil(foundGroup)
//
// err = db.TestVerifyOplog(t, rw, tt.args.target.GetPublicId(), db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))
// assert.NoError(err)
// })
// }
// }
internal/target/target.go
-16
View File
@@ -40,22 +40,6 @@ type Target interface {
Oplog(op oplog.OpType) oplog.Metadata
}
// TargetType defines the possible types for targets.
type TargetType uint32
const (
UnknownTargetType TargetType = 0
TcpTargetType TargetType = 1
)
// String returns a string representation of the target type.
func (t TargetType) String() string {
return [...]string{
"unknown",
"tcp",
}[t]
}
const (
targetsViewDefaultTable = "target_all_subtypes"
)
internal/target/tcp/repository_test.go
+1 -1
View File
@@ -164,7 +164,7 @@ func TestRepository_ListTargets(t *testing.T) {
createCnt: 5,
createScopeId: proj.PublicId,
args: args{
opt: []target.Option{target.WithTargetType(target.TcpTargetType), target.WithScopeIds([]string{proj.PublicId})},
opt: []target.Option{target.WithType(tcp.Subtype), target.WithScopeIds([]string{proj.PublicId})},
},
wantCnt: 5,
wantErr: false,