Update view to return host IDs in a set on set lookup (#1646)

This removes the optional call to the plugin.
pull/1647/head^2
Jeff Mitchell 5 years ago committed by GitHub
parent 6eba8a4787
commit 5e782b6ecc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -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';

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

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

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

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

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

@ -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()

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

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

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

Loading…
Cancel
Save