[homekit] Improve multiple instance management (#14016)

* [homekit] improve instance management

 * allow addressing individual instances for most console commands
 * don't restart all instances if simply adding/removing instances on
   config change
 * clear stored info when removing instances

* [homekit] reset instance identity when clearing pairings
* [homekit] log the actual interface we looked up

Signed-off-by: Cody Cutrer <cody@cutrer.us>

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/Homekit.java

@@ -13,7 +13,7 @@
 package org.openhab.io.homekit;
 
 import java.io.IOException;
-import java.util.List;
+import java.util.Collection;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 
@@ -45,17 +45,43 @@ public interface Homekit {
     void allowUnauthenticatedRequests(boolean allow);
 
     /**
-     * returns list of HomeKit accessories registered at bridge.
+     * returns list of HomeKit accessories registered on all bridge instances.
      */
-    List<HomekitAccessory> getAccessories();
+    Collection<HomekitAccessory> getAccessories();
 
     /**
-     * clear all pairings with HomeKit clients
+     * returns list of HomeKit accessories registered on a specific instance.
+     */
+    Collection<HomekitAccessory> getAccessories(int instance);
+
+    /**
+     * clear all pairings with HomeKit clients on all bridge instances.
      */
     void clearHomekitPairings();
 
+    /**
+     * clear all pairings with HomeKit clients for a specific instance.
+     * 
+     * @param instance the instance number (1-based)
+     */
+    void clearHomekitPairings(int instance);
+
     /**
      * Prune dummy accessories (accessories that no longer have associated items)
+     * on all bridge instances.
      */
     void pruneDummyAccessories();
+
+    /**
+     * Prune dummy accessories (accessories that no longer have associated items)
+     * for a specific instance
+     *
+     * @param instance the instance number (1-based)
+     */
+    void pruneDummyAccessories(int instance);
+
+    /**
+     * returns how many bridge instances there are
+     */
+    int getInstanceCount();
 }

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitAuthInfoImpl.java

@@ -147,15 +147,25 @@ public class HomekitAuthInfoImpl implements HomekitAuthInfo {
     }
 
     public void clear() {
-        logger.trace("clear all users");
         if (!this.blockUserDeletion) {
             for (String key : new HashSet<>(storage.getKeys())) {
                 if (isUserKey(key)) {
                     storage.remove(key);
                 }
             }
+            mac = HomekitServer.generateMac();
+            storage.put(STORAGE_MAC, mac);
+            storage.remove(STORAGE_SALT);
+            storage.remove(STORAGE_PRIVATE_KEY);
+            try {
+                initializeStorage();
+                logger.info("All users cleared from HomeKit bridge; re-pairing required.");
+            } catch (InvalidAlgorithmParameterException e) {
+                logger.warn(
+                        "Failed generating new encryption settings for HomeKit bridge; re-pairing required, but will likely fail.");
+            }
         } else {
-            logger.debug("deletion of users information was blocked by binding settings");
+            logger.warn("Deletion of HomeKit users was blocked by addon settings.");
         }
     }
 

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitCommandExtension.java

@@ -13,6 +13,7 @@
 package org.openhab.io.homekit.internal;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 
@@ -30,6 +31,7 @@ import org.osgi.service.component.annotations.Reference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import io.github.hapjava.accessories.HomekitAccessory;
 import io.github.hapjava.services.Service;
 
 /**
@@ -51,6 +53,9 @@ public class HomekitCommandExtension extends AbstractConsoleCommandExtension {
                     SUBCMD_ALLOW_UNAUTHENTICATED, SUBCMD_PRUNE_DUMMY_ACCESSORIES, SUBCMD_LIST_DUMMY_ACCESSORIES),
             false);
 
+    private static final String PARAM_INSTANCE = "--instance";
+    private static final String PARAM_INSTANCE_HELP = " [--instance <instance id>]";
+
     private class CommandCompleter implements ConsoleCommandCompleter {
         public boolean complete(String[] args, int cursorArgumentIndex, int cursorPosition, List<String> candidates) {
             if (cursorArgumentIndex == 0) {
@@ -70,37 +75,67 @@ public class HomekitCommandExtension extends AbstractConsoleCommandExtension {
     }
 
     @Override
-    public void execute(String[] args, Console console) {
-        if (args.length > 0) {
-            String subCommand = args[0];
+    public void execute(String[] argsArray, Console console) {
+        if (argsArray.length > 0) {
+            List<String> args = Arrays.asList(argsArray);
+            Integer instance = null;
+
+            // capture the common instance argument and take it out of args
+            for (int i = 0; i < args.size() - 1; ++i) {
+                if (PARAM_INSTANCE.equals(args.get(i))) {
+                    instance = Integer.parseInt(args.get(i + 1));
+                    int instanceCount = homekit.getInstanceCount();
+                    if (instance < 1 || instance > instanceCount) {
+                        console.println("Instance " + args.get(i + 1) + " out of range 1.." + instanceCount);
+                        return;
+                    }
+
+                    List<String> newArgs = args.subList(0, i);
+                    if (i < args.size() - 2) {
+                        newArgs.addAll(args.subList(i + 2, args.size() - 1));
+                    }
+                    args = newArgs;
+                    break;
+                }
+            }
+
+            String subCommand = args.get(0);
             switch (subCommand) {
                 case SUBCMD_CLEAR_PAIRINGS:
-                    clearHomekitPairings(console);
+                    if (args.size() != 1) {
+                        console.println("Unknown arguments; not clearing pairings");
+                    } else {
+                        clearHomekitPairings(console, instance);
+                    }
                     break;
 
                 case SUBCMD_ALLOW_UNAUTHENTICATED:
-                    if (args.length > 1) {
-                        boolean allow = Boolean.parseBoolean(args[1]);
+                    if (args.size() > 1) {
+                        boolean allow = Boolean.parseBoolean(args.get(1));
                         allowUnauthenticatedHomekitRequests(allow, console);
                     } else {
                         console.println("true/false is required as an argument");
                     }
                     break;
                 case SUBCMD_LIST_ACCESSORIES:
-                    listAccessories(console);
+                    listAccessories(console, instance);
                     break;
                 case SUBCMD_PRINT_ACCESSORY:
-                    if (args.length > 1) {
-                        printAccessory(args[1], console);
+                    if (args.size() > 1) {
+                        printAccessory(args.get(1), console, instance);
                     } else {
                         console.println("accessory id or name is required as an argument");
                     }
                     break;
                 case SUBCMD_PRUNE_DUMMY_ACCESSORIES:
-                    pruneDummyAccessories(console);
+                    if (args.size() != 1) {
+                        console.println("Unknown arguments; not pruning dummy accessories");
+                    } else {
+                        pruneDummyAccessories(console, instance);
+                    }
                     break;
                 case SUBCMD_LIST_DUMMY_ACCESSORIES:
-                    listDummyAccessories(console);
+                    listDummyAccessories(console, instance);
                     break;
                 default:
                     console.println("Unknown command '" + subCommand + "'");
@@ -114,16 +149,19 @@ public class HomekitCommandExtension extends AbstractConsoleCommandExtension {
 
     @Override
     public List<String> getUsages() {
-        return Arrays.asList(buildCommandUsage(SUBCMD_LIST_ACCESSORIES, "list all HomeKit accessories"),
-                buildCommandUsage(SUBCMD_PRINT_ACCESSORY + " <accessory id | accessory name>",
-                        "print additional details of the accessories which partially match provided ID or name."),
-                buildCommandUsage(SUBCMD_CLEAR_PAIRINGS, "removes all pairings with HomeKit clients."),
+        return Arrays.asList(
+                buildCommandUsage(SUBCMD_LIST_ACCESSORIES + PARAM_INSTANCE_HELP,
+                        "list all HomeKit accessories, optionally for a specific instance."),
+                buildCommandUsage(SUBCMD_PRINT_ACCESSORY + PARAM_INSTANCE_HELP + " <accessory id | accessory name>",
+                        "print additional details of the accessories which partially match provided ID or name, optionally searching a specific instance."),
+                buildCommandUsage(SUBCMD_CLEAR_PAIRINGS + PARAM_INSTANCE_HELP,
+                        "removes all pairings with HomeKit clients, optionally for a specific instance."),
                 buildCommandUsage(SUBCMD_ALLOW_UNAUTHENTICATED + " <boolean>",
                         "enables or disables unauthenticated access to facilitate debugging"),
-                buildCommandUsage(SUBCMD_PRUNE_DUMMY_ACCESSORIES,
-                        "removes dummy accessories whose items no longer exist."),
-                buildCommandUsage(SUBCMD_LIST_DUMMY_ACCESSORIES,
-                        "list dummy accessories whose items no longer exist."));
+                buildCommandUsage(SUBCMD_PRUNE_DUMMY_ACCESSORIES + PARAM_INSTANCE_HELP,
+                        "removes dummy accessories whose items no longer exist, optionally for a specific instance."),
+                buildCommandUsage(SUBCMD_LIST_DUMMY_ACCESSORIES + PARAM_INSTANCE_HELP,
+                        "list dummy accessories whose items no longer exist, optionally for a specific instance."));
     }
 
     @Reference
@@ -136,9 +174,14 @@ public class HomekitCommandExtension extends AbstractConsoleCommandExtension {
         return new CommandCompleter();
     }
 
-    private void clearHomekitPairings(Console console) {
-        homekit.clearHomekitPairings();
-        console.println("Cleared HomeKit pairings");
+    private void clearHomekitPairings(Console console, @Nullable Integer instance) {
+        if (instance != null) {
+            homekit.clearHomekitPairings(instance);
+            console.println("Cleared HomeKit pairings for instance " + instance);
+        } else {
+            homekit.clearHomekitPairings();
+            console.println("Cleared HomeKit pairings");
+        }
     }
 
     private void allowUnauthenticatedHomekitRequests(boolean allow, Console console) {
@@ -146,13 +189,18 @@ public class HomekitCommandExtension extends AbstractConsoleCommandExtension {
         console.println((allow ? "Enabled " : "Disabled ") + "unauthenticated HomeKit access");
     }
 
-    private void pruneDummyAccessories(Console console) {
-        homekit.pruneDummyAccessories();
-        console.println("Dummy accessories pruned.");
+    private void pruneDummyAccessories(Console console, @Nullable Integer instance) {
+        if (instance != null) {
+            homekit.pruneDummyAccessories(instance);
+            console.println("Dummy accessories pruned for instance " + instance);
+        } else {
+            homekit.pruneDummyAccessories();
+            console.println("Dummy accessories pruned");
+        }
     }
 
-    private void listAccessories(Console console) {
-        homekit.getAccessories().forEach(v -> {
+    private void listAccessories(Console console, @Nullable Integer instance) {
+        getInstanceAccessories(instance).forEach(v -> {
             try {
                 console.println(v.getId() + " " + v.getName().get());
             } catch (InterruptedException | ExecutionException e) {
@@ -161,8 +209,8 @@ public class HomekitCommandExtension extends AbstractConsoleCommandExtension {
         });
     }
 
-    private void listDummyAccessories(Console console) {
-        homekit.getAccessories().forEach(v -> {
+    private void listDummyAccessories(Console console, @Nullable Integer instance) {
+        getInstanceAccessories(instance).forEach(v -> {
             try {
                 if (v instanceof DummyHomekitAccessory) {
                     console.println(v.getSerialNumber().get());
@@ -191,8 +239,8 @@ public class HomekitCommandExtension extends AbstractConsoleCommandExtension {
         service.getLinkedServices().forEach((s) -> printService(console, s, indent + 2));
     }
 
-    private void printAccessory(String id, Console console) {
-        homekit.getAccessories().forEach(v -> {
+    private void printAccessory(String id, Console console, @Nullable Integer instance) {
+        getInstanceAccessories(instance).forEach(v -> {
             try {
                 if (("" + v.getId()).contains(id) || ((v.getName().get() != null)
                         && (v.getName().get().toUpperCase().contains(id.toUpperCase())))) {
@@ -206,4 +254,17 @@ public class HomekitCommandExtension extends AbstractConsoleCommandExtension {
             }
         });
     }
+
+    /**
+     * Get in-scope accessories
+     * 
+     * @param instance if null, means all accessories from all instances
+     */
+    private Collection<HomekitAccessory> getInstanceAccessories(@Nullable Integer instance) {
+        if (instance != null) {
+            return homekit.getAccessories(instance);
+        } else {
+            return homekit.getAccessories();
+        }
+    }
 }

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitImpl.java

