From bd59e883730c05138a10c4b175aa7edc0e470984 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Mon, 15 May 2023 16:33:27 +0200 Subject: Revert "Revert "Bjorncs/cloud app validation"" --- .../validation/CloudHttpConnectorValidator.java | 47 ++++++++++ .../model/application/validation/Validation.java | 5 +- .../model/builder/xml/dom/DomAdminV4Builder.java | 4 +- .../model/container/http/ConnectorFactory.java | 2 + .../model/container/xml/ContainerModelBuilder.java | 2 +- .../CloudHttpConnectorValidatorTest.java | 103 +++++++++++++++++++++ .../xml/JettyContainerModelBuilderTest.java | 10 +- 7 files changed, 159 insertions(+), 14 deletions(-) create mode 100644 config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidator.java create mode 100644 config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidatorTest.java diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidator.java new file mode 100644 index 00000000000..737042a3695 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidator.java @@ -0,0 +1,47 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.model.application.validation; + +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; +import com.yahoo.vespa.model.container.Container; +import com.yahoo.vespa.model.container.http.JettyHttpServer; +import com.yahoo.vespa.model.container.http.ssl.ConfiguredDirectSslProvider; +import com.yahoo.vespa.model.container.http.ssl.DefaultSslProvider; +import com.yahoo.vespa.model.container.xml.ContainerModelBuilder; + +import java.util.List; + +/** + * Enforces that Cloud applications cannot + * 1) override connector specific TLS configuration + * 2) add additional HTTP connectors + * + * @author bjorncs + */ +public class CloudHttpConnectorValidator extends Validator { + @Override + public void validate(VespaModel model, DeployState state) { + if (!state.isHostedTenantApplication(model.getAdmin().getApplicationType())) return; + + model.getContainerClusters().forEach((__, cluster) -> { + var http = cluster.getHttp(); + if (http == null) return; + var connectors = http.getHttpServer().map(JettyHttpServer::getConnectorFactories).orElse(List.of()); + for (var connector : connectors) { + int port = connector.getListenPort(); + if (!List.of(ContainerModelBuilder.HOSTED_VESPA_DATAPLANE_PORT, Container.BASEPORT).contains(port)) { + throw new IllegalArgumentException( + "Adding additional HTTP connectors is not allowed for Vespa Cloud applications. " + + "See https://cloud.vespa.ai/en/security/whitepaper."); + } + var sslProvider = connector.sslProvider(); + if (!(sslProvider instanceof ConfiguredDirectSslProvider || sslProvider instanceof DefaultSslProvider)) { + throw new IllegalArgumentException( + "Overriding connector specific TLS configuration is not allowed in Vespa Cloud. " + + "See https://cloud.vespa.ai/en/security/guide#data-plane."); + } + } + }); + } +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java index 4f2f8e7932c..efa02781f7e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.config.model.api.ConfigChangeAction; @@ -13,7 +12,6 @@ import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.change.CertificateRemovalChangeValidator; import com.yahoo.vespa.model.application.validation.change.ChangeValidator; import com.yahoo.vespa.model.application.validation.change.CloudAccountChangeValidator; -import com.yahoo.vespa.model.application.validation.change.ResourcesReductionValidator; import com.yahoo.vespa.model.application.validation.change.ConfigValueChangeValidator; import com.yahoo.vespa.model.application.validation.change.ContainerRestartValidator; import com.yahoo.vespa.model.application.validation.change.ContentClusterRemovalValidator; @@ -23,11 +21,11 @@ import com.yahoo.vespa.model.application.validation.change.IndexedSearchClusterC import com.yahoo.vespa.model.application.validation.change.IndexingModeChangeValidator; import com.yahoo.vespa.model.application.validation.change.NodeResourceChangeValidator; import com.yahoo.vespa.model.application.validation.change.RedundancyIncreaseValidator; +import com.yahoo.vespa.model.application.validation.change.ResourcesReductionValidator; import com.yahoo.vespa.model.application.validation.change.StartupCommandChangeValidator; import com.yahoo.vespa.model.application.validation.change.StreamingSearchClusterChangeValidator; import com.yahoo.vespa.model.application.validation.first.RedundancyValidator; -import java.time.Instant; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -88,6 +86,7 @@ public class Validation { new CloudDataPlaneFilterValidator().validate(model, deployState); new AccessControlFilterExcludeValidator().validate(model, deployState); new CloudUserFilterValidator().validate(model, deployState); + new CloudHttpConnectorValidator().validate(model, deployState); additionalValidators.forEach(v -> v.validate(model, deployState)); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java index 4990ddc9a53..588ecab537a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java @@ -94,9 +94,7 @@ public class DomAdminV4Builder extends DomAdminBuilderBase { private NodesSpecification createNodesSpecificationForLogserver() { DeployState deployState = context.getDeployState(); if ( deployState.getProperties().useDedicatedNodeForLogserver() - && context.getApplicationType() == ConfigModelContext.ApplicationType.DEFAULT - && deployState.isHosted() - && ! deployState.getProperties().applicationId().instance().isTester()) + && deployState.isHostedTenantApplication(context.getApplicationType())) return NodesSpecification.dedicated(1, context); else return NodesSpecification.nonDedicated(1, context); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ConnectorFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ConnectorFactory.java index c76077e6c7b..697cfc95039 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ConnectorFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ConnectorFactory.java @@ -59,6 +59,8 @@ public class ConnectorFactory extends SimpleComponent implements ConnectorConfig public void setDefaultResponseFilterChain(ComponentId filterChain) { this.defaultResponseFilterChain = filterChain; } + public SslProvider sslProvider() { return sslProviderComponent; } + public static class Builder { private final String name; private final int listenPort; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 3d1c8ca1d76..3cb0731c43a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -136,7 +136,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { static final String HOSTED_VESPA_STATUS_FILE = Defaults.getDefaults().underVespaHome("var/vespa/load-balancer/status.html"); // Data plane port for hosted Vespa - static final int HOSTED_VESPA_DATAPLANE_PORT = 4443; + public static final int HOSTED_VESPA_DATAPLANE_PORT = 4443; //Path to vip status file for container in Hosted Vespa. Only used if set, else use HOSTED_VESPA_STATUS_FILE private static final String HOSTED_VESPA_STATUS_FILE_SETTING = "VESPA_LB_STATUS_FILE"; diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidatorTest.java new file mode 100644 index 00000000000..6a2eed1d21b --- /dev/null +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidatorTest.java @@ -0,0 +1,103 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.model.application.validation; + +import com.yahoo.config.model.NullConfigModelRegistry; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.config.model.deploy.TestProperties; +import com.yahoo.config.model.test.MockApplicationPackage; +import com.yahoo.vespa.model.VespaModel; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * @author bjorncs + */ +class CloudHttpConnectorValidatorTest { + + private static final String CUSTOM_SSL_ON_8080 = + """ + + + /foo/key + /foo/cert + + + """; + + private static final String DEFAULT_SSL_ON_8080 = + """ + + """; + + private static final String ADDITIONAL_CONNECTOR = + """ + + + """; + + @Test + void fails_on_custom_ssl_for_cloud_application() { + var exception = assertThrows(IllegalArgumentException.class, () -> runValidatorOnApp(true, "", CUSTOM_SSL_ON_8080)); + var expected = "Overriding connector specific TLS configuration is not allowed in Vespa Cloud. " + + "See https://cloud.vespa.ai/en/security/guide#data-plane."; + assertEquals(expected, exception.getMessage()); + } + + @Test + void allows_custom_ssl_for_infra() { + assertDoesNotThrow(() -> runValidatorOnApp(true, " application-type='hosted-infrastructure'", CUSTOM_SSL_ON_8080)); + } + + @Test + void allows_custom_ssl_for_self_hosted() { + assertDoesNotThrow(() -> runValidatorOnApp(false, "", CUSTOM_SSL_ON_8080)); + } + + @Test + void fails_on_additional_connectors_for_cloud_application() { + var exception = assertThrows(IllegalArgumentException.class, () -> runValidatorOnApp(true, "", ADDITIONAL_CONNECTOR)); + var expected = "Illegal port 1234 in http server 'custom': Port must be set to 8080"; // Currently fails earlier in model construction + assertEquals(expected, exception.getMessage()); + } + + @Test + void allows_additional_connectors_for_self_hosted() { + assertDoesNotThrow(() -> runValidatorOnApp(false, "", ADDITIONAL_CONNECTOR)); + } + + @Test + void allows_default_ssl_for_cloud_application() { + assertDoesNotThrow(() -> runValidatorOnApp(true, "", DEFAULT_SSL_ON_8080)); + } + + @Test + void allows_default_ssl_for_self_hosted() { + assertDoesNotThrow(() -> runValidatorOnApp(false, "", DEFAULT_SSL_ON_8080)); + } + + private static void runValidatorOnApp(boolean hosted, String appTypeAttribute, String serverXml) throws Exception { + String servicesXml = """ + + + + %s + + + + """.formatted(appTypeAttribute, serverXml); + var state = new DeployState.Builder() + .applicationPackage( + new MockApplicationPackage.Builder() + .withServices(servicesXml) + .build()) + .properties(new TestProperties().setHostedVespa(hosted)) + .build(); + var model = new VespaModel(new NullConfigModelRegistry(), state); + new CloudHttpConnectorValidator().validate(model, state); + } + +} \ No newline at end of file diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java index 8b1217758ab..89cce7feacb 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java @@ -243,11 +243,7 @@ public class JettyContainerModelBuilderTest extends ContainerModelBuilderTestBas Element clusterElem = DomBuilderTest.parse( "", " ", - " ", - " ", - " /foo/key", - " /foo/cert", - " ", + " ", " ", " ", multiNode, @@ -272,8 +268,8 @@ public class JettyContainerModelBuilderTest extends ContainerModelBuilderTestBas .build(); MockRoot root = new MockRoot("root", deployState); createModel(root, deployState, null, clusterElem); - ConnectorConfig sslProvider = root.getConfig(ConnectorConfig.class, "default/http/jdisc-jetty/ssl"); - assertTrue(sslProvider.ssl().enabled()); + ConnectorConfig sslProvider = root.getConfig(ConnectorConfig.class, "default/http/jdisc-jetty/default"); + assertFalse(sslProvider.ssl().enabled()); assertEquals("", sslProvider.ssl().certificate()); assertEquals("", sslProvider.ssl().privateKey()); -- cgit v1.2.3