Merge branch '15107-alt-email'
authorTom Clegg <tclegg@veritasgenetics.com>
Thu, 14 Nov 2019 23:27:57 +0000 (18:27 -0500)
committerTom Clegg <tclegg@veritasgenetics.com>
Thu, 14 Nov 2019 23:27:57 +0000 (18:27 -0500)
refs #15107

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

lib/config/deprecated.go
lib/config/deprecated_test.go
sdk/java-v2/src/main/java/org/arvados/client/api/client/BaseApiClient.java
sdk/java-v2/src/main/java/org/arvados/client/api/client/factory/OkHttpClientFactory.java
sdk/java-v2/src/main/java/org/arvados/client/facade/ArvadosFacade.java
sdk/java-v2/src/test/java/org/arvados/client/api/client/factory/OkHttpClientFactoryTest.java
services/api/app/models/api_client.rb
services/api/test/unit/api_client_test.rb
services/dockercleaner/arvados-docker-cleaner.service

index 7b11e090eeee7479effd5c37b8c834798041c0a7..95116ec2e50c292a3fd4036d17f6d7603dbe7b5f 100644 (file)
@@ -370,27 +370,27 @@ const defaultKeepWebConfigPath = "/etc/arvados/keep-web/keep-web.yml"
 type oldKeepWebConfig struct {
        Client *arvados.Client
 
-       Listen string
+       Listen *string
 
-       AnonymousTokens    []string
-       AttachmentOnlyHost string
-       TrustAllContent    bool
+       AnonymousTokens    *[]string
+       AttachmentOnlyHost *string
+       TrustAllContent    *bool
 
        Cache struct {
-               TTL                  arvados.Duration
-               UUIDTTL              arvados.Duration
-               MaxCollectionEntries int
-               MaxCollectionBytes   int64
-               MaxPermissionEntries int
-               MaxUUIDEntries       int
+               TTL                  *arvados.Duration
+               UUIDTTL              *arvados.Duration
+               MaxCollectionEntries *int
+               MaxCollectionBytes   *int64
+               MaxPermissionEntries *int
+               MaxUUIDEntries       *int
        }
 
        // Hack to support old command line flag, which is a bool
        // meaning "get actual token from environment".
-       deprecatedAllowAnonymous bool
+       deprecatedAllowAnonymous *bool
 
        // Authorization token to be included in all health check requests.
-       ManagementToken string
+       ManagementToken *string
 }
 
 func (ldr *Loader) loadOldKeepWebConfig(cfg *arvados.Config) error {
@@ -412,22 +412,43 @@ func (ldr *Loader) loadOldKeepWebConfig(cfg *arvados.Config) error {
 
        loadOldClientConfig(cluster, oc.Client)
 
-       cluster.Services.WebDAV.InternalURLs[arvados.URL{Host: oc.Listen}] = arvados.ServiceInstance{}
-       cluster.Services.WebDAVDownload.InternalURLs[arvados.URL{Host: oc.Listen}] = arvados.ServiceInstance{}
-       cluster.Services.WebDAVDownload.ExternalURL = arvados.URL{Host: oc.AttachmentOnlyHost}
-       cluster.TLS.Insecure = oc.Client.Insecure
-       cluster.ManagementToken = oc.ManagementToken
-       cluster.Collections.TrustAllContent = oc.TrustAllContent
-       cluster.Collections.WebDAVCache.TTL = oc.Cache.TTL
-       cluster.Collections.WebDAVCache.UUIDTTL = oc.Cache.UUIDTTL
-       cluster.Collections.WebDAVCache.MaxCollectionEntries = oc.Cache.MaxCollectionEntries
-       cluster.Collections.WebDAVCache.MaxCollectionBytes = oc.Cache.MaxCollectionBytes
-       cluster.Collections.WebDAVCache.MaxPermissionEntries = oc.Cache.MaxPermissionEntries
-       cluster.Collections.WebDAVCache.MaxUUIDEntries = oc.Cache.MaxUUIDEntries
-       if len(oc.AnonymousTokens) > 0 {
-               cluster.Users.AnonymousUserToken = oc.AnonymousTokens[0]
-               if len(oc.AnonymousTokens) > 1 {
-                       ldr.Logger.Warn("More than 1 anonymous tokens configured, using only the first and discarding the rest.")
+       if oc.Listen != nil {
+               cluster.Services.WebDAV.InternalURLs[arvados.URL{Host: *oc.Listen}] = arvados.ServiceInstance{}
+               cluster.Services.WebDAVDownload.InternalURLs[arvados.URL{Host: *oc.Listen}] = arvados.ServiceInstance{}
+       }
+       if oc.AttachmentOnlyHost != nil {
+               cluster.Services.WebDAVDownload.ExternalURL = arvados.URL{Host: *oc.AttachmentOnlyHost}
+       }
+       if oc.ManagementToken != nil {
+               cluster.ManagementToken = *oc.ManagementToken
+       }
+       if oc.TrustAllContent != nil {
+               cluster.Collections.TrustAllContent = *oc.TrustAllContent
+       }
+       if oc.Cache.TTL != nil {
+               cluster.Collections.WebDAVCache.TTL = *oc.Cache.TTL
+       }
+       if oc.Cache.UUIDTTL != nil {
+               cluster.Collections.WebDAVCache.UUIDTTL = *oc.Cache.UUIDTTL
+       }
+       if oc.Cache.MaxCollectionEntries != nil {
+               cluster.Collections.WebDAVCache.MaxCollectionEntries = *oc.Cache.MaxCollectionEntries
+       }
+       if oc.Cache.MaxCollectionBytes != nil {
+               cluster.Collections.WebDAVCache.MaxCollectionBytes = *oc.Cache.MaxCollectionBytes
+       }
+       if oc.Cache.MaxPermissionEntries != nil {
+               cluster.Collections.WebDAVCache.MaxPermissionEntries = *oc.Cache.MaxPermissionEntries
+       }
+       if oc.Cache.MaxUUIDEntries != nil {
+               cluster.Collections.WebDAVCache.MaxUUIDEntries = *oc.Cache.MaxUUIDEntries
+       }
+       if oc.AnonymousTokens != nil {
+               if len(*oc.AnonymousTokens) > 0 {
+                       cluster.Users.AnonymousUserToken = (*oc.AnonymousTokens)[0]
+                       if len(*oc.AnonymousTokens) > 1 {
+                               ldr.Logger.Warn("More than 1 anonymous tokens configured, using only the first and discarding the rest.")
+                       }
                }
        }
 
@@ -439,11 +460,11 @@ const defaultGitHttpdConfigPath = "/etc/arvados/git-httpd/git-httpd.yml"
 
 type oldGitHttpdConfig struct {
        Client          *arvados.Client
-       Listen          string
-       GitCommand      string
-       GitoliteHome    string
-       RepoRoot        string
-       ManagementToken string
+       Listen          *string
+       GitCommand      *string
+       GitoliteHome    *string
+       RepoRoot        *string
+       ManagementToken *string
 }
 
 func (ldr *Loader) loadOldGitHttpdConfig(cfg *arvados.Config) error {
@@ -465,12 +486,21 @@ func (ldr *Loader) loadOldGitHttpdConfig(cfg *arvados.Config) error {
 
        loadOldClientConfig(cluster, oc.Client)
 
-       cluster.Services.GitHTTP.InternalURLs[arvados.URL{Host: oc.Listen}] = arvados.ServiceInstance{}
-       cluster.TLS.Insecure = oc.Client.Insecure
-       cluster.ManagementToken = oc.ManagementToken
-       cluster.Git.GitCommand = oc.GitCommand
-       cluster.Git.GitoliteHome = oc.GitoliteHome
-       cluster.Git.Repositories = oc.RepoRoot
+       if oc.Listen != nil {
+               cluster.Services.GitHTTP.InternalURLs[arvados.URL{Host: *oc.Listen}] = arvados.ServiceInstance{}
+       }
+       if oc.ManagementToken != nil {
+               cluster.ManagementToken = *oc.ManagementToken
+       }
+       if oc.GitCommand != nil {
+               cluster.Git.GitCommand = *oc.GitCommand
+       }
+       if oc.GitoliteHome != nil {
+               cluster.Git.GitoliteHome = *oc.GitoliteHome
+       }
+       if oc.RepoRoot != nil {
+               cluster.Git.Repositories = *oc.RepoRoot
+       }
 
        cfg.Clusters[cluster.ClusterID] = *cluster
        return nil
index ff1bb9434a42c8babc3cedef9165e7ad3d16d949..845c73c053629f6bceb77af9f317524d435e4ec3 100644 (file)
@@ -15,6 +15,9 @@ import (
        check "gopkg.in/check.v1"
 )
 
+// Configured at: sdk/python/tests/run_test_server.py
+const TestServerManagementToken = "e687950a23c3a9bceec28c6223a06c79"
+
 func testLoadLegacyConfig(content []byte, mungeFlag string, c *check.C) (*arvados.Cluster, error) {
        tmpfile, err := ioutil.TempFile("", "example")
        if err != nil {
@@ -133,6 +136,23 @@ func (s *LoadSuite) TestLegacyKeepWebConfig(c *check.C) {
        c.Check(cluster.ManagementToken, check.Equals, "xyzzy")
 }
 
+// Tests fix for https://dev.arvados.org/issues/15642
+func (s *LoadSuite) TestLegacyKeepWebConfigDoesntDisableMissingItems(c *check.C) {
+       content := []byte(`
+{
+       "Client": {
+               "Scheme": "",
+               "APIHost": "example.com",
+               "AuthToken": "abcdefg",
+       }
+}
+`)
+       cluster, err := testLoadLegacyConfig(content, "-legacy-keepweb-config", c)
+       c.Check(err, check.IsNil)
+       // The resulting ManagementToken should be the one set up on the test server.
+       c.Check(cluster.ManagementToken, check.Equals, TestServerManagementToken)
+}
+
 func (s *LoadSuite) TestLegacyKeepproxyConfig(c *check.C) {
        f := "-legacy-keepproxy-config"
        content := []byte(fmtKeepproxyConfig("", true))
@@ -217,6 +237,23 @@ func (s *LoadSuite) TestLegacyArvGitHttpdConfig(c *check.C) {
        c.Check(cluster.Services.Keepproxy.InternalURLs[arvados.URL{Host: ":9000"}], check.Equals, arvados.ServiceInstance{})
 }
 
+// Tests fix for https://dev.arvados.org/issues/15642
+func (s *LoadSuite) TestLegacyArvGitHttpdConfigDoesntDisableMissingItems(c *check.C) {
+       content := []byte(`
+{
+       "Client": {
+               "Scheme": "",
+               "APIHost": "example.com",
+               "AuthToken": "abcdefg",
+       }
+}
+`)
+       cluster, err := testLoadLegacyConfig(content, "-legacy-git-httpd-config", c)
+       c.Check(err, check.IsNil)
+       // The resulting ManagementToken should be the one set up on the test server.
+       c.Check(cluster.ManagementToken, check.Equals, TestServerManagementToken)
+}
+
 func (s *LoadSuite) TestLegacyKeepBalanceConfig(c *check.C) {
        f := "-legacy-keepbalance-config"
        content := []byte(fmtKeepBalanceConfig(""))
index 7e8a2979befaee7af09007bd51e3d0fbc878d312..a8d1a08cb09643262bf657498aefc53b727f168c 100644 (file)
@@ -34,9 +34,7 @@ abstract class BaseApiClient {
 
     BaseApiClient(ConfigProvider config) {
         this.config = config;
-        client = OkHttpClientFactory.builder()
-                .build()
-                .create(config.isApiHostInsecure());
+        this.client = OkHttpClientFactory.INSTANCE.create(config.isApiHostInsecure());
     }
 
     Request.Builder getRequestBuilder() {
index 0e95e661e7fccd1b24f433e2ace298effa9064c1..f9041c9281a0f4026e6136741dba7da04901fc05 100644 (file)
@@ -7,6 +7,7 @@
 
 package org.arvados.client.api.client.factory;
 
+import com.google.common.base.Suppliers;
 import okhttp3.OkHttpClient;
 import org.arvados.client.exception.ArvadosClientException;
 import org.slf4j.Logger;
@@ -19,31 +20,60 @@ import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 import java.security.cert.X509Certificate;
+import java.util.function.Supplier;
 
-public class OkHttpClientFactory {
-
+/**
+ * {@link OkHttpClient} instance factory that builds and configures client instances sharing
+ * the common resource pool: this is the recommended approach to optimize resource usage.
+ */
+public final class OkHttpClientFactory {
+    public static final OkHttpClientFactory INSTANCE = new OkHttpClientFactory();
     private final Logger log = org.slf4j.LoggerFactory.getLogger(OkHttpClientFactory.class);
+    private final OkHttpClient clientSecure = new OkHttpClient();
+    private final Supplier<OkHttpClient> clientUnsecure =
+            Suppliers.memoize(this::getDefaultClientAcceptingAllCertificates);
+
+    private OkHttpClientFactory() { /* singleton */}
 
-    OkHttpClientFactory() {
+    public OkHttpClient create(boolean apiHostInsecure) {
+        return apiHostInsecure ? getDefaultUnsecureClient() : getDefaultClient();
     }
 
-    public static OkHttpClientFactoryBuilder builder() {
-        return new OkHttpClientFactoryBuilder();
+    /**
+     * @return default secure {@link OkHttpClient} with shared resource pool.
+     */
+    public OkHttpClient getDefaultClient() {
+        return clientSecure;
     }
 
-    public OkHttpClient create(boolean apiHostInsecure) {
-        OkHttpClient.Builder builder = new OkHttpClient.Builder();
-        if (apiHostInsecure) {
-            trustAllCertificates(builder);
-        }
-        return builder.build();
+    /**
+     * @return default {@link OkHttpClient} with shared resource pool
+     * that will accept all SSL certificates by default.
+     */
+    public OkHttpClient getDefaultUnsecureClient() {
+        return clientUnsecure.get();
+    }
+
+    /**
+     * @return default {@link OkHttpClient.Builder} with shared resource pool.
+     */
+    public OkHttpClient.Builder getDefaultClientBuilder() {
+        return clientSecure.newBuilder();
+    }
+
+    /**
+     * @return default {@link OkHttpClient.Builder} with shared resource pool
+     * that is preconfigured to accept all SSL certificates.
+     */
+    public OkHttpClient.Builder getDefaultUnsecureClientBuilder() {
+        return clientUnsecure.get().newBuilder();
     }
 
-    private void trustAllCertificates(OkHttpClient.Builder builder) {
+    private OkHttpClient getDefaultClientAcceptingAllCertificates() {
         log.warn("Creating unsafe OkHttpClient. All SSL certificates will be accepted.");
         try {
             // Create a trust manager that does not validate certificate chains
-            final TrustManager[] trustAllCerts = new TrustManager[] { createX509TrustManager() };
+            final TrustManager[] trustAllCerts = {createX509TrustManager()};
 
             // Install the all-trusting trust manager
             SSLContext sslContext = SSLContext.getInstance("SSL");
@@ -51,8 +81,11 @@ public class OkHttpClientFactory {
             // Create an ssl socket factory with our all-trusting manager
             final SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
 
+            // Create the OkHttpClient.Builder with shared resource pool
+            final OkHttpClient.Builder builder = clientSecure.newBuilder();
             builder.sslSocketFactory(sslSocketFactory, (X509TrustManager) trustAllCerts[0]);
             builder.hostnameVerifier((hostname, session) -> true);
+            return builder.build();
         } catch (NoSuchAlgorithmException | KeyManagementException e) {
             throw new ArvadosClientException("Error establishing SSL context", e);
         }
@@ -60,30 +93,19 @@ public class OkHttpClientFactory {
 
     private static X509TrustManager createX509TrustManager() {
         return new X509TrustManager() {
-            
+
             @Override
-            public void checkClientTrusted(X509Certificate[] chain, String authType) {}
+            public void checkClientTrusted(X509Certificate[] chain, String authType) {
+            }
 
             @Override
-            public void checkServerTrusted(X509Certificate[] chain, String authType) {}
+            public void checkServerTrusted(X509Certificate[] chain, String authType) {
+            }
 
             @Override
             public X509Certificate[] getAcceptedIssuers() {
-                return new X509Certificate[] {};
+                return new X509Certificate[]{};
             }
         };
     }
-
-    public static class OkHttpClientFactoryBuilder {
-        OkHttpClientFactoryBuilder() {
-        }
-
-        public OkHttpClientFactory build() {
-            return new OkHttpClientFactory();
-        }
-
-        public String toString() {
-            return "OkHttpClientFactory.OkHttpClientFactoryBuilder()";
-        }
-    }
 }
index 858edf598b41e558951c6fd0c01ef02cad5eef3c..571cb2590906f9d041a342dbf26d95724184e3b0 100644 (file)
@@ -276,6 +276,18 @@ public class ArvadosFacade {
         return collectionsApiClient.list(listArgument);
     }
 
+    /**
+     * Gets project details by uuid.
+     *
+     * @param projectUuid uuid of project
+     * @return Group object containing information about project
+     */
+    public Group getProjectByUuid(String projectUuid) {
+        Group project = groupsApiClient.get(projectUuid);
+        log.debug("Retrieved " + project.getName() + " with UUID: " + project.getUuid());
+        return project;
+    }
+
     /**
      * Creates new project that will be a subproject of "home" for current user.
      *
index f7e18132941715a4340684e23183054730f3646c..f485d3bb02aff3e3faca87c77ef8af1f42c5e06d 100644 (file)
@@ -32,7 +32,7 @@ public class OkHttpClientFactoryTest extends ArvadosClientMockedWebServerTest {
     public void secureOkHttpClientIsCreated() throws Exception {
 
         // given
-        OkHttpClientFactory factory = OkHttpClientFactory.builder().build();
+        OkHttpClientFactory factory = OkHttpClientFactory.INSTANCE;
         // * configure HTTPS server
         SSLSocketFactory sf = getSSLSocketFactoryWithSelfSignedCertificate();
         server.useHttps(sf, false);
@@ -50,7 +50,7 @@ public class OkHttpClientFactoryTest extends ArvadosClientMockedWebServerTest {
     @Test
     public void insecureOkHttpClientIsCreated() throws Exception {
         // given
-        OkHttpClientFactory factory = OkHttpClientFactory.builder().build();
+        OkHttpClientFactory factory = OkHttpClientFactory.INSTANCE;
         // * configure HTTPS server
         SSLSocketFactory sf = getSSLSocketFactoryWithSelfSignedCertificate();
         server.useHttps(sf, false);
index 1f95d78c0c2446201c79e47f9200e3ae8123d5a4..8ed693f820d5eac0eff9389ac851166e800d6516 100644 (file)
@@ -13,4 +13,25 @@ class ApiClient < ArvadosModel
     t.add :url_prefix
     t.add :is_trusted
   end
+
+  def is_trusted
+    norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench1.ExternalURL) ||
+      norm(self.url_prefix) == norm(Rails.configuration.Services.Workbench2.ExternalURL) ||
+      super
+  end
+
+  protected
+
+  def norm url
+    # normalize URL for comparison
+    url = URI(url)
+    if url.scheme == "https"
+      url.port == "443"
+    end
+    if url.scheme == "http"
+      url.port == "80"
+    end
+    url.path = "/"
+    url
+  end
 end
index fc7d1ee2f429ffa30f885016d147b089889daf7b..df082c27fd8c35f7a8d1011bcd3faeba3d4bd4d8 100644 (file)
@@ -5,7 +5,27 @@
 require 'test_helper'
 
 class ApiClientTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  include CurrentApiClient
+
+  test "configured workbench is trusted" do
+    Rails.configuration.Services.Workbench1.ExternalURL = URI("http://wb1.example.com")
+    Rails.configuration.Services.Workbench2.ExternalURL = URI("https://wb2.example.com:443")
+
+    act_as_system_user do
+      [["http://wb0.example.com", false],
+       ["http://wb1.example.com", true],
+       ["http://wb2.example.com", false],
+       ["https://wb2.example.com", true],
+       ["https://wb2.example.com/", true],
+      ].each do |pfx, result|
+        a = ApiClient.create(url_prefix: pfx, is_trusted: false)
+        assert_equal result, a.is_trusted
+      end
+
+      a = ApiClient.create(url_prefix: "http://example.com", is_trusted: true)
+      a.save!
+      a.reload
+      assert a.is_trusted
+    end
+  end
 end
index fca8d8b1266eab91aa81ba47b5c06744c6a38e84..7e049144ae1ef49a700ad71580d50a48b0df144f 100644 (file)
@@ -6,7 +6,6 @@
 Description=Arvados Docker Image Cleaner
 Documentation=https://doc.arvados.org/
 After=network.target
-#AssertPathExists=/etc/arvados/docker-cleaner/docker-cleaner.json
 
 # systemd==229 (ubuntu:xenial) obeys StartLimitInterval in the [Unit] section
 StartLimitInterval=0