Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
tcsc committed Oct 15, 2024
1 parent 7ee17ea commit 83c37af
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 31 deletions.
8 changes: 0 additions & 8 deletions lib/auth/authclient/clt.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,10 +625,6 @@ func (c *Client) OktaClient() services.Okta {
return c.APIClient.OktaClient()
}

func (c *Client) ProvisioningStatesClient() services.ProvisioningStates {
return nil
}

func (c *Client) SCIMClient() services.SCIM {
return c.APIClient.SCIMClient()
}
Expand Down Expand Up @@ -1877,8 +1873,4 @@ type ClientI interface {

// GenerateAppToken creates a JWT token with application access.
GenerateAppToken(ctx context.Context, req types.GenerateAppTokenRequest) (string, error)

// ProvisioningStatesClient manages access to the downstream user and group
// provisioning state storage service
ProvisioningStatesClient() services.ProvisioningStates
}
14 changes: 7 additions & 7 deletions lib/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ type testPack struct {
spiffeFederations *local.SPIFFEFederationService
staticHostUsers services.StaticHostUser
autoUpdateService services.AutoUpdateService
provisioningStatesS services.ProvisioningStates
provisioningStates services.ProvisioningStates
}

// testFuncs are functions to support testing an object in a cache.
Expand Down Expand Up @@ -382,7 +382,7 @@ func newPackWithoutCache(dir string, opts ...packOption) (*testPack, error) {
return nil, trace.Wrap(err)
}

