Feature Toggle Management: allow editing PublicPreview toggles (#81562)

* Feature Toggle Management: allow editing PublicPreview toggles

* lint

* fix a bunch of tests

* tests are passing

* add permissions unit tests back

* fix display

* close dialog after submit

* use reload method after submit

* make local development easier

* always show editing alert in the UI

* fix readme

---------

Co-authored-by: Michael Mandrus <michael.mandrus@grafana.com>
This commit is contained in:
João Calisto 2024-02-09 17:48:56 +01:00 committed by GitHub
parent f60b5ecec4
commit 42d6e176bc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 587 additions and 22 deletions

View File

@ -98,6 +98,9 @@ type ToggleStatus struct {
// The flag description // The flag description
Description string `json:"description,omitempty"` Description string `json:"description,omitempty"`
// The feature toggle stage
Stage string `json:"stage"`
// Is the flag enabled // Is the flag enabled
Enabled bool `json:"enabled"` Enabled bool `json:"enabled"`

View File

@ -0,0 +1,5 @@
This package supports the [Feature toggle admin page](https://grafana.com/docs/grafana/latest/administration/feature-toggles/) feature.
In order to update feature toggles through the app, the PATCH handler calls a webhook that should update Grafana's configuration and restarts the instance.
For local development, set the app mode to `development` by adding `app_mode = development` to the top level of your Grafana .ini file.

View File

@ -51,6 +51,7 @@ func (b *FeatureFlagAPIBuilder) getResolvedToggleState(ctx context.Context) v0al
toggle := v0alpha1.ToggleStatus{ toggle := v0alpha1.ToggleStatus{
Name: name, Name: name,
Description: f.Description, // simplify the UI changes Description: f.Description, // simplify the UI changes
Stage: f.Stage.String(),
Enabled: state.Enabled[name], Enabled: state.Enabled[name],
Writeable: b.features.IsEditableFromAdminPage(name), Writeable: b.features.IsEditableFromAdminPage(name),
Source: startupRef, Source: startupRef,
@ -76,6 +77,17 @@ func (b *FeatureFlagAPIBuilder) getResolvedToggleState(ctx context.Context) v0al
return state return state
} }
func (b *FeatureFlagAPIBuilder) userCanRead(ctx context.Context, u *user.SignedInUser) bool {
if u == nil {
u, _ = appcontext.User(ctx)
if u == nil {
return false
}
}
ok, err := b.accessControl.Evaluate(ctx, u, ac.EvalPermission(ac.ActionFeatureManagementRead))
return ok && err == nil
}
func (b *FeatureFlagAPIBuilder) userCanWrite(ctx context.Context, u *user.SignedInUser) bool { func (b *FeatureFlagAPIBuilder) userCanWrite(ctx context.Context, u *user.SignedInUser) bool {
if u == nil { if u == nil {
u, _ = appcontext.User(ctx) u, _ = appcontext.User(ctx)
@ -93,7 +105,24 @@ func (b *FeatureFlagAPIBuilder) handleCurrentStatus(w http.ResponseWriter, r *ht
return return
} }
// Check if the user can access toggle info
ctx := r.Context()
user, err := appcontext.User(ctx)
if err != nil {
errhttp.Write(ctx, err, w)
return
}
if !b.userCanRead(ctx, user) {
err = errutil.Unauthorized("featuretoggle.canNotRead",
errutil.WithPublicMessage("missing read permission")).Errorf("user %s does not have read permissions", user.Login)
errhttp.Write(ctx, err, w)
return
}
// Write the state to the response body
state := b.getResolvedToggleState(r.Context()) state := b.getResolvedToggleState(r.Context())
w.WriteHeader(http.StatusOK)
_ = json.NewEncoder(w).Encode(state) _ = json.NewEncoder(w).Encode(state)
} }
@ -101,7 +130,9 @@ func (b *FeatureFlagAPIBuilder) handleCurrentStatus(w http.ResponseWriter, r *ht
func (b *FeatureFlagAPIBuilder) handlePatchCurrent(w http.ResponseWriter, r *http.Request) { func (b *FeatureFlagAPIBuilder) handlePatchCurrent(w http.ResponseWriter, r *http.Request) {
ctx := r.Context() ctx := r.Context()
if !b.features.IsFeatureEditingAllowed() { if !b.features.IsFeatureEditingAllowed() {
errhttp.Write(ctx, fmt.Errorf("feature editing is not enabled"), w) err := errutil.Forbidden("featuretoggle.disabled",
errutil.WithPublicMessage("feature toggles are read-only")).Errorf("feature toggles are not writeable due to missing configuration")
errhttp.Write(ctx, err, w)
return return
} }
@ -113,7 +144,7 @@ func (b *FeatureFlagAPIBuilder) handlePatchCurrent(w http.ResponseWriter, r *htt
if !b.userCanWrite(ctx, user) { if !b.userCanWrite(ctx, user) {
err = errutil.Unauthorized("featuretoggle.canNotWrite", err = errutil.Unauthorized("featuretoggle.canNotWrite",
errutil.WithPublicMessage("missing write permission")) errutil.WithPublicMessage("missing write permission")).Errorf("user %s does not have write permissions", user.Login)
errhttp.Write(ctx, err, w) errhttp.Write(ctx, err, w)
return return
} }
@ -127,7 +158,7 @@ func (b *FeatureFlagAPIBuilder) handlePatchCurrent(w http.ResponseWriter, r *htt
if len(request.Toggles) > 0 { if len(request.Toggles) > 0 {
err = errutil.BadRequest("featuretoggle.badRequest", err = errutil.BadRequest("featuretoggle.badRequest",
errutil.WithPublicMessage("can only path the enabled section")) errutil.WithPublicMessage("can only patch the enabled section")).Errorf("request payload included properties in the read-only Toggles section")
errhttp.Write(ctx, err, w) errhttp.Write(ctx, err, w)
return return
} }
@ -138,7 +169,7 @@ func (b *FeatureFlagAPIBuilder) handlePatchCurrent(w http.ResponseWriter, r *htt
if current != v { if current != v {
if !b.features.IsEditableFromAdminPage(k) { if !b.features.IsEditableFromAdminPage(k) {
err = errutil.BadRequest("featuretoggle.badRequest", err = errutil.BadRequest("featuretoggle.badRequest",
errutil.WithPublicMessage("can not edit toggle: "+k)) errutil.WithPublicMessage("invalid toggle passed in")).Errorf("can not edit toggle %s", k)
errhttp.Write(ctx, err, w) errhttp.Write(ctx, err, w)
w.WriteHeader(http.StatusBadRequest) w.WriteHeader(http.StatusBadRequest)
return return
@ -158,7 +189,8 @@ func (b *FeatureFlagAPIBuilder) handlePatchCurrent(w http.ResponseWriter, r *htt
} }
err = sendWebhookUpdate(b.features.Settings, payload) err = sendWebhookUpdate(b.features.Settings, payload)
if err != nil { if err != nil && b.cfg.Env != setting.Dev {
err = errutil.Internal("featuretoggle.webhookFailure", errutil.WithPublicMessage("an error occurred while updating feeature toggles")).Errorf("webhook error: %w", err)
errhttp.Write(ctx, err, w) errhttp.Write(ctx, err, w)
return return
} }

View File

@ -0,0 +1,460 @@
package featuretoggle
import (
"bytes"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1"
"github.com/grafana/grafana/pkg/infra/appcontext"
"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)
func TestGetFeatureToggles(t *testing.T) {
t.Run("fails without adequate permissions", func(t *testing.T) {
features := featuremgmt.WithFeatureManager(setting.FeatureMgmtSettings{}, []*featuremgmt.FeatureFlag{{
// Add this here to ensure the feature works as expected during tests
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}})
b := NewFeatureFlagAPIBuilder(features, actest.FakeAccessControl{ExpectedEvaluate: false}, &setting.Cfg{})
callGetWith(t, b, http.StatusUnauthorized)
})
t.Run("should be able to get feature toggles", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
disabled := []string{"toggle2"}
b := newTestAPIBuilder(t, features, disabled, setting.FeatureMgmtSettings{})
result := callGetWith(t, b, http.StatusOK)
assert.Len(t, result.Toggles, 2)
t1, _ := findResult(t, result, "toggle1")
assert.True(t, t1.Enabled)
t2, _ := findResult(t, result, "toggle2")
assert.False(t, t2.Enabled)
})
t.Run("toggles hidden by config are not present in the response", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
settings := setting.FeatureMgmtSettings{
HiddenToggles: map[string]struct{}{"toggle1": {}},
}
b := newTestAPIBuilder(t, features, []string{}, settings)
result := callGetWith(t, b, http.StatusOK)
assert.Len(t, result.Toggles, 1)
assert.Equal(t, "toggle2", result.Toggles[0].Name)
})
t.Run("toggles that are read-only by config have the readOnly field set", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
disabled := []string{"toggle2"}
settings := setting.FeatureMgmtSettings{
HiddenToggles: map[string]struct{}{"toggle1": {}},
ReadOnlyToggles: map[string]struct{}{"toggle2": {}},
AllowEditing: true,
UpdateWebhook: "bogus",
}
b := newTestAPIBuilder(t, features, disabled, settings)
result := callGetWith(t, b, http.StatusOK)
assert.Len(t, result.Toggles, 1)
assert.Equal(t, "toggle2", result.Toggles[0].Name)
assert.False(t, result.Toggles[0].Writeable)
})
t.Run("feature toggle defailts", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Stage: featuremgmt.FeatureStageUnknown,
}, {
Name: "toggle2",
Stage: featuremgmt.FeatureStageExperimental,
}, {
Name: "toggle3",
Stage: featuremgmt.FeatureStagePrivatePreview,
}, {
Name: "toggle4",
Stage: featuremgmt.FeatureStagePublicPreview,
AllowSelfServe: true,
}, {
Name: "toggle5",
Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: true,
}, {
Name: "toggle6",
Stage: featuremgmt.FeatureStageDeprecated,
AllowSelfServe: true,
}, {
Name: "toggle7",
Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: false,
},
}
t.Run("unknown, experimental, and private preview toggles are hidden by default", func(t *testing.T) {
b := newTestAPIBuilder(t, features, []string{}, setting.FeatureMgmtSettings{})
result := callGetWith(t, b, http.StatusOK)
assert.Len(t, result.Toggles, 4)
_, ok := findResult(t, result, "toggle1")
assert.False(t, ok)
_, ok = findResult(t, result, "toggle2")
assert.False(t, ok)
_, ok = findResult(t, result, "toggle3")
assert.False(t, ok)
})
t.Run("only public preview and GA with AllowSelfServe are writeable", func(t *testing.T) {
settings := setting.FeatureMgmtSettings{
AllowEditing: true,
UpdateWebhook: "bogus",
}
b := newTestAPIBuilder(t, features, []string{}, settings)
result := callGetWith(t, b, http.StatusOK)
t4, ok := findResult(t, result, "toggle4")
assert.True(t, ok)
assert.True(t, t4.Writeable)
t5, ok := findResult(t, result, "toggle5")
assert.True(t, ok)
assert.True(t, t5.Writeable)
t6, ok := findResult(t, result, "toggle6")
assert.True(t, ok)
assert.True(t, t6.Writeable)
})
t.Run("all toggles are read-only when server is misconfigured", func(t *testing.T) {
settings := setting.FeatureMgmtSettings{
AllowEditing: false,
UpdateWebhook: "",
}
b := newTestAPIBuilder(t, features, []string{}, settings)
result := callGetWith(t, b, http.StatusOK)
assert.Len(t, result.Toggles, 4)
t4, ok := findResult(t, result, "toggle4")
assert.True(t, ok)
assert.False(t, t4.Writeable)
t5, ok := findResult(t, result, "toggle5")
assert.True(t, ok)
assert.False(t, t5.Writeable)
t6, ok := findResult(t, result, "toggle6")
assert.True(t, ok)
assert.False(t, t6.Writeable)
})
})
}
func TestSetFeatureToggles(t *testing.T) {
t.Run("fails when the user doesn't have write permissions", func(t *testing.T) {
s := setting.FeatureMgmtSettings{
AllowEditing: true,
UpdateWebhook: "random",
}
features := featuremgmt.WithFeatureManager(s, []*featuremgmt.FeatureFlag{{
// Add this here to ensure the feature works as expected during tests
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}})
b := NewFeatureFlagAPIBuilder(features, actest.FakeAccessControl{ExpectedEvaluate: false}, &setting.Cfg{})
msg := callPatchWith(t, b, v0alpha1.ResolvedToggleState{}, http.StatusUnauthorized)
assert.Equal(t, "missing write permission", msg)
})
t.Run("fails when update toggle url is not set", func(t *testing.T) {
s := setting.FeatureMgmtSettings{
AllowEditing: true,
}
b := newTestAPIBuilder(t, nil, []string{}, s)
msg := callPatchWith(t, b, v0alpha1.ResolvedToggleState{}, http.StatusForbidden)
assert.Equal(t, "feature toggles are read-only", msg)
})
t.Run("fails with non-existent toggle", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: "toggle1",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
disabled := []string{"toggle2"}
update := v0alpha1.ResolvedToggleState{
Enabled: map[string]bool{
"toggle3": true,
},
}
s := setting.FeatureMgmtSettings{
AllowEditing: true,
UpdateWebhook: "random",
}
b := newTestAPIBuilder(t, features, disabled, s)
msg := callPatchWith(t, b, update, http.StatusBadRequest)
assert.Equal(t, "invalid toggle passed in", msg)
})
t.Run("fails with read-only toggles", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Stage: featuremgmt.FeatureStagePublicPreview,
}, {
Name: "toggle3",
Stage: featuremgmt.FeatureStageGeneralAvailability,
},
}
disabled := []string{"toggle2", "toggle3"}
s := setting.FeatureMgmtSettings{
AllowEditing: true,
UpdateWebhook: "random",
ReadOnlyToggles: map[string]struct{}{
"toggle3": {},
},
}
t.Run("because it is the feature toggle admin page toggle", func(t *testing.T) {
update := v0alpha1.ResolvedToggleState{
Enabled: map[string]bool{
featuremgmt.FlagFeatureToggleAdminPage: true,
},
}
b := newTestAPIBuilder(t, features, disabled, s)
callPatchWith(t, b, update, http.StatusNotModified)
})
t.Run("because it is not GA or Deprecated", func(t *testing.T) {
update := v0alpha1.ResolvedToggleState{
Enabled: map[string]bool{
"toggle2": true,
},
}
b := newTestAPIBuilder(t, features, disabled, s)
msg := callPatchWith(t, b, update, http.StatusBadRequest)
assert.Equal(t, "invalid toggle passed in", msg)
})
t.Run("because it is configured to be read-only", func(t *testing.T) {
update := v0alpha1.ResolvedToggleState{
Enabled: map[string]bool{
"toggle2": true,
},
}
b := newTestAPIBuilder(t, features, disabled, s)
msg := callPatchWith(t, b, update, http.StatusBadRequest)
assert.Equal(t, "invalid toggle passed in", msg)
})
})
t.Run("when all conditions met", func(t *testing.T) {
features := []*featuremgmt.FeatureFlag{
{
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle2",
Stage: featuremgmt.FeatureStagePublicPreview,
}, {
Name: "toggle3",
Stage: featuremgmt.FeatureStageGeneralAvailability,
}, {
Name: "toggle4",
Stage: featuremgmt.FeatureStageGeneralAvailability,
AllowSelfServe: true,
}, {
Name: "toggle5",
Stage: featuremgmt.FeatureStageDeprecated,
AllowSelfServe: true,
},
}
disabled := []string{"toggle2", "toggle3", "toggle4"}
s := setting.FeatureMgmtSettings{
AllowEditing: true,
UpdateWebhook: "random",
UpdateWebhookToken: "token",
ReadOnlyToggles: map[string]struct{}{
"toggle3": {},
},
}
update := v0alpha1.ResolvedToggleState{
Enabled: map[string]bool{
"toggle4": true,
"toggle5": false,
},
}
t.Run("fail when webhook request is not successful", func(t *testing.T) {
webhookServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
}))
defer webhookServer.Close()
s.UpdateWebhook = webhookServer.URL
b := newTestAPIBuilder(t, features, disabled, s)
msg := callPatchWith(t, b, update, http.StatusInternalServerError)
assert.Equal(t, "an error occurred while updating feeature toggles", msg)
})
t.Run("succeed when webhook request is not successful but app is in dev mode", func(t *testing.T) {
webhookServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
}))
defer webhookServer.Close()
s.UpdateWebhook = webhookServer.URL
b := newTestAPIBuilder(t, features, disabled, s)
b.cfg.Env = setting.Dev
callPatchWith(t, b, update, http.StatusOK)
})
t.Run("succeed when webhook request is successful", func(t *testing.T) {
webhookServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "Bearer "+s.UpdateWebhookToken, r.Header.Get("Authorization"))
var req featuremgmt.FeatureToggleWebhookPayload
require.NoError(t, json.NewDecoder(r.Body).Decode(&req))
assert.Equal(t, "true", req.FeatureToggles["toggle4"])
assert.Equal(t, "false", req.FeatureToggles["toggle5"])
w.WriteHeader(http.StatusOK)
}))
defer webhookServer.Close()
s.UpdateWebhook = webhookServer.URL
b := newTestAPIBuilder(t, features, disabled, s)
msg := callPatchWith(t, b, update, http.StatusOK)
assert.Equal(t, "feature toggles updated successfully", msg)
})
})
}
func findResult(t *testing.T, result v0alpha1.ResolvedToggleState, name string) (v0alpha1.ToggleStatus, bool) {
t.Helper()
for _, t := range result.Toggles {
if t.Name == name {
return t, true
}
}
return v0alpha1.ToggleStatus{}, false
}
func callGetWith(t *testing.T, b *FeatureFlagAPIBuilder, expectedCode int) v0alpha1.ResolvedToggleState {
w := response.CreateNormalResponse(http.Header{}, []byte{}, 0)
req := &http.Request{
Method: "GET",
Header: http.Header{},
}
req.Header.Add("content-type", "application/json")
req = req.WithContext(appcontext.WithUser(req.Context(), &user.SignedInUser{}))
b.handleCurrentStatus(w, req)
rts := v0alpha1.ResolvedToggleState{}
require.NoError(t, json.Unmarshal(w.Body(), &rts))
require.Equal(t, expectedCode, w.Status())
// Tests don't expect the feature toggle admin page feature to be present, so remove them from the resolved toggle state
for i, t := range rts.Toggles {
if t.Name == "featureToggleAdminPage" {
rts.Toggles = append(rts.Toggles[0:i], rts.Toggles[i+1:]...)
}
}
return rts
}
func callPatchWith(t *testing.T, b *FeatureFlagAPIBuilder, update v0alpha1.ResolvedToggleState, expectedCode int) string {
w := response.CreateNormalResponse(http.Header{}, []byte{}, 0)
body, err := json.Marshal(update)
require.NoError(t, err)
req := &http.Request{
Method: "PATCH",
Body: io.NopCloser(bytes.NewReader(body)),
Header: http.Header{},
}
req.Header.Add("content-type", "application/json")
req = req.WithContext(appcontext.WithUser(req.Context(), &user.SignedInUser{}))
b.handleCurrentStatus(w, req)
require.NotNil(t, w.Body())
require.Equal(t, expectedCode, w.Status())
// Extract the public facing message if this is an error
if w.Status() > 399 {
res := map[string]any{}
require.NoError(t, json.Unmarshal(w.Body(), &res))
return res["message"].(string)
}
return string(w.Body())
}
func newTestAPIBuilder(
t *testing.T,
serverFeatures []*featuremgmt.FeatureFlag,
disabled []string, // the flags that are disabled
settings setting.FeatureMgmtSettings,
) *FeatureFlagAPIBuilder {
t.Helper()
features := featuremgmt.WithFeatureManager(settings, append([]*featuremgmt.FeatureFlag{{
// Add this here to ensure the feature works as expected during tests
Name: featuremgmt.FlagFeatureToggleAdminPage,
Stage: featuremgmt.FeatureStageGeneralAvailability,
}}, serverFeatures...), disabled...)
return NewFeatureFlagAPIBuilder(features, actest.FakeAccessControl{ExpectedEvaluate: true}, &setting.Cfg{})
}

