Merge pull request #6593 from vespa-engine/bjorncs/use-ssl-socket-factory-node-admin

Bjorncs/use ssl socket factory node admin

8 files changed, 47 insertions, 191 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
index aea44e728ad..be82e615f7a 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
@@ -4,13 +4,13 @@ package com.yahoo.vespa.hosted.node.admin.configserver;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.yahoo.config.provision.HostName;
-import com.yahoo.vespa.athenz.api.AthenzService;
-import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider;
+import com.yahoo.vespa.athenz.identity.ServiceIdentitySslSocketFactory;
 import com.yahoo.vespa.athenz.identity.SiaIdentityProvider;
 import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier;
 import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo;
 import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 import org.apache.http.HttpHeaders;
+import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpDelete;
 import org.apache.http.client.methods.HttpGet;
@@ -19,11 +19,17 @@ import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpPut;
 import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.config.Registry;
+import org.apache.http.config.RegistryBuilder;
+import org.apache.http.conn.socket.ConnectionSocketFactory;
+import org.apache.http.conn.socket.PlainConnectionSocketFactory;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 
 import javax.net.ssl.HostnameVerifier;
-import javax.net.ssl.SSLContext;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
@@ -33,6 +39,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
+import static com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil.uncheck;
 import static java.util.Collections.singleton;
 
 /**
@@ -40,6 +47,7 @@ import static java.util.Collections.singleton;
  * content-type application/json
  *
  * @author dybdahl
+ * @author bjorncs
  */
 public class ConfigServerApiImpl implements ConfigServerApi {
     private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerApiImpl.class);
