From 5e782b6eccf6135c48f6e6079fd02ccccfd412b2 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 29 Oct 2021 15:35:06 -0400 Subject: [PATCH] Update view to return host IDs in a set on set lookup (#1646) This removes the optional call to the plugin. --- .../postgres/19/06_preferred_endpoints.up.sql | 36 +++-- internal/host/options.go | 11 -- internal/host/plugin/host.go | 3 +- internal/host/plugin/host_set.go | 90 ++++++++++- internal/host/plugin/host_set_member_test.go | 103 ++++++++---- internal/host/plugin/options.go | 8 + internal/host/plugin/options_test.go | 6 + internal/host/plugin/repository_host_set.go | 147 +----------------- .../host/plugin/repository_host_set_test.go | 2 +- .../handlers/host_sets/host_set_service.go | 6 +- 10 files changed, 206 insertions(+), 206 deletions(-) diff --git a/internal/db/schema/migrations/oss/postgres/19/06_preferred_endpoints.up.sql b/internal/db/schema/migrations/oss/postgres/19/06_preferred_endpoints.up.sql index aedabd48e0..cba52df653 100644 --- a/internal/db/schema/migrations/oss/postgres/19/06_preferred_endpoints.up.sql +++ b/internal/db/schema/migrations/oss/postgres/19/06_preferred_endpoints.up.sql @@ -50,23 +50,25 @@ create trigger immutable_preferred_endpoint -- values. The delimiter depends on the value objects (e.g. if they need -- ordering). create view host_plugin_host_set_with_value_obj as -select - hs.public_id, - hs.catalog_id, - hc.plugin_id, - hs.name, - hs.description, - hs.create_time, - hs.update_time, - hs.version, - hs.attributes, - -- the string_agg(..) column will be null if there are no associated value objects - string_agg(distinct concat_ws('=', hspe.priority, hspe.condition), '|') as preferred_endpoints -from - host_plugin_set hs - join host_plugin_catalog hc on hs.catalog_id = hc.public_id - left outer join host_set_preferred_endpoint hspe on hs.public_id = hspe.host_set_id -group by hs.public_id, hc.plugin_id; + select + hs.public_id, + hs.catalog_id, + hc.plugin_id, + hs.name, + hs.description, + hs.create_time, + hs.update_time, + hs.version, + hs.attributes, + -- the string_agg(..) column will be null if there are no associated value objects + string_agg(distinct concat_ws('=', hspe.priority, hspe.condition), '|') as preferred_endpoints, + string_agg(distinct hpsm.host_id, '|') as host_ids + from + host_plugin_set hs + join host_plugin_catalog hc on hs.catalog_id = hc.public_id + left outer join host_set_preferred_endpoint hspe on hs.public_id = hspe.host_set_id + left outer join host_plugin_set_member hpsm on hs.public_id = hpsm.set_id + group by hs.public_id, hc.plugin_id; comment on view host_plugin_host_set_with_value_obj is 'host plugin host set with its associated value objects'; diff --git a/internal/host/options.go b/internal/host/options.go index 87d27c1555..6e2afe4d67 100644 --- a/internal/host/options.go +++ b/internal/host/options.go @@ -19,7 +19,6 @@ type options struct { WithLimit int WithOrderByCreateTime bool Ascending bool - WithSetMembers bool } func getDefaultOptions() options { @@ -45,13 +44,3 @@ func WithOrderByCreateTime(ascending bool) Option { return nil } } - -// WithSetMembers controls whether to include set members in a lookup. This will -// always be true for static and may be true for plugin when we have caching, -// but for now this skips API calls on authentication. -func WithSetMembers(with bool) Option { - return func(o *options) error { - o.WithSetMembers = true - return nil - } -} diff --git a/internal/host/plugin/host.go b/internal/host/plugin/host.go index 2da402973b..73db33d152 100644 --- a/internal/host/plugin/host.go +++ b/internal/host/plugin/host.go @@ -24,13 +24,14 @@ type Host struct { // newHost creates a new in memory Host assigned to catalogId with an address. // Supported options: WithName, WithDescription, WithIpAddresses, WithDnsNames, -// WithPluginId. Others ignored. +// WithPluginId, WithPublicId. Others ignored. func NewHost(ctx context.Context, catalogId, externalId string, opt ...Option) *Host { opts := getOpts(opt...) h := &Host{ PluginId: opts.withPluginId, Host: &store.Host{ + PublicId: opts.withPublicId, CatalogId: catalogId, ExternalId: externalId, Name: opts.withName, diff --git a/internal/host/plugin/host_set.go b/internal/host/plugin/host_set.go index e0703c83a7..ee18fd8d92 100644 --- a/internal/host/plugin/host_set.go +++ b/internal/host/plugin/host_set.go @@ -2,7 +2,12 @@ package plugin import ( "context" + "fmt" + "sort" + "strconv" + "strings" + "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host/plugin/store" "github.com/hashicorp/boundary/internal/oplog" @@ -12,7 +17,9 @@ import ( // A HostSet is a collection of hosts from the set's catalog. type HostSet struct { *store.HostSet - tableName string `gorm:"-"` + PluginId string `gorm:"-"` + HostIds []string `gorm:"-"` + tableName string `gorm:"-"` } // NewHostSet creates a new in memory HostSet assigned to catalogId. Attributes, @@ -81,3 +88,84 @@ func (s *HostSet) oplog(op oplog.OpType) oplog.Metadata { } return metadata } + +// hostSetAgg is a view that aggregates the host set's value objects in to +// string fields delimited with the aggregateDelimiter of "|" +type hostSetAgg struct { + PublicId string `gorm:"primary_key"` + CatalogId string + PluginId string + Name string + Description string + CreateTime *timestamp.Timestamp + UpdateTime *timestamp.Timestamp + Version uint32 + Attributes []byte + PreferredEndpoints string + HostIds string +} + +func (agg *hostSetAgg) toHostSet(ctx context.Context) (*HostSet, error) { + const op = "plugin.(hostSetAgg).toHostSet" + const aggregateDelimiter = "|" + const priorityDelimiter = "=" + hs := allocHostSet() + hs.PublicId = agg.PublicId + hs.CatalogId = agg.CatalogId + hs.PluginId = agg.PluginId + hs.Name = agg.Name + hs.Description = agg.Description + hs.CreateTime = agg.CreateTime + hs.UpdateTime = agg.UpdateTime + hs.Version = agg.Version + hs.Attributes = agg.Attributes + if agg.HostIds != "" { + hs.HostIds = strings.Split(agg.HostIds, aggregateDelimiter) + } + if agg.PreferredEndpoints != "" { + eps := strings.Split(agg.PreferredEndpoints, aggregateDelimiter) + if len(eps) > 0 { + // We want to protect against someone messing with the DB + // and not panic, so we do a bit of a dance here + var sortErr error + sort.Slice(eps, func(i, j int) bool { + epi := strings.Split(eps[i], priorityDelimiter) + if len(epi) != 2 { + sortErr = errors.New(ctx, errors.NotSpecificIntegrity, op, fmt.Sprintf("preferred endpoint %s had unexpected fields", eps[i])) + return false + } + epj := strings.Split(eps[j], priorityDelimiter) + if len(epj) != 2 { + sortErr = errors.New(ctx, errors.NotSpecificIntegrity, op, fmt.Sprintf("preferred endpoint %s had unexpected fields", eps[j])) + return false + } + indexi, err := strconv.Atoi(epi[0]) + if err != nil { + sortErr = errors.Wrap(ctx, err, op) + return false + } + indexj, err := strconv.Atoi(epj[0]) + if err != nil { + sortErr = errors.Wrap(ctx, err, op) + return false + } + return indexi < indexj + }) + if sortErr != nil { + return nil, sortErr + } + for i, ep := range eps { + // At this point they're in the correct order, but we still + // have to strip off the priority + eps[i] = strings.Split(ep, priorityDelimiter)[1] + } + hs.PreferredEndpoints = eps + } + } + return hs, nil +} + +// TableName returns the table name for gorm +func (agg *hostSetAgg) TableName() string { + return "host_plugin_host_set_with_value_obj" +} diff --git a/internal/host/plugin/host_set_member_test.go b/internal/host/plugin/host_set_member_test.go index 47c67844ed..270289a468 100644 --- a/internal/host/plugin/host_set_member_test.go +++ b/internal/host/plugin/host_set_member_test.go @@ -32,60 +32,74 @@ func TestHostSetMember_InsertDelete(t *testing.T) { plg.GetPublicId(): NewWrappingPluginClient(&plgpb.UnimplementedHostPluginServiceServer{}), } + repo, err := NewRepository(rw, rw, kms, plgm) + require.NoError(t, err) + cats := TestCatalogs(t, conn, prj.PublicId, plg.PublicId, 2) blueCat := cats[0] blueSet1 := TestSet(t, conn, kms, blueCat, plgm) blueSet2 := TestSet(t, conn, kms, blueCat, plgm) + blueSet3 := TestSet(t, conn, kms, blueCat, plgm) hostId, err := db.NewPublicId(HostPrefix) require.NoError(t, err) - blueHost1 := NewHost(ctx, blueCat.PublicId, "abcd", withPluginId(plg.GetPublicId())) + blueHost1 := NewHost(ctx, blueCat.PublicId, "blue1", withPluginId(plg.GetPublicId())) blueHost1.PublicId = hostId require.NoError(t, rw.Create(ctx, blueHost1)) hostId, err = db.NewPublicId(HostPrefix) require.NoError(t, err) - blueHost2 := NewHost(ctx, blueCat.PublicId, "zyxw", withPluginId(plg.GetPublicId())) + blueHost2 := NewHost(ctx, blueCat.PublicId, "blue2", withPluginId(plg.GetPublicId())) blueHost2.PublicId = hostId require.NoError(t, rw.Create(ctx, blueHost2)) + hostId, err = db.NewPublicId(HostPrefix) + require.NoError(t, err) + blueHost3 := NewHost(ctx, blueCat.PublicId, "blue3", withPluginId(plg.GetPublicId())) + blueHost3.PublicId = hostId + require.NoError(t, rw.Create(ctx, blueHost3)) + + hostId, err = db.NewPublicId(HostPrefix) + require.NoError(t, err) + blueHost4 := NewHost(ctx, blueCat.PublicId, "blue4", withPluginId(plg.GetPublicId())) + blueHost4.PublicId = hostId + require.NoError(t, rw.Create(ctx, blueHost4)) + greenCat := cats[1] greenSet := TestSet(t, conn, kms, greenCat, plgm) tests := []struct { name string - sets []string - host *Host + set string + hosts []*Host wantErr bool direct bool }{ { - name: "valid-host-in-set", - sets: []string{blueSet1.PublicId}, - host: blueHost1, + name: "valid-host-in-set", + set: blueSet1.PublicId, + hosts: []*Host{blueHost1}, }, { - name: "valid-other-host-in-set", - sets: []string{blueSet2.PublicId}, - host: blueHost2, + name: "valid-other-host-in-set", + set: blueSet2.PublicId, + hosts: []*Host{blueHost2}, }, { - name: "invalid-diff-catalogs", - sets: []string{greenSet.PublicId}, - host: blueHost1, - wantErr: true, + name: "valid-two-hosts-in-set", + set: blueSet3.PublicId, + hosts: []*Host{blueHost3, blueHost4}, }, { - name: "test-vet-for-write-no-set", - host: blueHost1, - sets: []string{""}, + name: "invalid-diff-catalogs", + set: greenSet.PublicId, + hosts: []*Host{blueHost1}, wantErr: true, - direct: true, }, { - name: "test-vet-for-write-no-host", - sets: []string{blueSet1.PublicId}, + name: "test-vet-for-write-no-set", + hosts: []*Host{blueHost1}, wantErr: true, direct: true, }, @@ -94,19 +108,21 @@ func TestHostSetMember_InsertDelete(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - for _, set := range tt.sets { + var hostIds []string + for _, host := range tt.hosts { var got *HostSetMember var err error + hostIds = append(hostIds, host.PublicId) if !tt.direct { - got, err = NewHostSetMember(ctx, set, tt.host.PublicId) + got, err = NewHostSetMember(ctx, tt.set, host.PublicId) } else { got = &HostSetMember{ HostSetMember: &store.HostSetMember{ - SetId: set, + SetId: tt.set, }, } - if tt.host != nil { - got.HostId = tt.host.PublicId + if host != nil { + got.HostId = host.PublicId } } require.NoError(err) @@ -120,15 +136,21 @@ func TestHostSetMember_InsertDelete(t *testing.T) { } // Run a test on the aggregate to validate looking up sets - agg := &hostAgg{PublicId: tt.host.PublicId} - require.NoError(rw.LookupByPublicId(ctx, agg)) - h, err := agg.toHost(ctx) + for _, host := range tt.hosts { + agg := &hostAgg{PublicId: host.PublicId} + require.NoError(rw.LookupByPublicId(ctx, agg)) + h, err := agg.toHost(ctx) + require.NoError(err) + assert.ElementsMatch(h.SetIds, []string{tt.set}) + } + + set, _, err := repo.LookupSet(ctx, tt.set) require.NoError(err) - assert.ElementsMatch(h.SetIds, tt.sets) + require.NotNil(set) + require.ElementsMatch(set.HostIds, hostIds) }) } - repo, err := NewRepository(rw, rw, kms, plgm) - require.NoError(t, err) + hosts, err := repo.ListHostsBySetIds(ctx, []string{blueSet1.PublicId, blueSet2.PublicId}) require.NoError(t, err) require.Len(t, hosts, 2) @@ -136,7 +158,7 @@ func TestHostSetMember_InsertDelete(t *testing.T) { // Base case the count by catalog ID hosts, err = repo.ListHostsByCatalogId(ctx, blueCat.PublicId) require.NoError(t, err) - assert.Len(t, hosts, 2) + assert.Len(t, hosts, 4) // Delete first membership, validate host is gone got, err := NewHostSetMember(ctx, blueSet1.PublicId, blueHost1.PublicId) @@ -149,7 +171,7 @@ func TestHostSetMember_InsertDelete(t *testing.T) { assert.NoError(t, db.TestVerifyOplog(t, rw, blueHost1.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) hosts, err = repo.ListHostsByCatalogId(ctx, blueCat.PublicId) require.NoError(t, err) - require.Len(t, hosts, 1) + require.Len(t, hosts, 3) // Delete second, validate second host is gone got, err = NewHostSetMember(ctx, blueSet2.PublicId, blueHost2.PublicId) @@ -162,6 +184,21 @@ func TestHostSetMember_InsertDelete(t *testing.T) { assert.NoError(t, db.TestVerifyOplog(t, rw, blueHost2.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) hosts, err = repo.ListHostsByCatalogId(ctx, blueCat.PublicId) require.NoError(t, err) + require.Len(t, hosts, 2) + + // Delete third set, validate remaining hosts are gone + gotSet, err := NewHostSet(ctx, blueCat.PublicId) + require.NoError(t, err) + require.NotNil(t, got) + gotSet.PublicId = blueSet3.PublicId + num, err = rw.Delete(ctx, gotSet) + require.NoError(t, err) + assert.Equal(t, 1, num) + require.NoError(t, repo.DeleteOrphanedHosts(ctx, blueCat.ScopeId)) + assert.NoError(t, db.TestVerifyOplog(t, rw, blueHost3.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) + assert.NoError(t, db.TestVerifyOplog(t, rw, blueHost4.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) + hosts, err = repo.ListHostsByCatalogId(ctx, blueCat.PublicId) + require.NoError(t, err) require.Len(t, hosts, 0) } diff --git a/internal/host/plugin/options.go b/internal/host/plugin/options.go index 9829982f68..e84d85702a 100644 --- a/internal/host/plugin/options.go +++ b/internal/host/plugin/options.go @@ -16,6 +16,7 @@ type Option func(*options) // options = how options are represented type options struct { + withPublicId string withPluginId string withName string withDescription string @@ -41,6 +42,13 @@ func withPluginId(with string) Option { } } +// WithPublicId provides an optional public id. +func WithPublicId(with string) Option { + return func(o *options) { + o.withPublicId = with + } +} + // WithDescription provides an optional description. func WithDescription(desc string) Option { return func(o *options) { diff --git a/internal/host/plugin/options_test.go b/internal/host/plugin/options_test.go index a8c4b2b3f0..fce504999b 100644 --- a/internal/host/plugin/options_test.go +++ b/internal/host/plugin/options_test.go @@ -8,6 +8,12 @@ import ( func Test_GetOpts(t *testing.T) { t.Parallel() + t.Run("WithPublicId", func(t *testing.T) { + opts := getOpts(WithPublicId("test")) + testOpts := getDefaultOptions() + testOpts.withPublicId = "test" + assert.Equal(t, opts, testOpts) + }) t.Run("WithName", func(t *testing.T) { opts := getOpts(WithName("test")) testOpts := getDefaultOptions() diff --git a/internal/host/plugin/repository_host_set.go b/internal/host/plugin/repository_host_set.go index cf755b8b17..6f3d361f58 100644 --- a/internal/host/plugin/repository_host_set.go +++ b/internal/host/plugin/repository_host_set.go @@ -3,12 +3,8 @@ package plugin import ( "context" "fmt" - "sort" - "strconv" - "strings" "github.com/hashicorp/boundary/internal/db" - "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host" "github.com/hashicorp/boundary/internal/kms" @@ -16,7 +12,6 @@ import ( "github.com/hashicorp/boundary/internal/oplog" hostplugin "github.com/hashicorp/boundary/internal/plugin/host" hcpb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostcatalogs" - hspb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostsets" pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostsets" plgpb "github.com/hashicorp/boundary/sdk/pbs/plugin" "google.golang.org/grpc/codes" @@ -152,78 +147,26 @@ func (r *Repository) CreateSet(ctx context.Context, scopeId string, s *HostSet, // LookupSet will look up a host set in the repository and return the host set, // as well as host IDs that match. If the host set is not found, it will return -// nil, nil, nil, nil. Supported options: WithSetMembers, which requests that -// host IDs contained within the set are looked up and returned. (In the future -// we may make it automatic to return this if it's coming from the database.) -func (r *Repository) LookupSet(ctx context.Context, publicId string, opt ...host.Option) (*HostSet, []string, *hostplugin.Plugin, error) { +// nil, nil, nil. No options are currently supported. +func (r *Repository) LookupSet(ctx context.Context, publicId string, _ ...host.Option) (*HostSet, *hostplugin.Plugin, error) { const op = "plugin.(Repository).LookupSet" if publicId == "" { - return nil, nil, nil, errors.New(ctx, errors.InvalidParameter, op, "no public id") + return nil, nil, errors.New(ctx, errors.InvalidParameter, op, "no public id") } - opts, err := host.GetOpts(opt...) - if err != nil { - return nil, nil, nil, errors.Wrap(ctx, err, op) - } - - sets, plg, err := r.getSets(ctx, publicId, "", opt...) + sets, plg, err := r.getSets(ctx, publicId, "") if err != nil { - return nil, nil, nil, errors.Wrap(ctx, err, op) + return nil, nil, errors.Wrap(ctx, err, op) } switch { case len(sets) == 0: - return nil, nil, nil, nil // not an error to return no rows for a "lookup" + return nil, nil, nil // not an error to return no rows for a "lookup" case len(sets) > 1: - return nil, nil, nil, errors.New(ctx, errors.NotSpecificIntegrity, op, fmt.Sprintf("%s matched more than 1 ", publicId)) + return nil, nil, errors.New(ctx, errors.NotSpecificIntegrity, op, fmt.Sprintf("%s matched more than 1 ", publicId)) } - setToReturn := sets[0] - var hostIdsToReturn []string - - // FIXME: change to use the database - if plg != nil && opts.WithSetMembers { - cat, persisted, err := r.getCatalog(ctx, setToReturn.GetCatalogId()) - if err != nil { - return nil, nil, nil, errors.Wrap(ctx, err, op) - } - plgCat, err := toPluginCatalog(ctx, cat) - if err != nil { - return nil, nil, nil, errors.Wrap(ctx, err, op) - } - plgSet, err := toPluginSet(ctx, setToReturn) - if err != nil { - return nil, nil, nil, errors.Wrap(ctx, err, op) - } - plgClient, ok := r.plugins[plg.GetPublicId()] - if !ok { - return nil, nil, nil, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("no plugin found for plugin id %s", plg.GetPublicId())) - } - resp, err := plgClient.ListHosts(ctx, &plgpb.ListHostsRequest{ - Catalog: plgCat, - Sets: []*hspb.HostSet{plgSet}, - Persisted: persisted, - }) - switch { - case err != nil: - // If it's just not implemented, e.g. for tests, don't error out, return what we have - if status.Code(err) != codes.Unimplemented { - return nil, nil, nil, errors.Wrap(ctx, err, op) - } - case resp != nil: - for _, respHost := range resp.GetHosts() { - hostId, err := newHostId(ctx, setToReturn.GetCatalogId(), respHost.GetExternalId()) - if err != nil { - return nil, nil, nil, errors.Wrap(ctx, err, op) - } - hostIdsToReturn = append(hostIdsToReturn, hostId) - } - } - } - - sort.Strings(hostIdsToReturn) - - return setToReturn, hostIdsToReturn, plg, nil + return sets[0], plg, nil } // ListSets returns a slice of HostSets for the catalogId. WithLimit is the @@ -385,80 +328,6 @@ func (r *Repository) getSets(ctx context.Context, publicId string, catalogId str return sets, plg, nil } -// hostSetAgg is a view that aggregates the host set's value objects in to -// string fields delimited with the aggregateDelimiter of "|" -type hostSetAgg struct { - PublicId string `gorm:"primary_key"` - CatalogId string - PluginId string - Name string - Description string - CreateTime *timestamp.Timestamp - UpdateTime *timestamp.Timestamp - Version uint32 - Attributes []byte - PreferredEndpoints string -} - -func (agg *hostSetAgg) toHostSet(ctx context.Context) (*HostSet, error) { - const op = "plugin.(hostSetAgg).toHostSet" - const aggregateDelimiter = "|" - const priorityDelimiter = "=" - hs := allocHostSet() - hs.PublicId = agg.PublicId - hs.CatalogId = agg.CatalogId - hs.Name = agg.Name - hs.Description = agg.Description - hs.CreateTime = agg.CreateTime - hs.UpdateTime = agg.UpdateTime - hs.Version = agg.Version - hs.Attributes = agg.Attributes - if agg.PreferredEndpoints != "" { - eps := strings.Split(agg.PreferredEndpoints, aggregateDelimiter) - if len(eps) > 0 { - // We want to protect against someone messing with the DB - // and not panic, so we do a bit of a dance here - var sortErr error - sort.Slice(eps, func(i, j int) bool { - epi := strings.Split(eps[i], priorityDelimiter) - if len(epi) != 2 { - sortErr = errors.New(ctx, errors.NotSpecificIntegrity, op, fmt.Sprintf("preferred endpoint %s had unexpected fields", eps[i])) - return false - } - epj := strings.Split(eps[j], priorityDelimiter) - if len(epj) != 2 { - sortErr = errors.New(ctx, errors.NotSpecificIntegrity, op, fmt.Sprintf("preferred endpoint %s had unexpected fields", eps[j])) - return false - } - indexi, err := strconv.Atoi(epi[0]) - if err != nil { - sortErr = errors.Wrap(ctx, err, op) - return false - } - indexj, err := strconv.Atoi(epj[0]) - if err != nil { - sortErr = errors.Wrap(ctx, err, op) - return false - } - return indexi < indexj - }) - if sortErr != nil { - return nil, sortErr - } - for i, ep := range eps { - // At this point they're in the correct order, but we still - // have to strip off the priority - eps[i] = strings.Split(ep, priorityDelimiter)[1] - } - hs.PreferredEndpoints = eps - } - } - return hs, nil -} - -// TableName returns the table name for gorm -func (agg *hostSetAgg) TableName() string { return "host_plugin_host_set_with_value_obj" } - // toPluginSet returns a host set in the format expected by the host plugin system. func toPluginSet(ctx context.Context, in *HostSet) (*pb.HostSet, error) { const op = "plugin.toPluginSet" diff --git a/internal/host/plugin/repository_host_set_test.go b/internal/host/plugin/repository_host_set_test.go index 0a8895772a..be6312a5d4 100644 --- a/internal/host/plugin/repository_host_set_test.go +++ b/internal/host/plugin/repository_host_set_test.go @@ -359,7 +359,7 @@ func TestRepository_LookupSet(t *testing.T) { repo, err := NewRepository(rw, rw, kms, plgm) assert.NoError(err) require.NotNil(repo) - got, _, _, err := repo.LookupSet(ctx, tt.in) + got, _, err := repo.LookupSet(ctx, tt.in) if tt.wantIsErr != 0 { assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) diff --git a/internal/servers/controller/handlers/host_sets/host_set_service.go b/internal/servers/controller/handlers/host_sets/host_set_service.go index cd047b20af..a56a569ece 100644 --- a/internal/servers/controller/handlers/host_sets/host_set_service.go +++ b/internal/servers/controller/handlers/host_sets/host_set_service.go @@ -423,7 +423,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (host.Set, []host.H if err != nil { return nil, nil, nil, err } - hset, hostIds, hsplg, err := repo.LookupSet(ctx, id, host.WithSetMembers(true)) + hset, hsplg, err := repo.LookupSet(ctx, id) if err != nil { return nil, nil, nil, err } @@ -432,7 +432,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (host.Set, []host.H } hs = hset plg = toPluginInfo(hsplg) - for _, h := range hostIds { + for _, h := range hset.HostIds { hl = append(hl, &plugin.Host{ Host: &plugstore.Host{ PublicId: h, @@ -690,7 +690,7 @@ func (s Service) parentAndAuthResult(ctx context.Context, id string, a action.Ty } set = ss case plugin.Subtype: - ps, _, _, err := pluginRepo.LookupSet(ctx, id) + ps, _, err := pluginRepo.LookupSet(ctx, id) if err != nil { res.Error = err return nil, res