From 2d0c3ea015e9be40f5dbd2e7894a28ff6b7c277b Mon Sep 17 00:00:00 2001
From: Andrejs Cunskis <acunskis@gitlab.com>
Date: Thu, 13 Jun 2024 12:14:20 +0000
Subject: [PATCH] Correctly set gitaly semver tag in cng orchestrator

---
 .../cng/lib/deployment/default_values.rb      | 12 ++-
 .../spec/fixture/GITALY_SERVER_VERSION        |  1 -
 .../spec/fixture/GITLAB_SHELL_VERSION         |  1 -
 .../cng/deployment/default_values_spec.rb     | 84 +++++++++++++++++++
 .../cng/deployment/installation_spec.rb       | 65 ++++----------
 5 files changed, 109 insertions(+), 54 deletions(-)
 delete mode 100644 gems/gitlab-cng/spec/fixture/GITALY_SERVER_VERSION
 delete mode 100644 gems/gitlab-cng/spec/fixture/GITLAB_SHELL_VERSION
 create mode 100644 gems/gitlab-cng/spec/unit/gitlab/cng/deployment/default_values_spec.rb

diff --git a/gems/gitlab-cng/lib/gitlab/cng/lib/deployment/default_values.rb b/gems/gitlab-cng/lib/gitlab/cng/lib/deployment/default_values.rb
index d5368757d4616..3fa705b55f635 100644
--- a/gems/gitlab-cng/lib/gitlab/cng/lib/deployment/default_values.rb
+++ b/gems/gitlab-cng/lib/gitlab/cng/lib/deployment/default_values.rb
@@ -48,7 +48,7 @@ def common_values(domain)
           def component_ci_versions
             {
               "gitlab.gitaly.image.repository" => "#{IMAGE_REPOSITORY}/gitaly",
-              "gitlab.gitaly.image.tag" => gitaly_version,
+              "gitlab.gitaly.image.tag" => semver?(gitaly_version) ? "v#{gitaly_version}" : gitaly_version,
               "gitlab.gitlab-shell.image.repository" => "#{IMAGE_REPOSITORY}/gitlab-shell",
               "gitlab.gitlab-shell.image.tag" => "v#{gitlab_shell_version}",
               "gitlab.migrations.image.repository" => "#{IMAGE_REPOSITORY}/gitlab-toolbox-ee",
@@ -65,6 +65,16 @@ def component_ci_versions
               "gitlab.webservice.workhorse.tag" => commit_sha
             }
           end
+
+          private
+
+          # Semver compatible version
+          #
+          # @param [String] version
+          # @return [Boolean]
+          def semver?(version)
+            version.match?(/^[0-9]+\.[0-9]+\.[0-9]+(-rc[0-9]+)?(-ee)?$/)
+          end
         end
       end
     end
