Skip to content

Use CompareAndSwap for OktaAssignments instead of lock. #24727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mdwn merged 4 commits into from
Apr 18, 2023
Merged

Use CompareAndSwap for OktaAssignments instead of lock. #24727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d6809ed
Use CompareAndSwap for OktaAssignments instead of lock.
Apr 18, 2023
2da6247
Use currentAssignment instead of backendAssignment.
Apr 18, 2023
6bc7e9d
Update the last transition time.
Apr 18, 2023
6064538
Set Metadata during update.
Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions api/types/okta.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ func (o *OktaImportRuleMatchV1) CheckAndSetDefaults() error {
type OktaAssignment interface {
ResourceWithLabels

// SetMetadata will set the metadata for the Okta assignment.
SetMetadata(metadata Metadata)
// GetUser will return the user that the Okta assignment actions applies to.
GetUser() string
// GetActions will return the list of actions that will be performed as part of this assignment.
Expand All @@ -201,6 +203,11 @@ func NewOktaAssignment(metadata Metadata, spec OktaAssignmentSpecV1) (OktaAssign
return o, nil
}

// SetMetadata will set the metadata for the Okta assignment.
func (o *OktaAssignmentV1) SetMetadata(metadata Metadata) {
o.Metadata = metadata
}

// GetUser returns the user that the actions will be applied to.
func (o *OktaAssignmentV1) GetUser() string {
return o.Spec.User
Expand Down
69 changes: 21 additions & 48 deletions lib/services/local/okta.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package local

import (
"context"
"time"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
Expand All @@ -34,9 +33,6 @@ const (
oktaImportRuleMaxPageSize = 200
oktaAssignmentPrefix = "okta_assignment"
oktaAssignmentMaxPageSize = 200

oktaAssignmentModifyLockTTL = 10 * time.Second
oktaAssignmentModifyLock = "okta_assignment_modify_lock"
)

// OktaService manages Okta resources in the Backend.
Expand Down Expand Up @@ -116,17 +112,7 @@ func (o *OktaService) ListOktaAssignments(ctx context.Context, pageSize int, nex

// GetOktaAssignment returns the specified Okta assignment resources.
func (o *OktaService) GetOktaAssignment(ctx context.Context, name string) (types.OktaAssignment, error) {
var assignment types.OktaAssignment
err := o.assignmentSvc.RunWhileLocked(ctx, oktaAssignmentModifyLock, oktaAssignmentModifyLockTTL, func(ctx context.Context, b backend.Backend) error {
var err error
assignment, err = o.assignmentSvc.GetResource(ctx, name)
if err != nil {
return trace.Wrap(err)
}

return nil
})
return assignment, trace.Wrap(err)
return o.assignmentSvc.GetResource(ctx, name)
}

// CreateOktaAssignment creates a new Okta assignment resource.
Expand All @@ -137,42 +123,38 @@ func (o *OktaService) CreateOktaAssignment(ctx context.Context, assignment types
// UpdateOktaAssignment updates an existing Okta assignment resource.
func (o *OktaService) UpdateOktaAssignment(ctx context.Context, assignment types.OktaAssignment) (types.OktaAssignment, error) {
var previousAssignment types.OktaAssignment
err := o.assignmentSvc.RunWhileLocked(ctx, oktaAssignmentModifyLock, oktaAssignmentModifyLockTTL, func(ctx context.Context, b backend.Backend) error {
var err error
previousAssignment, err = o.assignmentSvc.GetResource(ctx, assignment.GetName())
if err != nil {
return trace.Wrap(err)
}

// Make a copy so that we don't modify the previous assignment.
previousCopy := previousAssignment.Copy()
previousActions := previousCopy.GetActions()
err := o.assignmentSvc.UpdateAndSwapResource(ctx, assignment.GetName(), func(currentAssignment types.OktaAssignment) error {
previousAssignment = currentAssignment.Copy()
currentActions := currentAssignment.GetActions()

if len(previousActions) != len(assignment.GetActions()) {
if len(currentActions) != len(assignment.GetActions()) {
return trace.BadParameter("Update to Okta assignment %s failed because the previous version has a different number of actions", assignment.GetName())
}

// Make sure that the status transitions of the updated assignment are valid.
for i, action := range assignment.GetActions() {
previousAction := previousActions[i]
currentAction := currentActions[i]

// Ensure that the previous actions are equal
if !actionsMatch(previousAction, action) {
if !actionsMatch(currentAction, action) {
return trace.BadParameter("action mismatch when updating Okta assignment %s", assignment.GetName())
}

// Don't check the status transition if the statuses are equal and the last transitions are equal.
if previousAction.GetStatus() == action.GetStatus() &&
previousAction.GetLastTransition().Equal(action.GetLastTransition()) {
if currentAction.GetStatus() == action.GetStatus() &&
currentAction.GetLastTransition().Equal(action.GetLastTransition()) {
continue
}

if err := previousAction.SetStatus(action.GetStatus()); err != nil {
if err := currentAction.SetStatus(action.GetStatus()); err != nil {
return trace.Wrap(err)
}
currentAction.SetLastTransition(action.GetLastTransition())
}

return o.assignmentSvc.UpdateResource(ctx, assignment)
currentAssignment.SetMetadata(assignment.GetMetadata())

return nil
})
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -185,38 +167,29 @@ func (o *OktaService) UpdateOktaAssignment(ctx context.Context, assignment types
// status is a valid transition. If a transition is invalid, it will be logged and the rest of the action statuses
// will be updated if possible.
func (o *OktaService) UpdateOktaAssignmentActionStatuses(ctx context.Context, name, status string) (types.OktaAssignment, error) {
var assignment types.OktaAssignment
err := o.assignmentSvc.RunWhileLocked(ctx, oktaAssignmentModifyLock, oktaAssignmentModifyLockTTL, func(ctx context.Context, b backend.Backend) error {
var err error
assignment, err = o.assignmentSvc.GetResource(ctx, name)
if err != nil {
return trace.Wrap(err)
}

var previousAssignment types.OktaAssignment
err := o.assignmentSvc.UpdateAndSwapResource(ctx, name, func(assignment types.OktaAssignment) error {
previousAssignment = assignment.Copy()
for _, action := range assignment.GetActions() {
Copy link
Copy Markdown
Contributor

@espadolini espadolini Apr 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does GetActions return a mutable slice, or do we need to then do an assignment.SetActions after modifying the actions' statuses?

Same goes for UpdateOktaAssignment.

Copy link
Copy Markdown
Contributor Author

@mdwn mdwn Apr 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does return a mutable slice.

if err := action.SetStatus(status); err != nil {
o.log.Warnf("Unable to transition status from %s -> %s", action.GetStatus(), status)
}
}

return o.assignmentSvc.UpdateResource(ctx, assignment)
return nil
})
return assignment, trace.Wrap(err)
return previousAssignment, trace.Wrap(err)

}

// DeleteOktaAssignment removes the specified Okta assignment resource.
func (o *OktaService) DeleteOktaAssignment(ctx context.Context, name string) error {
return trace.Wrap(o.assignmentSvc.RunWhileLocked(ctx, oktaAssignmentModifyLock, oktaAssignmentModifyLockTTL, func(ctx context.Context, b backend.Backend) error {
return o.assignmentSvc.DeleteResource(ctx, name)
}))
return o.assignmentSvc.DeleteResource(ctx, name)
}

// DeleteAllOktaAssignments removes all Okta assignments.
func (o *OktaService) DeleteAllOktaAssignments(ctx context.Context) error {
return trace.Wrap(o.assignmentSvc.RunWhileLocked(ctx, oktaAssignmentModifyLock, oktaAssignmentModifyLockTTL, func(ctx context.Context, b backend.Backend) error {
return o.assignmentSvc.DeleteAllResources(ctx)
}))
return o.assignmentSvc.DeleteAllResources(ctx)
}

// actionsMatch returns true if two actions match minus the status and last transition.
Expand Down