Copilot commented on code in PR #12605:
URL: https://github.com/apache/cloudstack/pull/12605#discussion_r2812100897
##########
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42020to42030.java:
##########
@@ -51,6 +55,44 @@ public InputStream[] getPrepareScripts() {
@Override
public void performDataMigration(Connection conn) {
+ unhideJsInterpretationEnabled(conn);
+ }
+
+ protected void unhideJsInterpretationEnabled(Connection conn) {
+ String value = getJsInterpretationEnabled(conn);
+ if (value != null) {
+ updateJsInterpretationEnabledFields(conn, value);
+ }
+ }
+
+ protected String getJsInterpretationEnabled(Connection conn) {
+ String query = "SELECT value FROM cloud.configuration WHERE name =
'js.interpretation.enabled' AND category = 'Hidden';";
+
+ try (PreparedStatement pstmt = conn.prepareStatement(query)) {
+ ResultSet rs = pstmt.executeQuery();
+ if (rs.next()) {
+ return rs.getString("value");
+ }
+ logger.debug("Unable to retrieve value of hidden configuration
'js.interpretation.enabled'. The configuration may already be unhidden.");
+ return null;
+ } catch (SQLException e) {
+ throw new CloudRuntimeException("Error while retrieving value of
hidden configuration 'js.interpretation.enabled'.", e);
+ }
+ }
+
+ protected void updateJsInterpretationEnabledFields(Connection conn, String
encryptedValue) {
+ String query = "UPDATE cloud.configuration SET value = ?, category =
'System' WHERE name = 'js.interpretation.enabled' AND category = 'Hidden';";
+
+ try (PreparedStatement pstmt = conn.prepareStatement(query)) {
+ String decryptedValue = DBEncryptionUtil.decrypt(encryptedValue);
+ logger.info("Updating setting 'js.interpretation.enabled' to
decrypted value [{}], and category 'System'.", decryptedValue);
+ pstmt.setString(1, decryptedValue);
+ pstmt.executeUpdate();
+ } catch (SQLException e) {
+ throw new CloudRuntimeException("Error while unhiding
configuration 'js.interpretation.enabled'.", e);
+ } catch (CloudRuntimeException e) {
+ logger.warn("Error while decrypting configuration
'js.interpretation.enabled'. The configuration may already be decrypted.");
+ }
Review Comment:
The error handling here catches a generic CloudRuntimeException, but
DBEncryptionUtil.decrypt() throws EncryptionOperationNotPossibleException when
the value is already decrypted. The catch block should specifically catch
EncryptionOperationNotPossibleException to distinguish between an
already-decrypted value and an actual error. Additionally, when catching this
exception, the code should proceed with using the encrypted value (since it's
already decrypted) instead of just logging a warning and not updating the
database.
##########
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42020to42030.java:
##########
@@ -51,6 +55,44 @@ public InputStream[] getPrepareScripts() {
@Override
public void performDataMigration(Connection conn) {
+ unhideJsInterpretationEnabled(conn);
+ }
+
+ protected void unhideJsInterpretationEnabled(Connection conn) {
+ String value = getJsInterpretationEnabled(conn);
+ if (value != null) {
+ updateJsInterpretationEnabledFields(conn, value);
+ }
+ }
+
+ protected String getJsInterpretationEnabled(Connection conn) {
+ String query = "SELECT value FROM cloud.configuration WHERE name =
'js.interpretation.enabled' AND category = 'Hidden';";
+
+ try (PreparedStatement pstmt = conn.prepareStatement(query)) {
+ ResultSet rs = pstmt.executeQuery();
+ if (rs.next()) {
+ return rs.getString("value");
+ }
+ logger.debug("Unable to retrieve value of hidden configuration
'js.interpretation.enabled'. The configuration may already be unhidden.");
+ return null;
+ } catch (SQLException e) {
+ throw new CloudRuntimeException("Error while retrieving value of
hidden configuration 'js.interpretation.enabled'.", e);
+ }
+ }
+
+ protected void updateJsInterpretationEnabledFields(Connection conn, String
encryptedValue) {
+ String query = "UPDATE cloud.configuration SET value = ?, category =
'System' WHERE name = 'js.interpretation.enabled' AND category = 'Hidden';";
+
+ try (PreparedStatement pstmt = conn.prepareStatement(query)) {
+ String decryptedValue = DBEncryptionUtil.decrypt(encryptedValue);
+ logger.info("Updating setting 'js.interpretation.enabled' to
decrypted value [{}], and category 'System'.", decryptedValue);
Review Comment:
The SQL UPDATE statement only updates the 'value' and 'category' fields, but
according to the PR description and the new ConfigKey definition in
JsInterpreterHelper, the 'component' field should also be updated from
'ManagementServer' to 'JsInterpreter', and the 'is_dynamic' field should be
updated to 1 (true). The SQL query should be: "UPDATE cloud.configuration SET
value = ?, category = 'System', component = 'JsInterpreter', is_dynamic = 1
WHERE name = 'js.interpretation.enabled' AND category = 'Hidden';"
```suggestion
String query = "UPDATE cloud.configuration SET value = ?, category =
'System', component = 'JsInterpreter', is_dynamic = 1 WHERE name =
'js.interpretation.enabled' AND category = 'Hidden';";
try (PreparedStatement pstmt = conn.prepareStatement(query)) {
String decryptedValue = DBEncryptionUtil.decrypt(encryptedValue);
logger.info("Updating setting 'js.interpretation.enabled' to
decrypted value [{}], category 'System', component 'JsInterpreter', and
is_dynamic = 1.", decryptedValue);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