View File

@ -17,6 +17,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/apiserver/builder" "github.com/grafana/grafana/pkg/services/apiserver/builder"
"github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
) )
var _ builder.APIGroupBuilder = (*FeatureFlagAPIBuilder)(nil) var _ builder.APIGroupBuilder = (*FeatureFlagAPIBuilder)(nil)
@ -27,17 +28,19 @@ var gv = v0alpha1.SchemeGroupVersion
type FeatureFlagAPIBuilder struct { type FeatureFlagAPIBuilder struct {
features *featuremgmt.FeatureManager features *featuremgmt.FeatureManager
accessControl accesscontrol.AccessControl accessControl accesscontrol.AccessControl
cfg *setting.Cfg
} }
func NewFeatureFlagAPIBuilder(features *featuremgmt.FeatureManager, accessControl accesscontrol.AccessControl) *FeatureFlagAPIBuilder { func NewFeatureFlagAPIBuilder(features *featuremgmt.FeatureManager, accessControl accesscontrol.AccessControl, cfg *setting.Cfg) *FeatureFlagAPIBuilder {
return &FeatureFlagAPIBuilder{features, accessControl} return &FeatureFlagAPIBuilder{features, accessControl, cfg}
} }
func RegisterAPIService(features *featuremgmt.FeatureManager, func RegisterAPIService(features *featuremgmt.FeatureManager,
accessControl accesscontrol.AccessControl, accessControl accesscontrol.AccessControl,
apiregistration builder.APIRegistrar, apiregistration builder.APIRegistrar,
cfg *setting.Cfg,
) *FeatureFlagAPIBuilder { ) *FeatureFlagAPIBuilder {
builder := NewFeatureFlagAPIBuilder(features, accessControl) builder := NewFeatureFlagAPIBuilder(features, accessControl, cfg)
apiregistration.RegisterAPI(builder) apiregistration.RegisterAPI(builder)
return builder return builder
} }

