Convert NetconfSalKeystoreService into a component

This is a very simple component, activate it as part of
sal-netconf-connector rather than from topology blueprints.

Also remove the initial merge transaction, as it does not (and has not
for a long time) introduce any semantic state.

JIRA: NETCONF-960
Change-Id: Ibc182675bbd31eb970be6b2df0559fe5c852090e
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/apps/netconf-topology-impl/src/main/resources/OSGI-INF/blueprint/netconf-topology.xml b/apps/netconf-topology-impl/src/main/resources/OSGI-INF/blueprint/netconf-topology.xml
index 0534272754be2c83504a5c4996e15ee3d1f4226f..dcc083419b40b7ecf52d156a49b104ac7bcbf0fd 100644 (file)
--- a/apps/netconf-topology-impl/src/main/resources/OSGI-INF/blueprint/netconf-topology.xml
+++ b/apps/netconf-topology-impl/src/main/resources/OSGI-INF/blueprint/netconf-topology.xml
@@ -53,13 +53,4 @@
         <argument ref="baseSchemas"/>
         <argument ref="deviceActionFactory"/>
     </bean>
-
-    <bean id="netconfKeystoreProvider"
-          class="org.opendaylight.netconf.sal.connect.util.NetconfSalKeystoreService">
-        <argument ref="dataBroker"/>
-        <argument ref="encryptionService"/>
-    </bean>
-
-    <odl:rpc-implementation ref="netconfKeystoreProvider"/>
-
 </blueprint>

diff --git a/apps/netconf-topology-singleton/src/main/resources/OSGI-INF/blueprint/netconf-topology-singleton.xml b/apps/netconf-topology-singleton/src/main/resources/OSGI-INF/blueprint/netconf-topology-singleton.xml
index 422465d7a70955b2435347a308fa44dc1a03546d..b2353f344bd243c01d9d7ca14eb63a3d19d81909 100644 (file)
--- a/apps/netconf-topology-singleton/src/main/resources/OSGI-INF/blueprint/netconf-topology-singleton.xml
+++ b/apps/netconf-topology-singleton/src/main/resources/OSGI-INF/blueprint/netconf-topology-singleton.xml
@@ -74,13 +74,4 @@ and is available at http://www.eclipse.org/legal/epl-v10.html
     </bean>
     <service ref="netconfTopologyManager"
              interface="org.opendaylight.netconf.topology.singleton.api.NetconfTopologySingletonService"/>
-
-    <bean id="netconfKeystoreProvider"
-          class="org.opendaylight.netconf.sal.connect.util.NetconfSalKeystoreService">
-        <argument ref="dataBroker"/>
-        <argument ref="encryptionService"/>
-    </bean>
-
-    <odl:rpc-implementation ref="netconfKeystoreProvider"/>
-
 </blueprint>

diff --git a/plugins/sal-netconf-connector/pom.xml b/plugins/sal-netconf-connector/pom.xml
index 931003327c4eab40e287f5a0c177db7a04490a52..d131fbb89fa8c60e3ddf18db749a6196466bdcf8 100644 (file)
--- a/plugins/sal-netconf-connector/pom.xml
+++ b/plugins/sal-netconf-connector/pom.xml
@@ -96,6 +96,12 @@
       <artifactId>javax.inject</artifactId>
       <optional>true</optional>
     </dependency>
+    <dependency>
+      <groupId>jakarta.annotation</groupId>
+      <artifactId>jakarta.annotation-api</artifactId>
+      <scope>provided</scope>
+      <optional>true</optional>
+    </dependency>
     <dependency>
       <groupId>org.osgi</groupId>
       <artifactId>org.osgi.service.component.annotations</artifactId>

