CA-358870: Backport the fixes from #67 to filter the XAPI ClusterD DB for XS8, Yangtze

tests/unit/test_filter_xapi_clusterd_db.py

@@ -1,11 +1,23 @@
 """tests/unit/test_filter_xapi_clusterd_db.py: Test filter_xapi_clusterd_db()
 
-TODO:
+Steps taken for TDD - Test-Driven Develpment - for fixing CA-388587:
 
-This test must be made to fail to cover two current bugtool bugs
+To write reliable unit tests, always start writing a failing test. And make
+sure it fails for the right reasons.
+
+Follow the Red, Green, Refactor principle of Test-Driven Development (TDD).
+
+- Write a failing test (Red)
+- make it pass, and    (Green)
+- refactor the code.   (Refactor)
+
+This plan was used for TDD of these test cases:
+-----------------------------------------------
+
+These tests were made to initially fail to cover two current bugtool bugs
 because the filter_xapi_clusterd_db() function is not implemented correctly.
 
-Current Bugs in filter_xapi_clusterd_db():
+Now fixed bugs in filter_xapi_clusterd_db():
 
 1.  The filter must replace "token": "secret-token", with "token": "REMOVED"
 2.  The filter must also work if the "pems" key is missing
@@ -21,7 +33,7 @@
 - The pytest.mark.xfail decorator must be removed after the bug is fixed
   to ensure the test is run and passes in the future.
 
-TODO START:
+DONE START:
 
 1.  Add additional XFAIL tests, one for each, which fail on the cases above
 [DONE]
@@ -37,10 +49,13 @@
 
 4.  The changes are added as additional commits and pushed.
     The individual sub-tests must be shown failing on each the cases above.
+[DONE] (done for each case)
 
 5.  The filter must be updated to handle these cases.
+[DONE] (done for each case)
 
 6.  The PR is then pushed again and then pass.
+[DONE]
 
     Changes to the test should only be needed if e.g. logging was added
     to test the logging output.
@@ -73,15 +88,13 @@
 
 If you run `pre-commit install`, it will run on every commit in your own clone.
 
-TODO END.
+DONE END.
 
 """
 
 import json
 import os
-
-import pytest
-
+import re
 
 # Minimal example of a xapi-clusterd/db file, with sensitive data
 # BUG: The new cluster_config has no "pems" key, so the filter has to be updated
@@ -107,7 +120,7 @@
 # Same as original, but with passwords and private data replaced by: "REMOVED"
 # BUG: "secret-token" has to be replaced by "REMOVED"
 EXPECTED = r"""{
-    "token": "secret-token",
+    "token": "REMOVED",
     "cluster_config": {
         "pems": {
             "blobs": "REMOVED"
@@ -189,7 +202,6 @@ def test_pems_blobs(isolated_bugtool):
 
 
 # CA-358870: filter_xapi_clusterd_db: remove token from the report
-@pytest.mark.xfail(reason="bugtool currently fails to remove the token")
 def test_remove_token(isolated_bugtool):
     """CA-358870: Assert that filter_xapi_clusterd_db() removes the token"""
 
@@ -201,7 +213,6 @@ def test_remove_token(isolated_bugtool):
     assert_filter_xapi_clusterd_db(isolated_bugtool, ORIGINAL, expected_json)
 
 
-@pytest.mark.xfail(reason="bugtool currently fails to handle missing authkey")
 def test_no_authkey(isolated_bugtool):
     """Assert that filter_xapi_clusterd_db() handles missing authkey"""
 
@@ -221,7 +232,6 @@ def remove_authkey(json_data):
     )
 
 
-@pytest.mark.xfail(reason="bugtool currently fails to handle missing pems")
 def test_no_pems(isolated_bugtool):
     """Assert that filter_xapi_clusterd_db() handles missing pems"""
 
@@ -241,7 +251,6 @@ def remove_pems(json_data):
     )
 
 
-@pytest.mark.xfail(reason="bugtool currently fails to handle missing pems.blobs")
 def test_no_blobs(isolated_bugtool):
     """Assert that filter_xapi_clusterd_db() handles missing blobs"""
 
@@ -311,4 +320,17 @@ def test_invalid_db(isolated_bugtool, capsys):
 
     with capsys.disabled():
         stdout = capsys.readouterr().out
-        assert stdout == ""
+        assert stdout.startswith("bugtool: Internal error: ")
+        assert "filter_xapi_clusterd_db" in stdout
+        assert "JSON" in stdout
+
+    with open(isolated_bugtool.XEN_BUGTOOL_LOG, "r") as f:
+        log = f.read()
+        assert "filter_xapi_clusterd_db" in log
+        assert "JSON" in log
+
+    # Python2 error message
+    log = re.sub(r"(?s)bugtool: Internal error:.*could be decoded\n\n", "", log)
+
+    with open(isolated_bugtool.XEN_BUGTOOL_LOG, "w") as f:
+        f.write(log)

xen-bugtool

@@ -470,6 +470,13 @@ def no_unicode(x):
         return x.encode('utf-8')
     return x
 
+
+def log_internal_error(e):
+    """Log an internal error and return an empty string as bugtool output data"""
+    log("bugtool: Internal error: " + traceback.format_exc())
+    return ""  # For now, no diagnostic output in the individual output files
+
+
 def log(x, print_output=True):
     if print_output:
         output(x)
@@ -1389,19 +1396,27 @@ def filter_xenstore_secrets(s, state):
 def filter_xapi_clusterd_db(cap):
     clusterd_data = {}
 
+    if not os.path.exists(XAPI_CLUSTERD):
+        return ""
+
     try:
         with open(XAPI_CLUSTERD, 'r') as f:
             clusterd_data = json.load(f)
 
         config_keys = ['cluster_config', 'old_cluster_config']
         for config_key in config_keys:
             if config_key in clusterd_data.keys():
-                clusterd_data[config_key]['pems']['blobs'] = "REMOVED"
-                clusterd_data[config_key]['authkey'] = "REMOVED"
-
+                conf = clusterd_data[config_key]
+                if "pems" in conf and "blobs" in conf["pems"]:
+                    conf["pems"]["blobs"] = "REMOVED"
+                if "authkey" in conf:
+                    conf["authkey"] = "REMOVED"
+
+        if "token" in clusterd_data:
+            clusterd_data["token"] = "REMOVED"
         output_str = json.dumps(clusterd_data)
-    except Exception:
-        output_str = ''
+    except Exception as e:
+        return log_internal_error(e)
 
     return output_str