From 254b2c2351d43c4d0e5d1ce044a746212998f6d4 Mon Sep 17 00:00:00 2001 From: Robert May Date: Fri, 20 Dec 2019 18:06:30 +0000 Subject: [PATCH 1/3] Stub DNS in the spec suite This implements stubbing throughout the spec suite for DNS queries, of which there are many. It includes a few tweaks to continue to allow resolution for local addresses. This solves an issue where non-standard DNS setups prevent most of the spec suite from working locally, and should also provide speed improvements to the CI pipeline on average by removing a source of external requests. --- spec/lib/gitlab/tcp_checker_spec.rb | 1 + spec/lib/gitlab/url_blocker_spec.rb | 9 +++++++++ spec/spec_helper.rb | 16 ++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/spec/lib/gitlab/tcp_checker_spec.rb b/spec/lib/gitlab/tcp_checker_spec.rb index 49f04f269ae64c..e69cf83aa457f6 100644 --- a/spec/lib/gitlab/tcp_checker_spec.rb +++ b/spec/lib/gitlab/tcp_checker_spec.rb @@ -4,6 +4,7 @@ describe Gitlab::TcpChecker do before do + allow(Addrinfo).to receive(:getaddrinfo).and_call_original @server = TCPServer.new('localhost', 0) _, @port, _, @ip = @server.addr end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index a68ba489986dc6..e72efc3b63befc 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -5,6 +5,15 @@ describe Gitlab::UrlBlocker do include StubRequests + before(:each) do + allow(Addrinfo).to receive(:getaddrinfo).and_call_original + + # Re-stub duff requests + allow(Addrinfo).to receive(:getaddrinfo).with(/foobar\.\w|1.1.1.1.1|8.8.8.8.8/i, anything, nil, :STREAM) do + raise SocketError.new("getaddrinfo: Name or service not known") + end + end + describe '#validate!' do subject { described_class.validate!(import_url) } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6393e482904534..84b93354b70bba 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -353,6 +353,22 @@ config.append_after do Gitlab::Database.reset_open_transactions_baseline end + + config.before do + # By default return no DNS result + allow(Addrinfo).to receive(:getaddrinfo).with(anything, anything, nil, :STREAM).and_return([]) + allow(Addrinfo).to receive(:getaddrinfo).with(anything, anything, nil, :STREAM, anything, anything).and_return([]) + + # Stub duff addresses + allow(Addrinfo).to receive(:getaddrinfo).with(/foobar\.\w|(\d{1,3}\.){4,}\d{1,3}/i, anything, nil, :STREAM) do + raise SocketError.new("getaddrinfo: Name or service not known") + end + + # Allow any local requests through + local_addresses = /(127|10)\.0\.0\.\d{1,3}|(192\.168|172\.16)\.\d{1,3}\.\d{1,3}|0\.0\.0\.0|localhost/i + allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM).and_call_original + allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM, anything, anything).and_call_original + end end # add simpler way to match asset paths containing digest strings -- GitLab From 1945c08cee36ba2feb23054a2d7826cb0d94f97f Mon Sep 17 00:00:00 2001 From: Robert May Date: Fri, 17 Jan 2020 11:33:28 +0000 Subject: [PATCH 2/3] Style fix for rubocop --- spec/lib/gitlab/url_blocker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index e72efc3b63befc..65e26d2c4d02cc 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -5,7 +5,7 @@ describe Gitlab::UrlBlocker do include StubRequests - before(:each) do + before do allow(Addrinfo).to receive(:getaddrinfo).and_call_original # Re-stub duff requests -- GitLab From 9072ffdab84f71d459d7930aae16b5fc9afcbe47 Mon Sep 17 00:00:00 2001 From: Robert May Date: Mon, 20 Jan 2020 14:22:08 +0000 Subject: [PATCH 3/3] Refactor DNS stubs into support files Moves the new stubs out into a helper and includes them by default using hooks. --- spec/lib/gitlab/tcp_checker_spec.rb | 3 +-- spec/lib/gitlab/url_blocker_spec.rb | 11 +---------- spec/spec_helper.rb | 16 --------------- spec/support/dns.rb | 20 +++++++++++++++++++ spec/support/helpers/dns_helpers.rb | 30 +++++++++++++++++++++++++++++ 5 files changed, 52 insertions(+), 28 deletions(-) create mode 100644 spec/support/dns.rb create mode 100644 spec/support/helpers/dns_helpers.rb diff --git a/spec/lib/gitlab/tcp_checker_spec.rb b/spec/lib/gitlab/tcp_checker_spec.rb index e69cf83aa457f6..9474e79cc5dc6a 100644 --- a/spec/lib/gitlab/tcp_checker_spec.rb +++ b/spec/lib/gitlab/tcp_checker_spec.rb @@ -2,9 +2,8 @@ require 'spec_helper' -describe Gitlab::TcpChecker do +describe Gitlab::TcpChecker, :permit_dns do before do - allow(Addrinfo).to receive(:getaddrinfo).and_call_original @server = TCPServer.new('localhost', 0) _, @port, _, @ip = @server.addr end diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index 65e26d2c4d02cc..97859c82e9eba4 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -2,18 +2,9 @@ require 'spec_helper' -describe Gitlab::UrlBlocker do +describe Gitlab::UrlBlocker, :stub_invalid_dns_only do include StubRequests - before do - allow(Addrinfo).to receive(:getaddrinfo).and_call_original - - # Re-stub duff requests - allow(Addrinfo).to receive(:getaddrinfo).with(/foobar\.\w|1.1.1.1.1|8.8.8.8.8/i, anything, nil, :STREAM) do - raise SocketError.new("getaddrinfo: Name or service not known") - end - end - describe '#validate!' do subject { described_class.validate!(import_url) } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 84b93354b70bba..6393e482904534 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -353,22 +353,6 @@ config.append_after do Gitlab::Database.reset_open_transactions_baseline end - - config.before do - # By default return no DNS result - allow(Addrinfo).to receive(:getaddrinfo).with(anything, anything, nil, :STREAM).and_return([]) - allow(Addrinfo).to receive(:getaddrinfo).with(anything, anything, nil, :STREAM, anything, anything).and_return([]) - - # Stub duff addresses - allow(Addrinfo).to receive(:getaddrinfo).with(/foobar\.\w|(\d{1,3}\.){4,}\d{1,3}/i, anything, nil, :STREAM) do - raise SocketError.new("getaddrinfo: Name or service not known") - end - - # Allow any local requests through - local_addresses = /(127|10)\.0\.0\.\d{1,3}|(192\.168|172\.16)\.\d{1,3}\.\d{1,3}|0\.0\.0\.0|localhost/i - allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM).and_call_original - allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM, anything, anything).and_call_original - end end # add simpler way to match asset paths containing digest strings diff --git a/spec/support/dns.rb b/spec/support/dns.rb new file mode 100644 index 00000000000000..3e5c8e698e1606 --- /dev/null +++ b/spec/support/dns.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require Rails.root.join("spec/support/helpers/dns_helpers") + +RSpec.configure do |config| + config.include DnsHelpers + + config.before do + block_dns! + end + + config.before(:each, :permit_dns) do + permit_dns! + end + + config.before(:each, :stub_invalid_dns_only) do + permit_dns! + stub_invalid_dns! + end +end diff --git a/spec/support/helpers/dns_helpers.rb b/spec/support/helpers/dns_helpers.rb new file mode 100644 index 00000000000000..941b57c2c97780 --- /dev/null +++ b/spec/support/helpers/dns_helpers.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module DnsHelpers + def block_dns! + stub_all_dns! + stub_invalid_dns! + permit_local_dns! + end + + def permit_dns! + allow(Addrinfo).to receive(:getaddrinfo).and_call_original + end + + def stub_all_dns! + allow(Addrinfo).to receive(:getaddrinfo).with(anything, anything, nil, :STREAM).and_return([]) + allow(Addrinfo).to receive(:getaddrinfo).with(anything, anything, nil, :STREAM, anything, anything).and_return([]) + end + + def stub_invalid_dns! + allow(Addrinfo).to receive(:getaddrinfo).with(/foobar\.\w|(\d{1,3}\.){4,}\d{1,3}/i, anything, nil, :STREAM) do + raise SocketError.new("getaddrinfo: Name or service not known") + end + end + + def permit_local_dns! + local_addresses = /(127|10)\.0\.0\.\d{1,3}|(192\.168|172\.16)\.\d{1,3}\.\d{1,3}|0\.0\.0\.0|localhost/i + allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM).and_call_original + allow(Addrinfo).to receive(:getaddrinfo).with(local_addresses, anything, nil, :STREAM, anything, anything).and_call_original + end +end -- GitLab