diff --git a/gems/gitlab-cng/spec/fixture/GITALY_SERVER_VERSION b/gems/gitlab-cng/spec/fixture/GITALY_SERVER_VERSION
deleted file mode 100644
index 4d8daed28d96a..0000000000000
--- a/gems/gitlab-cng/spec/fixture/GITALY_SERVER_VERSION
+++ /dev/null
@@ -1 +0,0 @@
-7aa06a578d76bdc294ee8e9acb4f063e7d9f1d5f
diff --git a/gems/gitlab-cng/spec/fixture/GITLAB_SHELL_VERSION b/gems/gitlab-cng/spec/fixture/GITLAB_SHELL_VERSION
deleted file mode 100644
index aab9f5577020b..0000000000000
--- a/gems/gitlab-cng/spec/fixture/GITLAB_SHELL_VERSION
+++ /dev/null
@@ -1 +0,0 @@
-14.35.0
diff --git a/gems/gitlab-cng/spec/unit/gitlab/cng/deployment/default_values_spec.rb b/gems/gitlab-cng/spec/unit/gitlab/cng/deployment/default_values_spec.rb
new file mode 100644
index 0000000000000..8e1be77b89d65
--- /dev/null
+++ b/gems/gitlab-cng/spec/unit/gitlab/cng/deployment/default_values_spec.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+RSpec.describe Gitlab::Cng::Deployment::DefaultValues do
+  let(:ci_project_dir) { "/builds/dir" }
+  let(:ci_commit_sha) { "0acb5ee6db0860436fafc2c31a2cd87849c51aa3" }
+  let(:ci_short_sha) { "0acb5ee6db08" }
+  let(:image_repository) { "registry.gitlab.com/gitlab-org/build/cng-mirror" }
+  let(:gitaly_version) { "7aa06a578d76bdc294ee8e9acb4f063e7d9f1d5f" }
+  let(:shell_version) { "14.0.5" }
+
+  let(:env) do
+    {
+      "CI_PROJECT_DIR" => ci_project_dir,
+      "CI_COMMIT_SHA" => ci_commit_sha,
+      "CI_COMMIT_SHORT_SHA" => ci_short_sha
+    }
+  end
+
+  before do
+    described_class.instance_variable_set(:@ci_project_dir, nil)
+    described_class.instance_variable_set(:@gitaly_version, nil)
+
+    allow(File).to receive(:read).with(File.join(ci_project_dir, "GITALY_SERVER_VERSION")).and_return(gitaly_version)
+    allow(File).to receive(:read).with(File.join(ci_project_dir, "GITLAB_SHELL_VERSION")).and_return(shell_version)
+  end
+
+  around do |example|
+    ClimateControl.modify(env) { example.run }
+  end
+
+  it "returns correct common values" do
+    expect(described_class.common_values("domain")).to eq({
+      global: {
+        hosts: {
+          domain: "domain",
+          https: false
+        },
+        ingress: {
+          configureCertmanager: false,
+          tls: {
+            enabled: false
+          }
+        },
+        appConfig: {
+          applicationSettingsCacheSeconds: 0
+        }
+      },
+      gitlab: { "gitlab-exporter": { enabled: false } },
+      redis: { metrics: { enabled: false } },
+      prometheus: { install: false },
+      certmanager: { install: false },
+      "gitlab-runner": { install: false }
+    })
+  end
+
+  it "returns correct ci components" do
+    expect(described_class.component_ci_versions).to eq({
+      "gitlab.gitaly.image.repository" => "#{image_repository}/gitaly",
+      "gitlab.gitaly.image.tag" => gitaly_version,
+      "gitlab.gitlab-shell.image.repository" => "#{image_repository}/gitlab-shell",
+      "gitlab.gitlab-shell.image.tag" => "v#{shell_version}",
+      "gitlab.migrations.image.repository" => "#{image_repository}/gitlab-toolbox-ee",
+      "gitlab.migrations.image.tag" => ci_commit_sha,
+      "gitlab.toolbox.image.repository" => "#{image_repository}/gitlab-toolbox-ee",
+      "gitlab.toolbox.image.tag" => ci_commit_sha,
+      "gitlab.sidekiq.annotations.commit" => ci_short_sha,
+      "gitlab.sidekiq.image.repository" => "#{image_repository}/gitlab-sidekiq-ee",
+      "gitlab.sidekiq.image.tag" => ci_commit_sha,
+      "gitlab.webservice.annotations.commit" => ci_short_sha,
+      "gitlab.webservice.image.repository" => "#{image_repository}/gitlab-webservice-ee",
+      "gitlab.webservice.image.tag" => ci_commit_sha,
+      "gitlab.webservice.workhorse.image" => "#{image_repository}/gitlab-workhorse-ee",
+      "gitlab.webservice.workhorse.tag" => ci_commit_sha
+    })
+  end
+
+  context "with semver gitaly version" do
+    let(:gitaly_version) { "17.0.1" }
+
+    it "correctly sets gitaly image tag" do
+      expect(described_class.component_ci_versions["gitlab.gitaly.image.tag"]).to eq("v#{gitaly_version}")
+    end
+  end
+end
diff --git a/gems/gitlab-cng/spec/unit/gitlab/cng/deployment/installation_spec.rb b/gems/gitlab-cng/spec/unit/gitlab/cng/deployment/installation_spec.rb
index 337cc92e6e8b5..40ade8969fbf7 100644
--- a/gems/gitlab-cng/spec/unit/gitlab/cng/deployment/installation_spec.rb
+++ b/gems/gitlab-cng/spec/unit/gitlab/cng/deployment/installation_spec.rb
@@ -38,44 +38,18 @@
       )
     end
 
-    let(:env) do
-      {
-        "QA_EE_LICENSE" => "license",
-        "CI_PROJECT_DIR" => File.expand_path("../../../../fixture", __dir__),
-        "CI_COMMIT_SHA" => "0acb5ee6db0860436fafc2c31a2cd87849c51aa3",
-        "CI_COMMIT_SHORT_SHA" => "0acb5ee6db08"
-      }
-    end
-
-    let(:values_yml) do
+    let(:expected_values_yml) do
       {
         global: {
-          hosts: {
-            domain: gitlab_domain,
-            https: false
-          },
-          ingress: {
-            configureCertmanager: false,
-            tls: {
-              enabled: false
-            }
-          },
-          appConfig: {
-            applicationSettingsCacheSeconds: 0
-          },
+          common: "val",
           extraEnv: {
             GITLAB_LICENSE_MODE: "test",
             CUSTOMER_PORTAL_URL: "https://customers.staging.gitlab.com"
           }
         },
         gitlab: {
-          "gitlab-exporter": { enabled: false },
           license: { secret: "gitlab-license" }
         },
-        redis: { metrics: { enabled: false } },
-        prometheus: { install: false },
-        certmanager: { install: false },
-        "gitlab-runner": { install: false },
         **config_values
       }.deep_stringify_keys.to_yaml
     end