View File

@ -79,6 +79,7 @@ func (p *DummyAPIFactory) MakeAPIServer(gv schema.GroupVersion) (builder.APIGrou
return featuretoggle.NewFeatureFlagAPIBuilder( return featuretoggle.NewFeatureFlagAPIBuilder(
featuremgmt.WithFeatureManager(setting.FeatureMgmtSettings{}, nil), // none... for now featuremgmt.WithFeatureManager(setting.FeatureMgmtSettings{}, nil), // none... for now
&actest.FakeAccessControl{ExpectedEvaluate: false}, &actest.FakeAccessControl{ExpectedEvaluate: false},
&setting.Cfg{},
), nil ), nil
case "testdata.datasource.grafana.app": case "testdata.datasource.grafana.app":

View File

@ -151,6 +151,7 @@ func (fm *FeatureManager) IsEditableFromAdminPage(key string) bool {
return false return false
} }
return flag.Stage == FeatureStageGeneralAvailability || return flag.Stage == FeatureStageGeneralAvailability ||
flag.Stage == FeatureStagePublicPreview ||
flag.Stage == FeatureStageDeprecated flag.Stage == FeatureStageDeprecated
} }

View File

@ -119,7 +119,7 @@ type FeatureFlag struct {
Owner codeowner `json:"-"` // Owner person or team that owns this feature flag Owner codeowner `json:"-"` // Owner person or team that owns this feature flag
// Recommended properties - control behavior of the feature toggle management page in the UI // Recommended properties - control behavior of the feature toggle management page in the UI
AllowSelfServe bool `json:"allowSelfServe,omitempty"` // allow users with the right privileges to toggle this from the UI (GeneralAvailability and Deprecated toggles only) AllowSelfServe bool `json:"allowSelfServe,omitempty"` // allow users with the right privileges to toggle this from the UI (GeneralAvailability, PublicPreview, and Deprecated toggles only)
HideFromAdminPage bool `json:"hideFromAdminPage,omitempty"` // GA, Deprecated, and PublicPreview toggles only: don't display this feature in the UI; if this is a GA toggle, add a comment with the reasoning HideFromAdminPage bool `json:"hideFromAdminPage,omitempty"` // GA, Deprecated, and PublicPreview toggles only: don't display this feature in the UI; if this is a GA toggle, add a comment with the reasoning
// CEL-GO expression. Using the value "true" will mean this is on by default // CEL-GO expression. Using the value "true" will mean this is on by default

View File

@ -4,6 +4,7 @@ export type FeatureToggle = {
name: string; name: string;
description?: string; description?: string;
enabled: boolean; enabled: boolean;
stage: string;
readOnly?: boolean; readOnly?: boolean;
hidden?: boolean; hidden?: boolean;
}; };
@ -28,6 +29,7 @@ interface K8sToggleSpec {
enabled: boolean; enabled: boolean;
writeable: boolean; writeable: boolean;
source: K8sToggleSource; source: K8sToggleSource;
stage: string;
} }
interface K8sToggleSource { interface K8sToggleSource {
@ -53,6 +55,7 @@ class K8sAPI implements FeatureTogglesAPI {
description: t.description!, description: t.description!,
enabled: t.enabled, enabled: t.enabled,
readOnly: !Boolean(t.writeable), readOnly: !Boolean(t.writeable),
stage: t.stage,
hidden: false, // only return visible things hidden: false, // only return visible things
})), })),
}; };

