From e256adfb3719ced95f2d3d65ecca40f2eafca1a4 Mon Sep 17 00:00:00 2001
From: Dmitry Gruzd <dgruzd@gitlab.com>
Date: Wed, 12 Mar 2025 14:06:43 +0100
Subject: [PATCH] Zoekt: Fix undefined feature flag handling in
gitlab:zoekt:info
Changelog: fixed
EE: true
---
ee/app/services/search/zoekt/info_service.rb | 19 +++++--
.../search/zoekt/info_service_spec.rb | 49 +++++++++++++++++--
2 files changed, 59 insertions(+), 9 deletions(-)
diff --git a/ee/app/services/search/zoekt/info_service.rb b/ee/app/services/search/zoekt/info_service.rb
index 155bd8397264e..e5648338810bf 100644
--- a/ee/app/services/search/zoekt/info_service.rb
+++ b/ee/app/services/search/zoekt/info_service.rb
@@ -48,10 +48,16 @@ def log_custom_feature_flags
# Get the state of each persisted flag
flag_states = {}
persisted_flags.each do |flag|
- enabled = Feature.enabled?(flag, Feature.current_request)
- state_text = enabled ? 'enabled' : 'disabled'
- state_color = enabled ? :green : :red
- flag_states[flag] = Rainbow(state_text).color(state_color)
+ # Skip flags without YAML definitions to avoid InvalidFeatureFlagError
+ if Feature::Definition.has_definition?(flag.to_sym)
+ enabled = Feature.enabled?(flag, Feature.current_request)
+ state_text = enabled ? 'enabled' : 'disabled'
+ state_color = enabled ? :green : :red
+ flag_states[flag] = Rainbow(state_text).color(state_color)
+ else
+ # Mark flags without definitions
+ flag_states[flag] = Rainbow('no definition').yellow
+ end
end
# Sort flags alphabetically and log each flag and its state
@@ -62,7 +68,7 @@ def log_custom_feature_flags
# Display default feature flags section - shows all flags using default values
def log_default_feature_flags
- # Get all zoekt-related feature flags
+ # Get all zoekt-related feature flags that have YAML definitions
all_flags = Feature::Definition.definitions.keys
zoekt_flags = all_flags.select { |name| name.to_s.start_with?('zoekt_') }
@@ -77,6 +83,9 @@ def log_default_feature_flags
# Get the state of each default flag
flag_states = {}
default_flags.each do |flag|
+ # Skip flags without YAML definitions to avoid errors
+ next unless Feature::Definition.has_definition?(flag)
+
enabled = Feature.enabled?(flag, Feature.current_request)
state_text = enabled ? 'enabled' : 'disabled'
state_color = enabled ? :green : :red
diff --git a/ee/spec/services/search/zoekt/info_service_spec.rb b/ee/spec/services/search/zoekt/info_service_spec.rb
index 86b5717e863ae..7a820f95d82e8 100644
--- a/ee/spec/services/search/zoekt/info_service_spec.rb
+++ b/ee/spec/services/search/zoekt/info_service_spec.rb
@@ -235,9 +235,18 @@
allow(Feature).to receive(:persisted_names).and_return(['zoekt_custom_flag'])
zoekt_flag = instance_double(Feature::Definition, to_s: 'zoekt_default_flag')
- allow(Feature::Definition).to receive(:definitions).and_return({
- 'zoekt_default_flag' => zoekt_flag
- })
+
+ # Combine all Feature::Definition stubs
+ allow(Feature::Definition).to receive_messages(
+ definitions: { 'zoekt_default_flag' => zoekt_flag },
+ has_definition?: false
+ )
+
+ # Set up specific has_definition? calls that override the default
+ allow(Feature::Definition).to receive(:has_definition?).with(:zoekt_custom_flag).and_return(true)
+ allow(Feature::Definition).to receive(:has_definition?).with('zoekt_custom_flag').and_return(true)
+ allow(Feature::Definition).to receive(:has_definition?).with(:zoekt_default_flag).and_return(true)
+ allow(Feature::Definition).to receive(:has_definition?).with('zoekt_default_flag').and_return(true)
allow(Feature).to receive(:persisted_name?).with('zoekt_default_flag').and_return(false)
allow(Feature).to receive(:enabled?).with('zoekt_custom_flag', nil).and_return(true)
@@ -265,7 +274,10 @@
persisted_name?: false,
enabled?: false
)
- allow(Feature::Definition).to receive(:definitions).and_return({})
+ allow(Feature::Definition).to receive_messages(
+ definitions: {},
+ has_definition?: false
+ )
end
it 'displays empty feature flags sections' do
@@ -283,6 +295,35 @@
expect(logger).to have_received(:info).with(/Feature flags:.+#{Rainbow('none').yellow}/).at_least(:once)
end
end
+
+ context 'with feature flags without YAML definitions' do
+ before do
+ allow(Feature).to receive(:persisted_names).and_return(%w[zoekt_custom_flag zoekt_undefined_flag])
+
+ # Set up has_definition? to return appropriate values for both string and symbol keys
+ allow(Feature::Definition).to receive(:has_definition?).and_return(false)
+ allow(Feature::Definition).to receive(:has_definition?).with(:zoekt_custom_flag).and_return(true)
+ allow(Feature::Definition).to receive(:has_definition?).with('zoekt_custom_flag').and_return(true)
+ allow(Feature::Definition).to receive(:has_definition?).with(:zoekt_undefined_flag).and_return(false)
+ allow(Feature::Definition).to receive(:has_definition?).with('zoekt_undefined_flag').and_return(false)
+ allow(Feature::Definition).to receive(:has_definition?).with(:zoekt_default_flag).and_return(true)
+ allow(Feature::Definition).to receive(:has_definition?).with('zoekt_default_flag').and_return(true)
+
+ # We should never call enabled? on the undefined flag
+ allow(Feature).to receive(:enabled?).with('zoekt_undefined_flag', nil)
+ .and_raise('This should not be called')
+ end
+
+ it 'displays flags without definitions as "no definition"' do
+ service.execute
+
+ # The flag with no definition should show up in the output
+ expect(logger).to have_received(:info).with(/- zoekt_undefined_flag:.+#{Rainbow('no definition').yellow}/)
+
+ # The regular flag should still work
+ expect(logger).to have_received(:info).with(/- zoekt_custom_flag:.+#{Rainbow('enabled').green}/)
+ end
+ end
end
end
end
--
GitLab