Fix setting Activity Log enable flag through the API (#10594)

* fix setting enable, update tests

* improve wording

* fix typo - left the testing enabled set in originally

* improve warning handling

* move from nested if to switch - TIL
This commit is contained in:
swayne275
2020-12-18 11:20:32 -07:00
committed by GitHub
parent 37969a0533
commit 077225fe0b
3 changed files with 44 additions and 16 deletions

View File

@@ -694,13 +694,15 @@ func TestActivityLog_API_ConfigCRUD(t *testing.T) {
req.Data["enabled"] = "default"
req.Data["retention_months"] = 24
req.Data["default_report_months"] = 12
originalEnabled := core.activityLog.GetEnabled()
newEnabled := activityLogEnabledDefault
resp, err := b.HandleRequest(namespace.RootContext(nil), req)
if err != nil {
t.Fatalf("err: %v", err)
}
if resp != nil {
t.Fatalf("bad: %#v", resp)
}
checkAPIWarnings(t, originalEnabled, newEnabled, resp)
req = logical.TestRequest(t, logical.ReadOperation, "internal/counters/config")
req.Storage = view
@@ -1640,6 +1642,23 @@ func TestActivityLog_DeleteWorker(t *testing.T) {
expectMissingSegment(t, core, ActivityLogPrefix+"directtokens/1111/1")
}
// checkAPIWarnings ensures there is a warning if switching from enabled -> disabled,
// and no response otherwise
func checkAPIWarnings(t *testing.T, originalEnabled, newEnabled bool, resp *logical.Response) {
t.Helper()
expectWarning := originalEnabled == true && newEnabled == false
switch {
case !expectWarning && resp != nil:
t.Fatalf("got unexpected response: %#v", resp)
case expectWarning && resp == nil:
t.Fatal("expected response (containing warning) when switching from enabled to disabled")
case expectWarning && len(resp.Warnings) == 0:
t.Fatal("expected warning when switching from enabled to disabled")
}
}
func TestActivityLog_EnableDisable(t *testing.T) {
timeutil.SkipAtEndOfMonth(t)
@@ -1650,6 +1669,8 @@ func TestActivityLog_EnableDisable(t *testing.T) {
enableRequest := func() {
t.Helper()
originalEnabled := a.GetEnabled()
req := logical.TestRequest(t, logical.UpdateOperation, "internal/counters/config")
req.Storage = view
req.Data["enabled"] = "enable"
@@ -1657,12 +1678,13 @@ func TestActivityLog_EnableDisable(t *testing.T) {
if err != nil {
t.Fatalf("err: %v", err)
}
if resp != nil {
t.Fatalf("bad: %#v", resp)
}
// don't really need originalEnabled, but might as well be correct
checkAPIWarnings(t, originalEnabled, true, resp)
}
disableRequest := func() {
t.Helper()
originalEnabled := a.GetEnabled()
req := logical.TestRequest(t, logical.UpdateOperation, "internal/counters/config")
req.Storage = view
req.Data["enabled"] = "disable"
@@ -1670,9 +1692,7 @@ func TestActivityLog_EnableDisable(t *testing.T) {
if err != nil {
t.Fatalf("err: %v", err)
}
if resp != nil {
t.Fatalf("bad: %#v", resp)
}
checkAPIWarnings(t, originalEnabled, false, resp)
}
// enable (if not already) and write a segment

View File

@@ -164,3 +164,10 @@ func (a *ActivityLog) SetEnable(enabled bool) {
defer a.fragmentLock.Unlock()
a.enabled = enabled
}
// GetEnabled returns the enabled flag on an activity log
func (a *ActivityLog) GetEnabled() bool {
a.fragmentLock.RLock()
defer a.fragmentLock.RUnlock()
return a.enabled
}

View File

@@ -190,19 +190,20 @@ func (b *SystemBackend) handleActivityConfigUpdate(ctx context.Context, req *log
if enabledRaw, ok := d.GetOk("enabled"); ok {
enabledStr := enabledRaw.(string)
// If currently enabled with the intent to disable or intent to revert to
// default in a OSS context, then we return a warning to the client.
// If we switch from enabled to disabled, then we return a warning to the client.
// We have to keep the default state of activity log enabled in mind
if config.Enabled == "enable" && enabledStr == "disable" ||
!activityLogEnabledDefault && config.Enabled == "enable" && enabledStr == "default" ||
activityLogEnabledDefault && config.Enabled == "default" && enabledStr == "disable" {
warnings = append(warnings, "the current monthly segment will be deleted because the activity log was disabled")
}
}
switch config.Enabled {
case "default", "enable", "disable":
default:
return logical.ErrorResponse("enabled must be one of \"default\", \"enable\", \"disable\""), logical.ErrInvalidRequest
switch enabledStr {
case "default", "enable", "disable":
config.Enabled = enabledStr
default:
return logical.ErrorResponse("enabled must be one of \"default\", \"enable\", \"disable\""), logical.ErrInvalidRequest
}
}
}