diff options
11 files changed, 138 insertions, 30 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index 78d195821bf..41896468769 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -13,12 +13,12 @@ import com.yahoo.config.application.api.ComponentInfo; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.UnparsedConfigDefinition; import com.yahoo.config.codegen.DefParser; +import com.yahoo.config.model.application.AbstractApplicationPackage; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; -import com.yahoo.config.model.application.AbstractApplicationPackage; import com.yahoo.io.HexDump; import com.yahoo.io.IOUtils; import com.yahoo.io.reader.NamedReader; @@ -33,7 +33,6 @@ import org.w3c.dom.Element; import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; - import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; @@ -48,6 +47,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.Reader; import java.io.StringReader; +import java.nio.file.Files; import java.security.MessageDigest; import java.util.ArrayList; import java.util.Arrays; @@ -61,7 +61,6 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.BiFunction; -import java.util.function.Function; import java.util.logging.Level; import java.util.logging.Logger; @@ -69,7 +68,7 @@ import static com.yahoo.text.Lowercase.toLowerCase; /** - * Application package derived from local files, ie. during deploy. + * Application package derived from local files, i.e. during deploy. * Construct using {@link com.yahoo.config.model.application.provider.FilesApplicationPackage#fromFile(java.io.File)} or * {@link com.yahoo.config.model.application.provider.FilesApplicationPackage#fromFileWithDeployData(java.io.File, DeployData)}. * @@ -749,4 +748,40 @@ public class FilesApplicationPackage extends AbstractApplicationPackage { return file; } + /* Validates that files in application dir and subdirectories have a known extension */ + public void validateFileExtensions(boolean throwIfInvalid) { + // TODO: Define this for all subdirs + Map<Path, Set<String>> validFileSuffixes = Map.of( + QUERY_PROFILES_DIR, Set.of(".xml"), + RULES_DIR, Set.of(".sr"), + SCHEMAS_DIR, Set.of(".sd", ".expression"), + SEARCH_DEFINITIONS_DIR, Set.of(".sd", ".expression")); + + validFileSuffixes.forEach((key, value) -> { + java.nio.file.Path path = appDir.toPath().resolve((key.toFile().toPath())); + File dir = path.toFile(); + if ( ! dir.exists() || ! dir.isDirectory()) return; + + try (var filesInPath = Files.list(path)) { + filesInPath.forEach(f -> { + validateFileSuffix(path, f, value, throwIfInvalid); + }); + } catch (IOException e) { + log.log(Level.WARNING, "Unable to list files in " + dir, e); + } + }); + } + + private void validateFileSuffix(java.nio.file.Path dir, java.nio.file.Path pathToFile, Set<String> allowedSuffixes, boolean throwIfInvalid) { + String fileName = pathToFile.toFile().getName(); + if (allowedSuffixes.stream().noneMatch(fileName::endsWith)) { + String message = "File in application package with unknown suffix: " + + appDir.toPath().relativize(dir).resolve(fileName) + " Please delete or move file to another directory."; + if (throwIfInvalid) + throw new IllegalArgumentException(message); + else + log.log(Level.INFO, message); + } + } + } diff --git a/config-application-package/src/test/java/com/yahoo/config/model/application/provider/FilesApplicationPackageTest.java b/config-application-package/src/test/java/com/yahoo/config/model/application/provider/FilesApplicationPackageTest.java index fd5437c27de..bea7efb40f4 100644 --- a/config-application-package/src/test/java/com/yahoo/config/model/application/provider/FilesApplicationPackageTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/model/application/provider/FilesApplicationPackageTest.java @@ -105,7 +105,7 @@ public class FilesApplicationPackageTest { } @Test - public void testLegacyOverrides() throws IOException { + public void testLegacyOverrides() { File appDir = new File("src/test/resources/app-legacy-overrides"); ApplicationPackage app = FilesApplicationPackage.fromFile(appDir); var overrides = app.legacyOverrides(); @@ -143,4 +143,25 @@ public class FilesApplicationPackageTest { .getMessage()); } + @Test + public void testValidFileExtensions() { + File appDir = new File("src/test/resources/app-with-deployment");; + FilesApplicationPackage app = FilesApplicationPackage.fromFile(appDir); + app.validateFileExtensions(true); + } + + @Test + public void testInvalidFileExtensions() { + File appDir = new File("src/test/resources/app-with-invalid-files-in-subdir");; + FilesApplicationPackage app = FilesApplicationPackage.fromFile(appDir); + try { + app.validateFileExtensions(true); + fail("expected an exception"); + } catch (IllegalArgumentException e) { + assertEquals("File in application package with unknown suffix: search/query-profiles/file-with-invalid.extension " + + "Please delete or move file to another directory.", + e.getMessage()); + } + } + } diff --git a/config-application-package/src/test/resources/app-with-deployment/search/query-profiles/default.xml b/config-application-package/src/test/resources/app-with-deployment/search/query-profiles/default.xml new file mode 100644 index 00000000000..6e3069d8f96 --- /dev/null +++ b/config-application-package/src/test/resources/app-with-deployment/search/query-profiles/default.xml @@ -0,0 +1,4 @@ +<query-profile id="default"> + <field name="maxHits">30000</field> + <field name="maxOffset">30000</field> +</query-profile> diff --git a/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/deployment.xml b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/deployment.xml new file mode 100644 index 00000000000..3aad0ca6a6a --- /dev/null +++ b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/deployment.xml @@ -0,0 +1,9 @@ +<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<deployment version='1.0'> + <test /> + <staging /> + <prod> + <region active="true">us-east</region> + <region active="false">us-west-1</region> + </prod> +</deployment> diff --git a/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/hosts.xml b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/hosts.xml new file mode 100644 index 00000000000..64a07644038 --- /dev/null +++ b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/hosts.xml @@ -0,0 +1,10 @@ +<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<hosts xmlns:deploy="vespa" xmlns:preprocess="properties"> + <preprocess:properties> + <node1.hostname>foo.yahoo.com</node1.hostname> + <node1.hostname deploy:environment="dev">bar.yahoo.com</node1.hostname> + </preprocess:properties> + <host name="${node1.hostname}"> + <alias>node1</alias> + </host> +</hosts> diff --git a/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/schemas/music.sd b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/schemas/music.sd new file mode 100644 index 00000000000..7da7c49c162 --- /dev/null +++ b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/schemas/music.sd @@ -0,0 +1,8 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +search music { + document music { + field f type string { + indexing: index | summary + } + } +} diff --git a/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/search/query-profiles/file-with-invalid.extension b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/search/query-profiles/file-with-invalid.extension new file mode 100644 index 00000000000..e69de29bb2d --- /dev/null +++ b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/search/query-profiles/file-with-invalid.extension diff --git a/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/services.xml b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/services.xml new file mode 100644 index 00000000000..60d08cb615c --- /dev/null +++ b/config-application-package/src/test/resources/app-with-invalid-files-in-subdir/services.xml @@ -0,0 +1,12 @@ +<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> +<services version='1.0'> + <admin version='2.0'> + <adminserver hostalias='node0'/> + </admin> + <content version='1.0' id='foo'> + <redundancy>1</redundancy> + <documents> + <document type="music.sd" mode="index" /> + </documents> + </content> +</services> diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 32422889b48..e4b1bf8ae58 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -360,9 +360,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye return deploy(applicationPackage, prepareParams, DeployHandlerLogger.forPrepareParams(prepareParams)); } - private PrepareResult deploy(File applicationPackage, PrepareParams prepareParams, DeployHandlerLogger logger) { + private PrepareResult deploy(File applicationDir, PrepareParams prepareParams, DeployHandlerLogger logger) { ApplicationId applicationId = prepareParams.getApplicationId(); - long sessionId = createSession(applicationId, prepareParams.getTimeoutBudget(), applicationPackage); + long sessionId = createSession(applicationId, prepareParams.getTimeoutBudget(), applicationDir); Deployment deployment = prepare(sessionId, prepareParams, logger); if ( ! prepareParams.isDryRun()) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index d292e1c2c0e..120a75bdd72 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -43,7 +43,9 @@ import com.yahoo.vespa.config.server.zookeeper.SessionCounter; import com.yahoo.vespa.config.server.zookeeper.ZKApplication; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; import com.yahoo.yolean.Exceptions; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.cache.ChildData; @@ -127,6 +129,7 @@ public class SessionRepository { private final ModelFactoryRegistry modelFactoryRegistry; private final ConfigDefinitionRepo configDefinitionRepo; private final int maxNodeSize; + private final BooleanFlag failDeploymentForFilesWithUnknownExtension; public SessionRepository(TenantName tenantName, TenantApplications applicationRepo, @@ -170,6 +173,7 @@ public class SessionRepository { this.modelFactoryRegistry = modelFactoryRegistry; this.configDefinitionRepo = configDefinitionRepo; this.maxNodeSize = maxNodeSize; + this.failDeploymentForFilesWithUnknownExtension = Flags.FAIL_DEPLOYMENT_FOR_FILES_WITH_UNKNOWN_EXTENSION.bindTo(flagSource); loadSessions(); // Needs to be done before creating cache below this.directoryCache = curator.createDirectoryCache(sessionsPath.getAbsolute(), false, false, zkCacheExecutor); @@ -673,17 +677,19 @@ public class SessionRepository { } DeployData deployData = new DeployData(user, userDir.getAbsolutePath(), applicationId, deployTimestamp, internalRedeploy, sessionId, currentlyActiveSessionId.orElse(nonExistingActiveSessionId)); - return FilesApplicationPackage.fromFileWithDeployData(configApplicationDir, deployData); + FilesApplicationPackage app = FilesApplicationPackage.fromFileWithDeployData(configApplicationDir, deployData); + app.validateFileExtensions(failDeploymentForFilesWithUnknownExtension.value()); + return app; } - private LocalSession createSessionFromApplication(File applicationFile, + private LocalSession createSessionFromApplication(File applicationDirectory, ApplicationId applicationId, boolean internalRedeploy, TimeoutBudget timeoutBudget) { long sessionId = getNextSessionId(); try { ensureSessionPathDoesNotExist(sessionId); - ApplicationPackage app = createApplicationPackage(applicationFile, applicationId, sessionId, internalRedeploy); + ApplicationPackage app = createApplicationPackage(applicationDirectory, applicationId, sessionId, internalRedeploy); log.log(Level.FINE, () -> TenantRepository.logPre(tenantName) + "Creating session " + sessionId + " in ZooKeeper"); SessionZooKeeperClient sessionZKClient = createSessionZooKeeperClient(sessionId); sessionZKClient.createNewSession(clock.instant()); @@ -697,7 +703,7 @@ public class SessionRepository { } } - private ApplicationPackage createApplicationPackage(File applicationFile, + private ApplicationPackage createApplicationPackage(File applicationDirectory, ApplicationId applicationId, long sessionId, boolean internalRedeploy) throws IOException { @@ -706,8 +712,8 @@ public class SessionRepository { synchronized (monitor) { Optional<Long> activeSessionId = getActiveSessionId(applicationId); File userApplicationDir = getSessionAppDir(sessionId); - copyApp(applicationFile, userApplicationDir); - ApplicationPackage applicationPackage = createApplication(applicationFile, + copyApp(applicationDirectory, userApplicationDir); + ApplicationPackage applicationPackage = createApplication(applicationDirectory, userApplicationDir, applicationId, sessionId, @@ -751,7 +757,7 @@ public class SessionRepository { log.log(Level.FINE, "Moving " + tempDestinationDir + " to " + destinationDir.getAbsolutePath()); Files.move(tempDestinationDir, destinationDir.toPath(), StandardCopyOption.ATOMIC_MOVE); } finally { - // In case some of the operations above fail + // In case some operations above fail if (tempDestinationDir != null) IOUtils.recursiveDeleteDir(tempDestinationDir.toFile()); } @@ -812,7 +818,7 @@ public class SessionRepository { sessionDir = fileDirectory.getFile(fileReference); } catch (IllegalArgumentException e) { // We cannot be guaranteed that the file reference exists (it could be that it has not - // been downloaded yet), and e.g when bootstrapping we cannot throw an exception in that case + // been downloaded yet), and e.g. when bootstrapping we cannot throw an exception in that case log.log(Level.FINE, () -> "File reference for session id " + sessionId + ": " + fileReference + " not found in " + fileDirectory); return; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java index 9071b12507e..cb4b0e9d5bd 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java @@ -29,7 +29,6 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; - import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; @@ -123,7 +122,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void require_that_activate_url_is_returned_on_success() throws Exception { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); HttpResponse response = request(HttpRequest.Method.PUT, sessionId); assertNotNull(response); assertEquals(OK, response.getStatus()); @@ -141,7 +140,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void require_verbose() throws Exception { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); HttpResponse response = createHandler().handle( createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.PREPARED, sessionId, "?verbose=true")); System.out.println(getRenderedString(response)); @@ -151,7 +150,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void require_get_response_activate_url_on_ok() throws Exception { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); request(HttpRequest.Method.PUT, sessionId); HttpResponse getResponse = request(HttpRequest.Method.GET, sessionId); assertResponseContains(getResponse, "\"activate\":\"http://foo:1337" + pathPrefix + @@ -160,7 +159,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void require_get_response_error_on_not_prepared() throws Exception { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); HttpResponse getResponse = request(HttpRequest.Method.GET, sessionId); assertHttpStatusCodeErrorCodeAndMessage(getResponse, BAD_REQUEST, @@ -186,7 +185,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void require_that_tenant_is_in_response() throws Exception { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); HttpResponse response = request(HttpRequest.Method.PUT, sessionId); assertNotNull(response); assertEquals(OK, response.getStatus()); @@ -200,7 +199,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { TenantName defaultTenant = TenantName.from("test2"); tenantRepository.addTenant(defaultTenant); ApplicationId applicationId1 = ApplicationId.from(defaultTenant, ApplicationName.from("app"), InstanceName.defaultName()); - long sessionId = applicationRepository.createSession(applicationId1, timeoutBudget, app); + long sessionId = createSession(applicationId1); pathPrefix = "/application/v2/tenant/" + defaultTenant + "/session/"; HttpResponse response = request(HttpRequest.Method.PUT, sessionId); @@ -209,7 +208,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { String applicationName = "myapp"; ApplicationId applicationId2 = ApplicationId.from(tenant.value(), applicationName, "default"); - long sessionId2 = applicationRepository.createSession(applicationId2, timeoutBudget, app); + long sessionId2 = createSession(applicationId2); assertEquals(sessionId, sessionId2); // Want to test when they are equal (but for different tenants) pathPrefix = "/application/v2/tenant/" + tenant + "/session/" + sessionId2 + @@ -219,7 +218,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { assertEquals(SessionHandlerTest.getRenderedString(response), OK, response.getStatus()); ApplicationId applicationId3 = ApplicationId.from(tenant.value(), applicationName, "quux"); - long sessionId3 = applicationRepository.createSession(applicationId3, timeoutBudget, app); + long sessionId3 = createSession(applicationId3); pathPrefix = "/application/v2/tenant/" + tenant + "/session/" + sessionId3 + "/prepared?applicationName=" + applicationName + "&instance=quux"; response = handler.handle(SessionHandlerTest.createTestRequest(pathPrefix)); @@ -229,14 +228,14 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void require_that_config_change_actions_are_in_response() throws Exception { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); HttpResponse response = request(HttpRequest.Method.PUT, sessionId); assertResponseContains(response, "\"configChangeActions\":{\"restart\":[],\"refeed\":[],\"reindex\":[]}"); } @Test public void require_that_config_change_actions_are_not_logged_if_not_existing() throws Exception { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); HttpResponse response = request(HttpRequest.Method.PUT, sessionId); assertResponseNotContains(response, "Change(s) between active and new application that may require restart"); assertResponseNotContains(response, "Change(s) between active and new application that may require re-feed"); @@ -245,7 +244,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void test_out_of_capacity_response() throws IOException { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); String exceptionMessage = "Node allocation failure"; FailingSessionPrepareHandler handler = new FailingSessionPrepareHandler(SessionPrepareHandler.testContext(), applicationRepository, @@ -260,7 +259,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void test_that_nullpointerexception_gives_internal_server_error() throws IOException { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); String exceptionMessage = "nullpointer thrown in test handler"; FailingSessionPrepareHandler handler = new FailingSessionPrepareHandler(SessionPrepareHandler.testContext(), applicationRepository, @@ -276,7 +275,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void test_application_lock_failure() throws IOException { String exceptionMessage = "Timed out after waiting PT1M to acquire lock '/provision/v1/locks/foo/bar/default'"; - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); FailingSessionPrepareHandler handler = new FailingSessionPrepareHandler(SessionPrepareHandler.testContext(), applicationRepository, configserverConfig, @@ -290,7 +289,7 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { @Test public void test_docker_image_repository() { - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, app); + long sessionId = createSession(applicationId()); String dockerImageRepository = "foo.bar.com:4443/baz"; request(HttpRequest.Method.PUT, sessionId, Map.of("dockerImageRepository", dockerImageRepository, "applicationName", applicationId().application().value())); @@ -329,6 +328,10 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { return ApplicationId.from(tenant.value(), "app", "default"); } + private long createSession(ApplicationId applicationId) { + return applicationRepository.createSession(applicationId, timeoutBudget, app); + } + private static class FailingSessionPrepareHandler extends SessionPrepareHandler { private final RuntimeException exceptionToBeThrown; |