View File

@ -10,15 +10,13 @@ import { getTogglesAPI } from './AdminFeatureTogglesAPI';
import { AdminFeatureTogglesTable } from './AdminFeatureTogglesTable'; import { AdminFeatureTogglesTable } from './AdminFeatureTogglesTable';
export default function AdminFeatureTogglesPage() { export default function AdminFeatureTogglesPage() {
const [reload] = useState(1); const [reload, setReload] = useState(1);
const togglesApi = getTogglesAPI(); const togglesApi = getTogglesAPI();
const featureState = useAsync(() => togglesApi.getFeatureToggles(), [reload]); const featureState = useAsync(() => togglesApi.getFeatureToggles(), [reload]);
const [updateSuccessful, setUpdateSuccessful] = useState(false);
const styles = useStyles2(getStyles); const styles = useStyles2(getStyles);
const handleUpdateSuccess = () => { const handleUpdateSuccess = () => {
setUpdateSuccessful(true); setReload(reload + 1);
// setReload(reload+1); << would trigger updating the server state!
}; };
const EditingAlert = () => { const EditingAlert = () => {
@ -28,7 +26,7 @@ export default function AdminFeatureTogglesPage() {
<Icon name="exclamation-triangle" /> <Icon name="exclamation-triangle" />
</div> </div>
<span className={styles.message}> <span className={styles.message}>
{featureState.value?.restartRequired || updateSuccessful {featureState.value?.restartRequired
? 'A restart is pending for your Grafana instance to apply the latest feature toggle changes' ? 'A restart is pending for your Grafana instance to apply the latest feature toggle changes'
: 'Saving feature toggle changes will prompt a restart of the instance, which may take a few minutes'} : 'Saving feature toggle changes will prompt a restart of the instance, which may take a few minutes'}
</span> </span>
@ -57,7 +55,7 @@ export default function AdminFeatureTogglesPage() {
{featureState.error} {featureState.error}
{featureState.loading && 'Fetching feature toggles'} {featureState.loading && 'Fetching feature toggles'}
{featureState.value?.restartRequired && <EditingAlert />} <EditingAlert />
{featureState.value && ( {featureState.value && (
<AdminFeatureTogglesTable <AdminFeatureTogglesTable
featureToggles={featureState.value.toggles} featureToggles={featureState.value.toggles}

View File

@ -1,6 +1,6 @@
import React, { useState, useRef } from 'react'; import React, { useState, useRef } from 'react';
import { Switch, InteractiveTable, Tooltip, type CellProps, Button, type SortByFn } from '@grafana/ui'; import { Switch, InteractiveTable, Tooltip, type CellProps, Button, ConfirmModal, type SortByFn } from '@grafana/ui';
import { FeatureToggle, getTogglesAPI } from './AdminFeatureTogglesAPI'; import { FeatureToggle, getTogglesAPI } from './AdminFeatureTogglesAPI';
@ -35,6 +35,7 @@ export function AdminFeatureTogglesTable({ featureToggles, allowEditing, onUpdat
const serverToggles = useRef<FeatureToggle[]>(featureToggles); const serverToggles = useRef<FeatureToggle[]>(featureToggles);
const [localToggles, setLocalToggles] = useState<FeatureToggle[]>(featureToggles); const [localToggles, setLocalToggles] = useState<FeatureToggle[]>(featureToggles);
const [isSaving, setIsSaving] = useState(false); const [isSaving, setIsSaving] = useState(false);
const [showSaveModel, setShowSaveModal] = useState(false);
const togglesApi = getTogglesAPI(); const togglesApi = getTogglesAPI();
const handleToggleChange = (toggle: FeatureToggle, newValue: boolean) => { const handleToggleChange = (toggle: FeatureToggle, newValue: boolean) => {
@ -58,6 +59,14 @@ export function AdminFeatureTogglesTable({ featureToggles, allowEditing, onUpdat
} }
}; };
const saveButtonRef = useRef<HTMLButtonElement | null>(null);
const showSaveChangesModal = (show: boolean) => () => {
setShowSaveModal(show);
if (!show && saveButtonRef.current) {
saveButtonRef.current.focus();
}
};
const getModifiedToggles = (): FeatureToggle[] => { const getModifiedToggles = (): FeatureToggle[] => {
return localToggles.filter((toggle, index) => toggle.enabled !== serverToggles.current[index].enabled); return localToggles.filter((toggle, index) => toggle.enabled !== serverToggles.current[index].enabled);
}; };
@ -72,11 +81,30 @@ export function AdminFeatureTogglesTable({ featureToggles, allowEditing, onUpdat
return 'Feature management is not configured for editing'; return 'Feature management is not configured for editing';
} }
if (readOnlyToggle) { if (readOnlyToggle) {
return 'Preview features are not editable'; return 'This is a non-editable feature';
} }
return ''; return '';
}; };
const getStageCell = (stage: string) => {
switch (stage) {
case 'GA':
return (
<Tooltip content={'General availability'}>
<div>GA</div>
</Tooltip>
);
case 'privatePreview':
case 'preview':
case 'experimental':
return 'Beta';
case 'deprecated':
return 'Deprecated';
default:
return stage;
}
};
const columns = [ const columns = [
{ {
id: 'name', id: 'name',
@ -90,20 +118,32 @@ export function AdminFeatureTogglesTable({ featureToggles, allowEditing, onUpdat
cell: ({ cell: { value } }: CellProps<FeatureToggle, string>) => <div>{value}</div>, cell: ({ cell: { value } }: CellProps<FeatureToggle, string>) => <div>{value}</div>,
sortType: sortByDescription, sortType: sortByDescription,
}, },
{
id: 'stage',
header: 'Stage',
cell: ({ cell: { value } }: CellProps<FeatureToggle, string>) => <div>{getStageCell(value)}</div>,
},
{ {
id: 'enabled', id: 'enabled',
header: 'State', header: 'State',
cell: ({ row }: CellProps<FeatureToggle, boolean>) => ( cell: ({ row }: CellProps<FeatureToggle, boolean>) => {
<Tooltip content={getToggleTooltipContent(row.original.readOnly)}> const renderStateSwitch = (
<div> <div>
<Switch <Switch
value={row.original.enabled} value={row.original.enabled}
disabled={row.original.readOnly} disabled={row.original.readOnly}
onChange={(e) => handleToggleChange(row.original, e.currentTarget.checked)} onChange={(e) => handleToggleChange(row.original, e.currentTarget.checked)}
transparent={row.original.readOnly}
/> />
</div> </div>
</Tooltip> );
),
return row.original.readOnly ? (
<Tooltip content={getToggleTooltipContent(row.original.readOnly)}>{renderStateSwitch}</Tooltip>
) : (
renderStateSwitch
);
},
sortType: sortByEnabled, sortType: sortByEnabled,
}, },
]; ];
@ -112,9 +152,28 @@ export function AdminFeatureTogglesTable({ featureToggles, allowEditing, onUpdat
<> <>
{allowEditing && ( {allowEditing && (
<div style={{ display: 'flex', justifyContent: 'flex-end', padding: '0 0 5px 0' }}> <div style={{ display: 'flex', justifyContent: 'flex-end', padding: '0 0 5px 0' }}>
<Button disabled={!hasModifications() || isSaving} onClick={handleSaveChanges}> <Button disabled={!hasModifications() || isSaving} onClick={showSaveChangesModal(true)} ref={saveButtonRef}>
{isSaving ? 'Saving...' : 'Save Changes'} {isSaving ? 'Saving...' : 'Save Changes'}
</Button> </Button>
<ConfirmModal
isOpen={showSaveModel}
title="Apply feature toggle changes"
body={
<div>
<p>
Some features are stable (GA) and enabled by default, whereas some are currently in their preliminary
Beta phase, available for early adoption.
</p>
<p>We advise understanding the implications of each feature change before making modifications.</p>
</div>
}
confirmText="Save changes"
onConfirm={async () => {
showSaveChangesModal(false)();
handleSaveChanges();
}}
onDismiss={showSaveChangesModal(false)}
/>
</div> </div>
)} )}
<InteractiveTable columns={columns} data={localToggles} getRowId={(featureToggle) => featureToggle.name} /> <InteractiveTable columns={columns} data={localToggles} getRowId={(featureToggle) => featureToggle.name} />