Add tests for labels and network alias functionality (#1253)

* Add tests for labels and network alias features

* Added extra short-circuits if labels and env vars are not conformant in case we refactor parts of the code later. This is more future proof [skip ci]

* Added skip unsafe behaviour and test lenght expectations [skip ci]

---------

Co-authored-by: Arne Tarara <[email protected]>

Author: ribalba

lib/scenario_runner.py

@@ -967,6 +967,7 @@ def setup_services(self):
                             print(TerminalColors.WARNING, warn_message, TerminalColors.ENDC)
                             continue
                         env_var_check_errors.append(f"- key '{env_key}' has wrong format. Only ^[A-Za-z_]+[A-Za-z0-9_]*$ is allowed - Maybe consider using --allow-unsafe or --skip-unsafe")
+                        continue # do not add to append string if not conformant
 
                     # Check the value of the environment var
                     # We only forbid long values (>1024), every character is allowed.
@@ -976,6 +977,7 @@ def setup_services(self):
                             print(TerminalColors.WARNING, arrows(f"Found environment var value with size {len(env_value)} (max allowed length is 1024) - Skipping env var '{env_key}'"), TerminalColors.ENDC)
                             continue
                         env_var_check_errors.append(f"- value of environment var '{env_key}' is too long {len(env_value)} (max allowed length is 1024) - Maybe consider using --allow-unsafe or --skip-unsafe")
+                        continue # do not add to append string if not conformant
 
                     docker_run_string.append('-e')
                     docker_run_string.append(f"{env_key}={env_value}")
@@ -1001,12 +1003,18 @@ def setup_services(self):
                             print(TerminalColors.WARNING, warn_message, TerminalColors.ENDC)
                             continue
                         labels_check_errors.append(f"- key '{label_key}' has wrong format. Only ^[A-Za-z_]+[A-Za-z0-9_.]*$ is allowed - Maybe consider using --allow-unsafe or --skip-unsafe")
+                        continue # do not add to append string if not conformant
 
                     # Check the value of the environment var
                     # We only forbid long values (>1024), every character is allowed.
                     # The value is directly passed to the container and is not evaluated on the host system, so there is no security related reason to forbid special characters.
                     if not self._allow_unsafe and len(label_value) > 1024:
+                        if self._skip_unsafe:
+                            warn_message= arrows(f"Found label length > 1024: {label_key} - Skipping")
+                            print(TerminalColors.WARNING, warn_message, TerminalColors.ENDC)
+                            continue
                         labels_check_errors.append(f"- value of label '{label_key}' is too long {len(label_value)} (max allowed length is 1024) - Maybe consider using --allow-unsafe or --skip-unsafe")
+                        continue # do not add to append string if not conformant
 
                     docker_run_string.append('-l')
                     docker_run_string.append(f"{label_key}={label_value}")

tests/data/usage_scenarios/labels_stress_allowed.yml

@@ -0,0 +1,22 @@
+---
+name: Labels Stress
+author: Dan Mateas
+description: test
+
+services:
+  test-container:
+    type: container
+    image: gcb_stress
+    build:
+      context: ../stress-application
+    labels:
+      TESTALLOWED: 'alpha-num123_'
+      test.label: 'example.com'
+      OTHER_LABEL: 'http://localhost:8080'
+flow:
+  - name: Stress
+    container: test-container
+    commands:
+      - type: console
+        command: stress-ng -c 1 -t 1 -q
+        note: Starting Stress

tests/data/usage_scenarios/labels_stress_forbidden.yml

@@ -0,0 +1,21 @@
+---
+name: Labels Stress
+author: Dan Mateas
+description: test
+
+services:
+  test-container:
+    type: container
+    image: gcb_stress
+    build:
+      context: ../stress-application
+    labels:
+      LABEL_ALLOWED: 'alpha-num123_'
+      LABEL_TOO_LONG: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
+flow:
+  - name: Stress
+    container: test-container
+    commands:
+      - type: console
+        command: stress-ng -c 1 -t 1 -q
+        note: Starting Stress

tests/data/usage_scenarios/network_alias.yml

@@ -0,0 +1,26 @@
+---
+name: Network Alias Test
+author: Dan Mateas
+description: test
+
+networks:
+  gmt-test-network:
+
+services:
+  test-container:
+    type: container
+    image: gcb_stress
+    build:
+      context: ../stress-application
+    networks:
+      gmt-test-network:
+        aliases:
+          - test-alias
+
+flow:
+  - name: Stress
+    container: test-container
+    commands:
+      - type: console
+        command: stress-ng -c 1 -t 1 -q
+        note: Starting Stress

tests/data/usage_scenarios/schema_checker/schema_checker_invalid_network_alias.yml

@@ -0,0 +1,27 @@
+---
+name: Test
+author: Dan Mateas
+description: test
+
+networks:
+  gmt-test-network:
+
+services:
+  test-container:
+    type: container
+    image: gcb_stress
+    networks:
+      gmt-test-network:
+        aliases:
+          - bad!alias
+
+flow:
+  - name: Stress
+    container: test-container
+    commands:
+      - type: console
+        command: stress-ng -c 1 -t 1 -q
+        note: Starting Stress
+        shell: bash
+        log-stdout: true
+        log-stderr: false

tests/data/usage_scenarios/schema_checker/schema_checker_valid_labels_as_dict.yml

@@ -0,0 +1,23 @@
+---
+name: Test
+author: Dan Mateas
+description: test
+
+services:
+  test-container:
+    type: container
+    image: gcb_stress
+    labels:
+      foo: bar
+      bar.baz: qux
+
+flow:
+  - name: Stress
+    container: test-container
+    commands:
+      - type: console
+        command: stress-ng -c 1 -t 1 -q
+        note: Starting Stress
+        shell: bash
+        log-stdout: true
+        log-stderr: false

tests/data/usage_scenarios/schema_checker/schema_checker_valid_labels_as_list.yml

@@ -0,0 +1,23 @@
+---
+name: Test
+author: Dan Mateas
+description: test
+
+services:
+  test-container:
+    type: container
+    image: gcb_stress
+    labels:
+      - foo=bar
+      - bar.baz=qux
+
+flow:
+  - name: Stress
+    container: test-container
+    commands:
+      - type: console
+        command: stress-ng -c 1 -t 1 -q
+        note: Starting Stress
+        shell: bash
+        log-stdout: true
+        log-stderr: false

tests/data/usage_scenarios/schema_checker/schema_checker_valid_network_alias.yml

@@ -0,0 +1,27 @@
+---
+name: Test
+author: Dan Mateas
+description: test
+
+networks:
+  gmt-test-network:
+
+services:
+  test-container:
+    type: container
+    image: gcb_stress
+    networks:
+      gmt-test-network:
+        aliases:
+          - test-alias
+
+flow:
+  - name: Stress
+    container: test-container
+    commands:
+      - type: console
+        command: stress-ng -c 1 -t 1 -q
+        note: Starting Stress
+        shell: bash
+        log-stdout: true
+        log-stderr: false

tests/lib/test_schema_checker.py

@@ -36,6 +36,44 @@ def test_schema_checker_both_network_types_valid():
     schema_checker_b.check_usage_scenario(usage_scenario_b)
 
 
+def test_schema_checker_labels_valid():
+    usage_scenario_name_dict = 'schema_checker_valid_labels_as_dict.yml'
+    usage_scenario_path_dict = os.path.join(CURRENT_DIR, '../data/usage_scenarios/schema_checker/', usage_scenario_name_dict)
+    with open(usage_scenario_path_dict, encoding='utf8') as file:
+        usage_scenario_dict = yaml.safe_load(file)
+    schema_checker_dict = SchemaChecker(validate_compose_flag=True)
+    schema_checker_dict.check_usage_scenario(usage_scenario_dict)
+
+    usage_scenario_name_list = 'schema_checker_valid_labels_as_list.yml'
+    usage_scenario_path_list = os.path.join(CURRENT_DIR, '../data/usage_scenarios/schema_checker/', usage_scenario_name_list)
+    with open(usage_scenario_path_list, encoding='utf8') as file:
+        usage_scenario_list = yaml.safe_load(file)
+    schema_checker_list = SchemaChecker(validate_compose_flag=True)
+    schema_checker_list.check_usage_scenario(usage_scenario_list)
+
+
+def test_schema_checker_network_alias():
+    usage_scenario_name = 'schema_checker_valid_network_alias.yml'
+    usage_scenario_path = os.path.join(CURRENT_DIR, '../data/usage_scenarios/schema_checker/', usage_scenario_name)
+    with open(usage_scenario_path, encoding='utf8') as file:
+        usage_scenario = yaml.safe_load(file)
+    schema_checker = SchemaChecker(validate_compose_flag=True)
+    schema_checker.check_usage_scenario(usage_scenario)
+
+
+def test_schema_checker_invalid_network_alias():
+    usage_scenario_name = 'schema_checker_invalid_network_alias.yml'
+    usage_scenario_path = os.path.join(CURRENT_DIR, '../data/usage_scenarios/schema_checker/', usage_scenario_name)
+    with open(usage_scenario_path, encoding='utf8') as file:
+        usage_scenario = yaml.safe_load(file)
+    schema_checker = SchemaChecker(validate_compose_flag=True)
+    with pytest.raises(SchemaError) as error:
+        schema_checker.check_usage_scenario(usage_scenario)
+    expected_exception = "bad!alias includes disallowed values: ['!']"
+    assert expected_exception in str(error.value), \
+        Tests.assertion_info(f"Exception: {expected_exception}", str(error.value))
+
+
 def test_schema_checker_invalid_missing_description():
     usage_scenario_name = 'schema_checker_invalid_missing_description.yml'
     usage_scenario_path = os.path.join(CURRENT_DIR, '../data/usage_scenarios/schema_checker/', usage_scenario_name)

tests/test_usage_scenario.py

@@ -6,6 +6,7 @@
 import os
 import re
 import subprocess
+import json
 
 GMT_DIR = os.path.realpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), '../'))
 
