Reject config request when major version is incompatible

6 files changed, 92 insertions, 24 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java
index 99f5cce642a..20b16614a53 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/RequestHandler.java
@@ -92,4 +92,8 @@ public interface RequestHandler {
      * @return set of file references that is owned by the application
      */
     Set<FileReference> listFileReferences(ApplicationId applicationId);
+
+    /** Returns whether the latest deployed version of application is compatible with given vespaVersion */
+    boolean compatibleWith(Optional<Version> vespaVersion, ApplicationId application);
+
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java
index 9fbc3eb21df..0e4fffaccef 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/SuperModelRequestHandler.java
@@ -108,6 +108,11 @@ public class SuperModelRequestHandler implements RequestHandler {
         throw new UnsupportedOperationException();
     }
 
+    @Override
+    public boolean compatibleWith(Optional<Version> vespaVersion, ApplicationId application) {
+        return true;
+    }
+
     public void enable() {
         enabled = true;
         superModelManager.markAsComplete();
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
index 937ec2bb0e7..7c669f290d3 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
@@ -26,6 +26,9 @@ import com.yahoo.vespa.curator.CompletionTimeoutException;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.curator.transaction.CuratorTransaction;
+import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.ListFlag;
+import com.yahoo.vespa.flags.PermanentFlags;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent;
 
@@ -71,11 +74,12 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica
     private final Clock clock;
     private final TenantFileSystemDirs tenantFileSystemDirs;
     private final ConfigserverConfig configserverConfig;
+    private final ListFlag<Integer> incompatibleMajorVersions;
 
     public TenantApplications(TenantName tenant, Curator curator, StripedExecutor<TenantName> zkWatcherExecutor,
                               ExecutorService zkCacheExecutor, Metrics metrics, ReloadListener reloadListener,
                               ConfigserverConfig configserverConfig, HostRegistry hostRegistry,
-                              TenantFileSystemDirs tenantFileSystemDirs, Clock clock) {
+                              TenantFileSystemDirs tenantFileSystemDirs, Clock clock, FlagSource flagSource) {
         this.curator = curator;
         this.database = new ApplicationCuratorDatabase(tenant, curator);
         this.tenant = tenant;
@@ -91,6 +95,7 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica
         this.tenantFileSystemDirs = tenantFileSystemDirs;
         this.clock = clock;
         this.configserverConfig = configserverConfig;
+        this.incompatibleMajorVersions = PermanentFlags.INCOMPATIBLE_MAJOR_VERSIONS.bindTo(flagSource);
     }
 
     /** The curator backed ZK storage of this. */
@@ -383,6 +388,15 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica
     }
 
     @Override
+    public boolean compatibleWith(Optional<Version> vespaVersion, ApplicationId application) {
+        if (vespaVersion.isEmpty()) return true;
+        Version latestDeployed = applicationMapper.getForVersion(application, Optional.empty(), clock.instant())
+                                                  .getVespaVersion();
+        boolean compatibleMajor = !incompatibleMajorVersions.value().contains(latestDeployed.getMajor());
+        return compatibleMajor || vespaVersion.get().getMajor() == latestDeployed.getMajor();
+    }
+
+    @Override
     public void verifyHosts(ApplicationId applicationId, Collection<String> newHosts) {
         hostRegistry.verifyHosts(applicationId, newHosts);
         reloadListener.verifyHostsAreAvailable(applicationId, newHosts);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java
index 88a30a9d72b..55645d83b3c 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/GetConfigProcessor.java
@@ -100,11 +100,16 @@ class GetConfigProcessor implements Runnable {
                 request.getVespaVersion().map(VespaVersion::toString).map(Version::fromString) :
                 Optional.empty();
         if (logDebug(trace)) {
-            debugLog(trace, "Using version " + getPrintableVespaVersion(vespaVersion));
+            debugLog(trace, "Using version " + printableVespaVersion(vespaVersion));
         }
 
         if ( ! context.requestHandler().hasApplication(context.applicationId(), vespaVersion)) {
-            handleError(request, ErrorCode.UNKNOWN_VESPA_VERSION, "Unknown Vespa version in request: " + getPrintableVespaVersion(vespaVersion));
+            handleError(request, ErrorCode.UNKNOWN_VESPA_VERSION, "Unknown Vespa version in request: " + printableVespaVersion(vespaVersion));
+            return null;
+        }
+
+        if ( ! context.requestHandler().compatibleWith(vespaVersion, context.applicationId())) {
+            handleError(request, ErrorCode.INCOMPATIBLE_VESPA_VERSION, "Version " + printableVespaVersion(vespaVersion) + " is binary incompatible with the latest deployed version");
             return null;
         }
 
@@ -161,7 +166,7 @@ class GetConfigProcessor implements Runnable {
                request.getConfigKey().getNamespace().equals(SentinelConfig.getDefNamespace());
     }
 
-    private static String getPrintableVespaVersion(Optional<Version> vespaVersion) {
+    private static String printableVespaVersion(Optional<Version> vespaVersion) {
         return (vespaVersion.isPresent() ? vespaVersion.get().toFullString() : "LATEST");
     }
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
index 7d53c383cf5..82d8a42bc92 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
@@ -342,7 +342,8 @@ public class TenantRepository {
                                        configserverConfig,
                                        hostRegistry,
                                        new TenantFileSystemDirs(configServerDB, tenantName),
-                                       clock);
+                                       clock,
+                                       flagSource);
         PermanentApplicationPackage permanentApplicationPackage = new PermanentApplicationPackage(configserverConfig);
         SessionPreparer sessionPreparer = new SessionPreparer(modelFactoryRegistry,
                                                               fileDistributionFactory,
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java
index 5afcd62a25c..2b361e825e5 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java
@@ -26,6 +26,8 @@ import com.yahoo.vespa.curator.CompletionTimeoutException;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
 import com.yahoo.vespa.curator.mock.MockCuratorFramework;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
+import com.yahoo.vespa.flags.PermanentFlags;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.VespaModelFactory;
 import org.apache.curator.framework.CuratorFramework;
@@ -47,6 +49,7 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
 import java.util.stream.IntStream;
 
 import static com.yahoo.vespa.config.server.application.TenantApplications.RemoveApplicationWaiter;
@@ -66,7 +69,6 @@ public class TenantApplicationsTest {
 
     private Curator curator;
     private CuratorFramework curatorFramework;
-    private TenantApplications applications;
     private ConfigserverConfig configserverConfig;
 
     @Rule
@@ -88,7 +90,6 @@ public class TenantApplicationsTest {
                 .build();
         tenantRepository.addTenant(TenantRepository.HOSTED_VESPA_TENANT);
         tenantRepository.addTenant(tenantName);
-        applications = createTenantApplications(tenantName, curator, configserverConfig, new MockReloadListener());
     }
 
     @Test
@@ -150,6 +151,43 @@ public class TenantApplicationsTest {
         assertEquals(0, repo.activeApplications().size());
     }
 
+    @Test
+    public void major_version_compatibility() throws Exception {
+        InMemoryFlagSource flagSource = new InMemoryFlagSource();
+        TenantApplications applications = createZKAppRepo(flagSource);
+        ApplicationId app1 = createApplicationId("myapp");
+        applications.createApplication(app1);
+        applications.createPutTransaction(app1, 1).commit();
+        VespaModel model = new VespaModel(FilesApplicationPackage.fromFile(new File("src/test/apps/app")));
+        Function<Version, ApplicationSet> createApplicationSet = (version) -> {
+            return ApplicationSet.from(new Application(model,
+                                                new ServerCache(),
+                                                1,
+                                                version,
+                                                MetricUpdater.createTestUpdater(),
+                                                app1));
+        };
+
+        Version deployedVersion0 = Version.fromString("6.1");
+        applications.activateApplication(createApplicationSet.apply(deployedVersion0), 1);
+        assertTrue("Empty version is compatible", applications.compatibleWith(Optional.empty(), app1));
+
+        Version nodeVersion0 = Version.fromString("6.0");
+        assertTrue("Lower version is compatible", applications.compatibleWith(Optional.of(nodeVersion0), app1));
+
+        Version deployedVersion1 = Version.fromString("7.1");
+        applications.activateApplication(createApplicationSet.apply(deployedVersion1), 1);
+        assertTrue("New major is compatible", applications.compatibleWith(Optional.of(nodeVersion0), app1));
+
+        flagSource.withListFlag(PermanentFlags.INCOMPATIBLE_MAJOR_VERSIONS.id(), List.of(8), Integer.class);
+        Version deployedVersion2 = Version.fromString("8.1");
+        applications.activateApplication(createApplicationSet.apply(deployedVersion2), 1);
+        assertFalse("New major is incompatible", applications.compatibleWith(Optional.of(nodeVersion0), app1));
+
+        Version nodeVersion1 = Version.fromString("8.0");
+        assertTrue("Node is compatible after upgrading", applications.compatibleWith(Optional.of(nodeVersion1), app1));
+    }
+
     public static class MockReloadListener implements ReloadListener {
         public final AtomicInteger reloaded = new AtomicInteger(0);
         final AtomicInteger removed = new AtomicInteger(0);
@@ -175,14 +213,10 @@ public class TenantApplicationsTest {
         }
     }
 
-    private void assertdefaultAppNotFound() {
-        assertFalse(applications.hasApplication(ApplicationId.defaultId(), Optional.of(vespaVersion)));
-    }
-
     @Test
     public void testListConfigs() throws IOException, SAXException {
-        applications = createTenantApplications(TenantName.defaultName(), new MockCurator(), configserverConfig, new MockReloadListener());
-        assertdefaultAppNotFound();
+        TenantApplications applications = createTenantApplications(TenantName.defaultName(), new MockCurator(), configserverConfig, new MockReloadListener(), new InMemoryFlagSource());
+        assertFalse(applications.hasApplication(ApplicationId.defaultId(), Optional.of(vespaVersion)));
 
         VespaModel model = new VespaModel(FilesApplicationPackage.fromFile(new File("src/test/apps/app")));
         ApplicationId applicationId = ApplicationId.defaultId();
@@ -209,6 +243,7 @@ public class TenantApplicationsTest {
 
     @Test
     public void testAppendIdsInNonRecursiveListing() {
+        TenantApplications applications = createTenantApplications(tenantName, curator, configserverConfig, new MockReloadListener(), new InMemoryFlagSource());
         assertEquals(applications.appendOneLevelOfId("search/music", "search/music/qrservers/default/qr.0"), "search/music/qrservers");
         assertEquals(applications.appendOneLevelOfId("search", "search/music/qrservers/default/qr.0"), "search/music");
         assertEquals(applications.appendOneLevelOfId("search/music/qrservers/default/qr.0", "search/music/qrservers/default/qr.0"), "search/music/qrservers/default/qr.0");
@@ -251,9 +286,12 @@ public class TenantApplicationsTest {
     }
 
     private TenantApplications createZKAppRepo() {
-        return createTenantApplications(tenantName, curator, configserverConfig, new MockReloadListener());
+        return createZKAppRepo(new InMemoryFlagSource());
     }
 
+    private TenantApplications createZKAppRepo(InMemoryFlagSource flagSource) {
+        return createTenantApplications(tenantName, curator, configserverConfig, new MockReloadListener(), flagSource);
+    }
 
     private static ApplicationId createApplicationId() {
         return createApplicationId("foo");
@@ -285,17 +323,18 @@ public class TenantApplicationsTest {
     private TenantApplications createTenantApplications(TenantName tenantName,
                        Curator curator,
                        ConfigserverConfig configserverConfig,
-                       ReloadListener reloadListener) {
+                       ReloadListener reloadListener, InMemoryFlagSource flagSource) {
         return new TenantApplications(tenantName,
-             curator,
-             new StripedExecutor<>(new InThreadExecutorService()),
-             new InThreadExecutorService(),
-             Metrics.createTestMetrics(),
-             reloadListener,
-             configserverConfig,
-             new HostRegistry(),
-             new TenantFileSystemDirs(new ConfigServerDB(configserverConfig), tenantName),
-             Clock.systemUTC());
+                                      curator,
+                                      new StripedExecutor<>(new InThreadExecutorService()),
+                                      new InThreadExecutorService(),
+                                      Metrics.createTestMetrics(),
+                                      reloadListener,
+                                      configserverConfig,
+                                      new HostRegistry(),
+                                      new TenantFileSystemDirs(new ConfigServerDB(configserverConfig), tenantName),
+                                      Clock.systemUTC(),
+                                      flagSource);
     }
 
     private static class MockCurator3ConfigServers extends Curator {