@@ -48,15 +56,7 @@ public class ConfigServerApiImpl implements ConfigServerApi {
 
     private final List<URI> configServers;
 
-    private Runnable runOnClose = () -> {};
-
-    /**
-     * The 'client' may be periodically re-created through calls to setSSLConnectionSocketFactory.
-     *
-     * The 'client' reference must be volatile because it is set and read in different threads, and visibility
-     * of changes is only guaranteed for volatile variables.
-     */
-    private volatile SelfCloseableHttpClient client;
+    private final CloseableHttpClient client;
 
     public static ConfigServerApiImpl create(ConfigServerInfo info, SiaIdentityProvider provider) {
         return new ConfigServerApiImpl(
@@ -77,29 +77,20 @@ public class ConfigServerApiImpl implements ConfigServerApi {
     private ConfigServerApiImpl(Collection<URI> configServers,
                                 HostnameVerifier verifier,
                                 SiaIdentityProvider identityProvider) {
-        this(configServers, createClient(identityProvider.getIdentitySslContext(), verifier));
-
-        // Register callback for updates to the SSLContext
-        ServiceIdentityProvider.Listener listener = (SSLContext sslContext, AthenzService identity) -> {
-            this.client = createClient(sslContext, verifier);
-        };
-        identityProvider.addIdentityListener(listener);
-        this.runOnClose = () -> identityProvider.removeIdentityListener(listener);
+        this(configServers, createClient(new SSLConnectionSocketFactory(new ServiceIdentitySslSocketFactory(identityProvider), verifier)));
     }
 
-    private ConfigServerApiImpl(Collection<URI> configServers, SelfCloseableHttpClient client) {
+    private ConfigServerApiImpl(Collection<URI> configServers, CloseableHttpClient client) {
         this.configServers = randomizeConfigServerUris(configServers);
         this.client = client;
     }
 
-    public static ConfigServerApiImpl createForTestingWithSocketFactory(
-            List<URI> configServerHosts,
-            SSLConnectionSocketFactory socketFactory) {
-        return new ConfigServerApiImpl(configServerHosts, new SelfCloseableHttpClient(socketFactory));
+    public static ConfigServerApiImpl createForTesting(List<URI> configServerHosts) {
+        return new ConfigServerApiImpl(configServerHosts, createClient(SSLConnectionSocketFactory.getSocketFactory()));
     }
 
     static ConfigServerApiImpl createForTestingWithClient(List<URI> configServerHosts,
-                                                          SelfCloseableHttpClient client) {
+                                                          CloseableHttpClient client) {
         return new ConfigServerApiImpl(configServerHosts, client);
     }
 
@@ -183,18 +174,38 @@ public class ConfigServerApiImpl implements ConfigServerApi {
 
     @Override
     public void close() {
-        runOnClose.run();
-        client.close();
+        uncheck(client::close);
     }
 
     private void setContentTypeToApplicationJson(HttpRequestBase request) {
         request.setHeader(HttpHeaders.CONTENT_TYPE, "application/json");
     }
 
-    private static SelfCloseableHttpClient createClient(
-            SSLContext sslContext, HostnameVerifier configServerVerifier) {
-        return new SelfCloseableHttpClient(
-                new SSLConnectionSocketFactory(sslContext, configServerVerifier));
+    private static CloseableHttpClient createClient(SSLConnectionSocketFactory socketFactory) {
+        Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create()
+                .register("http", PlainConnectionSocketFactory.getSocketFactory())
+                .register("https", socketFactory)
+                .build();
+
+        PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager(socketFactoryRegistry);
+        cm.setMaxTotal(200); // Increase max total connections to 200, which should be enough
+
+        // Have experienced hang in socket read, which may have been because of
+        // system defaults, therefore set explicit timeouts. Set arbitrarily to
+        // 15s > 10s used by Orchestrator lock timeout.
+        int timeoutMs = 15_000;
+        RequestConfig requestBuilder = RequestConfig.custom()
+                .setConnectTimeout(timeoutMs) // establishment of connection
+                .setConnectionRequestTimeout(timeoutMs) // connection from connection manager
+                .setSocketTimeout(timeoutMs) // waiting for data
+                .build();
+
+        return HttpClientBuilder.create()
+                .setDefaultRequestConfig(requestBuilder)
+                .disableAutomaticRetries()
+                .setUserAgent("node-admin")
+                .setConnectionManager(cm)
+                .build();
     }
 
     // Shuffle config server URIs to balance load
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java
deleted file mode 100644
index 38c111bc3ba..00000000000
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java
+++ /dev/null
@@ -1,78 +0,0 @@
-// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.node.admin.configserver;
-
-import com.yahoo.log.LogLevel;
-import org.apache.http.client.config.RequestConfig;
-import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.config.Registry;
-import org.apache.http.config.RegistryBuilder;
-import org.apache.http.conn.socket.ConnectionSocketFactory;
-import org.apache.http.conn.socket.PlainConnectionSocketFactory;
-import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClientBuilder;
-import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
-
-import java.io.IOException;
-import java.util.logging.Logger;
-
-/**
- * @author freva
- */
-class SelfCloseableHttpClient implements AutoCloseable {
-
-    private static final Logger log = Logger.getLogger(SelfCloseableHttpClient.class.getName());
-
-    private final CloseableHttpClient httpClient;
-
-    SelfCloseableHttpClient() {
-        this(SSLConnectionSocketFactory.getSocketFactory());
-    }
-
-    SelfCloseableHttpClient(SSLConnectionSocketFactory sslConnectionSocketFactory) {
-        Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create()
-                .register("http", PlainConnectionSocketFactory.getSocketFactory())
-                .register("https", sslConnectionSocketFactory)
-                .build();
-
-        PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager(socketFactoryRegistry);
-        cm.setMaxTotal(200); // Increase max total connections to 200, which should be enough
-
-        // Have experienced hang in socket read, which may have been because of
-        // system defaults, therefore set explicit timeouts. Set arbitrarily to
-        // 15s > 10s used by Orchestrator lock timeout.
-        int timeoutMs = 15_000;
-        RequestConfig requestBuilder = RequestConfig.custom()
-                .setConnectTimeout(timeoutMs) // establishment of connection
-                .setConnectionRequestTimeout(timeoutMs) // connection from connection manager
-                .setSocketTimeout(timeoutMs) // waiting for data
-                .build();
-
-        this.httpClient = HttpClientBuilder.create()
-                .setDefaultRequestConfig(requestBuilder)
-                .disableAutomaticRetries()
-                .setUserAgent("node-admin")
-                .setConnectionManager(cm).build();
-    }
-
-    public CloseableHttpResponse execute(HttpUriRequest request) throws IOException {
-        return httpClient.execute(request);
-    }
-
-    @Override
-    @SuppressWarnings("deprecation")  // finalize() is deprecated from Java 9
-    public void finalize() throws Throwable {
-        close();
-        super.finalize();
-    }
-
-    @Override
-    public void close() {
-        try {
-            httpClient.close();
-        } catch (Exception e) {
-            log.log(LogLevel.WARNING, "Ignoring exception thrown when closing http client", e);
-        }
-    }
-}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java
index a4230e4dc8d..da7b1e6a00c 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java
@@ -7,6 +7,7 @@ import org.apache.http.HttpVersion;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.entity.BasicHttpEntity;
+import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.message.BasicStatusLine;
 import org.junit.Before;
 import org.junit.Test;
@@ -54,7 +55,7 @@ public class ConfigServerApiImplTest {
 
     @Before
     public void initExecutor() throws IOException {
-        SelfCloseableHttpClient httpMock = mock(SelfCloseableHttpClient.class);
+        CloseableHttpClient httpMock = mock(CloseableHttpClient.class);
         when(httpMock.execute(any())).thenAnswer(invocationOnMock -> {
             HttpGet get = (HttpGet) invocationOnMock.getArguments()[0];
             mockLog.append(get.getMethod()).append(" ").append(get.getURI()).append("  ");
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
index 045b1116740..85b62660687 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java
@@ -10,7 +10,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi;
 import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApiImpl;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.testutils.ContainerConfig;
-import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -67,9 +66,8 @@ public class RealNodeRepositoryTest {
             try {
                 final int port = findRandomOpenPort();
                 container = JDisc.fromServicesXml(ContainerConfig.servicesXmlV2(port), Networking.enable);
-                ConfigServerApi configServerApi = ConfigServerApiImpl.createForTestingWithSocketFactory(
-                        Collections.singletonList(URI.create("http://127.0.0.1:" + port)),
-                        SSLConnectionSocketFactory.getSocketFactory());
+                ConfigServerApi configServerApi = ConfigServerApiImpl.createForTesting(
+                        Collections.singletonList(URI.create("http://127.0.0.1:" + port)));
                 waitForJdiscContainerToServe(configServerApi);
                 return;
             } catch (RuntimeException e) {
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/ServiceIdentityProvider.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/ServiceIdentityProvider.java
index f945783cf8a..6b318fb16be 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/ServiceIdentityProvider.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/ServiceIdentityProvider.java
@@ -15,10 +15,4 @@ import javax.net.ssl.SSLContext;
 public interface ServiceIdentityProvider {
     AthenzService identity();
     SSLContext getIdentitySslContext();
-    void addIdentityListener(Listener listener);
-    void removeIdentityListener(Listener listener);
-
-    interface Listener {
-        void onCredentialsUpdate(SSLContext sslContext, AthenzService identity);
-    }
 }
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/ServiceIdentityProviderListenerHelper.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/ServiceIdentityProviderListenerHelper.java
deleted file mode 100644
index bf50673fab8..00000000000
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/ServiceIdentityProviderListenerHelper.java
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.athenz.identity;
-
-import com.yahoo.vespa.athenz.api.AthenzService;
-
-import javax.net.ssl.SSLContext;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
-
-/**
- * A helper class managing {@link ServiceIdentityProvider.Listener} instances for implementations of {@link ServiceIdentityProvider}.
- *
- * @author bjorncs
- */
-public class ServiceIdentityProviderListenerHelper {
-
-    private final Set<ServiceIdentityProvider.Listener> listeners = new CopyOnWriteArraySet<>();
-    private final AthenzService identity;
-
-    public ServiceIdentityProviderListenerHelper(AthenzService identity) {
-        this.identity = identity;
-    }
-
-    public void addIdentityListener(ServiceIdentityProvider.Listener listener) {
-        listeners.add(listener);
-    }
-
-    public void removeIdentityListener(ServiceIdentityProvider.Listener listener) {
-        listeners.remove(listener);
-    }
-
-    public void onCredentialsUpdate(SSLContext sslContext) {
-        listeners.forEach(l -> l.onCredentialsUpdate(sslContext, identity));
-    }
-
-    public void clearListeners() {
-        listeners.clear();
-    }
-
-}
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/SiaIdentityProvider.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/SiaIdentityProvider.java
index ebff56a6f48..b06ae089b2a 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/SiaIdentityProvider.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identity/SiaIdentityProvider.java
@@ -7,7 +7,6 @@ import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.athenz.api.AthenzService;
 import com.yahoo.vespa.athenz.tls.KeyStoreType;
 import com.yahoo.vespa.athenz.tls.SslContextBuilder;
-import com.yahoo.vespa.athenz.utils.AthenzIdentities;
 import com.yahoo.vespa.athenz.utils.SiaUtils;
 
 import javax.net.ssl.SSLContext;
@@ -39,7 +38,6 @@ public class SiaIdentityProvider extends AbstractComponent implements ServiceIde
     private final File certificateFile;
     private final File trustStoreFile;
     private final ScheduledExecutorService scheduler;
-    private final ServiceIdentityProviderListenerHelper listenerHelper;
 
     @Inject
     public SiaIdentityProvider(SiaProviderConfig config) {
@@ -71,7 +69,6 @@ public class SiaIdentityProvider extends AbstractComponent implements ServiceIde
         this.trustStoreFile = trustStoreFile;
         this.scheduler = scheduler;
         this.sslContext.set(createIdentitySslContext());
-        this.listenerHelper = new ServiceIdentityProviderListenerHelper(service);
         scheduler.scheduleAtFixedRate(this::reloadSslContext, REFRESH_INTERVAL.toMinutes(), REFRESH_INTERVAL.toMinutes(), TimeUnit.MINUTES);
     }
 
@@ -93,16 +90,6 @@ public class SiaIdentityProvider extends AbstractComponent implements ServiceIde
         return sslContext.get();
     }
 
-    @Override
-    public void addIdentityListener(Listener listener) {
-        listenerHelper.addIdentityListener(listener);
-    }
-
-    @Override
-    public void removeIdentityListener(Listener listener) {
-        listenerHelper.removeIdentityListener(listener);
-    }
-
     private SSLContext createIdentitySslContext() {
         return new SslContextBuilder()
                 .withTrustStore(trustStoreFile, KeyStoreType.JKS)
@@ -115,7 +102,6 @@ public class SiaIdentityProvider extends AbstractComponent implements ServiceIde
         try {
             SSLContext sslContext = createIdentitySslContext();
             this.sslContext.set(sslContext);
-            listenerHelper.onCredentialsUpdate(sslContext);
         } catch (Exception e) {
             log.log(LogLevel.SEVERE, "Failed to update SSLContext: " + e.getMessage(), e);
         }
@@ -127,7 +113,6 @@ public class SiaIdentityProvider extends AbstractComponent implements ServiceIde
         try {
             scheduler.shutdownNow();
             scheduler.awaitTermination(90, TimeUnit.SECONDS);
-            listenerHelper.clearListeners();
         } catch (InterruptedException e) {
             throw new RuntimeException(e);
         }
diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java
index e40a0933002..266e2ebcefd 100644
--- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java
+++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java
@@ -18,7 +18,6 @@ import com.yahoo.vespa.athenz.api.ZToken;
 import com.yahoo.vespa.athenz.client.zts.DefaultZtsClient;
 import com.yahoo.vespa.athenz.client.zts.ZtsClient;
 import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider;
-import com.yahoo.vespa.athenz.identity.ServiceIdentityProviderListenerHelper;
 import com.yahoo.vespa.athenz.identity.SiaIdentityProvider;
 import com.yahoo.vespa.athenz.tls.KeyStoreType;
 import com.yahoo.vespa.athenz.tls.SslContextBuilder;
@@ -63,7 +62,6 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen
     private final ScheduledExecutorService scheduler;
     private final Clock clock;
     private final AthenzService identity;
-    private final ServiceIdentityProviderListenerHelper listenerHelper;
     private final String dnsSuffix;
     private final URI ztsEndpoint;
 
@@ -96,7 +94,6 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen
         this.scheduler = scheduler;
         this.clock = clock;
         this.identity = new AthenzService(config.domain(), config.service());
-        this.listenerHelper = new ServiceIdentityProviderListenerHelper(this.identity);
         this.dnsSuffix = config.athenzDnsSuffix();
         this.ztsEndpoint = URI.create(config.ztsUrl());
         roleSslContextCache = createCache(ROLE_SSL_CONTEXT_EXPIRY, this::createRoleSslContext);
@@ -148,16 +145,6 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen
     }
 
     @Override
-    public void addIdentityListener(Listener listener) {
-        listenerHelper.addIdentityListener(listener);
-    }
-
-    @Override
-    public void removeIdentityListener(Listener listener) {
-        listenerHelper.removeIdentityListener(listener);
-    }
-
-    @Override
     public SSLContext getRoleSslContext(String domain, String role) {
         // This ssl context should ideally be cached as it is quite expensive to create.
         try {
@@ -216,7 +203,6 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen
         try {
             scheduler.shutdownNow();
             scheduler.awaitTermination(AWAIT_TERMINTATION_TIMEOUT.getSeconds(), TimeUnit.SECONDS);
-            listenerHelper.clearListeners();
         } catch (InterruptedException e) {
             throw new RuntimeException(e);
         }
@@ -244,7 +230,6 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen
             credentials = isExpired(credentials)
                     ? athenzCredentialsService.registerInstance()
                     : athenzCredentialsService.updateCredentials(credentials.getIdentityDocument(), credentials.getIdentitySslContext());
-            listenerHelper.onCredentialsUpdate(credentials.getIdentitySslContext());
         } catch (Throwable t) {
             log.log(LogLevel.WARNING, "Failed to update credentials: " + t.getMessage(), t);
         }