From 1f06b3712037982c3773ffb69a4ae55e18a52665 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 21 May 2024 10:48:28 -0700
Subject: [PATCH] Fix toolbox deployments when no backup config is provided

In cf1ecb188 we simplified the condition for defining a backup config
secret as part of
https://gitlab.com/gitlab-org/charts/gitlab/-/merge_requests/3699. However,
if a GitLab admin disabled MinIO, the change would require the admin
to configure a backup secret unless Google Cloud Storage (GCS) were
used as the backend.

Restore the previous behavior of allowing the toolbox to be deployed
even if no backups are enabled. We only need to check if that value is
set for object storage to be configured. We can drop the check on
`global.minio.enabled` because:

1. For the internal MinIO instance,
   `gitlab.toolbox.backups.objectStorage.config` can be left empty.

2. If the internal MinIO instance is disabled, then that means either
   backups are configured via S3/Azure/GCS or not at all. In the
   former case, `gitlab.toolbox.backups.objectStorage.config` must not
   be empty. If the latter case, then
   `gitlab.toolbox.backups.objectStorage.config` can be left blank.

Relates to https://gitlab.com/gitlab-org/charts/gitlab/-/issues/5536

Changelog: fixed
---
 spec/integration/check_config/toolbox_spec.rb | 87 +++++++++++++++++++
 templates/_checkConfig_toolbox.tpl            |  2 +-
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/spec/integration/check_config/toolbox_spec.rb b/spec/integration/check_config/toolbox_spec.rb
index 0217a0509c..9395f3668f 100644
--- a/spec/integration/check_config/toolbox_spec.rb
+++ b/spec/integration/check_config/toolbox_spec.rb
@@ -65,6 +65,93 @@ describe 'checkConfig toolbox' do
                        success_description: 'when toolbox has a valid object storage backup secret configured',
                        error_description: 'when toolbox does not have a valid object storage backup secret configured'
 
+      describe 'with MinIO disabled, backups configured, and consolidated object storage enabled' do
+        let(:success_values) do
+          YAML.safe_load(%(
+            global:
+              appConfig:
+                object_store:
+                  enabled: true
+                  connection:
+                    secret: gitlab-object-storage
+                    key: connection
+              gitlab:
+                toolbox:
+                  enabled: true
+                  backups:
+                    objectStorage:
+                      config:
+                        secret: s3cmd-config
+                        key: config
+              minio:
+                enabled: false
+          )).merge(default_required_values)
+        end
+
+        let(:error_values) do
+          YAML.safe_load(%(
+            gitlab:
+              toolbox:
+                backups:
+                  objectStorage:
+                    config:
+                      secret:
+            )).deep_merge(success_values)
+        end
+
+        let(:error_output) { 'A valid object storage config secret is needed for backups.' }
+
+        include_examples 'config validation',
+                         success_description: 'when toolbox has MinIO disabled but no object storage config',
+                         error_description: 'when toolbox has MinIO disabled but no valid object storage backup secret'
+      end
+
+      describe 'with MinIO disabled, backups not configured, and type-specific object storage enabled' do
+        let(:success_values) do
+          YAML.safe_load(%(
+          global:
+            appConfig:
+              artifacts:
+                connection:
+                  secret: gitlab-object-storage
+                  key: connection
+              lfs:
+                connection:
+                  secret: gitlab-object-storage
+                  key: connection
+              packages:
+                connection:
+                  secret: gitlab-object-storage
+                  key: connection
+              uploads:
+                connection:
+                  secret: gitlab-object-storage
+                  key: connection
+            minio:
+              enabled: false
+          )).merge(default_required_values)
+        end
+
+        let(:error_values) do
+          YAML.safe_load(%(
+            gitlab:
+              toolbox:
+                enabled: true
+                backups:
+                  objectStorage:
+                    config:
+                      # secret: s3cmd-config
+                      key: config
+          )).deep_merge(success_values)
+        end
+
+        let(:error_output) { 'A valid object storage config secret is needed for backups.' }
+
+        include_examples 'config validation',
+                         success_description: 'when toolbox has MinIO disabled but no object storage config',
+                         error_description: 'when toolbox has MinIO disabled but no valid object storage backup secret'
+      end
+
       context 'with Google Cloud Storage backend' do
         let(:success_values) do
           YAML.safe_load(%(
diff --git a/templates/_checkConfig_toolbox.tpl b/templates/_checkConfig_toolbox.tpl
index 3bde44d493..f58f72ca71 100644
--- a/templates/_checkConfig_toolbox.tpl
+++ b/templates/_checkConfig_toolbox.tpl
@@ -5,7 +5,7 @@ no secrets have to be configured.
 */}}
 {{- define "gitlab.toolbox.backups.objectStorage.config.secret" -}}
 {{-   if .Values.gitlab.toolbox.enabled -}}
-{{-     if or .Values.gitlab.toolbox.backups.objectStorage.config (not .Values.global.minio.enabled) -}}
+{{-     if .Values.gitlab.toolbox.backups.objectStorage.config -}}
 {{-       if and (not (eq .Values.gitlab.toolbox.backups.objectStorage.backend "gcs")) (not .Values.gitlab.toolbox.backups.objectStorage.config.secret) -}}
 toolbox:
     A valid object storage config secret is needed for backups.
-- 
GitLab