feat: refactor the cli (#16)

* finish up adding topo

* refactor the cli

* formatting

README.md

@@ -5,9 +5,7 @@
 [![Build Status](https://github.com/tcm5343/cycl/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/tcm5343/cycl/actions)
 [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT)
 
-cycl is a CLI and Python SDK to help identify cross-stack import/export circular dependencies, for a given AWS account and region.
-
-The successor to [circular-dependency-detector](https://github.com/tcm5343/circular-dependency-detector), which was built at the University of Texas at Austin.
+cycl is a CLI and Python SDK to help identify cross-stack import/export circular dependencies, for a given AWS account and region. The successor to [circular-dependency-detector](https://github.com/tcm5343/circular-dependency-detector), which was built at the University of Texas at Austin.
 
 ## Getting started
 

TODO.md

@@ -4,8 +4,10 @@
   - `cycl check --ignore`
   - `cycl topo --ignore`
 - reducing the stacks, for example, a tag on a stack representing the github repo name
-  - `cycl check --reduce-dependencies-on`
-  - `cycl check --reduce-generations-on`
+  - `cycl check --reduce-on`
+  - `cycl check --reduce-on-jq`
+  - `cycl check --reduce-on-jmespath`
+  - `cycl topo --reduce-on {rootstack}` # can we resolve the root stack through cdk.out
 
 - [ ] [fully configure dependabot](https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions)
 - [ ] configure make file

src/cycl/cli.py

@@ -41,10 +41,18 @@ def app() -> None:
             type=pathlib.Path,
             help='Path to cdk.out, where stacks are CDK synthesized to CFN templates',
         )
+        p.add_argument(
+            '-in',
+            '--ignore-nodes',
+            nargs='+',
+            default=[],
+            type=str,
+            help='List of nodes to ignore when building the graph (ex. --ignore-nodes s1 s2 ...)',
+        )
         # p.add_argument(
         #     '-q', "--quiet",
         #     type=pathlib.Path,
-        #     default=argparse.SUPPRESS,
+        #     action='store_true',
         #     help="Suppress output",
         # )
 
@@ -55,7 +63,7 @@ def app() -> None:
     args = parser.parse_args()
     configure_log(getattr(logging, args.log_level))
 
-    dep_graph = build_dependency_graph(cdk_out_path=args.cdk_out)
+    dep_graph = build_dependency_graph(cdk_out_path=args.cdk_out, nodes_to_ignore=args.ignore_nodes)
     cycles = list(nx.simple_cycles(dep_graph))
     for cycle in cycles:
         print(f'cycle found between nodes: {cycle}')
@@ -65,7 +73,7 @@ def app() -> None:
             sys.exit(1)
     elif args.cmd == 'topo':
         if cycles:
-            print('\nerror: graph is cyclic, topological generations can only be computed on an acyclic graph')
+            log.error('graph is cyclic, topological generations can only be computed on an acyclic graph')
             sys.exit(1)
         generations = [sorted(generation) for generation in nx.topological_generations(dep_graph)]
         print(json.dumps(generations, indent=2))

src/cycl/cycl.py

@@ -26,6 +26,7 @@ def get_dependency_graph_data(cdk_out_path: Path | None = None) -> dict:
     )
     cfn_client = boto3.client('cloudformation', config=boto_config)
 
+    log.info('getting all exports')
     exports = get_all_exports(cfn_client=cfn_client)
     for export_name in cdk_out_imports:
         if export_name not in exports:
@@ -35,20 +36,25 @@ def get_dependency_graph_data(cdk_out_path: Path | None = None) -> dict:
                 cdk_out_imports[export_name],
             )
 
+    log.info('getting imports for %s exports', len(exports))
     for export in exports.values():
         export['ExportingStackName'] = parse_name_from_id(export['ExportingStackId'])
         export['ImportingStackNames'] = get_all_imports(export_name=export['Name'], cfn_client=cfn_client)
         export.setdefault('ImportingStackNames', []).extend(cdk_out_imports.get(export['Name'], []))
         if len(export['ImportingStackNames']) == 0:
-            log.info('Export found with no import: %s from %s', export['Name'], export['ExportingStackName'])
+            log.warning('Export found with no import: %s from %s', export['Name'], export['ExportingStackName'])
     return exports
 
 
-def build_dependency_graph(cdk_out_path: Path | None = None) -> nx.MultiDiGraph:
+def build_dependency_graph(cdk_out_path: Path | None = None, nodes_to_ignore: list[str] | None = None) -> nx.MultiDiGraph:
+    nodes_to_ignore = nodes_to_ignore or []
     dep_graph = nx.MultiDiGraph()
     graph_data = get_dependency_graph_data(cdk_out_path)
 
+    log.info('building dependency graph from graph data')
     for export in graph_data.values():
+        if export['ExportingStackName'] in nodes_to_ignore:
+            continue
         edges = [
             (export['ExportingStackName'], importing_stack_name) for importing_stack_name in export['ImportingStackNames']
         ]

src/cycl/utils/testing.py

@@ -0,0 +1,7 @@
+def is_circular_reversible_permutation(lst: list, candidate: list) -> bool:
+    """the output cycle is non-deterministic, it could be reversed and any node could be the start"""
+    extended_lst = lst + lst
+    extended_lst_reverse = lst[::-1] + lst[::-1]
+    return any(extended_lst[i : i + len(candidate)] == candidate for i in range(len(lst))) or any(
+        extended_lst_reverse[i : i + len(candidate)] == candidate for i in range(len(lst))
+    )

tests/cli_test.py

@@ -11,7 +11,7 @@
 
 
 @pytest.fixture(autouse=True)
-def mock_build_build_dependency_graph():
+def mock_build_dependency_graph():
     with patch.object(cli_module, 'build_dependency_graph') as mock:
         mock.return_value = nx.MultiDiGraph()
         yield mock
@@ -52,15 +52,15 @@ def test_app_check_acyclic():
     assert err.value.code == 0
 
 
-def test_app_check_cyclic(capsys, mock_build_build_dependency_graph):
+def test_app_check_cyclic(capsys, mock_build_dependency_graph):
     graph = nx.MultiDiGraph()
     graph.add_edges_from(
         [
             (1, 2),
             (2, 1),
         ]
     )
-    mock_build_build_dependency_graph.return_value = graph
+    mock_build_dependency_graph.return_value = graph
     sys.argv = ['cycl', 'check']
 
     with pytest.raises(SystemExit) as err:
@@ -71,15 +71,15 @@ def test_app_check_cyclic(capsys, mock_build_build_dependency_graph):
     assert 'cycle found between nodes: [1, 2]' in console_output
 
 
-def test_app_check_cyclic_exit_zero(capsys, mock_build_build_dependency_graph):
+def test_app_check_cyclic_exit_zero(capsys, mock_build_dependency_graph):
     graph = nx.MultiDiGraph()
     graph.add_edges_from(
         [
             (1, 2),
             (2, 1),
         ]
     )
-    mock_build_build_dependency_graph.return_value = graph
+    mock_build_dependency_graph.return_value = graph
     sys.argv = ['cycl', 'check', '--exit-zero']
 
     with pytest.raises(SystemExit) as err:
@@ -111,34 +111,35 @@ def test_app_acyclic_log_level(mock_configure_log, arg_value, log_level, cmd):
     mock_configure_log.assert_called_with(log_level)
 
 
-def test_app_topo_cyclic(capsys, mock_build_build_dependency_graph):
+def test_app_topo_cyclic(caplog, mock_build_dependency_graph):
+    caplog.set_level(logging.DEBUG)
     graph = nx.MultiDiGraph()
     graph.add_edges_from(
         [
             (1, 2),
             (2, 1),
         ]
     )
-    mock_build_build_dependency_graph.return_value = graph
+    mock_build_dependency_graph.return_value = graph
     sys.argv = ['cycl', 'topo']
 
     with pytest.raises(SystemExit) as err:
         app()
 
     assert err.value.code == 1
-    console_output = capsys.readouterr().out
-    assert 'error: graph is cyclic, topological generations can only be computed on an acyclic graph' in console_output
+    console_output = caplog.text
+    assert 'graph is cyclic, topological generations can only be computed on an acyclic graph' in console_output
 
 
-def test_app_topo_acyclic(capsys, mock_build_build_dependency_graph):
+def test_app_topo_acyclic(capsys, mock_build_dependency_graph):
     graph = nx.MultiDiGraph()
     graph.add_edges_from(
         [
             (2, 1),
             (3, 1),
         ]
     )
-    mock_build_build_dependency_graph.return_value = graph
+    mock_build_dependency_graph.return_value = graph
     expected_output = json.dumps([[2, 3], [1]], indent=2)
     sys.argv = ['cycl', 'topo']
 
@@ -150,9 +151,9 @@ def test_app_topo_acyclic(capsys, mock_build_build_dependency_graph):
     assert expected_output in console_output
 
 
-def test_app_topo_empty(capsys, mock_build_build_dependency_graph):
+def test_app_topo_empty(capsys, mock_build_dependency_graph):
     graph = nx.MultiDiGraph()
-    mock_build_build_dependency_graph.return_value = graph
+    mock_build_dependency_graph.return_value = graph
     expected_output = json.dumps([], indent=2)
     sys.argv = ['cycl', 'topo']
 
@@ -162,3 +163,14 @@ def test_app_topo_empty(capsys, mock_build_build_dependency_graph):
     assert err.value.code == 0
     console_output = capsys.readouterr().out
     assert expected_output in console_output
+
+
+@pytest.mark.parametrize('cmd', ['check', 'topo'])
+def test_app_topo_acyclic_ignore_passes_arg(mock_build_dependency_graph, cmd):
+    sys.argv = ['cycl', cmd, '--ignore-nodes', '3']
+
+    with pytest.raises(SystemExit) as err:
+        app()
+
+    mock_build_dependency_graph.assert_called_once_with(cdk_out_path=None, nodes_to_ignore=['3'])
+    assert err.value.code == 0

tests/cycl_test.py

@@ -6,6 +6,7 @@
 
 import cycl.cycl as cycl_module
 from cycl.cycl import build_dependency_graph
+from cycl.utils.testing import is_circular_reversible_permutation
 
 
 @pytest.fixture(autouse=True)
@@ -314,3 +315,131 @@ def test_config_defined_as_expected(mock_config, mock_boto3):
         },
     )
     mock_boto3.client.assert_called_once_with('cloudformation', config=mock_config.return_value)
