coderabbit feedback

src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobContainerService.java

@@ -33,7 +33,6 @@
 
 import com.github.dockerjava.api.DockerClient;
 import com.github.dockerjava.api.async.ResultCallback;
-import com.github.dockerjava.api.command.CreateContainerCmd;
 import com.github.dockerjava.api.command.CreateContainerResponse;
 import com.github.dockerjava.api.command.ExecCreateCmd;
 import com.github.dockerjava.api.command.ExecCreateCmdResponse;
@@ -102,21 +101,18 @@ public CreateContainerResponse configureContainer(String containerName, String i
             envVars.add("NO_PROXY=" + noProxy);
         }
         envVars.add("SCRIPT=" + buildScript);
-        CreateContainerCmd createContainerCmd = dockerClient.createContainerCmd(image).withName(containerName).withHostConfig(hostConfig).withEnv(envVars)
+        if (exerciseEnvVars != null && !exerciseEnvVars.isEmpty()) {
+            envVars.addAll(exerciseEnvVars);
+        }
+        return dockerClient.createContainerCmd(image).withName(containerName).withHostConfig(hostConfig).withEnv(envVars)
                 // Command to run when the container starts. This is the command that will be executed in the container's main process, which runs in the foreground and blocks the
                 // container from exiting until it finishes.
                 // It waits until the script that is running the tests (see below execCreateCmdResponse) is completed, and until the result files are extracted which is indicated
                 // by the creation of a file "stop_container.txt" in the container's root directory.
-                .withCmd("sh", "-c", "while [ ! -f " + LOCALCI_WORKING_DIRECTORY + "/stop_container.txt ]; do sleep 0.5; done");
-        // .withCmd("tail", "-f", "/dev/null"); // Activate for debugging purposes instead of the above command to get a running container that you can peek into using
-        // "docker exec -it <container-id> /bin/bash".
-
-        if (exerciseEnvVars != null && !exerciseEnvVars.isEmpty()) {
-            envVars.addAll(exerciseEnvVars);
-            createContainerCmd.withEnv(envVars);
-        }
-
-        return createContainerCmd.exec();
+                .withCmd("sh", "-c", "while [ ! -f " + LOCALCI_WORKING_DIRECTORY + "/stop_container.txt ]; do sleep 0.5; done")
+                // .withCmd("tail", "-f", "/dev/null") // Activate for debugging purposes instead of the above command to get a running container that you can peek into using
+                // "docker exec -it <container-id> /bin/bash".
+                .exec();
     }
 
     /**
@@ -140,7 +136,12 @@ public void runScriptInContainer(String containerId, String buildJobId, boolean
             // The "sh preScript.sh" execution command specified here is need to install dependencies and prepare the environment for the build script.
             log.info("Started running the pre-script for build job in container with id {}", containerId);
             executeDockerCommand(containerId, buildJobId, true, true, false, "bash", LOCALCI_WORKING_DIRECTORY + "/preScript.sh");
-            dockerClient.disconnectFromNetworkCmd().withContainerId(containerId).withNetworkId("bridge").exec();
+            try {
+                dockerClient.disconnectFromNetworkCmd().withContainerId(containerId).withNetworkId("bridge").exec();
+            }
+            catch (Exception e) {
+                log.error("Failed to disconnect container with id {} from network: {}", containerId, e.getMessage());
+            }
         }
 
         log.info("Started running the build script for build job in container with id {}", containerId);

src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobExecutionService.java

@@ -192,7 +192,7 @@ public BuildResult runBuildJob(BuildJobQueueItem buildJob, String containerName)
             index++;
         }
 
-        List<String> envVars = new ArrayList<>();
+        List<String> envVars = null;
         boolean isNetworkDisabled = false;
         if (buildJob.buildConfig().dockerRunConfig() != null) {
             envVars = buildJob.buildConfig().dockerRunConfig().getEnv();

src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingExerciseBuildConfig.java

@@ -330,12 +330,12 @@ public DockerRunConfig getDockerRunConfig() {
             for (List<String> entry : list) {
                 if (entry.size() != 2 || entry.get(0) == null || entry.get(1) == null || entry.get(0).isBlank() || entry.get(1).isBlank()
                         || !DockerRunConfig.AllowedDockerFlags.isAllowed(entry.get(0))) {
-                    log.error("Invalid Docker flag entry: {}. Skipping.", entry);
+                    log.warn("Invalid Docker flag entry: {}. Skipping.", entry);
                     continue;
                 }
                 switch (entry.get(0)) {
                     case "network":
-                        dockerRunConfig.setNetworkDisabled(entry.get(1).equals("none"));
+                        dockerRunConfig.setNetworkDisabled(entry.get(1).equalsIgnoreCase("none"));
                         break;
                     case "env":
                         dockerRunConfig.setEnv(parseEnvVariableString(entry.get(1)));

src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java

@@ -327,6 +327,7 @@ private static String getBaseScript() {
                     cd /var/tmp/testing-dir
                     #!/usr/bin/env bash
                     set -e
+                    echo "Starting dependency download script"
                     """;
         }
 

src/main/webapp/app/exercises/programming/manage/update/update-components/custom-build-plans/programming-exercise-build-configuration/programming-exercise-build-configuration.component.ts

@@ -41,7 +41,7 @@ export class ProgrammingExerciseBuildConfigurationComponent implements OnInit {
 
     ngOnInit() {
         if (this.programmingExercise()?.buildConfig?.dockerFlags) {
-            const dockerFlags = JSON.parse(this.programmingExercise()?.buildConfig?.dockerFlags || '{}') as [string, string][];
+            const dockerFlags = JSON.parse(this.programmingExercise()?.buildConfig?.dockerFlags || '[]') as [string, string][];
             dockerFlags.forEach(([key, value]) => {
                 if (key === this.NETWORK_KEY) {
                     this.isNetworkDisabled = value === 'none';
@@ -64,9 +64,6 @@ export class ProgrammingExerciseBuildConfigurationComponent implements OnInit {
 
     updateDockerFlags(key: string, value: string) {
         let existingFlags = JSON.parse(this.programmingExercise()?.buildConfig?.dockerFlags || '[]') as [string, string][];
-        if (!existingFlags || existingFlags.length === 0) {
-            existingFlags = [] as [string, string][];
-        }
         existingFlags = existingFlags.filter(([flag]) => ALLOWED_DOCKER_FLAG_OPTIONS.includes(flag) && flag !== key) || [];
         if (ALLOWED_DOCKER_FLAG_OPTIONS.includes(key) && value.trim() !== '') {
             existingFlags.push([key, value]);

src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalVCLocalCIIntegrationTest.java

@@ -972,7 +972,7 @@ void testFetchPush_instructorPracticeRepository() throws Exception {
     }
 
     @Nested
-    class DisabledBuildAgentTest {
+    class BuildJobConfigurationTest {
 
         @BeforeEach
         void setUp() {