Make the schema cache TTL user-configurable

The API schema is not checked for changes until after a TTL
is expired. A one-hour TTL was hardcoded which makes development
tedious because the only way to force a schema update is to
remember to remove files between invocations.

This adds a new environment variable, schema_ttl, to configure
the TTL returned by the server to schema() calls. This can be
set low to ensure a frequent refresh during development.

If the client is in compat mode, that is if client is working
against a server that doesn't support the schema() command,
then use the client's schema_ttl instead so that the user still
has control.

Re-check validity before writing the cache. This saves us both
a disk write and the possibility of updating the expiration
with a ttl of 0. This can happen if the fingerprint is still
valid (not expired, no language change) the schema check is
skipped so we have no server-provided ttl.

https://pagure.io/freeipa/issue/8492

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Stanislav Levin <slev@altlinux.org>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

client/man/default.conf.5

@@ -172,6 +172,9 @@ Specifies the Kerberos realm.
 .B replication_wait_timeout <seconds>
 The time to wait for a new entry to be replicated during replica installation. The default value is 300 seconds.
 .TP
+.B schema_ttl <seconds>
+The number of seconds for the ipa tool to cache the IPA API and help schema. Reducing this value during development is helpful so that API changes are seen sooner in the tool. Setting this on a server will define the TTL for all client versions > 4.3.1. Client versions > 4.3.1 that connect to IPA servers older than 4.3.1 will use the client-side configuration value. The default is 3600 seconds. 0 disables the cache. A change in the ttl will not be immediately recognized by clients. They will use the new value once their current cache expires.
+.TP
 .B server <hostname>
 Specifies the IPA Server hostname.
 .TP

ipaclient/remote_plugins/__init__.py

@@ -34,6 +34,7 @@ class ServerInfo(MutableMapping):
         hostname = DNSName(api.env.server).ToASCII()
         self._path = os.path.join(self._DIR, hostname)
         self._force_check = api.env.force_schema_check
+        self._now = time.time()
         self._dict = {}
 
         # copy-paste from ipalib/rpc.py
@@ -87,10 +88,9 @@ class ServerInfo(MutableMapping):
     def __len__(self):
         return len(self._dict)
 
-    def update_validity(self, ttl=None):
-        if ttl is None:
-            ttl = 3600
-        self['expiration'] = time.time() + ttl
+    def update_validity(self, ttl):
+        if not self.is_valid():
+            self['expiration'] = self._now + ttl
             self['language'] = self._language
             self._write()
 
@@ -105,8 +105,7 @@ class ServerInfo(MutableMapping):
             # if any of these is missing consider the entry expired
             return False
 
-        if expiration < time.time():
-            # validity passed
+        if expiration < self._now:
             return False
 
         if language != self._language:

ipaclient/remote_plugins/compat.py

@@ -58,7 +58,10 @@ def get_package(server_info, client):
             else:
                 server_version = '2.0'
         server_info['version'] = server_version
-        server_info.update_validity()
+
+        # in compat mode we don't get the schema TTL from the server
+        # so use the client context value.
+        server_info.update_validity(client.api.env.schema_ttl)
 
     server_version = APIVersion(server_version)
 

ipaclient/remote_plugins/schema.py

@@ -375,7 +375,7 @@ class Schema:
     namespaces = {'classes', 'commands', 'topics'}
     _DIR = os.path.join(USER_CACHE_PATH, 'ipa', 'schema', FORMAT)
 
-    def __init__(self, client, fingerprint=None):
+    def __init__(self, client, fingerprint=None, ttl=0):
         self._dict = {}
         self._namespaces = {}
         self._help = None
@@ -384,7 +384,6 @@ class Schema:
             self._dict[ns] = {}
             self._namespaces[ns] = _SchemaNameSpace(self, ns)
 
-        ttl = None
         read_failed = False
 
         if fingerprint is not None:
@@ -554,10 +553,10 @@ def get_package(server_info, client):
                 else:
                     schema = Schema(client, fingerprint)
             except SchemaUpToDate as e:
