diff options
author | Bjørn Christian Seime <bjorn.christian@seime.no> | 2020-03-18 14:18:15 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-03-18 14:18:15 +0100 |
commit | e6d74b114fc5f3fa1a35f534f1b8f12b4c541d34 (patch) | |
tree | 3aa97051a9fb5f37ec244894ec54a42c4ee9fbfc /config-model | |
parent | 706ee257707f2e9c8d8916589c5b491984a79916 (diff) |
Revert "Bjorncs/implicit access control"
Diffstat (limited to 'config-model')
6 files changed, 27 insertions, 110 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index e49ceffabc1..2e4cdd706f7 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -8,7 +8,6 @@ import com.yahoo.config.model.api.EndpointCertificateSecrets; import com.yahoo.config.model.api.ModelContext; import com.yahoo.config.model.api.TlsSecrets; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.Zone; @@ -44,7 +43,6 @@ public class TestProperties implements ModelContext.Properties { private Optional<EndpointCertificateSecrets> endpointCertificateSecrets = Optional.empty(); private boolean useNewAthenzFilter = false; private boolean usePhraseSegmenting = false; - private AthenzDomain athenzDomain; @Override public boolean multitenant() { return multitenant; } @Override public ApplicationId applicationId() { return applicationId; } @@ -66,7 +64,6 @@ public class TestProperties implements ModelContext.Properties { @Override public boolean useBucketSpaceMetric() { return true; } @Override public boolean useNewAthenzFilter() { return useNewAthenzFilter; } @Override public boolean usePhraseSegmenting() { return usePhraseSegmenting; } - @Override public Optional<AthenzDomain> athenzDomain() { return Optional.ofNullable(athenzDomain); } public TestProperties setDefaultTermwiseLimit(double limit) { defaultTermwiseLimit = limit; @@ -123,11 +120,6 @@ public class TestProperties implements ModelContext.Properties { return this; } - public TestProperties setAthenzDomain(AthenzDomain domain) { - this.athenzDomain = domain; - return this; - } - public static class Spec implements ConfigServerSpec { private final String hostName; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java index dd48e65c340..37657bea0be 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/AccessControl.java @@ -23,7 +23,6 @@ import java.util.stream.Stream; * Helper class for http access control. * * @author gjoranv - * @author bjorncs */ public final class AccessControl { @@ -67,9 +66,13 @@ public final class AccessControl { return this; } - public Builder setHandlers(ApplicationContainerCluster cluster) { - this.handlers = cluster.getHandlers(); - this.servlets = cluster.getAllServlets(); + public Builder setHandlers(Collection<Handler<?>> handlers) { + this.handlers = handlers; + return this; + } + + public Builder setServlets(Collection<Servlet> servlets) { + this.servlets = servlets; return this; } @@ -108,10 +111,6 @@ public final class AccessControl { .collect(Collectors.toCollection(ArrayList::new)); } - public static boolean hasHandlerThatNeedsProtection(ApplicationContainerCluster cluster) { - return cluster.getHandlers().stream().anyMatch(AccessControl::handlerNeedsProtection); - } - private Stream<Binding> getHandlerBindings() { return handlers.stream() .filter(this::shouldHandlerBeProtected) @@ -131,7 +130,7 @@ public final class AccessControl { && handler.getServerBindings().stream().noneMatch(excludedBindings::contains); } - private static boolean isBuiltinGetOnly(Handler<?> handler) { + public static boolean isBuiltinGetOnly(Handler<?> handler) { return UNPROTECTED_HANDLERS.contains(handler.getClassId().getName()); } @@ -147,6 +146,10 @@ public final class AccessControl { return Stream.of("http://*/").map(protocol -> protocol + servlet.bindingPath); } + public static boolean hasHandlerThatNeedsProtection(ApplicationContainerCluster cluster) { + return cluster.getHandlers().stream().anyMatch(AccessControl::handlerNeedsProtection); + } + private static boolean handlerNeedsProtection(Handler<?> handler) { return ! isBuiltinGetOnly(handler) && hasNonMbusBinding(handler); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java index dc74e2afbb1..9e0b8ad7424 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java @@ -6,7 +6,6 @@ import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.builder.xml.XmlHelper; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; -import com.yahoo.config.provision.AthenzDomain; import com.yahoo.text.XML; import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.model.builder.xml.dom.ModelElement; @@ -23,7 +22,6 @@ import org.w3c.dom.Element; import java.util.ArrayList; import java.util.List; import java.util.Optional; -import java.util.logging.Level; import static com.yahoo.vespa.model.container.http.AccessControl.ACCESS_CONTROL_CHAIN_ID; @@ -63,10 +61,12 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http> } private AccessControl buildAccessControl(DeployState deployState, AbstractConfigProducer ancestor, Element accessControlElem) { - AthenzDomain domain = getAccessControlDomain(deployState, accessControlElem); - AccessControl.Builder builder = new AccessControl.Builder(domain.value(), deployState.getDeployLogger()); + AccessControl.Builder builder = new AccessControl.Builder(accessControlElem.getAttribute("domain"), deployState.getDeployLogger()); - getContainerCluster(ancestor).ifPresent(builder::setHandlers); + getContainerCluster(ancestor).ifPresent(cluster -> { + builder.setHandlers(cluster.getHandlers()); + builder.setServlets(cluster.getAllServlets()); + }); XmlHelper.getOptionalAttribute(accessControlElem, "read").ifPresent( readAttr -> builder.readEnabled(Boolean.valueOf(readAttr))); @@ -82,28 +82,6 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http> return builder.build(); } - // TODO Fail if domain is not provided through deploy properties - private static AthenzDomain getAccessControlDomain(DeployState deployState, Element accessControlElem) { - AthenzDomain tenantDomain = deployState.getProperties().athenzDomain().orElse(null); - AthenzDomain explicitDomain = XmlHelper.getOptionalAttribute(accessControlElem, "domain") - .map(AthenzDomain::from) - .orElse(null); - if (tenantDomain == null) { - if (explicitDomain == null) { - throw new IllegalStateException("No Athenz domain provided for 'access-control'"); - } - deployState.getDeployLogger().log(Level.WARNING, "Athenz tenant is not provided by deploy call. This will soon be handled as failure."); - } - if (explicitDomain != null) { - if (tenantDomain != null && !explicitDomain.equals(tenantDomain)) { - throw new IllegalArgumentException( - String.format("Domain in access-control ('%s') does not match tenant domain ('%s')", tenantDomain, explicitDomain)); - } - deployState.getDeployLogger().log(Level.WARNING, "Domain in 'access-control' is deprecated and will be removed soon"); - } - return tenantDomain != null ? tenantDomain : explicitDomain; - } - private static Optional<ApplicationContainerCluster> getContainerCluster(AbstractConfigProducer configProducer) { AbstractConfigProducer currentProducer = configProducer; while (! ApplicationContainerCluster.class.isAssignableFrom(currentProducer.getClass())) { 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 165404ce9d4..7a620fad10c 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 @@ -19,7 +19,6 @@ import com.yahoo.config.model.builder.xml.ConfigModelBuilder; import com.yahoo.config.model.builder.xml.ConfigModelId; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; -import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterMembership; @@ -58,11 +57,9 @@ import com.yahoo.vespa.model.container.SecretStore; import com.yahoo.vespa.model.container.component.Component; import com.yahoo.vespa.model.container.component.FileStatusHandlerComponent; import com.yahoo.vespa.model.container.component.Handler; -import com.yahoo.vespa.model.container.component.chain.Chain; import com.yahoo.vespa.model.container.component.chain.ProcessingHandler; import com.yahoo.vespa.model.container.docproc.ContainerDocproc; import com.yahoo.vespa.model.container.docproc.DocprocChains; -import com.yahoo.vespa.model.container.http.AccessControl; import com.yahoo.vespa.model.container.http.ConnectorFactory; import com.yahoo.vespa.model.container.http.FilterChains; import com.yahoo.vespa.model.container.http.Http; @@ -91,7 +88,6 @@ import java.util.function.Consumer; import java.util.regex.Pattern; import java.util.stream.Collectors; -import static com.yahoo.vespa.model.container.http.AccessControl.ACCESS_CONTROL_CHAIN_ID; import static java.util.logging.Level.WARNING; /** @@ -196,7 +192,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { addStatusHandlers(cluster, context.getDeployState().isHosted()); addUserHandlers(deployState, cluster, spec); - addHttp(deployState, spec, cluster, context); + addHttp(deployState, spec, cluster, context.getApplicationType(), deployState.getProperties().applicationId().instance().isTester()); addAccessLogs(deployState, cluster, spec); addRoutingAliases(cluster, spec, deployState.zone().environment()); @@ -318,18 +314,18 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } - private void addHttp(DeployState deployState, Element spec, ApplicationContainerCluster cluster, ConfigModelContext context) { + private void addHttp(DeployState deployState, Element spec, ApplicationContainerCluster cluster, ApplicationType applicationType, boolean isTesterApplication) { Element httpElement = XML.getChild(spec, "http"); if (httpElement != null) { cluster.setHttp(buildHttp(deployState, cluster, httpElement)); } - if (isHostedTenantApplication(context)) { - addHostedImplicitHttpIfNotPresent(deployState, cluster); + if (deployState.isHosted() && applicationType == ApplicationType.DEFAULT && !isTesterApplication) { addAdditionalHostedConnector(deployState, cluster); } } private void addAdditionalHostedConnector(DeployState deployState, ApplicationContainerCluster cluster) { + addImplicitHttpIfNotPresent(cluster); JettyHttpServer server = cluster.getHttp().getHttpServer(); String serverName = server.getComponentId().getName(); @@ -350,17 +346,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } } - private static boolean isHostedTenantApplication(ConfigModelContext context) { - var deployState = context.getDeployState(); - boolean isTesterApplication = deployState.getProperties().applicationId().instance().isTester(); - return deployState.isHosted() && context.getApplicationType() == ApplicationType.DEFAULT && !isTesterApplication; - } - - private static void addHostedImplicitHttpIfNotPresent(DeployState deployState, ApplicationContainerCluster cluster) { + private static void addImplicitHttpIfNotPresent(ApplicationContainerCluster cluster) { if(cluster.getHttp() == null) { - Http http = deployState.getProperties().athenzDomain() - .map(tenantDomain -> createHostedImplicitHttpWithAccessControl(deployState, tenantDomain, cluster)) - .orElseGet(() -> createHostedImplicitHttpWithoutAccessControl(cluster)); + Http http = new Http(Collections.emptyList()); + http.setFilterChains(new FilterChains(cluster)); cluster.setHttp(http); } if(cluster.getHttp().getHttpServer() == null) { @@ -370,27 +359,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { } } - private static Http createHostedImplicitHttpWithAccessControl( - DeployState deployState, AthenzDomain tenantDomain, ApplicationContainerCluster cluster) { - AccessControl accessControl = - new AccessControl.Builder(tenantDomain.value(), deployState.getDeployLogger()) - .setHandlers(cluster) - .readEnabled(false) - .writeEnabled(false) - .build(); - Http http = new Http(accessControl.getBindings(), accessControl); - FilterChains filterChains = new FilterChains(cluster); - filterChains.add(new Chain<>(FilterChains.emptyChainSpec(ACCESS_CONTROL_CHAIN_ID))); - http.setFilterChains(filterChains); - return http; - } - - private static Http createHostedImplicitHttpWithoutAccessControl(ApplicationContainerCluster cluster) { - Http http = new Http(Collections.emptyList()); - http.setFilterChains(new FilterChains(cluster)); - return http; - } - private Http buildHttp(DeployState deployState, ApplicationContainerCluster cluster, Element httpElement) { Http http = new HttpBuilder().build(deployState, cluster, httpElement); diff --git a/config-model/src/main/resources/schema/container.rnc b/config-model/src/main/resources/schema/container.rnc index 034de1113c2..b9cb5e1fae6 100644 --- a/config-model/src/main/resources/schema/container.rnc +++ b/config-model/src/main/resources/schema/container.rnc @@ -23,9 +23,9 @@ Server = element server { } AccessControl = element access-control { - attribute domain { xsd:NCName }? & # TODO Vespa 8 Remove - attribute read { string "true" | string "false" }? & # TODO Vespa 8 Remove - attribute write { string "true" | string "false" }? & # TODO Vespa 8 Remove + attribute domain { xsd:NCName } & + attribute read { string "true" | string "false" }? & + attribute write { string "true" | string "false" }? & element vespa-domain { xsd:NCName }? & # TODO Remove after end of March 2020 element exclude { Binding+ diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index 73db6e35428..17dff46bed4 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -14,7 +14,6 @@ import com.yahoo.config.model.provision.InMemoryProvisioner; import com.yahoo.config.model.provision.SingleNodeProvisioner; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.model.test.MockRoot; -import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.RegionName; @@ -44,7 +43,6 @@ import com.yahoo.vespa.model.container.ApplicationContainer; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.SecretStore; import com.yahoo.vespa.model.container.component.Component; -import com.yahoo.vespa.model.container.http.AccessControl; import com.yahoo.vespa.model.container.http.ConnectorFactory; import com.yahoo.vespa.model.content.utils.ContentClusterUtils; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithFilePkg; @@ -835,28 +833,6 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { assertThat(connectorConfig.ssl().caCertificate(), isEmptyString()); } - @Test - public void access_control_is_implicitly_added_for_hosted_apps() { - Element clusterElem = DomBuilderTest.parse( - "<container version='1.0'>", - nodesXml, - "</container>" ); - AthenzDomain tenantDomain = AthenzDomain.from("my-tenant-domain"); - DeployState state = new DeployState.Builder().properties( - new TestProperties() - .setAthenzDomain(tenantDomain) - .setHostedVespa(true)) - .build(); - createModel(root, state, null, clusterElem); - Optional<AccessControl> maybeAccessControl = - ((ApplicationContainer) root.getProducer("container/container.0")).getHttp().getAccessControl(); - assertThat(maybeAccessControl.isPresent(), is(true)); - AccessControl accessControl = maybeAccessControl.get(); - assertThat(accessControl.writeEnabled, is(false)); - assertThat(accessControl.readEnabled, is(false)); - assertThat(accessControl.domain, equalTo(tenantDomain.value())); - } - private Element generateContainerElementWithRenderer(String rendererId) { return DomBuilderTest.parse( |