diff options
101 files changed, 2707 insertions, 741 deletions
diff --git a/application/pom.xml b/application/pom.xml index 2f0586145b2..8f9dc7999a0 100644 --- a/application/pom.xml +++ b/application/pom.xml @@ -139,8 +139,8 @@ <!-- START JETTY embedded jars --> <dependency> - <groupId>org.eclipse.jetty.http2</groupId> - <artifactId>http2-common</artifactId> + <groupId>org.eclipse.jetty.alpn</groupId> + <artifactId>alpn-api</artifactId> </dependency> <dependency> <groupId>org.eclipse.jetty.http2</groupId> @@ -152,43 +152,35 @@ </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-alpn-server</artifactId> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-client</artifactId> + <scope>test</scope> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-http</artifactId> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-io</artifactId> + <artifactId>jetty-continuation</artifactId> + <scope>test</scope> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-jmx</artifactId> + <scope>test</scope> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-server</artifactId> + <scope>test</scope> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-servlet</artifactId> + <scope>test</scope> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-util</artifactId> + <artifactId>jetty-servlets</artifactId> + <scope>test</scope> </dependency> <dependency> - <groupId>org.eclipse.jetty.toolchain</groupId> - <artifactId>jetty-jakarta-servlet-api</artifactId> - </dependency> - <!-- END JETTY embedded jars --> - - <dependency> <groupId>org.junit.jupiter</groupId> <artifactId>junit-jupiter-api</artifactId> <scope>test</scope> @@ -198,6 +190,8 @@ <artifactId>junit-jupiter-engine</artifactId> <scope>test</scope> </dependency> + <!-- END JETTY embedded jars --> + </dependencies> <build> diff --git a/client/go/script-utils/startcbinary/execvp_windows.go b/client/go/script-utils/startcbinary/execvp_windows.go deleted file mode 100644 index f0642dcf9a0..00000000000 --- a/client/go/script-utils/startcbinary/execvp_windows.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -// Author: arnej - -//go:build windows - -package startcbinary - -import ( - "fmt" -) - -func myexecvp(prog string, args []string, envv []string) error { - return fmt.Errorf("cannot execvp: %s", prog) -} diff --git a/client/go/script-utils/startcbinary/startcbinary.go b/client/go/script-utils/startcbinary/startcbinary.go index 7a2b13ebf87..ca1ab181f7e 100644 --- a/client/go/script-utils/startcbinary/startcbinary.go +++ b/client/go/script-utils/startcbinary/startcbinary.go @@ -6,6 +6,8 @@ package startcbinary import ( "fmt" "os" + + "github.com/vespa-engine/vespa/client/go/util" ) func startCbinary(spec *ProgSpec) int { @@ -40,5 +42,5 @@ func (spec *ProgSpec) run() error { spec.setenv(ENV_LD_PRELOAD, spec.vespaMallocPreload) } envv := spec.effectiveEnv() - return myexecvp(prog, args, envv) + return util.Execvpe(prog, args, envv) } diff --git a/client/go/script-utils/startcbinary/execvp.go b/client/go/util/execvp.go index 6ca34c4076b..2b47af4de1b 100644 --- a/client/go/script-utils/startcbinary/execvp.go +++ b/client/go/util/execvp.go @@ -3,7 +3,7 @@ //go:build !windows -package startcbinary +package util import ( "fmt" @@ -11,7 +11,6 @@ import ( "strings" "github.com/vespa-engine/vespa/client/go/trace" - "github.com/vespa-engine/vespa/client/go/util" "golang.org/x/sys/unix" ) @@ -19,19 +18,27 @@ func findInPath(prog string) string { if strings.Contains(prog, "/") { return prog } - path := strings.Split(os.Getenv(ENV_PATH), ":") + path := strings.Split(os.Getenv("PATH"), ":") for _, dir := range path { fn := dir + "/" + prog - if util.IsRegularFile(fn) { + if IsExecutableFile(fn) { return fn } } return prog } -func myexecvp(prog string, argv []string, envv []string) error { +func Execvp(prog string, argv []string) error { + return Execvpe(prog, argv, os.Environ()) +} + +func Execvpe(prog string, argv []string, envv []string) error { prog = findInPath(prog) argv[0] = prog + return Execve(prog, argv, envv) +} + +func Execve(prog string, argv []string, envv []string) error { trace.Trace("run cmd:", strings.Join(argv, " ")) err := unix.Exec(prog, argv, envv) return fmt.Errorf("cannot execute '%s': %v", prog, err) diff --git a/client/go/util/execvp_windows.go b/client/go/util/execvp_windows.go new file mode 100644 index 00000000000..24b67e221c8 --- /dev/null +++ b/client/go/util/execvp_windows.go @@ -0,0 +1,22 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Author: arnej + +//go:build windows + +package util + +import ( + "fmt" +) + +func Execvp(prog string, argv []string) error { + return fmt.Errorf("cannot execvp on windows: %s", prog) +} + +func Execvpe(prog string, argv []string, envv []string) error { + return fmt.Errorf("cannot execvp on windows: %s", prog) +} + +func Execve(prog string, argv []string, envv []string) error { + return fmt.Errorf("cannot execvp on windows: %s", prog) +} diff --git a/client/go/util/io.go b/client/go/util/io.go index 6aab64b8827..50122324fae 100644 --- a/client/go/util/io.go +++ b/client/go/util/io.go @@ -32,6 +32,15 @@ func IsRegularFile(path string) bool { return !errors.Is(err, os.ErrNotExist) && info != nil && info.Mode().IsRegular() } +// Returns true if the given path points to an executable +func IsExecutableFile(path string) bool { + info, err := os.Stat(path) + return !errors.Is(err, os.ErrNotExist) && + info != nil && + info.Mode().IsRegular() && + ((info.Mode() & 0111) == 0111) +} + // Returns the content of a reader as a string func ReaderToString(reader io.Reader) string { var buffer strings.Builder diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index 8ba9794d75c..03f14d7d65f 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -43,7 +43,8 @@ <javax.servlet-api.version>3.1.0</javax.servlet-api.version> <javax.ws.rs-api.version>2.0.1</javax.ws.rs-api.version> <jaxb.version>2.3.0</jaxb.version> - <jetty.version>11.0.12</jetty.version> + <jetty.version>9.4.49.v20220914</jetty.version> + <jetty-alpn.version>1.1.3.v20160715</jetty-alpn.version> <org.lz4.version>1.8.0</org.lz4.version> <org.json.version>20220320</org.json.version> <!-- TODO: Remove on Vespa 9 --> <slf4j.version>1.7.32</slf4j.version> <!-- WARNING: when updated, also update c.y.v.tenant:base pom --> @@ -199,20 +200,22 @@ <include>org.bouncycastle:bcpkix-jdk18on:[${bouncycastle.version}]:jar:test</include> <include>org.bouncycastle:bcprov-jdk18on:[${bouncycastle.version}]:jar:test</include> <include>org.bouncycastle:bcutil-jdk18on:[${bouncycastle.version}]:jar:test</include> + <include>org.eclipse.jetty.alpn:alpn-api:[${jetty-alpn.version}]:jar:test</include> <include>org.eclipse.jetty.http2:http2-common:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty.http2:http2-hpack:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty.http2:http2-server:[${jetty.version}]:jar:test</include> - <include>org.eclipse.jetty.toolchain:jetty-jakarta-servlet-api:5.0.2:jar:test</include> - <include>org.eclipse.jetty:jetty-alpn-client:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-alpn-java-server:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-alpn-server:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-client:[${jetty.version}]:jar:test</include> + <include>org.eclipse.jetty:jetty-continuation:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-http:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-io:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-jmx:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-security:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-server:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-servlet:[${jetty.version}]:jar:test</include> + <include>org.eclipse.jetty:jetty-servlets:[${jetty.version}]:jar:test</include> + <include>org.eclipse.jetty:jetty-util-ajax:[${jetty.version}]:jar:test</include> <include>org.eclipse.jetty:jetty-util:[${jetty.version}]:jar:test</include> <include>org.hamcrest:hamcrest-core:1.3:jar:test</include> <include>org.hdrhistogram:HdrHistogram:2.1.8:jar:test</include> diff --git a/container-core/pom.xml b/container-core/pom.xml index 52d7f3372f0..ed4b05495e3 100644 --- a/container-core/pom.xml +++ b/container-core/pom.xml @@ -14,7 +14,6 @@ <artifactId>container-core</artifactId> <version>8-SNAPSHOT</version> <packaging>container-plugin</packaging> - <dependencies> <!-- COMPILE scope --> @@ -115,119 +114,40 @@ <!-- START JETTY embedded jars --> <dependency> - <groupId>org.eclipse.jetty.http2</groupId> - <artifactId>http2-common</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> + <groupId>org.eclipse.jetty.alpn</groupId> + <artifactId>alpn-api</artifactId> </dependency> <dependency> <groupId>org.eclipse.jetty.http2</groupId> <artifactId>http2-server</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> </dependency> <dependency> - <!-- Required for JDK9ServerALPNProcessor through ServiceLoader API --> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-alpn-java-server</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-alpn-server</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-client</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-http</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-io</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> + <artifactId>jetty-continuation</artifactId> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-jmx</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-server</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-servlet</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-util</artifactId> - <exclusions> - <exclusion> - <groupId>org.slf4j</groupId> - <artifactId>slf4j-api</artifactId> - </exclusion> - </exclusions> - </dependency> - <dependency> - <groupId>org.eclipse.jetty.toolchain</groupId> - <artifactId>jetty-jakarta-servlet-api</artifactId> + <artifactId>jetty-servlets</artifactId> </dependency> <!-- END JETTY embedded jars --> @@ -336,6 +256,11 @@ <scope>provided</scope> </dependency> <dependency> + <groupId>javax.servlet</groupId> + <artifactId>javax.servlet-api</artifactId> + <scope>provided</scope> + </dependency> + <dependency> <groupId>javax.xml.bind</groupId> <artifactId>jaxb-api</artifactId> <scope>provided</scope> diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/utils/MultiPartFormParser.java b/container-core/src/main/java/com/yahoo/container/jdisc/utils/MultiPartFormParser.java index f974eb5f26c..104d2f8ae4a 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/utils/MultiPartFormParser.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/utils/MultiPartFormParser.java @@ -2,9 +2,9 @@ package com.yahoo.container.jdisc.utils; import com.yahoo.container.jdisc.HttpRequest; -import jakarta.servlet.http.Part; -import org.eclipse.jetty.server.MultiPartFormInputStream; +import org.eclipse.jetty.http.MultiPartFormInputStream; +import javax.servlet.http.Part; import java.io.IOException; import java.io.InputStream; import java.util.Map; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/Cookie.java b/container-core/src/main/java/com/yahoo/jdisc/http/Cookie.java index c2faa1cd10a..b194124294c 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/Cookie.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/Cookie.java @@ -2,7 +2,7 @@ package com.yahoo.jdisc.http; import org.eclipse.jetty.http.HttpCookie; -import org.eclipse.jetty.server.Cookies; +import org.eclipse.jetty.server.CookieCutter; import java.util.Arrays; import java.util.HashSet; @@ -180,7 +180,7 @@ public class Cookie { } public static List<Cookie> fromCookieHeader(String headerVal) { - Cookies cookieCutter = new Cookies(); + CookieCutter cookieCutter = new CookieCutter(); cookieCutter.addCookieField(headerVal); return Arrays.stream(cookieCutter.getCookies()) .map(servletCookie -> { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java b/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java index 4ad38a9f965..598a924b327 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/HttpRequest.java @@ -8,14 +8,15 @@ import com.yahoo.jdisc.handler.ContentChannel; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.service.CurrentContainer; +import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.util.MultiMap; -import org.eclipse.jetty.util.UrlEncoded; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.net.URI; import java.security.Principal; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.LinkedList; @@ -115,10 +116,15 @@ public class HttpRequest extends Request { } private static Map<String, List<String>> getUriQueryParameters(URI uri) { - if (uri.getRawQuery() == null) return Map.of(); - MultiMap<String> params = new MultiMap<>(); - UrlEncoded.decodeUtf8To(uri.getRawQuery(), params); - return Map.copyOf(params); + MultiMap<String> queryParameters = new MultiMap<>(); + new HttpURI(uri).decodeQueryTo(queryParameters); + + // Do a deep copy so we do not leak Jetty classes outside + Map<String, List<String>> deepCopiedQueryParameters = new HashMap<>(); + for (Map.Entry<String, List<String>> entry : queryParameters.entrySet()) { + deepCopiedQueryParameters.put(entry.getKey(), new ArrayList<>(entry.getValue())); + } + return deepCopiedQueryParameters; } public Method getMethod() { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java index 5b51eeee7d6..13a63efeaa9 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java @@ -8,7 +8,6 @@ import com.yahoo.container.logging.RequestLog; import com.yahoo.container.logging.RequestLogEntry; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.ServerConfig; -import jakarta.servlet.http.HttpServletRequest; import org.eclipse.jetty.http2.HTTP2Stream; import org.eclipse.jetty.http2.server.HttpTransportOverHTTP2; import org.eclipse.jetty.server.HttpChannel; @@ -17,6 +16,7 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import javax.servlet.http.HttpServletRequest; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java index 4e984d57808..6282e334409 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java @@ -9,8 +9,6 @@ import com.yahoo.jdisc.http.ssl.impl.DefaultConnectorSsl; import com.yahoo.security.tls.MixedMode; import com.yahoo.security.tls.TransportSecurityUtils; import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; -import org.eclipse.jetty.http.HttpCompliance; -import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.http2.server.AbstractHTTP2ServerConnectionFactory; import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory; import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; @@ -139,12 +137,8 @@ public class ConnectorFactory { httpConfig.setOutputBufferSize(connectorConfig.outputBufferSize()); httpConfig.setRequestHeaderSize(connectorConfig.requestHeaderSize()); httpConfig.setResponseHeaderSize(connectorConfig.responseHeaderSize()); - httpConfig.setHttpCompliance(HttpCompliance.RFC7230); - // TODO Vespa 9 Use default URI compliance (LEGACY == old Jetty 9.4 compliance) - httpConfig.setUriCompliance(UriCompliance.LEGACY); if (isSslEffectivelyEnabled(connectorConfig)) { - // Explicitly disable SNI checking as Jetty's SNI checking trust manager is not part of our SSLContext trust manager chain - httpConfig.addCustomizer(new SecureRequestCustomizer(false, false, -1, false)); + httpConfig.addCustomizer(new SecureRequestCustomizer()); } String serverNameFallback = connectorConfig.serverName().fallback(); if (!serverNameFallback.isBlank()) httpConfig.setServerAuthority(new HostPort(serverNameFallback)); @@ -180,7 +174,7 @@ public class ConnectorFactory { return connectionFactory; } - private SslContextFactory.Server createSslContextFactory() { + private SslContextFactory createSslContextFactory() { DefaultConnectorSsl ssl = new DefaultConnectorSsl(); sslProvider.configureSsl(ssl, connectorConfig.name(), connectorConfig.listenPort()); return ssl.createSslContextFactory(); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java index 342d7ab9c4a..ac50cbbb518 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java @@ -4,24 +4,14 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.concurrent.DaemonThreadFactory; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.security.SslContextBuilder; -import com.yahoo.security.TrustAllX509TrustManager; import com.yahoo.security.tls.TransportSecurityOptions; import com.yahoo.security.tls.TransportSecurityUtils; -import jakarta.servlet.AsyncContext; -import jakarta.servlet.AsyncEvent; -import jakarta.servlet.AsyncListener; -import jakarta.servlet.ServletException; -import jakarta.servlet.ServletOutputStream; -import jakarta.servlet.WriteListener; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; +import com.yahoo.security.TrustAllX509TrustManager; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory; import org.eclipse.jetty.client.api.ContentResponse; -import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.server.DetectorConnectionFactory; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.SslConnectionFactory; @@ -29,6 +19,14 @@ import org.eclipse.jetty.server.handler.HandlerWrapper; import org.eclipse.jetty.util.ssl.SslContextFactory; import javax.net.ssl.SSLContext; +import javax.servlet.AsyncContext; +import javax.servlet.AsyncEvent; +import javax.servlet.AsyncListener; +import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.time.Duration; import java.util.HashMap; @@ -91,7 +89,7 @@ class HealthCheckProxyHandler extends HandlerWrapper { Optional.ofNullable(targetConnector.getConnectionFactory(SslConnectionFactory.class)) .or(() -> Optional.ofNullable(targetConnector.getConnectionFactory(DetectorConnectionFactory.class)) .map(detectorConnFactory -> detectorConnFactory.getBean(SslConnectionFactory.class))) - .map(SslConnectionFactory::getSslContextFactory) + .map(connFactory -> (SslContextFactory.Server) connFactory.getSslContextFactory()) .orElseThrow(() -> new IllegalArgumentException("Health check proxy can only target https port")); boolean proxyProtocol = targetConnector.connectorConfig().proxyProtocol().enabled(); return new ProxyTarget(targetPort, clientTimeout,handlerTimeout, cacheExpiry, sslContextFactory, proxyProtocol); @@ -271,14 +269,13 @@ class HealthCheckProxyHandler extends HandlerWrapper { synchronized (this) { if (client == null) { int timeoutMillis = (int) clientTimeout.toMillis(); - var clientSsl = new SslContextFactory.Client(); + SslContextFactory.Client clientSsl = new SslContextFactory.Client(); clientSsl.setHostnameVerifier((__, ___) -> true); clientSsl.setSslContext(getSslContext(serverSsl)); - var connector = new ClientConnector(); - connector.setSslContextFactory(clientSsl); - HttpClient client = new HttpClient(new HttpClientTransportOverHTTP(connector)); + HttpClient client = new HttpClient(clientSsl); client.setMaxConnectionsPerDestination(4); client.setConnectTimeout(timeoutMillis); + client.setStopTimeout(timeoutMillis); client.setIdleTimeout(timeoutMillis); client.setUserAgentField(new HttpField(HttpHeader.USER_AGENT, "health-check-proxy-client")); client.start(); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index b4c933c1168..9292e2024df 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -12,11 +12,6 @@ import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.HttpHeaders; import com.yahoo.jdisc.http.HttpRequest; -import jakarta.servlet.AsyncContext; -import jakarta.servlet.AsyncEvent; -import jakarta.servlet.AsyncListener; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.server.HTTP2ServerConnection; import org.eclipse.jetty.io.Connection; @@ -25,6 +20,11 @@ import org.eclipse.jetty.server.HttpConnection; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.Callback; +import javax.servlet.AsyncContext; +import javax.servlet.AsyncEvent; +import javax.servlet.AsyncListener; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.time.Instant; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java index d45a8789e4c..8a298fb3268 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java @@ -3,10 +3,10 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.service.CurrentContainer; -import jakarta.servlet.http.HttpServletRequest; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.Utf8Appendable; +import javax.servlet.http.HttpServletRequest; import java.net.InetSocketAddress; import java.net.URI; import java.security.cert.X509Certificate; @@ -94,6 +94,6 @@ class HttpRequestFactory { } private static X509Certificate[] getCertChain(HttpServletRequest servletRequest) { - return (X509Certificate[]) servletRequest.getAttribute(RequestUtils.SERVLET_REQUEST_X509CERT); + return (X509Certificate[]) servletRequest.getAttribute("javax.servlet.request.X509Certificate"); } } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java index 81789881b68..3fb81cb5352 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java @@ -4,11 +4,6 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.ServerConfig; -import jakarta.servlet.AsyncEvent; -import jakarta.servlet.AsyncListener; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.AsyncContextEvent; @@ -16,8 +11,14 @@ import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.HttpChannelState; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.HandlerWrapper; +import org.eclipse.jetty.util.FutureCallback; import org.eclipse.jetty.util.component.Graceful; +import javax.servlet.AsyncEvent; +import javax.servlet.AsyncListener; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; @@ -26,10 +27,12 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.Future; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.LongAdder; import java.util.function.ObjLongConsumer; import java.util.stream.Collectors; @@ -46,7 +49,7 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful static final String requestTypeAttribute = "requestType"; - private final Shutdown shutdown; + private final AtomicReference<FutureCallback> shutdown = new AtomicReference<>(); private final List<String> monitoringHandlerPaths; private final List<String> searchHandlerPaths; private final Set<String> ignoredUserAgents; @@ -63,10 +66,6 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful this.monitoringHandlerPaths = monitoringHandlerPaths; this.searchHandlerPaths = searchHandlerPaths; this.ignoredUserAgents = Set.copyOf(ignoredUserAgents); - this.shutdown = new Shutdown(this) { - @Override public boolean isShutdownDone() { return inFlight.get() == 0; } - }; - } private final AsyncListener completionWatcher = new AsyncListener() { @@ -98,7 +97,7 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful try { Handler handler = getHandler(); - if (handler != null && !shutdown.isShutdown() && isStarted()) { + if (handler != null && shutdown.get() == null && isStarted()) { handler.handle(path, baseRequest, request, response); } else if ( ! baseRequest.isHandled()) { baseRequest.setHandled(true); @@ -130,9 +129,14 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful .increment()); } long live = inFlight.decrementAndGet(); - if (shutdown.isShutdown()) { - if (flushableResponse != null) flushableResponse.flushBuffer(); - if (live == 0) shutdown.check(); + FutureCallback shutdownCb = shutdown.get(); + if (shutdownCb != null) { + if (flushableResponse != null) { + flushableResponse.flushBuffer(); + } + if (live == 0) { + shutdownCb.succeeded(); + } } } @@ -158,19 +162,35 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful @Override protected void doStart() throws Exception { - shutdown.cancel(); + shutdown.set(null); super.doStart(); } @Override protected void doStop() throws Exception { - shutdown.cancel(); super.doStop(); + FutureCallback shutdownCb = shutdown.get(); + if ( ! shutdownCb.isDone()) { + shutdownCb.failed(new TimeoutException()); + } } - @Override public CompletableFuture<Void> shutdown() { return shutdown.shutdown(); } - @Override public boolean isShutdown() { return shutdown.isShutdown(); } + @Override + public Future<Void> shutdown() { + FutureCallback shutdownCb = new FutureCallback(false); + shutdown.compareAndSet(null, shutdownCb); + shutdownCb = shutdown.get(); + if (inFlight.get() == 0) { + shutdownCb.succeeded(); + } + return shutdownCb; + } + @Override + public boolean isShutdown() { + FutureCallback futureCallback = shutdown.get(); + return futureCallback != null && futureCallback.isDone(); + } static class Dimensions { final String protocol; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java index bd052f14867..4b4aff0a9bd 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java @@ -5,13 +5,13 @@ import com.yahoo.container.logging.AccessLogEntry; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.handler.OverloadException; import com.yahoo.jdisc.http.HttpRequest.Method; -import jakarta.servlet.ServletException; -import jakarta.servlet.annotation.WebServlet; -import jakarta.servlet.http.HttpServlet; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.server.Request; +import javax.servlet.ServletException; +import javax.servlet.annotation.WebServlet; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.Enumeration; import java.util.Map; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java index b17877cee84..b3069a64821 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java @@ -3,13 +3,16 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.ConnectorConfig; -import jakarta.servlet.ServletRequest; -import jakarta.servlet.http.HttpServletRequest; +import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.io.ConnectionStatistics; import org.eclipse.jetty.server.ConnectionFactory; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import javax.servlet.ServletRequest; +import javax.servlet.http.HttpServletRequest; +import java.net.Socket; +import java.net.SocketException; import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -23,6 +26,8 @@ class JDiscServerConnector extends ServerConnector { private final Metric.Context metricCtx; private final ConnectionStatistics statistics; private final ConnectorConfig config; + private final boolean tcpKeepAlive; + private final boolean tcpNoDelay; private final Metric metric; private final String connectorName; private final int listenPort; @@ -31,13 +36,14 @@ class JDiscServerConnector extends ServerConnector { ConnectionMetricAggregator connectionMetricAggregator, ConnectionFactory... factories) { super(server, factories); this.config = config; + this.tcpKeepAlive = config.tcpKeepAliveEnabled(); + this.tcpNoDelay = config.tcpNoDelay(); this.metric = metric; this.connectorName = config.name(); this.listenPort = config.listenPort(); this.metricCtx = metric.createContext(createConnectorDimensions(listenPort, connectorName, 0)); this.statistics = new ConnectionStatistics(); - setAcceptedTcpNoDelay(config.tcpNoDelay()); addBean(statistics); ConnectorConfig.Throttling throttlingConfig = config.throttling(); if (throttlingConfig.enabled()) { @@ -50,6 +56,17 @@ class JDiscServerConnector extends ServerConnector { setAcceptQueueSize(config.acceptQueueSize()); setReuseAddress(config.reuseAddress()); setIdleTimeout((long) (config.idleTimeout() * 1000)); + addBean(HttpCompliance.RFC7230); + } + + @Override + protected void configure(final Socket socket) { + super.configure(socket); + try { + socket.setKeepAlive(tcpKeepAlive); + socket.setTcpNoDelay(tcpNoDelay); + } catch (SocketException ignored) { + } } public ConnectionStatistics getStatistics() { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyConnectionLogger.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyConnectionLogger.java index d9a97d621ae..2e2eb257b6a 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyConnectionLogger.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyConnectionLogger.java @@ -30,7 +30,6 @@ import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.net.ssl.StandardConstants; import java.net.InetSocketAddress; -import java.net.SocketAddress; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateEncodingException; @@ -114,7 +113,7 @@ class JettyConnectionLogger extends AbstractLifeCycle implements Connection.List info.setProxyProtocolVersion("v2"); } if (connection.getEndPoint() instanceof ProxyConnectionFactory.ProxyEndPoint) { - var remoteAddress = connection.getEndPoint().getRemoteSocketAddress(); + InetSocketAddress remoteAddress = connection.getEndPoint().getRemoteAddress(); info.setRemoteAddress(remoteAddress); } }); @@ -244,7 +243,7 @@ class JettyConnectionLogger extends AbstractLifeCycle implements Connection.List private long httpBytesSent = 0; private long requests = 0; private long responses = 0; - private SocketAddress remoteAddress; + private InetSocketAddress remoteAddress; private byte[] sslSessionId; private String sslProtocol; private String sslCipherSuite; @@ -291,7 +290,7 @@ class JettyConnectionLogger extends AbstractLifeCycle implements Connection.List synchronized ConnectionInfo incrementResponses() { ++this.responses; return this; } - synchronized ConnectionInfo setRemoteAddress(SocketAddress remoteAddress) { + synchronized ConnectionInfo setRemoteAddress(InetSocketAddress remoteAddress) { this.remoteAddress = remoteAddress; return this; } @@ -355,9 +354,9 @@ class JettyConnectionLogger extends AbstractLifeCycle implements Connection.List builder.withLocalAddress(localAddress.getHostString()) .withLocalPort(localAddress.getPort()); } - if (remoteAddress instanceof InetSocketAddress isa) { - builder.withRemoteAddress(isa.getHostString()) - .withRemotePort(isa.getPort()); + if (remoteAddress != null) { + builder.withRemoteAddress(remoteAddress.getHostString()) + .withRemotePort(remoteAddress.getPort()); } if (sslProtocol != null && sslCipherSuite != null && sslSessionId != null) { builder.withSslProtocol(sslProtocol) diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index 7b723b3a48e..775c903f5f8 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -27,6 +27,8 @@ import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.eclipse.jetty.server.handler.gzip.GzipHttpOutputInterceptor; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.log.JavaUtilLog; +import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.thread.QueuedThreadPool; import javax.management.remote.JMXServiceURL; @@ -68,6 +70,8 @@ public class JettyHttpServer extends AbstractServerProvider { if (connectorFactories.allComponents().isEmpty()) throw new IllegalArgumentException("No connectors configured."); + initializeJettyLogging(); + server = new Server(); server.setStopTimeout((long)(serverConfig.stopTimeout() * 1000.0)); server.setRequestLog(new AccessLogRequestLog(requestLog, serverConfig.accessLog())); @@ -92,6 +96,15 @@ public class JettyHttpServer extends AbstractServerProvider { this.metricsReporter = new ServerMetricReporter(metric, server); } + private static void initializeJettyLogging() { + // Note: Jetty is logging stderr if no logger is explicitly configured + try { + Log.setLog(new JavaUtilLog()); + } catch (Exception e) { + throw new RuntimeException("Unable to initialize logging framework for Jetty"); + } + } + private static void setupJmx(Server server, ServerConfig serverConfig) { if (serverConfig.jmx().enabled()) { System.setProperty("java.rmi.server.hostname", "localhost"); @@ -139,7 +152,7 @@ public class JettyHttpServer extends AbstractServerProvider { } StatisticsHandler root = newGenericStatisticsHandler(); addChainToRoot(root, List.of( - newResponseStatisticsHandler(serverCfg), newGzipHandler(), perConnectorHandlers)); + newResponseStatisticsHandler(serverCfg), newGzipHandler(serverCfg), perConnectorHandlers)); return root; } @@ -240,18 +253,22 @@ public class JettyHttpServer extends AbstractServerProvider { return statisticsHandler; } - private static GzipHandler newGzipHandler() { return new GzipHandlerWithVaryHeaderFixed(); } + private static GzipHandler newGzipHandler(ServerConfig serverConfig) { + GzipHandler gzipHandler = new GzipHandlerWithVaryHeaderFixed(); + gzipHandler.setCompressionLevel(serverConfig.responseCompressionLevel()); + gzipHandler.setInflateBufferSize(8 * 1024); + gzipHandler.setIncludedMethods("GET", "POST", "PUT", "PATCH"); + return gzipHandler; + } /** A subclass which overrides Jetty's default behavior of including user-agent in the vary field */ private static class GzipHandlerWithVaryHeaderFixed extends GzipHandler { - GzipHandlerWithVaryHeaderFixed() { - setInflateBufferSize(8 * 1024); - setIncludedMethods("GET", "POST", "PUT", "PATCH"); + @Override + public HttpField getVaryField() { + return GzipHttpOutputInterceptor.VARY_ACCEPT_ENCODING; } - @Override public HttpField getVaryField() { return GzipHttpOutputInterceptor.VARY_ACCEPT_ENCODING; } - } } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java index da4de957739..1bc862bc787 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java @@ -1,12 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.server.jetty; -import jakarta.servlet.http.HttpServletRequest; import org.eclipse.jetty.http2.server.HTTP2ServerConnection; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.server.HttpConnection; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.SecureRequestCustomizer; + +import javax.servlet.http.HttpServletRequest; /** * @author bjorncs @@ -15,7 +15,7 @@ public class RequestUtils { public static final String JDISC_REQUEST_X509CERT = "jdisc.request.X509Certificate"; public static final String JDISC_REQUEST_CHAIN = "jdisc.request.chain"; public static final String JDISC_RESPONSE_CHAIN = "jdisc.response.chain"; - public static final String SERVLET_REQUEST_X509CERT = SecureRequestCustomizer.JAKARTA_SERVLET_REQUEST_X_509_CERTIFICATE; + public static final String SERVLET_REQUEST_X509CERT = "javax.servlet.request.X509Certificate"; // The local port as reported by servlet spec. This will be influenced by Host header and similar mechanisms. // The request URI uses the local listen port as the URI is used for handler routing/bindings. diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java index d853282a5f5..4b66715fcf7 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java @@ -2,9 +2,9 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.handler.CompletionHandler; -import jakarta.servlet.ServletOutputStream; -import jakarta.servlet.WriteListener; +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayDeque; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java index 2f2c48e0b48..3703878f595 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java @@ -3,10 +3,10 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.handler.CompletionHandler; import com.yahoo.jdisc.handler.ContentChannel; -import jakarta.servlet.ReadListener; -import jakarta.servlet.ServletInputStream; -import jakarta.servlet.http.HttpServletRequest; +import javax.servlet.ReadListener; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServletRequest; import java.io.IOException; import java.nio.ByteBuffer; import java.util.Objects; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java index 6afb55f5b13..e90dde0e4eb 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java @@ -9,10 +9,10 @@ import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.HttpHeaders; import com.yahoo.jdisc.http.HttpResponse; import com.yahoo.jdisc.service.BindingSetNotFoundException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.MimeTypes; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java index 96f0cdebd62..b420aabc598 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/TlsClientAuthenticationEnforcer.java @@ -3,13 +3,13 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.ConnectorConfig; -import jakarta.servlet.DispatcherType; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.HandlerWrapper; +import javax.servlet.DispatcherType; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; /** diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def index bdcc3f9e40a..ecbc451ead1 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def @@ -28,8 +28,7 @@ reuseAddress bool default=true # The maximum idle time for a connection, which roughly translates to the Socket.setSoTimeout(int). idleTimeout double default=180.0 -# TODO Vespa 9 Remove -# Has no effect since Jetty 11 upgrade +# Whether or not to have socket keep alive turned on. tcpKeepAliveEnabled bool default=false # Enable/disable TCP_NODELAY (disable/enable Nagle's algorithm). diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def index c15cb6b2cc4..f34fd523207 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def @@ -4,8 +4,7 @@ namespace=jdisc.http # Whether to enable developer mode, where stack traces etc are visible in response bodies. developerMode bool default=false -# TODO Vespa 9 Remove -# Has no effect since Jetty 11 upgrade +# The gzip compression level to use, if compression is enabled in a request. responseCompressionLevel int default=6 # Whether the request body of POSTed forms should be removed (form parameters are available as request parameters). diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java index ce205b1a893..1ff2783cc53 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java @@ -5,8 +5,6 @@ import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.http.ssl.impl.ConfiguredSslContextFactoryProvider; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.AbstractHandler; @@ -14,6 +12,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.Map; diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ErrorResponseContentCreatorTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ErrorResponseContentCreatorTest.java index fdb9f2226de..8b18c8cf09d 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ErrorResponseContentCreatorTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ErrorResponseContentCreatorTest.java @@ -2,9 +2,9 @@ package com.yahoo.jdisc.http.server.jetty; -import jakarta.servlet.http.HttpServletResponse; import org.junit.jupiter.api.Test; +import javax.servlet.http.HttpServletResponse; import java.nio.charset.StandardCharsets; import static org.junit.jupiter.api.Assertions.assertEquals; diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java index e4b82db5b9f..a23a3505bcb 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java @@ -8,17 +8,15 @@ import com.yahoo.jdisc.Response; import com.yahoo.jdisc.handler.RequestHandler; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.service.CurrentContainer; -import jakarta.servlet.http.HttpServletRequest; import org.junit.jupiter.api.Test; +import javax.servlet.http.HttpServletRequest; import java.net.URI; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; /** * @author Steinar Knutsen diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java index 502702ccf35..165659389ec 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java @@ -2,9 +2,6 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.http.server.jetty.HttpResponseStatisticsCollector.StatisticsEntry; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; @@ -13,7 +10,6 @@ import org.eclipse.jetty.http.MetaData.Response; import org.eclipse.jetty.server.AbstractConnector; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpChannel; -import org.eclipse.jetty.server.HttpChannelOverHttp; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpTransport; import org.eclipse.jetty.server.Request; @@ -23,6 +19,9 @@ import org.eclipse.jetty.util.Callback; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.nio.ByteBuffer; import java.util.List; @@ -165,8 +164,8 @@ public class HttpResponseStatisticsCollectorTest { } private Request testRequest(String scheme, int responseCode, String httpMethod, String path, com.yahoo.jdisc.Request.RequestType explicitRequestType) { - HttpChannel channel = new HttpChannelOverHttp(null, connector, new HttpConfiguration(), null, new DummyTransport()); - MetaData.Request metaData = new MetaData.Request(httpMethod, HttpURI.build(scheme + "://" + path), HttpVersion.HTTP_1_1, HttpFields.build()); + HttpChannel channel = new HttpChannel(connector, new HttpConfiguration(), null, new DummyTransport()); + MetaData.Request metaData = new MetaData.Request(httpMethod, new HttpURI(scheme + "://" + path), HttpVersion.HTTP_1_1, new HttpFields()); Request req = channel.getRequest(); if (explicitRequestType != null) req.setAttribute("requestType", explicitRequestType); @@ -193,7 +192,7 @@ public class HttpResponseStatisticsCollectorTest { private final class DummyTransport implements HttpTransport { @Override - public void send(MetaData.Request request, Response response, ByteBuffer byteBuffer, boolean b, Callback callback) { + public void send(Response info, boolean head, ByteBuffer content, boolean lastContent, Callback callback) { callback.succeeded(); } @@ -203,6 +202,11 @@ public class HttpResponseStatisticsCollectorTest { } @Override + public boolean isOptimizedForDirectBuffers() { + return false; + } + + @Override public void push(MetaData.Request request) { } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java index ae1a6494acd..7cce9f2a9ff 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java @@ -807,7 +807,6 @@ public class HttpServerConformanceTest extends ServerProviderConformanceTest { post.setProtocolVersion(HttpVersion.HTTP_1_1); request = post; } - request.addHeader("Connection", "close"); return executorService.submit(() -> httpClient.execute(request)); } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index 39b6dcdc6d5..318067ac634 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -186,10 +186,9 @@ public class HttpServerTest { @Test void requireThatServerCanEchoCompressed() throws Exception { final JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); - try (SimpleHttpClient client = driver.newClient(true)) { - client.get("/status.html") - .expectStatusCode(is(OK)); - } + SimpleHttpClient client = driver.newClient(true); + client.get("/status.html") + .expectStatusCode(is(OK)); assertTrue(driver.close()); } @@ -533,9 +532,9 @@ public class HttpServerTest { .withTrustStore(certificateFile) .build(); - try (var c = new SimpleHttpClient(trustStoreOnlyCtx, driver.server().getListenPort(), false)) { - c.get("/dummy.html").expectStatusCode(is(UNAUTHORIZED)); - } + new SimpleHttpClient(trustStoreOnlyCtx, driver.server().getListenPort(), false) + .get("/dummy.html") + .expectStatusCode(is(UNAUTHORIZED)); assertTrue(driver.close()); } @@ -551,9 +550,9 @@ public class HttpServerTest { .withTrustStore(certificateFile) .build(); - try (var c = new SimpleHttpClient(trustStoreOnlyCtx, driver.server().getListenPort(), false)) { - c.get("/status.html").expectStatusCode(is(OK)); - } + new SimpleHttpClient(trustStoreOnlyCtx, driver.server().getListenPort(), false) + .get("/status.html") + .expectStatusCode(is(OK)); assertTrue(driver.close()); } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java index 6cd6f05933a..d4d6dcee957 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java @@ -12,8 +12,6 @@ import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; import org.assertj.core.api.Assertions; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; -import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; -import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -187,15 +185,14 @@ class ProxyProtocolTest { // Using Jetty's http client as Apache httpclient does not support the proxy-protocol v1/v2. private static HttpClient createJettyHttpClient(Path certificateFile) throws Exception { - var ssl = new SslContextFactory.Client(); - ssl.setHostnameVerifier(NoopHostnameVerifier.INSTANCE); - ssl.setSslContext(new SslContextBuilder().withTrustStore(certificateFile).build()); - var connector = new ClientConnector(); - connector.setSslContextFactory(ssl); - HttpClient client = new HttpClient(new HttpClientTransportOverHTTP(connector)); - int timeout = 60 * 1000; - client.setConnectTimeout(timeout); - client.setIdleTimeout(timeout); + SslContextFactory.Client clientSslCtxFactory = new SslContextFactory.Client(); + clientSslCtxFactory.setHostnameVerifier(NoopHostnameVerifier.INSTANCE); + clientSslCtxFactory.setSslContext(new SslContextBuilder().withTrustStore(certificateFile).build()); + + HttpClient client = new HttpClient(clientSslCtxFactory); + client.setConnectTimeout(60*1000); + client.setStopTimeout(60*1000); + client.setIdleTimeout(60*1000); client.start(); return client; } diff --git a/container-dev/pom.xml b/container-dev/pom.xml index 9bbb5591fbf..0c88531a248 100644 --- a/container-dev/pom.xml +++ b/container-dev/pom.xml @@ -98,8 +98,8 @@ <!-- START JETTY embedded jars --> <exclusion> - <groupId>org.eclipse.jetty.http2</groupId> - <artifactId>http2-common</artifactId> + <groupId>org.eclipse.jetty.alpn</groupId> + <artifactId>alpn-api</artifactId> </exclusion> <exclusion> <groupId>org.eclipse.jetty.http2</groupId> @@ -111,19 +111,11 @@ </exclusion> <exclusion> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-alpn-server</artifactId> - </exclusion> - <exclusion> - <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-client</artifactId> </exclusion> <exclusion> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-http</artifactId> - </exclusion> - <exclusion> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-io</artifactId> + <artifactId>jetty-continuation</artifactId> </exclusion> <exclusion> <groupId>org.eclipse.jetty</groupId> @@ -139,11 +131,7 @@ </exclusion> <exclusion> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-util</artifactId> - </exclusion> - <exclusion> - <groupId>org.eclipse.jetty.toolchain</groupId> - <artifactId>jetty-jakarta-servlet-api</artifactId> + <artifactId>jetty-servlets</artifactId> </exclusion> <!-- END JETTY embedded jars --> </exclusions> diff --git a/container-test/pom.xml b/container-test/pom.xml index e0ff7f62e93..32a64a98b9e 100644 --- a/container-test/pom.xml +++ b/container-test/pom.xml @@ -113,8 +113,8 @@ <!-- START JETTY embedded jars --> <dependency> - <groupId>org.eclipse.jetty.http2</groupId> - <artifactId>http2-common</artifactId> + <groupId>org.eclipse.jetty.alpn</groupId> + <artifactId>alpn-api</artifactId> </dependency> <dependency> <groupId>org.eclipse.jetty.http2</groupId> @@ -126,19 +126,11 @@ </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-alpn-server</artifactId> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-client</artifactId> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-http</artifactId> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-io</artifactId> + <artifactId>jetty-continuation</artifactId> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> @@ -154,11 +146,7 @@ </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-util</artifactId> - </dependency> - <dependency> - <groupId>org.eclipse.jetty.toolchain</groupId> - <artifactId>jetty-jakarta-servlet-api</artifactId> + <artifactId>jetty-servlets</artifactId> </dependency> <!-- END JETTY embedded jars --> </dependencies> diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mailer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mailer.java index 18662edc85e..27eec34cf4e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mailer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/Mailer.java @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.organization; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; + /** * Allows sending e-mail from a particular <code>user@domain</code>. * @@ -17,4 +19,6 @@ public interface Mailer { /** Returns the domain this is configured to use. */ String domain(); + void sendVerificationMail(PendingMailVerification pendingMailVerification); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMailer.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMailer.java index cb2b76d845c..dcfa9b7ea07 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMailer.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockMailer.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.stubs; import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail; import com.yahoo.vespa.hosted.controller.api.integration.organization.Mailer; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; import java.util.ArrayList; import java.util.HashMap; @@ -47,6 +48,11 @@ public class MockMailer implements Mailer { return "domain"; } + @Override + public void sendVerificationMail(PendingMailVerification pendingMailVerification) { + send(new Mail(List.of(pendingMailVerification.getMailAddress()), "subject", "message")); + } + /** Returns the list of mails sent to the given recipient. Modifications affect the set of mails stored in this. */ public List<Mail> inbox(String recipient) { return mails.getOrDefault(recipient, List.of()); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java index fa28840d7ee..d26ebb62181 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/zone/ZoneRegistry.java @@ -110,4 +110,7 @@ public interface ZoneRegistry { /** IAM tenant developer role ARN */ Optional<String> tenantDeveloperRoleArn(TenantName tenant); + /** Returns athenz domain tied to the given cloud account */ + AthenzDomain cloudAccountAthenzDomain(CloudAccount cloudAccount); + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java index c2682334ce0..b8de63b61a9 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java @@ -58,6 +58,7 @@ enum PathGroup { "/application/v4/tenant/{tenant}/info/profile", "/application/v4/tenant/{tenant}/info/billing", "/application/v4/tenant/{tenant}/info/contacts", + "/application/v4/tenant/{tenant}/info/resend-mail-verification", "/application/v4/tenant/{tenant}/notifications", "/routing/v1/status/tenant/{tenant}/{*}"), @@ -255,7 +256,10 @@ enum PathGroup { /** Paths used to approve requests to access tenant resources */ accessRequestApproval(Matcher.tenant, "/application/v4/tenant/{tenant}/access/approve/operator", - "/application/v4/tenant/{tenant}/access/managed/operator"); + "/application/v4/tenant/{tenant}/access/managed/operator"), + + /** Path used for email verification */ + emailVerification("/user/v1/email/verify"); final List<String> pathSpecs; final List<Matcher> matchers; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java index 91eaec53aa4..2d4f98dfa8d 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java @@ -209,7 +209,11 @@ enum Policy { horizonProxyOperations(Privilege.grant(Action.all()) .on(PathGroup.horizonProxy) - .in(SystemName.PublicCd, SystemName.Public)); + .in(SystemName.PublicCd, SystemName.Public)), + + emailVerification(Privilege.grant(Action.create) + .on(PathGroup.emailVerification) + .in(SystemName.PublicCd, SystemName.Public)); private final Set<Privilege> privileges; diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java index a9d67c2d78a..e40c99a64be 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java @@ -28,7 +28,8 @@ public enum RoleDefinition { everyone(Policy.classifiedRead, Policy.publicRead, Policy.user, - Policy.tenantCreate), + Policy.tenantCreate, + Policy.emailVerification), /** Build service which may submit new applications for continuous deployment. */ buildService(everyone, diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Email.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Email.java new file mode 100644 index 00000000000..ea6e0e9c754 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/Email.java @@ -0,0 +1,56 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.tenant; + +import java.util.Objects; + +/** + * @author olaa + */ +public class Email { + + private final String emailAddress; + private final boolean isVerified; + + public Email(String emailAddress, boolean isVerified) { + this.emailAddress = emailAddress; + this.isVerified = isVerified; + } + + public String getEmailAddress() { + return emailAddress; + } + + public boolean isVerified() { + return isVerified; + } + + public static Email empty() { + return new Email("", true); + } + + public Email withEmailAddress(String emailAddress) { + return new Email(emailAddress, isVerified); + } + + public Email withVerification(boolean isVerified) { + return new Email(emailAddress, isVerified); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Email email = (Email) o; + return isVerified == email.isVerified && Objects.equals(emailAddress, email.emailAddress); + } + + @Override + public int hashCode() { + return Objects.hash(emailAddress, isVerified); + } + + @Override + public String toString() { + return emailAddress; + } +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/PendingMailVerification.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/PendingMailVerification.java new file mode 100644 index 00000000000..af5ae746d22 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/PendingMailVerification.java @@ -0,0 +1,81 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.tenant; + +import com.yahoo.config.provision.TenantName; + +import java.time.Instant; +import java.util.List; +import java.util.Objects; + +/** + * @author olaa + */ +public class PendingMailVerification { + + private final TenantName tenantName; + private final String mailAddress; + private final String verificationCode; + private final Instant verificationDeadline; + private final MailType mailType; + + public PendingMailVerification(TenantName tenantName, String mailAddress, String verificationCode, Instant verificationDeadline, MailType mailType) { + this.tenantName = tenantName; + this.mailAddress = mailAddress; + this.verificationCode = verificationCode; + this.verificationDeadline = verificationDeadline; + this.mailType = mailType; + } + + public TenantName getTenantName() { + return tenantName; + } + + public String getMailAddress() { + return mailAddress; + } + + public String getVerificationCode() { + return verificationCode; + } + + public Instant getVerificationDeadline() { + return verificationDeadline; + } + + public MailType getMailType() { + return mailType; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PendingMailVerification that = (PendingMailVerification) o; + return Objects.equals(tenantName, that.tenantName) && + Objects.equals(mailAddress, that.mailAddress) && + Objects.equals(verificationCode, that.verificationCode) && + Objects.equals(verificationDeadline, that.verificationDeadline) && + mailType == that.mailType; + } + + @Override + public int hashCode() { + return Objects.hash(tenantName, mailAddress, verificationCode, verificationDeadline, mailType); + } + + @Override + public String toString() { + return "PendingMailVerification{" + + "tenantName=" + tenantName + + ", mailAddress='" + mailAddress + '\'' + + ", verificationCode='" + verificationCode + '\'' + + ", verificationDeadline=" + verificationDeadline + + ", mailType=" + mailType + + '}'; + } + + public enum MailType { + TENANT_CONTACT, + NOTIFICATIONS + } +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantContact.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantContact.java index 3aa5600ed87..482bb26bcf9 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantContact.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantContact.java @@ -8,36 +8,36 @@ import java.util.Objects; */ public class TenantContact { private final String name; - private final String email; + private final Email email; private final String phone; - private TenantContact(String name, String email, String phone) { + private TenantContact(String name, Email email, String phone) { this.name = Objects.requireNonNull(name); this.email = Objects.requireNonNull(email); this.phone = Objects.requireNonNull(phone); } - public static TenantContact from(String name, String email, String phone) { + public static TenantContact from(String name, Email email, String phone) { return new TenantContact(name, email, phone); } - public static TenantContact from(String name, String email) { + public static TenantContact from(String name, Email email) { return TenantContact.from(name, email, ""); } public static TenantContact empty() { - return new TenantContact("", "", ""); + return new TenantContact("", Email.empty(), ""); } public String name() { return name; } - public String email() { return email; } + public Email email() { return email; } public String phone() { return phone; } public TenantContact withName(String name) { return new TenantContact(name, email, phone); } - public TenantContact withEmail(String email) { + public TenantContact withEmail(Email email) { return new TenantContact(name, email, phone); } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantContacts.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantContacts.java index bd8671d814f..7e0fc50660e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantContacts.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantContacts.java @@ -5,6 +5,7 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.stream.Collectors; /** * Tenant contacts are targets of the notification system. Sometimes they @@ -35,6 +36,13 @@ public class TenantContacts { return contacts; } + public <T extends Contact> List<T> ofType(Class<T> type) { + return contacts.stream() + .filter(type::isInstance) + .map(type::cast) + .collect(Collectors.toList()); + } + public boolean isEmpty() { return contacts.isEmpty(); } @@ -77,14 +85,18 @@ public class TenantContacts { } public static class EmailContact extends Contact { - private final String email; + private final Email email; - public EmailContact(List<Audience> audiences, String email) { + public EmailContact(List<Audience> audiences, Email email) { super(audiences); this.email = email; } - public String email() { return email; } + public Email email() { return email; } + + public EmailContact withEmail(Email email) { + return new EmailContact(audiences(), email); + } @Override public Type type() { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantInfo.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantInfo.java index 0ca7863fbce..36a302ed0d8 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantInfo.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/tenant/TenantInfo.java @@ -23,7 +23,7 @@ public class TenantInfo { private final TenantBilling billingContact; private final TenantContacts contacts; - TenantInfo(String name, String email, String website, String contactName, String contactEmail, + TenantInfo(String name, String email, String website, String contactName, Email contactEmail, TenantAddress address, TenantBilling billingContact, TenantContacts contacts) { this(name, email, website, TenantContact.from(contactName, contactEmail), address, billingContact, contacts); } @@ -39,7 +39,7 @@ public class TenantInfo { } public static TenantInfo empty() { - return new TenantInfo("", "", "", "", "", TenantAddress.empty(), TenantBilling.empty(), TenantContacts.empty()); + return new TenantInfo("", "", "", "", Email.empty(), TenantAddress.empty(), TenantBilling.empty(), TenantContacts.empty()); } public String name() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 7e284a8aef5..902c1803b6b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -670,7 +670,7 @@ public class ApplicationController { } } - private Optional<CloudAccount> decideCloudAccountOf(DeploymentId deployment, DeploymentSpec spec) { + public Optional<CloudAccount> decideCloudAccountOf(DeploymentId deployment, DeploymentSpec spec) { ZoneId zoneId = deployment.zoneId(); Optional<CloudAccount> requestedAccount = spec.instance(deployment.applicationId().instance()) .flatMap(instanceSpec -> instanceSpec.cloudAccount(zoneId.environment(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index af56666c6eb..22ce4db10e5 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -86,6 +86,7 @@ public class Controller extends AbstractComponent { private final NotificationsDb notificationsDb; private final SupportAccessControl supportAccessControl; private final Notifier notifier; + private final MailVerifier mailVerifier; /** * Creates a controller @@ -126,6 +127,7 @@ public class Controller extends AbstractComponent { notifier = new Notifier(curator, serviceRegistry.zoneRegistry(), serviceRegistry.mailer(), flagSource); notificationsDb = new NotificationsDb(this); supportAccessControl = new SupportAccessControl(this); + mailVerifier = new MailVerifier(tenantController, serviceRegistry.mailer(), curator, clock); // Record the version of this controller curator().writeControllerVersion(this.hostname(), serviceRegistry.controllerVersion()); @@ -339,4 +341,8 @@ public class Controller extends AbstractComponent { public Notifier notifier() { return notifier; } + + public MailVerifier mailVerifier() { + return mailVerifier; + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java new file mode 100644 index 00000000000..a7f3a3ca3b2 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/MailVerifier.java @@ -0,0 +1,116 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller; + +import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Mailer; +import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; +import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; +import com.yahoo.vespa.hosted.controller.tenant.Tenant; +import com.yahoo.vespa.hosted.controller.tenant.TenantContacts; +import com.yahoo.vespa.hosted.controller.tenant.TenantInfo; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; + +import java.time.Clock; +import java.time.Duration; +import java.util.Optional; +import java.util.UUID; + + +/** + * @author olaa + */ +public class MailVerifier { + + private final TenantController tenantController; + private final Mailer mailer; + private final CuratorDb curatorDb; + private final Clock clock; + private static final Duration VERIFICATION_DEADLINE = Duration.ofDays(7); + + public MailVerifier(TenantController tenantController, Mailer mailer, CuratorDb curatorDb, Clock clock) { + this.tenantController = tenantController; + this.mailer = mailer; + this.curatorDb = curatorDb; + this.clock = clock; + } + + public PendingMailVerification sendMailVerification(TenantName tenantName, String email, PendingMailVerification.MailType mailType) { + if (!email.contains("@")) { + throw new IllegalArgumentException("Invalid email address"); + } + + var verificationCode = UUID.randomUUID().toString(); + var verificationDeadline = clock.instant().plus(VERIFICATION_DEADLINE); + var pendingMailVerification = new PendingMailVerification(tenantName, email, verificationCode, verificationDeadline, mailType); + writePendingVerification(pendingMailVerification); + mailer.sendVerificationMail(pendingMailVerification); + return pendingMailVerification; + } + + public Optional<PendingMailVerification> resendMailVerification(TenantName tenantName, String email, PendingMailVerification.MailType mailType) { + var oldPendingVerification = curatorDb.listPendingMailVerifications() + .stream() + .filter(pendingMailVerification -> + pendingMailVerification.getMailAddress().equals(email) && + pendingMailVerification.getMailType().equals(mailType) && + pendingMailVerification.getTenantName().equals(tenantName) + ).findFirst(); + + if (oldPendingVerification.isEmpty()) + return Optional.empty(); + + try (var lock = curatorDb.lockPendingMailVerification(oldPendingVerification.get().getVerificationCode())) { + curatorDb.deletePendingMailVerification(oldPendingVerification.get()); + } + + return Optional.of(sendMailVerification(tenantName, email, mailType)); + } + + public boolean verifyMail(String verificationCode) { + return curatorDb.getPendingMailVerification(verificationCode) + .filter(pendingMailVerification -> pendingMailVerification.getVerificationDeadline().isAfter(clock.instant())) + .map(pendingMailVerification -> { + var tenant = requireCloudTenant(pendingMailVerification.getTenantName()); + var oldTenantInfo = tenant.info(); + var updatedTenantInfo = switch (pendingMailVerification.getMailType()) { + case NOTIFICATIONS -> withTenantContacts(oldTenantInfo, pendingMailVerification); + case TENANT_CONTACT -> oldTenantInfo.withContact(oldTenantInfo.contact() + .withEmail(oldTenantInfo.contact().email().withVerification(true))); + }; + + tenantController.lockOrThrow(tenant.name(), LockedTenant.Cloud.class, lockedTenant -> { + lockedTenant = lockedTenant.withInfo(updatedTenantInfo); + tenantController.store(lockedTenant); + }); + + try (var lock = curatorDb.lockPendingMailVerification(pendingMailVerification.getVerificationCode())) { + curatorDb.deletePendingMailVerification(pendingMailVerification); + } + return true; + }).orElse(false); + } + + private TenantInfo withTenantContacts(TenantInfo oldInfo, PendingMailVerification pendingMailVerification) { + var newContacts = oldInfo.contacts().ofType(TenantContacts.EmailContact.class) + .stream() + .map(contact -> { + if (pendingMailVerification.getMailAddress().equals(contact.email().getEmailAddress())) + return contact.withEmail(contact.email().withVerification(true)); + return contact; + }).toList(); + return oldInfo.withContacts(new TenantContacts(newContacts)); + } + + private void writePendingVerification(PendingMailVerification pendingMailVerification) { + try (var lock = curatorDb.lockPendingMailVerification(pendingMailVerification.getVerificationCode())) { + curatorDb.writePendingMailVerification(pendingMailVerification); + } + } + + private CloudTenant requireCloudTenant(TenantName tenantName) { + return tenantController.get(tenantName) + .filter(tenant -> tenant.type() == Tenant.Type.cloud) + .map(CloudTenant.class::cast) + .orElseThrow(() -> new IllegalStateException("Mail verification is only applicable for cloud tenants")); + } +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java index f2c9d55b2a2..1074649ca7a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java @@ -110,7 +110,10 @@ public class Notifier { private void dispatch(Notification notification, Collection<TenantContacts.EmailContact> contacts) { try { var content = formatter.format(notification); - mailer.send(mailOf(content, contacts.stream().map(c -> c.email()).collect(Collectors.toList()))); + mailer.send(mailOf(content, contacts.stream() + .filter(c -> c.email().isVerified()) + .map(c -> c.email().getEmailAddress()) + .collect(Collectors.toList()))); } catch (MailerException e) { log.log(Level.SEVERE, "Failed sending email", e); } catch (MissingOptionalException e) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 0e8f1648765..d582937cb0b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -34,6 +34,7 @@ import com.yahoo.vespa.hosted.controller.routing.RoutingPolicy; import com.yahoo.vespa.hosted.controller.routing.RoutingStatus; import com.yahoo.vespa.hosted.controller.routing.ZoneRoutingPolicy; import com.yahoo.vespa.hosted.controller.support.access.SupportAccess; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.versions.OsVersionStatus; import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget; @@ -94,6 +95,7 @@ public class CuratorDb { private static final Path changeRequestsRoot = root.append("changeRequests"); private static final Path notificationsRoot = root.append("notifications"); private static final Path supportAccessRoot = root.append("supportAccess"); + private static final Path mailVerificationRoot = root.append("mailVerification"); private final NodeVersionSerializer nodeVersionSerializer = new NodeVersionSerializer(); private final VersionStatusSerializer versionStatusSerializer = new VersionStatusSerializer(nodeVersionSerializer); @@ -231,6 +233,10 @@ public class CuratorDb { return curator.lock(lockRoot.append("deploymentRetriggerQueue"), defaultLockTimeout); } + public Mutex lockPendingMailVerification(String verificationCode) { + return curator.lock(lockRoot.append("pendingMailVerification").append(verificationCode), defaultLockTimeout); + } + // -------------- Helpers ------------------------------------------ /** Try locking with a low timeout, meaning it is OK to fail lock acquisition. @@ -655,6 +661,28 @@ public class CuratorDb { curator.set(deploymentRetriggerPath(), asJson(retriggerEntrySerializer.toSlime(retriggerEntries))); } + // -------------- Pending mail verification ------------------------------- + + public Optional<PendingMailVerification> getPendingMailVerification(String verificationCode) { + return readSlime(mailVerificationPath(verificationCode)).map(MailVerificationSerializer::fromSlime); + } + + public List<PendingMailVerification> listPendingMailVerifications() { + return curator.getChildren(mailVerificationRoot) + .stream() + .map(this::getPendingMailVerification) + .flatMap(Optional::stream) + .collect(Collectors.toList()); + } + + public void writePendingMailVerification(PendingMailVerification pendingMailVerification) { + curator.set(mailVerificationPath(pendingMailVerification.getVerificationCode()), asJson(MailVerificationSerializer.toSlime(pendingMailVerification))); + } + + public void deletePendingMailVerification(PendingMailVerification pendingMailVerification) { + curator.delete(mailVerificationPath(pendingMailVerification.getVerificationCode())); + } + // -------------- Paths --------------------------------------------------- private static Path upgradesPerMinutePath() { @@ -755,4 +783,8 @@ public class CuratorDb { return root.append("deploymentRetriggerQueue"); } + private static Path mailVerificationPath(String verificationCode) { + return mailVerificationRoot.append(verificationCode); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializer.java new file mode 100644 index 00000000000..6910d5c21c0 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializer.java @@ -0,0 +1,53 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.config.provision.TenantName; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.Slime; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; + +import java.time.Instant; + +/** + * @author olaa + */ +public class MailVerificationSerializer { + + private static final String tenantField = "tenant"; + private static final String audiencesField = "audiences"; + private static final String emailField = "email"; + private static final String emailTypeField = "emailType"; + private static final String emailVerificationCodeField = "emailVerificationCode"; + private static final String emailVerificationDeadlineField = "emailVerificationDeadline"; + private static final String rolesField = "roles"; + + public static Slime toSlime(PendingMailVerification pendingMailVerification) { + var slime = new Slime(); + var object = slime.setObject(); + toSlime(pendingMailVerification, object); + return slime; + } + + public static void toSlime(PendingMailVerification pendingMailVerification, Cursor object) { + object.setString(tenantField, pendingMailVerification.getTenantName().value()); + object.setString(emailVerificationCodeField, pendingMailVerification.getVerificationCode()); + object.setString(emailField, pendingMailVerification.getMailAddress()); + object.setLong(emailVerificationDeadlineField, pendingMailVerification.getVerificationDeadline().toEpochMilli()); + object.setString(emailTypeField, pendingMailVerification.getMailType().name()); + } + + public static PendingMailVerification fromSlime(Slime slime) { + return fromSlime(slime.get()); + } + + public static PendingMailVerification fromSlime(Inspector inspector) { + var tenant = TenantName.from(inspector.field(tenantField).asString()); + var address = inspector.field(emailField).asString(); + var verificationCode = inspector.field(emailVerificationCodeField).asString(); + var deadline = Instant.ofEpochMilli(inspector.field(emailVerificationDeadlineField).asLong()); + var type = PendingMailVerification.MailType.valueOf(inspector.field(emailTypeField).asString()); + return new PendingMailVerification(tenant, address, verificationCode, deadline, type); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java index fc7cafe4c89..b6d0155b6ab 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializer.java @@ -21,6 +21,7 @@ import com.yahoo.vespa.hosted.controller.tenant.ArchiveAccess; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.DeletedTenant; +import com.yahoo.vespa.hosted.controller.tenant.Email; import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.TenantAddress; @@ -234,7 +235,7 @@ public class TenantSerializer { .withWebsite(infoObject.field("website").asString()) .withContact(TenantContact.from( infoObject.field("contactName").asString(), - infoObject.field("contactEmail").asString())) + new Email(infoObject.field("contactEmail").asString(), asBoolOrTrue(infoObject.field("contactEmailVerified"))))) .withAddress(tenantInfoAddressFromSlime(infoObject.field("address"))) .withBilling(tenantInfoBillingContactFromSlime(infoObject.field("billingContact"))) .withContacts(tenantContactsFrom(infoObject.field("contacts"))); @@ -253,7 +254,7 @@ public class TenantSerializer { return TenantBilling.empty() .withContact(TenantContact.from( billingObject.field("name").asString(), - billingObject.field("email").asString(), + new Email(billingObject.field("email").asString(), true), billingObject.field("phone").asString())) .withAddress(tenantInfoAddressFromSlime(billingObject.field("address"))); } @@ -283,7 +284,8 @@ public class TenantSerializer { infoCursor.setString("email", info.email()); infoCursor.setString("website", info.website()); infoCursor.setString("contactName", info.contact().name()); - infoCursor.setString("contactEmail", info.contact().email()); + infoCursor.setString("contactEmail", info.contact().email().getEmailAddress()); + infoCursor.setBool("contactEmailVerified", info.contact().email().isVerified()); toSlime(info.address(), infoCursor); toSlime(info.billingContact(), infoCursor); toSlime(info.contacts(), infoCursor); @@ -305,7 +307,7 @@ public class TenantSerializer { Cursor addressCursor = parentCursor.setObject("billingContact"); addressCursor.setString("name", billingContact.contact().name()); - addressCursor.setString("email", billingContact.contact().email()); + addressCursor.setString("email", billingContact.contact().email().getEmailAddress()); addressCursor.setString("phone", billingContact.contact().phone()); toSlime(billingContact.address(), addressCursor); } @@ -386,7 +388,8 @@ public class TenantSerializer { switch (contact.type()) { case EMAIL: var email = (TenantContacts.EmailContact) contact; - data.setString("email", email.email()); + data.setString("email", email.email().getEmailAddress()); + data.setBool("emailVerified", email.email().isVerified()); return; default: throw new IllegalArgumentException("Serialization for contact type not implemented: " + contact.type()); @@ -401,7 +404,8 @@ public class TenantSerializer { .collect(Collectors.toUnmodifiableList()); switch (type) { case EMAIL: - return new TenantContacts.EmailContact(audiences, inspector.field("data").field("email").asString()); + var isVerified = asBoolOrTrue(inspector.field("data").field("emailVerified")); + return new TenantContacts.EmailContact(audiences, new Email(inspector.field("data").field("email").asString(), isVerified)); default: throw new IllegalArgumentException("Serialization for contact type not implemented: " + type); } @@ -460,4 +464,8 @@ public class TenantSerializer { } } + private boolean asBoolOrTrue(Inspector inspector) { + return !inspector.valid() || inspector.asBool(); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 35483309cae..23f945e02be 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -111,7 +111,9 @@ import com.yahoo.vespa.hosted.controller.tenant.ArchiveAccess; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.DeletedTenant; +import com.yahoo.vespa.hosted.controller.tenant.Email; import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.TenantAddress; import com.yahoo.vespa.hosted.controller.tenant.TenantBilling; @@ -301,6 +303,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/info/profile")) return withCloudTenant(path.get("tenant"), request, this::putTenantInfoProfile); if (path.matches("/application/v4/tenant/{tenant}/info/billing")) return withCloudTenant(path.get("tenant"), request, this::putTenantInfoBilling); if (path.matches("/application/v4/tenant/{tenant}/info/contacts")) return withCloudTenant(path.get("tenant"), request, this::putTenantInfoContacts); + if (path.matches("/application/v4/tenant/{tenant}/info/resend-mail-verification")) return withCloudTenant(path.get("tenant"), request, this::resendEmailVerification); if (path.matches("/application/v4/tenant/{tenant}/archive-access")) return allowAwsArchiveAccess(path.get("tenant"), request); // TODO(enygaard, 2022-05-25) Remove when no longer used by console if (path.matches("/application/v4/tenant/{tenant}/archive-access/aws")) return allowAwsArchiveAccess(path.get("tenant"), request); if (path.matches("/application/v4/tenant/{tenant}/archive-access/gcp")) return allowGcpArchiveAccess(path.get("tenant"), request); @@ -522,7 +525,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { infoCursor.setString("email", info.email()); infoCursor.setString("website", info.website()); infoCursor.setString("contactName", info.contact().name()); - infoCursor.setString("contactEmail", info.contact().email()); + infoCursor.setString("contactEmail", info.contact().email().getEmailAddress()); + infoCursor.setBool("contactEmailVerified", info.contact().email().isVerified()); toSlime(info.address(), infoCursor); toSlime(info.billingContact(), infoCursor); toSlime(info.contacts(), infoCursor); @@ -539,7 +543,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (!info.isEmpty()) { var contact = root.setObject("contact"); contact.setString("name", info.contact().name()); - contact.setString("email", info.contact().email()); + contact.setString("email", info.contact().email().getEmailAddress()); + contact.setBool("emailVerified", info.contact().email().isVerified()); var tenant = root.setObject("tenant"); tenant.setString("company", info.name()); @@ -560,9 +565,17 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { private SlimeJsonResponse putTenantInfoProfile(CloudTenant cloudTenant, Inspector inspector) { var info = cloudTenant.info(); + var mergedEmail = optional("email", inspector.field("contact")) + .filter(address -> !address.equals(info.contact().email().getEmailAddress())) + .map(address -> { + controller.mailVerifier().sendMailVerification(cloudTenant.name(), address, PendingMailVerification.MailType.TENANT_CONTACT); + return new Email(address, false); + }) + .orElse(info.contact().email()); + var mergedContact = TenantContact.empty() .withName(getString(inspector.field("contact").field("name"), info.contact().name())) - .withEmail(getString(inspector.field("contact").field("email"), info.contact().email())); + .withEmail(mergedEmail); var mergedAddress = updateTenantInfoAddress(inspector.field("address"), info.address()); @@ -592,7 +605,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { var contact = root.setObject("contact"); contact.setString("name", billingContact.contact().name()); - contact.setString("email", billingContact.contact().email()); + contact.setString("email", billingContact.contact().email().getEmailAddress()); contact.setString("phone", billingContact.contact().phone()); toSlime(billingContact.address(), root); // will create "address" on the parent @@ -606,7 +619,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { var contact = info.billingContact().contact(); var address = info.billingContact().address(); - var mergedContact = updateTenantInfoContact(inspector.field("contact"), contact); + var mergedContact = updateTenantInfoContact(inspector.field("contact"), cloudTenant.name(), contact, false); var mergedAddress = updateTenantInfoAddress(inspector.field("address"), info.billingContact().address()); var mergedBilling = info.billingContact() @@ -633,7 +646,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { private SlimeJsonResponse putTenantInfoContacts(CloudTenant cloudTenant, Inspector inspector) { var mergedInfo = cloudTenant.info() - .withContacts(updateTenantInfoContacts(inspector.field("contacts"), cloudTenant.info().contacts())); + .withContacts(updateTenantInfoContacts(inspector.field("contacts"), cloudTenant.name(), cloudTenant.info().contacts())); // Store changes controller.tenants().lockOrThrow(cloudTenant.name(), LockedTenant.Cloud.class, lockedTenant -> { @@ -649,10 +662,10 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (mergedInfo.contact().name().isBlank()) { throw new IllegalArgumentException("'contactName' cannot be empty"); } - if (mergedInfo.contact().email().isBlank()) { + if (mergedInfo.contact().email().getEmailAddress().isBlank()) { throw new IllegalArgumentException("'contactEmail' cannot be empty"); } - if (! mergedInfo.contact().email().contains("@")) { + if (! mergedInfo.contact().email().getEmailAddress().contains("@")) { // email address validation is notoriously hard - we should probably just try to send a // verification email to this address. checking for @ is a simple best-effort. throw new IllegalArgumentException("'contactEmail' needs to be an email address"); @@ -682,7 +695,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { Cursor addressCursor = parentCursor.setObject("billingContact"); addressCursor.setString("name", billingContact.contact().name()); - addressCursor.setString("email", billingContact.contact().email()); + addressCursor.setString("email", billingContact.contact().email().getEmailAddress()); addressCursor.setString("phone", billingContact.contact().phone()); toSlime(billingContact.address(), addressCursor); } @@ -696,7 +709,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { switch (contact.type()) { case EMAIL: var email = (TenantContacts.EmailContact) contact; - contactCursor.setString("email", email.email()); + contactCursor.setString("email", email.email().getEmailAddress()); + contactCursor.setBool("emailVerified", email.email().isVerified()); return; default: throw new IllegalArgumentException("Serialization for contact type not implemented: " + contact.type()); @@ -737,9 +751,17 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { // Merge info from request with the existing info Inspector insp = toSlime(request.getData()).get(); + var mergedEmail = optional("contactEmail", insp) + .filter(address -> !address.equals(oldInfo.contact().email().getEmailAddress())) + .map(address -> { + controller.mailVerifier().sendMailVerification(tenant.name(), address, PendingMailVerification.MailType.TENANT_CONTACT); + return new Email(address, false); + }) + .orElse(oldInfo.contact().email()); + TenantContact mergedContact = TenantContact.empty() .withName(getString(insp.field("contactName"), oldInfo.contact().name())) - .withEmail(getString(insp.field("contactEmail"), oldInfo.contact().email())); + .withEmail(mergedEmail); TenantInfo mergedInfo = TenantInfo.empty() .withName(getString(insp.field("name"), oldInfo.name())) @@ -747,8 +769,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { .withWebsite(getString(insp.field("website"), oldInfo.website())) .withContact(mergedContact) .withAddress(updateTenantInfoAddress(insp.field("address"), oldInfo.address())) - .withBilling(updateTenantInfoBillingContact(insp.field("billingContact"), oldInfo.billingContact())) - .withContacts(updateTenantInfoContacts(insp.field("contacts"), oldInfo.contacts())); + .withBilling(updateTenantInfoBillingContact(insp.field("billingContact"), tenant.name(), oldInfo.billingContact())) + .withContacts(updateTenantInfoContacts(insp.field("contacts"), tenant.name(), oldInfo.contacts())); validateMergedTenantInfo(mergedInfo); @@ -782,32 +804,34 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { throw new IllegalArgumentException("All address fields must be set"); } - private TenantContact updateTenantInfoContact(Inspector insp, TenantContact oldContact) { + private TenantContact updateTenantInfoContact(Inspector insp, TenantName tenantName, TenantContact oldContact, boolean isBillingContact) { if (!insp.valid()) return oldContact; - String email = getString(insp.field("email"), oldContact.email()); - - if (!email.isBlank() && !email.contains("@")) { - // email address validation is notoriously hard - we should probably just try to send a - // verification email to this address. checking for @ is a simple best-effort. - throw new IllegalArgumentException("'email' needs to be an email address"); - } + var mergedEmail = optional("email", insp) + .filter(address -> !address.equals(oldContact.email().getEmailAddress())) + .map(address -> { + if (isBillingContact) + return new Email(address, true); + controller.mailVerifier().sendMailVerification(tenantName, address, PendingMailVerification.MailType.TENANT_CONTACT); + return new Email(address, false); + }) + .orElse(oldContact.email()); return TenantContact.empty() .withName(getString(insp.field("name"), oldContact.name())) - .withEmail(getString(insp.field("email"), oldContact.email())) + .withEmail(mergedEmail) .withPhone(getString(insp.field("phone"), oldContact.phone())); } - private TenantBilling updateTenantInfoBillingContact(Inspector insp, TenantBilling oldContact) { + private TenantBilling updateTenantInfoBillingContact(Inspector insp, TenantName tenantName, TenantBilling oldContact) { if (!insp.valid()) return oldContact; return TenantBilling.empty() - .withContact(updateTenantInfoContact(insp, oldContact.contact())) + .withContact(updateTenantInfoContact(insp, tenantName, oldContact.contact(), true)) .withAddress(updateTenantInfoAddress(insp.field("address"), oldContact.address())); } - private TenantContacts updateTenantInfoContacts(Inspector insp, TenantContacts oldContacts) { + private TenantContacts updateTenantInfoContacts(Inspector insp, TenantName tenantName, TenantContacts oldContacts) { if (!insp.valid()) return oldContacts; List<TenantContacts.EmailContact> contacts = SlimeUtils.entriesStream(insp).map(inspector -> { @@ -816,11 +840,16 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { .map(audience -> fromAudience(audience.asString())) .toList(); - if (!email.contains("@")) { - throw new IllegalArgumentException("'email' needs to be an email address"); - } - - return new TenantContacts.EmailContact(audiences, email); + // If contact exists, update audience. Otherwise, create new unverified contact + return oldContacts.ofType(TenantContacts.EmailContact.class) + .stream() + .filter(contact -> contact.email().getEmailAddress().equals(email)) + .findAny() + .map(emailContact -> new TenantContacts.EmailContact(audiences, emailContact.email())) + .orElseGet(() -> { + controller.mailVerifier().sendMailVerification(tenantName, email, PendingMailVerification.MailType.NOTIFICATIONS); + return new TenantContacts.EmailContact(audiences, new Email(email, false)); + }); }).toList(); return new TenantContacts(contacts); @@ -1499,6 +1528,21 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { return new MessageResponse(type.jobName() + " for " + id + " resumed"); } + private SlimeJsonResponse resendEmailVerification(CloudTenant tenant, Inspector inspector) { + var mail = mandatory("mail", inspector).asString(); + var type = mandatory("mailType", inspector).asString(); + + var mailType = switch (type) { + case "contact" -> PendingMailVerification.MailType.TENANT_CONTACT; + case "notifications" -> PendingMailVerification.MailType.NOTIFICATIONS; + default -> throw new IllegalArgumentException("Unknown mail type " + type); + }; + + var pendingVerification = controller.mailVerifier().resendMailVerification(tenant.name(), mail, mailType); + return pendingVerification.isPresent() ? new MessageResponse("Re-sent verification mail to " + mail) : + ErrorResponse.notFoundError("No pending mail verification found for " + mail); + } + private void toSlime(Cursor object, Application application, HttpRequest request) { object.setString("tenant", application.id().tenant().value()); object.setString("application", application.id().application().value()); @@ -1814,6 +1858,12 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { application.projectId().ifPresent(i -> response.setString("screwdriverId", String.valueOf(i))); + controller.applications().decideCloudAccountOf(deploymentId, application.deploymentSpec()).ifPresent(cloudAccount -> { + Cursor enclave = response.setObject("enclave"); + enclave.setString("cloudAccount", cloudAccount.value()); + enclave.setString("athensDomain", controller.zoneRegistry().cloudAccountAthenzDomain(cloudAccount).value()); + }); + var instance = application.instances().get(deploymentId.applicationId().instance()); if (instance != null) { if (!instance.rotations().isEmpty() && deployment.zone().environment() == Environment.prod) @@ -2006,7 +2056,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { User user = getAttribute(request, User.ATTRIBUTE_NAME, User.class); TenantInfo info = controller.tenants().require(tenant, CloudTenant.class) .info() - .withContact(TenantContact.from(user.name(), user.email())); + .withContact(TenantContact.from(user.name(), new Email(user.email(), true))); // Store changes controller.tenants().lockOrThrow(tenant, LockedTenant.Cloud.class, lockedTenant -> { lockedTenant = lockedTenant.withInfo(info); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java index 3491ef4ab03..7a8ef1d4ee6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiHandler.java @@ -112,6 +112,7 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { private HttpResponse handlePOST(Path path, HttpRequest request) { if (path.matches("/user/v1/tenant/{tenant}")) return addTenantRoleMember(path.get("tenant"), request); + if (path.matches("/user/v1/email/verify")) return verifyEmail(request); return ErrorResponse.notFoundError(Text.format("No '%s' handler at '%s'", request.getMethod(), request.getUri().getPath())); @@ -311,6 +312,16 @@ public class UserApiHandler extends ThreadedHttpRequestHandler { return new MessageResponse(user + " is now a member of " + roles.stream().map(Role::toString).collect(Collectors.joining(", "))); } + private HttpResponse verifyEmail(HttpRequest request) { + var inspector = bodyInspector(request); + var verificationCode = require("verificationCode", Inspector::asString, inspector); + var verified = controller.mailVerifier().verifyMail(verificationCode); + + if (verified) + return new MessageResponse("Email with verification code " + verificationCode + " has been verified"); + return ErrorResponse.notFoundError("No pending email verification with code " + verificationCode + " found"); + } + private HttpResponse removeTenantRoleMember(String tenantName, HttpRequest request) { Inspector requestObject = bodyInspector(request); var tenant = TenantName.from(tenantName); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java new file mode 100644 index 00000000000..873ab435444 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MailVerifierTest.java @@ -0,0 +1,102 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller; + +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer; +import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; +import com.yahoo.vespa.hosted.controller.tenant.Email; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; +import com.yahoo.vespa.hosted.controller.tenant.Tenant; +import com.yahoo.vespa.hosted.controller.tenant.TenantContacts; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.time.Duration; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + + +/** + * @author olaa + */ +class MailVerifierTest { + + private final ControllerTester tester = new ControllerTester(SystemName.Public); + private final MockMailer mailer = tester.serviceRegistry().mailer(); + private final MailVerifier mailVerifier = new MailVerifier(tester.controller().tenants(), mailer, tester.curator(), tester.clock()); + + private static final TenantName tenantName = TenantName.from("scoober"); + private static final String mail = "unverified@bar.com"; + private static final List<TenantContacts.Audience> audiences = List.of(TenantContacts.Audience.NOTIFICATIONS, TenantContacts.Audience.TENANT); + + @BeforeEach + public void setup() { + tester.createTenant(tenantName.value(), Tenant.Type.cloud); + + tester.controller().tenants().lockOrThrow(tenantName, LockedTenant.Cloud.class, lockedTenant -> { + var contacts = List.of( + new TenantContacts.EmailContact(audiences, new Email("verified@bar.com", true)), + new TenantContacts.EmailContact(audiences, new Email(mail, false)), + new TenantContacts.EmailContact(audiences, new Email("another-unverified@bar.com", false)) + ); + lockedTenant = lockedTenant.withInfo(lockedTenant.get().info().withContacts(new TenantContacts(contacts))); + tester.controller().tenants().store(lockedTenant); + }); + } + + @Test + public void test_new_mail_verification() { + mailVerifier.sendMailVerification(tenantName, mail, PendingMailVerification.MailType.NOTIFICATIONS); + + // Verify mail is sent + var expectedMail = "message"; + assertEquals(1, mailer.inbox(mail).size()); + assertEquals(expectedMail, mailer.inbox(mail).get(0).message()); + + // Verify ZK data is updated + var writtenMailVerification = tester.curator().listPendingMailVerifications().get(0); + assertEquals(PendingMailVerification.MailType.NOTIFICATIONS, writtenMailVerification.getMailType()); + assertEquals(tenantName, writtenMailVerification.getTenantName()); + assertEquals(tester.clock().instant().plus(Duration.ofDays(7)), writtenMailVerification.getVerificationDeadline()); + assertEquals(mail, writtenMailVerification.getMailAddress()); + + // Mail verification is no-op if deadline has passed + tester.clock().advance(Duration.ofDays(14)); + assertFalse(mailVerifier.verifyMail(writtenMailVerification.getVerificationCode())); + assertFalse(tester.curator().listPendingMailVerifications().isEmpty()); + + // Mail is verified + tester.clock().retreat(Duration.ofDays(14)); + mailVerifier.verifyMail(writtenMailVerification.getVerificationCode()); + assertTrue(tester.curator().listPendingMailVerifications().isEmpty()); + var tenant = tester.controller().tenants().require(tenantName, CloudTenant.class); + var expectedContacts = List.of( + new TenantContacts.EmailContact(audiences, new Email("verified@bar.com", true)), + new TenantContacts.EmailContact(audiences, new Email(mail, true)), + new TenantContacts.EmailContact(audiences, new Email("another-unverified@bar.com", false)) + ); + assertEquals(expectedContacts, tenant.info().contacts().all()); + } + + @Test + public void resending_verification_deletes_old_one() { + var pendingMailVerification = mailVerifier.sendMailVerification(tenantName, mail, PendingMailVerification.MailType.NOTIFICATIONS); + var tenant = tester.controller().tenants().require(tenantName, CloudTenant.class); + + // Unknown mail is no-op + var resentVerification = mailVerifier.resendMailVerification(tenantName, "unknown-mail", PendingMailVerification.MailType.NOTIFICATIONS); + assertTrue(resentVerification.isEmpty()); + assertTrue(tester.curator().getPendingMailVerification(pendingMailVerification.getVerificationCode()).isPresent()); + + // Verification mail is re-sent, old data is replaced + resentVerification = mailVerifier.resendMailVerification(tenantName, mail, PendingMailVerification.MailType.NOTIFICATIONS); + assertTrue(resentVerification.isPresent()); + assertTrue(tester.curator().getPendingMailVerification(pendingMailVerification.getVerificationCode()).isEmpty()); + assertTrue(tester.curator().getPendingMailVerification(resentVerification.get().getVerificationCode()).isPresent()); + } + +}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java index a3ea56ccc72..dd0f2ca028c 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneRegistryMock.java @@ -255,6 +255,11 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry @Override public Optional<String> tenantDeveloperRoleArn(TenantName tenant) { return Optional.empty(); } @Override + public AthenzDomain cloudAccountAthenzDomain(CloudAccount cloudAccount) { + return AthenzDomain.from("vespa.enclave"); + } + + @Override public boolean hasZone(ZoneId zoneId) { return zones.stream().anyMatch(zone -> zone.getId().equals(zoneId)); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java index 4c1344650f8..852d4847b7e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotificationsDbTest.java @@ -22,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import com.yahoo.vespa.hosted.controller.tenant.ArchiveAccess; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; +import com.yahoo.vespa.hosted.controller.tenant.Email; import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo; import com.yahoo.vespa.hosted.controller.tenant.TenantContacts; import com.yahoo.vespa.hosted.controller.tenant.TenantInfo; @@ -49,7 +50,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class NotificationsDbTest { private static final TenantName tenant = TenantName.from("tenant1"); - private static final String email = "user1@example.com"; + private static final Email email = new Email("user1@example.com", true); private static final CloudTenant cloudTenant = new CloudTenant(tenant, Instant.now(), LastLoginInfo.EMPTY, @@ -111,19 +112,19 @@ public class NotificationsDbTest { ; var a = notifications.get(0); notificationsDb.setNotification(a.source(), a.type(), a.level(), a.messages()); - assertEquals(0, mailer.inbox(email).size()); + assertEquals(0, mailer.inbox(email.getEmailAddress()).size()); // Replace the 3rd notification. but don't change source or type notificationsDb.setNotification(notification1.source(), notification1.type(), notification1.level(), notification1.messages()); - assertEquals(0, mailer.inbox(email).size()); + assertEquals(0, mailer.inbox(email.getEmailAddress()).size()); // Notification for a new app, add without replacement notificationsDb.setNotification(notification2.source(), notification2.type(), notification2.level(), notification2.messages()); - assertEquals(1, mailer.inbox(email).size()); + assertEquals(1, mailer.inbox(email.getEmailAddress()).size()); // Notification for new type on existing app notificationsDb.setNotification(notification3.source(), notification3.type(), notification3.level(), notification3.messages()); - assertEquals(2, mailer.inbox(email).size()); + assertEquals(2, mailer.inbox(email.getEmailAddress()).size()); } @Test @@ -157,19 +158,19 @@ public class NotificationsDbTest { // No metrics, no new notification notificationsDb.setDeploymentMetricsNotifications(deploymentId, List.of()); - assertEquals(0, mailer.inbox(email).size()); + assertEquals(0, mailer.inbox(email.getEmailAddress()).size()); // Metrics that contain none of the feed block metrics does not create new notification notificationsDb.setDeploymentMetricsNotifications(deploymentId, List.of(clusterMetrics("cluster1", null, null, null, null, Map.of()))); - assertEquals(0, mailer.inbox(email).size()); + assertEquals(0, mailer.inbox(email.getEmailAddress()).size()); // One resource is at warning notificationsDb.setDeploymentMetricsNotifications(deploymentId, List.of(clusterMetrics("cluster1", 0.88, 0.9, 0.3, 0.5, Map.of()))); - assertEquals(1, mailer.inbox(email).size()); + assertEquals(1, mailer.inbox(email.getEmailAddress()).size()); // One resource over the limit notificationsDb.setDeploymentMetricsNotifications(deploymentId, List.of(clusterMetrics("cluster1", 0.95, 0.9, 0.3, 0.5, Map.of()))); - assertEquals(2, mailer.inbox(email).size()); + assertEquals(2, mailer.inbox(email.getEmailAddress()).size()); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java index 7c07192d633..c0501206494 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java @@ -13,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; import com.yahoo.vespa.hosted.controller.tenant.ArchiveAccess; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; +import com.yahoo.vespa.hosted.controller.tenant.Email; import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo; import com.yahoo.vespa.hosted.controller.tenant.TenantContacts; import com.yahoo.vespa.hosted.controller.tenant.TenantInfo; @@ -28,7 +29,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; public class NotifierTest { private static final TenantName tenant = TenantName.from("tenant1"); - private static final String email = "user1@example.com"; + private static final Email email = new Email("user1@example.com", true); private static final CloudTenant cloudTenant = new CloudTenant(tenant, Instant.now(), @@ -63,8 +64,8 @@ public class NotifierTest { List.of("test package has production tests, but no production tests are declared in deployment.xml", "see https://docs.vespa.ai/en/testing.html for details on how to write system tests for Vespa")); notifier.dispatch(notification); - assertEquals(1, mailer.inbox(email).size()); - var mail = mailer.inbox(email).get(0); + assertEquals(1, mailer.inbox(email.getEmailAddress()).size()); + var mail = mailer.inbox(email.getEmailAddress()).get(0); assertEquals("[WARNING] Test package Vespa Notification for tenant1.default.default", mail.subject()); assertEquals(""" diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializerTest.java new file mode 100644 index 00000000000..69c6e13ba62 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/MailVerificationSerializerTest.java @@ -0,0 +1,31 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.persistence; + +import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; +import org.junit.jupiter.api.Test; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * @author olaa + */ +public class MailVerificationSerializerTest { + + @Test + public void test_serialization() { + var original = new PendingMailVerification(TenantName.from("test-tenant"), + "email@mycomp.any", + "xyz-123", + Instant.now().truncatedTo(ChronoUnit.MILLIS), + PendingMailVerification.MailType.TENANT_CONTACT + ); + + var serialized = MailVerificationSerializer.toSlime(original); + var deserialized = MailVerificationSerializer.fromSlime(serialized); + assertEquals(original, deserialized); + } +}
\ No newline at end of file diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java index 636620acf07..5144c5cb7b4 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/TenantSerializerTest.java @@ -17,6 +17,7 @@ import com.yahoo.vespa.hosted.controller.tenant.ArchiveAccess; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.CloudTenant; import com.yahoo.vespa.hosted.controller.tenant.DeletedTenant; +import com.yahoo.vespa.hosted.controller.tenant.Email; import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo; import com.yahoo.vespa.hosted.controller.tenant.TenantAddress; import com.yahoo.vespa.hosted.controller.tenant.TenantBilling; @@ -190,7 +191,7 @@ public class TenantSerializerTest { Slime slime = new Slime(); Cursor parentObject = slime.setObject(); serializer.toSlime(partialInfo, parentObject); - assertEquals("{\"info\":{\"name\":\"\",\"email\":\"\",\"website\":\"\",\"contactName\":\"\",\"contactEmail\":\"\",\"address\":{\"addressLines\":\"\",\"postalCodeOrZip\":\"\",\"city\":\"Hønefoss\",\"stateRegionProvince\":\"\",\"country\":\"\"}}}", slime.toString()); + assertEquals("{\"info\":{\"name\":\"\",\"email\":\"\",\"website\":\"\",\"contactName\":\"\",\"contactEmail\":\"\",\"contactEmailVerified\":true,\"address\":{\"addressLines\":\"\",\"postalCodeOrZip\":\"\",\"city\":\"Hønefoss\",\"stateRegionProvince\":\"\",\"country\":\"\"}}}", slime.toString()); } @Test @@ -199,7 +200,7 @@ public class TenantSerializerTest { .withName("My Company") .withEmail("email@mycomp.any") .withWebsite("http://mycomp.any") - .withContact(TenantContact.from("My Name", "ceo@mycomp.any")) + .withContact(TenantContact.from("My Name", new Email("ceo@mycomp.any", true))) .withAddress(TenantAddress.empty() .withCity("Hønefoss") .withAddress("Riperbakken 2") @@ -207,7 +208,7 @@ public class TenantSerializerTest { .withCode("3510") .withRegion("Viken")) .withBilling(TenantBilling.empty() - .withContact(TenantContact.from("Thomas The Tank Engine", "thomas@sodor.com", "NA")) + .withContact(TenantContact.from("Thomas The Tank Engine", new Email("ceo@mycomp.any", true), "NA")) .withAddress(TenantAddress.empty() .withCity("Suddery") .withCountry("Sodor") @@ -226,8 +227,8 @@ public class TenantSerializerTest { void cloud_tenant_with_tenant_info_contacts() { TenantInfo tenantInfo = TenantInfo.empty() .withContacts(new TenantContacts(List.of( - new TenantContacts.EmailContact(List.of(TenantContacts.Audience.TENANT), "email1@email.com"), - new TenantContacts.EmailContact(List.of(TenantContacts.Audience.TENANT, TenantContacts.Audience.NOTIFICATIONS), "email2@email.com")))); + new TenantContacts.EmailContact(List.of(TenantContacts.Audience.TENANT), new Email("email1@email.com", true)), + new TenantContacts.EmailContact(List.of(TenantContacts.Audience.TENANT, TenantContacts.Audience.NOTIFICATIONS), new Email("email2@email.com", true))))); Slime slime = new Slime(); Cursor parentCursor = slime.setObject(); serializer.toSlime(tenantInfo, parentCursor); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java index af0a85f1a90..4a816ebeee9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java @@ -90,7 +90,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { .roles(Set.of(Role.administrator(tenantName))); tester.assertResponse(updateRequest, "{\"message\":\"Tenant info updated\"}", 200); - tester.assertResponse(request, "{\"contact\":{\"name\":\"Some Name\",\"email\":\"foo@example.com\"},\"tenant\":{\"company\":\"\",\"website\":\"https://example.com/\"}}", 200); + tester.assertResponse(request, "{\"contact\":{\"name\":\"Some Name\",\"email\":\"foo@example.com\",\"emailVerified\":false},\"tenant\":{\"company\":\"\",\"website\":\"https://example.com/\"}}", 200); } @Test @@ -117,7 +117,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { tester.assertResponse(request, "{\"contacts\":[]}", 200); - var fullContacts = "{\"contacts\":[{\"audiences\":[\"tenant\"],\"email\":\"contact1@example.com\"},{\"audiences\":[\"notifications\"],\"email\":\"contact2@example.com\"},{\"audiences\":[\"tenant\",\"notifications\"],\"email\":\"contact3@example.com\"}]}"; + var fullContacts = "{\"contacts\":[{\"audiences\":[\"tenant\"],\"email\":\"contact1@example.com\",\"emailVerified\":false},{\"audiences\":[\"notifications\"],\"email\":\"contact2@example.com\",\"emailVerified\":false},{\"audiences\":[\"tenant\",\"notifications\"],\"email\":\"contact3@example.com\",\"emailVerified\":false}]}"; var updateRequest = request("/application/v4/tenant/scoober/info/contacts", PUT) .data(fullContacts) .roles(Set.of(Role.administrator(tenantName))); @@ -147,12 +147,12 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { tester.assertResponse(postPartialContacts, "{\"message\":\"Tenant info updated\"}", 200); // Read back the updated info - tester.assertResponse(infoRequest, "{\"name\":\"\",\"email\":\"\",\"website\":\"\",\"contactName\":\"newName\",\"contactEmail\":\"foo@example.com\",\"billingContact\":{\"name\":\"billingName\",\"email\":\"\",\"phone\":\"\"},\"contacts\":[{\"audiences\":[\"tenant\"],\"email\":\"contact1@example.com\"}]}", 200); + tester.assertResponse(infoRequest, "{\"name\":\"\",\"email\":\"\",\"website\":\"\",\"contactName\":\"newName\",\"contactEmail\":\"foo@example.com\",\"contactEmailVerified\":false,\"billingContact\":{\"name\":\"billingName\",\"email\":\"\",\"phone\":\"\"},\"contacts\":[{\"audiences\":[\"tenant\"],\"email\":\"contact1@example.com\",\"emailVerified\":false}]}", 200); String fullAddress = "{\"addressLines\":\"addressLines\",\"postalCodeOrZip\":\"postalCodeOrZip\",\"city\":\"city\",\"stateRegionProvince\":\"stateRegionProvince\",\"country\":\"country\"}"; String fullBillingContact = "{\"name\":\"name\",\"email\":\"foo@example\",\"phone\":\"phone\",\"address\":" + fullAddress + "}"; - String fullContacts = "[{\"audiences\":[\"tenant\"],\"email\":\"contact1@example.com\"},{\"audiences\":[\"notifications\"],\"email\":\"contact2@example.com\"},{\"audiences\":[\"tenant\",\"notifications\"],\"email\":\"contact3@example.com\"}]"; - String fullInfo = "{\"name\":\"name\",\"email\":\"foo@example\",\"website\":\"https://yahoo.com\",\"contactName\":\"contactName\",\"contactEmail\":\"contact@example.com\",\"address\":" + fullAddress + ",\"billingContact\":" + fullBillingContact + ",\"contacts\":" + fullContacts + "}"; + String fullContacts = "[{\"audiences\":[\"tenant\"],\"email\":\"contact1@example.com\",\"emailVerified\":false},{\"audiences\":[\"notifications\"],\"email\":\"contact2@example.com\",\"emailVerified\":false},{\"audiences\":[\"tenant\",\"notifications\"],\"email\":\"contact3@example.com\",\"emailVerified\":false}]"; + String fullInfo = "{\"name\":\"name\",\"email\":\"foo@example\",\"website\":\"https://yahoo.com\",\"contactName\":\"contactName\",\"contactEmail\":\"contact@example.com\",\"contactEmailVerified\":false,\"address\":" + fullAddress + ",\"billingContact\":" + fullBillingContact + ",\"contacts\":" + fullContacts + "}"; // Now set all fields var postFull = @@ -163,6 +163,20 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { // Now compare the updated info with the full info we sent tester.assertResponse(infoRequest, fullInfo, 200); + + var invalidBody = "{\"mail\":\"contact1@example.com\", \"mailType\":\"blurb\"}"; + var resendMailRequest = + request("/application/v4/tenant/scoober/info/resend-mail-verification", PUT) + .data(invalidBody) + .roles(Set.of(Role.administrator(tenantName))); + tester.assertResponse(resendMailRequest, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Unknown mail type blurb\"}", 400); + + var resendMailBody = "{\"mail\":\"contact1@example.com\", \"mailType\":\"notifications\"}"; + resendMailRequest = + request("/application/v4/tenant/scoober/info/resend-mail-verification", PUT) + .data(resendMailBody) + .roles(Set.of(Role.administrator(tenantName))); + tester.assertResponse(resendMailRequest, "{\"message\":\"Re-sent verification mail to contact1@example.com\"}", 200); } @Test @@ -185,13 +199,13 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { var missingEmailResponse = request("/application/v4/tenant/scoober/info", PUT) .data(partialInfoMissingEmail) .roles(Set.of(Role.administrator(tenantName))); - tester.assertResponse(missingEmailResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"'contactEmail' cannot be empty\"}", 400); + tester.assertResponse(missingEmailResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid email address\"}", 400); var partialInfoBadEmail = "{\"contactName\": \"Scoober Rentals Inc.\", \"contactEmail\": \"somethingweird\"}"; var badEmailResponse = request("/application/v4/tenant/scoober/info", PUT) .data(partialInfoBadEmail) .roles(Set.of(Role.administrator(tenantName))); - tester.assertResponse(badEmailResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"'contactEmail' needs to be an email address\"}", 400); + tester.assertResponse(badEmailResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid email address\"}", 400); var invalidWebsite = "{\"contactName\": \"Scoober Rentals Inc.\", \"contactEmail\": \"email@scoober.com\", \"website\": \"scoober\" }"; var badWebsiteResponse = request("/application/v4/tenant/scoober/info", PUT) @@ -231,7 +245,7 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { var contactsWithInvalidEmailResponse = request("/application/v4/tenant/scoober/info", PUT) .data(contactsWithInvalidEmail) .roles(Set.of(Role.administrator(tenantName))); - tester.assertResponse(contactsWithInvalidEmailResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"'email' needs to be an email address\"}", 400); + tester.assertResponse(contactsWithInvalidEmailResponse, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Invalid email address\"}", 400); // duplicate contact is not allowed var contactsWithDuplicateEmail = "{\"contacts\": [{\"audiences\": [\"tenant\"],\"email\": \"contact1@email.com\"}, {\"audiences\": [\"tenant\"],\"email\": \"contact1@email.com\"}]}"; @@ -331,7 +345,6 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest { "\"error-code\":\"BAD_REQUEST\"," + "\"message\":\"Invalid application id\"" + "}", 400); - } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java index b573940d150..075e001655f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/UserApiTest.java @@ -10,11 +10,11 @@ import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.hosted.controller.ControllerTester; import com.yahoo.vespa.hosted.controller.api.integration.billing.PlanId; import com.yahoo.jdisc.http.filter.security.misc.User; -import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockUserManagement; import com.yahoo.vespa.hosted.controller.api.integration.user.UserId; import com.yahoo.vespa.hosted.controller.api.role.Role; import com.yahoo.vespa.hosted.controller.restapi.ContainerTester; import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerCloudTest; +import com.yahoo.vespa.hosted.controller.tenant.PendingMailVerification; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import org.junit.jupiter.api.Test; @@ -321,4 +321,20 @@ public class UserApiTest extends ControllerContainerCloudTest { } } + + @Test + public void verifyMail() { + var tester = new ContainerTester(container, responseFiles); + var controller = new ControllerTester(tester); + controller.createTenant("tenant1", Tenant.Type.cloud); + var pendingMailVerification = tester.controller().mailVerifier().sendMailVerification(TenantName.from("tenant1"), "foo@bar.com", PendingMailVerification.MailType.NOTIFICATIONS); + + tester.assertResponse(request("/user/v1/email/verify", POST) + .data("{\"verificationCode\":\"blurb\"}"), + "{\"error-code\":\"NOT_FOUND\",\"message\":\"No pending email verification with code blurb found\"}", 404); + + tester.assertResponse(request("/user/v1/email/verify", POST) + .data("{\"verificationCode\":\"" + pendingMailVerification.getVerificationCode() + "\"}"), + "{\"message\":\"Email with verification code " + pendingMailVerification.getVerificationCode() + " has been verified\"}", 200); + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-info-after-created.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-info-after-created.json index 2a33f35545c..6702eff8dde 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-info-after-created.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/user/responses/tenant-info-after-created.json @@ -4,5 +4,6 @@ "website": "", "contactName": "administrator", "contactEmail": "administrator@tenant", + "contactEmailVerified": true, "contacts": [ ] } diff --git a/defaults/src/vespa/defaults.h b/defaults/src/vespa/defaults.h index 19720caaeb8..4023a77c07b 100644 --- a/defaults/src/vespa/defaults.h +++ b/defaults/src/vespa/defaults.h @@ -17,11 +17,11 @@ public: static void bootstrap(const char *argv0); /** - * Compute the path prefix where Vespa files will live; - * the return value ends with a "/" so you can just append - * the relative pathname to the file(s) you want. + * Compute the path prefix where Vespa files will live. + * Note the return value won't end with a "/" - use + * underVespaHome() to construct sub-directory paths. * - * @return the vespa home directory, ending by "/" + * @return the vespa home directory **/ static const char *vespaHome(); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/file/FlagDbFile.java b/flags/src/main/java/com/yahoo/vespa/flags/file/FlagDbFile.java index 0b931aa7fa6..4cd33c968b6 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/file/FlagDbFile.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/file/FlagDbFile.java @@ -42,7 +42,7 @@ public class FlagDbFile { } public FlagDbFile(FileSystem fileSystem) { - this(fileSystem.getPath(Defaults.getDefaults().vespaHome() + "/var/vespa/flag.db")); + this(fileSystem.getPath(Defaults.getDefaults().underVespaHome("var/vespa/flag.db"))); } public FlagDbFile(Path path) { diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetriever.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetriever.java index 900f7df6ea2..05b146bbacd 100644 --- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetriever.java +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetriever.java @@ -161,12 +161,13 @@ public class ApplicationMetricsRetriever extends AbstractComponent implements Ru int numOk = 0; int numTried = futures.size(); for (Map.Entry<Node, Future<Boolean>> entry : futures.entrySet()) { + if (stopped) break; try { Boolean result = entry.getValue().get(taskTimeout.get().toMillis(), TimeUnit.MILLISECONDS); - if ((result != null) && result) numOk++; + if (result == Boolean.TRUE) numOk++; } catch (InterruptedException | ExecutionException | TimeoutException e) { Throwable cause = e.getCause(); - if ( e instanceof ExecutionException && ((cause instanceof SocketException) || cause instanceof ConnectTimeoutException)) { + if (e instanceof ExecutionException && ((cause instanceof SocketException) || cause instanceof ConnectTimeoutException)) { log.log(Level.FINE, "Failed retrieving metrics for '" + entry.getKey() + "' : " + cause.getMessage()); } else { log.log(Level.WARNING, "Failed retrieving metrics for '" + entry.getKey() + "' : ", e); diff --git a/parent/pom.xml b/parent/pom.xml index 068dc5579b2..62d57660bcf 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -821,9 +821,9 @@ <version>${eclipse-collections.version}</version> </dependency> <dependency> - <groupId>org.eclipse.jetty.http2</groupId> - <artifactId>http2-common</artifactId> - <version>${jetty.version}</version> + <groupId>org.eclipse.jetty.alpn</groupId> + <artifactId>alpn-api</artifactId> + <version>${jetty-alpn.version}</version> </dependency> <dependency> <groupId>org.eclipse.jetty.http2</groupId> @@ -837,37 +837,32 @@ </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-alpn-server</artifactId> - <version>${jetty.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-client</artifactId> <version>${jetty.version}</version> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-http</artifactId> + <artifactId>jetty-continuation</artifactId> <version>${jetty.version}</version> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-io</artifactId> + <artifactId>jetty-server</artifactId> <version>${jetty.version}</version> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-jmx</artifactId> + <artifactId>jetty-servlet</artifactId> <version>${jetty.version}</version> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-server</artifactId> + <artifactId>jetty-servlets</artifactId> <version>${jetty.version}</version> </dependency> <dependency> <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-servlet</artifactId> + <artifactId>jetty-jmx</artifactId> <version>${jetty.version}</version> </dependency> <dependency> @@ -876,11 +871,6 @@ <version>${jetty.version}</version> </dependency> <dependency> - <groupId>org.eclipse.jetty.toolchain</groupId> - <artifactId>jetty-jakarta-servlet-api</artifactId> - <version>${jetty-servlet-api.version}</version> - </dependency> - <dependency> <groupId>org.glassfish.jaxb</groupId> <artifactId>jaxb-runtime</artifactId> <version>2.3.2</version> <!-- 2.3.3 has a BROKEN manifest --> @@ -1043,8 +1033,8 @@ <felix.log.version>1.0.1</felix.log.version> <findbugs.version>3.0.2</findbugs.version> <!-- Should be kept in sync with guava --> <hdrhistogram.version>2.1.12</hdrhistogram.version> - <jetty.version>11.0.12</jetty.version> - <jetty-servlet-api.version>5.0.2</jetty-servlet-api.version> + <jetty.version>9.4.49.v20220914</jetty.version> + <jetty-alpn.version>1.1.3.v20160715</jetty-alpn.version> <jna.version>5.11.0</jna.version> <junit.version>5.8.1</junit.version> <maven-archiver.version>3.5.2</maven-archiver.version> diff --git a/searchlib/src/tests/attribute/searchable/CMakeLists.txt b/searchlib/src/tests/attribute/searchable/CMakeLists.txt index 29a4c122bf7..c3af20cc673 100644 --- a/searchlib/src/tests/attribute/searchable/CMakeLists.txt +++ b/searchlib/src/tests/attribute/searchable/CMakeLists.txt @@ -19,6 +19,7 @@ vespa_add_executable(searchlib_attribute_blueprint_test_app TEST attributeblueprint_test.cpp DEPENDS searchlib + searchlib_test GTest::GTest ) vespa_add_test(NAME searchlib_attribute_blueprint_test_app COMMAND searchlib_attribute_blueprint_test_app) diff --git a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp index 29dbf33d29c..b211261ef57 100644 --- a/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attributeblueprint_test.cpp @@ -1,10 +1,15 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/eval/eval/tensor_spec.h> +#include <vespa/eval/eval/value.h> +#include <vespa/eval/eval/value_codec.h> +#include <vespa/searchcommon/attribute/config.h> +#include <vespa/searchcommon/attribute/iattributecontext.h> +#include <vespa/searchlib/attribute/attribute.h> #include <vespa/searchlib/attribute/attribute_blueprint_factory.h> #include <vespa/searchlib/attribute/attribute_read_guard.h> #include <vespa/searchlib/attribute/attributecontext.h> #include <vespa/searchlib/attribute/attributefactory.h> -#include <vespa/searchlib/attribute/attribute.h> #include <vespa/searchlib/fef/matchdata.h> #include <vespa/searchlib/query/tree/location.h> #include <vespa/searchlib/query/tree/point.h> @@ -14,11 +19,7 @@ #include <vespa/searchlib/queryeval/leaf_blueprints.h> #include <vespa/searchlib/queryeval/nearest_neighbor_blueprint.h> #include <vespa/searchlib/tensor/dense_tensor_attribute.h> -#include <vespa/searchcommon/attribute/iattributecontext.h> -#include <vespa/searchcommon/attribute/config.h> -#include <vespa/eval/eval/tensor_spec.h> -#include <vespa/eval/eval/value.h> -#include <vespa/eval/eval/value_codec.h> +#include <vespa/searchlib/test/attribute_builder.h> #include <vespa/vespalib/gtest/gtest.h> #include <vespa/log/log.h> @@ -28,14 +29,18 @@ using search::AttributeGuard; using search::AttributeVector; using search::IAttributeManager; using search::attribute::IAttributeContext; +using search::attribute::test::AttributeBuilder; using search::fef::MatchData; using search::fef::TermFieldMatchData; using search::query::Location; using search::query::Node; using search::query::Point; +using search::query::SimpleDotProduct; using search::query::SimpleLocationTerm; using search::query::SimplePrefixTerm; using search::query::SimpleStringTerm; +using search::query::SimpleWandTerm; +using search::query::SimpleWeightedSetTerm; using search::query::Weight; using search::queryeval::Blueprint; using search::queryeval::EmptyBlueprint; @@ -44,6 +49,7 @@ using search::queryeval::FieldSpec; using search::queryeval::FilterWrapper; using search::queryeval::NearestNeighborBlueprint; using search::queryeval::SearchIterator; +using search::queryeval::SimpleResult; using std::string; using std::vector; using vespalib::eval::TensorSpec; @@ -100,7 +106,7 @@ public: } }; -constexpr uint32_t DOCID_LIMIT = 3; +constexpr uint32_t DOCID_LIMIT = 4; bool do_search(const Node &node, IAttributeManager &attribute_manager, bool expect_attribute_search_context = true) @@ -112,7 +118,7 @@ do_search(const Node &node, IAttributeManager &attribute_manager, bool expect_at Blueprint::UP result = source.createBlueprint(requestContext, FieldSpec(field, 0, 0), node); assert(result.get()); EXPECT_TRUE(!result->getState().estimate().empty); - EXPECT_EQ(3u, result->getState().estimate().estHits); + EXPECT_EQ(DOCID_LIMIT, result->getState().estimate().estHits); if (expect_attribute_search_context) { EXPECT_TRUE(result->get_attribute_search_context() != nullptr); } else { @@ -122,9 +128,10 @@ do_search(const Node &node, IAttributeManager &attribute_manager, bool expect_at result->setDocIdLimit(DOCID_LIMIT); SearchIterator::UP iterator = result->createSearch(*md, true); assert((bool)iterator); - iterator->initRange(1, 3); + iterator->initRange(1, DOCID_LIMIT); EXPECT_TRUE(!iterator->seek(1)); - return iterator->seek(2); + EXPECT_TRUE(!iterator->seek(2)); + return iterator->seek(3); } bool @@ -144,73 +151,33 @@ downcast(ParentType& parent) return *result; } -struct StringAttributeFiller { - using ValueType = vespalib::string; - static void add(AttributeVector& attr, const vespalib::string& value) { - auto& real = downcast<StringAttribute>(attr); - real.update(attr.getNumDocs() - 1, value); - real.commit(); - } -}; - -struct WsetStringAttributeFiller { - using ValueType = vespalib::string; - static void add(AttributeVector& attr, const vespalib::string& value) { - auto& real = downcast<StringAttribute>(attr); - uint32_t docid = attr.getNumDocs() - 1; - real.append(docid, value, 1); - real.commit(); - } -}; - -struct IntegerAttributeFiller { - using ValueType = int64_t; - static void add(AttributeVector& attr, int64_t value) { - auto& real = downcast<IntegerAttribute>(attr); - real.update(attr.getNumDocs() - 1, value); - real.commit(); - } -}; - -template <typename FillerType> -void -fill(AttributeVector& attr, typename FillerType::ValueType value) +AttributeVector::SP +make_string_attribute(const std::vector<vespalib::string>& values) { - AttributeVector::DocId docid; - attr.addDoc(docid); - attr.addDoc(docid); - attr.addDoc(docid); - assert(DOCID_LIMIT-1 == docid); - FillerType::add(attr, value); + Config cfg(BasicType::STRING, CollectionType::SINGLE); + return AttributeBuilder(field, cfg).fill(values).get(); } AttributeVector::SP make_string_attribute(const std::string& value) { - Config cfg(BasicType::STRING, CollectionType::SINGLE); - auto attr = AttributeFactory::createAttribute(field, cfg); - fill<StringAttributeFiller>(*attr, value); - return attr; + return make_string_attribute({"", "", value}); } AttributeVector::SP -make_wset_string_attribute(const std::string& value) +make_wset_string_attribute(const std::vector<std::vector<vespalib::string>>& values) { Config cfg(BasicType::STRING, CollectionType::WSET); // fast-search is needed to trigger use of DirectAttributeBlueprint. cfg.setFastSearch(true); - auto attr = AttributeFactory::createAttribute(field, cfg); - fill<WsetStringAttributeFiller>(*attr, value); - return attr; + return AttributeBuilder(field, cfg).fill_array(values).get(); } AttributeVector::SP make_int_attribute(int64_t value) { Config cfg(BasicType::INT32, CollectionType::SINGLE); - auto attr = AttributeFactory::createAttribute(field, cfg); - fill<IntegerAttributeFiller>(*attr, value); - return attr; + return AttributeBuilder(field, cfg).fill({-1, -1, value}).get(); } AttributeVector::SP @@ -218,9 +185,7 @@ make_fast_search_long_attribute(int64_t value) { Config cfg(BasicType::fromType(int64_t()), CollectionType::SINGLE); cfg.setFastSearch(true); - auto attr = AttributeFactory::createAttribute(field, cfg); - fill<IntegerAttributeFiller>(*attr, value); - return attr; + return AttributeBuilder(field, cfg).fill({-1, -1, value}).get(); } MyAttributeManager @@ -322,17 +287,21 @@ make_int_attribute(const vespalib::string& name) return AttributeFactory::createAttribute(name, cfg); } +using BFC = Blueprint::FilterConstraint; + class BlueprintFactoryFixture { public: + AttributeVector::SP attr; MyAttributeManager mgr; vespalib::string attr_name; AttributeContext attr_ctx; FakeRequestContext request_ctx; AttributeBlueprintFactory source; - BlueprintFactoryFixture(AttributeVector::SP attr) - : mgr(attr), - attr_name(attr->getName()), + BlueprintFactoryFixture(AttributeVector::SP attr_in) + : attr(attr_in), + mgr(attr_in), + attr_name(attr_in->getName()), attr_ctx(mgr), request_ctx(&attr_ctx), source() @@ -345,12 +314,30 @@ public: result->setDocIdLimit(DOCID_LIMIT); return result; } + void expect_document_weight_attribute() { + EXPECT_TRUE(attr->asDocumentWeightAttribute() != nullptr); + } + void expect_filter_search(const SimpleResult& upper_and_lower, const Node& term) { + expect_filter_search(upper_and_lower, upper_and_lower, term); + } + void expect_filter_search(const SimpleResult& upper, const SimpleResult& lower, const Node& term) { + auto blueprint = create_blueprint(term); + auto upper_itr = blueprint->createFilterSearch(true, BFC::UPPER_BOUND); + auto lower_itr = blueprint->createFilterSearch(true, BFC::LOWER_BOUND); + EXPECT_EQ(upper, SimpleResult().search(*upper_itr, DOCID_LIMIT)); + EXPECT_EQ(lower, SimpleResult().search(*lower_itr, DOCID_LIMIT)); + } + void expect_filter_wrapper(const Node& term) { + auto blueprint = create_blueprint(term); + auto itr = blueprint->createFilterSearch(true, BFC::UPPER_BOUND); + downcast<FilterWrapper>(*itr); + } }; class NearestNeighborFixture : public BlueprintFactoryFixture { public: - NearestNeighborFixture(AttributeVector::SP attr) - : BlueprintFactoryFixture(std::move(attr)) + NearestNeighborFixture(AttributeVector::SP attr_in) + : BlueprintFactoryFixture(std::move(attr_in)) { } ~NearestNeighborFixture() {} @@ -422,30 +409,60 @@ TEST(AttributeBlueprintTest, empty_blueprint_is_created_when_nearest_neighbor_te expect_empty_blueprint(make_tensor_attribute(field, "tensor(x[2])"), dense_x_3); // tensor types are not same size } -TEST(AttributeBlueprintTest, attribute_field_blueprint_wraps_filter_search_iterator) +TEST(AttributeBlueprintTest, attribute_field_blueprint_creates_exact_filter_search) { - BlueprintFactoryFixture f(make_string_attribute("foo")); + BlueprintFactoryFixture f(make_string_attribute({"foo", "x", "foo"})); SimpleStringTerm term("foo", field, 0, Weight(0)); - auto blueprint = f.create_blueprint(term); - - auto itr = blueprint->createFilterSearch(true, Blueprint::FilterConstraint::UPPER_BOUND); - auto& wrapper = downcast<FilterWrapper>(*itr); - wrapper.initRange(1, 3); - EXPECT_FALSE(wrapper.seek(1)); - EXPECT_TRUE(wrapper.seek(2)); + f.expect_filter_search(SimpleResult({1, 3}), term); + f.expect_filter_wrapper(term); } -TEST(AttributeBlueprintTest, direct_attribute_blueprint_wraps_filter_search_iterator) +TEST(AttributeBlueprintTest, direct_attribute_blueprint_creates_exact_filter_search) { - BlueprintFactoryFixture f(make_wset_string_attribute("foo")); + BlueprintFactoryFixture f(make_wset_string_attribute({{"foo"}, {}, {"foo"}})); + f.expect_document_weight_attribute(); SimpleStringTerm term("foo", field, 0, Weight(0)); - auto blueprint = f.create_blueprint(term); + f.expect_filter_search(SimpleResult({1, 3}), term); + f.expect_filter_wrapper(term); +} + +TEST(AttributeBlueprintTest, direct_wand_blueprint_creates_or_like_filter_search) +{ + BlueprintFactoryFixture f(make_wset_string_attribute({{"foo"}, {"x"}, {"bar"}})); + f.expect_document_weight_attribute(); + SimpleWandTerm term(2, field, 0, Weight(0), DOCID_LIMIT, 1000, 1.0); + term.addTerm("foo", Weight(10)); + term.addTerm("bar", Weight(20)); + f.expect_filter_search(SimpleResult({1, 3}), SimpleResult(), term); +} - auto itr = blueprint->createFilterSearch(true, Blueprint::FilterConstraint::UPPER_BOUND); - auto& wrapper = downcast<FilterWrapper>(*itr); - wrapper.initRange(1, 3); - EXPECT_FALSE(wrapper.seek(1)); - EXPECT_TRUE(wrapper.seek(2)); +TEST(AttributeBlueprintTest, direct_weighted_set_blueprint_creates_or_like_filter_search) +{ + BlueprintFactoryFixture f(make_wset_string_attribute({{"foo"}, {"x"}, {"bar"}})); + f.expect_document_weight_attribute(); + { + SimpleWeightedSetTerm term(2, field, 0, Weight(0)); + term.addTerm("foo", Weight(10)); + term.addTerm("bar", Weight(20)); + f.expect_filter_search(SimpleResult({1, 3}), term); + } + { + SimpleDotProduct term(2, field, 0, Weight(0)); + term.addTerm("foo", Weight(10)); + term.addTerm("bar", Weight(20)); + f.expect_filter_search(SimpleResult({1, 3}), term); + } +} + +TEST(AttributeBlueprintTest, attribute_weighted_set_blueprint_creates_or_like_filter_search) +{ + BlueprintFactoryFixture f(make_string_attribute({"foo", "x", "bar"})); + { + SimpleWeightedSetTerm term(2, field, 0, Weight(0)); + term.addTerm("foo", Weight(10)); + term.addTerm("bar", Weight(20)); + f.expect_filter_search(SimpleResult({1, 3}), term); + } } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/diskindex/diskindex/CMakeLists.txt b/searchlib/src/tests/diskindex/diskindex/CMakeLists.txt index 9a19c1bf1d5..7dff06507f4 100644 --- a/searchlib/src/tests/diskindex/diskindex/CMakeLists.txt +++ b/searchlib/src/tests/diskindex/diskindex/CMakeLists.txt @@ -3,7 +3,6 @@ vespa_add_executable(searchlib_diskindex_test_app TEST SOURCES diskindex_test.cpp DEPENDS - searchlib searchlib_test ) vespa_add_test(NAME searchlib_diskindex_test_app COMMAND searchlib_diskindex_test_app) diff --git a/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp b/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp index 2e8617b4dcb..d153481ef36 100644 --- a/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp +++ b/searchlib/src/tests/diskindex/diskindex/diskindex_test.cpp @@ -1,9 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/common/bitvectoriterator.h> #include <vespa/searchlib/diskindex/disktermblueprint.h> #include <vespa/searchlib/test/diskindex/testdiskindex.h> +#define ENABLE_GTEST_MIGRATION #include <vespa/searchlib/test/searchiteratorverifier.h> #include <vespa/searchlib/test/fakedata/fakeword.h> #include <vespa/searchlib/diskindex/zcposocciterators.h> @@ -12,38 +12,43 @@ #include <vespa/searchlib/queryeval/leaf_blueprints.h> #include <vespa/searchlib/queryeval/emptysearch.h> #include <vespa/searchlib/queryeval/fake_requestcontext.h> +#include <vespa/searchlib/queryeval/simpleresult.h> #include <vespa/searchlib/index/dummyfileheadercontext.h> #include <vespa/searchlib/test/fakedata/fpfactory.h> +#include <vespa/vespalib/gtest/gtest.h> +#include <vespa/vespalib/stllike/asciistream.h> #include <filesystem> -#include <iostream> #include <set> +using search::BitVector; using search::BitVectorIterator; -using namespace search::fef; -using namespace search::index; -using namespace search::query; -using namespace search::queryeval; -using namespace search::queryeval::blueprint; +using search::diskindex::DiskIndex; +using search::diskindex::DiskTermBlueprint; +using search::diskindex::TestDiskIndex; +using search::diskindex::ZcRareWordPosOccIterator; +using search::fef::TermFieldMatchDataArray; +using search::index::DummyFileHeaderContext; +using search::index::PostingListHandle; +using search::index::Schema; +using search::query::SimpleStringTerm; +using search::queryeval::Blueprint; +using search::queryeval::BooleanMatchIteratorWrapper; +using search::queryeval::EmptyBlueprint; +using search::queryeval::EmptySearch; +using search::queryeval::ExecuteInfo; +using search::queryeval::FakeRequestContext; +using search::queryeval::FieldSpec; +using search::queryeval::LeafBlueprint; +using search::queryeval::SearchIterator; +using search::queryeval::SimpleResult; using search::test::SearchIteratorVerifier; -using namespace search::fakedata; +using search::fakedata::FPFactory; +using search::fakedata::FakePosting; +using search::fakedata::FakeWord; +using search::fakedata::getFPFactory; +using LookupResult = DiskIndex::LookupResult; -namespace search { -namespace diskindex { - -typedef DiskIndex::LookupResult LookupResult; - -std::string -toString(SearchIterator & sb) -{ - std::ostringstream oss; - bool first = true; - for (sb.seek(1u); ! sb.isAtEnd(); sb.seek(sb.getDocId() + 1)) { - if (!first) oss << ","; - oss << sb.getDocId(); - first = false; - } - return oss.str(); -} +namespace { SimpleStringTerm makeTerm(const std::string & term) @@ -51,23 +56,6 @@ makeTerm(const std::string & term) return SimpleStringTerm(term, "field", 0, search::query::Weight(0)); } -class Test : public vespalib::TestApp, public TestDiskIndex { -private: - FakeRequestContext _requestContext; - - void requireThatLookupIsWorking(bool fieldEmpty, bool docEmpty, bool wordEmpty); - void requireThatWeCanReadPostingList(); - void require_that_we_can_get_field_length_info(); - void requireThatWeCanReadBitVector(); - void requireThatBlueprintIsCreated(); - void requireThatBlueprintCanCreateSearchIterators(); - void requireThatSearchIteratorsConforms(); -public: - Test(); - ~Test(); - int Main() override; -}; - class Verifier : public SearchIteratorVerifier { public: Verifier(FakePosting::SP fp); @@ -96,14 +84,67 @@ Verifier::Verifier(FakePosting::SP fp) Verifier::~Verifier() = default; +struct EmptySettings +{ + bool _empty_field; + bool _empty_doc; + bool _empty_word; + EmptySettings() + : _empty_field(false), + _empty_doc(false), + _empty_word(false) + { + } + EmptySettings empty_field() && { _empty_field = true; return *this; } + EmptySettings empty_doc() && { _empty_doc = true; return *this; } + EmptySettings empty_word() && { _empty_word = true; return *this; } +}; + +struct IOSettings +{ + bool _use_directio; + bool _use_mmap; + IOSettings() + : _use_directio(false), + _use_mmap(false) + { + } + IOSettings use_directio() && { _use_directio = true; return *this; } + IOSettings use_mmap() && { _use_mmap = true; return *this; } +}; + +class DiskIndexTest : public ::testing::Test, public TestDiskIndex { +private: + FakeRequestContext _requestContext; + +protected: + void requireThatLookupIsWorking(const EmptySettings& empty_settings); + void requireThatWeCanReadPostingList(); + void require_that_we_can_get_field_length_info(); + void requireThatWeCanReadBitVector(); + void requireThatBlueprintIsCreated(); + void requireThatBlueprintCanCreateSearchIterators(); + void requireThatSearchIteratorsConforms(); + void build_index(const IOSettings& io_settings, const EmptySettings& empty_settings); + void test_empty_settings(const EmptySettings& empty_settings); + void test_io_settings(const IOSettings& io_settings); +public: + DiskIndexTest(); + ~DiskIndexTest(); +}; + +DiskIndexTest::DiskIndexTest() = default; + +DiskIndexTest::~DiskIndexTest() = default; + void -Test::requireThatSearchIteratorsConforms() +DiskIndexTest::requireThatSearchIteratorsConforms() { FakePosting::SP tmp; Verifier verTmp(tmp); Schema schema; schema.addIndexField(Schema::IndexField("a", Schema::DataType::STRING)); - bitcompression::PosOccFieldsParams params; + search::bitcompression::PosOccFieldsParams params; params.setSchemaParams(schema, 0); search::fakedata::FakeWord fw(verTmp.getDocIdLimit(), verTmp.getExpectedDocIds(), "a", params, 0); TermFieldMatchData md; @@ -118,22 +159,23 @@ Test::requireThatSearchIteratorsConforms() "EGCompr64FilterOcc", "EGCompr64LEFilterOcc", "EGCompr64NoSkipFilterOcc", "EGCompr64SkipFilterOcc" }; for (auto postingType : search::fakedata::getPostingTypes()) { + SCOPED_TRACE(postingType); if (ignored.find(postingType) == ignored.end()) { - std::cerr << "Verifying " << postingType << std::endl; std::unique_ptr<FPFactory> ff(getFPFactory(postingType, schema)); ff->setup(v); FakePosting::SP f(ff->make(fw)); Verifier verifier(f); - TEST_DO(verifier.verify()); + verifier.verify(); } } } void -Test::requireThatLookupIsWorking(bool fieldEmpty, - bool docEmpty, - bool wordEmpty) +DiskIndexTest::requireThatLookupIsWorking(const EmptySettings& empty_settings) { + auto fieldEmpty = empty_settings._empty_field; + auto docEmpty = empty_settings._empty_doc; + auto wordEmpty = empty_settings._empty_word; uint32_t f1(_schema.getIndexFieldId("f1")); uint32_t f2(_schema.getIndexFieldId("f2")); uint32_t f3(_schema.getIndexFieldId("f3")); @@ -149,8 +191,8 @@ Test::requireThatLookupIsWorking(bool fieldEmpty, if (wordEmpty || fieldEmpty || docEmpty) { EXPECT_TRUE(!r || r->counts._numDocs == 0); } else { - EXPECT_EQUAL(1u, r->wordNum); - EXPECT_EQUAL(2u, r->counts._numDocs); + EXPECT_EQ(1u, r->wordNum); + EXPECT_EQ(2u, r->counts._numDocs); } r = _index->lookup(f1, "w2"); EXPECT_TRUE(!r || r->counts._numDocs == 0); @@ -160,15 +202,15 @@ Test::requireThatLookupIsWorking(bool fieldEmpty, if (wordEmpty || fieldEmpty || docEmpty) { EXPECT_TRUE(!r || r->counts._numDocs == 0); } else { - EXPECT_EQUAL(1u, r->wordNum); - EXPECT_EQUAL(3u, r->counts._numDocs); + EXPECT_EQ(1u, r->wordNum); + EXPECT_EQ(3u, r->counts._numDocs); } r = _index->lookup(f2, "w2"); if (wordEmpty || fieldEmpty || docEmpty) { EXPECT_TRUE(!r || r->counts._numDocs == 0); } else { - EXPECT_EQUAL(2u, r->wordNum); - EXPECT_EQUAL(17u, r->counts._numDocs); + EXPECT_EQ(2u, r->wordNum); + EXPECT_EQ(17u, r->counts._numDocs); } } { // field 'f3' doesn't exist @@ -180,35 +222,34 @@ Test::requireThatLookupIsWorking(bool fieldEmpty, } void -Test::requireThatWeCanReadPostingList() +DiskIndexTest::requireThatWeCanReadPostingList() { TermFieldMatchDataArray mda; { // field 'f1' LookupResult::UP r = _index->lookup(0, "w1"); PostingListHandle::UP h = _index->readPostingList(*r); SearchIterator * sb = h->createIterator(r->counts, mda); - sb->initFullRange(); - EXPECT_EQUAL("1,3", toString(*sb)); + EXPECT_EQ(SimpleResult({1,3}), SimpleResult().search(*sb)); delete sb; } } void -Test::require_that_we_can_get_field_length_info() +DiskIndexTest::require_that_we_can_get_field_length_info() { auto info = _index->get_field_length_info("f1"); - EXPECT_EQUAL(3.5, info.get_average_field_length()); - EXPECT_EQUAL(21u, info.get_num_samples()); + EXPECT_EQ(3.5, info.get_average_field_length()); + EXPECT_EQ(21u, info.get_num_samples()); info = _index->get_field_length_info("f2"); - EXPECT_EQUAL(4.0, info.get_average_field_length()); - EXPECT_EQUAL(23u, info.get_num_samples()); + EXPECT_EQ(4.0, info.get_average_field_length()); + EXPECT_EQ(23u, info.get_num_samples()); info = _index->get_field_length_info("f3"); - EXPECT_EQUAL(0.0, info.get_average_field_length()); - EXPECT_EQUAL(0u, info.get_num_samples()); + EXPECT_EQ(0.0, info.get_average_field_length()); + EXPECT_EQ(0u, info.get_num_samples()); } void -Test::requireThatWeCanReadBitVector() +DiskIndexTest::requireThatWeCanReadBitVector() { { // word 'w1' LookupResult::UP r = _index->lookup(1, "w1"); @@ -229,7 +270,7 @@ Test::requireThatWeCanReadBitVector() } void -Test::requireThatBlueprintIsCreated() +DiskIndexTest::requireThatBlueprintIsCreated() { { // unknown field Blueprint::UP b = @@ -245,7 +286,7 @@ Test::requireThatBlueprintIsCreated() Blueprint::UP b = _index->createBlueprint(_requestContext, FieldSpec("f1", 0, 0), makeTerm("w1")); EXPECT_TRUE(dynamic_cast<DiskTermBlueprint *>(b.get()) != NULL); - EXPECT_EQUAL(2u, b->getState().estimate().estHits); + EXPECT_EQ(2u, b->getState().estimate().estHits); EXPECT_TRUE(!b->getState().estimate().empty); } { // known field & word without hits @@ -254,121 +295,190 @@ Test::requireThatBlueprintIsCreated() // std::cerr << "BP = " << typeid(*b).name() << std::endl; EXPECT_TRUE((dynamic_cast<DiskTermBlueprint *>(b.get()) != NULL) || (dynamic_cast<EmptyBlueprint *>(b.get()) != NULL)); - EXPECT_EQUAL(0u, b->getState().estimate().estHits); + EXPECT_EQ(0u, b->getState().estimate().estHits); EXPECT_TRUE(b->getState().estimate().empty); } } void -Test::requireThatBlueprintCanCreateSearchIterators() +DiskIndexTest::requireThatBlueprintCanCreateSearchIterators() { TermFieldMatchData md; TermFieldMatchDataArray mda; mda.add(&md); Blueprint::UP b; SearchIterator::UP s; + SimpleResult result_f1_w1({1,3}); + SimpleResult result_f1_w2; + SimpleResult result_f2_w2({1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17}); + auto upper_bound = Blueprint::FilterConstraint::UPPER_BOUND; { // bit vector due to isFilter b = _index->createBlueprint(_requestContext, FieldSpec("f2", 0, 0, true), makeTerm("w2")); - b->fetchPostings(queryeval::ExecuteInfo::TRUE); - s = (dynamic_cast<LeafBlueprint *>(b.get()))->createLeafSearch(mda, true); + b->fetchPostings(search::queryeval::ExecuteInfo::TRUE); + auto& leaf_b = dynamic_cast<LeafBlueprint&>(*b); + s = leaf_b.createLeafSearch(mda, true); EXPECT_TRUE(dynamic_cast<BitVectorIterator *>(s.get()) != NULL); + EXPECT_EQ(result_f2_w2, SimpleResult().search(*s)); + EXPECT_EQ(result_f2_w2, SimpleResult().search(*leaf_b.createFilterSearch(true, upper_bound))); } { // bit vector due to no ranking needed b = _index->createBlueprint(_requestContext, FieldSpec("f2", 0, 0, false), makeTerm("w2")); - b->fetchPostings(queryeval::ExecuteInfo::TRUE); - s = (dynamic_cast<LeafBlueprint *>(b.get()))->createLeafSearch(mda, true); + b->fetchPostings(ExecuteInfo::TRUE); + auto& leaf_b = dynamic_cast<LeafBlueprint&>(*b); + s = leaf_b.createLeafSearch(mda, true); EXPECT_FALSE(dynamic_cast<BitVectorIterator *>(s.get()) != NULL); TermFieldMatchData md2; md2.tagAsNotNeeded(); TermFieldMatchDataArray mda2; mda2.add(&md2); EXPECT_TRUE(mda2[0]->isNotNeeded()); - s = (dynamic_cast<LeafBlueprint *>(b.get()))->createLeafSearch(mda2, false); + s = (dynamic_cast<LeafBlueprint *>(b.get()))->createLeafSearch(mda2, true); EXPECT_TRUE(dynamic_cast<BitVectorIterator *>(s.get()) != NULL); + EXPECT_EQ(result_f2_w2, SimpleResult().search(*s)); + EXPECT_EQ(result_f2_w2, SimpleResult().search(*leaf_b.createFilterSearch(true, upper_bound))); } { // fake bit vector b = _index->createBlueprint(_requestContext, FieldSpec("f1", 0, 0, true), makeTerm("w2")); // std::cerr << "BP = " << typeid(*b).name() << std::endl; - b->fetchPostings(queryeval::ExecuteInfo::TRUE); - s = (dynamic_cast<LeafBlueprint *>(b.get()))->createLeafSearch(mda, true); + b->fetchPostings(ExecuteInfo::TRUE); + auto& leaf_b = dynamic_cast<LeafBlueprint&>(*b); + s = leaf_b.createLeafSearch(mda, true); // std::cerr << "SI = " << typeid(*s).name() << std::endl; EXPECT_TRUE((dynamic_cast<BooleanMatchIteratorWrapper *>(s.get()) != NULL) || dynamic_cast<EmptySearch *>(s.get())); + EXPECT_EQ(result_f1_w2, SimpleResult().search(*s)); + EXPECT_EQ(result_f1_w2, SimpleResult().search(*leaf_b.createFilterSearch(true, upper_bound))); } { // posting list iterator b = _index->createBlueprint(_requestContext, FieldSpec("f1", 0, 0), makeTerm("w1")); - b->fetchPostings(queryeval::ExecuteInfo::TRUE); - s = (dynamic_cast<LeafBlueprint *>(b.get()))->createLeafSearch(mda, true); + b->fetchPostings(ExecuteInfo::TRUE); + auto& leaf_b = dynamic_cast<LeafBlueprint&>(*b); + s = leaf_b.createLeafSearch(mda, true); ASSERT_TRUE((dynamic_cast<ZcRareWordPosOccIterator<true, false> *>(s.get()) != NULL)); + EXPECT_EQ(result_f1_w1, SimpleResult().search(*s)); + EXPECT_EQ(result_f1_w1, SimpleResult().search(*leaf_b.createFilterSearch(true, upper_bound))); + } +} + +void +DiskIndexTest::build_index(const IOSettings& io_settings, const EmptySettings& empty_settings) +{ + vespalib::asciistream name; + int io_settings_num = 1; + if (io_settings._use_directio) { + io_settings_num += 1; + } + if (io_settings._use_mmap) { + io_settings_num += 2; } + name << "index/" << io_settings_num; + if (empty_settings._empty_field) { + name << "fe"; + } else { + buildSchema(); + } + if (empty_settings._empty_doc) { + name << "de"; + } + if (empty_settings._empty_word) { + name << "we"; + } + openIndex(name.str(), io_settings._use_directio, io_settings._use_mmap, empty_settings._empty_field, empty_settings._empty_doc, empty_settings._empty_word); } -Test::Test() = default; +void +DiskIndexTest::test_empty_settings(const EmptySettings& empty_settings) +{ + build_index(IOSettings(), empty_settings); + requireThatLookupIsWorking(empty_settings); +} -Test::~Test() = default; +void +DiskIndexTest::test_io_settings(const IOSettings& io_settings) +{ + EmptySettings empty_settings; + build_index(io_settings, empty_settings); + requireThatLookupIsWorking(empty_settings); + requireThatWeCanReadPostingList(); + require_that_we_can_get_field_length_info(); + requireThatWeCanReadBitVector(); + requireThatBlueprintIsCreated(); + requireThatBlueprintCanCreateSearchIterators(); +} -int -Test::Main() +TEST_F(DiskIndexTest, empty_settings_empty_field_empty_doc_empty_word) { - TEST_INIT("diskindex_test"); + test_empty_settings(EmptySettings().empty_field().empty_doc().empty_word()); +} - if (_argc > 0) { - DummyFileHeaderContext::setCreator(_argv[0]); - } +TEST_F(DiskIndexTest, empty_settings_empty_field_empty_doc) +{ + test_empty_settings(EmptySettings().empty_field().empty_doc()); +} - std::filesystem::create_directory(std::filesystem::path("index")); - TEST_DO(openIndex("index/1fedewe", false, false, true, true, true)); - TEST_DO(requireThatLookupIsWorking(true, true, true)); - TEST_DO(openIndex("index/1fede", false, false, true, true, false)); - TEST_DO(requireThatLookupIsWorking(true, true, false)); - TEST_DO(openIndex("index/1fewe", false, false, true, false, true)); - TEST_DO(requireThatLookupIsWorking(true, false, true)); - TEST_DO(openIndex("index/1fe", false, false, true, false, false)); - TEST_DO(requireThatLookupIsWorking(true, false, false)); - buildSchema(); - TEST_DO(openIndex("index/1dewe", false, false, false, true, true)); - TEST_DO(requireThatLookupIsWorking(false, true, true)); - TEST_DO(openIndex("index/1de", false, false, false, true, false)); - TEST_DO(requireThatLookupIsWorking(false, true, false)); - TEST_DO(openIndex("index/1we", false, false, false, false, true)); - TEST_DO(requireThatLookupIsWorking(false, false, true)); - TEST_DO(openIndex("index/1", false, false, false, false, false)); - TEST_DO(requireThatLookupIsWorking(false, false, false)); - TEST_DO(requireThatWeCanReadPostingList()); - TEST_DO(require_that_we_can_get_field_length_info()); - TEST_DO(requireThatWeCanReadBitVector()); - TEST_DO(requireThatBlueprintIsCreated()); - TEST_DO(requireThatBlueprintCanCreateSearchIterators()); - - TEST_DO(openIndex("index/2", true, false, false, false, false)); - TEST_DO(requireThatLookupIsWorking(false, false, false)); - TEST_DO(requireThatWeCanReadPostingList()); - TEST_DO(require_that_we_can_get_field_length_info()); - TEST_DO(requireThatWeCanReadBitVector()); - TEST_DO(requireThatBlueprintIsCreated()); - TEST_DO(requireThatBlueprintCanCreateSearchIterators()); - - TEST_DO(openIndex("index/3", false, true, false, false, false)); - TEST_DO(requireThatLookupIsWorking(false, false, false)); - TEST_DO(requireThatWeCanReadPostingList()); - TEST_DO(require_that_we_can_get_field_length_info()); - TEST_DO(requireThatWeCanReadBitVector()); - TEST_DO(requireThatBlueprintIsCreated()); - TEST_DO(requireThatBlueprintCanCreateSearchIterators()); - - TEST_DO(openIndex("index/4", true, true, false, false, false)); - TEST_DO(requireThatLookupIsWorking(false, false, false)); - TEST_DO(requireThatWeCanReadPostingList()); - TEST_DO(require_that_we_can_get_field_length_info()); - TEST_DO(requireThatWeCanReadBitVector()); - TEST_DO(requireThatBlueprintIsCreated()); - TEST_DO(requireThatBlueprintCanCreateSearchIterators()); - TEST_DO(requireThatSearchIteratorsConforms()); - - TEST_DONE(); +TEST_F(DiskIndexTest, empty_settings_empty_field_empty_word) +{ + test_empty_settings(EmptySettings().empty_field().empty_word()); } +TEST_F(DiskIndexTest, empty_settings_empty_field) +{ + test_empty_settings(EmptySettings().empty_field()); } + +TEST_F(DiskIndexTest, empty_settings_empty_doc_empty_word) +{ + test_empty_settings(EmptySettings().empty_doc().empty_word()); } -TEST_APPHOOK(search::diskindex::Test); +TEST_F(DiskIndexTest, empty_settings_empty_doc) +{ + test_empty_settings(EmptySettings().empty_doc()); +} + +TEST_F(DiskIndexTest, empty_settings_empty_word) +{ + test_empty_settings(EmptySettings().empty_word()); +} + +TEST_F(DiskIndexTest, io_settings_normal) +{ + test_io_settings(IOSettings()); +} + +TEST_F(DiskIndexTest, io_settings_directio) +{ + test_io_settings(IOSettings().use_directio()); +} + +TEST_F(DiskIndexTest, io_settings_mmap) +{ + test_io_settings(IOSettings().use_mmap()); +} + +TEST_F(DiskIndexTest, io_settings_directio_mmap) +{ + test_io_settings(IOSettings().use_directio().use_mmap()); +} + +TEST_F(DiskIndexTest, search_iterators_conformance) +{ + requireThatSearchIteratorsConforms(); +} + +} + +int +main(int argc, char* argv[]) +{ + if (argc > 0) { + DummyFileHeaderContext::setCreator(argv[0]); + } + ::testing::InitGoogleTest(&argc, argv); + std::filesystem::path index_path("index"); + std::filesystem::remove_all(index_path); + std::filesystem::create_directory(index_path); + auto rval = RUN_ALL_TESTS(); + std::filesystem::remove_all(index_path); + return rval; +} diff --git a/searchlib/src/vespa/searchlib/test/CMakeLists.txt b/searchlib/src/vespa/searchlib/test/CMakeLists.txt index b53b3097850..7decdb992e6 100644 --- a/searchlib/src/vespa/searchlib/test/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/test/CMakeLists.txt @@ -1,6 +1,7 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. vespa_add_library(searchlib_test SOURCES + attribute_builder.cpp document_weight_attribute_helper.cpp doc_builder.cpp imported_attribute_fixture.cpp diff --git a/searchlib/src/vespa/searchlib/test/attribute_builder.cpp b/searchlib/src/vespa/searchlib/test/attribute_builder.cpp new file mode 100644 index 00000000000..b804723d3be --- /dev/null +++ b/searchlib/src/vespa/searchlib/test/attribute_builder.cpp @@ -0,0 +1,125 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "attribute_builder.h" +#include <vespa/searchlib/attribute/attributefactory.h> +#include <vespa/searchlib/attribute/attributevector.h> +#include <vespa/searchlib/attribute/integerbase.h> +#include <vespa/searchlib/attribute/stringbase.h> +#include <cassert> + +namespace search::attribute::test { + +AttributeBuilder::AttributeBuilder(const vespalib::string& name, const Config& cfg) + : _attr_ptr(AttributeFactory::createAttribute(name, cfg)), + _attr(*_attr_ptr) +{ +} + +AttributeBuilder::AttributeBuilder(AttributeVector& attr) + : _attr_ptr(), + _attr(attr) +{ +} + +namespace { + +void +add_docs(AttributeVector& attr, size_t num_docs) +{ + attr.addReservedDoc(); + attr.addDocs(num_docs); +} + +template <typename AttrType, typename ValueType> +void +fill_helper(AttributeVector& attr, const std::vector<ValueType>& values) +{ + assert(attr.getConfig().collectionType() == CollectionType::SINGLE); + add_docs(attr, values.size()); + auto& real = dynamic_cast<AttrType&>(attr); + for (size_t i = 0; i < values.size(); ++i) { + uint32_t docid = (i + 1); + real.update(docid, values[i]); + } + attr.commit(true); +} + +template <typename AttrType, typename ValueType> +void +fill_array_helper(AttributeVector& attr, const std::vector<std::vector<ValueType>>& values) +{ + assert((attr.getConfig().collectionType() == CollectionType::ARRAY) || + (attr.getConfig().collectionType() == CollectionType::WSET)); + add_docs(attr, values.size()); + auto& real = dynamic_cast<AttrType&>(attr); + for (size_t i = 0; i < values.size(); ++i) { + uint32_t docid = (i + 1); + for (auto value : values[i]) { + real.append(docid, value, 1); + } + } + attr.commit(true); +} + +template <typename AttrType, typename ValueType> +void +fill_wset_helper(AttributeVector& attr, const std::vector<std::vector<std::pair<ValueType, int32_t>>>& values) +{ + assert(attr.getConfig().collectionType() == CollectionType::WSET); + add_docs(attr, values.size()); + auto& real = dynamic_cast<AttrType&>(attr); + for (size_t i = 0; i < values.size(); ++i) { + uint32_t docid = (i + 1); + for (auto value : values[i]) { + real.append(docid, value.first, value.second); + } + } + attr.commit(true); +} + +} + +AttributeBuilder& +AttributeBuilder::fill(const std::vector<int64_t>& values) +{ + fill_helper<IntegerAttribute, int64_t>(_attr, values); + return *this; +} + +AttributeBuilder& +AttributeBuilder::fill_array(const std::vector<std::vector<int64_t>>& values) +{ + fill_array_helper<IntegerAttribute, int64_t>(_attr, values); + return *this; +} + +AttributeBuilder& +AttributeBuilder::fill_wset(const std::vector<std::vector<std::pair<int64_t, int32_t>>>& values) +{ + fill_wset_helper<IntegerAttribute, int64_t>(_attr, values); + return *this; +} + +AttributeBuilder& +AttributeBuilder::fill(const std::vector<vespalib::string>& values) +{ + fill_helper<StringAttribute, vespalib::string>(_attr, values); + return *this; +} + +AttributeBuilder& +AttributeBuilder::fill_array(const std::vector<std::vector<vespalib::string>>& values) +{ + fill_array_helper<StringAttribute, vespalib::string>(_attr, values); + return *this; +} + +AttributeBuilder& +AttributeBuilder::fill_wset(const std::vector<std::vector<std::pair<vespalib::string, int32_t>>>& values) +{ + fill_wset_helper<StringAttribute, vespalib::string>(_attr, values); + return *this; +} + +} + diff --git a/searchlib/src/vespa/searchlib/test/attribute_builder.h b/searchlib/src/vespa/searchlib/test/attribute_builder.h new file mode 100644 index 00000000000..6d3099b5ea1 --- /dev/null +++ b/searchlib/src/vespa/searchlib/test/attribute_builder.h @@ -0,0 +1,41 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/searchcommon/attribute/config.h> +#include <vespa/vespalib/stllike/string.h> +#include <memory> +#include <utility> +#include <vector> + +namespace search { class AttributeVector; } +namespace search::attribute { class Config; } + +namespace search::attribute::test { + +/** + * Helper class used to build and fill AttributeVector instances in unit tests. + */ +class AttributeBuilder { +private: + std::shared_ptr<AttributeVector> _attr_ptr; + AttributeVector& _attr; + +public: + AttributeBuilder(const vespalib::string& name, const Config& cfg); + AttributeBuilder(AttributeVector& attr); + + // Fill functions for integer attributes + AttributeBuilder& fill(const std::vector<int64_t>& values); + AttributeBuilder& fill_array(const std::vector<std::vector<int64_t>>& values); + AttributeBuilder& fill_wset(const std::vector<std::vector<std::pair<int64_t, int32_t>>>& values); + + // Fill functions for string attributes + AttributeBuilder& fill(const std::vector<vespalib::string>& values); + AttributeBuilder& fill_array(const std::vector<std::vector<vespalib::string>>& values); + AttributeBuilder& fill_wset(const std::vector<std::vector<std::pair<vespalib::string, int32_t>>>& values); + + std::shared_ptr<AttributeVector> get() const { return _attr_ptr; } +}; + +} diff --git a/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java b/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java index 57c89c89665..3a1952da49d 100644 --- a/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java +++ b/security-utils/src/test/java/com/yahoo/security/SharedKeyTest.java @@ -52,6 +52,7 @@ public class SharedKeyTest { var theirSealed = SealedSharedKey.fromTokenString(publicToken); var theirShared = SharedKeyGenerator.fromSealedKey(theirSealed, receiverPrivate); + assertEquals(1, theirSealed.keyId()); assertEquals(expectedSharedSecret, hex(theirShared.secretKey().getEncoded())); } diff --git a/testutil/src/main/java/com/yahoo/test/json/JsonBuilder.java b/testutil/src/main/java/com/yahoo/test/json/JsonBuilder.java new file mode 100644 index 00000000000..bbb8586a290 --- /dev/null +++ b/testutil/src/main/java/com/yahoo/test/json/JsonBuilder.java @@ -0,0 +1,75 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.test.json; + +import com.fasterxml.jackson.core.io.JsonStringEncoder; + +/** + * String buffer for building a formatted JSON. + * + * @author hakonhall + */ +class JsonBuilder { + private final JsonStringEncoder jsonStringEncoder = JsonStringEncoder.getInstance(); + private final StringBuilder builder = new StringBuilder(); + private final String indentation; + private final boolean multiLine; + private final String colon; + + private boolean bol = true; + private int level = 0; + + static JsonBuilder forCompactJson() { return new JsonBuilder(0, true); } + static JsonBuilder forMultiLineJson(int spacesPerIndent) { return new JsonBuilder(spacesPerIndent, false); } + + JsonBuilder(int spacesPerIndent, boolean compact) { + this.indentation = compact ? "" : " ".repeat(spacesPerIndent); + this.multiLine = !compact; + this.colon = compact ? ":" : ": "; + } + + void appendLineAndIndent(String text) { appendLineAndIndent(text, 0); } + + void newLineIndentAndAppend(int levelShift, String text) { + appendNewLine(); + indent(levelShift); + append(text); + } + + void appendLineAndIndent(String text, int levelShift) { + appendLine(text); + indent(levelShift); + append(""); + } + + void appendColon() { builder.append(colon); } + + void appendStringValue(String rawString) { + builder.append('"'); + jsonStringEncoder.quoteAsString(rawString, builder); + builder.append('"'); + } + + void append(String textWithoutNewline) { + if (bol) { + builder.append(indentation.repeat(level)); + bol = false; + } + + builder.append(textWithoutNewline); + } + + private void indent(int levelShift) { level += levelShift; } + + private void appendLine(String text) { + append(text); + appendNewLine(); + } + + private void appendNewLine() { + if (multiLine) builder.append('\n'); + bol = true; + } + + @Override + public String toString() { return builder.toString(); } +} diff --git a/testutil/src/main/java/com/yahoo/test/json/JsonNodeFormatter.java b/testutil/src/main/java/com/yahoo/test/json/JsonNodeFormatter.java new file mode 100644 index 00000000000..d0e07e66e0b --- /dev/null +++ b/testutil/src/main/java/com/yahoo/test/json/JsonNodeFormatter.java @@ -0,0 +1,81 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.test.json; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; + +/** + * Formats a {@link JsonNode} to a normalized JSON string, see {@link JsonTestHelper}. + * + * @author hakonhall + */ +class JsonNodeFormatter { + private final JsonBuilder builder; + + /** See {@link JsonTestHelper}. */ + static String toNormalizedJson(JsonNode jsonNode, boolean compact) { + JsonNodeFormatter formatter = new JsonNodeFormatter(compact); + formatter.appendValue(jsonNode); + return formatter.toString(); + } + + private JsonNodeFormatter(boolean compact) { + builder = compact ? JsonBuilder.forCompactJson() : JsonBuilder.forMultiLineJson(2); + } + + private void appendValue(JsonNode jsonNode) { + switch (jsonNode.getNodeType()) { + case OBJECT -> { + ObjectNode objectNode = (ObjectNode) jsonNode; + ArrayList<String> fieldNames = new ArrayList<>(); + objectNode.fieldNames().forEachRemaining(fieldNames::add); + Collections.sort(fieldNames); + if (fieldNames.isEmpty()) { + builder.append("{}"); + } else { + boolean firstIteration = true; + for (var fieldName : fieldNames) { + if (firstIteration) { + builder.appendLineAndIndent("{", +1); + firstIteration = false; + } else { + builder.appendLineAndIndent(","); + } + + builder.appendStringValue(fieldName); + builder.appendColon(); + appendValue(objectNode.get(fieldName)); + } + + builder.newLineIndentAndAppend(-1, "}"); + } + } + case ARRAY -> { + Iterator<JsonNode> elements = jsonNode.elements(); + if (elements.hasNext()) { + builder.appendLineAndIndent("[", +1); + appendValue(elements.next()); + + while (elements.hasNext()) { + builder.appendLineAndIndent(","); + appendValue(elements.next()); + } + + builder.newLineIndentAndAppend(-1, "]"); + } else { + builder.append("[]"); + } + } + case BOOLEAN, NUMBER, NULL -> builder.append(jsonNode.asText()); + case STRING -> builder.appendStringValue(jsonNode.asText()); + case BINARY, MISSING, POJO -> throw new IllegalStateException(jsonNode.getNodeType().toString()); + } + } + + @Override + public String toString() { return builder.toString(); } +} diff --git a/testutil/src/main/java/com/yahoo/test/json/JsonTestHelper.java b/testutil/src/main/java/com/yahoo/test/json/JsonTestHelper.java index f7112ee9379..589cc3ab4ef 100644 --- a/testutil/src/main/java/com/yahoo/test/json/JsonTestHelper.java +++ b/testutil/src/main/java/com/yahoo/test/json/JsonTestHelper.java @@ -17,6 +17,31 @@ public class JsonTestHelper { private static final ObjectMapper mapper = new ObjectMapper(); /** + * Returns a normalized JSON String. + * + * <ol> + * <li>A JSON string with each object's names in sorted order.</li> + * <li>Two JSONs are equal iff their normalized JSON strings are equal.*</li> + * <li>The normalized JSON is (by default) an indented multi-line string to facilitate + * readability and line-based diff tools.</li> + * <li>The normalized string does not end with a newline (\n).</li> + * </ol> + * + * <p>*) No effort is done to normalize decimals and may cause false non-equality, + * e.g. 1.2e1 is not equal to 12. This may be fixed at a later time if needed.</p> + */ + public static String normalize(String json) { + JsonNode jsonNode; + try { + jsonNode = mapper.readTree(json); + } catch (JsonProcessingException e) { + throw new IllegalArgumentException("Invalid JSON", e); + } + + return JsonNodeFormatter.toNormalizedJson(jsonNode, false); + } + + /** * Convenience method to input JSON without escaping double quotes and newlines * Each parameter represents a line of JSON encoded data * The lines are joined with newline and single quotes are replaced with double quotes diff --git a/testutil/src/test/java/com/yahoo/test/json/JsonTestHelperTest.java b/testutil/src/test/java/com/yahoo/test/json/JsonTestHelperTest.java new file mode 100644 index 00000000000..d0798284da1 --- /dev/null +++ b/testutil/src/test/java/com/yahoo/test/json/JsonTestHelperTest.java @@ -0,0 +1,45 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.test.json; + +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * @author hakonhall + */ +public class JsonTestHelperTest { + @Test + public void normalize() { + verifyNormalization(""" + {"a": 1, "c": 2, + "b": [ {"n": 3, "m": 4}, 5 ] + } + """, + """ + { + "a": 1, + "b": [ + { + "m": 4, + "n": 3 + }, + 5 + ], + "c": 2 + }"""); + + verifyNormalization("[1,2]", """ + [ + 1, + 2 + ]"""); + + verifyNormalization("null", "null"); + verifyNormalization("{ \n}", "{}"); + } + + private static void verifyNormalization(String json, String normalizedJson) { + assertEquals(normalizedJson, JsonTestHelper.normalize(json)); + } +}
\ No newline at end of file diff --git a/vespa-hadoop/pom.xml b/vespa-hadoop/pom.xml index ca5360a482e..c10ad590a53 100644 --- a/vespa-hadoop/pom.xml +++ b/vespa-hadoop/pom.xml @@ -19,76 +19,15 @@ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> <hadoop.version>3.3.4</hadoop.version> <pig.version>0.16.0</pig.version> - <jetty9.version>9.4.49.v20220914</jetty9.version> </properties> <dependencyManagement> - <!-- Force 9.4.x version of transitive Jetty dependencies. Required by hadoop libraries --> <dependencies> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-alpn-client</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-client</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-http</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-io</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-security</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-server</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-servlet</artifactId> - <version>${jetty9.version}</version> - </dependency> + <!-- Force newer version of jetty-util (to match Jetty version of jdisc) --> <dependency> <groupId>org.eclipse.jetty</groupId> <artifactId>jetty-util</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-webapp</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty</groupId> - <artifactId>jetty-xml</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty.websocket</groupId> - <artifactId>jetty-websocket-api</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty.websocket</groupId> - <artifactId>jetty-websocket-client</artifactId> - <version>${jetty9.version}</version> - </dependency> - <dependency> - <groupId>org.eclipse.jetty.websocket</groupId> - <artifactId>jetty-websocket-common</artifactId> - <version>${jetty9.version}</version> + <version>${jetty.version}</version> </dependency> </dependencies> </dependencyManagement> diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java index f0ea46e5220..4d50d323222 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunner.java @@ -90,7 +90,7 @@ public class TestRunner implements com.yahoo.vespa.testrunner.TestRunner { command.add("-DfailIfNoTests=" + profile.failIfNoTests()); command.add("-Dvespa.test.config=" + vespaHome.resolve("tmp/config.json")); if (config.useAthenzCredentials()) - command.add("-Dvespa.test.credentials.root=" + Defaults.getDefaults().vespaHome() + "/var/vespa/sia"); + command.add("-Dvespa.test.credentials.root=" + Defaults.getDefaults().underVespaHome("var/vespa/sia")); else if (config.useTesterCertificate()) command.add("-Dvespa.test.credentials.root=" + config.artifactsPath()); command.add(String.format("-DargLine=-Xms%1$dm -Xmx%1$dm", config.surefireMemoryMb())); diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliOptions.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliOptions.java new file mode 100644 index 00000000000..7560c5f3b4c --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliOptions.java @@ -0,0 +1,65 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool; + +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; + +import java.io.PrintStream; +import java.io.PrintWriter; +import java.util.List; +import java.util.stream.Collectors; + +/** + * @author vekterli + * @author bjorncs + */ +class CliOptions { + + private static final Option HELP_OPTION = Option.builder("h") + .longOpt("help") + .hasArg(false) + .required(false) + .desc("Show help") + .build(); + + static Options withHelpOption(List<Option> options) { + var optionsWithHelp = new Options(); + options.forEach(optionsWithHelp::addOption); + optionsWithHelp.addOption(HELP_OPTION); + return optionsWithHelp; + } + + static void printTopLevelHelp(PrintStream out, List<Tool> tools) { + var formatter = new HelpFormatter(); + var writer = new PrintWriter(out); + formatter.printHelp( + writer, + formatter.getWidth(), + "vespa-security <tool> [TOOL OPTIONS]", + "Where <tool> is one of: %s".formatted(tools.stream().map(Tool::name).collect(Collectors.joining(", "))), + withHelpOption(List.of()), + formatter.getLeftPadding(), + formatter.getDescPadding(), + "Invoke vespa-security <tool> --help for tool-specific help"); + writer.flush(); + } + + static void printToolSpecificHelp(PrintStream out, String toolName, + ToolDescription toolDesc, + Options optionsWithHelp) { + var formatter = new HelpFormatter(); + var writer = new PrintWriter(out); + formatter.printHelp( + writer, + formatter.getWidth(), + "vespa-security %s %s".formatted(toolName, toolDesc.helpArgSuffix()), + toolDesc.helpHeader(), + optionsWithHelp, + formatter.getLeftPadding(), + formatter.getDescPadding(), + toolDesc.helpFooter()); + writer.flush(); + } +} + diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliUtils.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliUtils.java new file mode 100644 index 00000000000..e9b348ab2a2 --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/CliUtils.java @@ -0,0 +1,19 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool; + +import org.apache.commons.cli.CommandLine; + +/** + * @author vekterli + */ +public class CliUtils { + + public static String optionOrThrow(CommandLine arguments, String option) { + var value = arguments.getOptionValue(option); + if (value == null) { + throw new IllegalArgumentException("Required argument '--%s' must be provided".formatted(option)); + } + return value; + } + +} diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/Main.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/Main.java new file mode 100644 index 00000000000..4216ffb6ed4 --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/Main.java @@ -0,0 +1,104 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool; + +import com.yahoo.vespa.security.tool.crypto.DecryptTool; +import com.yahoo.vespa.security.tool.crypto.EncryptTool; +import com.yahoo.vespa.security.tool.crypto.KeygenTool; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.CommandLineParser; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; + +import java.io.PrintStream; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +/** + * Primary application entry point for security utility tools. Handles tool selection, + * CLI argument parsing and exception printing. + * + * Based on previous vespa-security-env tool. + * + * @author vekterli + * @author bjorncs + */ +public class Main { + + private final PrintStream stdOut; + private final PrintStream stdError; + + Main(PrintStream stdOut, PrintStream stdError) { + this.stdOut = stdOut; + this.stdError = stdError; + } + + public static void main(String[] args) { + var program = new Main(System.out, System.err); + int returnCode = program.execute(args, System.getenv()); + System.exit(returnCode); + } + + private static final List<Tool> TOOLS = List.of( + new KeygenTool(), new EncryptTool(), new DecryptTool()); + + private static Optional<Tool> toolFromCliArgs(String[] args) { + if (args.length == 0) { + return Optional.empty(); + } + String toolName = args[0]; + return TOOLS.stream().filter(t -> t.name().equals(toolName)).findFirst(); + } + + private static String[] withToolNameArgRemoved(String[] args) { + if (args.length == 0) { + throw new IllegalArgumentException("Argument array did not contain a tool name"); + } + String[] truncatedArgs = new String[args.length - 1]; + System.arraycopy(args, 1, truncatedArgs, 0, truncatedArgs.length); + return truncatedArgs; + } + + private static CommandLine parseCliArguments(String[] cliArgs, Options options) throws ParseException { + CommandLineParser parser = new DefaultParser(); + return parser.parse(options, cliArgs); + } + + public int execute(String[] args, Map<String, String> envVars) { + boolean debugMode = envVars.containsKey("VESPA_DEBUG"); + try { + var maybeTool = toolFromCliArgs(args); + if (maybeTool.isEmpty()) { // This also implicitly covers the top-level --help case. + CliOptions.printTopLevelHelp(stdOut, TOOLS); + return 0; + } + var tool = maybeTool.get(); + var toolDesc = tool.description(); + var cliOpts = CliOptions.withHelpOption(toolDesc.cliOptions()); + String[] truncatedArgs = withToolNameArgRemoved(args); + var cmdLine = parseCliArguments(truncatedArgs, cliOpts); + if (cmdLine.hasOption("help")) { + CliOptions.printToolSpecificHelp(stdOut, tool.name(), toolDesc, cliOpts); + return 0; + } + var invocation = new ToolInvocation(cmdLine, envVars, stdOut, stdError, debugMode); + return tool.invoke(invocation); + } catch (ParseException e) { + return handleException("Failed to parse command line arguments: " + e.getMessage(), e, debugMode); + } catch (IllegalArgumentException e) { + return handleException("Invalid command line arguments: " + e.getMessage(), e, debugMode); + } catch (Exception e) { + return handleException("Got unhandled exception: " + e.getMessage(), e, debugMode); + } + } + + private int handleException(String message, Exception exception, boolean debugMode) { + stdError.println(message); + if (debugMode) { + exception.printStackTrace(stdError); + } + return 1; + } + +} diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/Tool.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/Tool.java new file mode 100644 index 00000000000..251b2d40e3c --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/Tool.java @@ -0,0 +1,30 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool; + +/** + * A named tool that can be invoked via the parent Main program. + * + * @author vekterli + */ +public interface Tool { + + /** + * Name of the tool used verbatim on the command line. + */ + String name(); + + /** + * Description used when "--help" is invoked for a particular tool + */ + ToolDescription description(); + + /** + * Invokes the tool logic with a ToolInvocation that encapsulates the command line + * and input/ouput environment the tool was called in. + * + * @param invocation parameters and environment to be used by the tool + * @return exit code that will be returned by the main process + */ + int invoke(ToolInvocation invocation); + +} diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/ToolDescription.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/ToolDescription.java new file mode 100644 index 00000000000..168d8fba3ba --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/ToolDescription.java @@ -0,0 +1,19 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool; + +import org.apache.commons.cli.Option; + +import java.util.List; + +/** + * Used by Tool subclasses to describe their options and calling semantics via + * the "--help" output from the Main program. + * + * @author vekterli + */ +public record ToolDescription(String helpArgSuffix, + String helpHeader, + String helpFooter, + List<Option> cliOptions) { + +} diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/ToolInvocation.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/ToolInvocation.java new file mode 100644 index 00000000000..b7340ebb749 --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/ToolInvocation.java @@ -0,0 +1,18 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool; + +import org.apache.commons.cli.CommandLine; + +import java.io.PrintStream; +import java.util.Map; + +/** + * @author vekterli + */ +public record ToolInvocation(CommandLine arguments, + Map<String, String> envVars, + PrintStream stdOut, + PrintStream stdError, + boolean debugMode) { + +} diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/CipherUtils.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/CipherUtils.java new file mode 100644 index 00000000000..e3954558026 --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/CipherUtils.java @@ -0,0 +1,37 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool.crypto; + +import javax.crypto.Cipher; +import javax.crypto.CipherOutputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +/** + * @author vekterli + */ +public class CipherUtils { + + /** + * Streams the contents of fromPath into toPath after being wrapped by the input cipher. + * Depending on the Cipher mode, this either encrypts a plaintext file into ciphertext, + * or decrypts a ciphertext file into plaintext. + * + * @param fromPath source file path to read from + * @param toPath destination file path to write to + * @param cipher a Cipher in either ENCRYPT or DECRYPT mode + * @throws IOException if any file operation fails + */ + public static void streamEncipherFileContents(Path fromPath, Path toPath, Cipher cipher) throws IOException { + if (fromPath.equals(toPath)) { + throw new IllegalArgumentException("Can't use same file as both input and output for enciphering"); + } + try (var inStream = Files.newInputStream(fromPath); + var outStream = Files.newOutputStream(toPath); + var cipherStream = new CipherOutputStream(outStream, cipher)) { + inStream.transferTo(cipherStream); + cipherStream.flush(); + } + } + +} diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/DecryptTool.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/DecryptTool.java new file mode 100644 index 00000000000..b307ab76da8 --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/DecryptTool.java @@ -0,0 +1,111 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool.crypto; + +import com.yahoo.security.KeyUtils; +import com.yahoo.security.SealedSharedKey; +import com.yahoo.security.SharedKeyGenerator; +import com.yahoo.vespa.security.tool.CliUtils; +import com.yahoo.vespa.security.tool.Tool; +import com.yahoo.vespa.security.tool.ToolDescription; +import com.yahoo.vespa.security.tool.ToolInvocation; +import org.apache.commons.cli.Option; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.List; +import java.util.Optional; + +/** + * Tooling for decrypting a file using a private key that corresponds to the public key used + * to originally encrypt the file. + * + * Uses the opaque token abstraction from {@link SharedKeyGenerator}. + * + * @author vekterli + */ +public class DecryptTool implements Tool { + + static final String OUTPUT_FILE_OPTION = "output-file"; + static final String RECIPIENT_PRIVATE_KEY_FILE_OPTION = "recipient-private-key-file"; + static final String KEY_ID_OPTION = "key-id"; + static final String TOKEN_OPTION = "token"; + + private static final List<Option> OPTIONS = List.of( + Option.builder("o") + .longOpt(OUTPUT_FILE_OPTION) + .hasArg(true) + .required(false) + .desc("Output file for decrypted plaintext") + .build(), + Option.builder("k") + .longOpt(RECIPIENT_PRIVATE_KEY_FILE_OPTION) + .hasArg(true) + .required(false) + .desc("Recipient private key file") + .build(), + Option.builder("i") + .longOpt(KEY_ID_OPTION) + .hasArg(true) + .required(false) + .desc("Numeric ID of recipient key. If this is not provided, " + + "the key ID stored as part of the token is not verified.") + .build(), + Option.builder("t") + .longOpt(TOKEN_OPTION) + .hasArg(true) + .required(false) + .desc("Token generated when the input file was encrypted") + .build()); + + @Override + public String name() { + return "decrypt"; + } + + @Override + public ToolDescription description() { + return new ToolDescription( + "<encrypted file> <options>", + "Decrypts a file using a provided token and a secret private key. The file must " + + "previously have been encrypted using the public key component of the given private key.", + "Note: this is a BETA tool version; its interface may be changed at any time", + OPTIONS); + } + + @Override + public int invoke(ToolInvocation invocation) { + try { + var arguments = invocation.arguments(); + var leftoverArgs = arguments.getArgs(); + if (leftoverArgs.length != 1) { + throw new IllegalArgumentException("Expected exactly 1 file argument to decrypt"); + } + var inputPath = Paths.get(leftoverArgs[0]); + if (!inputPath.toFile().exists()) { + throw new IllegalArgumentException("Cannot decrypt file '%s' as it does not exist".formatted(inputPath.toString())); + } + var maybeKeyId = Optional.ofNullable(arguments.hasOption(KEY_ID_OPTION) + ? Integer.parseInt(arguments.getOptionValue(KEY_ID_OPTION)) + : null); + var outputPath = Paths.get(CliUtils.optionOrThrow(arguments, OUTPUT_FILE_OPTION)); + var privKeyPath = Paths.get(CliUtils.optionOrThrow(arguments, RECIPIENT_PRIVATE_KEY_FILE_OPTION)); + var tokenString = CliUtils.optionOrThrow(arguments, TOKEN_OPTION); + var sealedSharedKey = SealedSharedKey.fromTokenString(tokenString.strip()); + if (maybeKeyId.isPresent() && (maybeKeyId.get() != sealedSharedKey.keyId())) { + throw new IllegalArgumentException(("Key ID specified with --key-id (%d) does not match key ID " + + "used when generating the supplied token (%d)") + .formatted(maybeKeyId.get(), sealedSharedKey.keyId())); + } + var privateKey = KeyUtils.fromBase64EncodedX25519PrivateKey(Files.readString(privKeyPath).strip()); + var secretShared = SharedKeyGenerator.fromSealedKey(sealedSharedKey, privateKey); + var cipher = SharedKeyGenerator.makeAesGcmDecryptionCipher(secretShared); + + CipherUtils.streamEncipherFileContents(inputPath, outputPath, cipher); + + } catch (IOException e) { + throw new RuntimeException(e); + } + return 0; + } +} diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/EncryptTool.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/EncryptTool.java new file mode 100644 index 00000000000..5d6cee2fabc --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/EncryptTool.java @@ -0,0 +1,93 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool.crypto; + +import com.yahoo.security.KeyUtils; +import com.yahoo.security.SharedKeyGenerator; +import com.yahoo.vespa.security.tool.CliUtils; +import com.yahoo.vespa.security.tool.Tool; +import com.yahoo.vespa.security.tool.ToolDescription; +import com.yahoo.vespa.security.tool.ToolInvocation; +import org.apache.commons.cli.Option; + +import java.io.IOException; +import java.nio.file.Paths; +import java.util.List; + +/** + * Tooling to encrypt a file using a public key, emitting a non-secret token that can be + * passed on to a recipient holding the corresponding private key. + * + * Uses the opaque token abstraction from {@link SharedKeyGenerator}. + * + * @author vekterli + */ +public class EncryptTool implements Tool { + + static final String OUTPUT_FILE_OPTION = "output-file"; + static final String KEY_ID_OPTION = "key-id"; + static final String RECIPIENT_PUBLIC_KEY_OPTION = "recipient-public-key"; + + private static final List<Option> OPTIONS = List.of( + Option.builder("o") + .longOpt(OUTPUT_FILE_OPTION) + .hasArg(true) + .required(false) + .desc("Output file (will be truncated if it already exists)") + .build(), + Option.builder("r") + .longOpt(RECIPIENT_PUBLIC_KEY_OPTION) + .hasArg(true) + .required(false) + .desc("Recipient X25519 public key in Base64 encoded format") + .build(), + Option.builder("i") + .longOpt(KEY_ID_OPTION) + .hasArg(true) + .required(false) + .desc("Numeric ID of recipient key") + .build()); + + @Override + public String name() { + return "encrypt"; + } + + @Override + public ToolDescription description() { + return new ToolDescription( + "<input file> <options>", + "One-way encrypts a file using the public key of a recipient. A public token is printed on " + + "standard out. The recipient can use this token to decrypt the file using their private key. " + + "The token does not have to be kept secret.", + "Note: this is a BETA tool version; its interface may be changed at any time", + OPTIONS); + } + + @Override + public int invoke(ToolInvocation invocation) { + try { + var arguments = invocation.arguments(); + var leftoverArgs = arguments.getArgs(); + if (leftoverArgs.length != 1) { + throw new IllegalArgumentException("Expected exactly 1 file argument to encrypt"); + } + var inputPath = Paths.get(leftoverArgs[0]); + if (!inputPath.toFile().exists()) { + throw new IllegalArgumentException("Cannot encrypt file '%s' as it does not exist".formatted(inputPath.toString())); + } + var outputPath = Paths.get(CliUtils.optionOrThrow(arguments, OUTPUT_FILE_OPTION)); + + var recipientPubKey = KeyUtils.fromBase64EncodedX25519PublicKey(CliUtils.optionOrThrow(arguments, RECIPIENT_PUBLIC_KEY_OPTION).strip()); + int keyId = Integer.parseInt(CliUtils.optionOrThrow(arguments, KEY_ID_OPTION)); + var shared = SharedKeyGenerator.generateForReceiverPublicKey(recipientPubKey, keyId); + var cipher = SharedKeyGenerator.makeAesGcmEncryptionCipher(shared); + + CipherUtils.streamEncipherFileContents(inputPath, outputPath, cipher); + + invocation.stdOut().println(shared.sealedSharedKey().toTokenString()); + } catch (IOException e) { + throw new RuntimeException(e); + } + return 0; + } +} diff --git a/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/KeygenTool.java b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/KeygenTool.java new file mode 100644 index 00000000000..a0b9cce710b --- /dev/null +++ b/vespaclient-java/src/main/java/com/yahoo/vespa/security/tool/crypto/KeygenTool.java @@ -0,0 +1,105 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool.crypto; + +import com.yahoo.security.KeyUtils; +import com.yahoo.vespa.security.tool.CliUtils; +import com.yahoo.vespa.security.tool.Tool; +import com.yahoo.vespa.security.tool.ToolDescription; +import com.yahoo.vespa.security.tool.ToolInvocation; +import org.apache.commons.cli.Option; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermissions; +import java.security.interfaces.XECPrivateKey; +import java.security.interfaces.XECPublicKey; +import java.util.List; + +/** + * Tooling to generate random X25519 key pairs. + * + * @author vekterli + */ +public class KeygenTool implements Tool { + + static final String PRIVATE_OUT_FILE_OPTION = "private-out-file"; + static final String PUBLIC_OUT_FILE_OPTION = "public-out-file"; + static final String OVERWRITE_EXISTING_OPTION = "overwrite-existing"; + + private static final List<Option> OPTIONS = List.of( + Option.builder("k") + .longOpt(PRIVATE_OUT_FILE_OPTION) + .hasArg(true) + .required(false) + .desc("Output file for private (secret) key. Will be created with restrictive file permissions.") + .build(), + Option.builder("p") + .longOpt(PUBLIC_OUT_FILE_OPTION) + .hasArg(true) + .required(false) + .desc("Output file for public key") + .build(), + Option.builder() + .longOpt(OVERWRITE_EXISTING_OPTION) + .hasArg(false) + .required(false) + .desc("Overwrite existing key files instead of failing key generation if " + + "any files already exist. Use with great caution!") + .build()); + + @Override + public String name() { + return "keygen"; + } + + @Override + public ToolDescription description() { + return new ToolDescription( + "<options>", + "Generates an X25519 key pair and stores its private/public parts in " + + "separate files in Base64 encoded form.", + "Note: this is a BETA tool version; its interface may be changed at any time", + OPTIONS); + } + + private void handleExistingFileIfAny(Path filePath, boolean allowOverwrite) throws IOException { + if (filePath.toFile().exists()) { + if (!allowOverwrite) { + throw new IllegalArgumentException(("Output file '%s' already exists. No keys written. " + + "If you want to overwrite existing files, specify --%s.") + .formatted(filePath.toAbsolutePath().toString(), OVERWRITE_EXISTING_OPTION)); + } else { + // Explicitly delete the file since Files.createFile() will fail if it already exists. + Files.delete(filePath); + } + } + } + + @Override + public int invoke(ToolInvocation invocation) { + try { + var arguments = invocation.arguments(); + var privOutPath = Paths.get(CliUtils.optionOrThrow(arguments, PRIVATE_OUT_FILE_OPTION)); + var pubOutPath = Paths.get(CliUtils.optionOrThrow(arguments, PUBLIC_OUT_FILE_OPTION)); + + boolean allowOverwrite = arguments.hasOption(OVERWRITE_EXISTING_OPTION); + handleExistingFileIfAny(privOutPath, allowOverwrite); + handleExistingFileIfAny(pubOutPath, allowOverwrite); + + var keyPair = KeyUtils.generateX25519KeyPair(); + var privKey = (XECPrivateKey) keyPair.getPrivate(); + var pubKey = (XECPublicKey) keyPair.getPublic(); + + var privFilePerms = PosixFilePermissions.fromString("rw-------"); + Files.createFile( privOutPath, PosixFilePermissions.asFileAttribute(privFilePerms)); + Files.writeString(privOutPath, KeyUtils.toBase64EncodedX25519PrivateKey(privKey) + "\n"); + Files.writeString(pubOutPath, KeyUtils.toBase64EncodedX25519PublicKey(pubKey) + "\n"); + + } catch (IOException e) { + throw new RuntimeException(e); + } + return 0; + } +} diff --git a/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java b/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java new file mode 100644 index 00000000000..bb8024544cb --- /dev/null +++ b/vespaclient-java/src/test/java/com/yahoo/vespa/security/tool/CryptoToolsTest.java @@ -0,0 +1,296 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.security.tool; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * @author vekterli + */ +public class CryptoToolsTest { + + private static record ProcessOutput(int exitCode, String stdOut, String stdErr) {} + + @TempDir + public File tmpFolder; + + private void verifyStdoutMatchesFile(List<String> args, String expectedFile) throws IOException { + var procOut = runMain(args, Map.of()); + assertEquals(0, procOut.exitCode()); + assertEquals(readTestResource(expectedFile), procOut.stdOut()); + } + + private void verifyStderrEquals(List<String> args, String expectedMessage) throws IOException { + var procOut = runMain(args, Map.of()); + assertEquals(1, procOut.exitCode()); // Assume checking stderr is because of a failure. + assertEquals(expectedMessage, procOut.stdErr()); + } + + @Test + void top_level_help_page_printed_if_help_option_given() throws IOException { + verifyStdoutMatchesFile(List.of("--help"), "expected-help-output.txt"); + } + + @Test + void top_level_help_page_printed_if_no_option_given() throws IOException { + verifyStdoutMatchesFile(List.of(), "expected-help-output.txt"); + } + + @Test + void keygen_help_printed_if_help_option_given_to_subtool() throws IOException { + verifyStdoutMatchesFile(List.of("keygen", "--help"), "expected-keygen-help-output.txt"); + } + + @Test + void encrypt_help_printed_if_help_option_given_to_subtool() throws IOException { + verifyStdoutMatchesFile(List.of("encrypt", "--help"), "expected-encrypt-help-output.txt"); + } + + @Test + void decrypt_help_printed_if_help_option_given_to_subtool() throws IOException { + verifyStdoutMatchesFile(List.of("decrypt", "--help"), "expected-decrypt-help-output.txt"); + } + + @Test + void missing_required_parameter_prints_error_message() throws IOException { + // We don't test all possible input arguments to all tools, since it'd be too closely + // bound to the order in which the implementation checks for argument presence. + // This primarily verifies that IllegalArgumentExceptions thrown by a tool will be caught + // and printed to stderr as expected. + verifyStderrEquals(List.of("keygen"), + "Invalid command line arguments: Required argument '--private-out-file' must be provided\n"); + verifyStderrEquals(List.of("keygen", "--private-out-file", "foo.txt"), + "Invalid command line arguments: Required argument '--public-out-file' must be provided\n"); + } + + // We don't want to casually overwrite key material if someone runs a command twice by accident. + @Test + void keygen_fails_by_default_if_output_file_exists() throws IOException { + Path privKeyFile = pathInTemp("priv.txt"); + Path pubKeyFile = pathInTemp("pub.txt"); + Files.writeString(privKeyFile, TEST_PRIV_KEY); + + verifyStderrEquals(List.of("keygen", + "--private-out-file", absPathOf(privKeyFile), + "--public-out-file", absPathOf(pubKeyFile)), + ("Invalid command line arguments: Output file '%s' already exists. No keys written. " + + "If you want to overwrite existing files, specify --overwrite-existing.\n") + .formatted(absPathOf(privKeyFile))); + + Files.delete(privKeyFile); + Files.writeString(pubKeyFile, TEST_PUB_KEY); + + verifyStderrEquals(List.of("keygen", + "--private-out-file", absPathOf(privKeyFile), + "--public-out-file", absPathOf(pubKeyFile)), + ("Invalid command line arguments: Output file '%s' already exists. No keys written. " + + "If you want to overwrite existing files, specify --overwrite-existing.\n") + .formatted(absPathOf(pubKeyFile))); + } + + // ... but we'll allow it if someone enables the foot-gun option. + @Test + void keygen_allowed_if_output_file_exists_and_explicit_overwrite_option_specified() throws IOException { + Path privKeyFile = pathInTemp("priv.txt"); + Path pubKeyFile = pathInTemp("pub.txt"); + Files.writeString(privKeyFile, TEST_PRIV_KEY); + Files.writeString(pubKeyFile, TEST_PUB_KEY); + + var procOut = runMain(List.of("keygen", + "--private-out-file", absPathOf(privKeyFile), + "--public-out-file", absPathOf(pubKeyFile), + "--overwrite-existing")); + assertEquals(0, procOut.exitCode()); + + // Keys are random, so we don't know what they'll end up being. But the likelihood of them + // exactly matching the test keys is effectively and realistically zero. + assertNotEquals(TEST_PRIV_KEY, Files.readString(privKeyFile)); + assertNotEquals(TEST_PUB_KEY, Files.readString(pubKeyFile)); + } + + @Test + void keygen_writes_private_key_with_user_only_rw_permissions() throws IOException { + Path privKeyFile = pathInTemp("priv.txt"); + Path pubKeyFile = pathInTemp("pub.txt"); + + var procOut = runMain(List.of("keygen", + "--private-out-file", absPathOf(privKeyFile), + "--public-out-file", absPathOf(pubKeyFile))); + assertEquals(0, procOut.exitCode()); + var privKeyPerms = Files.getPosixFilePermissions(privKeyFile); + var expectedPerms = PosixFilePermissions.fromString("rw-------"); + assertEquals(expectedPerms, privKeyPerms); + } + + private static final String TEST_PRIV_KEY = "4qGcntygFn_a3uqeBa1PbDlygQ-cpOuNznTPIz9ftWE"; + private static final String TEST_PUB_KEY = "ROAH_S862tNMpbJ49lu1dPXFCPHFIXZK30pSrMZEmEg"; + // Token created for the above public key (matching the above private key), using key id 1 + private static final String TEST_TOKEN = "AQAAAQAgwyxd7bFNQB_2LdL3bw-xFlvrxXhs7WWNVCKZ4" + + "EFeNVtu42JMwM74bMN4E46v6mYcfQNPzcMGaP22Wl2cTnji0A"; + private static final int TEST_TOKEN_KEY_ID = 1; + + @Test + void encrypt_fails_with_error_message_if_no_input_file_is_given() throws IOException { + verifyStderrEquals(List.of("encrypt", + "--output-file", "foo", + "--recipient-public-key", TEST_PUB_KEY, + "--key-id", "1234"), + "Invalid command line arguments: Expected exactly 1 file argument to encrypt\n"); + } + + @Test + void encrypt_fails_with_error_message_if_input_file_does_not_exist() throws IOException { + verifyStderrEquals(List.of("encrypt", + "no-such-file", + "--output-file", "foo", + "--recipient-public-key", TEST_PUB_KEY, + "--key-id", "1234"), + "Invalid command line arguments: Cannot encrypt file 'no-such-file' as it does not exist\n"); + } + + @Test + void decrypt_fails_with_error_message_if_no_input_file_is_given() throws IOException { + Path privKeyFile = pathInTemp("priv.txt"); + Files.writeString(privKeyFile, TEST_PRIV_KEY); + + verifyStderrEquals(List.of("decrypt", + "--output-file", "foo", + "--recipient-private-key-file", absPathOf(privKeyFile), + "--token", TEST_TOKEN, + "--key-id", Integer.toString(TEST_TOKEN_KEY_ID)), + "Invalid command line arguments: Expected exactly 1 file argument to decrypt\n"); + } + + @Test + void decrypt_fails_with_error_message_if_input_file_does_not_exist() throws IOException { + Path privKeyFile = pathInTemp("priv.txt"); + Files.writeString(privKeyFile, TEST_PRIV_KEY); + + verifyStderrEquals(List.of("decrypt", + "no-such-file", + "--output-file", "foo", + "--recipient-private-key-file", absPathOf(privKeyFile), + "--token", TEST_TOKEN, + "--key-id", Integer.toString(TEST_TOKEN_KEY_ID)), + "Invalid command line arguments: Cannot decrypt file 'no-such-file' as it does not exist\n"); + } + + @Test + void decrypt_fails_with_error_message_if_expected_key_id_does_not_match_key_id_in_token() throws IOException { + Path privKeyFile = pathInTemp("priv.txt"); + Files.writeString(privKeyFile, TEST_PRIV_KEY); + + Path inputFile = pathInTemp("input.txt"); + Files.writeString(inputFile, "dummy-not-actually-encrypted-data"); + + verifyStderrEquals(List.of("decrypt", + absPathOf(inputFile), + "--output-file", "foo", + "--recipient-private-key-file", absPathOf(privKeyFile), + "--token", TEST_TOKEN, + "--key-id", Integer.toString(TEST_TOKEN_KEY_ID + 1)), + "Invalid command line arguments: Key ID specified with --key-id (2) does not match " + + "key ID used when generating the supplied token (1)\n"); + } + + @Test + void can_end_to_end_keygen_encrypt_and_decrypt() throws IOException { + String greatSecret = "Dogs can't look up"; + + Path secretFile = pathInTemp("secret.txt"); + Files.writeString(secretFile, greatSecret); + + var privPath = pathInTemp("priv.txt"); + var pubPath = pathInTemp("pub.txt"); + var procOut = runMain(List.of( + "keygen", + "--private-out-file", absPathOf(privPath), + "--public-out-file", absPathOf(pubPath))); + assertEquals(0, procOut.exitCode()); + assertEquals("", procOut.stdOut()); + assertEquals("", procOut.stdErr()); + + assertTrue(privPath.toFile().exists()); + assertTrue(pubPath.toFile().exists()); + + var encryptedPath = pathInTemp("encrypted.bin"); + // TODO support (and test) public key via file + procOut = runMain(List.of( + "encrypt", + absPathOf(secretFile), + "--output-file", absPathOf(encryptedPath), + "--recipient-public-key", Files.readString(pubPath), + "--key-id", "1234")); + assertEquals(0, procOut.exitCode()); + assertEquals("", procOut.stdErr()); + + var token = procOut.stdOut(); + assertFalse(token.isBlank()); + + assertTrue(encryptedPath.toFile().exists()); + + var decryptedPath = pathInTemp("decrypted.txt"); + procOut = runMain(List.of( + "decrypt", + absPathOf(encryptedPath), + "--output-file", absPathOf(decryptedPath), + "--recipient-private-key-file", absPathOf(privPath), + "--key-id", "1234", + "--token", token + )); + assertEquals(0, procOut.exitCode()); + assertEquals("", procOut.stdOut()); + assertEquals("", procOut.stdErr()); + + assertEquals(greatSecret, Files.readString(decryptedPath)); + } + + private ProcessOutput runMain(List<String> args) { + // Expect that this is used for running a command that is not supposed to fail. But if it does, + // include exception trace in stderr to make it easier to debug. + return runMain(args, Map.of("VESPA_DEBUG", "true")); + } + + private ProcessOutput runMain(List<String> args, Map<String, String> env) { + var stdOutBytes = new ByteArrayOutputStream(); + var stdErrBytes = new ByteArrayOutputStream(); + var stdOut = new PrintStream(stdOutBytes); + var stdError = new PrintStream(stdErrBytes); + + int exitCode = new Main(stdOut, stdError).execute(args.toArray(new String[0]), env); + + stdOut.flush(); + stdError.flush(); + + return new ProcessOutput(exitCode, stdOutBytes.toString(), stdErrBytes.toString()); + } + + private static String readTestResource(String fileName) throws IOException { + return Files.readString(Paths.get(CryptoToolsTest.class.getResource('/' + fileName).getFile())); + } + + private Path pathInTemp(String fileName) { + return tmpFolder.toPath().resolve(fileName); + } + + private static String absPathOf(Path path) { + return path.toAbsolutePath().toString(); + } + +} diff --git a/vespaclient-java/src/test/resources/expected-decrypt-help-output.txt b/vespaclient-java/src/test/resources/expected-decrypt-help-output.txt new file mode 100644 index 00000000000..979133a562b --- /dev/null +++ b/vespaclient-java/src/test/resources/expected-decrypt-help-output.txt @@ -0,0 +1,16 @@ +usage: vespa-security decrypt <encrypted file> <options> +Decrypts a file using a provided token and a secret private key. The file +must previously have been encrypted using the public key component of the +given private key. + -h,--help Show help + -i,--key-id <arg> Numeric ID of recipient key. If + this is not provided, the key ID + stored as part of the token is + not verified. + -k,--recipient-private-key-file <arg> Recipient private key file + -o,--output-file <arg> Output file for decrypted + plaintext + -t,--token <arg> Token generated when the input + file was encrypted +Note: this is a BETA tool version; its interface may be changed at any +time diff --git a/vespaclient-java/src/test/resources/expected-encrypt-help-output.txt b/vespaclient-java/src/test/resources/expected-encrypt-help-output.txt new file mode 100644 index 00000000000..848035f417e --- /dev/null +++ b/vespaclient-java/src/test/resources/expected-encrypt-help-output.txt @@ -0,0 +1,13 @@ +usage: vespa-security encrypt <input file> <options> +One-way encrypts a file using the public key of a recipient. A public +token is printed on standard out. The recipient can use this token to +decrypt the file using their private key. The token does not have to be +kept secret. + -h,--help Show help + -i,--key-id <arg> Numeric ID of recipient key + -o,--output-file <arg> Output file (will be truncated if it + already exists) + -r,--recipient-public-key <arg> Recipient X25519 public key in Base64 + encoded format +Note: this is a BETA tool version; its interface may be changed at any +time diff --git a/vespaclient-java/src/test/resources/expected-help-output.txt b/vespaclient-java/src/test/resources/expected-help-output.txt new file mode 100644 index 00000000000..45cf829c981 --- /dev/null +++ b/vespaclient-java/src/test/resources/expected-help-output.txt @@ -0,0 +1,4 @@ +usage: vespa-security <tool> [TOOL OPTIONS] +Where <tool> is one of: keygen, encrypt, decrypt + -h,--help Show help +Invoke vespa-security <tool> --help for tool-specific help diff --git a/vespaclient-java/src/test/resources/expected-keygen-help-output.txt b/vespaclient-java/src/test/resources/expected-keygen-help-output.txt new file mode 100644 index 00000000000..60629c4291f --- /dev/null +++ b/vespaclient-java/src/test/resources/expected-keygen-help-output.txt @@ -0,0 +1,13 @@ +usage: vespa-security keygen <options> +Generates an X25519 key pair and stores its private/public parts in +separate files in Base64 encoded form. + -h,--help Show help + -k,--private-out-file <arg> Output file for private (secret) key. Will + be created with restrictive file + permissions. + --overwrite-existing Overwrite existing key files instead of + failing key generation if any files already + exist. Use with great caution! + -p,--public-out-file <arg> Output file for public key +Note: this is a BETA tool version; its interface may be changed at any +time diff --git a/vespajlib/src/main/java/com/yahoo/io/IOUtils.java b/vespajlib/src/main/java/com/yahoo/io/IOUtils.java index 81e1306b29e..699ac22d278 100644 --- a/vespajlib/src/main/java/com/yahoo/io/IOUtils.java +++ b/vespajlib/src/main/java/com/yahoo/io/IOUtils.java @@ -413,7 +413,7 @@ public abstract class IOUtils { /** Read an input stream completely into a string */ public static String readAll(InputStream stream, Charset charset) throws IOException { - return readAll(new InputStreamReader(stream, charset)); + return new String(stream.readAllBytes(), charset); } /** Convenience method for closing a list of readers. Does nothing if the given reader list is null. */ diff --git a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java index c36b001d056..c2e11be34be 100644 --- a/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java +++ b/vespajlib/src/main/java/com/yahoo/slime/SlimeUtils.java @@ -3,6 +3,7 @@ package com.yahoo.slime; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; @@ -16,6 +17,8 @@ import java.util.Spliterators; import java.util.stream.Stream; import java.util.stream.StreamSupport; +import static com.yahoo.yolean.Exceptions.uncheck; + /** * Extra utilities/operations on slime trees. * @@ -31,34 +34,16 @@ public class SlimeUtils { } - private static void setObjectEntry(Inspector from, String name, Cursor to) { + public static void setObjectEntry(Inspector from, String name, Cursor to) { switch (from.type()) { - case NIX: - to.setNix(name); - break; - case BOOL: - to.setBool(name, from.asBool()); - break; - case LONG: - to.setLong(name, from.asLong()); - break; - case DOUBLE: - to.setDouble(name, from.asDouble()); - break; - case STRING: - to.setString(name, from.asString()); - break; - case DATA: - to.setData(name, from.asData()); - break; - case ARRAY: - Cursor array = to.setArray(name); - copyArray(from, array); - break; - case OBJECT: - Cursor object = to.setObject(name); - copyObject(from, object); - break; + case NIX -> to.setNix(name); + case BOOL -> to.setBool(name, from.asBool()); + case LONG -> to.setLong(name, from.asLong()); + case DOUBLE -> to.setDouble(name, from.asDouble()); + case STRING -> to.setString(name, from.asString()); + case DATA -> to.setData(name, from.asData()); + case ARRAY -> copyArray(from, to.setArray(name)); + case OBJECT -> copyObject(from, to.setObject(name)); } } @@ -71,32 +56,14 @@ public class SlimeUtils { private static void addValue(Inspector from, Cursor to) { switch (from.type()) { - case NIX: - to.addNix(); - break; - case BOOL: - to.addBool(from.asBool()); - break; - case LONG: - to.addLong(from.asLong()); - break; - case DOUBLE: - to.addDouble(from.asDouble()); - break; - case STRING: - to.addString(from.asString()); - break; - case DATA: - to.addData(from.asData()); - break; - case ARRAY: - Cursor array = to.addArray(); - copyArray(from, array); - break; - case OBJECT: - Cursor object = to.addObject(); - copyObject(from, object); - break; + case NIX -> to.addNix(); + case BOOL -> to.addBool(from.asBool()); + case LONG -> to.addLong(from.asLong()); + case DOUBLE -> to.addDouble(from.asDouble()); + case STRING -> to.addString(from.asString()); + case DATA -> to.addData(from.asData()); + case ARRAY -> copyArray(from, to.addArray()); + case OBJECT -> copyObject(from, to.addObject()); } } @@ -115,6 +82,21 @@ public class SlimeUtils { return baos.toByteArray(); } + public static String toJson(Slime slime) { + return toJson(slime.get()); + } + + public static String toJson(Inspector inspector) { + return toJson(inspector, true); + } + + public static String toJson(Inspector inspector, boolean compact) { + var outputStream = new ByteArrayOutputStream(); + var jsonFormat = new JsonFormat(compact ? 0 : 2); + uncheck(() -> jsonFormat.encode(outputStream, inspector)); + return outputStream.toString(StandardCharsets.UTF_8); + } + public static Slime jsonToSlime(byte[] json) { Slime slime = new Slime(); new JsonDecoder().decode(slime, json); |