From 67a952c34e8fa68d5bf0fab0d143a05448d968e4 Mon Sep 17 00:00:00 2001 From: Tania <10127682+undef1nd@users.noreply.github.com> Date: Fri, 27 Jun 2025 18:30:38 +0200 Subject: [PATCH] Implement OFREP compatible feature flag service (#105632) * Add ofrep pkg * api server: Use namespace from request in case user is not authenticated * Add handlers to ofrep api builder * Add NewOpenFeatureService to initialize mt apiserver * allow specifying CA and insecure * Compare namespace with eval ctx stackID * Organize ofrep package * Implement AllowedV0Alpha1Resources * Revert folderimpl changes * Handle default namespace * Fix extracting stack id from eval ctx * Add more logs * Update pkg/registry/apis/ofrep/register.go Co-authored-by: Dave Henderson * Update pkg/registry/apis/ofrep/register.go Co-authored-by: Dave Henderson * Apply review feedback * Replace contexthandler with types * Fix identifying authed request * Refactor checks in the handlers * Remove anonymous from isAuthenticatedRequest check --------- Co-authored-by: Todd Treece <360020+toddtreece@users.noreply.github.com> Co-authored-by: Gabriel Mabille Co-authored-by: Charandas Batra Co-authored-by: Dave Henderson --- pkg/registry/apis/apis.go | 2 + pkg/registry/apis/ofrep/proxy.go | 109 ++++++++ pkg/registry/apis/ofrep/publicflags.go | 11 + pkg/registry/apis/ofrep/register.go | 234 ++++++++++++++++++ pkg/registry/apis/ofrep/static.go | 41 +++ pkg/registry/apis/wireset.go | 2 + pkg/server/wire.go | 1 + pkg/services/apiserver/service.go | 22 ++ pkg/services/apiserver/service_test.go | 51 ++++ pkg/services/featuremgmt/openfeature.go | 102 +++----- pkg/services/featuremgmt/static_evaluator.go | 119 +++++++++ pkg/services/featuremgmt/static_provider.go | 10 +- .../featuremgmt/static_provider_test.go | 20 +- 13 files changed, 630 insertions(+), 94 deletions(-) create mode 100644 pkg/registry/apis/ofrep/proxy.go create mode 100644 pkg/registry/apis/ofrep/publicflags.go create mode 100644 pkg/registry/apis/ofrep/register.go create mode 100644 pkg/registry/apis/ofrep/static.go create mode 100644 pkg/services/apiserver/service_test.go create mode 100644 pkg/services/featuremgmt/static_evaluator.go diff --git a/pkg/registry/apis/apis.go b/pkg/registry/apis/apis.go index 9a255d4c1b8..e100c3c1af7 100644 --- a/pkg/registry/apis/apis.go +++ b/pkg/registry/apis/apis.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/featuretoggle" "github.com/grafana/grafana/pkg/registry/apis/folders" "github.com/grafana/grafana/pkg/registry/apis/iam" + "github.com/grafana/grafana/pkg/registry/apis/ofrep" "github.com/grafana/grafana/pkg/registry/apis/provisioning" "github.com/grafana/grafana/pkg/registry/apis/query" "github.com/grafana/grafana/pkg/registry/apis/secret" @@ -28,6 +29,7 @@ func ProvideRegistryServiceSink( _ *userstorage.UserStorageAPIBuilder, _ *secret.SecretAPIBuilder, _ *provisioning.APIBuilder, + _ *ofrep.APIBuilder, ) *Service { return &Service{} } diff --git a/pkg/registry/apis/ofrep/proxy.go b/pkg/registry/apis/ofrep/proxy.go new file mode 100644 index 00000000000..c75524272da --- /dev/null +++ b/pkg/registry/apis/ofrep/proxy.go @@ -0,0 +1,109 @@ +package ofrep + +import ( + "crypto/tls" + "crypto/x509" + "encoding/json" + "fmt" + "net/http" + "net/http/httputil" + "os" + "path" + + "github.com/grafana/grafana/pkg/util/proxyutil" +) + +func (b *APIBuilder) proxyAllFlagReq(isAuthedUser bool, w http.ResponseWriter, r *http.Request) { + proxy, err := b.newProxy(ofrepPath) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + proxy.ModifyResponse = func(resp *http.Response) error { + if resp.StatusCode == http.StatusOK && !isAuthedUser { + var result map[string]interface{} + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return err + } + _ = resp.Body.Close() + + filtered := make(map[string]any) + for k, v := range result { + if isPublicFlag(k) { + filtered[k] = v + } + } + + writeResponse(http.StatusOK, filtered, b.logger, w) + } + + return nil + } + + proxy.ServeHTTP(w, r) +} + +func (b *APIBuilder) proxyFlagReq(flagKey string, isAuthedUser bool, w http.ResponseWriter, r *http.Request) { + proxy, err := b.newProxy(path.Join(ofrepPath, flagKey)) + if err != nil { + b.logger.Error("Failed to create proxy", "key", flagKey, "error", err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + proxy.ModifyResponse = func(resp *http.Response) error { + if resp.StatusCode == http.StatusOK && !isAuthedUser && !isPublicFlag(flagKey) { + writeResponse(http.StatusUnauthorized, struct{}{}, b.logger, w) + } + return nil + } + + proxy.ServeHTTP(w, r) +} + +func (b *APIBuilder) newProxy(proxyPath string) (*httputil.ReverseProxy, error) { + if proxyPath == "" { + return nil, fmt.Errorf("proxy path is required") + } + + if b.url == nil { + return nil, fmt.Errorf("OpenFeatureService provider URL is not set") + } + + var caRoot *x509.CertPool + if b.caFile != "" { + var err error + caRoot, err = getCARoot(b.caFile) + if err != nil { + return nil, err + } + } + + director := func(req *http.Request) { + req.URL.Scheme = b.url.Scheme + req.URL.Host = b.url.Host + req.URL.Path = proxyPath + } + + proxy := proxyutil.NewReverseProxy(b.logger, director) + proxy.Transport = &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: b.insecure, + RootCAs: caRoot, + }, + } + return proxy, nil +} + +func getCARoot(caFile string) (*x509.CertPool, error) { + // It should be safe to ignore since caFile is passed as --internal.root-ca-file flag of apiserver + // nolint:gosec + caCert, err := os.ReadFile(caFile) + if err != nil { + return nil, err + } + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(caCert) + return caCertPool, nil +} diff --git a/pkg/registry/apis/ofrep/publicflags.go b/pkg/registry/apis/ofrep/publicflags.go new file mode 100644 index 00000000000..b129a0b4660 --- /dev/null +++ b/pkg/registry/apis/ofrep/publicflags.go @@ -0,0 +1,11 @@ +package ofrep + +// publicFlags contains the list of flags that can be evaluated by unauthenticated users +var publicFlags = map[string]bool{ + "testflag": true, +} + +func isPublicFlag(flagKey string) bool { + _, exists := publicFlags[flagKey] + return exists +} diff --git a/pkg/registry/apis/ofrep/register.go b/pkg/registry/apis/ofrep/register.go new file mode 100644 index 00000000000..344743780e3 --- /dev/null +++ b/pkg/registry/apis/ofrep/register.go @@ -0,0 +1,234 @@ +package ofrep + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/url" + "strconv" + "strings" + + "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/setting" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/authorization/authorizer" + genericapiserver "k8s.io/apiserver/pkg/server" + "k8s.io/kube-openapi/pkg/common" + "k8s.io/kube-openapi/pkg/spec3" + + "github.com/gorilla/mux" + "github.com/grafana/grafana/pkg/services/apiserver/builder" +) + +var _ builder.APIGroupBuilder = (*APIBuilder)(nil) +var _ builder.APIGroupRouteProvider = (*APIBuilder)(nil) +var _ builder.APIGroupVersionProvider = (*APIBuilder)(nil) + +const ofrepPath = "/ofrep/v1/evaluate/flags" + +type APIBuilder struct { + providerType string + url *url.URL + insecure bool + caFile string + staticEvaluator featuremgmt.StaticFlagEvaluator + logger log.Logger +} + +func NewAPIBuilder(providerType string, url *url.URL, insecure bool, caFile string, staticEvaluator featuremgmt.StaticFlagEvaluator) *APIBuilder { + return &APIBuilder{ + providerType: providerType, + url: url, + insecure: insecure, + caFile: caFile, + staticEvaluator: staticEvaluator, + logger: log.New("grafana-apiserver.feature-flags"), + } +} + +func RegisterAPIService(apiregistration builder.APIRegistrar, cfg *setting.Cfg, staticEvaluator featuremgmt.StaticFlagEvaluator) *APIBuilder { + b := NewAPIBuilder(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, true, "", staticEvaluator) + apiregistration.RegisterAPI(b) + return b +} + +func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { + return authorizer.AuthorizerFunc(func(ctx context.Context, attr authorizer.Attributes) (authorizer.Decision, string, error) { + // Allow all requests - we'll handle auth in the handler + return authorizer.DecisionAllow, "", nil + }) +} + +func (b *APIBuilder) GetGroupVersion() schema.GroupVersion { + return schema.GroupVersion{ + Group: "features.grafana.app", + Version: "v0alpha1", + } +} + +func (b *APIBuilder) InstallSchema(scheme *runtime.Scheme) error { + metav1.AddToGroupVersion(scheme, b.GetGroupVersion()) + return scheme.SetVersionPriority(b.GetGroupVersion()) +} + +func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupInfo, opts builder.APIGroupOptions) error { + return nil +} + +func (b *APIBuilder) GetOpenAPIDefinitions() common.GetOpenAPIDefinitions { + return func(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { + return map[string]common.OpenAPIDefinition{} + } +} + +func (b *APIBuilder) AllowedV0Alpha1Resources() []string { + return []string{builder.AllResourcesAllowed} +} + +func (b *APIBuilder) GetAPIRoutes(gv schema.GroupVersion) *builder.APIRoutes { + return &builder.APIRoutes{ + Namespace: []builder.APIRouteHandler{ + { + Path: "ofrep/v1/evaluate/flags/", + Spec: &spec3.PathProps{ + Post: &spec3.Operation{}, + }, + Handler: b.allFlagsHandler, + }, + { + Path: "ofrep/v1/evaluate/flags/{flagKey}", + Spec: &spec3.PathProps{ + Post: &spec3.Operation{}, + }, + Handler: b.oneFlagHandler, + }, + }, + } +} + +func (b *APIBuilder) oneFlagHandler(w http.ResponseWriter, r *http.Request) { + if !b.validateNamespace(r) { + b.logger.Error("stackId in evaluation context does not match requested namespace") + http.Error(w, "stackId in evaluation context does not match requested namespace", http.StatusUnauthorized) + return + } + + flagKey := mux.Vars(r)["flagKey"] + if flagKey == "" { + http.Error(w, "flagKey parameter is required", http.StatusBadRequest) + return + } + + isAuthedReq := b.isAuthenticatedRequest(r) + + // Unless the request is authenticated, we only allow public flags evaluations + if !isAuthedReq && !isPublicFlag(flagKey) { + b.logger.Error("Unauthorized to evaluate flag", "flagKey", flagKey) + http.Error(w, "unauthorized to evaluate flag", http.StatusUnauthorized) + return + } + + if b.providerType == setting.GOFFProviderType { + b.proxyFlagReq(flagKey, isAuthedReq, w, r) + return + } + + b.evalFlagStatic(flagKey, w, r) +} + +func (b *APIBuilder) allFlagsHandler(w http.ResponseWriter, r *http.Request) { + if !b.validateNamespace(r) { + b.logger.Error("stackId in evaluation context does not match requested namespace") + http.Error(w, "stackId in evaluation context does not match requested namespace", http.StatusUnauthorized) + return + } + + isAuthedReq := b.isAuthenticatedRequest(r) + + if b.providerType == setting.GOFFProviderType { + b.proxyAllFlagReq(isAuthedReq, w, r) + return + } + + b.evalAllFlagsStatic(isAuthedReq, w, r) +} + +func writeResponse(statusCode int, result any, logger log.Logger, w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(statusCode) + + if err := json.NewEncoder(w).Encode(result); err != nil { + logger.Error("Failed to encode flag evaluation result", "error", err) + } +} + +func (b *APIBuilder) stackIdFromEvalCtx(body []byte) string { + // Extract stackID from request body without consuming it + var evalCtx struct { + Context struct { + StackID int32 `json:"stackId"` + } `json:"context"` + } + + if err := json.Unmarshal(body, &evalCtx); err != nil { + b.logger.Debug("Failed to unmarshal evaluation context", "error", err, "body", string(body)) + return "" + } + + if evalCtx.Context.StackID <= 0 { + b.logger.Debug("Invalid or missing stackId in evaluation context", "stackId", evalCtx.Context.StackID) + return "" + } + + return strconv.Itoa(int(evalCtx.Context.StackID)) +} + +func removeStackPrefix(tenant string) string { + return strings.TrimPrefix(tenant, "stacks-") +} + +// isAuthenticatedRequest returns true if the request is authenticated +func (b *APIBuilder) isAuthenticatedRequest(r *http.Request) bool { + user, ok := types.AuthInfoFrom(r.Context()) + if !ok { + return false + } + return user.GetIdentityType() != "" +} + +// validateNamespace checks if the stackId in the evaluation context matches the namespace in the request +func (b *APIBuilder) validateNamespace(r *http.Request) bool { + // Extract namespace from request context or URL path + var namespace string + user, ok := types.AuthInfoFrom(r.Context()) + if !ok { + return false + } + + if user.GetNamespace() != "" { + namespace = user.GetNamespace() + } else { + namespace = mux.Vars(r)["namespace"] + } + + // Extract stackId from feature flag evaluation context + body, err := io.ReadAll(r.Body) + if err != nil { + b.logger.Error("Error reading evaluation request body", "error", err) + return false + } + r.Body = io.NopCloser(bytes.NewBuffer(body)) + + // "default" namespace case can only occur in on-prem grafana + if b.stackIdFromEvalCtx(body) == removeStackPrefix(namespace) || namespace == "default" { + return true + } + + return false +} diff --git a/pkg/registry/apis/ofrep/static.go b/pkg/registry/apis/ofrep/static.go new file mode 100644 index 00000000000..538e8d7e3da --- /dev/null +++ b/pkg/registry/apis/ofrep/static.go @@ -0,0 +1,41 @@ +package ofrep + +import ( + "net/http" + + "github.com/grafana/grafana/pkg/services/featuremgmt" +) + +func (b *APIBuilder) evalAllFlagsStatic(isAuthedUser bool, w http.ResponseWriter, r *http.Request) { + result, err := b.staticEvaluator.EvalAllFlags(r.Context()) + if err != nil { + b.logger.Error("Failed to evaluate all static flags", "error", err) + http.Error(w, "failed to evaluate flags", http.StatusInternalServerError) + return + } + + if !isAuthedUser { + var publicOnly []featuremgmt.OFREPFlag + + for _, flag := range result.Flags { + if isPublicFlag(flag.Key) { + publicOnly = append(publicOnly, flag) + } + } + + result.Flags = publicOnly + } + + writeResponse(http.StatusOK, result, b.logger, w) +} + +func (b *APIBuilder) evalFlagStatic(flagKey string, w http.ResponseWriter, r *http.Request) { + result, err := b.staticEvaluator.EvalFlag(r.Context(), flagKey) + if err != nil { + b.logger.Error("Failed to evaluate static flag", "key", flagKey, "error", err) + http.Error(w, "failed to evaluate flag", http.StatusInternalServerError) + return + } + + writeResponse(http.StatusOK, result, b.logger, w) +} diff --git a/pkg/registry/apis/wireset.go b/pkg/registry/apis/wireset.go index e79ccf45811..7f8c1497111 100644 --- a/pkg/registry/apis/wireset.go +++ b/pkg/registry/apis/wireset.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/folders" "github.com/grafana/grafana/pkg/registry/apis/iam" "github.com/grafana/grafana/pkg/registry/apis/iam/noopstorage" + "github.com/grafana/grafana/pkg/registry/apis/ofrep" "github.com/grafana/grafana/pkg/registry/apis/provisioning" "github.com/grafana/grafana/pkg/registry/apis/provisioning/webhooks" "github.com/grafana/grafana/pkg/registry/apis/query" @@ -59,4 +60,5 @@ var WireSet = wire.NewSet( query.RegisterAPIService, secret.RegisterAPIService, userstorage.RegisterAPIService, + ofrep.RegisterAPIService, ) diff --git a/pkg/server/wire.go b/pkg/server/wire.go index a3b2a2ed4e2..57c5c475717 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -324,6 +324,7 @@ var wireBasicSet = wire.NewSet( featuremgmt.ProvideManagerService, featuremgmt.ProvideToggles, featuremgmt.ProvideOpenFeatureService, + featuremgmt.ProvideStaticEvaluator, dashboardservice.ProvideDashboardServiceImpl, wire.Bind(new(dashboards.PermissionsRegistrationService), new(*dashboardservice.DashboardServiceImpl)), dashboardservice.ProvideDashboardService, diff --git a/pkg/services/apiserver/service.go b/pkg/services/apiserver/service.go index 28ea5f38a6b..52c8ad568b7 100644 --- a/pkg/services/apiserver/service.go +++ b/pkg/services/apiserver/service.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "path" + "strings" "github.com/prometheus/client_golang/prometheus" "k8s.io/apimachinery/pkg/runtime" @@ -17,6 +18,7 @@ import ( clientrest "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + "github.com/grafana/authlib/types" "github.com/grafana/dskit/services" "github.com/grafana/grafana-plugin-sdk-go/backend" dataplaneaggregator "github.com/grafana/grafana/pkg/aggregator/apiserver" @@ -44,6 +46,7 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" + "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/legacysql/dualwrite" "github.com/grafana/grafana/pkg/storage/unified/apistore" @@ -178,6 +181,11 @@ func ProvideService( } if c.SignedInUser != nil { + // For unauthenticated requests, we set the namespace to the requested one + if !c.IsSignedIn { + useNamespaceFromPath(req.URL.Path, c.SignedInUser) + } + ctx := identity.WithRequester(req.Context(), c.SignedInUser) req = req.WithContext(ctx) } @@ -185,6 +193,7 @@ func ProvideService( resp := responsewriter.WrapForHTTP1Or2(c.Resp) s.handler.ServeHTTP(resp, req) } + k8sRoute.Any("/features.grafana.app/v0alpha1/*", handler) k8sRoute.Any("/", middleware.ReqSignedIn, handler) k8sRoute.Any("/*", middleware.ReqSignedIn, handler) } @@ -532,3 +541,16 @@ func (p *pluginContextProvider) GetPluginContext(ctx context.Context, pluginID s return p.contextProvider.PluginContextForDataSource(ctx, s) } + +func useNamespaceFromPath(path string, user *user.SignedInUser) { + if strings.HasPrefix(path, "/apis/") && len(path) > 6 { + parts := strings.Split(path[6:], "/") + if len(parts) >= 4 && parts[2] == "namespaces" { + ns, err := types.ParseNamespace(parts[3]) + if err == nil { + user.Namespace = ns.Value + user.OrgID = ns.OrgID + } + } + } +} diff --git a/pkg/services/apiserver/service_test.go b/pkg/services/apiserver/service_test.go new file mode 100644 index 00000000000..0e047545e3f --- /dev/null +++ b/pkg/services/apiserver/service_test.go @@ -0,0 +1,51 @@ +package apiserver + +import ( + "testing" + + "github.com/grafana/grafana/pkg/services/user" + "github.com/stretchr/testify/require" +) + +func Test_useNamespaceFromPath(t *testing.T) { + tests := []struct { + name string + path string + expNs string + }{ + { + name: "no namespace in path", + path: "/apis/folder.grafana.app/", + expNs: "", + }, + { + name: "namespace in path", + path: "/apis/folder.grafana.app/v1alpha1/namespaces/stacks-11/folders", + expNs: "stacks-11", + }, + { + name: "invalid namespace in path", + path: "/apis/folder.grafana.app/v1alpha1/namespaces/invalid/folders", + expNs: "invalid", + }, + { + name: "org namespace in path", + path: "/apis/folder.grafana.app/v1alpha1/namespaces/org-123/folders", + expNs: "org-123", + }, + { + name: "default namespace in path", + path: "/apis/folder.grafana.app/v1alpha1/namespaces/default/folders", + expNs: "default", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + user := &user.SignedInUser{} + useNamespaceFromPath(tt.path, user) + if user.Namespace != tt.expNs { + require.Equal(t, tt.expNs, user.Namespace, "expected namespace to be %s, got %s", tt.expNs, user.Namespace) + } + }) + } +} diff --git a/pkg/services/featuremgmt/openfeature.go b/pkg/services/featuremgmt/openfeature.go index 26caec9e210..76c07e52bbe 100644 --- a/pkg/services/featuremgmt/openfeature.go +++ b/pkg/services/featuremgmt/openfeature.go @@ -1,8 +1,8 @@ package featuremgmt import ( - "context" "fmt" + "net/url" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/setting" @@ -11,102 +11,58 @@ import ( ) type OpenFeatureService struct { - cfg *setting.Cfg log log.Logger provider openfeature.FeatureProvider Client openfeature.IClient } +// ProvideOpenFeatureService is used for wiring dependencies in single tenant grafana func ProvideOpenFeatureService(cfg *setting.Cfg) (*OpenFeatureService, error) { - var provider openfeature.FeatureProvider - var err error - if cfg.OpenFeature.ProviderType == setting.GOFFProviderType { - if cfg.OpenFeature.URL == nil { - return nil, fmt.Errorf("feature provider url is required for GOFFProviderType") - } - - provider, err = newGOFFProvider(cfg.OpenFeature.URL.String()) - } else { - provider, err = newStaticProvider(cfg) - } - + confFlags, err := setting.ReadFeatureTogglesFromInitFile(cfg.Raw.Section("feature_toggles")) if err != nil { - return nil, fmt.Errorf("failed to create %s feature provider: %w", cfg.OpenFeature.ProviderType, err) - } - - if err := openfeature.SetProviderAndWait(provider); err != nil { - return nil, fmt.Errorf("failed to set global %s feature provider: %w", cfg.OpenFeature.ProviderType, err) + return nil, fmt.Errorf("failed to read feature toggles from config: %w", err) } openfeature.SetEvaluationContext(openfeature.NewEvaluationContext(cfg.OpenFeature.TargetingKey, cfg.OpenFeature.ContextAttrs)) + return newOpenFeatureService(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, confFlags) +} + +// TODO: might need to be public, so other MT services could set up open feature client +func newOpenFeatureService(pType string, u *url.URL, staticFlags map[string]bool) (*OpenFeatureService, error) { + p, err := createProvider(pType, u, staticFlags) + if err != nil { + return nil, fmt.Errorf("failed to create feature provider: type %s, %w", pType, err) + } + + if err := openfeature.SetProviderAndWait(p); err != nil { + return nil, fmt.Errorf("failed to set global feature provider: %s, %w", pType, err) + } client := openfeature.NewClient("grafana-openfeature-client") return &OpenFeatureService{ - cfg: cfg, log: log.New("openfeatureservice"), - provider: provider, + provider: p, Client: client, }, nil } -func (s *OpenFeatureService) EvalFlagWithStaticProvider(ctx context.Context, flagKey string) (openfeature.BooleanEvaluationDetails, error) { - _, ok := s.provider.(*inMemoryBulkProvider) - if !ok { - return openfeature.BooleanEvaluationDetails{}, fmt.Errorf("not a static provider, request must be sent to open feature service") +func createProvider(providerType string, u *url.URL, staticFlags map[string]bool) (openfeature.FeatureProvider, error) { + if providerType != setting.GOFFProviderType { + return newStaticProvider(staticFlags) } - result, err := s.Client.BooleanValueDetails(ctx, flagKey, false, openfeature.TransactionContext(ctx)) - if err != nil { - return openfeature.BooleanEvaluationDetails{}, fmt.Errorf("failed to evaluate flag %s: %w", flagKey, err) + if u.String() == "" { + return nil, fmt.Errorf("feature provider url is required for GOFFProviderType") } - return result, nil + return newGOFFProvider(u.String()) } -func (s *OpenFeatureService) EvalAllFlagsWithStaticProvider(ctx context.Context) (OFREPBulkResponse, error) { - p, ok := s.provider.(*inMemoryBulkProvider) - if !ok { - return OFREPBulkResponse{}, fmt.Errorf("not a static provider, request must be sent to open feature service") +func createClient(provider openfeature.FeatureProvider) (openfeature.IClient, error) { + if err := openfeature.SetProviderAndWait(provider); err != nil { + return nil, fmt.Errorf("failed to set global feature provider: %w", err) } - flags, err := p.ListFlags() - if err != nil { - return OFREPBulkResponse{}, fmt.Errorf("static provider failed to list all flags: %w", err) - } - - allFlags := make([]OFREPFlag, 0, len(flags)) - for _, flagKey := range flags { - result, err := s.Client.BooleanValueDetails(ctx, flagKey, false, openfeature.TransactionContext(ctx)) - if err != nil { - s.log.Error("failed to evaluate flag during bulk evaluation", "flagKey", flagKey, "error", err) - continue - } - - allFlags = append(allFlags, OFREPFlag{ - Key: flagKey, - Value: result.Value, - Reason: "static provider evaluation result", - Variant: result.Variant, - ErrorCode: string(result.ErrorCode), - ErrorDetails: result.ErrorMessage, - }) - } - - return OFREPBulkResponse{Flags: allFlags}, nil -} - -// Bulk evaluation response -type OFREPBulkResponse struct { - Flags []OFREPFlag `json:"flags"` - Metadata map[string]any `json:"metadata,omitempty"` -} - -type OFREPFlag struct { - Key string `json:"key"` - Value bool `json:"value"` - Reason string `json:"reason"` - Variant string `json:"variant,omitempty"` - Metadata map[string]any `json:"metadata,omitempty"` - ErrorCode string `json:"errorCode,omitempty"` - ErrorDetails string `json:"errorDetails,omitempty"` + client := openfeature.NewClient("grafana-openfeature-client") + return client, nil } diff --git a/pkg/services/featuremgmt/static_evaluator.go b/pkg/services/featuremgmt/static_evaluator.go new file mode 100644 index 00000000000..f306c984f17 --- /dev/null +++ b/pkg/services/featuremgmt/static_evaluator.go @@ -0,0 +1,119 @@ +package featuremgmt + +import ( + "context" + "fmt" + "net/url" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/setting" + "github.com/open-feature/go-sdk/openfeature" +) + +// StaticFlagEvaluator provides methods for evaluating static feature flags +// it is only used when static provider is configured +type StaticFlagEvaluator interface { + EvalFlag(ctx context.Context, flagKey string) (openfeature.BooleanEvaluationDetails, error) + EvalAllFlags(ctx context.Context) (OFREPBulkResponse, error) +} + +// ProvideStaticEvaluator creates a static evaluator from configuration +// This can be used in wire dependency injection +func ProvideStaticEvaluator(cfg *setting.Cfg) (StaticFlagEvaluator, error) { + if cfg.OpenFeature.ProviderType == setting.GOFFProviderType { + l := log.New("static-evaluator") + l.Debug("cannot create static evaluator if configured provider is goff") + return &staticEvaluator{}, nil + } + + confFlags, err := setting.ReadFeatureTogglesFromInitFile(cfg.Raw.Section("feature_toggles")) + if err != nil { + return nil, fmt.Errorf("failed to read feature toggles from config: %w", err) + } + + return createStaticEvaluator(cfg.OpenFeature.ProviderType, cfg.OpenFeature.URL, confFlags) +} + +// createStaticEvaluator evaluator that allows evaluating static flags from config.ini +func createStaticEvaluator(providerType string, u *url.URL, staticFlags map[string]bool) (StaticFlagEvaluator, error) { + provider, err := createProvider(providerType, u, staticFlags) + if err != nil { + return nil, err + } + + staticProvider, ok := provider.(*inMemoryBulkProvider) + if !ok { + return nil, fmt.Errorf("provider is not a static provider") + } + + client, err := createClient(provider) + if err != nil { + return nil, err + } + + return &staticEvaluator{ + provider: staticProvider, + client: client, + log: log.New("static-evaluator"), + }, nil +} + +// staticEvaluator implements StaticFlagEvaluator for static providers +type staticEvaluator struct { + provider *inMemoryBulkProvider + client openfeature.IClient + log log.Logger +} + +func (s *staticEvaluator) EvalFlag(ctx context.Context, flagKey string) (openfeature.BooleanEvaluationDetails, error) { + result, err := s.client.BooleanValueDetails(ctx, flagKey, false, openfeature.TransactionContext(ctx)) + if err != nil { + return openfeature.BooleanEvaluationDetails{}, fmt.Errorf("failed to evaluate flag %s: %w", flagKey, err) + } + + return result, nil +} + +func (s *staticEvaluator) EvalAllFlags(ctx context.Context) (OFREPBulkResponse, error) { + flags, err := s.provider.ListFlags() + if err != nil { + return OFREPBulkResponse{}, fmt.Errorf("static provider failed to list all flags: %w", err) + } + + allFlags := make([]OFREPFlag, 0, len(flags)) + for _, flagKey := range flags { + result, err := s.client.BooleanValueDetails(ctx, flagKey, false, openfeature.TransactionContext(ctx)) + if err != nil { + s.log.Error("failed to evaluate flag during bulk evaluation", "flagKey", flagKey, "error", err) + continue + } + + allFlags = append(allFlags, OFREPFlag{ + Key: flagKey, + Value: result.Value, + Reason: "static provider evaluation result", + Variant: result.Variant, + ErrorCode: string(result.ErrorCode), + ErrorDetails: result.ErrorMessage, + }) + } + + return OFREPBulkResponse{Flags: allFlags}, nil +} + +// OFREPBulkResponse represents the response for bulk flag evaluation +type OFREPBulkResponse struct { + Flags []OFREPFlag `json:"flags"` + Metadata map[string]any `json:"metadata,omitempty"` +} + +// OFREPFlag represents a single flag in the bulk response +type OFREPFlag struct { + Key string `json:"key"` + Value bool `json:"value"` + Reason string `json:"reason"` + Variant string `json:"variant,omitempty"` + Metadata map[string]any `json:"metadata,omitempty"` + ErrorCode string `json:"errorCode,omitempty"` + ErrorDetails string `json:"errorDetails,omitempty"` +} diff --git a/pkg/services/featuremgmt/static_provider.go b/pkg/services/featuremgmt/static_provider.go index a69e7c28b4e..f384bd00de1 100644 --- a/pkg/services/featuremgmt/static_provider.go +++ b/pkg/services/featuremgmt/static_provider.go @@ -1,9 +1,6 @@ package featuremgmt import ( - "fmt" - - "github.com/grafana/grafana/pkg/setting" "github.com/open-feature/go-sdk/openfeature" "github.com/open-feature/go-sdk/openfeature/memprovider" ) @@ -31,12 +28,7 @@ func (p *inMemoryBulkProvider) ListFlags() ([]string, error) { return keys, nil } -func newStaticProvider(cfg *setting.Cfg) (openfeature.FeatureProvider, error) { - confFlags, err := setting.ReadFeatureTogglesFromInitFile(cfg.Raw.Section("feature_toggles")) - if err != nil { - return nil, fmt.Errorf("failed to read feature toggles from config: %w", err) - } - +func newStaticProvider(confFlags map[string]bool) (openfeature.FeatureProvider, error) { flags := make(map[string]memprovider.InMemoryFlag, len(standardFeatureFlags)) // Add flags from config.ini file diff --git a/pkg/services/featuremgmt/static_provider_test.go b/pkg/services/featuremgmt/static_provider_test.go index 86d0eaa318c..4c244ed8cad 100644 --- a/pkg/services/featuremgmt/static_provider_test.go +++ b/pkg/services/featuremgmt/static_provider_test.go @@ -20,7 +20,7 @@ func Test_StaticProvider(t *testing.T) { stFeatValue := stFeat.Expression == "true" t.Run("empty config loads standard flags", func(t *testing.T) { - p := provider(t, []byte(``)) + p := setup(t, []byte(``)) // Check for one of the standard flags feat, err := p.Client.BooleanValueDetails(ctx, stFeatName, !stFeatValue, evalCtx) assert.NoError(t, err) @@ -32,14 +32,14 @@ func Test_StaticProvider(t *testing.T) { [feature_toggles] featureOne = true `) - p := provider(t, conf) + p := setup(t, conf) feat, err := p.Client.BooleanValueDetails(ctx, "featureOne", false, evalCtx) assert.NoError(t, err) assert.True(t, feat.Value) }) t.Run("missing feature should return default evaluation value and an error", func(t *testing.T) { - p := provider(t, []byte(``)) + p := setup(t, []byte(``)) missingFeature, err := p.Client.BooleanValueDetails(ctx, "missingFeature", true, evalCtx) assert.Error(t, err) assert.True(t, missingFeature.Value) @@ -47,7 +47,7 @@ featureOne = true }) } -func provider(t *testing.T, conf []byte) *OpenFeatureService { +func setup(t *testing.T, conf []byte) *OpenFeatureService { t.Helper() cfg, err := setting.NewCfgFromBytes(conf) require.NoError(t, err) @@ -64,16 +64,12 @@ func Test_CompareStaticProviderWithFeatureManager(t *testing.T) { _, err = sec.NewKey("ABCD", "true") require.NoError(t, err) - p, err := ProvideOpenFeatureService(cfg) + // Use StaticFlagEvaluator instead of OpenFeatureService for static evaluation + staticEvaluator, err := ProvideStaticEvaluator(cfg) require.NoError(t, err) - _, ok := p.provider.(*inMemoryBulkProvider) - if !ok { - t.Fatalf("expected inMemoryBulkProvider, got %T", p.provider) - } - ctx := openfeature.WithTransactionContext(context.Background(), openfeature.NewEvaluationContext("grafana", nil)) - allFlags, err := p.EvalAllFlagsWithStaticProvider(ctx) + allFlags, err := staticEvaluator.EvalAllFlags(ctx) require.NoError(t, err) openFeatureEnabledFlags := map[string]bool{} @@ -86,7 +82,7 @@ func Test_CompareStaticProviderWithFeatureManager(t *testing.T) { mgr, err := ProvideManagerService(cfg) require.NoError(t, err) - // compare enabled feature flags match between OpenFeature static provider and Feature Manager + // compare enabled feature flags match between StaticFlagEvaluator and Feature Manager enabledFeatureManager := mgr.GetEnabled(ctx) assert.Equal(t, openFeatureEnabledFlags, enabledFeatureManager) }