|
| 1 | +From 4c0173293907af9cb942a6683c465c3f1e9fdb5c Mon Sep 17 00:00:00 2001 |
| 2 | +From: Tim Vaillancourt <tim@timvaillancourt.com> |
| 3 | +Date: Tue, 24 Feb 2026 20:21:37 +0100 |
| 4 | +Subject: [PATCH] Restore: make loading compressor commands from `MANIFEST` |
| 5 | + opt-in (#19460) |
| 6 | + |
| 7 | +Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com> |
| 8 | +Co-authored-by: Mohamed Hamza <mhamza@fastmail.com> |
| 9 | + |
| 10 | +Upstream Patch reference: https://github.com/vitessio/vitess/commit/4c0173293907af9cb942a6683c465c3f1e9fdb5c.patch |
| 11 | +--- |
| 12 | + go/flags/endtoend/vtbackup.txt | 1 + |
| 13 | + go/flags/endtoend/vtcombo.txt | 1 + |
| 14 | + go/flags/endtoend/vttablet.txt | 1 + |
| 15 | + go/flags/endtoend/vttestserver.txt | 1 + |
| 16 | + .../backup/vtctlbackup/backup_test.go | 1 + |
| 17 | + .../backup/vtctlbackup/backup_utils.go | 4 + |
| 18 | + .../backup/xtrabackup/xtrabackup_test.go | 1 + |
| 19 | + go/vt/mysqlctl/builtinbackupengine.go | 5 +- |
| 20 | + go/vt/mysqlctl/compression.go | 18 +++- |
| 21 | + .../compression_external_decompressor_test.go | 93 +++++++++++++++++++ |
| 22 | + go/vt/mysqlctl/xtrabackupengine.go | 5 +- |
| 23 | + 11 files changed, 122 insertions(+), 9 deletions(-) |
| 24 | + create mode 100644 go/vt/mysqlctl/compression_external_decompressor_test.go |
| 25 | + |
| 26 | +diff --git a/go/flags/endtoend/vtbackup.txt b/go/flags/endtoend/vtbackup.txt |
| 27 | +index 002f4af..a2bc014 100644 |
| 28 | +--- a/go/flags/endtoend/vtbackup.txt |
| 29 | ++++ b/go/flags/endtoend/vtbackup.txt |
| 30 | +@@ -124,6 +124,7 @@ Flags: |
| 31 | + --emit_stats If set, emit stats to push-based monitoring and stats backends |
| 32 | + --external-compressor string command with arguments to use when compressing a backup. |
| 33 | + --external-compressor-extension string extension to use when using an external compressor. |
| 34 | ++ --external-decompressor-use-manifest allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED. |
| 35 | + --external-decompressor string command with arguments to use when decompressing a backup. |
| 36 | + --file_backup_storage_root string Root directory for the file backup storage. |
| 37 | + --gcs_backup_storage_bucket string Google Cloud Storage bucket to use for backups. |
| 38 | +diff --git a/go/flags/endtoend/vtcombo.txt b/go/flags/endtoend/vtcombo.txt |
| 39 | +index 8b62d4e..a8c9ace 100644 |
| 40 | +--- a/go/flags/endtoend/vtcombo.txt |
| 41 | ++++ b/go/flags/endtoend/vtcombo.txt |
| 42 | +@@ -135,6 +135,7 @@ Flags: |
| 43 | + --external-compressor string command with arguments to use when compressing a backup. |
| 44 | + --external-compressor-extension string extension to use when using an external compressor. |
| 45 | + --external-decompressor string command with arguments to use when decompressing a backup. |
| 46 | ++ --external-decompressor-use-manifest allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED. |
| 47 | + --external_topo_server Should vtcombo use an external topology server instead of starting its own in-memory topology server. If true, vtcombo will use the flags defined in topo/server.go to open topo server |
| 48 | + --foreign_key_mode string This is to provide how to handle foreign key constraint in create/alter table. Valid values are: allow, disallow (default "allow") |
| 49 | + --gate_query_cache_memory int gate server query cache size in bytes, maximum amount of memory to be cached. vtgate analyzes every incoming query and generate a query plan, these plans are being cached in a lru cache. This config controls the capacity of the lru cache. (default 33554432) |
| 50 | +diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt |
| 51 | +index 7eae51c..a87c0c0 100644 |
| 52 | +--- a/go/flags/endtoend/vttablet.txt |
| 53 | ++++ b/go/flags/endtoend/vttablet.txt |
| 54 | +@@ -159,6 +159,7 @@ Flags: |
| 55 | + --external-compressor string command with arguments to use when compressing a backup. |
| 56 | + --external-compressor-extension string extension to use when using an external compressor. |
| 57 | + --external-decompressor string command with arguments to use when decompressing a backup. |
| 58 | ++ --external-decompressor-use-manifest allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED. |
| 59 | + --file_backup_storage_root string Root directory for the file backup storage. |
| 60 | + --filecustomrules string file based custom rule path |
| 61 | + --filecustomrules_watch set up a watch on the target file and reload query rules when it changes |
| 62 | +diff --git a/go/flags/endtoend/vttestserver.txt b/go/flags/endtoend/vttestserver.txt |
| 63 | +index 72d9d0b..f0803ba 100644 |
| 64 | +--- a/go/flags/endtoend/vttestserver.txt |
| 65 | ++++ b/go/flags/endtoend/vttestserver.txt |
| 66 | +@@ -39,6 +39,7 @@ Flags: |
| 67 | + --external-compressor string command with arguments to use when compressing a backup. |
| 68 | + --external-compressor-extension string extension to use when using an external compressor. |
| 69 | + --external-decompressor string command with arguments to use when decompressing a backup. |
| 70 | ++ --external-decompressor-use-manifest allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED. |
| 71 | + --external_topo_global_root string the path of the global topology data in the global topology server for vtcombo process |
| 72 | + --external_topo_global_server_address string the address of the global topology server for vtcombo process |
| 73 | + --external_topo_implementation string the topology implementation to use for vtcombo process |
| 74 | +diff --git a/go/test/endtoend/backup/vtctlbackup/backup_test.go b/go/test/endtoend/backup/vtctlbackup/backup_test.go |
| 75 | +index 92c7a2f..154f2d1 100644 |
| 76 | +--- a/go/test/endtoend/backup/vtctlbackup/backup_test.go |
| 77 | ++++ b/go/test/endtoend/backup/vtctlbackup/backup_test.go |
| 78 | +@@ -57,6 +57,7 @@ func TestBuiltinBackupWithExternalZstdCompressionAndManifestedDecompressor(t *te |
| 79 | + CompressorEngineName: "external", |
| 80 | + ExternalCompressorCmd: "zstd", |
| 81 | + ExternalCompressorExt: ".zst", |
| 82 | ++ ExternalDecompressorUseManifest: true, |
| 83 | + ManifestExternalDecompressorCmd: "zstd -d", |
| 84 | + } |
| 85 | + |
| 86 | +diff --git a/go/test/endtoend/backup/vtctlbackup/backup_utils.go b/go/test/endtoend/backup/vtctlbackup/backup_utils.go |
| 87 | +index a70d180..3823eef 100644 |
| 88 | +--- a/go/test/endtoend/backup/vtctlbackup/backup_utils.go |
| 89 | ++++ b/go/test/endtoend/backup/vtctlbackup/backup_utils.go |
| 90 | +@@ -97,6 +97,7 @@ type CompressionDetails struct { |
| 91 | + ExternalCompressorCmd string |
| 92 | + ExternalCompressorExt string |
| 93 | + ExternalDecompressorCmd string |
| 94 | ++ ExternalDecompressorUseManifest bool |
| 95 | + ManifestExternalDecompressorCmd string |
| 96 | + } |
| 97 | + |
| 98 | +@@ -287,6 +288,9 @@ func getCompressorArgs(cDetails *CompressionDetails) []string { |
| 99 | + if cDetails.ExternalDecompressorCmd != "" { |
| 100 | + args = append(args, fmt.Sprintf("--external-decompressor=%s", cDetails.ExternalDecompressorCmd)) |
| 101 | + } |
| 102 | ++ if cDetails.ExternalDecompressorUseManifest { |
| 103 | ++ args = append(args, "--external-decompressor-use-manifest") |
| 104 | ++ } |
| 105 | + if cDetails.ManifestExternalDecompressorCmd != "" { |
| 106 | + args = append(args, fmt.Sprintf("--manifest-external-decompressor=%s", cDetails.ManifestExternalDecompressorCmd)) |
| 107 | + } |
| 108 | +diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go |
| 109 | +index 3402a17..e18f229 100644 |
| 110 | +--- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go |
| 111 | ++++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go |
| 112 | +@@ -59,6 +59,7 @@ func TestXtrabackupWithExternalZstdCompressionAndManifestedDecompressor(t *testi |
| 113 | + CompressorEngineName: "external", |
| 114 | + ExternalCompressorCmd: "zstd", |
| 115 | + ExternalCompressorExt: ".zst", |
| 116 | ++ ExternalDecompressorUseManifest: true, |
| 117 | + ManifestExternalDecompressorCmd: "zstd -d", |
| 118 | + } |
| 119 | + |
| 120 | +diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go |
| 121 | +index 94ed7bd..fcf7521 100644 |
| 122 | +--- a/go/vt/mysqlctl/builtinbackupengine.go |
| 123 | ++++ b/go/vt/mysqlctl/builtinbackupengine.go |
| 124 | +@@ -1120,10 +1120,7 @@ func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestorePa |
| 125 | + // for backward compatibility |
| 126 | + deCompressionEngine = PgzipCompressor |
| 127 | + } |
| 128 | +- externalDecompressorCmd := ExternalDecompressorCmd |
| 129 | +- if externalDecompressorCmd == "" && bm.ExternalDecompressor != "" { |
| 130 | +- externalDecompressorCmd = bm.ExternalDecompressor |
| 131 | +- } |
| 132 | ++ externalDecompressorCmd := resolveExternalDecompressor(bm.ExternalDecompressor) |
| 133 | + if externalDecompressorCmd != "" { |
| 134 | + if deCompressionEngine == ExternalCompressor { |
| 135 | + deCompressionEngine = externalDecompressorCmd |
| 136 | +diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go |
| 137 | +index c2d3cbb..5737b70 100644 |
| 138 | +--- a/go/vt/mysqlctl/compression.go |
| 139 | ++++ b/go/vt/mysqlctl/compression.go |
| 140 | +@@ -52,9 +52,10 @@ var ( |
| 141 | + ExternalCompressorCmd string |
| 142 | + ExternalCompressorExt string |
| 143 | + ExternalDecompressorCmd string |
| 144 | ++ ExternalDecompressorUseManifest bool |
| 145 | + ManifestExternalDecompressorCmd string |
| 146 | + |
| 147 | +- errUnsupportedDeCompressionEngine = errors.New("unsupported engine in MANIFEST. You need to provide --external-decompressor if using 'external' compression engine") |
| 148 | ++ errUnsupportedDeCompressionEngine = errors.New("unsupported engine in MANIFEST. You need to provide --external-decompressor if using 'external' compression engine. Alternatively, set --external-decompressor-use-manifest to use the decompressor command from the backup manifest, but this is NOT RECOMMENDED as it is a security risk") |
| 149 | + errUnsupportedCompressionEngine = errors.New("unsupported engine value for --compression-engine-name. supported values are 'external', 'pgzip', 'pargzip', 'zstd', 'lz4'") |
| 150 | + |
| 151 | + // this is used by getEngineFromExtension() to figure out which engine to use in case the user didn't specify |
| 152 | +@@ -77,6 +78,7 @@ func registerBackupCompressionFlags(fs *pflag.FlagSet) { |
| 153 | + fs.StringVar(&ExternalCompressorCmd, "external-compressor", ExternalCompressorCmd, "command with arguments to use when compressing a backup.") |
| 154 | + fs.StringVar(&ExternalCompressorExt, "external-compressor-extension", ExternalCompressorExt, "extension to use when using an external compressor.") |
| 155 | + fs.StringVar(&ExternalDecompressorCmd, "external-decompressor", ExternalDecompressorCmd, "command with arguments to use when decompressing a backup.") |
| 156 | ++ fs.BoolVar(&ExternalDecompressorUseManifest, "external-decompressor-use-manifest", ExternalDecompressorUseManifest, "allows the decompressor command stored in the backup manifest to be used at restore time. Enabling this is a security risk: an attacker with write access to the backup storage could modify the manifest to execute arbitrary commands on the tablet as the Vitess user. NOT RECOMMENDED.") |
| 157 | + fs.StringVar(&ManifestExternalDecompressorCmd, "manifest-external-decompressor", ManifestExternalDecompressorCmd, "command with arguments to store in the backup manifest when compressing a backup with an external compression engine.") |
| 158 | + } |
| 159 | + |
| 160 | +@@ -91,6 +93,20 @@ func getExtensionFromEngine(engine string) (string, error) { |
| 161 | + return "", fmt.Errorf("%w %q", errUnsupportedCompressionEngine, engine) |
| 162 | + } |
| 163 | + |
| 164 | ++// resolveExternalDecompressor returns the external decompressor command to use |
| 165 | ++// at restore time. The CLI flag (--external-decompressor) takes precedence. The |
| 166 | ++// backup manifest value is only used when --external-decompressor-use-manifest |
| 167 | ++// is explicitly set to true. |
| 168 | ++func resolveExternalDecompressor(manifestDecompressor string) string { |
| 169 | ++ if ExternalDecompressorCmd != "" { |
| 170 | ++ return ExternalDecompressorCmd |
| 171 | ++ } |
| 172 | ++ if ExternalDecompressorUseManifest && manifestDecompressor != "" { |
| 173 | ++ return manifestDecompressor |
| 174 | ++ } |
| 175 | ++ return "" |
| 176 | ++} |
| 177 | ++ |
| 178 | + // Validates if the external decompressor exists and return its path. |
| 179 | + func validateExternalCmd(cmd string) (string, error) { |
| 180 | + if cmd == "" { |
| 181 | +diff --git a/go/vt/mysqlctl/compression_external_decompressor_test.go b/go/vt/mysqlctl/compression_external_decompressor_test.go |
| 182 | +new file mode 100644 |
| 183 | +index 0000000..fd4732a |
| 184 | +--- /dev/null |
| 185 | ++++ b/go/vt/mysqlctl/compression_external_decompressor_test.go |
| 186 | +@@ -0,0 +1,93 @@ |
| 187 | ++/* |
| 188 | ++Copyright 2026 The Vitess Authors. |
| 189 | ++ |
| 190 | ++Licensed under the Apache License, Version 2.0 (the "License"); |
| 191 | ++you may not use this file except in compliance with the License. |
| 192 | ++You may obtain a copy of the License at |
| 193 | ++ |
| 194 | ++ http://www.apache.org/licenses/LICENSE-2.0 |
| 195 | ++ |
| 196 | ++Unless required by applicable law or agreed to in writing, software |
| 197 | ++distributed under the License is distributed on an "AS IS" BASIS, |
| 198 | ++WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| 199 | ++See the License for the specific language governing permissions and |
| 200 | ++limitations under the License. |
| 201 | ++*/ |
| 202 | ++ |
| 203 | ++package mysqlctl |
| 204 | ++ |
| 205 | ++import ( |
| 206 | ++ "testing" |
| 207 | ++ |
| 208 | ++ "github.com/stretchr/testify/assert" |
| 209 | ++) |
| 210 | ++ |
| 211 | ++func TestResolveExternalDecompressor(t *testing.T) { |
| 212 | ++ tests := []struct { |
| 213 | ++ name string |
| 214 | ++ cliDecompressorCmd string |
| 215 | ++ useManifest bool |
| 216 | ++ manifestDecompressor string |
| 217 | ++ expected string |
| 218 | ++ }{ |
| 219 | ++ { |
| 220 | ++ name: "CLI flag takes precedence over manifest", |
| 221 | ++ cliDecompressorCmd: "zstd -d", |
| 222 | ++ useManifest: true, |
| 223 | ++ manifestDecompressor: "gzip -d", |
| 224 | ++ expected: "zstd -d", |
| 225 | ++ }, |
| 226 | ++ { |
| 227 | ++ name: "CLI flag takes precedence even when use-manifest is false", |
| 228 | ++ cliDecompressorCmd: "zstd -d", |
| 229 | ++ useManifest: false, |
| 230 | ++ manifestDecompressor: "gzip -d", |
| 231 | ++ expected: "zstd -d", |
| 232 | ++ }, |
| 233 | ++ { |
| 234 | ++ name: "manifest used when use-manifest is true and no CLI flag", |
| 235 | ++ cliDecompressorCmd: "", |
| 236 | ++ useManifest: true, |
| 237 | ++ manifestDecompressor: "gzip -d", |
| 238 | ++ expected: "gzip -d", |
| 239 | ++ }, |
| 240 | ++ { |
| 241 | ++ name: "manifest ignored when use-manifest is false", |
| 242 | ++ cliDecompressorCmd: "", |
| 243 | ++ useManifest: false, |
| 244 | ++ manifestDecompressor: "gzip -d", |
| 245 | ++ expected: "", |
| 246 | ++ }, |
| 247 | ++ { |
| 248 | ++ name: "empty when nothing is set", |
| 249 | ++ cliDecompressorCmd: "", |
| 250 | ++ useManifest: false, |
| 251 | ++ manifestDecompressor: "", |
| 252 | ++ expected: "", |
| 253 | ++ }, |
| 254 | ++ { |
| 255 | ++ name: "empty when use-manifest is true but manifest is empty", |
| 256 | ++ cliDecompressorCmd: "", |
| 257 | ++ useManifest: true, |
| 258 | ++ manifestDecompressor: "", |
| 259 | ++ expected: "", |
| 260 | ++ }, |
| 261 | ++ } |
| 262 | ++ |
| 263 | ++ for _, tt := range tests { |
| 264 | ++ t.Run(tt.name, func(t *testing.T) { |
| 265 | ++ origCmd := ExternalDecompressorCmd |
| 266 | ++ origAllow := ExternalDecompressorUseManifest |
| 267 | ++ t.Cleanup(func() { |
| 268 | ++ ExternalDecompressorCmd = origCmd |
| 269 | ++ ExternalDecompressorUseManifest = origAllow |
| 270 | ++ }) |
| 271 | ++ |
| 272 | ++ ExternalDecompressorCmd = tt.cliDecompressorCmd |
| 273 | ++ ExternalDecompressorUseManifest = tt.useManifest |
| 274 | ++ |
| 275 | ++ result := resolveExternalDecompressor(tt.manifestDecompressor) |
| 276 | ++ assert.Equal(t, tt.expected, result) |
| 277 | ++ }) |
| 278 | ++ } |
| 279 | ++} |
| 280 | +diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go |
| 281 | +index 3f8491f..7552a90 100644 |
| 282 | +--- a/go/vt/mysqlctl/xtrabackupengine.go |
| 283 | ++++ b/go/vt/mysqlctl/xtrabackupengine.go |
| 284 | +@@ -644,10 +644,7 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log |
| 285 | + // then we assign the default value of compressionEngine. |
| 286 | + deCompressionEngine = PgzipCompressor |
| 287 | + } |
| 288 | +- externalDecompressorCmd := ExternalDecompressorCmd |
| 289 | +- if externalDecompressorCmd == "" && bm.ExternalDecompressor != "" { |
| 290 | +- externalDecompressorCmd = bm.ExternalDecompressor |
| 291 | +- } |
| 292 | ++ externalDecompressorCmd := resolveExternalDecompressor(bm.ExternalDecompressor) |
| 293 | + if externalDecompressorCmd != "" { |
| 294 | + if deCompressionEngine == ExternalCompressor { |
| 295 | + deCompressionEngine = externalDecompressorCmd |
| 296 | +-- |
| 297 | +2.43.0 |
| 298 | + |
0 commit comments