@@ -17,10 +17,12 @@ import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.security.InvalidAlgorithmParameterException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Dictionary;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.ScheduledExecutorService;
 
 import javax.jmdns.JmDNS;
@@ -96,8 +98,7 @@ public class HomekitImpl implements Homekit, NetworkAddressChangeListener, Ready
     public HomekitImpl(@Reference StorageService storageService, @Reference ItemRegistry itemRegistry,
             @Reference NetworkAddressService networkAddressService, @Reference MetadataRegistry metadataRegistry,
             @Reference ConfigurationAdmin configAdmin, @Reference MDNSClient mdnsClient,
-            @Reference ReadyService readyService, Map<String, Object> properties)
-            throws IOException, InvalidAlgorithmParameterException {
+            @Reference ReadyService readyService, Map<String, Object> properties) {
         this.storageService = storageService;
         this.networkAddressService = networkAddressService;
         this.configAdmin = configAdmin;
@@ -160,14 +161,29 @@ public class HomekitImpl implements Homekit, NetworkAddressChangeListener, Ready
                     || !oldSettings.setupId.equals(settings.setupId)
                     || (oldSettings.networkInterface != null
                             && !oldSettings.networkInterface.equals(settings.networkInterface))
-                    || oldSettings.port != settings.port || oldSettings.useOHmDNS != settings.useOHmDNS
-                    || oldSettings.instances != settings.instances) {
+                    || oldSettings.port != settings.port || oldSettings.useOHmDNS != settings.useOHmDNS) {
                 // the HomeKit server settings changed. we do a complete re-init
+                networkInterface = null;
+
+                // Clear out pairing info for instances that have been removed
+                for (int i = oldSettings.instances - 1; i >= settings.instances; --i) {
+                    clearStorage(i);
+                }
                 stopHomekitServer();
                 if (currentStartLevel >= StartLevelService.STARTLEVEL_STATES) {
                     startHomekitServer();
                 }
             } else {
+                // Stop removed instances
+                for (int i = oldSettings.instances - 1; i >= settings.instances; --i) {
+                    clearStorage(i);
+                    stopHomekitServer(i);
+                }
+                // Start up new instances
+                for (int i = oldSettings.instances; i < settings.instances; ++i) {
+                    startHomekitServer(i);
+                }
+                // Notify remaining instances of the change
                 for (HomekitChangeListener changeListener : changeListeners) {
                     changeListener.updateSettings(settings);
                 }
@@ -212,60 +228,74 @@ public class HomekitImpl implements Homekit, NetworkAddressChangeListener, Ready
         return bridge;
     }
 
