diff --git a/app/controllers/v3/feature_flags_controller.rb b/app/controllers/v3/feature_flags_controller.rb index f68067968e2..1ae35f1cad2 100644 --- a/app/controllers/v3/feature_flags_controller.rb +++ b/app/controllers/v3/feature_flags_controller.rb @@ -6,6 +6,8 @@ require 'fetchers/feature_flag_list_fetcher' class FeatureFlagsController < ApplicationController + 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 + def index message = FeatureFlagsListMessage.from_params(query_params) invalid_param!(message.errors.full_messages) unless message.valid? @@ -38,6 +40,7 @@ def update unprocessable!(message.errors.full_messages) unless message.valid? flag = VCAP::CloudController::FeatureFlagUpdate.new.update(flag, message) + add_warning_headers([OVERRIDE_IN_MANIFEST_MSG]) if VCAP::CloudController::FeatureFlag.config_overridden?(hashed_params[:name]) render status: :ok, json: Presenters::V3::FeatureFlagPresenter.new(flag) rescue FeatureFlagUpdate::Error => e unprocessable!(e) diff --git a/app/models/runtime/feature_flag.rb b/app/models/runtime/feature_flag.rb index e5d4fc634f7..3621990c631 100644 --- a/app/models/runtime/feature_flag.rb +++ b/app/models/runtime/feature_flag.rb @@ -37,6 +37,8 @@ class UndefinedFeatureFlagError < StandardError ADMIN_READ_ONLY_SKIPPABLE = [:space_developer_env_var_visibility].freeze + @feature_flag_overrides = nil + export_attributes :name, :enabled, :error_message import_attributes :name, :enabled, :error_message @@ -86,6 +88,32 @@ def self.admin_read_only? VCAP::CloudController::SecurityContext.admin_read_only? end + def self.override_default_flags(feature_flag_overrides) + invalid_keys = feature_flag_overrides.keys.to_set - FeatureFlag::DEFAULT_FLAGS.keys.to_set + raise "Invalid feature flag name(s): #{invalid_keys.to_a}" if invalid_keys.any? + + invalid_values = feature_flag_overrides.reject { |_, v| v.is_a?(TrueClass) || v.is_a?(FalseClass) } + raise "Invalid feature flag value(s): #{invalid_values}" if invalid_values.any? + + feature_flag_overrides.each do |flag_name, flag_value| + feature_flag = FeatureFlag.find(name: flag_name.to_s) + if feature_flag + next if feature_flag.enabled == flag_value + else + feature_flag = FeatureFlag.new(name: flag_name.to_s) + end + + feature_flag.enabled = flag_value + feature_flag.save + end + + @feature_flag_overrides = feature_flag_overrides + end + + def self.config_overridden?(feature_flag_name) + @feature_flag_overrides && @feature_flag_overrides.keys.include?(feature_flag_name.to_sym) + end + private_class_method :admin? end end diff --git a/config/initializers/feature_flag_overrides.rb b/config/initializers/feature_flag_overrides.rb new file mode 100644 index 00000000000..79212efa4de --- /dev/null +++ b/config/initializers/feature_flag_overrides.rb @@ -0,0 +1,9 @@ +module CCInitializers + def self.feature_flag_overrides(cc_config) + @logger ||= Steno.logger('cc.feature_flag_overrides') + @logger.info("Initializing feature_flag_overrides: #{cc_config[:feature_flag_overrides]}") + return unless cc_config[:feature_flag_overrides] + + VCAP::CloudController::FeatureFlag.override_default_flags(cc_config[:feature_flag_overrides]) + end +end diff --git a/lib/cloud_controller/config_schemas/api_schema.rb b/lib/cloud_controller/config_schemas/api_schema.rb index 8eb4be21525..82a56fed048 100644 --- a/lib/cloud_controller/config_schemas/api_schema.rb +++ b/lib/cloud_controller/config_schemas/api_schema.rb @@ -426,7 +426,8 @@ class ApiSchema < VCAP::Config update_metric_tags_on_rename: bool, app_instance_stopping_state: bool, - optional(:enable_ipv6) => bool + optional(:enable_ipv6) => bool, + optional(:feature_flag_overrides) => Hash } end # rubocop:enable Metrics/BlockLength diff --git a/lib/cloud_controller/config_schemas/clock_schema.rb b/lib/cloud_controller/config_schemas/clock_schema.rb index c2499cd23e2..175f3be5fcc 100644 --- a/lib/cloud_controller/config_schemas/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/clock_schema.rb @@ -218,7 +218,9 @@ class ClockSchema < VCAP::Config max_labels_per_resource: Integer, max_annotations_per_resource: Integer, - custom_metric_tag_prefix_list: Array + custom_metric_tag_prefix_list: Array, + + optional(:feature_flag_overrides) => Hash } end # rubocop:enable Metrics/BlockLength diff --git a/lib/cloud_controller/config_schemas/deployment_updater_schema.rb b/lib/cloud_controller/config_schemas/deployment_updater_schema.rb index d0b18faeb69..4f6fef2e163 100644 --- a/lib/cloud_controller/config_schemas/deployment_updater_schema.rb +++ b/lib/cloud_controller/config_schemas/deployment_updater_schema.rb @@ -164,7 +164,9 @@ class DeploymentUpdaterSchema < VCAP::Config max_labels_per_resource: Integer, max_annotations_per_resource: Integer, - custom_metric_tag_prefix_list: Array + custom_metric_tag_prefix_list: Array, + + optional(:feature_flag_overrides) => Hash } end # rubocop:enable Metrics/BlockLength diff --git a/lib/cloud_controller/config_schemas/worker_schema.rb b/lib/cloud_controller/config_schemas/worker_schema.rb index 86053bbff69..c92475602bf 100644 --- a/lib/cloud_controller/config_schemas/worker_schema.rb +++ b/lib/cloud_controller/config_schemas/worker_schema.rb @@ -223,7 +223,9 @@ class WorkerSchema < VCAP::Config max_labels_per_resource: Integer, max_annotations_per_resource: Integer, custom_metric_tag_prefix_list: Array, - default_app_lifecycle: enum('buildpack', 'cnb') + default_app_lifecycle: enum('buildpack', 'cnb'), + + optional(:feature_flag_overrides) => Hash } end # rubocop:enable Metrics/BlockLength diff --git a/spec/unit/controllers/v3/feature_flags_controller_spec.rb b/spec/unit/controllers/v3/feature_flags_controller_spec.rb index 6d59fa1c2de..91b86223c1d 100644 --- a/spec/unit/controllers/v3/feature_flags_controller_spec.rb +++ b/spec/unit/controllers/v3/feature_flags_controller_spec.rb @@ -230,6 +230,51 @@ expect(parsed_body['custom_error_message']).to be_nil end end + + context 'when there are overrides in configuration' do + before do + stub_const('VCAP::CloudController::FeatureFlag::DEFAULT_FLAGS', { + flag1: false, flag2: true, flag3: true, flag4: false + }) + VCAP::CloudController::FeatureFlag.override_default_flags({ flag1: false, flag4: true }) + end + + it 'returns a warning message for the overridden flags' do + patch :update, params: { + name: 'flag1', + enabled: false + }, as: :json + + expect(response).to have_http_status :ok + expect(response).to have_warning_message FeatureFlagsController::OVERRIDE_IN_MANIFEST_MSG + + patch :update, params: { + name: 'flag4', + enabled: false + }, as: :json + + expect(response).to have_http_status :ok + expect(response).to have_warning_message FeatureFlagsController::OVERRIDE_IN_MANIFEST_MSG + end + + it 'returns no warning message for not overridden flags' do + patch :update, params: { + name: 'flag2', + enabled: false + }, as: :json + + expect(response).to have_http_status :ok + expect(response.headers['X-Cf-Warnings']).to be_nil + + patch :update, params: { + name: 'flag3', + enabled: false + }, as: :json + + expect(response).to have_http_status :ok + expect(response.headers['X-Cf-Warnings']).to be_nil + end + end end end end diff --git a/spec/unit/models/runtime/feature_flag_spec.rb b/spec/unit/models/runtime/feature_flag_spec.rb index 12a1e0a1449..82dea09e712 100644 --- a/spec/unit/models/runtime/feature_flag_spec.rb +++ b/spec/unit/models/runtime/feature_flag_spec.rb @@ -225,5 +225,112 @@ module VCAP::CloudController end end end + + describe 'default flag override in config' do + let(:key) { :diego_docker } + let(:default_value) { FeatureFlag::DEFAULT_FLAGS[key] } + + context 'when there was no previously set conflicting value' do + let(:config_value) { !default_value } + + before do + FeatureFlag.override_default_flags({ key => config_value }) + end + + context 'and the value is not changed by admin' do + it 'returns the config-set value' do + expect(FeatureFlag.enabled?(key)).to be config_value + end + end + + context 'and the value is changed by admin' do + let(:admin_value) { !config_value } + let(:admin_override) do + flag = FeatureFlag.find(name: key.to_s) + flag.enabled = admin_value + flag.save + end + + before do + admin_override + end + + it 'returns the admin-set value' do + expect(FeatureFlag.enabled?(key)).to be admin_value + end + end + end + + context 'when there was previously set conflicting value' do + let(:admin_value) { !default_value } + + before do + FeatureFlag.make(name: key.to_s, enabled: admin_value) + end + + it 'overwrites the existing admin-set value' do + expect(FeatureFlag.enabled?(key)).to be admin_value + FeatureFlag.override_default_flags({ key => !admin_value }) + expect(FeatureFlag.enabled?(key)).to be !admin_value + end + end + end + + describe '.override_default_flags' do + context 'with invalid flags' do + it 'raises an error for the one and only invalid name' do + feature_flag_overrides = { an_invalid_name: true } + expect do + FeatureFlag.override_default_flags(feature_flag_overrides) + end.to raise_error('Invalid feature flag name(s): [:an_invalid_name]') + end + + it 'raises an error for a mix of valid and invalid names' do + feature_flag_overrides = { diego_docker: true, an_invalid_name: true } + expect do + FeatureFlag.override_default_flags(feature_flag_overrides) + end.to raise_error('Invalid feature flag name(s): [:an_invalid_name]') + end + + it 'raises an error for all invalid names' do + feature_flag_overrides = { invalid_name1: true, invalid_name2: false } + expect do + FeatureFlag.override_default_flags(feature_flag_overrides) + end.to raise_error('Invalid feature flag name(s): [:invalid_name1, :invalid_name2]') + end + + it 'raises an error for invalid values' do + feature_flag_overrides = { diego_docker: 'an invalid value', user_org_creation: false } + expect do + FeatureFlag.override_default_flags(feature_flag_overrides) + end.to raise_error('Invalid feature flag value(s): {:diego_docker=>"an invalid value"}') + end + end + + context 'with valid flags' do + let(:default_diego_docker_value) { FeatureFlag::DEFAULT_FLAGS[:diego_docker] } + let(:default_user_org_creation_value) { FeatureFlag::DEFAULT_FLAGS[:user_org_creation] } + + before do + expect do + FeatureFlag.override_default_flags({ diego_docker: !default_diego_docker_value, user_org_creation: !default_user_org_creation_value }) + end.not_to raise_error + end + + it 'updates values' do + expect(FeatureFlag.enabled?(:diego_docker)).to be !default_diego_docker_value + expect(FeatureFlag.enabled?(:user_org_creation)).to be !default_user_org_creation_value + end + end + + context 'with empty flags' do + it 'no effect' do + FeatureFlag.override_default_flags({}) + FeatureFlag::DEFAULT_FLAGS.each do |key, value| + expect(FeatureFlag.enabled?(key)).to eq value + end + end + end + end end end