Skip to content

Commit

Permalink
CA-388587: Fix 3: Log any errors in filter_xapi_clusterd_db()
Browse files Browse the repository at this point in the history
Signed-off-by: Bernhard Kaindl <[email protected]>
(cherry picked from commit 5fd22f8)
  • Loading branch information
bernhardkaindl committed Feb 14, 2024
1 parent 77776c7 commit 946a873
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 9 deletions.
42 changes: 35 additions & 7 deletions tests/unit/test_filter_xapi_clusterd_db.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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]
Expand All @@ -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.
Expand Down Expand Up @@ -73,13 +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 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
Expand Down Expand Up @@ -305,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)
14 changes: 12 additions & 2 deletions xen-bugtool
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -1389,6 +1396,9 @@ 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)
Expand All @@ -1405,8 +1415,8 @@ def filter_xapi_clusterd_db(cap):
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

Expand Down

0 comments on commit 946a873

Please sign in to comment.