Automatic merge of gitlab-org/gitlab master

.gitlab/ci/workhorse.gitlab-ci.yml

@@ -9,24 +9,21 @@ workhorse:verify:
 
 .workhorse:test:
   extends: .workhorse:rules:workhorse
-  services:
-    - name: registry.gitlab.com/gitlab-org/build/cng/gitaly:latest
-      # Disable the hooks so we don't have to stub the GitLab API
-      command: ["/usr/bin/env", "GITALY_TESTING_NO_GIT_HOOKS=1", "/scripts/process-wrapper"]
-      alias: gitaly
   variables:
-    GITALY_ADDRESS: "tcp://gitaly:8075"
+    GITALY_ADDRESS: "tcp://127.0.0.1:8075"
   stage: test
-  needs: []
+  needs:
+    - setup-test-env
   script:
     - go version
     - apt-get update && apt-get -y install libimage-exiftool-perl
+    - scripts/gitaly-test-build
     - make -C workhorse test
 
 workhorse:test using go 1.16:
   extends: .workhorse:test
-  image: ${GITLAB_DEPENDENCY_PROXY}golang:1.16
+  image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7-golang-1.16-git-2.31
 
 workhorse:test using go 1.17:
   extends: .workhorse:test
-  image: ${GITLAB_DEPENDENCY_PROXY}golang:1.17
+  image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7-golang-1.17-git-2.31

doc/ci/index.md

@@ -129,13 +129,17 @@ See also:
 
 - [Enable or disable GitLab CI/CD in a project](enable_or_disable_ci.md).
 
