Optional facts

.github/workflows/test.yml

@@ -5,66 +5,35 @@ on:
     - master
 
 jobs:
-  lint_flake8:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v1
+  tests:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        command:
+          - lint_flake8
+          - lint_pylint
+          - lint_safety
+          - lint_bandit
+          - lint_readme
+          - test_py36
+          - test_py37
+          - test_py38
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-python@v2
+    - name: Install Pipenv
+      run: |
+        pip install pipenv tox
     - name: Flake8
-      run: make lint_flake8
-  lint_pylint:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v1
-    - name: Pylint
-      run: make lint_pylint
-  lint_safety:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v1
-    - name: Safety
-      run: make lint_safety
-  lint_bandit:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v1
-    - name: Bandit
-      run: make lint_bandit
-  lint_readme:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v1
-    - name: Readme
-      run: make lint_readme
-  test_py36:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v1
-    - name: Run tests on Python 3.6
-      run: make test_py36
-  test_py37:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v1
-    - name: Run tests on Python 3.7
-      run: make test_py37
-  test_py38:
-    runs-on: ubuntu-latest
-    steps:
-    - uses: actions/checkout@v1
+      run: make ${{ matrix.command }}
+
     - name: Run tests on Python 3.8
       run: make test_py38
   build:
     needs:
-    - lint_flake8
-    - lint_pylint
-    - lint_safety
-    - lint_bandit
-    - lint_readme
-    - test_py36
-    - test_py37
-    - test_py38
+    - tests
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v1
+    - uses: actions/checkout@v2
     - name: Build image
       run: make docker

Pipfile

@@ -24,5 +24,5 @@ build_kapitan_helm_binding = "./tools/build_kapitan_helm_binding.sh"
 autopep = "autopep8 --in-place --aggressive --recursive --verbose ./"
 local_reveal = "./tools/reveal.sh"
 compile = "kapitan compile -J . dependencies/ --refs-path ./catalog/refs"
-test = "docker run -it --rm -v $PWD:/src:ro -v $PWD/.dtox:/app/.tox docker.io/painless/tox"
+test = "tox"
 update_requirements = "tox -e requirements"

commodore/cluster.py

@@ -21,37 +21,39 @@ def fetch_cluster(cfg, clusterid):
 
 def reconstruct_api_response(target_yml):
     target_data = yaml_load(target_yml)['parameters']