-    private void startHomekitServer() throws IOException, InvalidAlgorithmParameterException {
-        logger.trace("start HomeKit bridge");
-        if (homekitServers.isEmpty()) {
-            try {
-                networkInterface = InetAddress
-                        .getByName(((settings.networkInterface != null) && (!settings.networkInterface.isEmpty()))
-                                ? settings.networkInterface
-                                : networkAddressService.getPrimaryIpv4HostAddress());
-            } catch (UnknownHostException e) {
-                logger.warn("cannot resolve the Pv4 address / hostname {}.",
-                        networkAddressService.getPrimaryIpv4HostAddress());
+    private void startHomekitServer(int instance) throws IOException, InvalidAlgorithmParameterException {
+        logger.trace("starting HomeKit bridge instance {}", instance + 1);
+
+        InetAddress localNetworkInterface = ensureNetworkInterface();
+
+        String storageKey = HomekitAuthInfoImpl.STORAGE_KEY;
+        if (instance != 0) {
+            storageKey += instance;
+        }
+        Storage<Object> storage = storageService.getStorage(storageKey);
+        HomekitAuthInfoImpl authInfo = new HomekitAuthInfoImpl(storage, settings.pin, settings.setupId,
+                settings.blockUserDeletion);
+
+        @Nullable
+        HomekitServer homekitServer = null;
+        if (settings.useOHmDNS) {
+            for (JmDNS mdns : mdnsClient.getClientInstances()) {
+                if (mdns.getInetAddress().equals(localNetworkInterface)) {
+                    logger.trace("suitable mDNS client for IP {} found and will be used for HomeKit",
+                            localNetworkInterface);
+                    homekitServer = new HomekitServer(mdns, settings.port + instance);
+                }
             }
+        }
+        if (homekitServer == null) {
+            if (settings.useOHmDNS) {
+                logger.trace("no suitable mDNS server for IP {} found", localNetworkInterface);
+            }
+            logger.trace("create HomeKit server with dedicated mDNS server");
+            homekitServer = new HomekitServer(localNetworkInterface, settings.port + instance);
+        }
+        homekitServers.add(homekitServer);
+        HomekitChangeListener changeListener = new HomekitChangeListener(itemRegistry, settings, metadataRegistry,
+                storage, instance + 1);
+        changeListeners.add(changeListener);
+        startBridge(homekitServer, authInfo, changeListener, instance + 1);
+        authInfos.add(authInfo);
+    }
 
+    private void startHomekitServer() throws IOException, InvalidAlgorithmParameterException {
+        if (homekitServers.isEmpty()) {
             for (int i = 0; i < settings.instances; ++i) {
-                String storage_key = HomekitAuthInfoImpl.STORAGE_KEY;
-                if (i != 0) {
-                    storage_key += i;
-                }
-                Storage<Object> storage = storageService.getStorage(storage_key);
-                HomekitAuthInfoImpl authInfo = new HomekitAuthInfoImpl(storage, settings.pin, settings.setupId,
-                        settings.blockUserDeletion);
-
-                @Nullable
-                HomekitServer homekitServer = null;
-                if (settings.useOHmDNS) {
-                    for (JmDNS mdns : mdnsClient.getClientInstances()) {
-                        if (mdns.getInetAddress().equals(networkInterface)) {
-                            logger.trace("suitable mDNS client for IP {} found and will be used for HomeKit",
-                                    networkInterface);
-                            homekitServer = new HomekitServer(mdns, settings.port + i);
-                        }
-                    }
-                }
-                if (homekitServer == null) {
-                    if (settings.useOHmDNS) {
-                        logger.trace("no suitable mDNS server for IP {} found", networkInterface);
-                    }
-                    logger.trace("create HomeKit server with dedicated mDNS server");
-                    homekitServer = new HomekitServer(networkInterface, settings.port + i);
-                }
-                homekitServers.add(homekitServer);
-                HomekitChangeListener changeListener = new HomekitChangeListener(itemRegistry, settings,
-                        metadataRegistry, storage, i + 1);
-                changeListeners.add(changeListener);
-                bridges.add(startBridge(homekitServer, authInfo, changeListener, i + 1));
-                authInfos.add(authInfo);
+                startHomekitServer(i);
             }
         } else {
             logger.warn("trying to start HomeKit server but it is already initialized");
         }
     }
 
+    private InetAddress ensureNetworkInterface() throws IOException {
+        InetAddress localNetworkInterface = networkInterface;
+        if (localNetworkInterface != null) {
+            return localNetworkInterface;
+        }
+
+        String interfaceName = ((settings.networkInterface != null) && (!settings.networkInterface.isEmpty()))
+                ? settings.networkInterface
+                : networkAddressService.getPrimaryIpv4HostAddress();
+        try {
+            return (networkInterface = Objects.requireNonNull(InetAddress.getByName(interfaceName)));
+        } catch (UnknownHostException e) {
+            logger.warn("cannot resolve the IPv4 address / hostname {}.", interfaceName);
+            throw e;
+        }
+    }
+
     private void stopHomekitServer() {
-        logger.trace("stop HomeKit bridge");
+        logger.trace("stopping HomeKit bridge");
         changeListeners.parallelStream().forEach(HomekitChangeListener::stop);
         bridges.parallelStream().forEach(HomekitRoot::stop);
         homekitServers.parallelStream().forEach(HomekitServer::stop);
@@ -275,6 +305,26 @@ public class HomekitImpl implements Homekit, NetworkAddressChangeListener, Ready
         authInfos.clear();
     }
 
+    private void stopHomekitServer(int instance) {
+        logger.trace("stopping HomeKit bridge instance {}", instance + 1);
+        changeListeners.get(instance).stop();
+        bridges.get(instance).stop();
+        homekitServers.get(instance).stop();
+        changeListeners.remove(instance);
+        bridges.remove(instance);
+        homekitServers.remove(instance);
+        authInfos.remove(instance);
+    }
+
+    private void clearStorage(int index) {
+        String storageKey = HomekitAuthInfoImpl.STORAGE_KEY;
+        if (index != 0) {
+            storageKey += index;
+        }
+        Storage<Object> storage = storageService.getStorage(storageKey);
+        storage.getKeys().forEach(k -> storage.remove(k));
+    }
+
     @Deactivate
     protected void deactivate() {
         networkAddressService.removeNetworkAddressChangeListener(this);
@@ -296,7 +346,7 @@ public class HomekitImpl implements Homekit, NetworkAddressChangeListener, Ready
     }
 
     @Override
-    public List<HomekitAccessory> getAccessories() {
+    public Collection<HomekitAccessory> getAccessories() {
         List<HomekitAccessory> accessories = new ArrayList<>();
         for (HomekitChangeListener changeListener : changeListeners) {
             accessories.addAll(changeListener.getAccessories().values());
@@ -304,13 +354,33 @@ public class HomekitImpl implements Homekit, NetworkAddressChangeListener, Ready
         return accessories;
     }
 
+    @Override
+    public Collection<HomekitAccessory> getAccessories(int instance) {
+        if (instance < 1 || instance > changeListeners.size()) {
+            logger.warn("Instance {} is out of range 1..{}.", instance, changeListeners.size());
+            return List.of();
+        }
+
+        return changeListeners.get(instance - 1).getAccessories().values();
+    }
+
     @Override
     public void clearHomekitPairings() {
+        for (int i = 1; i <= authInfos.size(); ++i) {
+            clearHomekitPairings(i);
+        }
+    }
+
+    @Override
+    public void clearHomekitPairings(int instance) {
+        if (instance < 1 || instance > authInfos.size()) {
+            logger.warn("Instance {} is out of range 1..{}.", instance, authInfos.size());
+            return;
+        }
+
         try {
-            for (HomekitAuthInfoImpl authInfo : authInfos) {
-                authInfo.clear();
-            }
-            refreshAuthInfo();
+            authInfos.get(instance - 1).clear();
+            bridges.get(instance - 1).refreshAuthInfo();
         } catch (Exception e) {
             logger.warn("could not clear HomeKit pairings", e);
         }
@@ -323,6 +393,21 @@ public class HomekitImpl implements Homekit, NetworkAddressChangeListener, Ready
         }
     }
 
+    @Override
+    public void pruneDummyAccessories(int instance) {
+        if (instance < 1 || instance > authInfos.size()) {
+            logger.warn("Instance {} is out of range 1..{}.", instance, authInfos.size());
+            return;
+        }
+
+        changeListeners.get(instance - 1).pruneDummyAccessories();
+    }
+
+    @Override
+    public int getInstanceCount() {
+        return homekitServers.size();
+    }
+
     @Override
     public synchronized void onChanged(final List<CidrAddress> added, final List<CidrAddress> removed) {
         logger.trace("HomeKit bridge reacting on network interface changes.");