@@ -89,6 +90,58 @@ def test_env_variable_allow_unsafe_true():
     assert 'TEST_ALLOWED' in env_var_output, Tests.assertion_info('TEST_ALLOWED in env vars', env_var_output)
     assert 'TEST_TOO_LONG' in env_var_output, Tests.assertion_info('TEST_TOO_LONG in env vars', env_var_output)
 
+# labels: [object] (optional)
+# Key-Value pairs for labels on the container
+
+def get_labels():
+    ps = subprocess.run(
+        ['docker', 'inspect', 'test-container'],
+        check=True,
+        stderr=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        encoding='UTF-8',
+    )
+    labels = json.loads(ps.stdout)[0].get('Config', {}).get('Labels', {})
+    return labels
+
+def test_labels_allowed_characters():
+
+    runner = ScenarioRunner(uri=GMT_DIR, uri_type='folder', filename='tests/data/usage_scenarios/labels_stress_allowed.yml', skip_unsafe=False, skip_system_checks=True, dev_cache_build=True, dev_no_sleeps=True, dev_no_metrics=True, dev_no_phase_stats=True)
+    with Tests.RunUntilManager(runner) as context:
+        context.run_until('setup_services')
+        labels = get_labels()
+
+        assert labels.get('TESTALLOWED') == 'alpha-num123_', Tests.assertion_info('TESTALLOWED label', labels)
+        assert labels.get('test.label') == 'example.com', Tests.assertion_info('test.label label', labels)
+        assert labels.get('OTHER_LABEL') == 'http://localhost:8080', Tests.assertion_info('OTHER_LABEL label', labels)
+
+def test_labels_too_long():
+    runner = ScenarioRunner(uri=GMT_DIR, uri_type='folder', filename='tests/data/usage_scenarios/labels_stress_forbidden.yml', skip_system_checks=True, dev_no_metrics=True, dev_no_phase_stats=True, dev_no_sleeps=True, dev_cache_build=True)
+    with pytest.raises(RuntimeError) as e:
+        with Tests.RunUntilManager(runner) as context:
+            context.run_until('setup_services')
+
+    assert "- value of label 'LABEL_TOO_LONG' is too long 1075 (max allowed length is 1024) - Maybe consider using --allow-unsafe or --skip-unsafe" == str(e.value), Tests.assertion_info('Label value is too long', str(e.value))
+
+def test_labels_skip_unsafe_true():
+    runner = ScenarioRunner(uri=GMT_DIR, uri_type='folder', filename='tests/data/usage_scenarios/labels_stress_forbidden.yml', skip_unsafe=True, skip_system_checks=True, dev_no_metrics=True, dev_no_phase_stats=True, dev_no_sleeps=True, dev_cache_build=True)
+
+    with Tests.RunUntilManager(runner) as context:
+        context.run_until('setup_services')
+        labels = get_labels()
+
+    assert 'LABEL_ALLOWED' in labels, Tests.assertion_info('LABEL_ALLOWED in labels', labels)
+    assert 'LABEL_TOO_LONG' not in labels, Tests.assertion_info('LABEL_TOO_LONG not in labels', labels)
+
+def test_labels_allow_unsafe_true():
+    runner = ScenarioRunner(uri=GMT_DIR, uri_type='folder', filename='tests/data/usage_scenarios/labels_stress_forbidden.yml', allow_unsafe=True, skip_system_checks=True, dev_no_metrics=True, dev_no_phase_stats=True, dev_no_sleeps=True, dev_cache_build=True)
+    with Tests.RunUntilManager(runner) as context:
+        context.run_until('setup_services')
+        labels = get_labels()
+
+    assert 'LABEL_ALLOWED' in labels, Tests.assertion_info('LABEL_ALLOWED in labels', labels)
+    assert 'LABEL_TOO_LONG' in labels, Tests.assertion_info('LABEL_TOO_LONG in labels', labels)
+
 # ports: [int:int] (optional)
 # Docker container portmapping on host OS to be used with --allow-unsafe flag.
 
@@ -514,6 +567,18 @@ def test_container_is_in_network():
         inspect = ps.stdout
     assert 'test-container' in inspect, Tests.assertion_info('test-container', inspect)
 
+def test_network_alias_added():
+    runner = ScenarioRunner(uri=GMT_DIR, uri_type='folder', filename='tests/data/usage_scenarios/network_alias.yml', skip_system_checks=True, dev_no_metrics=True, dev_no_phase_stats=True, dev_no_sleeps=True, dev_cache_build=True)
+    out = io.StringIO()
+    err = io.StringIO()
+    with redirect_stdout(out), redirect_stderr(err):
+        with Tests.RunUntilManager(runner) as context:
+            context.run_until('setup_services')
+
+    assert 'Adding network alias test-alias for network gmt-test-network in service test-container' in out.getvalue()
+    docker_run_command = re.search(r"docker run with: (.*)", out.getvalue()).group(1)
+    assert '--network-alias test-alias' in docker_run_command
+
 
 
 def test_cmd_entrypoint():