diff --git a/lib/datadog/appsec/configuration/settings.rb b/lib/datadog/appsec/configuration/settings.rb index ea10271b6b8..c7e21cab9cb 100644 --- a/lib/datadog/appsec/configuration/settings.rb +++ b/lib/datadog/appsec/configuration/settings.rb @@ -259,11 +259,13 @@ def self.add_settings!(base) APPSEC_VALID_TRACK_USER_EVENTS_ENABLED_VALUES.include?(env_value.strip.downcase) end end - o.after_set do - Core.log_deprecation(key: :appsec_track_user_events_enabled) do - 'The appsec.track_user_events.enabled setting has been deprecated for removal. ' \ - 'Please remove it from your Datadog.configure block and use ' \ - 'appsec.auto_user_instrumentation.mode instead.' + o.after_set do |_, _, precedence| + unless precedence == Datadog::Core::Configuration::Option::Precedence::DEFAULT + Core.log_deprecation(key: :appsec_track_user_events_enabled) do + 'The appsec.track_user_events.enabled setting is deprecated. ' \ + 'Please remove it from your Datadog.configure block and use ' \ + 'appsec.auto_user_instrumentation.mode instead.' + end end end end @@ -287,11 +289,13 @@ def self.add_settings!(base) SAFE_TRACK_USER_EVENTS_MODE end end - o.after_set do - Core.log_deprecation(key: :appsec_track_user_events_mode) do - 'The appsec.track_user_events.mode setting has been deprecated for removal. ' \ - 'Please remove it from your Datadog.configure block and use ' \ - 'appsec.auto_user_instrumentation.mode instead.' + o.after_set do |_, _, precedence| + unless precedence == Datadog::Core::Configuration::Option::Precedence::DEFAULT + Core.log_deprecation(key: :appsec_track_user_events_mode) do + 'The appsec.track_user_events.mode setting is deprecated. ' \ + 'Please remove it from your Datadog.configure block and use ' \ + 'appsec.auto_user_instrumentation.mode instead.' + end end end end diff --git a/lib/datadog/core/configuration/option.rb b/lib/datadog/core/configuration/option.rb index 635b179c673..a31ca784b72 100644 --- a/lib/datadog/core/configuration/option.rb +++ b/lib/datadog/core/configuration/option.rb @@ -120,11 +120,15 @@ def unset(precedence) end def get - if @is_set - @value - else - set_value_from_env_or_default + unless @is_set + # Ensures that both the default value and the environment value are set. + # This approach handles scenarios where an environment value is unset + # by falling back to the default value consistently. + set_default_value + set_env_value end + + @value end def reset @@ -258,7 +262,8 @@ def validate(type, value) when NilClass true # No validation is performed when option is typeless else - raise ArgumentError, "The option #{@definition.name} is using an unsupported type option `#{@definition.type}`" + raise InvalidDefinitionError, + "The option #{@definition.name} is using an unsupported type option `#{@definition.type}`" end end @@ -271,7 +276,7 @@ def internal_set(value, precedence, resolved_env) @resolved_env = resolved_env # Store original value to ensure we can always safely call `#internal_set` # when restoring a value from `@value_per_precedence`, and we are only running `definition.setter` - # on the original value, not on a valud that has already been processed by `definition.setter`. + # on the original value, not on a value that has already been processed by `definition.setter`. @value_per_precedence[precedence] = value context_exec(v, old_value, precedence, &definition.after_set) if definition.after_set end @@ -285,9 +290,12 @@ def context_eval(&block) @context.instance_eval(&block) end - def set_value_from_env_or_default + def set_default_value + set(default_value, precedence: Precedence::DEFAULT) + end + + def set_env_value value = nil - precedence = nil resolved_env = nil if definition.env @@ -296,7 +304,6 @@ def set_value_from_env_or_default resolved_env = env value = coerce_env_variable(ENV[env]) - precedence = Precedence::ENVIRONMENT break end end @@ -304,16 +311,13 @@ def set_value_from_env_or_default if value.nil? && definition.deprecated_env && ENV[definition.deprecated_env] resolved_env = definition.deprecated_env value = coerce_env_variable(ENV[definition.deprecated_env]) - precedence = Precedence::ENVIRONMENT Datadog::Core.log_deprecation do "#{definition.deprecated_env} environment variable is deprecated, use #{definition.env} instead." end end - option_value = value.nil? ? default_value : value - - set(option_value, precedence: precedence || Precedence::DEFAULT, resolved_env: resolved_env) + set(value, precedence: Precedence::ENVIRONMENT, resolved_env: resolved_env) unless value.nil? rescue ArgumentError raise ArgumentError, "Expected environment variable #{resolved_env} to be a #{@definition.type}, " \ diff --git a/sig/datadog/core/configuration/option.rbs b/sig/datadog/core/configuration/option.rbs index 21230e96691..cff57effb01 100644 --- a/sig/datadog/core/configuration/option.rbs +++ b/sig/datadog/core/configuration/option.rbs @@ -51,7 +51,9 @@ module Datadog def context_eval: () { () -> untyped } -> untyped - def set_value_from_env_or_default: () -> untyped + def set_default_value: () -> void + + def set_env_value: () -> void end end end diff --git a/spec/datadog/appsec/configuration/settings_spec.rb b/spec/datadog/appsec/configuration/settings_spec.rb index 31b6146e8d6..5906feaaa36 100644 --- a/spec/datadog/appsec/configuration/settings_spec.rb +++ b/spec/datadog/appsec/configuration/settings_spec.rb @@ -486,7 +486,7 @@ def patcher it 'writes the deprication message' do expect(Datadog::Core).to receive(:log_deprecation) do |_, &block| - expect(block.call).to match(/setting has been deprecated for removal/) + expect(block.call).to match(/setting is deprecated/) end expect(enabled).to eq(true) end @@ -580,7 +580,7 @@ def patcher it 'writes the deprication message' do expect(Datadog::Core).to receive(:log_deprecation) do |_, &block| - expect(block.call).to match(/setting has been deprecated for removal/) + expect(block.call).to match(/setting is deprecated/) end set_appsec_track_user_events_mode diff --git a/spec/datadog/core/configuration/option_spec.rb b/spec/datadog/core/configuration/option_spec.rb index 62dbafcd195..b406f570dad 100644 --- a/spec/datadog/core/configuration/option_spec.rb +++ b/spec/datadog/core/configuration/option_spec.rb @@ -36,8 +36,9 @@ # to make sure specs pass when comparing result ex. expect(result).to be value # we ensure that frozen_or_dup returns the same instance before do - allow(Datadog::Core::Utils::SafeDup).to receive(:frozen_or_dup) do |args, _block| - args + # |args, _block| is not working with arrays + allow(Datadog::Core::Utils::SafeDup).to receive(:frozen_or_dup) do |*args, &_block| + args.first end end @@ -316,11 +317,11 @@ end context 'when type is defined' do - context 'type is invalid value' do + context 'type is invalid' do let(:type) { :nullable_string } let(:value) { 'Hello' } it 'raise exception' do - expect { set }.to raise_exception(ArgumentError) + expect { set }.to raise_exception(Datadog::Core::Configuration::Option::InvalidDefinitionError) end end @@ -689,9 +690,11 @@ subject(:get) { option.get } shared_examples_for 'env coercion' do + # As we now always set default value, we also need to change default to corresponding type context 'when type is defined' do context ':hash' do let(:type) { :hash } + let(:default) { {} } context 'value with commas' do let(:env_value) { 'key1:value1,key2:value2' } @@ -712,6 +715,7 @@ context ':int' do let(:type) { :int } + let(:default) { 0 } let(:env_value) { '1234' } it 'coerce value' do @@ -742,6 +746,7 @@ context ':float' do let(:type) { :float } + let(:default) { 0.0 } let(:env_value) { '12.34' } it 'coerce value' do @@ -758,7 +763,7 @@ context ':array' do let(:type) { :array } - + let(:default) { [] } context 'value with commas' do let(:env_value) { '12,34' } @@ -778,7 +783,7 @@ context ':bool' do let(:type) { :bool } - + let(:default) { false } context 'with value 1' do let(:env_value) { '1' } @@ -806,6 +811,7 @@ context 'invalid type' do let(:type) { :invalid_type } + let(:default) { '0' } let(:env_value) { '1' } it 'raise exception' do @@ -823,10 +829,7 @@ end it 'passes the env variable value to the env_parser' do - expect(context).to receive(:instance_exec) do |*args, &block| - expect(args.first).to eq(env_value) - expect(block).to eq env_parser - end + expect(context).to receive(:instance_exec).with(env_value, &env_parser) get end @@ -865,6 +868,12 @@ expect(option.send(:precedence_set)).to eq described_class::Precedence::ENVIRONMENT end + it 'falls back to default when unsetting env' do + option.get + option.unset(described_class::Precedence::ENVIRONMENT) + expect(option.get).to eq default + end + it_behaves_like 'env coercion' it_behaves_like 'with env_parser' end