-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Port activation flags with dynamic registration #29237
Changes from 7 commits
bbc63cb
b5d697f
114c46b
7c6b409
43999de
59dc51a
6d761e8
5539863
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:feature | ||
**Activation Flag (core)**: A simple, one-time flag that lets users turn on specific Vault functionalities when they need them. | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
// Copyright (c) HashiCorp, Inc. | ||
// SPDX-License-Identifier: BUSL-1.1 | ||
|
||
package activationflags | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"maps" | ||
"sync" | ||
|
||
"github.com/hashicorp/vault/sdk/logical" | ||
) | ||
|
||
const ( | ||
storagePathActivationFlags = "activation-flags" | ||
) | ||
|
||
type FeatureActivationFlags struct { | ||
activationFlagsLock sync.RWMutex | ||
storage logical.Storage | ||
activationFlags map[string]bool | ||
} | ||
|
||
func NewFeatureActivationFlags() *FeatureActivationFlags { | ||
return &FeatureActivationFlags{ | ||
activationFlags: map[string]bool{}, | ||
} | ||
} | ||
|
||
func (f *FeatureActivationFlags) Initialize(ctx context.Context, storage logical.Storage) error { | ||
f.activationFlagsLock.Lock() | ||
defer f.activationFlagsLock.Unlock() | ||
|
||
if storage == nil { | ||
return fmt.Errorf("unable to access storage") | ||
} | ||
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 I wonder if we need to be so defensive... Wouldn't this mean that the systemBarrier is nil? Presumably we would have already bailed long before this point if so. Same argument for the rest of the |
||
|
||
f.storage = storage | ||
|
||
entry, err := f.storage.Get(ctx, storagePathActivationFlags) | ||
if err != nil { | ||
return fmt.Errorf("failed to get activation flags from storage: %w", err) | ||
} | ||
if entry == nil { | ||
f.activationFlags = map[string]bool{} | ||
return nil | ||
} | ||
|
||
var activationFlags map[string]bool | ||
if err := entry.DecodeJSON(&activationFlags); err != nil { | ||
return fmt.Errorf("failed to decode activation flags from storage: %w", err) | ||
} | ||
|
||
f.activationFlags = activationFlags | ||
|
||
return nil | ||
} | ||
|
||
// Get is the helper function called by the activation-flags API read endpoint. This reads the | ||
// actual values from storage, then updates the in-memory cache of the activation-flags. It | ||
// returns a slice of the feature names which have already been activated. | ||
func (f *FeatureActivationFlags) Get(ctx context.Context) ([]string, error) { | ||
f.activationFlagsLock.Lock() | ||
defer f.activationFlagsLock.Unlock() | ||
|
||
// Don't use nil slice declaration, we want the JSON to show "[]" instead of null | ||
activated := []string{} | ||
|
||
if f.storage == nil { | ||
return activated, nil | ||
} | ||
|
||
entry, err := f.storage.Get(ctx, storagePathActivationFlags) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to get activation flags from storage: %w", err) | ||
} | ||
if entry == nil { | ||
return activated, nil | ||
} | ||
|
||
var activationFlags map[string]bool | ||
if err := entry.DecodeJSON(&activationFlags); err != nil { | ||
return nil, fmt.Errorf("failed to decode activation flags from storage: %w", err) | ||
} | ||
|
||
// Update the in-memory flags after loading the latest values from storage | ||
f.activationFlags = activationFlags | ||
|
||
for flag, set := range activationFlags { | ||
if set { | ||
activated = append(activated, flag) | ||
} | ||
} | ||
|
||
return activated, nil | ||
} | ||
|
||
// Write is the helper function called by the activation-flags API write endpoint. This stores | ||
// the boolean value for the activation-flag feature name into Vault storage across the cluster | ||
// and updates the in-memory cache upon success. | ||
func (f *FeatureActivationFlags) Write(ctx context.Context, featureName string, activate bool) (err error) { | ||
f.activationFlagsLock.Lock() | ||
defer f.activationFlagsLock.Unlock() | ||
|
||
if f.storage == nil { | ||
return fmt.Errorf("unable to access storage") | ||
} | ||
|
||
activationFlags := f.activationFlags | ||
|
||
clonedFlags := maps.Clone(f.activationFlags) | ||
clonedFlags[featureName] = activate | ||
// The cloned flags are updated but the in-memory state is only updated on success of the storage update. | ||
defer func() { | ||
if err == nil { | ||
activationFlags[featureName] = activate | ||
} | ||
}() | ||
|
||
entry, err := logical.StorageEntryJSON(storagePathActivationFlags, clonedFlags) | ||
if err != nil { | ||
return fmt.Errorf("failed to marshal object to JSON: %w", err) | ||
} | ||
|
||
err = f.storage.Put(ctx, entry) | ||
if err != nil { | ||
return fmt.Errorf("failed to save object in storage: %w", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// IsActivationFlagEnabled is true if the specified flag is enabled in the core. | ||
func (f *FeatureActivationFlags) IsActivationFlagEnabled(featureName string) bool { | ||
f.activationFlagsLock.RLock() | ||
defer f.activationFlagsLock.RUnlock() | ||
|
||
activated, ok := f.activationFlags[featureName] | ||
|
||
return ok && activated | ||
} | ||
banks marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,140 @@ | ||
// Copyright (c) HashiCorp, Inc. | ||
// SPDX-License-Identifier: BUSL-1.1 | ||
|
||
package vault | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"slices" | ||
"strings" | ||
|
||
"github.com/hashicorp/vault/sdk/framework" | ||
"github.com/hashicorp/vault/sdk/logical" | ||
) | ||
|
||
const ( | ||
paramFeatureName = "feature_name" | ||
descFeatureName = "The name of the feature to be activated." | ||
summaryList = "Returns the available and activated activation-flagged features." | ||
summaryUpdate = "Activate a flagged feature." | ||
|
||
prefixActivationFlags = "activation-flags" | ||
verbActivationFlagsActivate = "activate" | ||
verbActivationFlagsDeactivate = "deactivate" | ||
|
||
fieldActivated = "activated" | ||
fieldUnactivated = "unactivated" | ||
|
||
helpSynopsis = "Returns information about Vault's features that require a one-time activation step." | ||
helpDescription = ` | ||
This path responds to the following HTTP methods. | ||
GET / | ||
Returns the available and activated activation-flags. | ||
|
||
PUT|POST /<feature-name>/activate | ||
Activates the specified feature. Cannot be undone.` | ||
) | ||
|
||
// Register CRUD functions dynamically. | ||
banks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// These variables should only be mutated during initialization or server construction. | ||
// It is unsafe to modify them once the Vault core is running. | ||
var ( | ||
readActivationFlag = func(ctx context.Context, b *SystemBackend, req *logical.Request, fd *framework.FieldData) (*logical.Response, error) { | ||
return b.readActivationFlag(ctx, req, fd) | ||
} | ||
|
||
writeActivationFlag = func(ctx context.Context, b *SystemBackend, req *logical.Request, fd *framework.FieldData, isActivate bool) (*logical.Response, error) { | ||
return b.writeActivationFlagWrite(ctx, req, fd, isActivate) | ||
} | ||
) | ||
|
||
func (b *SystemBackend) activationFlagsPaths() []*framework.Path { | ||
return []*framework.Path{ | ||
{ | ||
Pattern: fmt.Sprintf("%s$", prefixActivationFlags), | ||
DisplayAttrs: &framework.DisplayAttributes{ | ||
OperationVerb: "read", | ||
OperationSuffix: prefixActivationFlags, | ||
}, | ||
Operations: map[logical.Operation]framework.OperationHandler{ | ||
logical.ReadOperation: &framework.PathOperation{ | ||
Callback: b.handleActivationFlagRead, | ||
Summary: summaryList, | ||
}, | ||
}, | ||
HelpSynopsis: helpSynopsis, | ||
HelpDescription: helpDescription, | ||
}, | ||
{ | ||
Pattern: fmt.Sprintf("%s/%s/%s", prefixActivationFlags, "activation-test", verbActivationFlagsActivate), | ||
DisplayAttrs: &framework.DisplayAttributes{ | ||
OperationPrefix: prefixActivationFlags, | ||
OperationVerb: verbActivationFlagsActivate, | ||
}, | ||
Operations: map[logical.Operation]framework.OperationHandler{ | ||
logical.UpdateOperation: &framework.PathOperation{ | ||
Callback: b.handleActivationFlagsActivate, | ||
ForwardPerformanceSecondary: true, | ||
ForwardPerformanceStandby: true, | ||
Summary: summaryUpdate, | ||
}, | ||
}, | ||
HelpSynopsis: helpSynopsis, | ||
HelpDescription: helpDescription, | ||
}, | ||
} | ||
} | ||
|
||
func (b *SystemBackend) handleActivationFlagRead(ctx context.Context, req *logical.Request, fd *framework.FieldData) (*logical.Response, error) { | ||
return readActivationFlag(ctx, b, req, fd) | ||
} | ||
|
||
func (b *SystemBackend) handleActivationFlagsActivate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { | ||
return writeActivationFlag(ctx, b, req, data, true) | ||
} | ||
|
||
func (b *SystemBackend) readActivationFlag(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can probably just drop the FieldData arg here, right (same with the other handler internals)? |
||
activationFlags, err := b.Core.FeatureActivationFlags.Get(ctx) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return b.activationFlagsToResponse(activationFlags), nil | ||
} | ||
|
||
func (b *SystemBackend) writeActivationFlagWrite(ctx context.Context, req *logical.Request, _ *framework.FieldData, isActivate bool) (*logical.Response, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a big deal, but we've got two "writes" in this method name. Maybe |
||
// We need to manually parse out the feature_name from the path because we can't use FieldSchema parameters | ||
// in the path to make generic endpoints. We need each activation-flag path to be a separate endpoint. | ||
// Path starts out as activation-flags/<feature_name>/verb | ||
// Removes activation-flags/ from the path | ||
trimPrefix := strings.TrimPrefix(req.Path, prefixActivationFlags+"/") | ||
// Removes /verb from the path | ||
featureName := trimPrefix[:strings.LastIndex(trimPrefix, "/")] | ||
|
||
err := b.Core.FeatureActivationFlags.Write(ctx, featureName, isActivate) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to write new activation flags: %w", err) | ||
} | ||
|
||
// We read back the value after writing it to storage so that we can try forcing a cache update right away. | ||
// If this fails, it's still okay to proceed as the write has been successful and the cache will get updated | ||
// at the time of an endpoint getting called. However, we can only return the one feature name we just activated | ||
// in the response since the read to retrieve any others did not succeed. | ||
banks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
activationFlags, err := b.Core.FeatureActivationFlags.Get(ctx) | ||
if err != nil { | ||
resp := b.activationFlagsToResponse([]string{featureName}) | ||
return resp, fmt.Errorf("failed to read activation-flags back after write: %w", err) | ||
} | ||
|
||
return b.activationFlagsToResponse(activationFlags), nil | ||
} | ||
|
||
func (b *SystemBackend) activationFlagsToResponse(activationFlags []string) *logical.Response { | ||
slices.Sort(activationFlags) | ||
return &logical.Response{ | ||
Data: map[string]interface{}{ | ||
fieldActivated: activationFlags, | ||
}, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 this already existed in previous versions right (just Enterprise only)? Was there no changelog entry at the time? It might be confusing for folks in the future if they see this in 1.19 but then see that older versions documented the API and had instructions for using with secret sync? We do have public docs for the endpoint already for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no Changelog for that... I can mention this was an enterprise only feature and now it's being ported to CE. Would that sound better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'd also maybe not put it in as a "feature" as it's more of a mechanism for enabling actual features. (it's not use to users unless we wire it to something they care about). Maybe
enhancement
is better?Maybe we could hint at the history without details like:
Or something?