Skip to content

Commit

Permalink
Skip embedded Node.js runtime deployment if sonar.nodejs.executable…
Browse files Browse the repository at this point in the history 
… is set (#4616)
  • Loading branch information
@ilia-kebets-sonarsource
ilia-kebets-sonarsource authored Mar 26, 2024
1 parent d0fefb2 commit f71d3e0
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 30 deletions.
13 changes: 11 additions & 2 deletions ...javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import static java.util.Collections.emptyList;
import static org.sonar.plugins.javascript.bridge.NetUtils.findOpenPort;
import static org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl.NODE_EXECUTABLE_PROPERTY;

import com.google.gson.Gson;
import com.google.gson.JsonSyntaxException;
Expand All @@ -47,6 +48,7 @@
import javax.annotation.Nullable;
import org.sonar.api.SonarProduct;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.config.Configuration;
import org.sonar.api.utils.TempFolder;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
Expand Down Expand Up @@ -162,8 +164,15 @@ int getTimeoutSeconds() {
*
* @throws IOException
*/
void deploy() throws IOException {
void deploy(Configuration configuration) throws IOException {
bundle.deploy(temporaryDeployLocation);
if (configuration.get(NODE_EXECUTABLE_PROPERTY).isPresent()) {
LOG.info(
"'{}' is set. Skipping embedded Node.js runtime deployment.",
NODE_EXECUTABLE_PROPERTY
);
return;
}
embeddedNode.deploy();
}

Expand Down Expand Up @@ -285,7 +294,7 @@ public void startServerLazily(SensorContext context) throws IOException {
status = Status.FAILED;
throw new ServerAlreadyFailedException();
}
deploy();
deploy(context.config());
List<Path> deployedBundles = rulesBundles.deploy(temporaryDeployLocation.resolve("package"));
rulesBundles
.getUcfgRulesBundle()
Expand Down
43 changes: 15 additions & 28 deletions ...script-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import static org.sonar.api.utils.log.LoggerLevel.INFO;
import static org.sonar.api.utils.log.LoggerLevel.WARN;
import static org.sonar.plugins.javascript.bridge.AnalysisMode.DEFAULT_LINTER_ID;
import static org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl.NODE_EXECUTABLE_PROPERTY;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -126,7 +127,6 @@ public void tearDown() {
@Test
void should_throw_when_not_existing_script() throws Exception {
bridgeServer = createBridgeServer("NOT_EXISTING.js");
bridgeServer.deploy();
List<Path> deployedBundles = emptyList();

assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles))
Expand Down Expand Up @@ -157,7 +157,6 @@ void should_throw_if_failed_to_build_node_command() throws Exception {
tempFolder,
unsupportedEmbeddedRuntime
);
bridgeServer.deploy();
List<Path> deployedBundles = emptyList();

assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles))
Expand All @@ -168,7 +167,6 @@ void should_throw_if_failed_to_build_node_command() throws Exception {
@Test
void should_forward_process_streams() throws Exception {
bridgeServer = createBridgeServer("logging.js");
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

assertThat(logTester.logs(DEBUG)).contains("testing debug log");
Expand All @@ -180,7 +178,6 @@ void should_forward_process_streams() throws Exception {
@Test
void should_get_answer_from_server() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

DefaultInputFile inputFile = TestInputFileBuilder
Expand All @@ -194,7 +191,6 @@ void should_get_answer_from_server() throws Exception {
@Test
void test_init() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

List<EslintRule> rules = Collections.singletonList(
Expand Down Expand Up @@ -223,7 +219,6 @@ void test_init() throws Exception {
@Test
void should_get_answer_from_server_for_ts_request() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

DefaultInputFile inputFile = TestInputFileBuilder
Expand All @@ -250,7 +245,6 @@ void should_get_answer_from_server_for_ts_request() throws Exception {
@Test
void should_get_answer_from_server_for_yaml_request() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

DefaultInputFile inputFile = TestInputFileBuilder
Expand Down Expand Up @@ -278,7 +272,6 @@ private static JsAnalysisRequest createRequest(DefaultInputFile inputFile) {
@Test
void should_get_answer_from_server_for_program_based_requests() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

TsProgram programCreated = bridgeServer.createProgram(
Expand Down Expand Up @@ -308,7 +301,6 @@ void should_get_answer_from_server_for_program_based_requests() throws Exception
@Test
void should_create_tsconfig_files() throws IOException {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

var tsConfig = bridgeServer.createTsConfigFile("{\"include\":[\"/path/to/project/**/*\"]}");
Expand All @@ -318,7 +310,6 @@ void should_create_tsconfig_files() throws IOException {
@Test
void should_not_fail_when_error_during_create_program() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

TsProgram programCreated = bridgeServer.createProgram(
Expand All @@ -332,7 +323,6 @@ void should_not_fail_when_error_during_create_program() throws Exception {
@Test
void should_get_answer_from_server_for_css_request() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

DefaultInputFile inputFile = TestInputFileBuilder
Expand All @@ -350,7 +340,6 @@ void should_get_answer_from_server_for_css_request() throws Exception {
@Test
void should_throw_if_failed_to_start() throws Exception {
bridgeServer = createBridgeServer("throw.js");
bridgeServer.deploy();
List<Path> deployedBundles = emptyList();

assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles))
Expand All @@ -364,7 +353,6 @@ void should_return_command_info() throws Exception {
assertThat(bridgeServer.getCommandInfo())
.isEqualTo("Node.js command to start the bridge server was not built yet.");

bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

assertThat(bridgeServer.getCommandInfo())
Expand All @@ -378,7 +366,6 @@ void should_set_max_old_space_size() throws Exception {
assertThat(bridgeServer.getCommandInfo())
.isEqualTo("Node.js command to start the bridge server was not built yet.");

bridgeServer.deploy();
context.setSettings(new MapSettings().setProperty("sonar.javascript.node.maxspace", 2048));
bridgeServer.startServer(context, emptyList());

Expand All @@ -388,7 +375,6 @@ void should_set_max_old_space_size() throws Exception {
@Test
void should_set_allowTsParserJsFiles_to_false() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
context.setSettings(
new MapSettings().setProperty("sonar.javascript.allowTsParserJsFiles", "false")
);
Expand All @@ -401,7 +387,6 @@ void should_set_allowTsParserJsFiles_to_false() throws Exception {
@Test
void allowTsParserJsFiles_default_value_is_true() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());
bridgeServer.stop();

Expand Down Expand Up @@ -510,7 +495,6 @@ void should_throw_if_server_not_alive() throws Exception {
@Test
void should_fail_if_bad_json_response() throws Exception {
bridgeServer = createBridgeServer("badResponse.js");
bridgeServer.deploy();
bridgeServer.startServerLazily(context);

DefaultInputFile inputFile = TestInputFileBuilder
Expand All @@ -535,7 +519,6 @@ void should_fail_if_bad_json_response() throws Exception {
@Test
void should_not_search_typescript_when_no_ts_file() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
SensorContextTester ctx = SensorContextTester.create(moduleBase);
ctx.fileSystem().setWorkDir(workDir);
Path tsDir = moduleBase.resolve("dir/node_modules/typescript");
Expand All @@ -547,15 +530,13 @@ void should_not_search_typescript_when_no_ts_file() throws Exception {
@Test
void should_reload_tsconfig() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());
assertThat(bridgeServer.newTsConfig()).isTrue();
}

@Test
void should_return_files_for_tsconfig() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());
String tsconfig = "path/to/tsconfig.json";
BridgeServerImpl.TsConfigResponse tsConfigResponse = bridgeServer.tsConfigFiles(tsconfig);
Expand All @@ -571,7 +552,6 @@ void should_return_files_for_tsconfig() throws Exception {
@Test
void should_return_no_files_for_tsconfig_bad_response() throws Exception {
bridgeServer = createBridgeServer("badResponse.js");
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());
BridgeServerImpl.TsConfigResponse response = bridgeServer.tsConfigFiles(
"path/to/tsconfig.json"
Expand All @@ -583,7 +563,6 @@ void should_return_no_files_for_tsconfig_bad_response() throws Exception {
@Test
void should_return_no_files_for_tsconfig_no_response() throws Exception {
bridgeServer = createBridgeServer("badResponse.js");
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());
assertThat(bridgeServer.tsConfigFiles("path/to/tsconfig.json").files).isEmpty();
TsConfigFile tsConfigFile = bridgeServer.loadTsConfig("path/to/tsconfig.json");
Expand All @@ -593,7 +572,6 @@ void should_return_no_files_for_tsconfig_no_response() throws Exception {
@Test
void should_return_no_files_for_tsconfig_on_error() throws Exception {
bridgeServer = createBridgeServer("tsConfigError.js");
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

TsConfigFile tsConfigFile = bridgeServer.loadTsConfig("path/to/tsconfig.json");
Expand All @@ -604,7 +582,6 @@ void should_return_no_files_for_tsconfig_on_error() throws Exception {
@Test
void log_error_when_timeout() throws Exception {
bridgeServer = createBridgeServer("timeout.js");
bridgeServer.deploy();
bridgeServer.startServer(context, emptyList());

assertThatThrownBy(() -> bridgeServer.loadTsConfig("any.ts"))
Expand All @@ -626,7 +603,6 @@ void test_rule_tostring() {
@Test
void should_load_custom_rules() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2")));
bridgeServer.stop();

Expand All @@ -637,7 +613,6 @@ void should_load_custom_rules() throws Exception {
@Test
void should_skip_metrics_on_sonarlint() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
context.setRuntime(SonarRuntimeImpl.forSonarLint(Version.create(7, 9)));
bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2")));
bridgeServer.stop();
Expand All @@ -648,7 +623,6 @@ void should_skip_metrics_on_sonarlint() throws Exception {
@Test
void should_pass_debug_memory_option() throws Exception {
bridgeServer = createBridgeServer(START_SERVER_SCRIPT);
bridgeServer.deploy();
context.setSettings(new MapSettings().setProperty("sonar.javascript.node.debugMemory", "true"));
bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2")));
bridgeServer.stop();
Expand Down Expand Up @@ -715,7 +689,6 @@ void enabled_monitoring() throws Exception {
tempFolder,
unsupportedEmbeddedRuntime
);
bridgeServer.deploy();
bridgeServer.startServerLazily(context);
bridgeServer.stop();
assertThat(logTester.logs(INFO).stream().anyMatch(s -> s.startsWith("no-commented-code")))
Expand Down Expand Up @@ -750,6 +723,20 @@ void test_ucfg_bundle_version() throws Exception {
.contains("Security Frontend version is available: [some_bundle_version]");
}

@Test
void should_not_deploy_runtime_if_sonar_nodejs_executable_is_set() throws Exception {
var existingDoesntMatterScript = "logging.js";
bridgeServer = createBridgeServer(existingDoesntMatterScript);
context.setSettings(new MapSettings().setProperty(NODE_EXECUTABLE_PROPERTY, "whatever"));
assertThatThrownBy(() -> bridgeServer.startServerLazily(context))
.isInstanceOf(NodeCommandException.class);

assertThat(logTester.logs(INFO))
.contains(
"'" + NODE_EXECUTABLE_PROPERTY + "' is set. Skipping embedded Node.js runtime deployment."
);
}

private BridgeServerImpl createBridgeServer(String startServerScript) {
return new BridgeServerImpl(
builder(),
Expand Down

0 comments on commit f71d3e0

Please sign in to comment.