From 1091dd34cd2dd0cd459d51075793ebbb43a7b282 Mon Sep 17 00:00:00 2001 From: Victor Pellan Date: Mon, 7 Oct 2024 17:37:51 +0200 Subject: [PATCH] - Added mocked agent and force trace formatting to test Datadog-Client-Computed-Stats header and _sampling_priority_v1 tag - Reproduced system-tests ASM Standalone tests --- .../contrib/rack/integration_test_spec.rb | 97 ++++-- .../contrib/rails/integration_test_spec.rb | 62 +++- .../contrib/sinatra/integration_test_spec.rb | 59 +++- .../support/integration/shared_examples.rb | 290 +++++++++++++++++- 4 files changed, 440 insertions(+), 68 deletions(-) diff --git a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb index d669f31c3d..325541ed0b 100644 --- a/spec/datadog/appsec/contrib/rack/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rack/integration_test_spec.rb @@ -19,9 +19,34 @@ RSpec.describe 'Rack integration tests' do include Rack::Test::Methods + # We send the trace to a mocked agent to verify that the trace includes the headers that we want + # In the future, it might be a good idea to use the traces that the mocked agent + # receives in the tests/shared examples + let(:agent_http_client) do + Datadog::Tracing::Transport::HTTP.default do |t| + t.adapter agent_http_adapter + end + end + + let(:agent_http_adapter) { Datadog::Core::Transport::HTTP::Adapters::Net.new(agent_settings) } + + let(:agent_settings) do + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new( + adapter: nil, + ssl: false, + uds_path: nil, + hostname: 'localhost', + port: 6218, + timeout_seconds: 30, + ) + end + + let(:agent_tested_headers) { {} } + + let(:tracing_enabled) { true } let(:appsec_enabled) { true } + let(:appsec_standalone_enabled) { false } - let(:tracing_enabled) { true } let(:remote_enabled) { false } let(:appsec_ip_passlist) { [] } let(:appsec_ip_denylist) { [] } @@ -142,6 +167,22 @@ } end + # Mocked agent with correct headers + stub_request(:post, 'http://localhost:6218/v0.4/traces') + .with do |request| + agent_tested_headers <= request.headers + end + .to_return(status: 200) + + # DEV: Would it be faster to do another stub for requests that don't match the headers + # rather than waiting for the TCP connection to fail? + + # TODO: Mocked agent that matches a given body, then use it in the shared examples, + # That way it would be real integration tests + + # We must format the trace to have the same result as the agent + # This is especially important for _sampling_priority_v1 metric + unless remote_enabled Datadog.configure do |c| c.tracing.enabled = tracing_enabled @@ -205,11 +246,12 @@ let(:client_ip) { remote_addr } let(:service_span) do - span = spans.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } - - expect(span.name).to eq 'rack.request' + spans.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } + end - span + let(:span) do + Datadog::Tracing::Transport::TraceFormatter.format!(trace) + spans.find { |s| s.name == 'rack.request' } end context 'with remote configuration' do @@ -243,24 +285,6 @@ map '/success/' do run(proc { |_env| [200, { 'Content-Type' => 'text/html' }, ['OK']] }) end - - map '/requestdownstream' do - run( - proc do |_env| - uri = URI('http://localhost:3000/returnheaders') - ext_request = nil - ext_response = nil - - Net::HTTP.start(uri.host, uri.port) do |http| - ext_request = Net::HTTP::Get.new(uri) - - ext_response = http.request(ext_request) - end - - [200, { 'Content-Type' => 'application/json' }, [ext_response.body]] - end - ) - end end end @@ -626,6 +650,24 @@ end ) end + + map '/requestdownstream' do + run( + proc do |_env| + uri = URI('http://localhost:3000/returnheaders') + ext_request = nil + ext_response = nil + + Net::HTTP.start(uri.host, uri.port) do |http| + ext_request = Net::HTTP::Get.new(uri) + + ext_response = http.request(ext_request) + end + + [200, { 'Content-Type' => 'application/json' }, [ext_response.body]] + end + ) + end end end @@ -851,13 +893,6 @@ it_behaves_like 'a trace with AppSec api security tags' end end - - context 'with APM disabled' do - let(:appsec_standalone_enabled) { true } - - it_behaves_like 'normal with tracing disable' - it_behaves_like 'a trace with ASM Standalone tags' - end end describe 'POST request' do @@ -987,6 +1022,8 @@ end end end + + it_behaves_like 'appsec standalone billing' end end end diff --git a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb index 7b039e6d8f..b564e477c9 100644 --- a/spec/datadog/appsec/contrib/rails/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/rails/integration_test_spec.rb @@ -9,7 +9,33 @@ RSpec.describe 'Rails integration tests' do include Rack::Test::Methods + # We send the trace to a mocked agent to verify that the trace includes the headers that we want + # In the future, it might be a good idea to use the traces that the mocked agent + # receives in the tests/shared examples + let(:agent_http_client) do + Datadog::Tracing::Transport::HTTP.default do |t| + t.adapter agent_http_adapter + end + end + + let(:agent_http_adapter) { Datadog::Core::Transport::HTTP::Adapters::Net.new(agent_settings) } + + let(:agent_settings) do + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new( + adapter: nil, + ssl: false, + uds_path: nil, + hostname: 'localhost', + port: 6218, + timeout_seconds: 30, + ) + end + let(:sorted_spans) do + # We must format the trace to have the same result as the agent + # This is especially important for _sampling_priority_v1 metric + Datadog::Tracing::Transport::TraceFormatter.format!(trace) + chain = lambda do |start| loop.with_object([start]) do |_, o| # root reached (default) @@ -27,15 +53,19 @@ sort.call(spans) end + let(:agent_tested_headers) { {} } + let(:rack_span) { sorted_spans.reverse.find { |x| x.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST } } + let(:tracing_enabled) { true } let(:appsec_enabled) { true } + + let(:appsec_instrument_rack) { false } + let(:appsec_standalone_enabled) { false } - let(:tracing_enabled) { true } let(:appsec_ip_denylist) { [] } let(:appsec_user_id_denylist) { [] } let(:appsec_ruleset) { :recommended } - let(:appsec_instrument_rack) { false } let(:api_security_enabled) { false } let(:api_security_sample) { 0.0 } @@ -100,6 +130,19 @@ } end + # Mocked agent with correct headers + stub_request(:post, 'http://localhost:6218/v0.4/traces') + .with do |request| + agent_tested_headers <= request.headers + end + .to_return(status: 200) + + # DEV: Would it be faster to do another stub for requests that don't match the headers + # rather than waiting for the TCP connection to fail? + + # TODO: Mocked agent that matches a given body, then use it in the shared examples, + # That way it would be real integration tests + Datadog.configure do |c| c.tracing.enabled = tracing_enabled @@ -185,11 +228,7 @@ def request_downstream let(:client_ip) { remote_addr } let(:service_span) do - span = sorted_spans.reverse.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } - - expect(span.name).to eq 'rack.request' - - span + sorted_spans.reverse.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } end let(:span) { rack_span } @@ -343,13 +382,6 @@ def request_downstream it_behaves_like 'a trace with AppSec api security tags' end end - - context 'with APM disabled' do - let(:appsec_standalone_enabled) { true } - - it_behaves_like 'normal with tracing disable' - it_behaves_like 'a trace with ASM Standalone tags' - end end describe 'POST request' do @@ -525,6 +557,8 @@ def request_downstream end end end + + it_behaves_like 'appsec standalone billing' end end end diff --git a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb index eba67d0721..e9f25a12b1 100644 --- a/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb +++ b/spec/datadog/appsec/contrib/sinatra/integration_test_spec.rb @@ -19,7 +19,33 @@ RSpec.describe 'Sinatra integration tests' do include Rack::Test::Methods + # We send the trace to a mocked agent to verify that the trace includes the headers that we want + # In the future, it might be a good idea to use the traces that the mocked agent + # receives in the tests/shared examples + let(:agent_http_client) do + Datadog::Tracing::Transport::HTTP.default do |t| + t.adapter agent_http_adapter + end + end + + let(:agent_http_adapter) { Datadog::Core::Transport::HTTP::Adapters::Net.new(agent_settings) } + + let(:agent_settings) do + Datadog::Core::Configuration::AgentSettingsResolver::AgentSettings.new( + adapter: nil, + ssl: false, + uds_path: nil, + hostname: 'localhost', + port: 6218, + timeout_seconds: 30, + ) + end + let(:sorted_spans) do + # We must format the trace to have the same result as the agent + # This is especially important for _sampling_priority_v1 metric + Datadog::Tracing::Transport::TraceFormatter.format!(trace) + chain = lambda do |start| loop.with_object([start]) do |_, o| # root reached (default) @@ -37,13 +63,16 @@ sort.call(spans) end + let(:agent_tested_headers) { {} } + let(:sinatra_span) { sorted_spans.reverse.find { |x| x.name == Datadog::Tracing::Contrib::Sinatra::Ext::SPAN_REQUEST } } let(:route_span) { sorted_spans.find { |x| x.name == Datadog::Tracing::Contrib::Sinatra::Ext::SPAN_ROUTE } } let(:rack_span) { sorted_spans.reverse.find { |x| x.name == Datadog::Tracing::Contrib::Rack::Ext::SPAN_REQUEST } } + let(:tracing_enabled) { true } let(:appsec_enabled) { true } + let(:appsec_standalone_enabled) { false } - let(:tracing_enabled) { true } let(:appsec_ip_denylist) { [] } let(:appsec_user_id_denylist) { [] } let(:appsec_ruleset) { :recommended } @@ -108,6 +137,19 @@ } end + # Mocked agent with correct headers + stub_request(:post, 'http://localhost:6218/v0.4/traces') + .with do |request| + agent_tested_headers <= request.headers + end + .to_return(status: 200) + + # DEV: Would it be faster to do another stub for requests that don't match the headers + # rather than waiting for the TCP connection to fail? + + # TODO: Mocked agent that matches a given body, then use it in the shared examples, + # That way it would be real integration tests + Datadog.configure do |c| c.tracing.enabled = tracing_enabled @@ -163,11 +205,7 @@ let(:client_ip) { remote_addr } let(:service_span) do - span = sorted_spans.reverse.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } - - expect(span.name).to eq 'rack.request' - - span + sorted_spans.reverse.find { |s| s.metrics.fetch('_dd.top_level', -1.0) > 0.0 } end let(:span) { rack_span } @@ -347,13 +385,6 @@ it_behaves_like 'a trace with AppSec api security tags' end end - - context 'with APM disabled' do - let(:appsec_standalone_enabled) { true } - - it_behaves_like 'normal with tracing disable' - it_behaves_like 'a trace with ASM Standalone tags' - end end describe 'POST request' do @@ -441,6 +472,8 @@ it_behaves_like 'a trace with AppSec api security tags' end end + + it_behaves_like 'appsec standalone billing' end end end diff --git a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb index 1d8fcdafd0..a2ebbe02c4 100644 --- a/spec/datadog/appsec/contrib/support/integration/shared_examples.rb +++ b/spec/datadog/appsec/contrib/support/integration/shared_examples.rb @@ -168,20 +168,288 @@ end end -RSpec.shared_examples 'a trace with ASM Standalone tags' do - context 'with appsec enabled' do - let(:appsec_enabled) { true } - it do - expect(service_span.send(:metrics)['_dd.apm.enabled']).to eq(0) - expect(service_span.send(:metrics)['_dd.appsec.enabled']).to eq(1.0) +RSpec.shared_examples 'a trace with ASM Standalone tags' do |params = {}| + let(:tag_apm_enabled) { params[:tag_apm_enabled] || 0 } + let(:tag_appsec_enabled) { params[:tag_appsec_enabled] || 1.0 } + let(:tag_appsec_propagation) { params[:tag_appsec_propagation] } + let(:tag_other_propagation) { params[:tag_other_propagation] || :any } + # We use a lambda as we may change the comparison type + let(:tag_sampling_priority_condition) { params[:tag_sampling_priority_condition] || ->(x) { x == 0 } } + let(:tag_trace_id) { params[:tag_trace_id] || headers_trace_id.to_i } + + it do + expect(span.send(:metrics)['_dd.apm.enabled']).to eq(tag_apm_enabled) + expect(span.send(:metrics)['_dd.appsec.enabled']).to eq(tag_appsec_enabled) + expect(span.send(:metrics)['_sampling_priority_v1']).to(satisfy { |x| tag_sampling_priority_condition.call(x) }) + + expect(span.send(:meta)['_dd.p.appsec']).to eq(tag_appsec_propagation) + expect(span.send(:meta)['_dd.p.other']).to eq(tag_other_propagation) unless tag_other_propagation == :any + + expect(span.send(:trace_id)).to eq(tag_trace_id) + expect(trace.send(:spans)[0].send(:trace_id)).to eq(tag_trace_id) + end +end + +RSpec.shared_examples 'a request with propagated headers' do |params = {}| + let(:res_origin) { params[:res_origin] } + let(:res_parent_id_not_equal) { params[:res_parent_id_not_equal] } + let(:res_tags) { params[:res_tags] } + let(:res_sampling_priority_condition) { params[:res_sampling_priority_condition] || ->(x) { x.nil? } } + let(:res_trace_id) { params[:res_trace_id] } + + let(:res_headers) { JSON.parse(response.body) } + + it do + expect(res_headers['X-Datadog-Origin']).to eq(res_origin) + expect(res_headers['X-Datadog-Parent']).to_not eq(res_parent_id_not_equal) if res_parent_id_not_equal + expect(res_headers['X-Datadog-Sampling-Priority']).to(satisfy { |x| res_sampling_priority_condition.call(x) }) + expect(res_headers['X-Datadog-Trace-Id']).to eq(res_trace_id) + expect(res_headers['X-Datadog-Tags'].split(',')).to include(*res_tags) if res_tags + end +end + +RSpec.shared_examples 'a trace sent to agent with Datadog-Client-Computed-Stats header' do + let(:agent_tested_headers) { { 'Datadog-Client-Computed-Stats' => 'yes' } } + + it do + agent_return = agent_http_client.send_traces(traces) + expect(agent_return[0].ok?).to be true + end +end + +RSpec.shared_examples 'appsec standalone billing' do + subject(:response) { get url, params, env } + + let(:appsec_standalone_enabled) { true } + + let(:url) { '/requestdownstream' } + let(:params) { {} } + let(:headers) do + { + 'HTTP_X_DATADOG_TRACE_ID' => headers_trace_id, + 'HTTP_X_DATADOG_PARENT_ID' => headers_parent_id, + 'HTTP_X_DATADOG_SAMPLING_PRIORITY' => headers_sampling_priority, + 'HTTP_X_DATADOG_ORIGIN' => headers_origin, + 'HTTP_X_DATADOG_TAGS' => headers_tags, + 'HTTP_USER_AGENT' => user_agent + } + end + let(:env) { headers } + + # Default values for headers + let(:headers_trace_id) { '1212121212121212121' } + let(:headers_parent_id) { '34343434' } + let(:headers_origin) { 'rum' } + let(:headers_sampling_priority) { '-1' } + let(:headers_tags) { '_dd.p.other=1' } + let(:user_agent) { nil } + + context 'without appsec upstream without attack and trace is kept with priority 1' do + context 'from -1 sampling priority' do + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_other_propagation: '1', + tag_sampling_priority_condition: ->(x) { x < 2 } + } + it_behaves_like 'a request with propagated headers' + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 0 sampling priority' do + let(:headers_sampling_priority) { '0' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_other_propagation: '1', + tag_sampling_priority_condition: ->(x) { x < 2 } + } + it_behaves_like 'a request with propagated headers' + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 1 sampling priority' do + let(:headers_sampling_priority) { '1' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_other_propagation: '1', + tag_sampling_priority_condition: ->(x) { x < 2 } + } + it_behaves_like 'a request with propagated headers' + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 2 sampling priority' do + let(:headers_sampling_priority) { '2' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_other_propagation: '1', + tag_sampling_priority_condition: ->(x) { x < 2 } + } + it_behaves_like 'a request with propagated headers' + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' end end - context 'with appsec disabled' do - let(:appsec_enabled) { false } - it do - expect(service_span.send(:metrics)['_dd.apm.enabled']).to be_nil - expect(service_span.send(:metrics)['_dd.appsec.enabled']).to be_nil + context 'without upstream appsec propagation with attack and trace is kept with priority 2' do + let(:user_agent) { 'Arachni/v1' } + + context 'from -1 sampling priority' do + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.other=1', '_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 0 sampling priority' do + let(:headers_sampling_priority) { '0' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.other=1', '_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + end + + context 'with upstream appsec propagation without attack and trace is propagated as is' do + let(:headers_tags) { '_dd.p.appsec=1' } + + context 'from 0 sampling priority' do + let(:headers_sampling_priority) { '0' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { [0, 2].include?(x) } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { ['0', '2'].include?(x) }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 1 sampling priority' do + let(:headers_sampling_priority) { '1' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { [1, 2].include?(x) } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { ['1', '2'].include?(x) }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 2 sampling priority' do + let(:headers_sampling_priority) { '2' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + end + + context 'with any upstream propagation with attack and raises trace priority to 2' do + let(:user_agent) { 'Arachni/v1' } + let(:headers_tags) { nil } + + context 'from -1 sampling priority' do + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 0 sampling priority' do + let(:headers_sampling_priority) { '0' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' + end + + context 'from 1 sampling priority' do + let(:headers_sampling_priority) { '1' } + + it_behaves_like 'a trace with ASM Standalone tags', + { + tag_appsec_propagation: '1', + tag_sampling_priority_condition: ->(x) { x == 2 } + } + it_behaves_like 'a request with propagated headers', + { + res_origin: 'rum', + res_parent_id_not_equal: '34343434', + res_tags: ['_dd.p.appsec=1'], + res_sampling_priority_condition: ->(x) { x == '2' }, + res_trace_id: '1212121212121212121' + } + it_behaves_like 'a trace sent to agent with Datadog-Client-Computed-Stats header' end end end