p.provisioningStatesS, err = local.NewProvisioningStateService(p.backend)
p.provisioningStates, err = local.NewProvisioningStateService(p.backend)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -437,7 +437,7 @@ func newPack(dir string, setupConfig func(c Config) Config, opts ...packOption)
DatabaseObjects: p.databaseObjects,
StaticHostUsers: p.staticHostUsers,
AutoUpdateService: p.autoUpdateService,
ProvisioningStates: p.provisioningStatesS,
ProvisioningStates: p.provisioningStates,
MaxRetryPeriod: 200 * time.Millisecond,
EventsC: p.eventsC,
}))
Expand Down Expand Up @@ -846,7 +846,7 @@ func TestCompletenessInit(t *testing.T) {
SPIFFEFederations: p.spiffeFederations,
StaticHostUsers: p.staticHostUsers,
AutoUpdateService: p.autoUpdateService,
ProvisioningStates: p.provisioningStatesS,
ProvisioningStates: p.provisioningStates,
MaxRetryPeriod: 200 * time.Millisecond,
EventsC: p.eventsC,
}))
Expand Down Expand Up @@ -928,7 +928,7 @@ func TestCompletenessReset(t *testing.T) {
SPIFFEFederations: p.spiffeFederations,
StaticHostUsers: p.staticHostUsers,
AutoUpdateService: p.autoUpdateService,
ProvisioningStates: p.provisioningStatesS,
ProvisioningStates: p.provisioningStates,
MaxRetryPeriod: 200 * time.Millisecond,
EventsC: p.eventsC,
}))
Expand Down Expand Up @@ -1136,7 +1136,7 @@ func TestListResources_NodesTTLVariant(t *testing.T) {
SPIFFEFederations: p.spiffeFederations,
StaticHostUsers: p.staticHostUsers,
AutoUpdateService: p.autoUpdateService,
ProvisioningStates: p.provisioningStatesS,
ProvisioningStates: p.provisioningStates,
MaxRetryPeriod: 200 * time.Millisecond,
EventsC: p.eventsC,
neverOK: true, // ensure reads are never healthy
Expand Down Expand Up @@ -1229,7 +1229,7 @@ func initStrategy(t *testing.T) {
SPIFFEFederations: p.spiffeFederations,
StaticHostUsers: p.staticHostUsers,
AutoUpdateService: p.autoUpdateService,
ProvisioningStates: p.provisioningStatesS,
ProvisioningStates: p.provisioningStates,
MaxRetryPeriod: 200 * time.Millisecond,
EventsC: p.eventsC,
}))
Expand Down
4 changes: 2 additions & 2 deletions lib/cache/provisioning.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (

type provisioningStateGetter interface {
GetProvisioningState(context.Context, services.DownstreamID, services.ProvisioningStateID) (*provisioningv1.PrincipalState, error)
ListAllProvisioningStates(context.Context, int, *pagination.PageRequestToken) ([]*provisioningv1.PrincipalState, pagination.NextPageToken, error)
ListProvisioningStatesForAllDownstreams(context.Context, int, *pagination.PageRequestToken) ([]*provisioningv1.PrincipalState, pagination.NextPageToken, error)
}

type provisioningStateExecutor struct{}
Expand All @@ -49,7 +49,7 @@ func (provisioningStateExecutor) getAll(ctx context.Context, cache *Cache, loadS
var resourcesPage []*provisioningv1.PrincipalState
var err error

resourcesPage, nextPage, err := cache.ProvisioningStates.ListAllProvisioningStates(ctx, 0, &page)
resourcesPage, nextPage, err := cache.ProvisioningStates.ListProvisioningStatesForAllDownstreams(ctx, 0, &page)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
14 changes: 7 additions & 7 deletions lib/cache/provisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestProvisioningPrincipalState(t *testing.T) {
var result []*provisioningv1.PrincipalState
var pageToken pagination.PageRequestToken
for {
page, nextPage, err := src.ListAllProvisioningStates(ctx, 0, &pageToken)
page, nextPage, err := src.ListProvisioningStatesForAllDownstreams(ctx, 0, &pageToken)
if err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -82,28 +82,28 @@ func TestProvisioningPrincipalState(t *testing.T) {
return newProvisioningPrincipalState(s), nil
},
create: func(ctx context.Context, item *provisioningv1.PrincipalState) error {
_, err := fixturePack.provisioningStatesS.CreateProvisioningState(ctx, item)
_, err := fixturePack.provisioningStates.CreateProvisioningState(ctx, item)
return trace.Wrap(err)
},
update: func(ctx context.Context, item *provisioningv1.PrincipalState) error {
_, err := fixturePack.provisioningStatesS.UpdateProvisioningState(ctx, item)
_, err := fixturePack.provisioningStates.UpdateProvisioningState(ctx, item)
return trace.Wrap(err)
},
list: func(ctx context.Context) ([]*provisioningv1.PrincipalState, error) {
return collect(ctx, fixturePack.provisioningStatesS)
return collect(ctx, fixturePack.provisioningStates)
},
delete: func(ctx context.Context, id string) error {
return trace.Wrap(fixturePack.provisioningStatesS.DeleteProvisioningState(
return trace.Wrap(fixturePack.provisioningStates.DeleteProvisioningState(
ctx, testDownstreamID, services.ProvisioningStateID(id)))
},
deleteAll: func(ctx context.Context) error {
return trace.Wrap(fixturePack.provisioningStatesS.DeleteAllProvisioningStates(ctx))
return trace.Wrap(fixturePack.provisioningStates.DeleteAllProvisioningStates(ctx))
},
cacheList: func(ctx context.Context) ([]*provisioningv1.PrincipalState, error) {
return collect(ctx, fixturePack.cache.provisioningStatesCache)
},
cacheGet: func(ctx context.Context, id string) (*provisioningv1.PrincipalState, error) {
r, err := fixturePack.provisioningStatesS.GetProvisioningState(
r, err := fixturePack.provisioningStates.GetProvisioningState(
ctx, testDownstreamID, services.ProvisioningStateID(id))
return r, trace.Wrap(err)
},
Expand Down
8 changes: 6 additions & 2 deletions lib/services/local/provisioningstates.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,15 @@ func (ss *ProvisioningStateService) ListProvisioningStates(ctx context.Context,
return resp, pagination.NextPageToken(nextPage), nil
}

// ListAllProvisioningStates lists all provisioning state records for all
// ListProvisioningStatesForAllDownstreams lists all provisioning state records for all
// downstream receivers. Note that the returned record names may not be unique
// across all downstream receivers. Check the records' `DownstreamID` field
// to disambiguate.
func (ss *ProvisioningStateService) ListAllProvisioningStates(ctx context.Context, pageSize int, page *pagination.PageRequestToken) ([]*provisioningv1.PrincipalState, pagination.NextPageToken, error) {
func (ss *ProvisioningStateService) ListProvisioningStatesForAllDownstreams(
ctx context.Context,
pageSize int,
page *pagination.PageRequestToken,
) ([]*provisioningv1.PrincipalState, pagination.NextPageToken, error) {
if pageSize == 0 {
pageSize = provisioningStatePageSize
}
Expand Down
10 changes: 5 additions & 5 deletions lib/services/provisioningstates.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ type DownstreamProvisioningStates interface {
type ProvisioningStates interface {
DownstreamProvisioningStates

// ListProvisioningStates lists all provisioning state records for all
// downstream receivers. Note that the returned record names may not be unique
// across all downstream receivers. Check the records' `DownstreamID` field
// to disambiguate.
ListAllProvisioningStates(context.Context, int, *pagination.PageRequestToken) ([]*provisioningv1.PrincipalState, pagination.NextPageToken, error)
// ListProvisioningStatesForAllDownstreams lists all provisioning state
// records for all downstream receivers. Note that the returned record names
// may not be unique across all downstream receivers. Check the records'
// `DownstreamID` field to disambiguate.
ListProvisioningStatesForAllDownstreams(context.Context, int, *pagination.PageRequestToken) ([]*provisioningv1.PrincipalState, pagination.NextPageToken, error)

// DeleteAllProvisioningStates deletes all provisioning state records for
// all downstream receivers.
Expand Down

0 comments on commit 83c37af

Please sign in to comment.