Merge pull request #946 from amvanbaren/bugfix/issue-945

Fix Internal Server Error on publish
eclipse · Jun 23, 2024 · a53f1c6 · a53f1c6
2 parents 91ba91d + 579da8f
commit a53f1c6
Show file tree

Hide file tree

Showing 9 changed files with 47 additions and 36 deletions.
diff --git a/server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobRequestHandler.java b/server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobRequestHandler.java
@@ -63,19 +63,16 @@ public void run(HandlerJobRequest<?> jobRequest) throws Exception {
 
     private void createKeyPair() {
         var activeKeyPair = repositories.findActiveKeyPair();
-        Streamable<ExtensionVersion> extVersions;
         if(activeKeyPair == null) {
-            service.generateKeyPair();
-            extVersions = repositories.findVersions();
+            renewKeyPair();
         } else {
-            extVersions = repositories.findVersionsWithout(activeKeyPair);
+            repositories.findVersionsWithout(activeKeyPair).forEach(this::enqueueCreateSignatureJob);
         }
-
-        extVersions.forEach(this::enqueueCreateSignatureJob);
     }
 
     private void renewKeyPair() {
-        service.renewKeyPair();
+        var keyPair = service.generateKeyPair();
+        service.updateKeyPair(keyPair);
         repositories.findVersions().forEach(this::enqueueCreateSignatureJob);
     }
 

diff --git a/server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobService.java b/server/src/main/java/org/eclipse/openvsx/migration/GenerateKeyPairJobService.java
@@ -20,14 +20,14 @@
 import org.bouncycastle.util.io.pem.PemWriter;
 import org.eclipse.openvsx.entities.SignatureKeyPair;
 import org.eclipse.openvsx.repositories.RepositoryService;
+import org.eclipse.openvsx.util.TimeUtil;
 import org.springframework.stereotype.Component;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.nio.charset.StandardCharsets;
 import java.security.SecureRandom;
-import java.time.LocalDateTime;
 import java.util.UUID;
 
 @Component
@@ -42,17 +42,12 @@ public GenerateKeyPairJobService(EntityManager entityManager, RepositoryService
     }
 
     @Transactional
-    public void renewKeyPair() {
-        var activeKeyPair = repositories.findActiveKeyPair();
-        if(activeKeyPair != null) {
-            activeKeyPair.setActive(false);
-        }
-
-        generateKeyPair();
+    public void updateKeyPair(SignatureKeyPair keyPair) {
+        repositories.deactivateKeyPairs();
+        entityManager.persist(keyPair);
     }
 
-    @Transactional
-    public void generateKeyPair() {
+    public SignatureKeyPair generateKeyPair() {
         var generator = new Ed25519KeyPairGenerator();
         generator.init(new Ed25519KeyGenerationParameters(new SecureRandom()));
         var pair = generator.generateKeyPair();
@@ -61,9 +56,9 @@ public void generateKeyPair() {
         keyPair.setPublicId(UUID.randomUUID().toString());
         keyPair.setPrivateKey(((Ed25519PrivateKeyParameters) pair.getPrivate()).getEncoded());
         keyPair.setPublicKeyText(getPublicKeyText(pair));
-        keyPair.setCreated(LocalDateTime.now());
+        keyPair.setCreated(TimeUtil.getCurrentUTC());
         keyPair.setActive(true);
-        entityManager.persist(keyPair);
+        return keyPair;
     }
 
     private String getPublicKeyText(AsymmetricCipherKeyPair pair) {

diff --git a/server/src/main/java/org/eclipse/openvsx/migration/MigrationScheduler.java b/server/src/main/java/org/eclipse/openvsx/migration/MigrationScheduler.java
@@ -15,7 +15,9 @@
 import org.springframework.context.event.EventListener;
 import org.springframework.stereotype.Component;
 
+import java.nio.charset.StandardCharsets;
 import java.time.Instant;
+import java.util.UUID;
 
 @Component
 public class MigrationScheduler {
@@ -25,13 +27,18 @@ public class MigrationScheduler {
     @Value("${ovsx.migrations.delay.seconds:0}")
     long delay;
 
+    @Value("${ovsx.registry.version:}")
+    String registryVersion;
+
     public MigrationScheduler(JobRequestScheduler scheduler) {
         this.scheduler = scheduler;
     }
 
     @EventListener
     public void applicationStarted(ApplicationStartedEvent event) {
         var instant = Instant.now().plusSeconds(delay);
-        scheduler.schedule(instant, new HandlerJobRequest<>(MigrationRunner.class));
+        var jobIdText = "MigrationRunner::" + registryVersion;
+        var jobId = UUID.nameUUIDFromBytes(jobIdText.getBytes(StandardCharsets.UTF_8));
+        scheduler.schedule(jobId, instant, new HandlerJobRequest<>(MigrationRunner.class));
     }
 }
diff --git a/server/src/main/java/org/eclipse/openvsx/publish/ExtensionVersionIntegrityService.java b/server/src/main/java/org/eclipse/openvsx/publish/ExtensionVersionIntegrityService.java
@@ -156,9 +156,12 @@ private byte[] generateSignatureManifest(TempFile extensionFile) throws IOExcept
             zip.stream()
                     .filter(entry -> !entry.isDirectory())
                     .forEach(entry -> {
-                        var content = ArchiveUtil.readEntry(zip, entry, ObservationRegistry.NOOP);
-                        var manifestEntry = generateManifestEntry(content, mapper, base64);
-                        manifestEntries.set(new String(base64.encode(entry.getName().getBytes(StandardCharsets.UTF_8))), manifestEntry);
+                        try {
+                            var manifestEntry = generateManifestEntry(zip.getInputStream(entry).readAllBytes(), mapper, base64);
+                            manifestEntries.set(new String(base64.encode(entry.getName().getBytes(StandardCharsets.UTF_8))), manifestEntry);
+                        } catch (IOException e) {
+                            logger.warn("Failed to add entry to manifest", e);
+                        }
                     });
         }
 

diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java b/server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java
@@ -609,4 +609,8 @@ public List<ExtensionVersion> findLatestVersions(UserData user) {
     public List<String> findExtensionTargetPlatforms(Extension extension) {
         return extensionVersionJooqRepo.findDistinctTargetPlatforms(extension);
     }
+
+    public void deactivateKeyPairs() {
+        signatureKeyPairRepo.updateActiveSetFalse();
+    }
 }
diff --git a/server/src/main/java/org/eclipse/openvsx/repositories/SignatureKeyPairRepository.java b/server/src/main/java/org/eclipse/openvsx/repositories/SignatureKeyPairRepository.java
@@ -10,6 +10,8 @@
 package org.eclipse.openvsx.repositories;
 
 import org.eclipse.openvsx.entities.SignatureKeyPair;
+import org.springframework.data.jpa.repository.Modifying;
+import org.springframework.data.jpa.repository.Query;
 import org.springframework.data.repository.Repository;
 
 public interface SignatureKeyPairRepository extends Repository<SignatureKeyPair, Long> {
@@ -19,4 +21,8 @@ public interface SignatureKeyPairRepository extends Repository<SignatureKeyPair,
     void deleteAll();
 
     SignatureKeyPair findByPublicId(String publicId);
+
+    @Modifying
+    @Query("update SignatureKeyPair k set k.active = false")
+    void updateActiveSetFalse();
 }
diff --git a/server/src/main/java/org/eclipse/openvsx/storage/StorageMigration.java b/server/src/main/java/org/eclipse/openvsx/storage/StorageMigration.java
@@ -23,6 +23,8 @@
 import org.springframework.transaction.support.TransactionTemplate;
 import org.springframework.web.client.RestTemplate;
 
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -95,7 +97,8 @@ public void findResources(ApplicationStartedEvent event) {
                             + migrations.get(i) + " to " + storageType + ".");
                 }
             }
-            scheduledFuture = taskScheduler.scheduleWithFixedDelay(this::migrateResources, migrationDelay);
+            var duration = Duration.of(migrationDelay, ChronoUnit.MILLIS);
+            scheduledFuture = taskScheduler.scheduleWithFixedDelay(this::migrateResources, duration);
         }
     }
 

diff --git a/server/src/test/java/org/eclipse/openvsx/publish/ExtensionVersionIntegrityServiceTest.java b/server/src/test/java/org/eclipse/openvsx/publish/ExtensionVersionIntegrityServiceTest.java
@@ -12,15 +12,16 @@
 import io.micrometer.observation.ObservationRegistry;
 import jakarta.persistence.EntityManager;
 import org.eclipse.openvsx.cache.CacheService;
-import org.eclipse.openvsx.entities.*;
+import org.eclipse.openvsx.entities.Extension;
+import org.eclipse.openvsx.entities.ExtensionVersion;
+import org.eclipse.openvsx.entities.FileResource;
+import org.eclipse.openvsx.entities.Namespace;
 import org.eclipse.openvsx.migration.GenerateKeyPairJobService;
 import org.eclipse.openvsx.repositories.RepositoryService;
 import org.eclipse.openvsx.util.ArchiveUtil;
 import org.eclipse.openvsx.util.TempFile;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Mockito;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.TestConfiguration;
 import org.springframework.boot.test.mock.mockito.MockBean;
@@ -34,12 +35,9 @@
 import static org.junit.jupiter.api.Assertions.*;
 
 @ExtendWith(SpringExtension.class)
-@MockBean({ CacheService.class, RepositoryService.class })
+@MockBean({ CacheService.class, RepositoryService.class, EntityManager.class })
 public class ExtensionVersionIntegrityServiceTest {
 
-    @MockBean
-    EntityManager entityManager;
-
     @Autowired
     ExtensionVersionIntegrityService integrityService;
 
@@ -48,10 +46,7 @@ public class ExtensionVersionIntegrityServiceTest {
 
     @Test
     public void testGenerateSignature() throws IOException {
-        keyPairService.generateKeyPair();
-        var keyPairCaptor = ArgumentCaptor.forClass(SignatureKeyPair.class);
-        Mockito.verify(entityManager).persist(keyPairCaptor.capture());
-        var keyPair = keyPairCaptor.getValue();
+        var keyPair = keyPairService.generateKeyPair();
 
         var namespace = new Namespace();
         namespace.setName("foo");

diff --git a/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java b/server/src/test/java/org/eclipse/openvsx/repositories/RepositoryServiceSmokeTest.java
@@ -202,7 +202,8 @@ void testExecuteQueries() {
                 () -> repositories.findLatestVersion(extension, "targetPlatform", false, false),
                 () -> repositories.findLatestVersions(namespace),
                 () -> repositories.findLatestVersions(userData),
-                () -> repositories.findExtensionTargetPlatforms(extension)
+                () -> repositories.findExtensionTargetPlatforms(extension),
+                () -> repositories.deactivateKeyPairs()
         );
 
         // check that we did not miss anything