You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "DaanHoogland (via GitHub)" <gi...@apache.org> on 2023/01/24 08:49:26 UTC

[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7003: utils,framework/db: Introduce new database encryption cipher based on AesGcmJce

DaanHoogland commented on code in PR #7003:
URL: https://github.com/apache/cloudstack/pull/7003#discussion_r1084960875


##########
utils/src/test/java/com/cloud/utils/crypt/EncryptionSecretKeyCheckerTest.java:
##########
@@ -0,0 +1,58 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.utils.crypt;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Properties;
+
+public class EncryptionSecretKeyCheckerTest {
+    @Before
+    public void setup() {
+        EncryptionSecretKeyChecker.initEncryptor("managementkey");
+    }
+
+    @After
+    public void tearDown() {
+        EncryptionSecretKeyChecker.resetEncryptor();
+    }
+
+    @Test
+    public void decryptPropertyIfNeededTest() {
+        String rawValue = "ENC(iYVsCZXiGiC6SzZLMNBvBL93hoUpntxkuRjyaqC8L+JYKXw=)";
+        String result = EncryptionSecretKeyChecker.decryptPropertyIfNeeded(rawValue);
+        Assert.assertEquals("encthis", result);
+    }
+
+    @Test
+    public void decryptAnyPropertiesTest() {
+        Properties props = new Properties();
+        props.setProperty("db.cloud.encrypt.secret", "ENC(iYVsCZXiGiC6SzZLMNBvBL93hoUpntxkuRjyaqC8L+JYKXw=)");
+        props.setProperty("other.unencrypted", "somevalue");
+
+        EncryptionSecretKeyChecker.decryptAnyProperties(props);
+
+        Assert.assertEquals("encthis", props.getProperty("db.cloud.encrypt.secret"));
+        Assert.assertEquals("somevalue", props.getProperty("other.unencrypted"));
+    }
+}

Review Comment:
   new line at end of file



##########
framework/db/src/main/java/com/cloud/utils/crypt/EncryptionSecretKeyChanger.java:
##########
@@ -22,149 +22,332 @@
 import java.io.FileNotFoundException;
 import java.io.FileWriter;
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.nio.charset.StandardCharsets;
 import java.sql.Connection;
+import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.Arrays;
-import java.util.Iterator;
-import java.util.List;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
 import java.util.Properties;
-
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.CommandLineParser;
+import org.apache.commons.cli.DefaultParser;
+import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.ParseException;
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.PropertiesConfiguration;
-import org.jasypt.encryption.pbe.StandardPBEStringEncryptor;
-import org.jasypt.encryption.pbe.config.SimpleStringPBEConfig;
-import org.jasypt.exceptions.EncryptionOperationNotPossibleException;
-import org.jasypt.properties.EncryptableProperties;
+import org.apache.commons.lang3.StringUtils;
 
 import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.ReflectUtil;
+import com.cloud.utils.db.Encrypt;
 import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.utils.exception.CloudRuntimeException;
 
+import com.google.gson.Gson;
+import com.google.gson.JsonSyntaxException;
+
+import javax.persistence.Column;
+import javax.persistence.Table;
+
 /*
  * EncryptionSecretKeyChanger updates Management Secret Key / DB Secret Key or both.
  * DB secret key is validated against the key in db.properties
  * db.properties is updated with values encrypted using new MS secret key
+ * server.properties is updated with values encrypted using new MS secret key
  * DB data migrated using new DB secret key
  */
 public class EncryptionSecretKeyChanger {
 
-    private StandardPBEStringEncryptor oldEncryptor = new StandardPBEStringEncryptor();
-    private StandardPBEStringEncryptor newEncryptor = new StandardPBEStringEncryptor();
-    private static final String keyFile = "/etc/cloudstack/management/key";
+    private CloudStackEncryptor oldEncryptor;
+    private CloudStackEncryptor newEncryptor;
+    private static final String KEY_FILE = "/etc/cloudstack/management/key";
+    private static final String ENV_NEW_MANAGEMENT_KEY = "CLOUD_SECRET_KEY_NEW";
+    private final Gson gson = new Gson();
+    private static final String PASSWORD = "password";
+
+    private static final Options options = initializeOptions();
+    private static final HelpFormatter helper = initializeHelper();
+    private static final String CMD_LINE_SYNTAX = "cloudstack-migrate-databases";
+    private static final int WIDTH = 100;
+    private static final String HEADER = "Options:";
+    private static final String FOOTER = " \nExamples: \n" +
+            "  " + CMD_LINE_SYNTAX + " -m password -d password -n newmgmtkey -v V2 \n" +
+            "       Migrate cloudstack properties (db.properties and server.properties) \n" +
+            "       with new management key and encryptor V2. \n" +
+            "  " + CMD_LINE_SYNTAX + " -m password -d password -n newmgmtkey -e newdbkey \n" +
+            "       Migrate cloudstack properties and databases with new management key and database secret key. \n" +
+            "  " + CMD_LINE_SYNTAX + " -m password -d password -n newmgmtkey -e newdbkey -s -v V2 \n" +
+            "       Migrate cloudstack properties with new keys and encryptor V2, but skip database migration. \n" +
+            "  " + CMD_LINE_SYNTAX + " -m password -d password -l -f \n" +
+            "       Migrate cloudstack properties with new management key (load from $CLOUD_SECRET_KEY_NEW), \n" +
+            "       and migrate database with old db key. \n" +
+            " \nReturn codes: \n" +
+            "  0 - Succeed to change keys and/or migrate databases \n" +
+            "  1 - Fail to parse the command line arguments \n" +
+            "  2 - Fail to validate parameters \n" +
+            "  3 - Fail to migrate database";
+    private static final String OLD_MS_KEY_OPTION = "oldMSKey";
+    private static final String OLD_DB_KEY_OPTION = "oldDBKey";
+    private static final String NEW_MS_KEY_OPTION = "newMSKey";
+    private static final String NEW_DB_KEY_OPTION = "newDBKey";
+    private static final String ENCRYPTOR_VERSION_OPTION = "version";
+    private static final String LOAD_NEW_MS_KEY_FROM_ENV_FLAG = "load-new-management-key-from-env";
+    private static final String FORCE_DATABASE_MIGRATION_FLAG = "force-database-migration";
+    private static final String SKIP_DATABASE_MIGRATION_FLAG = "skip-database-migration";
+    private static final String HELP_FLAG = "help";
 
     public static void main(String[] args) {
-        List<String> argsList = Arrays.asList(args);
-        Iterator<String> iter = argsList.iterator();
-        String oldMSKey = null;
-        String oldDBKey = null;
-        String newMSKey = null;
-        String newDBKey = null;
+        if (args.length == 0 || StringUtils.equalsAny(args[0], "-h", "--help")) {
+            helper.printHelp(WIDTH, CMD_LINE_SYNTAX, HEADER, options, FOOTER, true);
+            System.exit(0);
+        }
+
+        CommandLine cmdLine = null;
+        CommandLineParser parser = new DefaultParser();
+        try {
+            cmdLine = parser.parse(options, args);
+        } catch (ParseException e) {
+            System.out.println(e.getMessage());
+            helper.printHelp(WIDTH, CMD_LINE_SYNTAX, HEADER, options, FOOTER, true);
+            System.exit(1);
+        }
+
+        String oldMSKey = cmdLine.getOptionValue(OLD_MS_KEY_OPTION);
+        String oldDBKey = cmdLine.getOptionValue(OLD_DB_KEY_OPTION);
+        String newMSKey = cmdLine.getOptionValue(NEW_MS_KEY_OPTION);
+        String newDBKey = cmdLine.getOptionValue(NEW_DB_KEY_OPTION);
+        String newEncryptorVersion = cmdLine.getOptionValue(ENCRYPTOR_VERSION_OPTION);
+        boolean loadNewMsKeyFromEnv = cmdLine.hasOption(LOAD_NEW_MS_KEY_FROM_ENV_FLAG);
+        boolean forced = cmdLine.hasOption(FORCE_DATABASE_MIGRATION_FLAG);
+        boolean skipped = cmdLine.hasOption(SKIP_DATABASE_MIGRATION_FLAG);
+
+        if (!validateParameters(oldMSKey, oldDBKey, newMSKey, newDBKey, newEncryptorVersion, loadNewMsKeyFromEnv)) {
+            helper.printHelp(WIDTH, CMD_LINE_SYNTAX, HEADER, options, FOOTER, true);
+            System.exit(2);
+        }
+
+        System.out.println("Started database migration at " + new Date());
+        if (!migrateEverything(oldMSKey, oldDBKey, newMSKey, newDBKey, newEncryptorVersion, loadNewMsKeyFromEnv, forced, skipped)) {
+            System.out.println("Got error during database migration at " + new Date());
+            System.exit(3);
+        }
+        System.out.println("Finished database migration at " + new Date());
+    }
 
-        //Parse command-line args
-        while (iter.hasNext()) {
-            String arg = iter.next();
-            // Old MS Key
-            if (arg.equals("-m")) {
-                oldMSKey = iter.next();
+    private static Options initializeOptions() {
+        Options options = new Options();
+
+        Option oldMSKey = Option.builder("m").longOpt(OLD_MS_KEY_OPTION).argName(OLD_MS_KEY_OPTION).required(true).hasArg().desc("(required) Current Mgmt Secret Key").build();
+        Option oldDBKey = Option.builder("d").longOpt(OLD_DB_KEY_OPTION).argName(OLD_DB_KEY_OPTION).required(true).hasArg().desc("(required) Current DB Secret Key").build();
+        Option newMSKey = Option.builder("n").longOpt(NEW_MS_KEY_OPTION).argName(NEW_MS_KEY_OPTION).required(false).hasArg().desc("New Mgmt Secret Key").build();
+        Option newDBKey = Option.builder("e").longOpt(NEW_DB_KEY_OPTION).argName(NEW_DB_KEY_OPTION).required(false).hasArg().desc("New DB Secret Key").build();
+        Option encryptorVersion = Option.builder("v").longOpt(ENCRYPTOR_VERSION_OPTION).argName(ENCRYPTOR_VERSION_OPTION).required(false).hasArg().desc("New DB Encryptor Version. Options are V1, V2.").build();
+
+        Option loadNewMsKeyFromEnv = Option.builder("l").longOpt(LOAD_NEW_MS_KEY_FROM_ENV_FLAG).desc("Load new management key from environment variable " + ENV_NEW_MANAGEMENT_KEY).build();
+        Option forceDatabaseMigration = Option.builder("f").longOpt(FORCE_DATABASE_MIGRATION_FLAG).desc("Force database migration even if DB Secret key is not changed").build();
+        Option skipDatabaseMigration = Option.builder("s").longOpt(SKIP_DATABASE_MIGRATION_FLAG).desc("Skip database migration even if DB Secret key is changed").build();
+        Option help = Option.builder("h").longOpt(HELP_FLAG).desc("Show help message").build();
+
+        options.addOption(oldMSKey);
+        options.addOption(oldDBKey);
+        options.addOption(newMSKey);
+        options.addOption(newDBKey);
+        options.addOption(encryptorVersion);
+        options.addOption(loadNewMsKeyFromEnv);
+        options.addOption(forceDatabaseMigration);
+        options.addOption(skipDatabaseMigration);
+        options.addOption(help);
+
+        return options;
+    }
+
+    private static HelpFormatter initializeHelper() {
+        HelpFormatter helper = new HelpFormatter();
+
+        helper.setOptionComparator((o1, o2) -> {
+            if (o1.isRequired() && !o2.isRequired()) {
+                return -1;
             }
-            // Old DB Key
-            if (arg.equals("-d")) {
-                oldDBKey = iter.next();
+            if (!o1.isRequired() && o2.isRequired()) {
+                return 1;
             }
-            // New MS Key
-            if (arg.equals("-n")) {
-                newMSKey = iter.next();
+            if (o1.hasArg() && !o2.hasArg()) {
+                return -1;
             }
-            // New DB Key
-            if (arg.equals("-e")) {
-                newDBKey = iter.next();
+            if (!o1.hasArg() && o2.hasArg()) {
+                return 1;
             }
-        }
+            return o1.getOpt().compareTo(o2.getOpt());
+        });
+
+        return helper;
+    }
+
+    private static boolean validateParameters(String oldMSKey, String oldDBKey, String newMSKey, String newDBKey,

Review Comment:
   this method is doing a lot, it could be dissected into smaller methods.



##########
framework/db/src/main/java/com/cloud/utils/crypt/EncryptionSecretKeyChanger.java:
##########
@@ -22,149 +22,332 @@
 import java.io.FileNotFoundException;
 import java.io.FileWriter;
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
+import java.nio.charset.StandardCharsets;
 import java.sql.Connection;
+import java.sql.DatabaseMetaData;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.Arrays;
-import java.util.Iterator;
-import java.util.List;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
 import java.util.Properties;
-
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.commons.cli.CommandLine;
+import org.apache.commons.cli.CommandLineParser;
+import org.apache.commons.cli.DefaultParser;
+import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.Option;
+import org.apache.commons.cli.Options;
+import org.apache.commons.cli.ParseException;
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.PropertiesConfiguration;
-import org.jasypt.encryption.pbe.StandardPBEStringEncryptor;
-import org.jasypt.encryption.pbe.config.SimpleStringPBEConfig;
-import org.jasypt.exceptions.EncryptionOperationNotPossibleException;
-import org.jasypt.properties.EncryptableProperties;
+import org.apache.commons.lang3.StringUtils;
 
 import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.ReflectUtil;
+import com.cloud.utils.db.Encrypt;
 import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.utils.exception.CloudRuntimeException;
 
+import com.google.gson.Gson;
+import com.google.gson.JsonSyntaxException;
+
+import javax.persistence.Column;
+import javax.persistence.Table;
+
 /*
  * EncryptionSecretKeyChanger updates Management Secret Key / DB Secret Key or both.
  * DB secret key is validated against the key in db.properties
  * db.properties is updated with values encrypted using new MS secret key
+ * server.properties is updated with values encrypted using new MS secret key
  * DB data migrated using new DB secret key
  */
 public class EncryptionSecretKeyChanger {
 
-    private StandardPBEStringEncryptor oldEncryptor = new StandardPBEStringEncryptor();
-    private StandardPBEStringEncryptor newEncryptor = new StandardPBEStringEncryptor();
-    private static final String keyFile = "/etc/cloudstack/management/key";
+    private CloudStackEncryptor oldEncryptor;
+    private CloudStackEncryptor newEncryptor;
+    private static final String KEY_FILE = "/etc/cloudstack/management/key";
+    private static final String ENV_NEW_MANAGEMENT_KEY = "CLOUD_SECRET_KEY_NEW";
+    private final Gson gson = new Gson();
+    private static final String PASSWORD = "password";
+
+    private static final Options options = initializeOptions();
+    private static final HelpFormatter helper = initializeHelper();
+    private static final String CMD_LINE_SYNTAX = "cloudstack-migrate-databases";
+    private static final int WIDTH = 100;
+    private static final String HEADER = "Options:";
+    private static final String FOOTER = " \nExamples: \n" +
+            "  " + CMD_LINE_SYNTAX + " -m password -d password -n newmgmtkey -v V2 \n" +
+            "       Migrate cloudstack properties (db.properties and server.properties) \n" +
+            "       with new management key and encryptor V2. \n" +
+            "  " + CMD_LINE_SYNTAX + " -m password -d password -n newmgmtkey -e newdbkey \n" +
+            "       Migrate cloudstack properties and databases with new management key and database secret key. \n" +
+            "  " + CMD_LINE_SYNTAX + " -m password -d password -n newmgmtkey -e newdbkey -s -v V2 \n" +
+            "       Migrate cloudstack properties with new keys and encryptor V2, but skip database migration. \n" +
+            "  " + CMD_LINE_SYNTAX + " -m password -d password -l -f \n" +
+            "       Migrate cloudstack properties with new management key (load from $CLOUD_SECRET_KEY_NEW), \n" +
+            "       and migrate database with old db key. \n" +
+            " \nReturn codes: \n" +
+            "  0 - Succeed to change keys and/or migrate databases \n" +
+            "  1 - Fail to parse the command line arguments \n" +
+            "  2 - Fail to validate parameters \n" +
+            "  3 - Fail to migrate database";
+    private static final String OLD_MS_KEY_OPTION = "oldMSKey";
+    private static final String OLD_DB_KEY_OPTION = "oldDBKey";
+    private static final String NEW_MS_KEY_OPTION = "newMSKey";
+    private static final String NEW_DB_KEY_OPTION = "newDBKey";
+    private static final String ENCRYPTOR_VERSION_OPTION = "version";
+    private static final String LOAD_NEW_MS_KEY_FROM_ENV_FLAG = "load-new-management-key-from-env";
+    private static final String FORCE_DATABASE_MIGRATION_FLAG = "force-database-migration";
+    private static final String SKIP_DATABASE_MIGRATION_FLAG = "skip-database-migration";
+    private static final String HELP_FLAG = "help";
 
     public static void main(String[] args) {
-        List<String> argsList = Arrays.asList(args);
-        Iterator<String> iter = argsList.iterator();
-        String oldMSKey = null;
-        String oldDBKey = null;
-        String newMSKey = null;
-        String newDBKey = null;
+        if (args.length == 0 || StringUtils.equalsAny(args[0], "-h", "--help")) {
+            helper.printHelp(WIDTH, CMD_LINE_SYNTAX, HEADER, options, FOOTER, true);
+            System.exit(0);
+        }
+
+        CommandLine cmdLine = null;
+        CommandLineParser parser = new DefaultParser();
+        try {
+            cmdLine = parser.parse(options, args);
+        } catch (ParseException e) {
+            System.out.println(e.getMessage());
+            helper.printHelp(WIDTH, CMD_LINE_SYNTAX, HEADER, options, FOOTER, true);
+            System.exit(1);
+        }
+
+        String oldMSKey = cmdLine.getOptionValue(OLD_MS_KEY_OPTION);
+        String oldDBKey = cmdLine.getOptionValue(OLD_DB_KEY_OPTION);
+        String newMSKey = cmdLine.getOptionValue(NEW_MS_KEY_OPTION);
+        String newDBKey = cmdLine.getOptionValue(NEW_DB_KEY_OPTION);
+        String newEncryptorVersion = cmdLine.getOptionValue(ENCRYPTOR_VERSION_OPTION);
+        boolean loadNewMsKeyFromEnv = cmdLine.hasOption(LOAD_NEW_MS_KEY_FROM_ENV_FLAG);
+        boolean forced = cmdLine.hasOption(FORCE_DATABASE_MIGRATION_FLAG);
+        boolean skipped = cmdLine.hasOption(SKIP_DATABASE_MIGRATION_FLAG);
+
+        if (!validateParameters(oldMSKey, oldDBKey, newMSKey, newDBKey, newEncryptorVersion, loadNewMsKeyFromEnv)) {
+            helper.printHelp(WIDTH, CMD_LINE_SYNTAX, HEADER, options, FOOTER, true);
+            System.exit(2);
+        }
+
+        System.out.println("Started database migration at " + new Date());
+        if (!migrateEverything(oldMSKey, oldDBKey, newMSKey, newDBKey, newEncryptorVersion, loadNewMsKeyFromEnv, forced, skipped)) {
+            System.out.println("Got error during database migration at " + new Date());
+            System.exit(3);
+        }
+        System.out.println("Finished database migration at " + new Date());
+    }
 
-        //Parse command-line args
-        while (iter.hasNext()) {
-            String arg = iter.next();
-            // Old MS Key
-            if (arg.equals("-m")) {
-                oldMSKey = iter.next();
+    private static Options initializeOptions() {
+        Options options = new Options();
+
+        Option oldMSKey = Option.builder("m").longOpt(OLD_MS_KEY_OPTION).argName(OLD_MS_KEY_OPTION).required(true).hasArg().desc("(required) Current Mgmt Secret Key").build();
+        Option oldDBKey = Option.builder("d").longOpt(OLD_DB_KEY_OPTION).argName(OLD_DB_KEY_OPTION).required(true).hasArg().desc("(required) Current DB Secret Key").build();
+        Option newMSKey = Option.builder("n").longOpt(NEW_MS_KEY_OPTION).argName(NEW_MS_KEY_OPTION).required(false).hasArg().desc("New Mgmt Secret Key").build();
+        Option newDBKey = Option.builder("e").longOpt(NEW_DB_KEY_OPTION).argName(NEW_DB_KEY_OPTION).required(false).hasArg().desc("New DB Secret Key").build();
+        Option encryptorVersion = Option.builder("v").longOpt(ENCRYPTOR_VERSION_OPTION).argName(ENCRYPTOR_VERSION_OPTION).required(false).hasArg().desc("New DB Encryptor Version. Options are V1, V2.").build();
+
+        Option loadNewMsKeyFromEnv = Option.builder("l").longOpt(LOAD_NEW_MS_KEY_FROM_ENV_FLAG).desc("Load new management key from environment variable " + ENV_NEW_MANAGEMENT_KEY).build();
+        Option forceDatabaseMigration = Option.builder("f").longOpt(FORCE_DATABASE_MIGRATION_FLAG).desc("Force database migration even if DB Secret key is not changed").build();
+        Option skipDatabaseMigration = Option.builder("s").longOpt(SKIP_DATABASE_MIGRATION_FLAG).desc("Skip database migration even if DB Secret key is changed").build();
+        Option help = Option.builder("h").longOpt(HELP_FLAG).desc("Show help message").build();
+
+        options.addOption(oldMSKey);
+        options.addOption(oldDBKey);
+        options.addOption(newMSKey);
+        options.addOption(newDBKey);
+        options.addOption(encryptorVersion);
+        options.addOption(loadNewMsKeyFromEnv);
+        options.addOption(forceDatabaseMigration);
+        options.addOption(skipDatabaseMigration);
+        options.addOption(help);
+
+        return options;
+    }
+
+    private static HelpFormatter initializeHelper() {
+        HelpFormatter helper = new HelpFormatter();
+
+        helper.setOptionComparator((o1, o2) -> {
+            if (o1.isRequired() && !o2.isRequired()) {
+                return -1;
             }
-            // Old DB Key
-            if (arg.equals("-d")) {
-                oldDBKey = iter.next();
+            if (!o1.isRequired() && o2.isRequired()) {
+                return 1;
             }
-            // New MS Key
-            if (arg.equals("-n")) {
-                newMSKey = iter.next();
+            if (o1.hasArg() && !o2.hasArg()) {
+                return -1;
             }
-            // New DB Key
-            if (arg.equals("-e")) {
-                newDBKey = iter.next();
+            if (!o1.hasArg() && o2.hasArg()) {
+                return 1;
             }
-        }
+            return o1.getOpt().compareTo(o2.getOpt());
+        });
+
+        return helper;
+    }
+
+    private static boolean validateParameters(String oldMSKey, String oldDBKey, String newMSKey, String newDBKey,
+                                              String newEncryptorVersion, boolean loadNewMsKeyFromEnv) {
 
         if (oldMSKey == null || oldDBKey == null) {
-            System.out.println("Existing MS secret key or DB secret key is not provided");
-            usage();
-            return;
+            System.out.println("Existing Management secret key or DB secret key is not provided");
+            return false;
+        }
+
+        if (loadNewMsKeyFromEnv) {
+            if (StringUtils.isNotEmpty(newMSKey)) {
+                System.out.println("The new management key has already been set. Please check if it is set twice.");
+                return false;
+            }
+            newMSKey = System.getenv(ENV_NEW_MANAGEMENT_KEY);
+            if (StringUtils.isEmpty(newMSKey)) {
+                System.out.println("Environment variable " + ENV_NEW_MANAGEMENT_KEY + " is not set or empty");
+                return false;
+            }
         }
 
         if (newMSKey == null && newDBKey == null) {
-            System.out.println("New MS secret key and DB secret are both not provided");
-            usage();
-            return;
+            System.out.println("New Management secret key and DB secret are both not provided");
+            return false;
+        }
+
+        if (newEncryptorVersion != null) {
+            try {
+                CloudStackEncryptor.EncryptorVersion.fromString(newEncryptorVersion);
+            } catch (CloudRuntimeException ex) {
+                System.out.println(ex.getMessage());
+                return false;
+            }
         }
 
+        return true;
+    }
+
+    private static boolean migrateEverything(String oldMSKey, String oldDBKey, String newMSKey, String newDBKey,

Review Comment:
   this is a god-method it could be renamed to `migrateAllEncryptedFields` and should be dissected to smaller methods.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org