diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-17 09:07:23 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-17 09:07:23 +0300 |
commit | 172e4a12748fd146fdd0e9eca12ade4c51dabda9 (patch) | |
tree | 7d4be9fa2966dbaf4f9f98937db051627e78b816 | |
parent | 4c872af312f27f2e2da967a6efebd76e88119caa (diff) |
Add latest changes from gitlab-org/gitlab@master
25 files changed, 738 insertions, 177 deletions
@@ -203,7 +203,7 @@ gem 'seed-fu', '~> 2.3.7' # rubocop:todo Gemfile/MissingFeatureCategory gem 'elasticsearch-model', '~> 7.2' # rubocop:todo Gemfile/MissingFeatureCategory gem 'elasticsearch-rails', '~> 7.2', require: 'elasticsearch/rails/instrumentation' # rubocop:todo Gemfile/MissingFeatureCategory gem 'elasticsearch-api', '7.13.3' # rubocop:todo Gemfile/MissingFeatureCategory -gem 'aws-sdk-core', '~> 3.190.2' # rubocop:todo Gemfile/MissingFeatureCategory +gem 'aws-sdk-core', '~> 3.190.3' # rubocop:todo Gemfile/MissingFeatureCategory gem 'aws-sdk-cloudformation', '~> 1' # rubocop:todo Gemfile/MissingFeatureCategory gem 'aws-sdk-s3', '~> 1.142.0' # rubocop:todo Gemfile/MissingFeatureCategory gem 'faraday_middleware-aws-sigv4', '~>0.3.0' # rubocop:todo Gemfile/MissingFeatureCategory diff --git a/Gemfile.checksum b/Gemfile.checksum index 7c3fe677ea8..e5cf7f7719a 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -37,7 +37,7 @@ {"name":"aws-eventstream","version":"1.3.0","platform":"ruby","checksum":"f1434cc03ab2248756eb02cfa45e900e59a061d7fbdc4a9fd82a5dd23d796d3f"}, {"name":"aws-partitions","version":"1.877.0","platform":"ruby","checksum":"9552ed7bbd3700ed1eeb0121c160ceaf64fa5dbaff5a1ff5fe6fd8481ecd9cfd"}, {"name":"aws-sdk-cloudformation","version":"1.41.0","platform":"ruby","checksum":"31e47539719734413671edf9b1a31f8673fbf9688549f50c41affabbcb1c6b26"}, -{"name":"aws-sdk-core","version":"3.190.2","platform":"ruby","checksum":"5d97bd8ebfff08b51c38e37dace3d919fa7c708696da01b1d343f2bbaf472e7d"}, +{"name":"aws-sdk-core","version":"3.190.3","platform":"ruby","checksum":"f9b66775d79112bcffcd031aa2c97aac2b1819fa2d917b1c72557635260f3d41"}, {"name":"aws-sdk-kms","version":"1.76.0","platform":"ruby","checksum":"e7f75013cba9ba357144f66bbc600631c192e2cda9dd572794be239654e2cf49"}, {"name":"aws-sdk-s3","version":"1.142.0","platform":"ruby","checksum":"79cd888eca66fd2ef3ae8b74d76173a2eccbeff6a1bba62a60b7c7dadc8dd7e9"}, {"name":"aws-sigv4","version":"1.8.0","platform":"ruby","checksum":"84dd99768b91b93b63d1d8e53ee837cfd06ab402812772a7899a78f9f9117cbc"}, diff --git a/Gemfile.lock b/Gemfile.lock index c7be326191a..9d9617019d7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -301,7 +301,7 @@ GEM aws-sdk-cloudformation (1.41.0) aws-sdk-core (~> 3, >= 3.99.0) aws-sigv4 (~> 1.1) - aws-sdk-core (3.190.2) + aws-sdk-core (3.190.3) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.651.0) aws-sigv4 (~> 1.8) @@ -1815,7 +1815,7 @@ DEPENDENCIES autoprefixer-rails (= 10.2.5.1) awesome_print aws-sdk-cloudformation (~> 1) - aws-sdk-core (~> 3.190.2) + aws-sdk-core (~> 3.190.3) aws-sdk-s3 (~> 1.142.0) axe-core-rspec (~> 4.8.0) babosa (~> 2.0) diff --git a/app/graphql/types/ci/pipeline_type.rb b/app/graphql/types/ci/pipeline_type.rb index dfdc3752916..65244235e10 100644 --- a/app/graphql/types/ci/pipeline_type.rb +++ b/app/graphql/types/ci/pipeline_type.rb @@ -199,6 +199,10 @@ module Types field :stuck, GraphQL::Types::Boolean, method: :stuck?, null: false, description: "If the pipeline is stuck." + field :yaml_errors, GraphQL::Types::Boolean, method: :yaml_errors?, null: false, description: "If the pipeline has YAML errors." + + field :trigger, GraphQL::Types::Boolean, method: :trigger?, null: false, description: "If the pipeline was created by a Trigger request." + def commit BatchLoader::GraphQL.wrap(object.commit) end diff --git a/app/views/projects/_importing_alert.haml b/app/views/projects/_importing_alert.haml new file mode 100644 index 00000000000..192f952e460 --- /dev/null +++ b/app/views/projects/_importing_alert.haml @@ -0,0 +1,6 @@ +- return unless project.import_in_progress? + +- content_for :page_level_alert do + = render Pajamas::AlertComponent.new(variant: :warning, dismissible: false, title: _('Import in progress'), alert_options: { class: 'gl-mb-3' }) do |c| + - c.with_body do + = s_('Import|This project is being imported. Do not make any changes to the project until the import is complete.') diff --git a/config/feature_flags/development/bitbucket_server_importer_exponential_backoff.yml b/config/feature_flags/development/bitbucket_server_importer_exponential_backoff.yml deleted file mode 100644 index c167efddf49..00000000000 --- a/config/feature_flags/development/bitbucket_server_importer_exponential_backoff.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: bitbucket_server_importer_exponential_backoff -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/137974 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/432974 -milestone: '16.7' -type: development -group: group::import and integrate -default_enabled: false diff --git a/doc/administration/redis/troubleshooting.md b/doc/administration/redis/troubleshooting.md index 0dcb19c1999..6cfbce0443f 100644 --- a/doc/administration/redis/troubleshooting.md +++ b/doc/administration/redis/troubleshooting.md @@ -79,8 +79,17 @@ repl_backlog_histlen:0 ## High CPU usage on Redis instance -High CPU usage on Redis instance can be cause by Sidekiq `BRPOP` calls. The `BRPOP` command is expensive and increases CPU usage on Redis. -Increase the [`SIDEKIQ_SEMI_RELIABLE_FETCH_TIMEOUT` environment variable](../environment_variables.md) to improve CPU usage on Redis. +By default, GitLab uses over 600 Sidekiq queues, each stored as a Redis list. Each Sidekiq thread issues a `BRPOP` command with all the queues listed in a long string. +Redis CPU utilization grows as the number of queues and the rate of `BRPOP` calls increases. If your GitLab instance has many Sidekiq processes, this can cause Redis +CPU utilization to approach 100%. High CPU utilization degrades GitLab performance significantly. + +To reduce CPU usage on Redis caused by Sidekiq you can both: + +- Use [routing rules](../sidekiq/processing_specific_job_classes.md#routing-rules) to reduce the number of Sidekiq queues. +- If you are using GitLab 16.6 and earlier, increase the [`SIDEKIQ_SEMI_RELIABLE_FETCH_TIMEOUT` environment variable](../environment_variables.md) to improve CPU usage on Redis. + On GitLab 16.7 and later, the [default value is 5](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139583), which should be sufficient. + +The `SIDEKIQ_SEMI_RELIABLE_FETCH_TIMEOUT` option reduces the overhead that tearing down and connecting causes, but increase the shutdown delay of Sidekiq. ## Troubleshooting Sentinel diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 70d4d3cf7da..37515ddce91 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -24052,6 +24052,7 @@ Returns [`UserMergeRequestInteraction`](#usermergerequestinteraction). | <a id="pipelinestuck"></a>`stuck` | [`Boolean!`](#boolean) | If the pipeline is stuck. | | <a id="pipelinetestreportsummary"></a>`testReportSummary` | [`TestReportSummary!`](#testreportsummary) | Summary of the test report generated by the pipeline. | | <a id="pipelinetotaljobs"></a>`totalJobs` | [`Int!`](#int) | The total number of jobs in the pipeline. | +| <a id="pipelinetrigger"></a>`trigger` | [`Boolean!`](#boolean) | If the pipeline was created by a Trigger request. | | <a id="pipelinetriggeredbypath"></a>`triggeredByPath` | [`String`](#string) | The path that triggered this pipeline. | | <a id="pipelineupdatedat"></a>`updatedAt` | [`Time!`](#time) | Timestamp of the pipeline's last activity. | | <a id="pipelineupstream"></a>`upstream` | [`Pipeline`](#pipeline) | Pipeline that triggered the pipeline. | @@ -24060,6 +24061,7 @@ Returns [`UserMergeRequestInteraction`](#usermergerequestinteraction). | <a id="pipelineusesneeds"></a>`usesNeeds` | [`Boolean`](#boolean) | Indicates if the pipeline has jobs with `needs` dependencies. | | <a id="pipelinewarningmessages"></a>`warningMessages` | [`[PipelineMessage!]`](#pipelinemessage) | Pipeline warning messages. | | <a id="pipelinewarnings"></a>`warnings` | [`Boolean!`](#boolean) | Indicates if a pipeline has warnings. | +| <a id="pipelineyamlerrors"></a>`yamlErrors` | [`Boolean!`](#boolean) | If the pipeline has YAML errors. | #### Fields with arguments diff --git a/doc/development/bitbucket_cloud_importer.md b/doc/development/bitbucket_cloud_importer.md index 98620ca39eb..bab93f5075e 100644 --- a/doc/development/bitbucket_cloud_importer.md +++ b/doc/development/bitbucket_cloud_importer.md @@ -65,6 +65,11 @@ For every issue, a job for the `Gitlab::BitbucketImport::ImportIssueNotesWorker` This worker completes the import process by performing some housekeeping such as marking the import as completed. +## Backoff and retry + +In order to handle rate limiting, requests are wrapped with `Bitbucket::ExponentialBackoff`. +This wrapper catches rate limit errors and retries after a delay up to three times. + ## Set up Bitbucket authentication on GDK To set up Bitbucket authentication on GDK: diff --git a/doc/development/bitbucket_server_importer.md b/doc/development/bitbucket_server_importer.md index 3a1f5a4febd..f1a91020594 100644 --- a/doc/development/bitbucket_server_importer.md +++ b/doc/development/bitbucket_server_importer.md @@ -82,3 +82,8 @@ same email does not exist on GitLab, this can lead to incorrect users being tagg To get around this, we build a cache containing all users who have access to the Bitbucket project and then convert mentions in pull request descriptions and notes. + +## Backoff and retry + +In order to handle rate limiting, requests are wrapped with `BitbucketServer::RetryWithDelay`. +This wrapper checks if the response is rate limited and retries once after the delay specified in the response headers. diff --git a/doc/user/ai_features.md b/doc/user/ai_features.md index 780a8de63cd..906fa9cc724 100644 --- a/doc/user/ai_features.md +++ b/doc/user/ai_features.md @@ -18,7 +18,7 @@ GitLab is creating AI-assisted features across our DevSecOps platform. These fea | Assists with quickly getting everyone up to speed on lengthy conversations to help ensure you are all on the same page. | [Discussion summary](#summarize-issue-discussions-with-discussion-summary) | **(ULTIMATE SAAS EXPERIMENT)** | | Generates issue descriptions. | [Issue description generation](#summarize-an-issue-with-issue-description-generation) | **(ULTIMATE SAAS EXPERIMENT)** | | Helps you write code more efficiently by viewing code suggestions as you type. <br><br><i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Watch overview](https://www.youtube.com/watch?v=hCAyCTacdAQ) | [Code Suggestions](project/repository/code_suggestions/index.md) | For SaaS: **(FREE)**<br><br> For self-managed: **(PREMIUM)** | -| Automates repetitive tasks and helps catch bugs early. | [Test generation](gitlab_duo_chat.md#write-tests-in-the-ide) | **(ULTIMATE SAAS EXPERIMENT)** | +| Automates repetitive tasks and helps catch bugs early. | [Test generation](gitlab_duo_chat.md#write-tests-in-the-ide) | **(ULTIMATE BETA)** | | Generates a description for the merge request based on the contents of the template. | [Merge request template population](project/merge_requests/ai_in_merge_requests.md#fill-in-merge-request-templates) | **(ULTIMATE SAAS EXPERIMENT)** | | Assists in creating faster and higher-quality reviews by automatically suggesting reviewers for your merge request. <br><br><i class="fa fa-youtube-play youtube" aria-hidden="true"></i> [Watch overview](https://www.youtube.com/watch?v=ivwZQgh4Rxw) | [Suggested Reviewers](project/merge_requests/reviews/index.md#gitlab-duo-suggested-reviewers) | **(ULTIMATE SAAS)** | | Efficiently communicates the impact of your merge request changes. | [Merge request summary](project/merge_requests/ai_in_merge_requests.md#summarize-merge-request-changes) | **(ULTIMATE SAAS EXPERIMENT)** | @@ -195,7 +195,7 @@ For details about this Beta feature, see [GitLab Duo Chat](gitlab_duo_chat.md). | [Discussion summary](#summarize-issue-discussions-with-discussion-summary) | Vertex AI Codey [`text-bison`](https://cloud.google.com/vertex-ai/docs/generative-ai/model-reference/text) | | [Issue description generation](#summarize-an-issue-with-issue-description-generation) | Anthropic [`Claude-2`](https://docs.anthropic.com/claude/reference/selecting-a-model) | | [Code Suggestions](project/repository/code_suggestions/index.md) | For Code Completion: Vertex AI Codey [`code-gecko`](https://cloud.google.com/vertex-ai/docs/generative-ai/model-reference/code-completion) For Code Generation: Anthropic [`Claude-2`](https://docs.anthropic.com/claude/reference/selecting-a-model) | -| [Test generation](gitlab_duo_chat.md#write-tests-in-the-ide) | Vertex AI Codey [`text-bison`](https://cloud.google.com/vertex-ai/docs/generative-ai/model-reference/text) | +| [Test generation](gitlab_duo_chat.md#write-tests-in-the-ide) | Anthropic [`Claude-2`](https://docs.anthropic.com/claude/reference/selecting-a-model) | | [Merge request template population](project/merge_requests/ai_in_merge_requests.md#fill-in-merge-request-templates) | Vertex AI Codey [`text-bison`](https://cloud.google.com/vertex-ai/docs/generative-ai/model-reference/text) | | [Suggested Reviewers](project/merge_requests/reviews/index.md#gitlab-duo-suggested-reviewers) | GitLab creates a machine learning model for each project, which is used to generate reviewers [View the issue](https://gitlab.com/gitlab-org/modelops/applied-ml/applied-ml-updates/-/issues/10) | | [Merge request summary](project/merge_requests/ai_in_merge_requests.md#summarize-merge-request-changes) | Vertex AI Codey [`text-bison`](https://cloud.google.com/vertex-ai/docs/generative-ai/model-reference/text) | diff --git a/lib/bitbucket_server/connection.rb b/lib/bitbucket_server/connection.rb index 668a4e79da0..703f984077a 100644 --- a/lib/bitbucket_server/connection.rb +++ b/lib/bitbucket_server/connection.rb @@ -32,13 +32,9 @@ module BitbucketServer end def get(path, extra_query = {}) - response = if Feature.enabled?(:bitbucket_server_importer_exponential_backoff) - retry_with_delay do - Gitlab::HTTP.get(build_url(path), basic_auth: auth, headers: accept_headers, query: extra_query) - end - else - Gitlab::HTTP.get(build_url(path), basic_auth: auth, headers: accept_headers, query: extra_query) - end + response = retry_with_delay do + Gitlab::HTTP.get(build_url(path), basic_auth: auth, headers: accept_headers, query: extra_query) + end check_errors!(response) @@ -48,13 +44,9 @@ module BitbucketServer end def post(path, body) - response = if Feature.enabled?(:bitbucket_server_importer_exponential_backoff) - retry_with_delay do - Gitlab::HTTP.post(build_url(path), basic_auth: auth, headers: post_headers, body: body) - end - else - Gitlab::HTTP.post(build_url(path), basic_auth: auth, headers: post_headers, body: body) - end + response = retry_with_delay do + Gitlab::HTTP.post(build_url(path), basic_auth: auth, headers: post_headers, body: body) + end check_errors!(response) @@ -70,13 +62,9 @@ module BitbucketServer def delete(resource, path, body) url = delete_url(resource, path) - response = if Feature.enabled?(:bitbucket_server_importer_exponential_backoff) - retry_with_delay do - Gitlab::HTTP.delete(url, basic_auth: auth, headers: post_headers, body: body) - end - else - Gitlab::HTTP.delete(url, basic_auth: auth, headers: post_headers, body: body) - end + response = retry_with_delay do + Gitlab::HTTP.delete(url, basic_auth: auth, headers: post_headers, body: body) + end check_errors!(response) diff --git a/lib/gitlab/ci/parsers/sbom/component.rb b/lib/gitlab/ci/parsers/sbom/component.rb new file mode 100644 index 00000000000..1a4aa5071ae --- /dev/null +++ b/lib/gitlab/ci/parsers/sbom/component.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Parsers + module Sbom + class Component + include Gitlab::Utils::StrongMemoize + + TRIVY_SOURCE_PACKAGE_FIELD = 'SrcName' + + def initialize(data) + @data = data + end + + def parse + ::Gitlab::Ci::Reports::Sbom::Component.new( + type: data['type'], + name: data['name'], + purl: purl, + version: data['version'], + properties: properties, + source_package_name: source_package_name + ) + end + + private + + attr_reader :data + + def purl + return unless data['purl'] + + ::Sbom::PackageUrl.parse(data['purl']) + end + strong_memoize_attr :purl + + def properties + CyclonedxProperties.parse_trivy_source(data['properties']) + end + strong_memoize_attr :properties + + def source_package_name + return unless container_scanning_component? + + properties&.data&.dig(TRIVY_SOURCE_PACKAGE_FIELD) || data['name'] + end + + def container_scanning_component? + return false unless data['purl'] + + Enums::Sbom.container_scanning_purl_type?(purl.type) + end + strong_memoize_attr :container_scanning_component? + end + end + end + end +end diff --git a/lib/gitlab/ci/parsers/sbom/cyclonedx.rb b/lib/gitlab/ci/parsers/sbom/cyclonedx.rb index 62cd322e141..9c48dd69a41 100644 --- a/lib/gitlab/ci/parsers/sbom/cyclonedx.rb +++ b/lib/gitlab/ci/parsers/sbom/cyclonedx.rb @@ -58,15 +58,7 @@ module Gitlab def parse_components data['components']&.each_with_index do |component_data, index| - properties = component_data['properties'] - component = ::Gitlab::Ci::Reports::Sbom::Component.new( - type: component_data['type'], - name: component_data['name'], - purl: component_data['purl'], - version: component_data['version'] - ) - - component.properties = CyclonedxProperties.parse_trivy_source(properties) if properties + component = Component.new(component_data).parse report.add_component(component) if component.ingestible? rescue ::Sbom::PackageUrl::InvalidPackageUrl report.add_error("/components/#{index}/purl is invalid") diff --git a/lib/gitlab/ci/reports/sbom/component.rb b/lib/gitlab/ci/reports/sbom/component.rb index 1a3f689c1d7..6cc588d113c 100644 --- a/lib/gitlab/ci/reports/sbom/component.rb +++ b/lib/gitlab/ci/reports/sbom/component.rb @@ -8,14 +8,15 @@ module Gitlab include Gitlab::Utils::StrongMemoize attr_reader :component_type, :version, :path - attr_accessor :properties + attr_accessor :properties, :purl, :source_package_name - def initialize(type:, name:, purl:, version:, properties: nil) + def initialize(type:, name:, purl:, version:, properties: nil, source_package_name: nil) @component_type = type @name = name - @raw_purl = purl + @purl = purl @version = version @properties = properties + @source_package_name = source_package_name end def <=>(other) @@ -26,13 +27,6 @@ module Gitlab supported_component_type? && supported_purl_type? end - def purl - return unless @raw_purl - - ::Sbom::PackageUrl.parse(@raw_purl) - end - strong_memoize_attr :purl - def purl_type purl.type end diff --git a/spec/factories/ci/reports/sbom/components.rb b/spec/factories/ci/reports/sbom/components.rb index 76bfbe13acb..231fefff99c 100644 --- a/spec/factories/ci/reports/sbom/components.rb +++ b/spec/factories/ci/reports/sbom/components.rb @@ -10,6 +10,7 @@ FactoryBot.define do transient do purl_type { 'npm' } namespace { nil } + source_package_name { nil } end purl do @@ -18,7 +19,7 @@ FactoryBot.define do name: name, namespace: namespace, version: version - ).to_s + ) end skip_create @@ -28,7 +29,8 @@ FactoryBot.define do type: type, name: name, purl: purl, - version: version + version: version, + source_package_name: source_package_name ) end end diff --git a/spec/graphql/types/ci/pipeline_type_spec.rb b/spec/graphql/types/ci/pipeline_type_spec.rb index 26dfc0b10c6..45fe33f34a1 100644 --- a/spec/graphql/types/ci/pipeline_type_spec.rb +++ b/spec/graphql/types/ci/pipeline_type_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Types::Ci::PipelineType do upstream path project active user_permissions warnings commit commit_path uses_needs test_report_summary test_suite ref ref_path warning_messages merge_request_event_type name total_jobs triggered_by_path child source stuck - latest merge_request ref_text failure_reason + latest merge_request ref_text failure_reason yaml_errors trigger ] if Gitlab.ee? diff --git a/spec/lib/bitbucket_server/connection_spec.rb b/spec/lib/bitbucket_server/connection_spec.rb index 59eda91285f..68b35979955 100644 --- a/spec/lib/bitbucket_server/connection_spec.rb +++ b/spec/lib/bitbucket_server/connection_spec.rb @@ -22,32 +22,28 @@ RSpec.describe BitbucketServer::Connection, feature_category: :importers do subject.get(url) end - shared_examples 'handles get requests' do - it 'returns JSON body' do - expect(subject.get(url, { something: 1 })).to eq(payload) - end + it 'returns JSON body' do + expect(subject.get(url, { something: 1 })).to eq(payload) + end - it 'throws an exception if the response is not 200' do - WebMock.stub_request(:get, url).with(headers: { 'Accept' => 'application/json' }).to_return(body: payload.to_json, status: 500, headers: headers) + it 'throws an exception if the response is not 200' do + WebMock.stub_request(:get, url).with(headers: { 'Accept' => 'application/json' }).to_return(body: payload.to_json, status: 500, headers: headers) - expect { subject.get(url) }.to raise_error(described_class::ConnectionError) - end + expect { subject.get(url) }.to raise_error(described_class::ConnectionError) + end - it 'throws an exception if the response is not JSON' do - WebMock.stub_request(:get, url).with(headers: { 'Accept' => 'application/json' }).to_return(body: 'bad data', status: 200, headers: headers) + it 'throws an exception if the response is not JSON' do + WebMock.stub_request(:get, url).with(headers: { 'Accept' => 'application/json' }).to_return(body: 'bad data', status: 200, headers: headers) - expect { subject.get(url) }.to raise_error(described_class::ConnectionError) - end + expect { subject.get(url) }.to raise_error(described_class::ConnectionError) + end - it 'throws an exception upon a network error' do - WebMock.stub_request(:get, url).with(headers: { 'Accept' => 'application/json' }).to_raise(OpenSSL::SSL::SSLError) + it 'throws an exception upon a network error' do + WebMock.stub_request(:get, url).with(headers: { 'Accept' => 'application/json' }).to_raise(OpenSSL::SSL::SSLError) - expect { subject.get(url) }.to raise_error(described_class::ConnectionError) - end + expect { subject.get(url) }.to raise_error(described_class::ConnectionError) end - it_behaves_like 'handles get requests' - context 'when the response is a 429 rate limit reached error' do let(:response) do instance_double(HTTParty::Response, parsed_response: payload, code: 429, headers: headers.merge('retry-after' => '0')) @@ -65,14 +61,6 @@ RSpec.describe BitbucketServer::Connection, feature_category: :importers do expect { subject.get(url) }.to raise_error(BitbucketServer::Connection::ConnectionError) end end - - context 'when the bitbucket_server_importer_exponential_backoff feature flag is disabled' do - before do - stub_feature_flags(bitbucket_server_importer_exponential_backoff: false) - end - - it_behaves_like 'handles get requests' - end end describe '#post' do @@ -88,38 +76,26 @@ RSpec.describe BitbucketServer::Connection, feature_category: :importers do subject.post(url, payload) end - shared_examples 'handles post requests' do - it 'returns JSON body' do - expect(subject.post(url, payload)).to eq(payload) - end - - it 'throws an exception if the response is not 200' do - WebMock.stub_request(:post, url).with(headers: headers).to_return(body: payload.to_json, status: 500, headers: headers) - - expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError) - end + it 'returns JSON body' do + expect(subject.post(url, payload)).to eq(payload) + end - it 'throws an exception upon a network error' do - WebMock.stub_request(:post, url).with(headers: { 'Accept' => 'application/json' }).to_raise(OpenSSL::SSL::SSLError) + it 'throws an exception if the response is not 200' do + WebMock.stub_request(:post, url).with(headers: headers).to_return(body: payload.to_json, status: 500, headers: headers) - expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError) - end + expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError) + end - it 'throws an exception if the URI is invalid' do - stub_request(:post, url).with(headers: { 'Accept' => 'application/json' }).to_raise(URI::InvalidURIError) + it 'throws an exception upon a network error' do + WebMock.stub_request(:post, url).with(headers: { 'Accept' => 'application/json' }).to_raise(OpenSSL::SSL::SSLError) - expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError) - end + expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError) end - it_behaves_like 'handles post requests' - - context 'when the bitbucket_server_importer_exponential_backoff feature flag is disabled' do - before do - stub_feature_flags(bitbucket_server_importer_exponential_backoff: false) - end + it 'throws an exception if the URI is invalid' do + stub_request(:post, url).with(headers: { 'Accept' => 'application/json' }).to_raise(URI::InvalidURIError) - it_behaves_like 'handles post requests' + expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError) end end @@ -141,32 +117,20 @@ RSpec.describe BitbucketServer::Connection, feature_category: :importers do subject.delete(:branches, branch_path, payload) end - shared_examples 'handles delete requests' do - it 'returns JSON body' do - expect(subject.delete(:branches, branch_path, payload)).to eq(payload) - end - - it 'throws an exception if the response is not 200' do - WebMock.stub_request(:delete, branch_url).with(headers: headers).to_return(body: payload.to_json, status: 500, headers: headers) - - expect { subject.delete(:branches, branch_path, payload) }.to raise_error(described_class::ConnectionError) - end + it 'returns JSON body' do + expect(subject.delete(:branches, branch_path, payload)).to eq(payload) + end - it 'throws an exception upon a network error' do - WebMock.stub_request(:delete, branch_url).with(headers: headers).to_raise(OpenSSL::SSL::SSLError) + it 'throws an exception if the response is not 200' do + WebMock.stub_request(:delete, branch_url).with(headers: headers).to_return(body: payload.to_json, status: 500, headers: headers) - expect { subject.delete(:branches, branch_path, payload) }.to raise_error(described_class::ConnectionError) - end + expect { subject.delete(:branches, branch_path, payload) }.to raise_error(described_class::ConnectionError) end - it_behaves_like 'handles delete requests' - - context 'with the bitbucket_server_importer_exponential_backoff feature flag disabled' do - before do - stub_feature_flags(bitbucket_server_importer_exponential_backoff: false) - end + it 'throws an exception upon a network error' do + WebMock.stub_request(:delete, branch_url).with(headers: headers).to_raise(OpenSSL::SSL::SSLError) - it_behaves_like 'handles delete requests' + expect { subject.delete(:branches, branch_path, payload) }.to raise_error(described_class::ConnectionError) end end end diff --git a/spec/lib/gitlab/ci/parsers/sbom/component_spec.rb b/spec/lib/gitlab/ci/parsers/sbom/component_spec.rb new file mode 100644 index 00000000000..17cc3fb21f5 --- /dev/null +++ b/spec/lib/gitlab/ci/parsers/sbom/component_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Parsers::Sbom::Component, feature_category: :dependency_management do + describe "#parse" do + subject(:component) { described_class.new(data).parse } + + context "with dependency scanning component" do + let(:data) do + { + "name" => "activesupport", + "version" => "5.1.4", + "purl" => "pkg:gem/activesupport@5.1.4", + "type" => "library", + "bom-ref" => "pkg:gem/activesupport@5.1.4" + } + end + + it "sets the expected values" do + is_expected.to be_kind_of(::Gitlab::Ci::Reports::Sbom::Component) + + expect(component.component_type).to eq("library") + expect(component.name).to eq("activesupport") + expect(component.version).to eq("5.1.4") + expect(component.purl).to be_kind_of(::Sbom::PackageUrl) + expect(component.purl.name).to eq("activesupport") + expect(component.properties).to be_nil + expect(component.source_package_name).to be_nil + end + end + + context "with container scanning component" do + let(:property_name) { 'aquasecurity:trivy:PkgType' } + let(:property_value) { 'alpine' } + let(:purl) { "pkg:apk/alpine/alpine-baselayout-data@3.4.3-r1?arch=x86_64&distro=3.18.4" } + let(:data) do + { + "name" => "alpine-baselayout-data", + "version" => "3.4.3-r1", + "purl" => purl, + "type" => "library", + "bom-ref" => purl, + "properties" => [ + { + "name" => property_name, + "value" => property_value + } + ] + } + end + + context "with an aquasecurity:trivy:SrcName property" do + let(:property_name) { "aquasecurity:trivy:SrcName" } + let(:property_value) { "alpine-baselayout" } + + it "sets properties field with parsed data" do + property_data = component.properties.data + + expect(property_data).to match({ "SrcName" => "alpine-baselayout" }) + end + + it "sets the source_package_name from the aquasecurity:trivy:SrcName property" do + expect(component.source_package_name).to eq(property_value) + end + end + + context "without an aquasecurity:trivy:SrcName property" do + it "sets properties field with parsed data" do + property_data = component.properties.data + + expect(property_data).to match({ "PkgType" => "alpine" }) + end + + it "sets the source_package_name from the component name" do + expect(component.source_package_name).to eq("alpine-baselayout-data") + end + end + + context "without properties" do + it "sets the source_package_name from the component name" do + data.delete('properties') + expect(component.source_package_name).to eq("alpine-baselayout-data") + end + end + + context "with corrupted purl" do + let(:purl) { "unknown:apk/alpine/alpine-baselayout-data@3.4.3-r1?arch=x86_64&distro=3.18.4" } + + it "raises an error" do + expect { component }.to raise_error(::Sbom::PackageUrl::InvalidPackageUrl) + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/parsers/sbom/cyclonedx_spec.rb b/spec/lib/gitlab/ci/parsers/sbom/cyclonedx_spec.rb index 6a6fe59bce1..0c9a6be7100 100644 --- a/spec/lib/gitlab/ci/parsers/sbom/cyclonedx_spec.rb +++ b/spec/lib/gitlab/ci/parsers/sbom/cyclonedx_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Gitlab::Ci::Parsers::Sbom::Cyclonedx, feature_category: :dependen let(:raw_report_data) { report_data.to_json } let(:report_valid?) { true } let(:validator_errors) { [] } - let(:properties_parser) { class_double('Gitlab::Ci::Parsers::Sbom::CyclonedxProperties') } + let(:properties_parser) { Gitlab::Ci::Parsers::Sbom::CyclonedxProperties } let(:uuid) { 'c9d550a3-feb8-483b-a901-5aa892d039f9' } let(:base_report_data) do @@ -28,8 +28,6 @@ RSpec.describe Gitlab::Ci::Parsers::Sbom::Cyclonedx, feature_category: :dependen allow(validator).to receive(:errors).and_return(validator_errors) end - allow(properties_parser).to receive(:parse_source) - stub_const('Gitlab::Ci::Parsers::Sbom::CyclonedxProperties', properties_parser) allow(SecureRandom).to receive(:uuid).and_return(uuid) end @@ -65,8 +63,9 @@ RSpec.describe Gitlab::Ci::Parsers::Sbom::Cyclonedx, feature_category: :dependen end end - context 'when report has components' do + context 'when report has dependency_scanning components' do let(:report_data) { base_report_data.merge({ 'components' => components }) } + let(:components) do [ { @@ -128,8 +127,8 @@ RSpec.describe Gitlab::Ci::Parsers::Sbom::Cyclonedx, feature_category: :dependen context 'when component is trivy type' do let(:parsed_properties) do { - 'PkgID' => 'adduser@3.134', - 'PkgType' => 'debian' + 'PkgID' => 'activesupport@5.1.4', + 'PkgType' => 'gem' } end @@ -144,28 +143,25 @@ RSpec.describe Gitlab::Ci::Parsers::Sbom::Cyclonedx, feature_category: :dependen "properties" => [ { "name" => "aquasecurity:trivy:PkgID", - "value" => "apt@2.6.1" + "value" => "activesupport@5.1.4" }, { "name" => "aquasecurity:trivy:PkgType", - "value" => "debian" + "value" => "gem" } ] } ] end - before do - allow(properties_parser).to receive(:parse_trivy_source).and_return(parsed_properties) - stub_const('Gitlab::Ci::Parsers::Sbom::CyclonedxProperties', properties_parser) - end - it 'adds each component, ignoring unused attributes' do expect(report).to receive(:add_component) .with( an_object_having_attributes( component_type: "library", - properties: parsed_properties, + properties: an_object_having_attributes( + data: parsed_properties + ), purl: an_object_having_attributes( type: "gem" ) @@ -195,6 +191,74 @@ RSpec.describe Gitlab::Ci::Parsers::Sbom::Cyclonedx, feature_category: :dependen end end + context 'when report has container_scanning components' do + let(:report_data) { base_report_data.merge({ 'components' => components }) } + + let(:parsed_properties) do + { + 'SrcName' => 'alpine-baselayout' + } + end + + let(:components) do + [ + { + "name" => "alpine", + "version" => "3.4.3-r1", + "purl" => "pkg:apk/alpine/alpine@3.4.3-r1?arch=x86_64&distro=3.18.4", + "type" => "library", + "bom-ref" => "pkg:apk/alpine/alpine@3.4.3-r1?arch=x86_64&distro=3.18.4" + }, + { + "name" => "alpine-baselayout-data", + "version" => "3.4.3-r1", + "purl" => "pkg:apk/alpine/alpine-baselayout-data@3.4.3-r1?arch=x86_64&distro=3.18.4", + "type" => "library", + "bom-ref" => "pkg:apk/alpine/alpine-baselayout-data@3.4.3-r1?arch=x86_64&distro=3.18.4", + "properties" => [ + { + "name" => "aquasecurity:trivy:SrcName", + "value" => "alpine-baselayout" + } + ] + } + ] + end + + before do + allow(report).to receive(:add_component) + end + + it 'adds each component, ignoring unused attributes' do + expect(report).to receive(:add_component) + .with( + an_object_having_attributes( + name: "alpine/alpine", + version: "3.4.3-r1", + component_type: "library", + purl: an_object_having_attributes(type: "apk"), + properties: nil, + source_package_name: 'alpine' + ) + ) + expect(report).to receive(:add_component) + .with( + an_object_having_attributes( + name: "alpine/alpine-baselayout-data", + version: "3.4.3-r1", + component_type: "library", + purl: an_object_having_attributes(type: "apk"), + properties: an_object_having_attributes( + data: parsed_properties + ), + source_package_name: 'alpine-baselayout' + ) + ) + + parse! + end + end + context 'when report has metadata tools, author and properties' do let(:report_data) { base_report_data.merge(metadata) } diff --git a/spec/lib/gitlab/ci/reports/sbom/component_spec.rb b/spec/lib/gitlab/ci/reports/sbom/component_spec.rb index 4c9fd00f96a..242c9e4071b 100644 --- a/spec/lib/gitlab/ci/reports/sbom/component_spec.rb +++ b/spec/lib/gitlab/ci/reports/sbom/component_spec.rb @@ -6,15 +6,17 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen let(:component_type) { 'library' } let(:name) { 'component-name' } let(:purl_type) { 'npm' } - let(:purl) { Sbom::PackageUrl.new(type: purl_type, name: name, version: version).to_s } + let(:purl) { Sbom::PackageUrl.new(type: purl_type, name: name, version: version) } let(:version) { 'v0.0.1' } + let(:source_package_name) { 'source-component' } subject(:component) do described_class.new( type: component_type, name: name, purl: purl, - version: version + version: version, + source_package_name: source_package_name ) end @@ -23,7 +25,8 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen component_type: component_type, name: name, purl: an_object_having_attributes(type: purl_type), - version: version + version: version, + source_package_name: source_package_name ) end @@ -34,18 +37,10 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen context 'with namespace' do let(:purl) do - 'pkg:maven/org.NameSpace/Name@v0.0.1' + Sbom::PackageUrl.new(type: 'maven', namespace: 'org.NameSpace', name: 'Name', version: 'v0.0.1') end it { is_expected.to eq('org.NameSpace/Name') } - - context 'when needing normalization' do - let(:purl) do - 'pkg:pypi/org.NameSpace/Name@v0.0.1' - end - - it { is_expected.to eq('org.namespace/name') } - end end end @@ -63,14 +58,18 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen describe '#<=>' do where do + purl_a = Sbom::PackageUrl.new(type: 'npm', name: 'component-a', version: '1.0.0') + purl_b = Sbom::PackageUrl.new(type: 'npm', name: 'component-b', version: '1.0.0') + purl_composer = Sbom::PackageUrl.new(type: 'composer', name: 'component-a', version: '1.0.0') + { 'equal' => { a_name: 'component-a', b_name: 'component-a', a_type: 'library', b_type: 'library', - a_purl: 'pkg:npm/component-a@1.0.0', - b_purl: 'pkg:npm/component-a@1.0.0', + a_purl: purl_a, + b_purl: purl_a, a_version: '1.0.0', b_version: '1.0.0', expected: 0 @@ -80,8 +79,8 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen b_name: 'component-b', a_type: 'library', b_type: 'library', - a_purl: 'pkg:npm/component-a@1.0.0', - b_purl: 'pkg:npm/component-b@1.0.0', + a_purl: purl_a, + b_purl: purl_b, a_version: '1.0.0', b_version: '1.0.0', expected: -1 @@ -91,8 +90,8 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen b_name: 'component-a', a_type: 'library', b_type: 'library', - a_purl: 'pkg:npm/component-b@1.0.0', - b_purl: 'pkg:npm/component-a@1.0.0', + a_purl: purl_b, + b_purl: purl_a, a_version: '1.0.0', b_version: '1.0.0', expected: 1 @@ -102,8 +101,8 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen b_name: 'component-a', a_type: 'library', b_type: 'library', - a_purl: 'pkg:composer/component-a@1.0.0', - b_purl: 'pkg:npm/component-a@1.0.0', + a_purl: purl_composer, + b_purl: purl_a, a_version: '1.0.0', b_version: '1.0.0', expected: -1 @@ -113,8 +112,8 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen b_name: 'component-a', a_type: 'library', b_type: 'library', - a_purl: 'pkg:npm/component-a@1.0.0', - b_purl: 'pkg:composer/component-a@1.0.0', + a_purl: purl_a, + b_purl: purl_composer, a_version: '1.0.0', b_version: '1.0.0', expected: 1 @@ -125,7 +124,7 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen a_type: 'library', b_type: 'library', a_purl: nil, - b_purl: 'pkg:npm/component-a@1.0.0', + b_purl: purl_a, a_version: '1.0.0', b_version: '1.0.0', expected: -1 @@ -135,8 +134,8 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen b_name: 'component-a', a_type: 'library', b_type: 'library', - a_purl: 'pkg:npm/component-a@1.0.0', - b_purl: 'pkg:npm/component-a@1.0.0', + a_purl: purl_a, + b_purl: purl_a, a_version: '1.0.0', b_version: '2.0.0', expected: -1 @@ -146,8 +145,8 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen b_name: 'component-a', a_type: 'library', b_type: 'library', - a_purl: 'pkg:npm/component-a@1.0.0', - b_purl: 'pkg:npm/component-a@1.0.0', + a_purl: purl_a, + b_purl: purl_a, a_version: '2.0.0', b_version: '1.0.0', expected: 1 @@ -157,8 +156,8 @@ RSpec.describe Gitlab::Ci::Reports::Sbom::Component, feature_category: :dependen b_name: 'component-a', a_type: 'library', b_type: 'library', - a_purl: 'pkg:npm/component-a@1.0.0', - b_purl: 'pkg:npm/component-a@1.0.0', + a_purl: purl_a, + b_purl: purl_a, a_version: nil, b_version: '1.0.0', expected: -1 diff --git a/workhorse/.gitignore b/workhorse/.gitignore index 3cd5f522336..6c5018cf841 100644 --- a/workhorse/.gitignore +++ b/workhorse/.gitignore @@ -11,3 +11,4 @@ coverage.html cover.out /*.toml /gitaly.pid +/_support/bin/golangci-* diff --git a/workhorse/.golangci.yml b/workhorse/.golangci.yml new file mode 100644 index 00000000000..ea889783d8b --- /dev/null +++ b/workhorse/.golangci.yml @@ -0,0 +1,366 @@ +# This file contains all available configuration options +# with their default values. + +# options for analysis running +run: + # default concurrency is a available CPU number + # concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 30m + + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + + # include test files or not, default is true + tests: true + + # list of build tags, all linters use it. Default is empty list. + # build-tags: + # - mytag + + # which dirs to skip: issues from them won't be reported; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but default dirs are skipped independently + # from this option's value (see skip-dirs-use-default). + # skip-dirs: + # - src/external_libs + # - autogenerated_by_my_lib + + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default: true + + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + # skip-files: + # - ".*\\.my\\.go$" + # - lib/bad.go + + # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules": + # If invoked with -mod=readonly, the go command is disallowed from the implicit + # automatic updating of go.mod described above. Instead, it fails when any changes + # to go.mod are needed. This setting is most useful to check that go.mod does + # not need updates, such as in a continuous integration and testing system. + # If invoked with -mod=vendor, the go command assumes that the vendor + # directory holds the correct copies of dependencies and ignores + # the dependency descriptions in go.mod. + # modules-download-mode: readonly|release|vendor + +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" + format: line-number + + # print lines of code with issue, default is true + print-issued-lines: true + + # print linter name in the end of issue text, default is true + print-linter-name: true + +# all available settings of specific linters +linters-settings: + errcheck: + # report about not checking of errors in type assetions: `a := b.(MyStruct)`; + # default is false: such cases aren't reported by default. + check-type-assertions: false + + # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; + # default is false: such cases aren't reported by default. + check-blank: false + + # [deprecated] comma-separated list of pairs of the form pkg:regex + # the regex is used to ignore names within pkg. (default "fmt:.*"). + # see https://github.com/kisielk/errcheck#the-deprecated-method for details + # ignore: fmt:.*,io/ioutil:^Read.* + + # path to a file containing a list of functions to exclude from checking + # see https://github.com/kisielk/errcheck#excluding-functions for details + # exclude: /path/to/file.txt + + # Disable error checking, as errorcheck detects more errors and is more configurable. + gosec: + exclude: + - "G104" + + funlen: + lines: 60 + statements: 40 + + govet: + # report about shadowed variables + check-shadowing: true + + # settings per analyzer + settings: + printf: # analyzer name, run `go tool vet help` to see all analyzers + funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf + + # enable or disable analyzers by name + # enable: + # - atomicalign + # enable-all: false + # disable: + # - shadow + # disable-all: false + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + goimports: + # put imports beginning with prefix after 3rd-party packages; + # it's a comma-separated list of prefixes + # local-prefixes: github.com/org/project + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 30 + gocognit: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 20 + maligned: + # print struct with more effective memory layout or not, false by default + suggest-new: true + depguard: + rules: + main: + allow: + - $gostd + - github.com/stretchr/testify + - github.com/alecthomas/chroma/v2 + - gitlab.com/gitlab-org/labkit + - gitlab.com/gitlab-org/gitlab/workhorse + - github.com/sirupsen/logrus + - github.com/gorilla/websocket + - github.com/prometheus/client_golang/prometheus + - github.com/johannesboyne/gofakes3 + - github.com/aws/aws-sdk-go + - github.com/golang-jwt/jwt + - github.com/redis/go-redis + - github.com/sebest/xff + - gitlab.com/gitlab-org/gitaly + - github.com/Azure/azure-sdk-for-go/sdk + - github.com/getsentry/raven-go + - github.com/disintegration/imaging + - github.com/BurntSushi/toml + - github.com/golang/gddo/httputil + - github.com/grpc-ecosystem/go-grpc-prometheus + - github.com/mitchellh/copystructure + - github.com/jpillora/backoff + dupl: + # tokens count to trigger issue, 150 by default + threshold: 100 + goconst: + # minimal length of string constant, 3 by default + min-len: 3 + # minimal occurrences count to trigger, 3 by default + min-occurrences: 3 + # depguard: + # list-type: blacklist + # include-go-root: false + # packages: + # - github.com/sirupsen/logrus + # packages-with-error-messages: + # # specify an error message to output when a blacklisted package is used + # github.com/sirupsen/logrus: "logging is allowed only by logutils.Log" + misspell: + # Correct spellings using locale preferences for US or UK. + # Default is to use a neutral variety of English. + # Setting locale to US will correct the British spelling of 'colour' to 'color'. + locale: US + ignore-words: + - GitLab + lll: + # max line length, lines longer will be reported. Default is 120. + # '\t' is counted as 1 character by default, and can be changed with the tab-width option + line-length: 120 + # tab width in spaces. Default to 1. + tab-width: 1 + unused: + # treat code as a program (not a library) and report unused exported identifiers; default is false. + # XXX: if you enable this setting, unused will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find funcs usages. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + unparam: + # Inspect exported functions, default is false. Set to true if no external program/library imports your code. + # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors: + # if it's called for subdir of a project it can't find external interfaces. All text editor integrations + # with golangci-lint call it on a directory with the changed file. + check-exported: false + nakedret: + # make an issue if func has more lines of code than this setting and it has naked returns; default is 30 + max-func-lines: 30 + prealloc: + # XXX: we don't recommend using this linter before doing performance profiling. + # For most programs usage of prealloc will be a premature optimization. + + # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them. + # True by default. + simple: true + range-loops: true # Report preallocation suggestions on range loops, true by default + for-loops: false # Report preallocation suggestions on for loops, false by default + gocritic: + # Which checks should be enabled; can't be combined with 'disabled-checks'; + # See https://go-critic.github.io/overview#checks-overview + # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run` + # By default list of stable checks is used. + # enabled-checks: + # - rangeValCopy + + # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty + # disabled-checks: + # - regexpMust + + # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks. + # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". + # enabled-tags: + # - performance + + settings: # settings passed to gocritic + captLocal: # must be valid enabled check name + paramsOnly: true + # rangeValCopy: + # sizeThreshold: 32 + godox: + # report any comments starting with keywords, this is useful for TODO or FIXME comments that + # might be left in the code accidentally and should be resolved before merging + keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting + - TODO + - BUG + - FIXME + - NOTE + - OPTIMIZE # marks code that should be optimized before merging + - HACK # marks hack-arounds that should be removed before merging + dogsled: + # checks assignments with too many blank identifiers; default is 2 + max-blank-identifiers: 2 + + whitespace: + multi-if: false # Enforces newlines (or comments) after every multi-line if statement + multi-func: false # Enforces newlines (or comments) after every multi-line function signature + wsl: + # If true append is only allowed to be cuddled if appending value is + # matching variables, fields or types on line above. Default is true. + strict-append: true + # Allow calls and assignments to be cuddled as long as the lines have any + # matching variables, fields or types. Default is true. + allow-assign-and-call: true + # Allow multiline assignments to be cuddled. Default is true. + allow-multiline-assign: true + # Allow declarations (var) to be cuddled. + allow-cuddle-declarations: false + # Allow trailing comments in ending of blocks + allow-trailing-comment: false + # Force newlines in end of case at this limit (0 = never). + force-case-trailing-whitespace: 0 + +linters: + # please, do not use `enable-all`: it's deprecated and will be removed soon. + # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint + disable-all: true + enable: + - bodyclose + - depguard + - dogsled + - dupl + - errcheck + - exportloopref + - funlen + - gocognit + - goconst + - gocritic + - godox + - gofmt + - goimports + - gosec + - gosimple + - govet + - ineffassign + - misspell + - nakedret + - revive + - staticcheck + - stylecheck + - testifylint + - typecheck + - unconvert + - unparam + - unused + - whitespace + # don't enable: + # - deadcode + # - gochecknoglobals + # - gochecknoinits + # - gocyclo + # - lll + # - maligned + # - prealloc + # - varcheck + +issues: + # List of regexps of issue texts to exclude, empty list by default. + # But independently from this option we use default exclude patterns, + # it can be disabled by `exclude-use-default: false`. To list all + # excluded by default patterns execute `golangci-lint run --help` + # exclude: + # - abcdef + + # Excluding configuration per-path, per-linter, per-text and per-source + exclude-rules: + # Exclude some linters from running on tests files. + - path: _test\.go + linters: + - gocyclo + - errcheck + - dupl + - gosec + - funlen + + # Exclude known linters from partially hard-vendored code, + # which is impossible to exclude via "nolint" comments. + # - path: internal/hmac/ + # text: "weak cryptographic primitive" + # linters: + # - gosec + + # Exclude some staticcheck messages + # - linters: + # - staticcheck + # text: "SA9003:" + + # Exclude lll issues for long lines with go:generate + - linters: + - lll + source: "^//go:generate " + + # Independently from option `exclude` we use default exclude patterns, + # it can be disabled by this option. To list all + # excluded by default patterns execute `golangci-lint run --help`. + # Default value for this option is true. + exclude-use-default: false + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter: 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues: 0 + + # Show only new issues: if there are unstaged changes or untracked files, + # only those changes are analyzed, else only changes in HEAD~ are analyzed. + # It's a super-useful option for integration of golangci-lint into existing + # large codebase. It's not practical to fix all existing issues at the moment + # of integration: much better don't allow issues in new code. + # Default is false. + new: false + + # Show only new issues created after git revision `REV` + # This should be passed as flag during individual CI jobs. + # new-from-rev: REV + + # Show only new issues created in git patch with set file path. + # new-from-patch: path/to/patch/file diff --git a/workhorse/Makefile b/workhorse/Makefile index ec937c50c2c..1980db4ed48 100644 --- a/workhorse/Makefile +++ b/workhorse/Makefile @@ -21,6 +21,13 @@ EXE_ALL := gitlab-resize-image gitlab-zip-cat gitlab-zip-metadata gitlab-workhor INSTALL := install BUILD_TAGS := tracer_static tracer_static_jaeger continuous_profiler_stackdriver +OS := $(shell uname | tr A-Z a-z) +ARCH ?= $(shell uname -m | sed -e 's/x86_64/amd64/' | sed -e 's/aarch64/arm64/') + +GOLANGCI_LINT_VERSION := 1.55.2 +GOLANGCI_LINT_ARCH ?= ${ARCH} +GOLANGCI_LINT_FILE := _support/bin/golangci-lint-${GOLANGCI_LINT_VERSION} + ifeq (${FIPS_MODE}, 1) # boringcrypto tag is added automatically by golang-fips compiler BUILD_TAGS += fips @@ -155,6 +162,14 @@ lint: go install golang.org/x/lint/golint @_support/lint.sh ./... +.PHONY: golangci +golangci: ${GOLANGCI_LINT_FILE} + ${GOLANGCI_LINT_FILE} run --issues-exit-code 0 --print-issued-lines=false ${GOLANGCI_LINT_ARGS} + +${GOLANGCI_LINT_FILE}: + mkdir -p $(shell dirname ${GOLANGCI_LINT_FILE}) + curl -L https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/golangci-lint-${GOLANGCI_LINT_VERSION}-${OS}-${GOLANGCI_LINT_ARCH}.tar.gz | tar --strip-components 1 -zOxf - golangci-lint-${GOLANGCI_LINT_VERSION}-${OS}-${GOLANGCI_LINT_ARCH}/golangci-lint > ${GOLANGCI_LINT_FILE} && chmod +x ${GOLANGCI_LINT_FILE} + .PHONY: vet vet: $(call message,Verify: $@) diff --git a/workhorse/internal/config/config_test.go b/workhorse/internal/config/config_test.go index e7b8462a4a7..03cad69d335 100644 --- a/workhorse/internal/config/config_test.go +++ b/workhorse/internal/config/config_test.go @@ -1,7 +1,6 @@ package config import ( - "os" "path/filepath" "testing" @@ -150,8 +149,7 @@ func TestRegisterGoCloudGoogleURLOpenersWithApplicationDefault(t *testing.T) { path, err := filepath.Abs("../../testdata/google_dummy_credentials.json") require.NoError(t, err) - os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", path) - defer os.Unsetenv("GOOGLE_APPLICATION_CREDENTIALS") + t.Setenv("GOOGLE_APPLICATION_CREDENTIALS", path) testRegisterGoCloudURLOpener(t, cfg, "gs") } |