From 0f41e05ebfa4159af2a289abfb5d12f4931177d8 Mon Sep 17 00:00:00 2001 From: Christian Bruckmayer Date: Tue, 2 May 2023 09:23:01 +0100 Subject: [PATCH 1/4] Safeguard SecureRandom.uuid --- lib/buildkite/test_collector.rb | 2 +- lib/buildkite/test_collector/ci.rb | 4 ++-- lib/buildkite/test_collector/library_hooks/rspec.rb | 2 +- .../test_collector/minitest_plugin/trace.rb | 2 +- lib/buildkite/test_collector/rspec_plugin/trace.rb | 2 +- lib/buildkite/test_collector/uuid.rb | 12 ++++++++++++ spec/test_collector/ci_spec.rb | 2 +- 7 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 lib/buildkite/test_collector/uuid.rb diff --git a/lib/buildkite/test_collector.rb b/lib/buildkite/test_collector.rb index 64d9136..7abd033 100644 --- a/lib/buildkite/test_collector.rb +++ b/lib/buildkite/test_collector.rb @@ -11,7 +11,6 @@ module TestCollector require "time" require "timeout" require "tmpdir" -require "securerandom" require "active_support/core_ext/object/blank" require "active_support/core_ext/hash/indifferent_access" @@ -26,6 +25,7 @@ module TestCollector require_relative "test_collector/object" require_relative "test_collector/tracer" require_relative "test_collector/session" +require_relative "test_collector/uuid" module Buildkite module TestCollector diff --git a/lib/buildkite/test_collector/ci.rb b/lib/buildkite/test_collector/ci.rb index d8f34a2..b185ef8 100644 --- a/lib/buildkite/test_collector/ci.rb +++ b/lib/buildkite/test_collector/ci.rb @@ -21,7 +21,7 @@ def ci_env { "CI" => nil, - "key" => SecureRandom.uuid, + "key" => Buildkite::TestCollector::Uuid.call, } end @@ -44,7 +44,7 @@ def analytics_env def generic { "CI" => "generic", - "key" => SecureRandom.uuid, + "key" => Buildkite::TestCollector::Uuid.call, } end diff --git a/lib/buildkite/test_collector/library_hooks/rspec.rb b/lib/buildkite/test_collector/library_hooks/rspec.rb index 76918cb..aab05d2 100644 --- a/lib/buildkite/test_collector/library_hooks/rspec.rb +++ b/lib/buildkite/test_collector/library_hooks/rspec.rb @@ -30,7 +30,7 @@ config.after(:suite) do if Buildkite::TestCollector.artifact_path - filename = File.join(Buildkite::TestCollector.artifact_path, "buildkite-test-collector-rspec-#{SecureRandom.uuid}.json.gz") + filename = File.join(Buildkite::TestCollector.artifact_path, "buildkite-test-collector-rspec-#{Buildkite::TestCollector::Uuid.call}.json.gz") data_set = { results: Buildkite::TestCollector.uploader.traces.values.map(&:as_hash) } File.open(filename, "wb") do |f| gz = Zlib::GzipWriter.new(f) diff --git a/lib/buildkite/test_collector/minitest_plugin/trace.rb b/lib/buildkite/test_collector/minitest_plugin/trace.rb index 907841d..9d0832c 100644 --- a/lib/buildkite/test_collector/minitest_plugin/trace.rb +++ b/lib/buildkite/test_collector/minitest_plugin/trace.rb @@ -16,7 +16,7 @@ class Trace FILE_PATH_REGEX = /^(.*?\.(rb|feature))/ def initialize(example, history:) - @id = SecureRandom.uuid + @id = Buildkite::TestCollector::Uuid.call @example = example @history = history end diff --git a/lib/buildkite/test_collector/rspec_plugin/trace.rb b/lib/buildkite/test_collector/rspec_plugin/trace.rb index a76ce88..d96c1a6 100644 --- a/lib/buildkite/test_collector/rspec_plugin/trace.rb +++ b/lib/buildkite/test_collector/rspec_plugin/trace.rb @@ -8,7 +8,7 @@ class Trace FILE_PATH_REGEX = /^(.*?\.(rb|feature))/ def initialize(example, history:, failure_reason: nil, failure_expanded: []) - @id = SecureRandom.uuid + @id = Buildkite::TestCollector::Uuid.call @example = example @history = history @failure_reason = failure_reason diff --git a/lib/buildkite/test_collector/uuid.rb b/lib/buildkite/test_collector/uuid.rb new file mode 100644 index 0000000..0d181d4 --- /dev/null +++ b/lib/buildkite/test_collector/uuid.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "securerandom" + +class Buildkite::TestCollector::Uuid + GET_UUID = SecureRandom.method(:uuid) + private_constant :GET_UUID + + def self.call + GET_UUID.call + end +end diff --git a/spec/test_collector/ci_spec.rb b/spec/test_collector/ci_spec.rb index 4904f15..7ed6ee9 100644 --- a/spec/test_collector/ci_spec.rb +++ b/spec/test_collector/ci_spec.rb @@ -3,7 +3,7 @@ RSpec.describe Buildkite::TestCollector::CI do describe ".env" do let(:ci) { "true" } - let(:key) { SecureRandom.uuid } + let(:key) { Buildkite::TestCollector::Uuid.call } let(:url) { "http://example.com" } let(:branch) { "not-main" } let(:sha) { "a2c5ef54" } From 232ab47090dc7da5f427639eb094469805821d4f Mon Sep 17 00:00:00 2001 From: Grant Colegate Date: Wed, 3 May 2023 09:41:46 +0930 Subject: [PATCH 2/4] Capitalize UUID acronym --- lib/buildkite/test_collector/ci.rb | 4 ++-- lib/buildkite/test_collector/library_hooks/rspec.rb | 2 +- lib/buildkite/test_collector/minitest_plugin/trace.rb | 2 +- lib/buildkite/test_collector/rspec_plugin/trace.rb | 2 +- lib/buildkite/test_collector/uuid.rb | 2 +- spec/test_collector/ci_spec.rb | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/buildkite/test_collector/ci.rb b/lib/buildkite/test_collector/ci.rb index b185ef8..aeb39d4 100644 --- a/lib/buildkite/test_collector/ci.rb +++ b/lib/buildkite/test_collector/ci.rb @@ -21,7 +21,7 @@ def ci_env { "CI" => nil, - "key" => Buildkite::TestCollector::Uuid.call, + "key" => Buildkite::TestCollector::UUID.call, } end @@ -44,7 +44,7 @@ def analytics_env def generic { "CI" => "generic", - "key" => Buildkite::TestCollector::Uuid.call, + "key" => Buildkite::TestCollector::UUID.call, } end diff --git a/lib/buildkite/test_collector/library_hooks/rspec.rb b/lib/buildkite/test_collector/library_hooks/rspec.rb index aab05d2..de5e79c 100644 --- a/lib/buildkite/test_collector/library_hooks/rspec.rb +++ b/lib/buildkite/test_collector/library_hooks/rspec.rb @@ -30,7 +30,7 @@ config.after(:suite) do if Buildkite::TestCollector.artifact_path - filename = File.join(Buildkite::TestCollector.artifact_path, "buildkite-test-collector-rspec-#{Buildkite::TestCollector::Uuid.call}.json.gz") + filename = File.join(Buildkite::TestCollector.artifact_path, "buildkite-test-collector-rspec-#{Buildkite::TestCollector::UUID.call}.json.gz") data_set = { results: Buildkite::TestCollector.uploader.traces.values.map(&:as_hash) } File.open(filename, "wb") do |f| gz = Zlib::GzipWriter.new(f) diff --git a/lib/buildkite/test_collector/minitest_plugin/trace.rb b/lib/buildkite/test_collector/minitest_plugin/trace.rb index 9d0832c..be502cf 100644 --- a/lib/buildkite/test_collector/minitest_plugin/trace.rb +++ b/lib/buildkite/test_collector/minitest_plugin/trace.rb @@ -16,7 +16,7 @@ class Trace FILE_PATH_REGEX = /^(.*?\.(rb|feature))/ def initialize(example, history:) - @id = Buildkite::TestCollector::Uuid.call + @id = Buildkite::TestCollector::UUID.call @example = example @history = history end diff --git a/lib/buildkite/test_collector/rspec_plugin/trace.rb b/lib/buildkite/test_collector/rspec_plugin/trace.rb index d96c1a6..689ec2b 100644 --- a/lib/buildkite/test_collector/rspec_plugin/trace.rb +++ b/lib/buildkite/test_collector/rspec_plugin/trace.rb @@ -8,7 +8,7 @@ class Trace FILE_PATH_REGEX = /^(.*?\.(rb|feature))/ def initialize(example, history:, failure_reason: nil, failure_expanded: []) - @id = Buildkite::TestCollector::Uuid.call + @id = Buildkite::TestCollector::UUID.call @example = example @history = history @failure_reason = failure_reason diff --git a/lib/buildkite/test_collector/uuid.rb b/lib/buildkite/test_collector/uuid.rb index 0d181d4..dca6c08 100644 --- a/lib/buildkite/test_collector/uuid.rb +++ b/lib/buildkite/test_collector/uuid.rb @@ -2,7 +2,7 @@ require "securerandom" -class Buildkite::TestCollector::Uuid +class Buildkite::TestCollector::UUID GET_UUID = SecureRandom.method(:uuid) private_constant :GET_UUID diff --git a/spec/test_collector/ci_spec.rb b/spec/test_collector/ci_spec.rb index 7ed6ee9..114e719 100644 --- a/spec/test_collector/ci_spec.rb +++ b/spec/test_collector/ci_spec.rb @@ -3,7 +3,7 @@ RSpec.describe Buildkite::TestCollector::CI do describe ".env" do let(:ci) { "true" } - let(:key) { Buildkite::TestCollector::Uuid.call } + let(:key) { Buildkite::TestCollector::UUID.call } let(:url) { "http://example.com" } let(:branch) { "not-main" } let(:sha) { "a2c5ef54" } From a9cf25d33b7a793497fcc97d0a1c3d3035f43a05 Mon Sep 17 00:00:00 2001 From: Grant Colegate Date: Wed, 3 May 2023 09:47:28 +0930 Subject: [PATCH 3/4] Stub UUID for tests --- spec/test_collector/ci_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/test_collector/ci_spec.rb b/spec/test_collector/ci_spec.rb index 114e719..7a17fb8 100644 --- a/spec/test_collector/ci_spec.rb +++ b/spec/test_collector/ci_spec.rb @@ -239,7 +239,7 @@ before do fake_env("CI", ci) - allow(SecureRandom).to receive(:uuid) { "845ac829-2ab3-4bbb-9e24-3529755a6d37" } + allow(Buildkite::TestCollector::UUID).to receive(:call) { "845ac829-2ab3-4bbb-9e24-3529755a6d37" } end it "returns all env" do @@ -287,7 +287,7 @@ context "when not running on a CI platform" do before do - allow(SecureRandom).to receive(:uuid) { "845ac829-2ab3-4bbb-9e24-3529755a6d37" } + allow(Buildkite::TestCollector::UUID).to receive(:call) { "845ac829-2ab3-4bbb-9e24-3529755a6d37" } end it "returns all env" do From d43e7195540d1eba4a59314bd58923c7c5aaa5ea Mon Sep 17 00:00:00 2001 From: niceking Date: Wed, 17 May 2023 14:54:14 +1200 Subject: [PATCH 4/4] stop sending id --- lib/buildkite/test_collector/minitest_plugin/trace.rb | 4 +--- lib/buildkite/test_collector/rspec_plugin/trace.rb | 4 +--- spec/test_collector/http_client_spec.rb | 1 - 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/buildkite/test_collector/minitest_plugin/trace.rb b/lib/buildkite/test_collector/minitest_plugin/trace.rb index be502cf..9b3e1d2 100644 --- a/lib/buildkite/test_collector/minitest_plugin/trace.rb +++ b/lib/buildkite/test_collector/minitest_plugin/trace.rb @@ -4,7 +4,7 @@ module Buildkite::TestCollector::MinitestPlugin class Trace attr_accessor :example attr_writer :failure_reason, :failure_expanded - attr_reader :id, :history + attr_reader :history RESULT_CODES = { '.' => 'passed', @@ -16,7 +16,6 @@ class Trace FILE_PATH_REGEX = /^(.*?\.(rb|feature))/ def initialize(example, history:) - @id = Buildkite::TestCollector::UUID.call @example = example @history = history end @@ -31,7 +30,6 @@ def source_location def as_hash strip_invalid_utf8_chars( - id: id, scope: example.class.name, name: example.name, location: location, diff --git a/lib/buildkite/test_collector/rspec_plugin/trace.rb b/lib/buildkite/test_collector/rspec_plugin/trace.rb index 689ec2b..6af52b9 100644 --- a/lib/buildkite/test_collector/rspec_plugin/trace.rb +++ b/lib/buildkite/test_collector/rspec_plugin/trace.rb @@ -3,12 +3,11 @@ module Buildkite::TestCollector::RSpecPlugin class Trace attr_accessor :example, :failure_reason, :failure_expanded - attr_reader :id, :history + attr_reader :history FILE_PATH_REGEX = /^(.*?\.(rb|feature))/ def initialize(example, history:, failure_reason: nil, failure_expanded: []) - @id = Buildkite::TestCollector::UUID.call @example = example @history = history @failure_reason = failure_reason @@ -25,7 +24,6 @@ def result def as_hash strip_invalid_utf8_chars( - id: id, scope: example.example_group.metadata[:full_description], name: example.description, location: example.location, diff --git a/spec/test_collector/http_client_spec.rb b/spec/test_collector/http_client_spec.rb index 1345b3b..cedf98a 100644 --- a/spec/test_collector/http_client_spec.rb +++ b/spec/test_collector/http_client_spec.rb @@ -31,7 +31,6 @@ }, "format": "json", "data": [{ - "id": trace.id, "scope": "i love to eat pies", "name": "mince and cheese", "location": "123 Pie St",