diff --git a/README.md b/README.md index 9a4e1a42e..fe43fa96b 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,61 @@ Flipper.enable_percentage_of_actors :search, 2 Read more about [getting started with Flipper](https://flippercloud.io/docs?utm_source=oss&utm_medium=readme&utm_campaign=getting_started) and [enabling features](https://flippercloud.io/docs/features?utm_source=oss&utm_medium=readme&utm_campaign=enabling_features). +## Named Instances + +Applications that need independent sets of feature flags can configure named +instances. Each name has its own adapter stack, features, groups, request cache, and +preload behavior, while the existing top-level `Flipper` instance is unchanged. + +```ruby +Flipper.configure do |config| + config.named(:cross_app) do |cross_app| + cross_app.adapter do + Flipper::Adapters::ActiveRecord.new(table_prefix: "cross_app_") + end + end +end + +Flipper.cross_app.register(:beta_organizations) do |actor| + actor.respond_to?(:beta_organization?) && actor.beta_organization? +end + +Flipper.enabled?(:product_feature, current_user) +Flipper.cross_app.enabled?(:shared_feature, current_user) +Flipper.named(:cross_app).enabled?(:shared_feature, current_user) +``` + +Named instances work with any adapter and do not require Flipper Cloud. When using +Active Record, generate and update the prefixed tables separately: + +```shell +bin/rails generate flipper:active_record --table-prefix=cross_app_ +bin/rails generate flipper:update --table-prefix=cross_app_ +``` + +A named instance can optionally use a separate Cloud project and webhook: + +```ruby +Flipper.configure do |config| + config.named(:cross_app) do |cross_app| + cross_app.adapter do + Flipper::Adapters::ActiveRecord.new(table_prefix: "cross_app_") + end + cross_app.cloud(path: "_flipper/cross_app") + end +end +``` + +For that example, Rails credentials can be stored under +`flipper.cross_app.cloud_token` and `flipper.cross_app.cloud_sync_secret`, or provided +through `FLIPPER_CLOUD_CROSS_APP_TOKEN` and +`FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET`. Named instances never inherit the default +`FLIPPER_CLOUD_TOKEN` or `FLIPPER_CLOUD_SYNC_SECRET`. + +Applications sharing a Cloud project must register the same named group predicates +and return the same `flipper_id` for shared actors. Cloud synchronizes group names and +actor identifiers, not Ruby group definitions or application identity mappings. + ## Flipper Cloud Like Flipper and want more? Check out [Flipper Cloud](https://www.flippercloud.io?utm_source=oss&utm_medium=readme&utm_campaign=check_out), which comes with: diff --git a/docs/NAMED_INSTANCES_PLAN.md b/docs/NAMED_INSTANCES_PLAN.md new file mode 100644 index 000000000..7cbfefdd2 --- /dev/null +++ b/docs/NAMED_INSTANCES_PLAN.md @@ -0,0 +1,400 @@ +# Named Flipper Instances + +Status: Implemented locally; pending review + +## Motivation + +An organization may run many Rails applications, with one Flipper Cloud project per +product for feature isolation and permissions, while also needing a separate Cloud +project for features shared across every product. + +Today each application can configure only the process-wide `Flipper` instance. A +shared feature therefore has to be copied into every product project and kept in +sync manually. + +The desired application code is: + +```ruby +Flipper.enabled?(:product_feature, person) +Flipper.cross_app.enabled?(:shared_feature, person) + +# Optional application-level alias. +CrossAppFlipper = Flipper.cross_app +CrossAppFlipper.enabled?(:shared_feature, person) +``` + +Each named instance must have its own adapter stack, Cloud project, webhook, +memoization, preload configuration, groups, and permissions. + +Active Record table isolation is already supported by the adapter's `table_prefix:` +option and is not part of this work. + +Named instances are a Flipper core feature, not a Flipper Cloud feature. A named +instance may use Memory, Active Record, Redis, or any other adapter without loading +Cloud. Cloud configuration is an optional capability of a named instance. + +## Compatibility Requirement + +This feature is additive. An application that does not configure a named instance +must behave exactly as it does today. + +In particular: + +- Keep the signatures and behavior of `Flipper.configure`, `Flipper.configuration`, + `Flipper.configuration=`, `Flipper.instance`, and `Flipper.instance=` unchanged. +- Keep every existing top-level DSL delegate, global group API, environment variable, + Rails initializer, middleware key, Cloud route, and test helper behavior unchanged. +- Keep the default instance in `Thread.current[:flipper_instance]`. +- Do not add middleware, routes, polling, telemetry, or adapter construction when no + named instances are configured. +- Do not change default instrumentation payloads. +- Run the existing test suite without modifying existing assertions merely to + accommodate the new feature. Add separate coverage for named behavior. + +Avoid implementing the existing default as a public named instance. A parallel +named-instance path is less likely to change default lifecycle or lookup behavior. + +## Public API + +The implementation uses a named child configuration rather than a block that only +returns a DSL: + +```ruby +Flipper.configure do |config| + config.named(:cross_app) do |cross_app| + cross_app.adapter do + Flipper::Adapters::ActiveRecord.new(table_prefix: "cross_app_") + end + + cross_app.cloud(path: "_flipper/cross_app") + end +end + +Flipper.cross_app.register(:beta_organizations) do |actor| + actor.respond_to?(:beta_organization?) && actor.beta_organization? +end +``` + +Access would be available through both: + +```ruby +Flipper.named(:cross_app) +Flipper.cross_app +``` + +`Flipper.named(:cross_app)` is the universal lookup API. `Flipper.cross_app` is +explicitly defined convenience sugar; it must not be implemented with a general +`method_missing`. + +The returned object should be a stable proxy that resolves the current named, +thread-local DSL for every operation. That allows an application constant such as +`CrossAppFlipper = Flipper.cross_app` to continue working after configuration or test +state is reset instead of retaining a stale adapter. + +### Names + +A name must: + +- normalize consistently to a symbol; +- be a valid Ruby method name for direct accessor generation; +- be unique among named instances; and +- not collide with any existing public, protected, or private method callable on + `Flipper`, including inherited methods. + +Invalid, duplicate, and reserved names should fail during configuration with a +specific exception. Replacing or clearing configuration must not leave stale +dynamically defined accessors behind. + +## Why a Child Configuration + +A simple factory such as `config.named(:cross_app) { Flipper::Cloud.new(...) }` is +enough to construct a DSL, but it leaves several behaviors outside the model: + +- adapter composition with `config.use`; +- Rails strict and actor-limit wrappers; +- instrumentation; +- per-instance test adapters and reset behavior; +- group registration; +- memoization, preload, environment key, and Cloud route metadata; and +- explicit Cloud credentials without accidental fallback to the default project. + +A child configuration can reuse the existing adapter builder and default factory +concept while adding only the metadata needed for named instances. Existing +`Flipper::Configuration` behavior must remain unchanged. + +## Core Design + +### Registry and lifecycle + +- Store named configurations in the root `Flipper::Configuration`. +- Create one DSL per name per thread, analogous to the existing default instance. +- Version named configurations so replacement is observed by DSL caches in every + thread; clearing only the thread that performed the replacement is insufficient. +- Use a collision-resistant internal thread-local container rather than creating one + arbitrary thread-local key per user-supplied name. +- Keep group registries on the named configuration, not the thread-local DSL. Groups + registered during boot must be visible to every request thread. +- Reset named thread-local DSLs whenever their configuration changes. +- Provide an internal reset path that tests can exercise without changing the + existing `Flipper.instance=` contract. +- Make duplicate registration deterministic and thread safe. +- Define when Rails freezes the set of names used for automatic middleware and + routes. A name registered after `config/initializers` cannot silently receive an + incomplete request lifecycle. + +### Groups + +Groups cannot remain process-global for named instances. The same group name may +legitimately have different predicates in a product project and the cross-product +project. + +Add group lookup and registration to the owning configuration/DSL. Update group gate +evaluation, feature group operations, group value wrapping, Cloud webhook reporting, +and UI/API validation to resolve groups through the owning instance. + +The default DSL must continue to resolve the existing live global +`Flipper.groups_registry`. Do not copy that registry into the default configuration; +late calls to `Flipper.register` and `Flipper.groups_registry=` must retain their +current behavior. + +Cloud sync stores enabled group names, not Ruby predicates. Every application using +the shared project must deploy the same named group definitions. The feature should +document this operational requirement; it cannot synchronize application code. +The applications must also produce the same `flipper_id` for shared actors. Cloud can +synchronize an enabled actor ID, but it cannot make different application actor +models agree on identity. + +### Expressions + +`feature_enabled` expressions currently call the top-level default `Flipper`. A +feature reference inside an expression must instead resolve through the DSL that owns +the expression. + +- Named expressions only reference features in the same named instance. +- Default expressions keep their existing behavior. +- Circular-evaluation tracking must include instance identity as well as feature name + so equal feature names in different instances do not interfere. +- Any new resolver passed into `Feature`, gates, types, or expressions must be + optional and preserve current constructor behavior. + +### Rails memoization and preload + +Each configured named instance needs its own Rack environment key and memoizer pass: + +1. Put the named proxy/DSL into its environment key with `SetupEnv`. +2. Run `Memoizer` with that same key and the named preload setting. +3. Ensure memoization is disabled in an `ensure` block, as it is for the default. + +The default middleware and `flipper` environment key remain untouched. Environment +keys must be unique, and middleware order must allow default and named instances to +memoize and preload independently during the same request. + +Rails settings such as instrumenter, strict mode, actor limit, memoize, and preload +need explicit inheritance rules. The likely default is to inherit the root Rails +setting and permit a named override, but this must be decided before implementation. + +### Flipper Cloud + +Each named Cloud instance must use an explicit token and sync secret for its project. +It must not accidentally inherit `FLIPPER_CLOUD_TOKEN` or +`FLIPPER_CLOUD_SYNC_SECRET`, which remain reserved for the default instance. + +Named Cloud instances must be explicitly declared; do not discover instance names by +scanning environment variables. Once declared, a name may opt into Cloud and use +name-scoped automatic configuration. For `cross_app`, the proposed variables are: + +```text +FLIPPER_CLOUD_CROSS_APP_TOKEN +FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET +``` + +The corresponding Rails credentials are: + +```yaml +flipper: + cross_app: + cloud_token: "..." + cloud_sync_secret: "..." +``` + +Resolution order for named credentials is: + +1. explicit named configuration; +2. named Rails credentials; +3. name-scoped environment variables; and +4. missing, which disables or rejects Cloud configuration as appropriate. + +There is deliberately no fallback from a named credential to the default +`FLIPPER_CLOUD_TOKEN`, `FLIPPER_CLOUD_SYNC_SECRET`, or default Rails credential. The +existing default credential lookup and `Flipper::Cloud.new` behavior remain +unchanged. The implementation may need an additive Cloud configuration option that +disables default environment fallback when building a named Cloud DSL. + +Each project also needs a distinct webhook mount and Rack environment key. Existing +`Flipper::Cloud.app(flipper, env_key:)` already accepts an explicit instance and can +be reused. Automatic Rails mounting, if provided, must reject duplicate paths and +must leave the existing `/_flipper` route unchanged. + +Polling and telemetry should continue to deduplicate by Cloud URL and token. Audit +fork behavior with two Cloud instances. If instrumentation needs instance identity, +add it only for named events; preserve the shape of default events. + +### Active Record schema lifecycle + +The default and named adapters may share a database connection, but they must not +share table names. For example: + +```ruby +product_adapter = Flipper::Adapters::ActiveRecord.new +cross_app_adapter = Flipper::Adapters::ActiveRecord.new(table_prefix: "cross_app_") +``` + +The adapter creates isolated internal model subclasses for a prefix, so configuring +one adapter must not mutate the table names used by another adapter. Keep the +existing regression coverage for that property. + +Applications own these tables through Rails migrations. They should generate the +named schema with the same prefix used by the adapter: + +```shell +bin/rails generate flipper:active_record --table-prefix=cross_app_ +``` + +When a future Flipper release changes the expected schema, the application must run +the Flipper update generator once for every table set, including the prefix: + +```shell +bin/rails generate flipper:update +bin/rails generate flipper:update --table-prefix=cross_app_ +``` + +This is how prefixed tables keep the same columns, indexes, and types as the default +tables. Named instances should not perform runtime schema creation or mutation. + +### UI, API, and CLI + +The UI and API already accept an explicit Flipper object and environment key. They +should work with a named proxy after their global group validation is changed to use +the supplied instance. Applications can mount separate UIs/APIs deliberately rather +than receiving automatic routes. + +The CLI should continue to target the default instance. A future `--instance NAME` +option can be considered separately and is not required for the first release. + +### Tests + +`Flipper::TestHelp` must keep its current default behavior. Add named helpers or +configuration hooks that allow a named Cloud instance to use shared in-memory storage +during tests and clear its features between examples. + +Per-example reset should preserve boot-time named group definitions, just as the +existing helper preserves global groups. Full configuration replacement must reset +the named group registry, thread-local DSLs, generated accessors, and retained proxy +resolution. Loading test help must not contact Cloud. + +## Expected Code Surface + +The repository audit found named-instance assumptions in more than the top-level +delegation layer. The likely implementation surface is: + +- registry and access: `lib/flipper.rb`, `lib/flipper/configuration.rb`, and new + proxy/configuration classes; +- ownership propagation: `lib/flipper/dsl.rb`, `lib/flipper/feature.rb`, + `lib/flipper/feature_check_context.rb`, `lib/flipper/gates/group.rb`, + `lib/flipper/types/group.rb`, and `lib/flipper/expressions/feature_enabled.rb`; +- Rails lifecycle: `lib/flipper/engine.rb`, `SetupEnv`, and `Memoizer` integration; +- Cloud: configuration, middleware group reporting, and routes; +- UI/API: group-gate validation against the supplied instance rather than global + `Flipper`; and +- test support plus core, Rails, Cloud, UI, API, generator, thread, and fork specs. + +This inventory should be re-run during implementation. It is a guard against +shipping an accessor that appears to work for booleans and actors while groups, +expressions, webhooks, or tests still fall back to the default instance. + +## Implementation Phases + +### 1. Core registry and access + +- Add named child configurations, registry, proxy, and explicit accessors. +- Validate names and collisions. +- Add per-thread lifecycle and reset behavior. +- Verify the complete existing top-level API remains unchanged. + +### 2. Instance-owned evaluation + +- Add isolated group registries while preserving the live default global registry. +- Make group gates, feature helpers, and type wrapping owner-aware. +- Make feature expressions owner-aware and fix circular-evaluation identity. +- Update UI, API, and Cloud group lookups. + +### 3. Rails integration + +- Define named Rails settings and inheritance rules. +- Install paired setup/memoizer middleware only for configured names. +- Verify independent preloading and memoization for default plus multiple names. +- Apply strict, actor-limit, and instrumentation configuration consistently. + +### 4. Cloud integration + +- Require unambiguous per-name credentials. +- Support distinct webhook mounts and environment keys. +- Verify sync, polling, telemetry, and fork behavior with two projects. + +### 5. Test support and documentation + +- Add named test configuration/reset helpers without changing existing helpers. +- Document Active Record prefixes and migrations for each named local store. +- Document shared actor ID and group-definition requirements across applications. +- Add a complete Rails example with product and cross-product Cloud projects. + +## Verification Matrix + +At minimum, cover: + +- zero named instances: all existing tests and public API behavior are unchanged; +- existing method arities, return values, custom `Flipper.configuration=` objects, + and direct `Flipper.instance=` overrides remain compatible; +- default plus one and multiple named instances in one process; +- separate adapters and Active Record table prefixes do not overwrite one another; +- create and update generators apply the same schema changes to prefixed tables; +- same feature name has independent state in each instance; +- same group name can have independent predicates in each instance; +- late default global group registration still works; +- a named `feature_enabled` expression resolves only within its owner; +- retained proxy constants follow configuration replacement; +- name collisions, duplicates, invalid names, and configuration clearing; +- isolation between request threads and correct visibility of boot-time groups; +- default and named request memoization/preload independently start and stop; +- exceptions cannot leave either instance memoizing; +- distinct Cloud tokens, sync secrets, webhooks, and Rack environment keys; +- no Cloud network activity from test helpers; +- UI/API operations and group validation use the supplied instance; and +- supported Ruby and Rails versions through the repository's existing test matrix. + +## Decisions + +1. Configure a child with `config.named(:cross_app)`. Do not add arguments to + `Flipper.configure`; even an optional argument changes its reflected arity and + weakens the compatibility guarantee. +2. Access a child through both `Flipper.named(:cross_app)` and the explicitly defined + `Flipper.cross_app` convenience method. +3. Inherit app-wide Rails instrumenter, strict mode, actor limit, memoize, and preload + settings, while permitting named overrides. Credentials, Rack environment keys, + and webhook paths are always instance-specific. +4. Automatically mount a named Cloud webhook only when that instance has a webhook + path and sync secret configured. +5. Register groups through the named proxy, such as + `Flipper.cross_app.register(:beta)`. Registration updates the configuration-owned + group registry shared by every thread-local DSL for that name. +6. `Flipper::TestHelp` replaces every configured named adapter with a separate shared + Memory adapter and preserves registered groups between examples. +7. Named Rails instances must be declared during initialization to receive automatic + middleware and routes. A later core registration remains usable but requires + explicit application middleware and route mounts. +8. Named instances are adapter-agnostic core functionality. A name opts into Cloud + explicitly; Cloud environment variables do not implicitly create names. + +The implementation includes group ownership, configuration reset, and independent +Rails memoization/preload behavior so those constraints are part of the public API +review rather than deferred follow-up work. diff --git a/lib/flipper.rb b/lib/flipper.rb index badc40a4f..0c74e9ac5 100644 --- a/lib/flipper.rb +++ b/lib/flipper.rb @@ -21,7 +21,13 @@ def new(adapter, options = {}) # # Yields Flipper::Configuration instance. def configure - yield configuration if block_given? + return unless block_given? + + begin + yield configuration + ensure + refresh_named_instance_accessors + end end # Public: Returns Flipper::Configuration instance. @@ -33,7 +39,11 @@ def configuration def configuration=(configuration) # need to reset flipper instance if configuration changes self.instance = nil + Thread.current[:__flipper_named_instances__] = nil + remove_named_instance_accessors @configuration = configuration + refresh_named_instance_accessors + configuration end # Public: Default per thread flipper instance if configured. You should not @@ -53,6 +63,84 @@ def instance=(flipper) Thread.current[:flipper_instance] = flipper end + # Public: Returns a stable proxy for a configured named instance. + def named(name) + name = normalize_named_instance_name(name) + named_configuration(name) + named_instance_proxies_mutex.synchronize do + named_instance_proxies[name] ||= NamedProxy.new(name) + end + end + + # Internal: Returns the current named configuration or raises if missing. + def named_configuration(name) + name = normalize_named_instance_name(name) + configured = configuration.respond_to?(:named_configuration) && configuration.named_configuration(name) + return configured if configured + + raise NamedInstanceNotFound, "Named instance #{name.inspect} has not been configured" + end + + # Internal: Returns the per-thread DSL for a configured named instance. + def named_instance(name) + configured = named_configuration(name) + instances = Thread.current[:__flipper_named_instances__] ||= {} + cached = instances[name] + + if cached && cached[:configuration].equal?(configured) && cached[:version] == configured.version + cached[:instance] + else + loop do + version = configured.version + instance = configured.default + next unless version == configured.version + + instances[name] = { + configuration: configured, + version: version, + instance: instance, + } + return instance + end + end + end + + # Internal: Reset named DSL caches for the current thread. + def reset_named_instances + Thread.current[:__flipper_named_instances__] = nil + end + + # Internal: Normalize and validate a named instance name. + def normalize_named_instance_name(name) + unless name.is_a?(String) || name.is_a?(Symbol) + raise InvalidNamedInstanceName, "Named instance name must be a String or Symbol" + end + + normalized = name.to_s + unless normalized.match?(/\A[a-z_][a-z0-9_]*\z/) + raise InvalidNamedInstanceName, "Named instance #{name.inspect} must use lowercase snake case" + end + + normalized.to_sym + end + + # Internal: Reject names that would replace Flipper's existing API. + def validate_named_instance_name!(name) + name = normalize_named_instance_name(name) + return name if named_instance_accessor_names.include?(name) + + if singleton_class.public_method_defined?(name) || + singleton_class.protected_method_defined?(name) || + singleton_class.private_method_defined?(name) + raise InvalidNamedInstanceName, "Named instance #{name.inspect} conflicts with an existing Flipper method" + end + + name + end + + private :named_configuration, :named_instance, :reset_named_instances, + :normalize_named_instance_name, :validate_named_instance_name! + # Public: All the methods delegated to instance. These should match the # interface of Flipper::DSL. def_delegators :instance, @@ -197,7 +285,10 @@ def groups_registry=(registry) require 'flipper/adapters/memory' require 'flipper/adapters/strict' require 'flipper/adapter_builder' +require 'flipper/registry' require 'flipper/configuration' +require 'flipper/named_configuration' +require 'flipper/named_proxy' require 'flipper/dsl' require 'flipper/errors' require 'flipper/feature' @@ -208,7 +299,6 @@ def groups_registry=(registry) require 'flipper/middleware/memoizer' require 'flipper/middleware/setup_env' require 'flipper/poller' -require 'flipper/registry' require 'flipper/expression' require 'flipper/type' require 'flipper/types/actor' @@ -227,4 +317,52 @@ def groups_registry=(registry) Flipper.configuration Flipper.groups_registry +module Flipper + class << self + private + + def named_instance_proxies + @named_instance_proxies ||= {} + end + + def named_instance_proxies_mutex + @named_instance_proxies_mutex ||= Mutex.new + end + + def named_instance_accessor_names + @named_instance_accessor_names ||= Set.new + end + + def refresh_named_instance_accessors + return unless @configuration.respond_to?(:named_instance_names) + + @configuration.named_instance_names.each do |name| + next if named_instance_accessor_names.include?(name) + + validate_named_instance_name!(name) + proxy = named(name) + define_singleton_method(name) { proxy } + named_instance_accessor_names.add(name) + named_instance_accessor_methods[name] = method(name) + end + end + + def remove_named_instance_accessors + named_instance_accessor_names.each do |name| + generated_method = named_instance_accessor_methods[name] + current_method = method(name) if respond_to?(name, true) + if current_method == generated_method && singleton_class.instance_methods(false).include?(name) + singleton_class.send(:remove_method, name) + end + end + named_instance_accessor_names.clear + named_instance_accessor_methods.clear + end + + def named_instance_accessor_methods + @named_instance_accessor_methods ||= {} + end + end +end + require "flipper/engine" if defined?(Rails) diff --git a/lib/flipper/api/v1/actions/groups_gate.rb b/lib/flipper/api/v1/actions/groups_gate.rb index 37e67e81f..a60c1252f 100644 --- a/lib/flipper/api/v1/actions/groups_gate.rb +++ b/lib/flipper/api/v1/actions/groups_gate.rb @@ -34,7 +34,7 @@ def ensure_valid_params end return if allow_unregistered_groups? - return if Flipper.group_exists?(group_name) + return if flipper.group_exists?(group_name) json_error_response(:group_not_registered) end diff --git a/lib/flipper/cloud/configuration.rb b/lib/flipper/cloud/configuration.rb index fcceb932a..5df337b77 100644 --- a/lib/flipper/cloud/configuration.rb +++ b/lib/flipper/cloud/configuration.rb @@ -149,6 +149,11 @@ def url=(value) @url = UrlValidator.validate(value) end + # Internal: Immutable identity for registries shared by Cloud endpoints. + def endpoint_key + [url.dup.freeze, token.dup.freeze].freeze + end + private def app_adapter @@ -157,7 +162,7 @@ def app_adapter end def poller - Flipper::Poller.get(@url + @token, { + Flipper::Poller.get(endpoint_key, { interval: sync_interval, remote_adapter: http_adapter, instrumenter: instrumenter, diff --git a/lib/flipper/cloud/middleware.rb b/lib/flipper/cloud/middleware.rb index 8df6c9b53..cfd3c2d17 100644 --- a/lib/flipper/cloud/middleware.rb +++ b/lib/flipper/cloud/middleware.rb @@ -42,7 +42,7 @@ def call!(env) begin flipper.sync(cache_bust: true) body = JSON.generate({ - groups: Flipper.group_names.map { |name| {name: name}} + groups: flipper.group_names.map { |name| {name: name}} }) rescue Flipper::Adapters::Http::Error => error status = error.response.code.to_i == 402 ? 402 : 500 diff --git a/lib/flipper/cloud/routes.rb b/lib/flipper/cloud/routes.rb index 87be70edd..6fae0d6ab 100644 --- a/lib/flipper/cloud/routes.rb +++ b/lib/flipper/cloud/routes.rb @@ -1,5 +1,7 @@ # Default routes loaded by Flipper::Cloud::Engine Rails.application.routes.draw do + cloud_mounts = [] + if ENV["FLIPPER_CLOUD_TOKEN"] && !ENV.fetch("FLIPPER_CLOUD_SYNC_SECRET", "").empty? require 'flipper/cloud' config = Rails.application.config.flipper @@ -9,6 +11,31 @@ memoizer_options: { preload: config.preload } ) - mount cloud_app, at: config.cloud_path + cloud_mounts << [config.cloud_path, cloud_app] + end + + if !Rails.application.config.flipper.test_help && Flipper.configuration.respond_to?(:named_instance_names) + Flipper.configuration.named_instance_names.each do |name| + named = Flipper.configuration.named_configuration(name) + next unless named.cloud? && named.cloud_path + + cloud_options = named.resolve_cloud_credentials + sync_secret = cloud_options[:sync_secret] + next if sync_secret.nil? || sync_secret == false || sync_secret.empty? + + require "flipper/cloud" + cloud_app = Flipper::Cloud.app(Flipper.named(name), + env_key: named.env_key, + memoizer_options: { preload: named.preload } + ) + cloud_mounts << [named.cloud_path, cloud_app] + end + end + + cloud_mounts.sort_by do |path, _| + normalized_path = path.to_s.sub(%r{\A/+}, "").sub(%r{/+\z}, "") + -normalized_path.length + end.each do |path, cloud_app| + mount cloud_app, at: path end end diff --git a/lib/flipper/cloud/telemetry.rb b/lib/flipper/cloud/telemetry.rb index ef14d51fd..c76b19f1c 100644 --- a/lib/flipper/cloud/telemetry.rb +++ b/lib/flipper/cloud/telemetry.rb @@ -19,11 +19,11 @@ def self.reset instances.each { |_, instance| instance.stop }.clear end - # Internal: Fetch an instance of telemetry once per process per url + - # token (aka cloud endpoint). Should only ever be one instance unless you + # Internal: Fetch an instance of telemetry once per process per cloud + # endpoint. Should only ever be one instance unless you # are doing some funky stuff. def self.instance_for(cloud_configuration) - instances.compute_if_absent(cloud_configuration.url + cloud_configuration.token) do + instances.compute_if_absent(cloud_configuration.endpoint_key) do new(cloud_configuration) end end diff --git a/lib/flipper/configuration.rb b/lib/flipper/configuration.rb index 7a9ec745a..ab47cc6f6 100644 --- a/lib/flipper/configuration.rb +++ b/lib/flipper/configuration.rb @@ -3,6 +3,9 @@ class Configuration def initialize(options = {}) @builder = AdapterBuilder.new { store Flipper::Adapters::Memory } @default = -> { Flipper.new(@builder.to_adapter) } + @named_configurations = {} + @pending_named_instance_names = Set.new + @named_configurations_mutex = Mutex.new end # The default adapter to use. @@ -66,6 +69,49 @@ def default(&block) end end + # Public: Configure a named Flipper instance. + # + # name - Lowercase snake-case name used by Flipper.named and the generated + # convenience method (for example, Flipper.cross_app). + # block - Configuration block yielded a Flipper::NamedConfiguration. + # + # Returns the newly created named configuration. + def named(name) + name = Flipper.send(:normalize_named_instance_name, name) + Flipper.send(:validate_named_instance_name!, name) + + @named_configurations_mutex.synchronize do + if @named_configurations.key?(name) || @pending_named_instance_names.include?(name) + raise DuplicateNamedInstance, "Named instance #{name.inspect} has already been configured" + end + + @pending_named_instance_names.add(name) + end + + begin + named_configuration = NamedConfiguration.new(name) + yield named_configuration if block_given? + @named_configurations_mutex.synchronize do + @named_configurations[name] = named_configuration + end + ensure + @named_configurations_mutex.synchronize { @pending_named_instance_names.delete(name) } + end + Flipper.send(:refresh_named_instance_accessors) if Flipper.configuration.equal?(self) + named_configuration + end + + # Public: Returns a configured named child without creating it. + def named_configuration(name) + name = Flipper.send(:normalize_named_instance_name, name) + @named_configurations_mutex.synchronize { @named_configurations[name] } + end + + # Public: Returns the configured named instance names. + def named_instance_names + @named_configurations_mutex.synchronize { @named_configurations.keys.dup } + end + def statsd require 'flipper/instrumentation/statsd_subscriber' Flipper::Instrumentation::StatsdSubscriber.client diff --git a/lib/flipper/dsl.rb b/lib/flipper/dsl.rb index d00ded85c..e6e0685e3 100644 --- a/lib/flipper/dsl.rb +++ b/lib/flipper/dsl.rb @@ -21,6 +21,9 @@ class DSL # :memoize - Should adapter be wrapped by memoize adapter or not. def initialize(adapter, options = {}) @instrumenter = options.fetch(:instrumenter, Instrumenters::Noop) + @group_resolver = options.fetch(:group_resolver, Flipper) + @feature_resolver = options.fetch(:feature_resolver, Flipper) + @instance_key = options[:instance_key] memoize = options.fetch(:memoize, true) adapter = Adapters::Memoizable.new(adapter) if memoize @adapter = adapter @@ -30,6 +33,15 @@ def initialize(adapter, options = {}) @memoized_features = Concurrent::Map.new end + # Internal: Assign ownership for a DSL created by a named configuration. + def instance_owner=(configuration) + @group_resolver = configuration + @feature_resolver = self + @instance_key = configuration.name + @memoized_features.clear + self + end + # Public: Check if a feature is enabled. # # name - The String or Symbol name of the feature. @@ -223,7 +235,11 @@ def feature(name) end @memoized_features.compute_if_absent(name.to_sym) do - Feature.new(name, @adapter, instrumenter: instrumenter) + Feature.new(name, @adapter, + instrumenter: instrumenter, + group_resolver: @group_resolver, + feature_resolver: @feature_resolver, + instance_key: @instance_key) end end @@ -259,7 +275,32 @@ def preload_all # # Returns an instance of Flipper::Group. def group(name) - Flipper.group(name) + @group_resolver.group(name) + end + + # Public: Register a group for this DSL's owning instance. + def register(name, &block) + @group_resolver.register(name, &block) + end + + # Public: Returns registered groups for this DSL's owning instance. + def groups + @group_resolver.groups + end + + # Public: Returns registered group names for this DSL's owning instance. + def group_names + @group_resolver.group_names + end + + # Public: Check if a group exists for this DSL's owning instance. + def group_exists?(name) + @group_resolver.group_exists?(name) + end + + # Public: Clear registered groups for this DSL's owning instance. + def unregister_groups + @group_resolver.unregister_groups end # Public: Gets the expression for the feature. diff --git a/lib/flipper/engine.rb b/lib/flipper/engine.rb index f1a1091c3..19a9f6c17 100644 --- a/lib/flipper/engine.rb +++ b/lib/flipper/engine.rb @@ -73,6 +73,26 @@ def self.default_strict_value config.use Flipper::Adapters::Strict, flipper.strict if flipper.strict config.use Flipper::Adapters::ActorLimit, flipper.actor_limit if flipper.actor_limit end + + if flipper.test_help + require "flipper/test_help" + Flipper::TestHelp.flipper_configure_named_instances + end + + if Flipper.configuration.respond_to?(:named_instance_names) + Flipper.configuration.named_instance_names.each do |name| + named = Flipper.configuration.named_configuration(name) + named.inherit_rails_configuration(flipper) + if named.cloud? && !flipper.test_help + named.resolve_cloud_credentials({ + token: app.credentials.dig(:flipper, name, :cloud_token), + sync_secret: app.credentials.dig(:flipper, name, :cloud_sync_secret), + }) + end + named.use Flipper::Adapters::Strict, named.strict if named.strict + named.use Flipper::Adapters::ActorLimit, named.actor_limit if named.actor_limit + end + end end initializer "flipper.memoizer", after: :load_config_initializers do |app| @@ -85,6 +105,54 @@ def self.default_strict_value if: flipper.memoize.respond_to?(:call) ? flipper.memoize : nil } end + + if Flipper.configuration.respond_to?(:named_instance_names) + named_configurations = Flipper.configuration.named_instance_names.map do |name| + Flipper.configuration.named_configuration(name) + end + + env_keys = [flipper.env_key] + named_configurations.map(&:env_key) + if env_keys.uniq.length != env_keys.length + raise InvalidConfigurationValue, "Flipper Rack environment keys must be unique" + end + + named_configurations.each do |named| + next unless named.memoize + + app.middleware.use Flipper::Middleware::SetupEnv, Flipper.named(named.name), { + env_key: named.env_key, + } + app.middleware.use Flipper::Middleware::Memoizer, { + env_key: named.env_key, + preload: named.preload, + if: named.memoize.respond_to?(:call) ? named.memoize : nil, + } + end + end + end + + initializer "flipper.named_cloud_paths", after: :load_config_initializers do |app| + next if app.config.flipper.test_help + next unless Flipper.configuration.respond_to?(:named_instance_names) + + named_paths = Flipper.configuration.named_instance_names.map do |name| + named = Flipper.configuration.named_configuration(name) + next unless named.cloud? && named.cloud_path + + sync_secret = named.resolve_cloud_credentials[:sync_secret] + named.cloud_path if sync_secret && !sync_secret.empty? + end.compact + default_paths = if cloud? && !ENV.fetch("FLIPPER_CLOUD_SYNC_SECRET", "").empty? + [app.config.flipper.cloud_path] + else + [] + end + paths = (default_paths + named_paths).map do |path| + path.to_s.sub(%r{\A/+}, "").sub(%r{/+\z}, "") + end + if paths.uniq.length != paths.length + raise InvalidConfigurationValue, "Flipper Cloud webhook paths must be unique" + end end initializer "flipper.test" do |app| diff --git a/lib/flipper/errors.rb b/lib/flipper/errors.rb index 14e5753d3..d429f0837 100644 --- a/lib/flipper/errors.rb +++ b/lib/flipper/errors.rb @@ -12,6 +12,15 @@ def initialize(actor) # Raised when attempting to declare a group name that has already been used. class DuplicateGroup < Error; end + # Raised when attempting to declare a named instance more than once. + class DuplicateNamedInstance < Error; end + + # Raised when a named instance has not been configured. + class NamedInstanceNotFound < Error; end + + # Raised when a named instance cannot be exposed as a Flipper method. + class InvalidNamedInstanceName < Error; end + # Raised when an invalid value is set to a configuration property class InvalidConfigurationValue < Flipper::Error def initialize(message = nil) diff --git a/lib/flipper/expressions/feature_enabled.rb b/lib/flipper/expressions/feature_enabled.rb index d13491aa4..024330067 100644 --- a/lib/flipper/expressions/feature_enabled.rb +++ b/lib/flipper/expressions/feature_enabled.rb @@ -9,24 +9,28 @@ def self.call(feature_name, context:) evaluating = Thread.current[EVALUATING_KEY] ||= Set.new feature_name = feature_name.to_s current_feature = context[:feature_name].to_s + instance_key = context[:flipper_instance_key] + feature_identity = instance_key ? [instance_key, feature_name] : feature_name + current_identity = instance_key ? [instance_key, current_feature] : current_feature # Track the current feature so A -> B -> A is caught - added_current = evaluating.add?(current_feature) + added_current = evaluating.add?(current_identity) begin # Circular dependency: return false to break the cycle - return false if evaluating.include?(feature_name) + return false if evaluating.include?(feature_identity) - evaluating.add(feature_name) + evaluating.add(feature_identity) actor = context[:actor] + feature_resolver = context.fetch(:feature_resolver, Flipper) if actor - Flipper.enabled?(feature_name, actor) + feature_resolver.enabled?(feature_name, actor) else - Flipper.enabled?(feature_name) + feature_resolver.enabled?(feature_name) end ensure - evaluating.delete(feature_name) - evaluating.delete(current_feature) if added_current + evaluating.delete(feature_identity) + evaluating.delete(current_identity) if added_current end end end diff --git a/lib/flipper/feature.rb b/lib/flipper/feature.rb index 2f16a802e..bfcb06697 100644 --- a/lib/flipper/feature.rb +++ b/lib/flipper/feature.rb @@ -34,6 +34,9 @@ def initialize(name, adapter, options = {}) @key = name.to_s @instrumenter = options.fetch(:instrumenter, Instrumenters::Noop) @adapter = adapter + @group_resolver = options.fetch(:group_resolver, Flipper) + @feature_resolver = options.fetch(:feature_resolver, Flipper) + @instance_key = options[:instance_key] end # Public: Enable this feature for something. @@ -98,7 +101,9 @@ def enabled?(*actors) context = FeatureCheckContext.new( feature_name: @name, values: gate_values, - actors: actors + actors: actors, + feature_resolver: @feature_resolver, + instance_key: @instance_key ) if open_gate = gates.detect { |gate| gate.open?(context) } @@ -157,7 +162,7 @@ def enable_actor(actor) # # Returns result of enable. def enable_group(group) - enable Types::Group.wrap(group) + enable wrap_group(group) end # Public: Enables a feature a percentage of time. @@ -231,7 +236,7 @@ def disable_actor(actor) # # Returns result of disable. def disable_group(group) - disable Types::Group.wrap(group) + disable wrap_group(group) end # Public: Disables a feature a percentage of time. @@ -295,7 +300,7 @@ def gate_values # # Returns Set of Flipper::Types::Group instances. def enabled_groups - groups_value.map { |name| Flipper.group(name) }.to_set + groups_value.map { |name| @group_resolver.group(name) }.to_set end alias_method :groups, :enabled_groups @@ -303,7 +308,7 @@ def enabled_groups # # Returns Set of Flipper::Types::Group instances. def disabled_groups - Flipper.groups - enabled_groups + @group_resolver.groups - enabled_groups end def expression @@ -416,7 +421,7 @@ def gates_hash actor: Gates::Actor.new, percentage_of_actors: Gates::PercentageOfActors.new, percentage_of_time: Gates::PercentageOfTime.new, - group: Gates::Group.new, + group: Gates::Group.new(group_resolver: @group_resolver), }.freeze end @@ -439,6 +444,12 @@ def gate_for(actor) private + def wrap_group(group) + return group if group.is_a?(Types::Group) + + @group_resolver.group(group) + end + # Internal: Mirrors a trusted remote expression during adapter sync, # including legacy expressions that predate empty-group validation. # diff --git a/lib/flipper/feature_check_context.rb b/lib/flipper/feature_check_context.rb index d8fa5594a..6d2aa8194 100644 --- a/lib/flipper/feature_check_context.rb +++ b/lib/flipper/feature_check_context.rb @@ -10,10 +10,18 @@ class FeatureCheckContext # Public: The actors we want to know if a feature is enabled for. attr_reader :actors - def initialize(feature_name:, values:, actors:) + # Internal: Resolver used by expressions that reference another feature. + attr_reader :feature_resolver + + # Internal: Identity used to isolate circular expression tracking. + attr_reader :instance_key + + def initialize(feature_name:, values:, actors:, feature_resolver: Flipper, instance_key: nil) @feature_name = feature_name @values = values @actors = actors + @feature_resolver = feature_resolver + @instance_key = instance_key end def actors? diff --git a/lib/flipper/gates/expression.rb b/lib/flipper/gates/expression.rb index c309e59c7..a02b90b2a 100644 --- a/lib/flipper/gates/expression.rb +++ b/lib/flipper/gates/expression.rb @@ -30,10 +30,22 @@ def open?(context) expression = Flipper::Expression.build(data) if context.actors.nil? || context.actors.empty? - !!expression.evaluate(feature_name: context.feature_name, properties: DEFAULT_PROPERTIES, actor: nil) + !!expression.evaluate( + feature_name: context.feature_name, + properties: DEFAULT_PROPERTIES, + actor: nil, + feature_resolver: context.feature_resolver, + flipper_instance_key: context.instance_key + ) else context.actors.any? do |actor| - !!expression.evaluate(feature_name: context.feature_name, properties: properties(actor), actor: actor) + !!expression.evaluate( + feature_name: context.feature_name, + properties: properties(actor), + actor: actor, + feature_resolver: context.feature_resolver, + flipper_instance_key: context.instance_key + ) end end end diff --git a/lib/flipper/gates/group.rb b/lib/flipper/gates/group.rb index 0f966b2ef..d2added87 100644 --- a/lib/flipper/gates/group.rb +++ b/lib/flipper/gates/group.rb @@ -1,6 +1,10 @@ module Flipper module Gates class Group < Gate + def initialize(group_resolver: Flipper) + @group_resolver = group_resolver + end + # Internal: The name of the gate. Used for instrumentation, etc. def name :group @@ -27,13 +31,15 @@ def open?(context) context.values.groups.any? do |name| context.actors.any? do |actor| - Flipper.group(name).match?(actor, context) + @group_resolver.group(name).match?(actor, context) end end end def wrap(thing) - Types::Group.wrap(thing) + return thing if thing.is_a?(Types::Group) + + @group_resolver.group(thing) end def protects?(thing) diff --git a/lib/flipper/named_configuration.rb b/lib/flipper/named_configuration.rb new file mode 100644 index 000000000..85f5d7c6f --- /dev/null +++ b/lib/flipper/named_configuration.rb @@ -0,0 +1,188 @@ +module Flipper + class NamedConfiguration < Configuration + attr_reader :name, :groups_registry + attr_accessor :memoize, :preload, :env_key, :strict, :actor_limit, :cloud_path + attr_reader :instrumenter + + def initialize(name) + super() + @name = name + @groups_registry = Registry.new + @version = 0 + @version_mutex = Mutex.new + @memoize = nil + @preload = nil + @env_key = "flipper_#{name}" + @strict = nil + @actor_limit = nil + @cloud_path = nil + @instrumenter = nil + @cloud = false + @cloud_options = {} + @resolved_cloud_options = nil + @default = -> { + Flipper.new(adapter, instrumenter: instrumenter || Instrumenters::Noop) + } + end + + def adapter(&block) + result = super + changed! if block_given? + result + end + + def named(*) + raise InvalidConfigurationValue, "Named Flipper instances cannot be nested" + end + + if RUBY_VERSION >= '3.0' + def use(klass, *args, **kwargs, &block) + result = super + changed! + result + end + else + def use(klass, *args, &block) + result = super + changed! + result + end + end + + def default(&block) + if block_given? + @cloud = false + @cloud_options = {} + @resolved_cloud_options = nil + self.cloud_path = nil + result = super + changed! + result + else + instance = super + unless instance.respond_to?(:instance_owner=) + raise InvalidConfigurationValue, "Named instance #{name.inspect} must return a Flipper DSL" + end + + instance.instance_owner = self + instance + end + end + + def version + @version_mutex.synchronize { @version } + end + + def instrumenter=(instrumenter) + @instrumenter = instrumenter + changed! + end + + # Public: Configure this named instance to use Flipper Cloud. + # + # Project credentials are resolved explicitly for this name and never + # fall back to the default FLIPPER_CLOUD_TOKEN or sync secret. + def cloud(options = {}) + options = options.dup + if options.key?(:local_adapter) + raise InvalidConfigurationValue, "Use the named adapter configuration instead of :local_adapter" + end + + self.cloud_path = options.delete(:path) if options.key?(:path) + self.instrumenter = options.delete(:instrumenter) if options.key?(:instrumenter) + @cloud = true + @cloud_options = options + @resolved_cloud_options = nil + @default = -> { + require "flipper/cloud" + resolved = resolve_cloud_credentials + Flipper::Cloud.new(resolved.merge( + local_adapter: adapter, + instrumenter: instrumenter || Instrumenters::Noop + )) + } + changed! + self + end + + def cloud? + @cloud + end + + # Internal: Resolve named credentials with explicit isolation from the + # existing default Cloud environment variables. + def resolve_cloud_credentials(credentials = {}, env = ENV) + return @resolved_cloud_options if @resolved_cloud_options + + prefix = "FLIPPER_CLOUD_#{name.to_s.upcase}" + token = cloud_value(:token, credentials, env["#{prefix}_TOKEN"]) + sync_secret = cloud_value(:sync_secret, credentials, env["#{prefix}_SYNC_SECRET"]) + + if token.nil? || token.empty? + raise InvalidConfigurationValue, + "Cloud token for named instance #{name.inspect} is missing; configure :token or #{prefix}_TOKEN" + end + + resolved = @cloud_options.merge( + token: token, + sync_secret: sync_secret + ) + @resolved_cloud_options = resolved unless credentials.empty? + resolved + end + + # Internal: Apply app-wide Rails settings that were not overridden by the + # named configuration. + def inherit_rails_configuration(flipper) + @memoize = flipper.memoize if @memoize.nil? + @preload = flipper.preload if @preload.nil? + @strict = flipper.strict if @strict.nil? + @actor_limit = flipper.actor_limit if @actor_limit.nil? + self.instrumenter = flipper.instrumenter if @instrumenter.nil? + self + end + + def register(name, &block) + group = Types::Group.new(name, &block) + groups_registry.add(group.name, group) + group + rescue Registry::DuplicateKey + raise DuplicateGroup, "Group #{name.inspect} has already been registered" + end + + def groups + groups_registry.values.to_set + end + + def group_names + groups_registry.keys.to_set + end + + def group_exists?(name) + groups_registry.key?(name) + end + + def group(name) + groups_registry.get(name) || Types::Group.new(name) + end + + def unregister_groups + groups_registry.clear + end + + private + + def cloud_value(key, credentials, env_value) + return @cloud_options[key] if @cloud_options.key?(key) + + credential_value = credentials[key] + return credential_value unless credential_value.nil? + + env_value + end + + def changed! + @version_mutex.synchronize { @version += 1 } + end + end +end diff --git a/lib/flipper/named_proxy.rb b/lib/flipper/named_proxy.rb new file mode 100644 index 000000000..62f3dc1ba --- /dev/null +++ b/lib/flipper/named_proxy.rb @@ -0,0 +1,64 @@ +module Flipper + class NamedProxy + attr_reader :name + + def initialize(name) + @name = name + end + + def instance + Flipper.send(:named_instance, name) + end + + def register(name, &block) + configuration.register(name, &block) + end + + def groups + configuration.groups + end + + def group_names + configuration.group_names + end + + def group_exists?(name) + configuration.group_exists?(name) + end + + def group(name) + configuration.group(name) + end + + def unregister_groups + configuration.unregister_groups + end + + def inspect + "#<#{self.class.name} name=#{name.inspect}>" + end + + def respond_to_missing?(method_name, include_private = false) + return false if method_name == :call + + instance.respond_to?(method_name, include_private) || super + end + + private + + def configuration + Flipper.send(:named_configuration, name) + end + + def method_missing(method_name, *args, **kwargs, &block) + target = instance + return super unless target.respond_to?(method_name) + + if kwargs.empty? + target.public_send(method_name, *args, &block) + else + target.public_send(method_name, *args, **kwargs, &block) + end + end + end +end diff --git a/lib/flipper/test_help.rb b/lib/flipper/test_help.rb index 88f8e3fe3..2cb1f7df0 100644 --- a/lib/flipper/test_help.rb +++ b/lib/flipper/test_help.rb @@ -10,6 +10,18 @@ def flipper_configure Flipper.configure do |config| config.adapter { adapter } config.default { Flipper.new(config.adapter) } + flipper_configure_named_instances(config) + end + end + + def flipper_configure_named_instances(config = Flipper.configuration) + return unless config.respond_to?(:named_instance_names) + + config.named_instance_names.each do |name| + named = config.named_configuration(name) + named_adapter = Flipper::Adapters::Memory.new + named.adapter { named_adapter } + named.default { Flipper.new(named.adapter) } end end @@ -17,8 +29,19 @@ def flipper_reset # Remove all features Flipper.features.each(&:remove) rescue nil + if Flipper.configuration.respond_to?(:named_instance_names) + Flipper.configuration.named_instance_names.each do |name| + named = Flipper.configuration.named_configuration(name) + named_adapter = Flipper::Adapters::Memory.new + named.adapter { named_adapter } + named.default { Flipper.new(named.adapter) } + Flipper.named(name).features.each(&:remove) rescue nil + end + end + # Reset previous DSL instance Flipper.instance = nil + Flipper.send(:reset_named_instances) end end end diff --git a/lib/flipper/ui/actions/groups_gate.rb b/lib/flipper/ui/actions/groups_gate.rb index ef5cf37a2..5a35fb963 100644 --- a/lib/flipper/ui/actions/groups_gate.rb +++ b/lib/flipper/ui/actions/groups_gate.rb @@ -22,7 +22,7 @@ def post feature = flipper[feature_name] value = params['value'].to_s.strip - if Flipper.group_exists?(value) + if flipper.group_exists?(value) case params['operation'] when 'enable' feature.enable_group value diff --git a/spec/flipper/api/v1/actions/groups_gate_spec.rb b/spec/flipper/api/v1/actions/groups_gate_spec.rb index f9f35054b..dd58fccc7 100644 --- a/spec/flipper/api/v1/actions/groups_gate_spec.rb +++ b/spec/flipper/api/v1/actions/groups_gate_spec.rb @@ -175,4 +175,20 @@ expect(flipper[:my_feature].groups_value).to be_empty end end + + context "with a named Flipper instance" do + let(:app) { build_api(Flipper.cross_app, env_key: "flipper_cross_app") } + + before do + Flipper.configure { |config| config.named(:cross_app) } + Flipper.cross_app.register(:named_group) { true } + post '/features/my_feature/groups', name: 'named_group' + end + + it "validates and enables groups from the named registry" do + expect(last_response.status).to eq(200) + expect(Flipper.cross_app[:my_feature].groups_value).to include("named_group") + expect(Flipper.group_exists?(:named_group)).to be(false) + end + end end diff --git a/spec/flipper/cloud/middleware_spec.rb b/spec/flipper/cloud/middleware_spec.rb index 794f4b3b9..0acc77cc5 100644 --- a/spec/flipper/cloud/middleware_spec.rb +++ b/spec/flipper/cloud/middleware_spec.rb @@ -66,6 +66,35 @@ end end + context "with a named Flipper instance" do + let(:app) { Flipper::Cloud.app(Flipper.cross_app, env_key: "flipper_cross_app") } + let(:signature) { + Flipper::Cloud::MessageVerifier.new(secret: "regular_tasty").generate(request_body, timestamp) + } + + before do + Flipper.register(:default_group) { false } + Flipper.configure do |config| + config.named(:cross_app).default { flipper } + end + Flipper.cross_app.register(:cross_app_group) { true } + end + + it "reports groups owned by the named instance" do + stub = stub_request_for_token("regular") + + post "/", request_body, { + "HTTP_FLIPPER_CLOUD_SIGNATURE" => signature_header_value, + } + + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)).to eq({ + "groups" => [{"name" => "cross_app_group"}], + }) + expect(stub).to have_been_requested + end + end + context 'when signature is invalid' do let(:app) { Flipper::Cloud.app(flipper) } let(:signature) { diff --git a/spec/flipper/engine_spec.rb b/spec/flipper/engine_spec.rb index d918031da..624d8d958 100644 --- a/spec/flipper/engine_spec.rb +++ b/spec/flipper/engine_spec.rb @@ -183,6 +183,325 @@ def initializers }) end + it "configures independent middleware and inherited defaults for a named instance" do + initializer do + Flipper.configure do |flipper_config| + flipper_config.named(:cross_app) do |named| + named.preload = [:chat] + end + end + end + + middleware = subject.middleware + named = Flipper.configuration.named_configuration(:cross_app) + setup = middleware.detect do |entry| + entry.klass == Flipper::Middleware::SetupEnv && entry.args.last[:env_key] == "flipper_cross_app" + end + memoizer = middleware.detect do |entry| + entry.klass == Flipper::Middleware::Memoizer && entry.args.first[:env_key] == "flipper_cross_app" + end + + expect(named.instrumenter).to be(ActiveSupport::Notifications) + expect(named.memoize).to be(true) + expect(named.preload).to eq([:chat]) + expect(named.actor_limit).to eq(100) + expect(setup.args.first).to be(Flipper.cross_app) + expect(memoizer.args.first).to eq({ + env_key: "flipper_cross_app", + preload: [:chat], + if: nil, + }) + expect(Flipper.cross_app.instance.instrumenter).to be(ActiveSupport::Notifications) + expect(Flipper.cross_app.adapter_stack).to include("actor_limit") + end + + it "memoizes and preloads the default and named instances during the same request" do + adapter_class = Class.new(Flipper::Adapters::Memory) do + attr_reader :get_all_calls + + def initialize + super + @get_all_calls = 0 + end + + def get_all(**_kwargs) + @get_all_calls += 1 + super + end + end + regular_adapter = adapter_class.new + cross_app_adapter = adapter_class.new + + initializer do + config.preload = true + Flipper.configure do |flipper_config| + flipper_config.adapter { regular_adapter } + flipper_config.named(:cross_app) do |named| + named.adapter { cross_app_adapter } + named.preload = true + end + end + end + + silence { application.initialize! } + endpoint = ->(env) { + body = JSON.generate({ + regular: Flipper.memoizing?, + cross_app: Flipper.cross_app.memoizing?, + named_env: env["flipper_cross_app"].equal?(Flipper.cross_app), + }) + [200, {Rack::CONTENT_TYPE => "application/json"}, [body]] + } + middleware = application.middleware.select do |entry| + [Flipper::Middleware::SetupEnv, Flipper::Middleware::Memoizer].include?(entry.klass) + end + rack_app = middleware.reverse.inject(endpoint) { |app, entry| entry.build(app) } + response = Rack::MockRequest.new(rack_app).get("/memoization") + + expect(response.status).to eq(200) + expect(JSON.parse(response.body)).to eq({ + "regular" => true, + "cross_app" => true, + "named_env" => true, + }) + expect(regular_adapter.get_all_calls).to eq(1) + expect(cross_app_adapter.get_all_calls).to eq(1) + expect(Flipper.memoizing?).to be(false) + expect(Flipper.cross_app.memoizing?).to be(false) + end + + it "stops memoizing default and named instances when a request raises" do + initializer do + Flipper.configure do |flipper_config| + flipper_config.named(:cross_app) + end + end + + silence { application.initialize! } + endpoint = ->(_env) { raise "request failed" } + middleware = application.middleware.select do |entry| + [Flipper::Middleware::SetupEnv, Flipper::Middleware::Memoizer].include?(entry.klass) + end + rack_app = middleware.reverse.inject(endpoint) { |app, entry| entry.build(app) } + + expect { Rack::MockRequest.new(rack_app).get("/memoization") }. + to raise_error("request failed") + expect(Flipper.memoizing?).to be(false) + expect(Flipper.cross_app.memoizing?).to be(false) + end + + it "allows a named instance to opt out of inherited memoization" do + initializer do + Flipper.configure do |flipper_config| + flipper_config.named(:cross_app) { |named| named.memoize = false } + end + end + + middleware = subject.middleware + + expect(middleware.none? do |entry| + entry.klass == Flipper::Middleware::SetupEnv && entry.args.last[:env_key] == "flipper_cross_app" + end).to be(true) + end + + it "does not construct a named adapter while Rails boots" do + constructions = 0 + initializer do + Flipper.configure do |flipper_config| + flipper_config.named(:cross_app) do |named| + named.adapter do + constructions += 1 + Flipper::Adapters::Memory.new + end + end + end + end + + subject + + expect(constructions).to eq(0) + Flipper.cross_app.instance + expect(constructions).to eq(1) + end + + it "rejects duplicate Rack environment keys" do + initializer do + Flipper.configure do |flipper_config| + flipper_config.named(:cross_app) { |named| named.env_key = "flipper" } + end + end + + expect { subject }. + to raise_error(Flipper::InvalidConfigurationValue, /environment keys must be unique/) + end + + it "boots test apps with named Cloud configured and no credentials" do + Rails.env = "test" + initializer do + Flipper.configure do |flipper_config| + flipper_config.named(:cross_app) do |named| + named.cloud(path: "_cross_app") + end + end + end + + expect { subject }.not_to raise_error + expect(Flipper.cross_app.instance).to be_instance_of(Flipper::DSL) + expect(Flipper.cross_app.enabled?(:chat)).to be(false) + end + + context "with named Cloud" do + let(:app) { application.routes } + let(:named_cloud_path) { "_cross_app" } + let(:named_sync_secret) { "named-secret" } + let(:named_cloud_options) { {path: named_cloud_path} } + let(:request_body) do + JSON.generate({ + "environment_id" => 1, + "webhook_id" => 1, + "delivery_id" => SecureRandom.uuid, + "action" => "sync", + }) + end + let(:timestamp) { Time.now } + let(:signature) do + Flipper::Cloud::MessageVerifier.new(secret: "named-secret").generate(request_body, timestamp) + end + let(:signature_header_value) do + Flipper::Cloud::MessageVerifier.new(secret: "header-secret").header(signature, timestamp) + end + + before do + ENV["FLIPPER_CLOUD_CROSS_APP_TOKEN"] = "named-token" + ENV["FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET"] = named_sync_secret + initializer do + Flipper.configure do |flipper_config| + flipper_config.named(:cross_app) do |named| + named.cloud(named_cloud_options) + named.register(:cross_app_group) { true } + end + end + end + end + + after do + ENV.delete("FLIPPER_CLOUD_CROSS_APP_TOKEN") + ENV.delete("FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET") + end + + it "mounts a separate webhook using the named credentials and groups" do + silence { application.initialize! } + stub = stub_request(:get, /features\?_cb=\d+&exclude_gate_names=true/).with({ + headers: { "flipper-cloud-token" => "named-token" }, + }).to_return(status: 200, body: JSON.generate({features: {}}), headers: {}) + + post "/_cross_app", request_body, { + "HTTP_FLIPPER_CLOUD_SIGNATURE" => signature_header_value, + } + + expect(last_response.status).to eq(200) + expect(JSON.parse(last_response.body)).to eq({ + "groups" => [{"name" => "cross_app_group"}], + }) + expect(stub).to have_been_requested + expect(Flipper.instance).to be_a(Flipper::DSL) + expect(Flipper.cross_app.instance).to be_a(Flipper::Cloud::DSL) + end + + context "when nested under the default Cloud path" do + let(:named_cloud_path) { "_flipper/cross_app" } + + before do + ENV["FLIPPER_CLOUD_TOKEN"] = "default-token" + ENV["FLIPPER_CLOUD_SYNC_SECRET"] = "default-secret" + end + + after do + ENV.delete("FLIPPER_CLOUD_TOKEN") + ENV.delete("FLIPPER_CLOUD_SYNC_SECRET") + end + + it "routes the more specific named webhook first" do + silence { application.initialize! } + stub = stub_request(:get, /features\?_cb=\d+&exclude_gate_names=true/).with({ + headers: { "flipper-cloud-token" => "named-token" }, + }).to_return(status: 200, body: JSON.generate({features: {}}), headers: {}) + + post "/_flipper/cross_app", request_body, { + "HTTP_FLIPPER_CLOUD_SIGNATURE" => signature_header_value, + } + + expect(last_response.status).to eq(200) + expect(stub).to have_been_requested + end + end + + context "when the path matches the default" do + let(:named_cloud_path) { "/_flipper/" } + + it "normalizes paths before checking uniqueness" do + ENV["FLIPPER_CLOUD_TOKEN"] = "default-token" + ENV["FLIPPER_CLOUD_SYNC_SECRET"] = "default-secret" + + expect { silence { application.initialize! } }. + to raise_error(Flipper::InvalidConfigurationValue, /webhook paths must be unique/) + ensure + ENV.delete("FLIPPER_CLOUD_TOKEN") + ENV.delete("FLIPPER_CLOUD_SYNC_SECRET") + end + end + + context "with an empty sync secret" do + let(:named_sync_secret) { "" } + + it "does not mount a webhook" do + silence { application.initialize! } + + post "/_cross_app", request_body, { + "HTTP_FLIPPER_CLOUD_SIGNATURE" => signature_header_value, + } + + expect(last_response.status).to eq(404) + end + end + + context "with a false sync secret" do + let(:named_cloud_options) { {path: named_cloud_path, sync_secret: false} } + + it "does not mount a webhook" do + silence { application.initialize! } + + post "/_cross_app", request_body, { + "HTTP_FLIPPER_CLOUD_SIGNATURE" => signature_header_value, + } + + expect(last_response.status).to eq(404) + end + end + end + + it "loads named Cloud credentials from the matching Rails credential scope" do + allow(application).to receive(:credentials).and_return({ + flipper: { + cross_app: { + cloud_token: "credentials-token", + cloud_sync_secret: "credentials-secret", + }, + }, + }) + initializer do + Flipper.configure do |flipper_config| + flipper_config.named(:cross_app) { |named| named.cloud } + end + end + + silence { application.initialize! } + cloud = Flipper.cross_app.instance.cloud_configuration + + expect(cloud.token).to eq("credentials-token") + expect(cloud.sync_secret).to eq("credentials-secret") + end + context "test_help" do it "is loaded if RAILS_ENV=test" do Rails.env = "test" diff --git a/spec/flipper/named_instances_spec.rb b/spec/flipper/named_instances_spec.rb new file mode 100644 index 000000000..932b7277d --- /dev/null +++ b/spec/flipper/named_instances_spec.rb @@ -0,0 +1,432 @@ +RSpec.describe "named Flipper instances" do + def with_env(values) + original = values.to_h { |key, _| [key, ENV[key]] } + values.each { |key, value| value.nil? ? ENV.delete(key) : ENV[key] = value } + yield + ensure + original.each { |key, value| value.nil? ? ENV.delete(key) : ENV[key] = value } + end + + def configure_named(name = :cross_app, adapter: Flipper::Adapters::Memory.new) + Flipper.configure do |config| + config.named(name) do |named| + named.adapter { adapter } + end + end + end + + it "keeps the existing default configuration and instance API unchanged" do + expect(Flipper.method(:configure).arity).to eq(0) + expect(Flipper.method(:configuration).arity).to eq(0) + expect(Flipper.method(:configuration=).arity).to eq(1) + expect(Flipper.method(:instance).arity).to eq(0) + expect(Flipper.method(:instance=).arity).to eq(1) + + default = Flipper.new(Flipper::Adapters::Memory.new) + Flipper.configure { |config| config.default { default } } + + expect(Flipper.instance).to be(default) + end + + it "configures a named child and exposes dynamic and direct access" do + named_configuration = nil + Flipper.configure do |config| + named_configuration = config.named(:cross_app) + end + + expect(named_configuration).to be_a(Flipper::NamedConfiguration) + expect(named_configuration.name).to eq(:cross_app) + expect(Flipper.named(:cross_app)).to be(Flipper.cross_app) + expect(Flipper.cross_app.instance).to be_a(Flipper::DSL) + end + + it "exposes direct access when registered on the active configuration" do + Flipper.configuration.named(:cross_app) + + expect(Flipper.cross_app).to be(Flipper.named(:cross_app)) + end + + it "isolates feature state from the default instance" do + configure_named + + Flipper.enable(:chat) + + expect(Flipper.enabled?(:chat)).to be(true) + expect(Flipper.cross_app.enabled?(:chat)).to be(false) + + Flipper.cross_app.enable(:chat) + + expect(Flipper.cross_app.enabled?(:chat)).to be(true) + end + + it "isolates group definitions from the default instance" do + configure_named + actor = Flipper::Actor.new("User;1") + Flipper.register(:beta) { false } + Flipper.cross_app.register(:beta) { true } + + Flipper.enable_group(:chat, :beta) + Flipper.cross_app.enable_group(:chat, :beta) + + expect(Flipper.enabled?(:chat, actor)).to be(false) + expect(Flipper.cross_app.enabled?(:chat, actor)).to be(true) + expect(Flipper.group(:beta)).not_to be(Flipper.cross_app.group(:beta)) + end + + it "shares named group definitions with every thread-local DSL" do + configure_named + actor = Flipper::Actor.new("User;1") + Flipper.cross_app.register(:beta) { true } + Flipper.cross_app.enable_group(:chat, :beta) + + expect(Thread.new { Flipper.cross_app.enabled?(:chat, actor) }.value).to be(true) + end + + it "keeps late default group registration visible to the existing default DSL" do + actor = Flipper::Actor.new("User;1") + default = Flipper.instance + Flipper.register(:beta) { true } + default.enable_group(:chat, :beta) + + expect(default.enabled?(:chat, actor)).to be(true) + end + + it "resolves feature expressions within the named instance" do + configure_named + Flipper.enable(:dependency) + Flipper.cross_app.enable_expression(:chat, Flipper.feature_enabled(:dependency)) + + expect(Flipper.cross_app.enabled?(:chat)).to be(false) + + Flipper.cross_app.enable(:dependency) + + expect(Flipper.cross_app.enabled?(:chat)).to be(true) + end + + it "keeps circular expression tracking isolated by named instance" do + configure_named + Flipper.enable(:dependency) + Flipper.cross_app.enable_expression(:chat, Flipper.feature_enabled(:dependency)) + evaluating = Thread.current[Flipper::Expressions::FeatureEnabled::EVALUATING_KEY] = Set.new(["dependency"]) + + expect(Flipper.cross_app.enabled?(:chat)).to be(false) + + Flipper.cross_app.enable(:dependency) + + expect(Flipper.cross_app.enabled?(:chat)).to be(true) + ensure + evaluating&.clear + end + + it "uses a separate DSL in each thread with a shared configured adapter" do + adapter = Flipper::Adapters::Memory.new + configure_named(adapter: adapter) + main_instance = Flipper.cross_app.instance + other_instance = Thread.new { Flipper.cross_app.instance }.value + + expect(other_instance).not_to be(main_instance) + expect(other_instance.adapter.adapter).to be(adapter) + expect(main_instance.adapter.adapter).to be(adapter) + end + + it "invalidates cached DSLs in other threads when named configuration changes" do + configure_named + named_configuration = Flipper.configuration.named_configuration(:cross_app) + ready = Queue.new + continue = Queue.new + + thread = Thread.new do + original = Flipper.cross_app.instance + ready << original + continue.pop + [original, Flipper.cross_app.instance] + end + + ready.pop + replacement = Flipper.new(Flipper::Adapters::Memory.new) + named_configuration.default { replacement } + continue << true + original, current = thread.value + + expect(current).not_to be(original) + expect(current).to be(replacement) + end + + it "does not publish a named instance until configuration finishes" do + adapter = Flipper::Adapters::Memory.new + configuring = Queue.new + continue = Queue.new + thread = Thread.new do + Flipper.configure do |config| + config.named(:cross_app) do |named| + configuring << true + continue.pop + named.adapter { adapter } + end + end + end + + configuring.pop + expect { Flipper.named(:cross_app) }. + to raise_error(Flipper::NamedInstanceNotFound) + + continue << true + thread.value + + expect(Flipper.cross_app.instance.adapter.adapter).to be(adapter) + ensure + continue << true if thread&.alive? + thread&.join + end + + it "releases a pending name when configuration is cancelled" do + configuring = Queue.new + continue = Queue.new + thread = Thread.new do + Flipper.configuration.named(:cross_app) do + configuring << true + continue.pop + end + end + + configuring.pop + thread.kill + thread.join + + expect { Flipper.configuration.named(:cross_app) }.not_to raise_error + expect(Flipper.cross_app).to be(Flipper.named(:cross_app)) + ensure + thread&.kill + continue << true if thread&.alive? + thread&.join + end + + it "keeps retained proxies current when configuration is replaced" do + configure_named + retained = Flipper.cross_app + original = retained.instance + + replacement_configuration = Flipper::Configuration.new + replacement = Flipper.new(Flipper::Adapters::Memory.new) + replacement_configuration.named(:cross_app).default { replacement } + Flipper.configuration = replacement_configuration + + expect(Flipper.cross_app).to be(retained) + expect(retained.instance).to be(replacement) + expect(retained.instance).not_to be(original) + end + + it "removes generated accessors when configuration is cleared" do + configure_named + expect(Flipper).to respond_to(:cross_app) + + Flipper.configuration = nil + + expect(Flipper).not_to respond_to(:cross_app) + expect { Flipper.named(:cross_app) }. + to raise_error(Flipper::NamedInstanceNotFound) + end + + it "keeps accessors consistent when configuration raises after registration" do + expect do + Flipper.configure do |config| + config.named(:cross_app) + raise "configuration failed" + end + end.to raise_error("configuration failed") + + expect(Flipper.configuration.named_instance_names).to include(:cross_app) + expect(Flipper.cross_app).to be(Flipper.named(:cross_app)) + end + + it "rolls back a named registration when its configuration block raises" do + expect do + Flipper.configure do |config| + config.named(:cross_app) { raise "named configuration failed" } + end + end.to raise_error("named configuration failed") + + expect(Flipper.configuration.named_instance_names).not_to include(:cross_app) + expect(Flipper).not_to respond_to(:cross_app) + end + + it "rejects duplicate names" do + expect do + Flipper.configure do |config| + config.named(:cross_app) + config.named(:cross_app) + end + end.to raise_error(Flipper::DuplicateNamedInstance) + end + + it "rejects nested named instances" do + named = Flipper.configuration.named(:cross_app) + + expect { named.named(:nested) }. + to raise_error(Flipper::InvalidConfigurationValue, /cannot be nested/) + end + + it "rejects invalid and conflicting names" do + expect { Flipper.configuration.named("cross-app") }. + to raise_error(Flipper::InvalidNamedInstanceName, /lowercase snake case/) + expect { Flipper.configuration.named(:configuration) }. + to raise_error(Flipper::InvalidNamedInstanceName, /existing Flipper method/) + expect { Flipper.configuration.named(:display) }. + to raise_error(Flipper::InvalidNamedInstanceName, /existing Flipper method/) + end + + it "does not break custom default configuration objects" do + custom = Object.new + default = Flipper.new(Flipper::Adapters::Memory.new) + custom.define_singleton_method(:default) { default } + + Flipper.configuration = custom + + expect(Flipper.instance).to be(default) + expect { Flipper.named(:cross_app) }. + to raise_error(Flipper::NamedInstanceNotFound) + end + + it "works without Cloud and does not require Cloud configuration" do + configure_named + + expect(Flipper.cross_app.instance.class).to be(Flipper::DSL) + end + + it "clears Cloud metadata when a custom default replaces Cloud" do + named = Flipper.configuration.named(:cross_app) + named.cloud(token: "cloud-token", sync_secret: "cloud-secret", path: "_cross_app") + replacement = Flipper.new(Flipper::Adapters::Memory.new) + + named.default { replacement } + + expect(named.cloud?).to be(false) + expect(named.cloud_path).to be(nil) + expect(Flipper.cross_app.instance).to be(replacement) + end + + it "honors an instrumenter configured through named Cloud options" do + instrumenter = Object.new + Flipper.configure do |config| + config.named(:cross_app) do |named| + named.cloud(token: "cloud-token", sync_secret: "cloud-secret", instrumenter: instrumenter) + end + end + + cloud_instrumenter = Flipper.cross_app.instance.cloud_configuration.instrumenter + + expect(cloud_instrumenter.instrumenter).to be(instrumenter) + end + + it "isolates Cloud workers when endpoint strings overlap" do + stub_request(:get, %r{\Ahttps://example\.test/a(?:b)?/features}). + to_return(status: 200, body: JSON.generate({features: {}})) + Flipper.configure do |config| + config.named(:people) do |named| + named.cloud(token: "bc", url: "https://example.test/a") + end + config.named(:cross_app) do |named| + named.cloud(token: "c", url: "https://example.test/ab") + end + config.named(:people_copy) do |named| + named.cloud(token: "bc", url: "https://example.test/a") + end + end + + people = Flipper.people.instance.cloud_configuration + cross_app = Flipper.cross_app.instance.cloud_configuration + people_copy = Flipper.people_copy.instance.cloud_configuration + + expect(people.send(:poller)).not_to be(cross_app.send(:poller)) + expect(people.telemetry).not_to be(cross_app.telemetry) + expect(people.send(:poller)).to be(people_copy.send(:poller)) + expect(people.telemetry).to be(people_copy.telemetry) + end + + it "uses only name-scoped environment credentials for named Cloud" do + with_env( + "FLIPPER_CLOUD_TOKEN" => "default-token", + "FLIPPER_CLOUD_SYNC_SECRET" => "default-secret", + "FLIPPER_CLOUD_CROSS_APP_TOKEN" => "named-token", + "FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET" => "named-secret" + ) do + Flipper.configure do |config| + config.named(:cross_app) { |named| named.cloud } + end + + cloud = Flipper.cross_app.instance.cloud_configuration + + expect(cloud.token).to eq("named-token") + expect(cloud.sync_secret).to eq("named-secret") + end + end + + it "never falls back to default Cloud credentials for a named instance" do + with_env( + "FLIPPER_CLOUD_TOKEN" => "default-token", + "FLIPPER_CLOUD_SYNC_SECRET" => "default-secret", + "FLIPPER_CLOUD_CROSS_APP_TOKEN" => nil, + "FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET" => nil + ) do + Flipper.configure do |config| + config.named(:cross_app) { |named| named.cloud } + end + + expect { Flipper.cross_app.instance }. + to raise_error(Flipper::InvalidConfigurationValue, /CROSS_APP_TOKEN/) + end + end + + it "prefers explicit named Cloud credentials over scoped environment credentials" do + with_env( + "FLIPPER_CLOUD_CROSS_APP_TOKEN" => "environment-token", + "FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET" => "environment-secret" + ) do + Flipper.configure do |config| + config.named(:cross_app) do |named| + named.cloud(token: "explicit-token", sync_secret: "explicit-secret") + end + end + + cloud = Flipper.cross_app.instance.cloud_configuration + + expect(cloud.token).to eq("explicit-token") + expect(cloud.sync_secret).to eq("explicit-secret") + end + end + + it "prefers named Rails-style credentials over scoped environment credentials" do + with_env( + "FLIPPER_CLOUD_CROSS_APP_TOKEN" => "environment-token", + "FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET" => "environment-secret" + ) do + named = Flipper.configuration.named(:cross_app) + named.cloud + + resolved = named.resolve_cloud_credentials({ + token: "credentials-token", + sync_secret: "credentials-secret", + }) + + expect(resolved[:token]).to eq("credentials-token") + expect(resolved[:sync_secret]).to eq("credentials-secret") + end + end + + it "preserves a false named Rails-style sync secret" do + with_env( + "FLIPPER_CLOUD_CROSS_APP_TOKEN" => "environment-token", + "FLIPPER_CLOUD_CROSS_APP_SYNC_SECRET" => "environment-secret" + ) do + named = Flipper.configuration.named(:cross_app) + named.cloud + + resolved = named.resolve_cloud_credentials({ + token: "credentials-token", + sync_secret: false, + }) + + expect(resolved[:sync_secret]).to be(false) + end + end +end diff --git a/spec/flipper/test_help_spec.rb b/spec/flipper/test_help_spec.rb new file mode 100644 index 000000000..b5ba23e97 --- /dev/null +++ b/spec/flipper/test_help_spec.rb @@ -0,0 +1,66 @@ +RSpec.describe Flipper::TestHelp do + it "uses a separate shared Memory adapter for each named instance" do + Flipper.configure do |config| + config.named(:cross_app) + config.named(:internal) + end + + described_class.flipper_configure + Flipper.cross_app.enable(:chat) + + expect(Thread.new { Flipper.cross_app.enabled?(:chat) }.value).to be(true) + expect(Flipper.internal.enabled?(:chat)).to be(false) + expect(Flipper.enabled?(:chat)).to be(false) + end + + it "replaces a named Cloud default without contacting Cloud" do + Flipper.configure do |config| + config.named(:cross_app) do |named| + named.cloud(token: "cloud-token", sync_secret: "cloud-secret") + end + end + + described_class.flipper_configure + + expect(Flipper.cross_app.instance.class).to be(Flipper::DSL) + expect(a_request(:any, /flippercloud/)).not_to have_been_made + end + + it "replaces a named polling Cloud instance added after test setup" do + described_class.flipper_configure + Flipper.configure do |config| + config.named(:cross_app) do |named| + named.cloud(token: "cloud-token", sync_secret: "") + end + end + + described_class.flipper_reset + + expect(Flipper.cross_app.instance.class).to be(Flipper::DSL) + expect(a_request(:any, /flippercloud/)).not_to have_been_made + end + + it "shares Memory for a non-Cloud named instance added after test setup" do + described_class.flipper_configure + Flipper.configure { |config| config.named(:cross_app) } + + described_class.flipper_reset + Flipper.cross_app.enable(:chat) + + expect(Thread.new { Flipper.cross_app.enabled?(:chat) }.value).to be(true) + end + + it "clears named features while preserving registered groups" do + Flipper.configure do |config| + config.named(:cross_app) + end + Flipper.cross_app.register(:beta) { true } + described_class.flipper_configure + Flipper.cross_app.enable(:chat) + + described_class.flipper_reset + + expect(Flipper.cross_app.enabled?(:chat)).to be(false) + expect(Flipper.cross_app.group_exists?(:beta)).to be(true) + end +end diff --git a/spec/flipper/ui/actions/groups_gate_spec.rb b/spec/flipper/ui/actions/groups_gate_spec.rb index 283beb83d..1d71a0815 100644 --- a/spec/flipper/ui/actions/groups_gate_spec.rb +++ b/spec/flipper/ui/actions/groups_gate_spec.rb @@ -141,4 +141,22 @@ end end end + + context "with a named Flipper instance" do + let(:app) { build_app(Flipper.cross_app, env_key: "flipper_cross_app") } + + before do + Flipper.configure { |config| config.named(:cross_app) } + Flipper.cross_app.register(:named_group) { true } + post 'features/search/groups', + { 'value' => 'named_group', 'operation' => 'enable', 'authenticity_token' => token }, + 'rack.session' => session + end + + it "validates and enables groups from the named registry" do + expect(last_response.status).to be(302) + expect(Flipper.cross_app[:search].groups_value).to include("named_group") + expect(Flipper.group_exists?(:named_group)).to be(false) + end + end end diff --git a/test/adapters/active_record_test.rb b/test/adapters/active_record_test.rb index ca1e04fd8..734bdc7ff 100644 --- a/test/adapters/active_record_test.rb +++ b/test/adapters/active_record_test.rb @@ -75,12 +75,19 @@ def test_table_prefix_uses_internal_model_subclasses assert_equal "cross_product_flipper_features", feature_class.table_name assert_equal "cross_product_flipper_gates", gate_class.table_name - flipper = Flipper.new(adapter) - flipper[:search].enable - - assert flipper[:search].enabled? - assert_equal ["search"], feature_class.pluck(:key) - assert_equal [["search", "boolean", "true"]], gate_class.pluck(:feature_key, :key, :value) + regular_flipper = Flipper.new(@adapter) + cross_product_flipper = Flipper.new(adapter) + + regular_flipper[:product_only].enable + cross_product_flipper[:cross_product_only].enable + + assert regular_flipper[:product_only].enabled? + refute regular_flipper[:cross_product_only].enabled? + assert cross_product_flipper[:cross_product_only].enabled? + refute cross_product_flipper[:product_only].enabled? + assert_equal ["product_only"], Flipper::Adapters::ActiveRecord::Feature.pluck(:key) + assert_equal ["cross_product_only"], feature_class.pluck(:key) + assert_equal [["cross_product_only", "boolean", "true"]], gate_class.pluck(:feature_key, :key, :value) end def test_models_honor_table_name_prefixes_and_suffixes