+
+
+def test_build_dependency_graph_returns_cyclic_graph(
+    mock_get_all_exports, mock_get_all_imports, subtests, mock_get_cdk_out_imports
+):
+    """
+    Visual representation of expected output graph:
+    some-exporting-stack-id-1-stack-name
+        │
+        ├──► some-importing-stack-name-1
+        │
+        ├──► some-importing-stack-name-2
+    """
+    mock_get_all_exports.return_value = {
+        'some-name-1': {
+            'ExportingStackId': 'some-exporting-stack-id-1',
+            'Name': 'some-name-1',
+            'Value': 'some-value-1',
+        },
+        'some-name-2': {
+            'ExportingStackId': 'some-exporting-stack-id-2',
+            'Name': 'some-name-2',
+            'Value': 'some-value-2',
+        },
+    }
+
+    def mock_get_all_imports_side_effect_func(export_name, *_args, **_kwargs):
+        if export_name == 'some-name-1':
+            return [
+                'some-exporting-stack-id-2-stack-name',
+            ]
+        if export_name == 'some-name-2':
+            return [
+                'some-exporting-stack-id-1-stack-name',
+            ]
+        return []  # handles export with no import
+
+    mock_get_all_imports.side_effect = mock_get_all_imports_side_effect_func
+    expected_edges = [
+        ('some-exporting-stack-id-1-stack-name', 'some-exporting-stack-id-2-stack-name'),
+        ('some-exporting-stack-id-2-stack-name', 'some-exporting-stack-id-1-stack-name'),
+    ]
+    expected_nodes = [
+        'some-exporting-stack-id-1-stack-name',
+        'some-exporting-stack-id-2-stack-name',
+    ]
+    expected_cycles = [['some-exporting-stack-id-2-stack-name', 'some-exporting-stack-id-1-stack-name']]
+
+    actual_graph = build_dependency_graph()
+
+    mock_get_cdk_out_imports.assert_not_called()
+    for expected_node in expected_nodes:
+        with subtests.test(msg='assert graph has node', expected_node=expected_node):
+            assert actual_graph.has_node(expected_node)
+    assert nx.number_of_nodes(actual_graph) == len(expected_nodes)
+
+    for expected_edge in expected_edges:
+        with subtests.test(msg='assert graph has edge', expected_edge=expected_edge):
+            assert actual_graph.has_edge(*expected_edge)
+    assert nx.number_of_edges(actual_graph) == len(expected_edges)
+
+    assert not nx.is_directed_acyclic_graph(actual_graph)
+    actual_cycles = list(nx.simple_cycles(actual_graph))
+
+    for expected_cycle in expected_cycles:
+        with subtests.test(msg='assert cycle is expected', expected_cycles=expected_cycles):
+            assert any(is_circular_reversible_permutation(actual_cycle, expected_cycle) for actual_cycle in actual_cycles)
+
+    assert len(actual_cycles) == len(expected_cycles)
+
+
+def test_build_dependency_graph_make_cyclic_graph_acyclic_with_ignore_nodes(
+    mock_get_all_exports, mock_get_all_imports, subtests, mock_get_cdk_out_imports
+):
+    """
+    Visual representation of expected output graph:
+    some-exporting-stack-id-1-stack-name
+        │
+        ├──► some-importing-stack-name-1
+        │
+        ├──► some-importing-stack-name-2
+    """
+    mock_get_all_exports.return_value = {
+        'some-name-1': {
+            'ExportingStackId': 'some-exporting-stack-id-1',
+            'Name': 'some-name-1',
+            'Value': 'some-value-1',
+        },
+        'some-name-2': {
+            'ExportingStackId': 'some-exporting-stack-id-2',
+            'Name': 'some-name-2',
+            'Value': 'some-value-2',
+        },
+    }
+
+    def mock_get_all_imports_side_effect_func(export_name, *_args, **_kwargs):
+        if export_name == 'some-name-1':
+            return [
+                'some-exporting-stack-id-2-stack-name',
+            ]
+        if export_name == 'some-name-2':
+            return [
+                'some-exporting-stack-id-1-stack-name',
+            ]
+        return []  # handles export with no import
+
+    mock_get_all_imports.side_effect = mock_get_all_imports_side_effect_func
+    expected_edges = [('some-exporting-stack-id-1-stack-name', 'some-exporting-stack-id-2-stack-name')]
+    expected_nodes = [
+        'some-exporting-stack-id-1-stack-name',
+        'some-exporting-stack-id-2-stack-name',
+    ]
+
+    actual_graph = build_dependency_graph(nodes_to_ignore=['some-exporting-stack-id-2-stack-name'])
+
+    mock_get_cdk_out_imports.assert_not_called()
+    for expected_node in expected_nodes:
+        with subtests.test(msg='assert graph has node', expected_node=expected_node):
+            assert actual_graph.has_node(expected_node)
+    assert nx.number_of_nodes(actual_graph) == len(expected_nodes)
+
+    for expected_edge in expected_edges:
+        with subtests.test(msg='assert graph has edge', expected_edge=expected_edge):
+            assert actual_graph.has_edge(*expected_edge)
+    assert nx.number_of_edges(actual_graph) == len(expected_edges)
+
+    assert nx.is_directed_acyclic_graph(actual_graph)
+    assert list(nx.simple_cycles(actual_graph)) == []

tests/utils/testing_test.py

@@ -0,0 +1,20 @@
+import itertools
+
+import pytest
+
+from cycl.utils.testing import is_circular_reversible_permutation
+
+# neat that every permutation of 1, 2, 3 is True
+permutations = list(itertools.permutations([1, 2, 3]))
+
+
+@pytest.mark.parametrize(
+    ('lst', 'candidate', 'expected'),
+    [
+        *[([1, 2, 3], list(candidate), True) for candidate in permutations],
+        ([1, 2, 3, 4], [3, 4, 2, 1], False),
+    ],
+)
+def test_is_circular_reversible_permutation(lst, candidate, expected):
+    actual = is_circular_reversible_permutation(lst, candidate)
+    assert actual == expected