|
| 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/vttablet.txt | 1 + |
| 14 | + go/flags/endtoend/vttestserver.txt | 1 + |
| 15 | + .../backup/vtctlbackup/backup_test.go | 1 + |
| 16 | + .../backup/vtctlbackup/backup_utils.go | 4 + |
| 17 | + .../backup/xtrabackup/xtrabackup_test.go | 1 + |
| 18 | + go/vt/mysqlctl/builtinbackupengine.go | 5 +- |
| 19 | + go/vt/mysqlctl/compression.go | 18 +++- |
| 20 | + .../compression_external_decompressor_test.go | 93 +++++++++++++++++++ |
| 21 | + go/vt/mysqlctl/xtrabackupengine.go | 5 +- |
| 22 | + 10 files changed, 121 insertions(+), 9 deletions(-) |
| 23 | + create mode 100644 go/vt/mysqlctl/compression_external_decompressor_test.go |
| 24 | + |
| 25 | +diff --git a/go/flags/endtoend/vtbackup.txt b/go/flags/endtoend/vtbackup.txt |
| 26 | +index e979381..2895154 100644 |
| 27 | +--- a/go/flags/endtoend/vtbackup.txt |
| 28 | ++++ b/go/flags/endtoend/vtbackup.txt |
| 29 | +@@ -79,6 +79,7 @@ Usage of vtbackup: |
| 30 | + --emit_stats If set, emit stats to push-based monitoring and stats backends |
| 31 | + --external-compressor string command with arguments to use when compressing a backup. |
| 32 | + --external-compressor-extension string extension to use when using an external compressor. |
| 33 | ++ --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. |
| 34 | + --external-decompressor string command with arguments to use when decompressing a backup. |
| 35 | + --file_backup_storage_root string Root directory for the file backup storage. |
| 36 | + --gcs_backup_storage_bucket string Google Cloud Storage bucket to use for backups. |
| 37 | +diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt |
| 38 | +index f431979..0d4c39f 100644 |
| 39 | +--- a/go/flags/endtoend/vttablet.txt |
| 40 | ++++ b/go/flags/endtoend/vttablet.txt |
| 41 | +@@ -117,6 +117,7 @@ Usage of vttablet: |
| 42 | + --external-compressor string command with arguments to use when compressing a backup. |
| 43 | + --external-compressor-extension string extension to use when using an external compressor. |
| 44 | + --external-decompressor string command with arguments to use when decompressing a backup. |
| 45 | ++ --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. |
| 46 | + --file_backup_storage_root string Root directory for the file backup storage. |
| 47 | + --filecustomrules string file based custom rule path |
| 48 | + --filecustomrules_watch set up a watch on the target file and reload query rules when it changes |
| 49 | +diff --git a/go/flags/endtoend/vttestserver.txt b/go/flags/endtoend/vttestserver.txt |
| 50 | +index 668e31f..cdcba7a 100644 |
| 51 | +--- a/go/flags/endtoend/vttestserver.txt |
| 52 | ++++ b/go/flags/endtoend/vttestserver.txt |
| 53 | +@@ -33,6 +33,7 @@ Usage of vttestserver: |
| 54 | + --external-compressor string command with arguments to use when compressing a backup. |
| 55 | + --external-compressor-extension string extension to use when using an external compressor. |
| 56 | + --external-decompressor string command with arguments to use when decompressing a backup. |
| 57 | ++ --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. |
| 58 | + --external_topo_global_root string the path of the global topology data in the global topology server for vtcombo process |
| 59 | + --external_topo_global_server_address string the address of the global topology server for vtcombo process |
| 60 | + --external_topo_implementation string the topology implementation to use for vtcombo process |
| 61 | +diff --git a/go/test/endtoend/backup/vtctlbackup/backup_test.go b/go/test/endtoend/backup/vtctlbackup/backup_test.go |
| 62 | +index 92c7a2f..154f2d1 100644 |
| 63 | +--- a/go/test/endtoend/backup/vtctlbackup/backup_test.go |
| 64 | ++++ b/go/test/endtoend/backup/vtctlbackup/backup_test.go |
| 65 | +@@ -57,6 +57,7 @@ func TestBuiltinBackupWithExternalZstdCompressionAndManifestedDecompressor(t *te |
| 66 | + CompressorEngineName: "external", |
| 67 | + ExternalCompressorCmd: "zstd", |
| 68 | + ExternalCompressorExt: ".zst", |
| 69 | ++ ExternalDecompressorUseManifest: true, |
| 70 | + ManifestExternalDecompressorCmd: "zstd -d", |
| 71 | + } |
| 72 | + |
| 73 | +diff --git a/go/test/endtoend/backup/vtctlbackup/backup_utils.go b/go/test/endtoend/backup/vtctlbackup/backup_utils.go |
| 74 | +index d93b932..78a0e57 100644 |
| 75 | +--- a/go/test/endtoend/backup/vtctlbackup/backup_utils.go |
| 76 | ++++ b/go/test/endtoend/backup/vtctlbackup/backup_utils.go |
| 77 | +@@ -95,6 +95,7 @@ type CompressionDetails struct { |
| 78 | + ExternalCompressorCmd string |
| 79 | + ExternalCompressorExt string |
| 80 | + ExternalDecompressorCmd string |
| 81 | ++ ExternalDecompressorUseManifest bool |
| 82 | + ManifestExternalDecompressorCmd string |
| 83 | + } |
| 84 | + |
| 85 | +@@ -263,6 +264,9 @@ func getCompressorArgs(cDetails *CompressionDetails) []string { |
| 86 | + if cDetails.ExternalDecompressorCmd != "" { |
| 87 | + args = append(args, fmt.Sprintf("--external-decompressor=%s", cDetails.ExternalDecompressorCmd)) |
| 88 | + } |
| 89 | ++ if cDetails.ExternalDecompressorUseManifest { |
| 90 | ++ args = append(args, "--external-decompressor-use-manifest") |
| 91 | ++ } |
| 92 | + if cDetails.ManifestExternalDecompressorCmd != "" { |
| 93 | + args = append(args, fmt.Sprintf("--manifest-external-decompressor=%s", cDetails.ManifestExternalDecompressorCmd)) |
| 94 | + } |
| 95 | +diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go |
| 96 | +index 3402a17..e18f229 100644 |
| 97 | +--- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go |
| 98 | ++++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go |
| 99 | +@@ -59,6 +59,7 @@ func TestXtrabackupWithExternalZstdCompressionAndManifestedDecompressor(t *testi |
| 100 | + CompressorEngineName: "external", |
| 101 | + ExternalCompressorCmd: "zstd", |
| 102 | + ExternalCompressorExt: ".zst", |
| 103 | ++ ExternalDecompressorUseManifest: true, |
| 104 | + ManifestExternalDecompressorCmd: "zstd -d", |
| 105 | + } |
| 106 | + |
| 107 | +diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go |
| 108 | +index 8fac602..1106a65 100644 |
| 109 | +--- a/go/vt/mysqlctl/builtinbackupengine.go |
| 110 | ++++ b/go/vt/mysqlctl/builtinbackupengine.go |
| 111 | +@@ -1020,10 +1020,7 @@ func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestorePa |
| 112 | + // for backward compatibility |
| 113 | + deCompressionEngine = PgzipCompressor |
| 114 | + } |
| 115 | +- externalDecompressorCmd := ExternalDecompressorCmd |
| 116 | +- if externalDecompressorCmd == "" && bm.ExternalDecompressor != "" { |
| 117 | +- externalDecompressorCmd = bm.ExternalDecompressor |
| 118 | +- } |
| 119 | ++ externalDecompressorCmd := resolveExternalDecompressor(bm.ExternalDecompressor) |
| 120 | + if externalDecompressorCmd != "" { |
| 121 | + if deCompressionEngine == ExternalCompressor { |
| 122 | + deCompressionEngine = externalDecompressorCmd |
| 123 | +diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go |
| 124 | +index c2d3cbb..5737b70 100644 |
| 125 | +--- a/go/vt/mysqlctl/compression.go |
| 126 | ++++ b/go/vt/mysqlctl/compression.go |
| 127 | +@@ -52,9 +52,10 @@ var ( |
| 128 | + ExternalCompressorCmd string |
| 129 | + ExternalCompressorExt string |
| 130 | + ExternalDecompressorCmd string |
| 131 | ++ ExternalDecompressorUseManifest bool |
| 132 | + ManifestExternalDecompressorCmd string |
| 133 | + |
| 134 | +- errUnsupportedDeCompressionEngine = errors.New("unsupported engine in MANIFEST. You need to provide --external-decompressor if using 'external' compression engine") |
| 135 | ++ 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") |
| 136 | + errUnsupportedCompressionEngine = errors.New("unsupported engine value for --compression-engine-name. supported values are 'external', 'pgzip', 'pargzip', 'zstd', 'lz4'") |
| 137 | + |
| 138 | + // this is used by getEngineFromExtension() to figure out which engine to use in case the user didn't specify |
| 139 | +@@ -77,6 +78,7 @@ func registerBackupCompressionFlags(fs *pflag.FlagSet) { |
| 140 | + fs.StringVar(&ExternalCompressorCmd, "external-compressor", ExternalCompressorCmd, "command with arguments to use when compressing a backup.") |
| 141 | + fs.StringVar(&ExternalCompressorExt, "external-compressor-extension", ExternalCompressorExt, "extension to use when using an external compressor.") |
| 142 | + fs.StringVar(&ExternalDecompressorCmd, "external-decompressor", ExternalDecompressorCmd, "command with arguments to use when decompressing a backup.") |
| 143 | ++ 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.") |
| 144 | + 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.") |
| 145 | + } |
| 146 | + |
| 147 | +@@ -91,6 +93,20 @@ func getExtensionFromEngine(engine string) (string, error) { |
| 148 | + return "", fmt.Errorf("%w %q", errUnsupportedCompressionEngine, engine) |
| 149 | + } |
| 150 | + |
| 151 | ++// resolveExternalDecompressor returns the external decompressor command to use |
| 152 | ++// at restore time. The CLI flag (--external-decompressor) takes precedence. The |
| 153 | ++// backup manifest value is only used when --external-decompressor-use-manifest |
| 154 | ++// is explicitly set to true. |
| 155 | ++func resolveExternalDecompressor(manifestDecompressor string) string { |
| 156 | ++ if ExternalDecompressorCmd != "" { |
| 157 | ++ return ExternalDecompressorCmd |
| 158 | ++ } |
| 159 | ++ if ExternalDecompressorUseManifest && manifestDecompressor != "" { |
| 160 | ++ return manifestDecompressor |
| 161 | ++ } |
| 162 | ++ return "" |
| 163 | ++} |
| 164 | ++ |
| 165 | + // Validates if the external decompressor exists and return its path. |
| 166 | + func validateExternalCmd(cmd string) (string, error) { |
| 167 | + if cmd == "" { |
| 168 | +diff --git a/go/vt/mysqlctl/compression_external_decompressor_test.go b/go/vt/mysqlctl/compression_external_decompressor_test.go |
| 169 | +new file mode 100644 |
| 170 | +index 0000000..fd4732a |
| 171 | +--- /dev/null |
| 172 | ++++ b/go/vt/mysqlctl/compression_external_decompressor_test.go |
| 173 | +@@ -0,0 +1,93 @@ |
| 174 | ++/* |
| 175 | ++Copyright 2026 The Vitess Authors. |
| 176 | ++ |
| 177 | ++Licensed under the Apache License, Version 2.0 (the "License"); |
| 178 | ++you may not use this file except in compliance with the License. |
| 179 | ++You may obtain a copy of the License at |
| 180 | ++ |
| 181 | ++ http://www.apache.org/licenses/LICENSE-2.0 |
| 182 | ++ |
| 183 | ++Unless required by applicable law or agreed to in writing, software |
| 184 | ++distributed under the License is distributed on an "AS IS" BASIS, |
| 185 | ++WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| 186 | ++See the License for the specific language governing permissions and |
| 187 | ++limitations under the License. |
| 188 | ++*/ |
| 189 | ++ |
| 190 | ++package mysqlctl |
| 191 | ++ |
| 192 | ++import ( |
| 193 | ++ "testing" |
| 194 | ++ |
| 195 | ++ "github.com/stretchr/testify/assert" |
| 196 | ++) |
| 197 | ++ |
| 198 | ++func TestResolveExternalDecompressor(t *testing.T) { |
| 199 | ++ tests := []struct { |
| 200 | ++ name string |
| 201 | ++ cliDecompressorCmd string |
| 202 | ++ useManifest bool |
| 203 | ++ manifestDecompressor string |
| 204 | ++ expected string |
| 205 | ++ }{ |
| 206 | ++ { |
| 207 | ++ name: "CLI flag takes precedence over manifest", |
| 208 | ++ cliDecompressorCmd: "zstd -d", |
| 209 | ++ useManifest: true, |
| 210 | ++ manifestDecompressor: "gzip -d", |
| 211 | ++ expected: "zstd -d", |
| 212 | ++ }, |
| 213 | ++ { |
| 214 | ++ name: "CLI flag takes precedence even when use-manifest is false", |
| 215 | ++ cliDecompressorCmd: "zstd -d", |
| 216 | ++ useManifest: false, |
| 217 | ++ manifestDecompressor: "gzip -d", |
| 218 | ++ expected: "zstd -d", |
| 219 | ++ }, |
| 220 | ++ { |
| 221 | ++ name: "manifest used when use-manifest is true and no CLI flag", |
| 222 | ++ cliDecompressorCmd: "", |
| 223 | ++ useManifest: true, |
| 224 | ++ manifestDecompressor: "gzip -d", |
| 225 | ++ expected: "gzip -d", |
| 226 | ++ }, |
| 227 | ++ { |
| 228 | ++ name: "manifest ignored when use-manifest is false", |
| 229 | ++ cliDecompressorCmd: "", |
| 230 | ++ useManifest: false, |
| 231 | ++ manifestDecompressor: "gzip -d", |
| 232 | ++ expected: "", |
| 233 | ++ }, |
| 234 | ++ { |
| 235 | ++ name: "empty when nothing is set", |
| 236 | ++ cliDecompressorCmd: "", |
| 237 | ++ useManifest: false, |
| 238 | ++ manifestDecompressor: "", |
| 239 | ++ expected: "", |
| 240 | ++ }, |
| 241 | ++ { |
| 242 | ++ name: "empty when use-manifest is true but manifest is empty", |
| 243 | ++ cliDecompressorCmd: "", |
| 244 | ++ useManifest: true, |
| 245 | ++ manifestDecompressor: "", |
| 246 | ++ expected: "", |
| 247 | ++ }, |
| 248 | ++ } |
| 249 | ++ |
| 250 | ++ for _, tt := range tests { |
| 251 | ++ t.Run(tt.name, func(t *testing.T) { |
| 252 | ++ origCmd := ExternalDecompressorCmd |
| 253 | ++ origAllow := ExternalDecompressorUseManifest |
| 254 | ++ t.Cleanup(func() { |
| 255 | ++ ExternalDecompressorCmd = origCmd |
| 256 | ++ ExternalDecompressorUseManifest = origAllow |
| 257 | ++ }) |
| 258 | ++ |
| 259 | ++ ExternalDecompressorCmd = tt.cliDecompressorCmd |
| 260 | ++ ExternalDecompressorUseManifest = tt.useManifest |
| 261 | ++ |
| 262 | ++ result := resolveExternalDecompressor(tt.manifestDecompressor) |
| 263 | ++ assert.Equal(t, tt.expected, result) |
| 264 | ++ }) |
| 265 | ++ } |
| 266 | ++} |
| 267 | +diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go |
| 268 | +index 6b3d04c..5976a93 100644 |
| 269 | +--- a/go/vt/mysqlctl/xtrabackupengine.go |
| 270 | ++++ b/go/vt/mysqlctl/xtrabackupengine.go |
| 271 | +@@ -637,10 +637,7 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log |
| 272 | + // then we assign the default value of compressionEngine. |
| 273 | + deCompressionEngine = PgzipCompressor |
| 274 | + } |
| 275 | +- externalDecompressorCmd := ExternalDecompressorCmd |
| 276 | +- if externalDecompressorCmd == "" && bm.ExternalDecompressor != "" { |
| 277 | +- externalDecompressorCmd = bm.ExternalDecompressor |
| 278 | +- } |
| 279 | ++ externalDecompressorCmd := resolveExternalDecompressor(bm.ExternalDecompressor) |
| 280 | + if externalDecompressorCmd != "" { |
| 281 | + if deCompressionEngine == ExternalCompressor { |
| 282 | + deCompressionEngine = externalDecompressorCmd |
| 283 | +-- |
| 284 | +2.43.0 |
| 285 | + |
0 commit comments