@@ -85,12 +59,15 @@
       allow(Gitlab::Cng::Kubectl::Client).to receive(:new).with("gitlab").and_return(kubeclient)
       allow(Gitlab::Cng::Helm::Client).to receive(:new).and_return(helmclient)
       allow(Gitlab::Cng::Deployment::Configurations::Kind).to receive(:new).and_return(configuration)
+      allow(Gitlab::Cng::Deployment::DefaultValues).to receive(:common_values).with(gitlab_domain).and_return({
+        global: { common: "val" }
+      })
 
       allow(installation).to receive(:execute_shell)
     end
 
     around do |example|
-      ClimateControl.modify(env) { example.run }
+      ClimateControl.modify({ "QA_EE_LICENSE" => "license" }) { example.run }
     end
 
     context "without ci" do
@@ -105,7 +82,7 @@
           chart_reference,
           namespace: "gitlab",
           timeout: "10m",
-          values: values_yml,
+          values: expected_values_yml,
           args: []
         )
 
@@ -121,6 +98,11 @@
     context "with ci and specific sha" do
       let(:ci) { true }
       let(:chart_sha) { "sha" }
+      let(:ci_components) { { "gitlab.gitaly.image.repository" => "repo", "gitlab.gitaly.image.tag" => "tag" } }
+
+      before do
+        allow(Gitlab::Cng::Deployment::DefaultValues).to receive(:component_ci_versions).and_return(ci_components)
+      end
 
       it "runs helm install with correctly merged values and component versions" do
         expect { installation.create }.to output(/Creating CNG deployment 'gitlab'/).to_stdout
@@ -131,27 +113,8 @@
           chart_reference,
           namespace: "gitlab",
           timeout: "10m",
-          values: values_yml,
-          # rubocop:disable Layout/LineLength -- fitting the args in to 120 would make the definition quite unreadable
-          args: %W[
-            --set gitlab.gitaly.image.repository=registry.gitlab.com/gitlab-org/build/cng-mirror/gitaly
-            --set gitlab.gitaly.image.tag=7aa06a578d76bdc294ee8e9acb4f063e7d9f1d5f
-            --set gitlab.gitlab-shell.image.repository=registry.gitlab.com/gitlab-org/build/cng-mirror/gitlab-shell
-            --set gitlab.gitlab-shell.image.tag=v14.35.0
-            --set gitlab.migrations.image.repository=registry.gitlab.com/gitlab-org/build/cng-mirror/gitlab-toolbox-ee
-            --set gitlab.migrations.image.tag=#{env['CI_COMMIT_SHA']}
-            --set gitlab.toolbox.image.repository=registry.gitlab.com/gitlab-org/build/cng-mirror/gitlab-toolbox-ee
-            --set gitlab.toolbox.image.tag=#{env['CI_COMMIT_SHA']}
-            --set gitlab.sidekiq.annotations.commit=#{env['CI_COMMIT_SHORT_SHA']}
-            --set gitlab.sidekiq.image.repository=registry.gitlab.com/gitlab-org/build/cng-mirror/gitlab-sidekiq-ee
-            --set gitlab.sidekiq.image.tag=#{env['CI_COMMIT_SHA']}
-            --set gitlab.webservice.annotations.commit=#{env['CI_COMMIT_SHORT_SHA']}
-            --set gitlab.webservice.image.repository=registry.gitlab.com/gitlab-org/build/cng-mirror/gitlab-webservice-ee
-            --set gitlab.webservice.image.tag=#{env['CI_COMMIT_SHA']}
-            --set gitlab.webservice.workhorse.image=registry.gitlab.com/gitlab-org/build/cng-mirror/gitlab-workhorse-ee
-            --set gitlab.webservice.workhorse.tag=#{env['CI_COMMIT_SHA']}
-          ]
-          # rubocop:enable Layout/LineLength
+          values: expected_values_yml,
+          args: ci_components.flat_map { |k, v| ["--set", "#{k}=#{v}"] }
         )
       end
     end
-- 
GitLab