-                schema = Schema(client, e.fingerprint)
+                schema = Schema(client, e.fingerprint, e.ttl)
         except NotAvailable:
             fingerprint = None
-            ttl = None
+            ttl = 3600  # set a ttl so we don't hammer the remote server
         except SchemaUpToDate as e:
             fingerprint = e.fingerprint
             ttl = e.ttl

ipalib/constants.py

@@ -183,6 +183,9 @@ DEFAULT_CONFIG = (
     # How long to wait for a certmonger request to finish
     ('certmonger_wait_timeout', 300),
 
+    # Number of seconds before client should check for schema update.
+    ('schema_ttl', 3600),
+
     # Web Application mount points
     ('mount_ipa', '/ipa/'),
 

ipaserver/plugins/schema.py

@@ -20,12 +20,6 @@ from ipalib.request import context
 from ipalib.text import _
 from ipapython.version import API_VERSION
 
-# Schema TTL sent to clients in response to schema call.
-# Number of seconds before client should check for schema update.
-# This should be long enough to not slow down regular work or skripts
-# but also short enough to ensure schema will be retvieved soon after
-# it was updated
-SCHEMA_TTL = 3600  # default: 1 hour
 
 __doc__ = _("""
 API Schema
@@ -855,7 +849,7 @@ class schema(Command):
             schema = self._generate_schema(**kwargs)
             self.api._schema[langs] = schema
 
-        schema['ttl'] = SCHEMA_TTL
+        schema['ttl'] = self.api.env.schema_ttl
 
         if schema['fingerprint'] in kwargs.get('known_fingerprints', []):
             raise errors.SchemaUpToDate(

ipatests/test_cmdline/test_schema.py

@@ -0,0 +1,80 @@
+#
+# Copyright (C) 2021  FreeIPA Contributors see COPYING for license
+#
+import pytest
+import time
+
+from ipaclient.remote_plugins import ServerInfo
+
+
+class TestServerInfo(ServerInfo):
+    """Simplified ServerInfo class with hardcoded values"""
+    def __init__(self, fingerprint='deadbeef', hostname='ipa.example.test',
+                 force_check=False, language='en_US',
+                 version='2.0', expiration=None):
+        self._force_check = force_check
+        self._language = language
+        self._now = time.time()
+        self._dict = {
+            'fingerprint': fingerprint,
+            'expiration': expiration or time.time() + 3600,
+            'language': language,
+            'version': version,
+        }
+
+    def _read(self):
+        """Running on test controller, this is a no-op"""
+
+    def _write(self):
+        """Running on test controller, this is a no-op"""
+
+
+@pytest.mark.tier0
+class TestIPAServerInfo:
+    """Test that ServerInfo detects changes in remote configuration"""
+
+    def test_valid(self):
+        server_info = TestServerInfo()
+        assert server_info.is_valid() is True
+
+    def test_force_check(self):
+        server_info = TestServerInfo(force_check=True)
+        assert server_info.is_valid() is False
+
+    def test_language_change(self):
+        server_info = TestServerInfo()
+        assert server_info.is_valid() is True
+        server_info._language = 'fr_FR'
+        assert server_info.is_valid() is False
+        server_info._language = 'en_US'
+
+    def test_expired(self):
+        server_info = TestServerInfo(expiration=time.time() + 2)
+        assert server_info.is_valid() is True
+
+        # skip past the expiration time
+        server_info._now = time.time() + 5
+        assert server_info.is_valid() is False
+
+        # set a new expiration time in the future
+        server_info.update_validity(10)
+        assert server_info.is_valid() is True
+
+        # move to the future beyond expiration
+        server_info._now = time.time() + 15
+        assert server_info.is_valid() is False
+
+    def test_update_validity(self):
+        server_info = TestServerInfo(expiration=time.time() + 1)
+
+        # Expiration and time are one second off so the cache is ok
+        assert server_info.is_valid() is True
+
+        # Simulate time passing by
+        server_info._now = time.time() + 2
+
+        # the validity should be updated because it is now expired
+        server_info.update_validity(3600)
+
+        # the cache is now valid for another hour
+        assert server_info.is_valid() is True