diff --git a/plugins/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/util/NetconfSalKeystoreService.java b/plugins/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/util/NetconfSalKeystoreService.java
index 8c28de8667757a1c71e25c0e5014a605cd50af46..f97958b1832fc0e696a899c4f9926bf2b061b972 100644 (file)
--- a/plugins/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/util/NetconfSalKeystoreService.java
+++ b/plugins/sal-netconf-connector/src/main/java/org/opendaylight/netconf/sal/connect/util/NetconfSalKeystoreService.java
@@ -7,15 +7,20 @@
  */
 package org.opendaylight.netconf.sal.connect.util;
 
+import static java.util.Objects.requireNonNull;
+
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.SettableFuture;
 import java.util.List;
-import java.util.concurrent.ExecutionException;
 import java.util.stream.Collectors;
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.inject.Singleton;
 import org.opendaylight.aaa.encrypt.AAAEncryptionService;
 import org.opendaylight.mdsal.binding.api.DataBroker;
+import org.opendaylight.mdsal.binding.api.RpcProviderService;
 import org.opendaylight.mdsal.binding.api.WriteTransaction;
 import org.opendaylight.mdsal.common.api.CommitInfo;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
@@ -29,7 +34,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.AddTrustedCertificateOutput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.AddTrustedCertificateOutputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.Keystore;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.KeystoreBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.NetconfKeystoreService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.RemoveKeystoreEntryInput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.RemoveKeystoreEntryOutput;
@@ -47,43 +51,46 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.keystore.entry.KeyCredentialKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.trusted.certificates.TrustedCertificate;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.trusted.certificates.TrustedCertificateKey;
+import org.opendaylight.yangtools.concepts.Registration;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.opendaylight.yangtools.yang.common.RpcResultBuilder;
+import org.osgi.service.component.annotations.Activate;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
+import org.osgi.service.component.annotations.Reference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class NetconfSalKeystoreService implements NetconfKeystoreService {
-
+@Singleton
+@Component(service = { })
+public final class NetconfSalKeystoreService implements NetconfKeystoreService, AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(NetconfSalKeystoreService.class);
+    private static final InstanceIdentifier<Keystore> KEYSTORE_IID = InstanceIdentifier.create(Keystore.class);
 
+    // FIXME: we are populating config datastore, but there may be risks with concurrent access. We really should be
+    //        using cluster singleton service here.
     private final DataBroker dataBroker;
     private final AAAEncryptionService encryptionService;
+    private final Registration reg;
 
-    private final InstanceIdentifier<Keystore> keystoreIid = InstanceIdentifier.create(Keystore.class);
-
-    public NetconfSalKeystoreService(final DataBroker dataBroker,
-                                     final AAAEncryptionService encryptionService) {
-        LOG.info("Starting NETCONF keystore service.");
+    @Inject
+    @Activate
+    public NetconfSalKeystoreService(@Reference final DataBroker dataBroker,
+            @Reference final AAAEncryptionService encryptionService, @Reference final RpcProviderService rpcProvider) {
+        this.dataBroker = requireNonNull(dataBroker);
+        this.encryptionService = requireNonNull(encryptionService);
 
-        this.dataBroker = dataBroker;
-        this.encryptionService = encryptionService;
-
-        initKeystore();
+        reg = rpcProvider.registerRpcImplementation(NetconfKeystoreService.class, this);
+        LOG.info("NETCONF keystore service started");
     }
 
-    private void initKeystore() {
-        final Keystore keystore = new KeystoreBuilder().build();
-
-        final WriteTransaction writeTransaction = dataBroker.newWriteOnlyTransaction();
-        writeTransaction.merge(LogicalDatastoreType.CONFIGURATION, keystoreIid, keystore);
-
-        try {
-            writeTransaction.commit().get();
-            LOG.debug("init keystore done");
-        } catch (InterruptedException | ExecutionException exception) {
-            LOG.error("Unable to initialize Netconf key-pair store.", exception);
-        }
+    @PreDestroy
+    @Deactivate
+    @Override
+    public void close() {
+        reg.close();
+        LOG.info("NETCONF keystore service stopped");
     }
 
     @Override
@@ -95,7 +102,7 @@ public class NetconfSalKeystoreService implements NetconfKeystoreService {
 
         for (final String id : input.getKeyId()) {
             writeTransaction.delete(LogicalDatastoreType.CONFIGURATION,
-                    keystoreIid.child(KeyCredential.class, new KeyCredentialKey(id)));
+                    KEYSTORE_IID.child(KeyCredential.class, new KeyCredentialKey(id)));
         }
 
         final SettableFuture<RpcResult<RemoveKeystoreEntryOutput>> rpcResult = SettableFuture.create();
@@ -131,7 +138,7 @@ public class NetconfSalKeystoreService implements NetconfKeystoreService {
 
         for (KeyCredential keypair : keypairs) {
             writeTransaction.merge(LogicalDatastoreType.CONFIGURATION,
-                    keystoreIid.child(KeyCredential.class, keypair.key()), keypair);
+                    KEYSTORE_IID.child(KeyCredential.class, keypair.key()), keypair);
         }
 
         final SettableFuture<RpcResult<AddKeystoreEntryOutput>> rpcResult = SettableFuture.create();
@@ -160,7 +167,7 @@ public class NetconfSalKeystoreService implements NetconfKeystoreService {
 
         for (TrustedCertificate certificate : input.nonnullTrustedCertificate().values()) {
             writeTransaction.merge(LogicalDatastoreType.CONFIGURATION,
-                    keystoreIid.child(TrustedCertificate.class, certificate.key()), certificate);
+                    KEYSTORE_IID.child(TrustedCertificate.class, certificate.key()), certificate);
         }
 
         final SettableFuture<RpcResult<AddTrustedCertificateOutput>> rpcResult = SettableFuture.create();
@@ -189,7 +196,7 @@ public class NetconfSalKeystoreService implements NetconfKeystoreService {
 
         for (final String name : input.getName()) {
             writeTransaction.delete(LogicalDatastoreType.CONFIGURATION,
-                    keystoreIid.child(TrustedCertificate.class, new TrustedCertificateKey(name)));
+                    KEYSTORE_IID.child(TrustedCertificate.class, new TrustedCertificateKey(name)));
         }
 
         final SettableFuture<RpcResult<RemoveTrustedCertificateOutput>> rpcResult = SettableFuture.create();
@@ -217,7 +224,7 @@ public class NetconfSalKeystoreService implements NetconfKeystoreService {
 
         for (PrivateKey key: input.nonnullPrivateKey().values()) {
             writeTransaction.merge(LogicalDatastoreType.CONFIGURATION,
-                    keystoreIid.child(PrivateKey.class, key.key()), key);
+                    KEYSTORE_IID.child(PrivateKey.class, key.key()), key);
         }
 
         final SettableFuture<RpcResult<AddPrivateKeyOutput>> rpcResult = SettableFuture.create();
@@ -245,7 +252,7 @@ public class NetconfSalKeystoreService implements NetconfKeystoreService {
 
         for (final String name : input.getName()) {
             writeTransaction.delete(LogicalDatastoreType.CONFIGURATION,
-                    keystoreIid.child(PrivateKey.class, new PrivateKeyKey(name)));
+                    KEYSTORE_IID.child(PrivateKey.class, new PrivateKeyKey(name)));
         }
 
         final SettableFuture<RpcResult<RemovePrivateKeyOutput>> rpcResult = SettableFuture.create();

diff --git a/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfSalKeystoreServiceTest.java b/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfSalKeystoreServiceTest.java
index 1aa7832a36b90d5ff84ef74fbabe80a1210d9c25..eb0218c7dcbef244994d916e85fdbd75082f5459 100644 (file)
--- a/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfSalKeystoreServiceTest.java
+++ b/plugins/sal-netconf-connector/src/test/java/org/opendaylight/netconf/sal/connect/netconf/util/NetconfSalKeystoreServiceTest.java
@@ -8,6 +8,7 @@
 package org.opendaylight.netconf.sal.connect.netconf.util;
 
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.times;
@@ -25,6 +26,7 @@ import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.opendaylight.aaa.encrypt.AAAEncryptionService;
 import org.opendaylight.mdsal.binding.api.DataBroker;
+import org.opendaylight.mdsal.binding.api.RpcProviderService;
 import org.opendaylight.mdsal.binding.api.WriteTransaction;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.netconf.api.xml.XmlUtil;
@@ -33,12 +35,14 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.AddPrivateKeyInputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.AddTrustedCertificateInput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.AddTrustedCertificateInputBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.NetconfKeystoreService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017._private.keys.PrivateKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017._private.keys.PrivateKeyBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017._private.keys.PrivateKeyKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.trusted.certificates.TrustedCertificate;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.trusted.certificates.TrustedCertificateBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netconf.keystore.rev171017.trusted.certificates.TrustedCertificateKey;
+import org.opendaylight.yangtools.concepts.ObjectRegistration;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.w3c.dom.Document;
@@ -61,36 +65,42 @@ public class NetconfSalKeystoreServiceTest {
     private DataBroker dataBroker;
     @Mock
     private AAAEncryptionService encryptionService;
+    @Mock
+    private RpcProviderService rpcProvider;
+    @Mock
+    private ObjectRegistration<?> rpcReg;
 
     @Before
     public void setUp() {
         doReturn(writeTx).when(dataBroker).newWriteOnlyTransaction();
         doNothing().when(writeTx)
             .merge(any(LogicalDatastoreType.class), any(InstanceIdentifier.class), any(DataObject.class));
+        doReturn(rpcReg).when(rpcProvider).registerRpcImplementation(eq(NetconfKeystoreService.class), any());
+        doNothing().when(rpcReg).close();
     }
 
     @Test
     public void testAddPrivateKey() throws Exception {
         doReturn(emptyFluentFuture()).when(writeTx).commit();
-        NetconfSalKeystoreService keystoreService = new NetconfSalKeystoreService(dataBroker, encryptionService);
+        try (var keystoreService = new NetconfSalKeystoreService(dataBroker, encryptionService, rpcProvider)) {
+            final AddPrivateKeyInput input = getPrivateKeyInput();
+            keystoreService.addPrivateKey(input);
 
-        final AddPrivateKeyInput input = getPrivateKeyInput();
-        keystoreService.addPrivateKey(input);
-
-        verify(writeTx, times(input.getPrivateKey().size() + 1))
-            .merge(any(LogicalDatastoreType.class), any(InstanceIdentifier.class), any(DataObject.class));
+            verify(writeTx, times(input.nonnullPrivateKey().size()))
+                .merge(any(LogicalDatastoreType.class), any(InstanceIdentifier.class), any(DataObject.class));
+        }
     }
 
     @Test
     public void testAddTrustedCertificate() throws Exception {
         doReturn(emptyFluentFuture()).when(writeTx).commit();
-        NetconfSalKeystoreService keystoreService = new NetconfSalKeystoreService(dataBroker, encryptionService);
-
-        final AddTrustedCertificateInput input = getTrustedCertificateInput();
-        keystoreService.addTrustedCertificate(input);
+        try (var keystoreService = new NetconfSalKeystoreService(dataBroker, encryptionService, rpcProvider)) {
+            final AddTrustedCertificateInput input = getTrustedCertificateInput();
+            keystoreService.addTrustedCertificate(input);
 
-        verify(writeTx, times(input.getTrustedCertificate().size() + 1))
-            .merge(any(LogicalDatastoreType.class), any(InstanceIdentifier.class), any(DataObject.class));
+            verify(writeTx, times(input.nonnullTrustedCertificate().size()))
+                .merge(any(LogicalDatastoreType.class), any(InstanceIdentifier.class), any(DataObject.class));
+        }
     }
 
     private AddPrivateKeyInput getPrivateKeyInput() throws Exception {