-    return {
-        "id": target_data['cluster']['name'],
-        "facts": {
-            "cloud": target_data['cloud']['region'],
-            "distribution": target_data['cluster']['dist'],
-            "region": target_data['cloud']['region'],
+    api_resp = {
+        'id': target_data['cluster']['name'],
+        'facts': {
+            'cloud': target_data['cloud']['provider'],
+            'distribution': target_data['cluster']['dist'],
         },
-        "gitRepo": {
-            "url": target_data['cluster']['catalog_url'],
+        'gitRepo': {
+            'url': target_data['cluster']['catalog_url'],
         },
-        "tenant": target_data['customer']['name'],
+        'tenant': target_data['customer']['name'],
     }
+    if 'region' in target_data['cloud']:
+        api_resp['facts']['region'] = target_data['cloud']['region']
+    return api_resp
 
 
 def _full_target(cluster, components, catalog):
-    cloud_provider = cluster['facts']['cloud']
-    cloud_region = cluster['facts']['region']
-    cluster_distro = cluster['facts']['distribution']
+    cluster_facts = cluster['facts']
+    for required_fact in ['distribution', 'cloud']:
+        if required_fact not in cluster_facts:
+            raise click.ClickException(f"Required fact '{required_fact}' not set")
+
+    cluster_distro = cluster_facts['distribution']
+    cloud_provider = cluster_facts['cloud']
     cluster_id = cluster['id']
     customer = cluster['tenant']
     component_defaults = [f"defaults.{cn}" for cn in components if
                           (P('inventory/classes/defaults') / f"{cn}.yml").is_file()]
     global_defaults = ['global.common', f"global.{cluster_distro}", f"global.{cloud_provider}"]
-    if not cluster_distro:
-        raise click.ClickException("Required fact 'distribution' not set")
-    if not cloud_provider:
-        raise click.ClickException("Required fact 'cloud' not set")
-    if cloud_region:
-        global_defaults.append(f"global.{cloud_provider}.{cloud_region}")
+    if 'region' in cluster_facts:
+        global_defaults.append(f"global.{cloud_provider}.{cluster_facts['region']}")
     global_defaults.append(f"{customer}.{cluster_id}")
-    return {
+    target = {
         'classes': component_defaults + global_defaults,
         'parameters': {
             'target_name': 'cluster',
@@ -62,13 +64,15 @@ def _full_target(cluster, components, catalog):
             },
             'cloud': {
                 'provider': f"{cloud_provider}",
-                'region': f"{cloud_region}",
             },
             'customer': {
                 'name': f"{customer}"
             },
         }
     }
+    if 'region' in cluster_facts:
+        target['parameters']['cloud']['region'] = cluster_facts['region']
+    return target
 
 
 def update_target(cfg, cluster):

commodore/git.py

@@ -161,10 +161,10 @@ def stage_all(repo):
                     # and a_blob as after.
                     before = c.b_blob.data_stream.read().decode('utf-8').split('\n')
                     after = c.a_blob.data_stream.read().decode('utf-8').split('\n')
-                    u = difflib.unified_diff(before, after, lineterm='',
-                                             fromfile=c.b_path, tofile=c.a_path)
-                    u = [_colorize_diff(l) for l in u]
-                    difftext.append('\n'.join(u).strip())
+                    diff = difflib.unified_diff(before, after, lineterm='',
+                                                fromfile=c.b_path, tofile=c.a_path)
+                    diff = [_colorize_diff(line) for line in diff]
+                    difftext.append('\n'.join(diff).strip())
 
     return '\n'.join(difftext), changed
 

tests/test_target.py

@@ -0,0 +1,122 @@
+"""
+Unit-tests for target generation
+"""
+
+import click
+import pytest
+import commodore.cluster as cluster
+
+
+cluster_obj = {
+    'id': 'mycluster',
+    'tenant': 'mytenant',
+    'facts': {
+        'distribution': 'rancher',
+        'cloud': 'cloudscale',
+    }
+}
+components = ['test-component']
+catalog = 'ssh://git@git.example.com/cluster-catalogs/mycluster'
+
+
+def test_render_target():
+    target = cluster._full_target(cluster_obj, components, catalog)
+    facts = cluster_obj['facts']
+    assert target != ""
+    all_classes = ([f"defaults.{cn}" for cn in components] +
+                   ['global.common',
+                       f"global.{facts['distribution']}",
+                       f"global.{facts['cloud']}",
+                       f"{cluster_obj['tenant']}.{cluster_obj['id']}",
+                    ])
+    assert len(target['classes']) == len(
+        all_classes), "rendered target includes different amount of classes"
+    # Test order of included classes
+    for i in range(len(all_classes)):
+        assert target['classes'][i] == all_classes[i]
+    assert target['parameters']['target_name'] == 'cluster'
+
+
+def test_optional_facts():
+    cluster_obj['facts']['region'] = 'rma1'
+    target = cluster._full_target(cluster_obj, components, catalog)
+    facts = cluster_obj['facts']
+    assert f"global.{facts['cloud']}.{facts['region']}" in target['classes']
+
+
+def test_missing_facts():
+    cl = {
+        'id': 'mycluster',
+        'tenant': 'mytenant',
+        'facts': {
+            'distribution': 'rancher',
+        }
+    }
+    with pytest.raises(click.ClickException):
+        cluster._full_target(cl, components, catalog)
+
+
+def test_reconstruct_api_response(tmp_path):
+    targetyml = tmp_path / 'cluster.yml'
+    with open(targetyml, 'w') as file:
+        file.write('''classes:
+- defaults.argocd
+- global.common
+- global.k3d
+- global.localdev
+- t-delicate-pine-3938.c-twilight-water-9032
+parameters:
+  cloud:
+    provider: localdev
+    region: north
+  cluster:
+    catalog_url: ssh://git@git.vshn.net/syn-dev/cluster-catalogs/srueg-k3d-int.git
+    dist: k3d
+    name: c-twilight-water-9032
+  customer:
+    name: t-delicate-pine-3938
+  target_name: cluster ''')
+
+    api_response = cluster.reconstruct_api_response(targetyml)
+    assert api_response['id'] == 'c-twilight-water-9032'
+    assert api_response['tenant'] == 't-delicate-pine-3938'
+    assert api_response['facts']['distribution'] == 'k3d'
+    assert api_response['facts']['region'] == 'north'
+
+
+def test_reconstruct_api_response_no_region(tmp_path):
+    targetyml = tmp_path / 'cluster.yml'
+    with open(targetyml, 'w') as file:
+        file.write('''classes:
+parameters:
+  cloud:
+    provider: localdev
+  cluster:
+    catalog_url: ssh://git@git.vshn.net/syn-dev/cluster-catalogs/srueg-k3d-int.git
+    dist: k3d
+    name: c-twilight-water-9032
+  customer:
+    name: t-delicate-pine-3938
+  target_name: cluster ''')
+
+    api_response = cluster.reconstruct_api_response(targetyml)
+    assert 'region' not in api_response['facts']
+
+
+def test_reconstruct_api_response_missing_fact(tmp_path):
+    targetyml = tmp_path / 'cluster.yml'
+    with open(targetyml, 'w') as file:
+        file.write('''classes:
+parameters:
+  cloud:
+    region: north
+  cluster:
+    catalog_url: ssh://git@git.vshn.net/syn-dev/cluster-catalogs/srueg-k3d-int.git
+    dist: k3d
+    name: c-twilight-water-9032
+  customer:
+    name: t-delicate-pine-3938
+  target_name: cluster ''')
+
+    with pytest.raises(KeyError):
+        cluster.reconstruct_api_response(targetyml)

tox.mk

@@ -1,29 +1,32 @@
-.PHONY: lint_flake8 lint_pylint lint_safety lint_bandit lint_readme
+.PHONY: tox lint_flake8 lint_pylint lint_safety lint_bandit lint_readme
 
-TOX_COMMAND = docker run --rm -v $(PWD):/src:ro -v $(PWD)/.tox:/app/.tox docker.io/painless/tox
+TOX_COMMAND = pipenv run tox
+
+tox:
+	$(TOX_COMMAND)
 
 lint_flake8:
-	$(TOX_COMMAND) tox -e flake8
+	$(TOX_COMMAND) -e flake8
 
 lint_pylint:
-	$(TOX_COMMAND) tox -e pylint
+	$(TOX_COMMAND) -e pylint
 
 lint_safety:
-	$(TOX_COMMAND) tox -e safety
+	$(TOX_COMMAND) -e safety
 
 lint_bandit:
-	$(TOX_COMMAND) tox -e bandit
+	$(TOX_COMMAND) -e bandit
 
 lint_readme:
-	$(TOX_COMMAND) tox -e readme
+	$(TOX_COMMAND) -e readme
 
 .PHONY: test_py36 test_py37 test_py38
 
 test_py36:
-	$(TOX_COMMAND) tox -e py36
+	$(TOX_COMMAND) -e py36
 
 test_py37:
-	$(TOX_COMMAND) tox -e py37
+	$(TOX_COMMAND) -e py37
 
 test_py38:
-	$(TOX_COMMAND) tox -e py38
+	$(TOX_COMMAND) -e py38