-## References
+## Related topics
 
 Learn more about GitLab CI/CD:
 
 - [Why you might choose GitLab CI/CD](https://about.gitlab.com/blog/2016/10/17/gitlab-ci-oohlala/).
 - [Reasons you might migrate from another platform](https://about.gitlab.com/blog/2016/07/22/building-our-web-app-on-gitlab-ci/).
 - [5 Teams that made the switch to GitLab CI/CD](https://about.gitlab.com/blog/2019/04/25/5-teams-that-made-the-switch-to-gitlab-ci-cd/)
+- If you use VS Code to edit your GitLab CI/CD configuration, the
+  [GitLab Workflow VS Code extension](../user/project/repository/vscode.md) helps you
+  [validate your configuration](https://marketplace.visualstudio.com/items?itemName=GitLab.gitlab-workflow#validate-gitlab-ci-configuration)
+  and [view your pipeline status](https://marketplace.visualstudio.com/items?itemName=GitLab.gitlab-workflow#information-about-your-branch-pipelines-mr-closing-issue).
 
 See also the [Why CI/CD?](https://docs.google.com/presentation/d/1OGgk2Tcxbpl7DJaIOzCX4Vqg3dlwfELC3u2jEeCBbDk) presentation.
 

doc/ci/lint.md

@@ -14,6 +14,9 @@ issues as well.
 To access the CI Lint tool, navigate to **CI/CD > Pipelines** or **CI/CD > Jobs**
 in your project and click **CI lint**.
 
+If you use VS Code, you can also validate your CI/CD configuration with the
+[GitLab Workflow VS Code extension](../user/project/repository/vscode.md).
+
 ## Validate basic logic and syntax
 
 By default, the CI lint checks the syntax of your CI YAML configuration and also runs

doc/ci/pipelines/index.md

@@ -127,6 +127,11 @@ you can filter the pipeline list by:
 [Starting in GitLab 14.2](https://gitlab.com/gitlab-org/gitlab/-/issues/26621), you can change the
 pipeline column to display the pipeline ID or the pipeline IID.
 
+If you use VS Code to edit your GitLab CI/CD configuration, the
+[GitLab Workflow VS Code extension](../../user/project/repository/vscode.md) helps you
+[validate your configuration](https://marketplace.visualstudio.com/items?itemName=GitLab.gitlab-workflow#validate-gitlab-ci-configuration)
+and [view your pipeline status](https://marketplace.visualstudio.com/items?itemName=GitLab.gitlab-workflow#information-about-your-branch-pipelines-mr-closing-issue).
+
 ### Run a pipeline manually
 
 Pipelines can be manually executed, with predefined or manually-specified [variables](../variables/index.md).

doc/development/testing_guide/best_practices.md

@@ -64,6 +64,9 @@ use the `GITLAB_TEST_EAGER_LOAD` environment variable:
 GITLAB_TEST_EAGER_LOAD=1 bin/rspec spec/models/project_spec.rb
 ```
 
+If your test depends on all the application code that is being loaded, add the `:eager_load` tag.
+This ensures that the application code is eagerly loaded before the test execution.
+
 ### Ruby warnings
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47767) in GitLab 13.7.

ee/spec/models/concerns/elastic/note_spec.rb

@@ -192,7 +192,7 @@ RSpec.describe Note, :elastic, :clean_gitlab_redis_shared_state do
     note.update!(note: 'some other text here')
   end
 
-  it 'uses same index for Note subclasses' do
+  it 'uses same index for Note subclasses', :eager_load do
     Note.subclasses.each do |note_class|
       expect(note_class.index_name).to eq(Note.index_name)
       expect(note_class.document_type).to eq(Note.document_type)

ee/spec/models/concerns/elastic/snippet_spec.rb

@@ -63,7 +63,7 @@ RSpec.describe Snippet, :elastic do
     expect(snippet.__elasticsearch__.as_indexed_json).to eq(expected_hash)
   end
 
-  it 'uses same index for Snippet subclasses' do
+  it 'uses same index for Snippet subclasses', :eager_load do
     Snippet.subclasses.each do |snippet_class|
       expect(snippet_class.index_name).to eq(Snippet.index_name)
       expect(snippet_class.document_type).to eq(Snippet.document_type)

spec/db/schema_spec.rb

@@ -210,7 +210,7 @@ RSpec.describe 'Database schema' do
 
   # We are skipping GEO models for now as it adds up complexity
   describe 'for jsonb columns' do
-    it 'uses json schema validator' do
+    it 'uses json schema validator', :eager_load do
       columns_name_with_jsonb.each do |hash|
         next if models_by_table_name[hash["table_name"]].nil?
 

spec/lib/banzai/pipeline/plain_markdown_pipeline_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe Banzai::Pipeline::PlainMarkdownPipeline do
         expect(result[:escaped_literals]).to be_truthy
       end
 
-      it 'ensure we handle all the GitLab reference characters' do
+      it 'ensure we handle all the GitLab reference characters', :eager_load do
         reference_chars = ObjectSpace.each_object(Class).map do |klass|
           next unless klass.included_modules.include?(Referable)
           next unless klass.respond_to?(:reference_prefix)

spec/models/concerns/reactive_caching_spec.rb

@@ -375,7 +375,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do
   end
 
   describe 'classes including this concern' do
-    it 'sets reactive_cache_work_type' do
+    it 'sets reactive_cache_work_type', :eager_load do
       classes = ObjectSpace.each_object(Class).select do |klass|
         klass < described_class && klass.name
       end

spec/services/merge_requests/mergeability/run_checks_service_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do
   let_it_be(:merge_request) { create(:merge_request) }
 
   describe '#CHECKS' do
-    it 'contains every subclass of the base checks service' do
+    it 'contains every subclass of the base checks service', :eager_load do
       expect(described_class::CHECKS).to contain_exactly(*MergeRequests::Mergeability::CheckBaseService.subclasses)
     end
   end
@@ -19,7 +19,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do
     let(:params) { {} }
     let(:success_result) { Gitlab::MergeRequests::Mergeability::CheckResult.success }
 
-    context 'when every check is skipped' do
+    context 'when every check is skipped', :eager_load do
       before do
         MergeRequests::Mergeability::CheckBaseService.subclasses.each do |subclass|
           expect_next_instance_of(subclass) do |service|

spec/spec_helper.rb

@@ -431,6 +431,10 @@ RSpec.configure do |config|
     Gitlab::Metrics.reset_registry!
   end
 
+  config.before(:example, :eager_load) do
+    Rails.application.eager_load!
+  end
+
   # This makes sure the `ApplicationController#can?` method is stubbed with the
   # original implementation for all view specs.
   config.before(:each, type: :view) do

workhorse/Makefile

@@ -13,6 +13,8 @@ else
 	BUILD_TIME := $(shell date -u "$(DATE_FMT)")
 endif
 GOBUILD := go build -ldflags "-X main.Version=$(VERSION_STRING) -X main.BuildTime=$(BUILD_TIME)"
+GITALY  := tmp/tests/gitaly/_build/bin/gitaly
+GITALY_PID_FILE := gitaly.pid
 EXE_ALL := gitlab-resize-image gitlab-zip-cat gitlab-zip-metadata gitlab-workhorse
 INSTALL := install
 BUILD_TAGS := tracer_static tracer_static_jaeger continuous_profiler_stackdriver
@@ -63,7 +65,16 @@ install: $(EXE_ALL)
 .PHONY:	test
 test: prepare-tests
 	$(call message,$@)
-	@go test -tags "$(BUILD_TAGS)" ./...
+	go test -tags "$(BUILD_TAGS)" ./... ;\
+	status="$$?" ;\
+	if [ -f "$(GITALY_PID_FILE)" ] ; then \
+		echo "Clean up Gitaly server for workhorse integration test" ;\
+		kill -9 $$(cat $(GITALY_PID_FILE)) ;\
+		rm $(GITALY_PID_FILE) ;\
+	else \
+		echo "Gitaly integration test not running" ;\
+	fi ;\
+	exit "$$status"
 	@echo SUCCESS
 
 .PHONY:	clean
@@ -82,9 +93,27 @@ clean-build:
 	rm -rf $(TARGET_DIR)
 
 .PHONY:	prepare-tests
+prepare-tests:	run-gitaly
 prepare-tests:	testdata/data/group/test.git $(EXE_ALL)
 prepare-tests:	testdata/scratch
 
+.PHONY: run-gitaly
+run-gitaly: gitaly.pid
+
+$(GITALY_PID_FILE): gitaly.toml
+	@{ \
+	if [ -z "$${GITALY_ADDRESS+x}" ] ; then \
+		echo "To run gitaly integration tests set GITALY_ADDRESS=tcp://127.0.0.1:8075" ; \
+	else \
+		cd .. ; \
+		GITALY_TESTING_NO_GIT_HOOKS=1 GITALY_PID_FILE=workhorse/$(GITALY_PID_FILE) $(GITALY) workhorse/gitaly.toml ; \
+	fi \
+	} &
+
+gitaly.toml: ../tmp/tests/gitaly/config.toml
+	sed -e 's/^socket_path.*$$/listen_addr = "0.0.0.0:8075"/;s/^\[auth\]$$//;s/^token.*$$//;s/^internal_socket_dir.*$$//' \
+		$< > $@
+
 testdata/data/group/test.git:
 	$(call message,$@)
 	git clone --quiet --bare https://gitlab.com/gitlab-org/gitlab-test.git $@

workhorse/gitaly_integration_test.go

@@ -16,6 +16,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 
@@ -53,6 +54,10 @@ func realGitalyOkBody(t *testing.T) *api.Response {
 	return realGitalyAuthResponse(gitOkBody(t))
 }
 
+func realGitalyOkBodyWithSidechannel(t *testing.T) *api.Response {
+	return realGitalyAuthResponse(gitOkBodyWithSidechannel(t))
+}
+
 func ensureGitalyRepository(t *testing.T, apiResponse *api.Response) error {
 	ctx, namespace, err := gitaly.NewNamespaceClient(context.Background(), apiResponse.GitalyServer)
 	if err != nil {
@@ -83,10 +88,18 @@ func ensureGitalyRepository(t *testing.T, apiResponse *api.Response) error {
 }
 
 func TestAllowedClone(t *testing.T) {
+	testAllowedClone(t, realGitalyOkBody(t))
+}
+
+func TestAllowedCloneWithSidechannel(t *testing.T) {
+	gitaly.InitializeSidechannelRegistry(logrus.StandardLogger())
+	testAllowedClone(t, realGitalyOkBodyWithSidechannel(t))
+}
+
+func testAllowedClone(t *testing.T, apiResponse *api.Response) {
 	skipUnlessRealGitaly(t)
 
 	// Create the repository in the Gitaly server
-	apiResponse := realGitalyOkBody(t)
 	require.NoError(t, ensureGitalyRepository(t, apiResponse))
 
 	// Prepare test server and backend
@@ -107,10 +120,18 @@ func TestAllowedClone(t *testing.T) {
 }
 
 func TestAllowedShallowClone(t *testing.T) {
+	testAllowedShallowClone(t, realGitalyOkBody(t))
+}
+
+func TestAllowedShallowCloneWithSidechannel(t *testing.T) {
+	gitaly.InitializeSidechannelRegistry(logrus.StandardLogger())
+	testAllowedShallowClone(t, realGitalyOkBodyWithSidechannel(t))
+}
+
+func testAllowedShallowClone(t *testing.T, apiResponse *api.Response) {
 	skipUnlessRealGitaly(t)
 
 	// Create the repository in the Gitaly server
-	apiResponse := realGitalyOkBody(t)
 	require.NoError(t, ensureGitalyRepository(t, apiResponse))
 
 	// Prepare test server and backend