Skip to content

Commit 00a4564

Browse files
Gerghsinn0
andauthored
Show warning message if setting deployment configured feature flag via API (round 2) (#4540)
* feat: Platform Engineer sees a warning when specifying deployment-configured feature flags - Added a warning message to present when a deployment-configured feature flag is updated via API. - TNZ-48303 * Update interface for checking if feature flag set - `FeatureFlag.config_overridden?` now takes a symbol - Backfill tests to cover "unset" case. This hopefully shouldn't happen in real life. It's worth covering anyway, especially since the `nil` guard is already implemented. --------- Co-authored-by: Hongchol Sinn <hongchol.sinn@broadcom.com>
1 parent c00ebc1 commit 00a4564

File tree

4 files changed

+84
-0
lines changed

4 files changed

+84
-0
lines changed

app/controllers/v3/feature_flags_controller.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
require 'fetchers/feature_flag_list_fetcher'
77

88
class FeatureFlagsController < ApplicationController
9+
OVERRIDE_IN_MANIFEST_MSG = 'The feature flag has an override configured in the bosh manifest and can be overwritten when the deployment is updated.'.freeze
10+
911
def index
1012
message = FeatureFlagsListMessage.from_params(query_params)
1113
invalid_param!(message.errors.full_messages) unless message.valid?
@@ -38,6 +40,7 @@ def update
3840
unprocessable!(message.errors.full_messages) unless message.valid?
3941

4042
flag = VCAP::CloudController::FeatureFlagUpdate.new.update(flag, message)
43+
add_warning_headers([OVERRIDE_IN_MANIFEST_MSG]) if VCAP::CloudController::FeatureFlag.config_overridden?(hashed_params[:name].to_sym)
4144
render status: :ok, json: Presenters::V3::FeatureFlagPresenter.new(flag)
4245
rescue FeatureFlagUpdate::Error => e
4346
unprocessable!(e)

app/models/runtime/feature_flag.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ class UndefinedFeatureFlagError < StandardError
3737

3838
ADMIN_READ_ONLY_SKIPPABLE = [:space_developer_env_var_visibility].freeze
3939

40+
@feature_flag_overrides = nil
41+
4042
export_attributes :name, :enabled, :error_message
4143
import_attributes :name, :enabled, :error_message
4244

@@ -104,6 +106,12 @@ def self.override_default_flags(feature_flag_overrides)
104106
feature_flag.enabled = flag_value
105107
feature_flag.save
106108
end
109+
110+
@feature_flag_overrides = feature_flag_overrides
111+
end
112+
113+
def self.config_overridden?(feature_flag_name)
114+
@feature_flag_overrides && @feature_flag_overrides.keys.include?(feature_flag_name)
107115
end
108116

109117
private_class_method :admin?

spec/unit/controllers/v3/feature_flags_controller_spec.rb

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,51 @@
230230
expect(parsed_body['custom_error_message']).to be_nil
231231
end
232232
end
233+
234+
context 'when there are overrides in configuration' do
235+
before do
236+
stub_const('VCAP::CloudController::FeatureFlag::DEFAULT_FLAGS', {
237+
flag1: false, flag2: true, flag3: true, flag4: false
238+
})
239+
VCAP::CloudController::FeatureFlag.override_default_flags({ flag1: false, flag4: true })
240+
end
241+
242+
it 'returns a warning message for the overridden flags' do
243+
patch :update, params: {
244+
name: 'flag1',
245+
enabled: false
246+
}, as: :json
247+
248+
expect(response).to have_http_status :ok
249+
expect(response).to have_warning_message FeatureFlagsController::OVERRIDE_IN_MANIFEST_MSG
250+
251+
patch :update, params: {
252+
name: 'flag4',
253+
enabled: false
254+
}, as: :json
255+
256+
expect(response).to have_http_status :ok
257+
expect(response).to have_warning_message FeatureFlagsController::OVERRIDE_IN_MANIFEST_MSG
258+
end
259+
260+
it 'returns no warning message for not overridden flags' do
261+
patch :update, params: {
262+
name: 'flag2',
263+
enabled: false
264+
}, as: :json
265+
266+
expect(response).to have_http_status :ok
267+
expect(response.headers['X-Cf-Warnings']).to be_nil
268+
269+
patch :update, params: {
270+
name: 'flag3',
271+
enabled: false
272+
}, as: :json
273+
274+
expect(response).to have_http_status :ok
275+
expect(response.headers['X-Cf-Warnings']).to be_nil
276+
end
277+
end
233278
end
234279
end
235280
end

spec/unit/models/runtime/feature_flag_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,5 +332,33 @@ module VCAP::CloudController
332332
end
333333
end
334334
end
335+
336+
describe '.config_overridden?' do
337+
context 'when overrides have not been set yet' do
338+
before do
339+
FeatureFlag.instance_variable_set(:@feature_flag_overrides, nil)
340+
end
341+
342+
it 'returns nil' do
343+
expect(FeatureFlag.config_overridden?(:diego_docker)).to be_nil
344+
end
345+
end
346+
347+
context 'when overrides have been set' do
348+
let(:default_diego_docker_value) { FeatureFlag::DEFAULT_FLAGS[:diego_docker] }
349+
350+
before do
351+
FeatureFlag.override_default_flags({ diego_docker: !default_diego_docker_value })
352+
end
353+
354+
it 'return true for the overriden flag' do
355+
expect(FeatureFlag.config_overridden?(:diego_docker)).to be true
356+
end
357+
358+
it 'return false for other flags' do
359+
expect(FeatureFlag.config_overridden?(:potato)).to be false
360+
end
361+
end
362+
end
335363
end
336364
end

0 commit comments

Comments
 (0)