Merge pull request #2561 from jestabro/sagitta-http-api

http-api: T5782: simplifications for config mode http-api

8 files changed, 140 insertions, 196 deletions

diff --git a/data/config-mode-dependencies/vyos-1x.json b/data/config-mode-dependencies/vyos-1x.json
index 37333a19c..b9dd22f5a 100644
--- a/data/config-mode-dependencies/vyos-1x.json
+++ b/data/config-mode-dependencies/vyos-1x.json
@@ -1,6 +1,5 @@
 {
   "firewall": {"group_resync": ["nat", "policy-route"]},
-  "http_api": {"https": ["https"]},
   "interfaces_bonding": {
            "ethernet": ["interfaces-ethernet"]
          },
diff --git a/interface-definitions/https.xml.in b/interface-definitions/https.xml.in
index 05c552e6b..ca5a5f088 100644
--- a/interface-definitions/https.xml.in
+++ b/interface-definitions/https.xml.in
@@ -54,10 +54,9 @@
               #include <include/allow-client.xml.i>
             </children>
           </tagNode>
-          <node name="api" owner="${vyos_conf_scripts_dir}/http-api.py">
+          <node name="api">
             <properties>
               <help>VyOS HTTP API configuration</help>
-              <priority>1002</priority>
             </properties>
             <children>
               <node name="keys">
diff --git a/python/vyos/configdiff.py b/python/vyos/configdiff.py
index 1ec2dfafe..03b06c6d9 100644
--- a/python/vyos/configdiff.py
+++ b/python/vyos/configdiff.py
@@ -165,6 +165,30 @@ class ConfigDiff(object):
             return True
         return False
 
+    def node_changed_presence(self, path=[]) -> bool:
+        if self._diff_tree is None:
+            raise NotImplementedError("diff_tree class not available")
+
+        path = self._make_path(path)
+        before = self._diff_tree.left.exists(path)
+        after = self._diff_tree.right.exists(path)
+        return (before and not after) or (not before and after)
+
+    def node_changed_children(self, path=[]) -> list:
+        if self._diff_tree is None:
+            raise NotImplementedError("diff_tree class not available")
+
+        path = self._make_path(path)
+        add = self._diff_tree.add
+        sub = self._diff_tree.sub
+        children = set()
+        if add.exists(path):
+            children.update(add.list_nodes(path))
+        if sub.exists(path):
+            children.update(sub.list_nodes(path))
+
+        return list(children)
+
     def get_child_nodes_diff_str(self, path=[]):
         ret = {'add': {}, 'change': {}, 'delete': {}}
 
diff --git a/python/vyos/defaults.py b/python/vyos/defaults.py
index b7f39ecb0..2f3580571 100644
--- a/python/vyos/defaults.py
+++ b/python/vyos/defaults.py
@@ -50,12 +50,6 @@ https_data = {
     'listen_addresses' : { '*': ['_'] }
 }
 
-api_data = {
-    'strict' : False,
-    'debug' : False,
-    'api_keys' : [ {'id' : 'testapp', 'key' : 'qwerty'} ]
-}
-
 vyos_cert_data = {
     'conf' : '/etc/nginx/snippets/vyos-cert.conf',
     'crt' : '/etc/ssl/certs/vyos-selfsigned.crt',
diff --git a/smoketest/scripts/cli/test_service_https.py b/smoketest/scripts/cli/test_service_https.py
index 24e1f1299..6cb91bcf1 100755
--- a/smoketest/scripts/cli/test_service_https.py
+++ b/smoketest/scripts/cli/test_service_https.py
@@ -254,6 +254,35 @@ class TestHTTPSService(VyOSUnitTestSHIM.TestCase):
         self.assertTrue(success)
 
     @ignore_warning(InsecureRequestWarning)
+    def test_api_add_delete(self):
+        address = '127.0.0.1'
+        key = 'VyOS-key'
+        url = f'https://{address}/retrieve'
+        payload = {'data': '{"op": "showConfig", "path": []}', 'key': f'{key}'}
+        headers = {}
+
+        self.cli_set(base_path)
+        self.cli_commit()
+
+        r = request('POST', url, verify=False, headers=headers, data=payload)
+        # api not configured; expect 503
+        self.assertEqual(r.status_code, 503)
+
+        self.cli_set(base_path + ['api', 'keys', 'id', 'key-01', 'key', key])
+        self.cli_commit()
+
+        r = request('POST', url, verify=False, headers=headers, data=payload)
+        # api configured; expect 200
+        self.assertEqual(r.status_code, 200)
+
+        self.cli_delete(base_path + ['api'])
+        self.cli_commit()
+
+        r = request('POST', url, verify=False, headers=headers, data=payload)
+        # api deleted; expect 503
+        self.assertEqual(r.status_code, 503)
+
+    @ignore_warning(InsecureRequestWarning)
     def test_api_show(self):
         address = '127.0.0.1'
         key = 'VyOS-key'
diff --git a/src/conf_mode/http-api.py b/src/conf_mode/http-api.py
deleted file mode 100755
index d8fe3b736..000000000
--- a/src/conf_mode/http-api.py
+++ /dev/null
@@ -1,154 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (C) 2019-2021 VyOS maintainers and contributors
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License version 2 or later as
-# published by the Free Software Foundation.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-import sys
-import os
-import json
-
-from time import sleep
-from copy import deepcopy
-
-import vyos.defaults
-
-from vyos.config import Config
-from vyos.configdep import set_dependents, call_dependents
-from vyos.template import render
-from vyos.utils.process import call
-from vyos.utils.process import is_systemd_service_running
-from vyos import ConfigError
-from vyos import airbag
-airbag.enable()
-
-api_conf_file = '/etc/vyos/http-api.conf'
-systemd_service = '/run/systemd/system/vyos-http-api.service'
-
-vyos_conf_scripts_dir=vyos.defaults.directories['conf_mode']
-
-def _translate_values_to_boolean(d: dict) -> dict:
-    for k in list(d):
-        if d[k] == {}:
-            d[k] = True
-        elif isinstance(d[k], dict):
-            _translate_values_to_boolean(d[k])
-        else:
-            pass
-
-def get_config(config=None):
-    http_api = deepcopy(vyos.defaults.api_data)
-    x = http_api.get('api_keys')
-    if x is None:
-        default_key = None
-    else:
-        default_key = x[0]
-    keys_added = False
-
-    if config:
-        conf = config
-    else:
-        conf = Config()
-
-    # reset on creation/deletion of 'api' node
-    https_base = ['service', 'https']
-    if conf.exists(https_base):
-        set_dependents("https", conf)
-
-    base = ['service', 'https', 'api']
-    if not conf.exists(base):
-        return None
-
-    api_dict = conf.get_config_dict(base, key_mangling=('-', '_'),
-                                    no_tag_node_value_mangle=True,
-                                    get_first_key=True,
-                                    with_recursive_defaults=True)
-
-    # One needs to 'flatten' the keys dict from the config into the
-    # http-api.conf format for api_keys:
-    if 'keys' in api_dict:
-        api_dict['api_keys'] = []
-        for el in list(api_dict['keys'].get('id', {})):
-            key = api_dict['keys']['id'][el].get('key', '')
-            if key:
-                api_dict['api_keys'].append({'id': el, 'key': key})
-        del api_dict['keys']
-
-    # Do we run inside a VRF context?
-    vrf_path = ['service', 'https', 'vrf']
-    if conf.exists(vrf_path):
-        http_api['vrf'] = conf.return_value(vrf_path)
-
-    if 'api_keys' in api_dict:
-        keys_added = True
-
-    if api_dict.from_defaults(['graphql']):
-        del api_dict['graphql']
-
-    http_api.update(api_dict)
-
-    if keys_added and default_key:
-        if default_key in http_api['api_keys']:
-            http_api['api_keys'].remove(default_key)
-
-    # Finally, translate entries in http_api into boolean settings for
-    # backwards compatability of JSON http-api.conf file
-    _translate_values_to_boolean(http_api)
-
-    return http_api
-
-def verify(http_api):
-    return None
-
-def generate(http_api):
-    if http_api is None:
-        if os.path.exists(systemd_service):
-            os.unlink(systemd_service)
-        return None
-
-    if not os.path.exists('/etc/vyos'):
-        os.mkdir('/etc/vyos')
-
-    with open(api_conf_file, 'w') as f:
-        json.dump(http_api, f, indent=2)
-
-    render(systemd_service, 'https/vyos-http-api.service.j2', http_api)
-    return None
-
-def apply(http_api):
-    # Reload systemd manager configuration
-    call('systemctl daemon-reload')
-    service_name = 'vyos-http-api.service'
-
-    if http_api is not None:
-        if is_systemd_service_running(f'{service_name}'):
-            call(f'systemctl reload {service_name}')
-        else:
-            call(f'systemctl restart {service_name}')
-    else:
-        call(f'systemctl stop {service_name}')
-
-    # Let uvicorn settle before restarting Nginx
-    sleep(1)
-
-    call_dependents()
-
-if __name__ == '__main__':
-    try:
-        c = get_config()
-        verify(c)
-        generate(c)
-        apply(c)
-    except ConfigError as e:
-        print(e)
-        sys.exit(1)
diff --git a/src/conf_mode/https.py b/src/conf_mode/https.py
index 5cbdd1651..40b7de557 100755
--- a/src/conf_mode/https.py
+++ b/src/conf_mode/https.py
@@ -16,19 +16,24 @@
 
 import os
 import sys
+import json
 
 from copy import deepcopy
+from time import sleep
 
 import vyos.defaults
 import vyos.certbot_util
 
 from vyos.config import Config
+from vyos.configdiff import get_config_diff
 from vyos.configverify import verify_vrf
 from vyos import ConfigError
 from vyos.pki import wrap_certificate
 from vyos.pki import wrap_private_key
 from vyos.template import render
 from vyos.utils.process import call
+from vyos.utils.process import is_systemd_service_running
+from vyos.utils.process import is_systemd_service_active
 from vyos.utils.network import check_port_availability
 from vyos.utils.network import is_listen_port_bind_service
 from vyos.utils.file import write_file
@@ -42,6 +47,9 @@ cert_dir = '/etc/ssl/certs'
 key_dir = '/etc/ssl/private'
 certbot_dir = vyos.defaults.directories['certbot']
 
+api_config_state = '/run/http-api-state'
+systemd_service = '/run/systemd/system/vyos-http-api.service'
+
 # https config needs to coordinate several subsystems: api, certbot,
 # self-signed certificate, as well as the virtual hosts defined within the
 # https config definition itself. Consequently, one needs a general dict,
@@ -52,7 +60,7 @@ default_server_block = {
     'address'   : '*',
     'port'      : '443',
     'name'      : ['_'],
-    'api'       : {},
+    'api'       : False,
     'vyos_cert' : {},
     'certbot'   : False
 }
@@ -67,11 +75,35 @@ def get_config(config=None):
     if not conf.exists(base):
         return None
 
+    diff = get_config_diff(conf)
+
     https = conf.get_config_dict(base, get_first_key=True)
 
     if https:
         https['pki'] = conf.get_config_dict(['pki'], key_mangling=('-', '_'),
-                                get_first_key=True, no_tag_node_value_mangle=True)
+                                            no_tag_node_value_mangle=True,
+                                            get_first_key=True)
+
+    https['children_changed'] = diff.node_changed_children(base)
+    https['api_add_or_delete'] = diff.node_changed_presence(base + ['api'])
+
+    if 'api' not in https:
+        return https
+
+    http_api = conf.get_config_dict(base + ['api'], key_mangling=('-', '_'),
+                                    no_tag_node_value_mangle=True,
+                                    get_first_key=True,
+                                    with_recursive_defaults=True)
+
+    if http_api.from_defaults(['graphql']):
+        del http_api['graphql']
+
+    # Do we run inside a VRF context?
+    vrf_path = ['service', 'https', 'vrf']
+    if conf.exists(vrf_path):
+        http_api['vrf'] = conf.return_value(vrf_path)
+
+    https['api'] = http_api
 
     return https
 
@@ -103,7 +135,7 @@ def verify(https):
 
         if 'certbot' in https['certificates']:
             vhost_names = []
-            for vh, vh_conf in https.get('virtual-host', {}).items():
+            for _, vh_conf in https.get('virtual-host', {}).items():
                 vhost_names += vh_conf.get('server-name', [])
             domains = https['certificates']['certbot'].get('domain-name', [])
             domains_found = [domain for domain in domains if domain in vhost_names]
@@ -167,6 +199,14 @@ def generate(https):
     if https is None:
         return None
 
+    if 'api' not in https:
+        if os.path.exists(systemd_service):
+            os.unlink(systemd_service)
+    else:
+        render(systemd_service, 'https/vyos-http-api.service.j2', https['api'])
+        with open(api_config_state, 'w') as f:
+            json.dump(https['api'], f, indent=2)
+
     server_block_list = []
 
     # organize by vhosts
@@ -232,35 +272,18 @@ def generate(https):
                     # certbot organizes certificates by first domain
                     sb['certbot_domain_dir'] = cert_domains[0]
 
-    # get api data
-
-    api_set = False
-    api_data = {}
     if 'api' in list(https):
-        api_set = True
-        api_data = vyos.defaults.api_data
-    api_settings = https.get('api', {})
-    if api_settings:
-        vhosts = https.get('api-restrict', {}).get('virtual-host', [])
-        if vhosts:
-            api_data['vhost'] = vhosts[:]
-
-    if api_data:
-        vhost_list = api_data.get('vhost', [])
+        vhost_list = https.get('api-restrict', {}).get('virtual-host', [])
         if not vhost_list:
             for block in server_block_list:
-                block['api'] = api_data
+                block['api'] = True
         else:
             for block in server_block_list:
                 if block['id'] in vhost_list:
-                    block['api'] = api_data
-
-    if 'server_block_list' not in https or not https['server_block_list']:
-        https['server_block_list'] = [default_server_block]
+                    block['api'] = True
 
     data = {
         'server_block_list': server_block_list,
-        'api_set': api_set,
         'certbot': certbot
     }
 
@@ -271,10 +294,31 @@ def generate(https):
 def apply(https):
     # Reload systemd manager configuration
     call('systemctl daemon-reload')
-    if https is not None:
-        call('systemctl restart nginx.service')
-    else:
-        call('systemctl stop nginx.service')
+    http_api_service_name = 'vyos-http-api.service'
+    https_service_name = 'nginx.service'
+
+    if https is None:
+        if is_systemd_service_active(f'{http_api_service_name}'):
+            call(f'systemctl stop {http_api_service_name}')
+        call(f'systemctl stop {https_service_name}')
+        return
+
+    if 'api' in https['children_changed']:
+        if 'api' in https:
+            if is_systemd_service_running(f'{http_api_service_name}'):
+                call(f'systemctl reload {http_api_service_name}')
+            else:
+                call(f'systemctl restart {http_api_service_name}')
+            # Let uvicorn settle before (possibly) restarting nginx
+            sleep(1)
+        else:
+            if is_systemd_service_active(f'{http_api_service_name}'):
+                call(f'systemctl stop {http_api_service_name}')
+
+    if (not is_systemd_service_running(f'{https_service_name}') or
+        https['api_add_or_delete'] or
+        set(https['children_changed']) - set(['api'])):
+        call(f'systemctl restart {https_service_name}')
 
 if __name__ == '__main__':
     try:
diff --git a/src/services/vyos-http-api-server b/src/services/vyos-http-api-server
index 85d7884b6..bfd50cc80 100755
--- a/src/services/vyos-http-api-server
+++ b/src/services/vyos-http-api-server
@@ -50,7 +50,7 @@ from vyos.configsession import ConfigSession, ConfigSessionError
 
 import api.graphql.state
 
-DEFAULT_CONFIG_FILE = '/etc/vyos/http-api.conf'
+api_config_state = '/run/http-api-state'
 CFG_GROUP = 'vyattacfg'
 
 debug = True
@@ -68,7 +68,7 @@ else:
 lock = threading.Lock()
 
 def load_server_config():
-    with open(DEFAULT_CONFIG_FILE) as f:
+    with open(api_config_state) as f:
         config = json.load(f)
     return config
 
@@ -860,19 +860,28 @@ def shutdown_handler(signum, frame):
     logger.info('Server shutdown...')
     shutdown = True
 
+def flatten_keys(d: dict) -> list[dict]:
+    keys_list = []
+    for el in list(d['keys'].get('id', {})):
+        key = d['keys']['id'][el].get('key', '')
+        if key:
+            keys_list.append({'id': el, 'key': key})
+    return keys_list
+
 def initialization(session: ConfigSession, app: FastAPI = app):
     global server
     try:
         server_config = load_server_config()
+        keys = flatten_keys(server_config)
     except Exception as e:
         logger.critical(f'Failed to load the HTTP API server config: {e}')
         sys.exit(1)
 
     app.state.vyos_session = session
-    app.state.vyos_keys = server_config['api_keys']
+    app.state.vyos_keys = keys
 
-    app.state.vyos_debug = server_config['debug']
-    app.state.vyos_strict = server_config['strict']
+    app.state.vyos_debug = bool('debug' in server_config)
+    app.state.vyos_strict = bool('strict' in server_config)
     app.state.vyos_origins = server_config.get('cors', {}).get('allow_origin', [])
     if 'graphql' in server_config:
         app.state.vyos_graphql = True
@@ -881,7 +890,7 @@ def initialization(session: ConfigSession, app: FastAPI = app):
                 app.state.vyos_introspection = True
             else:
                 app.state.vyos_introspection = False
-            # default value is merged in conf_mode http-api.py, if not set
+            # default values if not set explicitly
             app.state.vyos_auth_type = server_config['graphql']['authentication']['type']
             app.state.vyos_token_exp = server_config['graphql']['authentication']['expiration']
             app.state.vyos_secret_len = server_config['graphql']['authentication']['secret_length']