Merge pull request #358 from dgraeber/feature/parameter-validation

raise error if metadata or valuefrom parameter is none

CHANGELOG.md

@@ -10,7 +10,7 @@ This project adheres to [Semantic Versioning](http://semver.org/) and [Keep a Ch
 - adding qualifier support for bootstrap roles
 
 ### Changes
-
+- raise error if a metadata parameter or value_from parameter is not available
 ### Fixes
 
 ## v2.8.0 (2023-05-23)

seedfarmer/commands/_parameter_commands.py

@@ -19,6 +19,7 @@
 
 from aws_codeseeder import EnvVar, EnvVarType
 
+import seedfarmer.errors
 from seedfarmer import config
 from seedfarmer.mgmt.module_info import get_module_metadata
 from seedfarmer.models.manifests import DeploymentManifest, ModuleParameter
@@ -65,7 +66,12 @@ def load_parameter_values(
                 module_metatdata = _module_metatdata(
                     deployment_name, parameter, parameter_values_cache, deployment_manifest
                 )
-                parameter_values.append(module_metatdata) if module_metatdata else None
+                if module_metatdata:
+                    parameter_values.append(module_metatdata)
+                else:
+                    raise seedfarmer.errors.InvalidManifestError(
+                        f"The module metadata parameter ({parameter.value_from.module_metadata}) is not available"
+                    )
             elif parameter.value_from.env_variable:
                 parameter_values.append(
                     ModuleParameter(name=parameter.name, value=os.getenv(parameter.value_from.env_variable, ""))
@@ -86,11 +92,15 @@ def load_parameter_values(
                 )
             elif parameter.value_from.parameter_value:
                 p_value = deployment_manifest.get_parameter_value(
-                    parameter=parameter.value_from.parameter_value, account_alias=target_account, region=target_region
+                    parameter=parameter.value_from.parameter_value, account_id=target_account, region=target_region
                 )
-                if p_value:
+                if p_value is not None:
                     p_value = str(p_value) if isinstance(p_value, str) else json.dumps(p_value)
                     parameter_values.append(ModuleParameter(name=parameter.name, value=p_value))
+                else:
+                    raise seedfarmer.errors.InvalidManifestError(
+                        f"The parameter value defined ({parameter.value_from.parameter_value}) is not available"
+                    )
 
     return parameter_values
 

test/unit-test/test_commands_parameters.py

@@ -8,6 +8,7 @@
 from moto import mock_sts
 
 import seedfarmer.commands._parameter_commands as pc
+import seedfarmer.errors
 from seedfarmer.models.manifests import DeploymentManifest, ModuleManifest, ModuleParameter
 from seedfarmer.services._service_utils import boto3_client
 from seedfarmer.services.session_manager import SessionManager
@@ -114,6 +115,55 @@ def session_manager(sts_client):
     ],
 }
 
+deployment_manifest_fail_json = {
+    "name": "mlops",
+    "toolchain_region": "us-east-1",
+    "groups": [
+        {
+            "name": "core",
+            "path": "manifests/mlops/core-modules.yaml",
+            "modules": [
+                {
+                    "name": "efs",
+                    "path": "modules/core/efs",
+                    "parameters": [
+                        {"name": "removal-policy", "value": "DESTROY"},
+                        {
+                            "name": "vpc-id",
+                            "value_from": {
+                                "module_metadata": {"name": "networking", "group": "optionals", "key": "VpcId"},
+                            },
+                        },
+                        {"name": "test-secrets-manager", "value_from": {"secretsManager": "my-secret-vpc-id"}},
+                        {"name": "test-ssm-store", "value_from": {"parameterStore": "my-ssm-name"}},
+                        {"name": "test-regional-param", "value_from": {"parameterValue": "testRegionalParam"}},
+                    ],
+                    "target_account": "primary",
+                    "target_region": "us-east-1",
+                }
+            ],
+        },
+    ],
+    "target_account_mappings": [
+        {
+            "alias": "primary",
+            "account_id": "123456789012",
+            "default": True,
+            "parameters_global": {
+                "dockerCredentialsSecret": "aws-addf-docker-credentials",
+                "testRegionalParam": "afda",
+            },
+            "region_mappings": [
+                {
+                    "region": "us-east-1",
+                    "default": True,
+                    "parameters_regional": {"testRegionalParam": "somethingawesomehere"},
+                }
+            ],
+        }
+    ],
+}
+
 
 @pytest.mark.commands
 @pytest.mark.commands_parameters
@@ -150,10 +200,69 @@ def test_load_parameter_values(session_manager, mocker):
         target_region="us-east-1",
     )
     names = []
-    print(params)
     for module_parameter in params:
         names.append(module_parameter.name)
     assert ("removal-policy" in names) == True
     assert ("vpc-id" in names) == True
     assert ("test-secrets-manager" in names) == True
     assert ("test-ssm-store" in names) == True
+    assert ("test-regional-param" in names) == True
+
+
+@pytest.mark.commands
+@pytest.mark.commands_parameters
+def test_load_parameter_values_missing_metadata(session_manager, mocker):
+    mocker.patch(
+        "seedfarmer.commands._parameter_commands.get_module_metadata",
+        return_value={
+            "IsolatedSubnetIds": [],
+            "PrivateSubnetIds": ["subnet-0758c0b5ba97e0fc9", "subnet-0dc60fe4557261145"],
+            "PublicSubnetIds": ["subnet-089b632dada2c71e8", "subnet-0296fff0ba0fa48c0"],
+            "VpcId": "vpc-01e556d052f429282",
+        },
+    )
+    faulty_d = deployment_manifest_fail_json
+    faulty_d["groups"][0]["modules"][0]["parameters"][4] = {
+        "name": "vpc-id-bad",
+        "value_from": {
+            "module_metadata": {"name": "networking", "group": "optionals", "key": "VpcId-NONE"},
+        },
+    }
+    dep = DeploymentManifest(**faulty_d)
+    dep.validate_and_set_module_defaults()
+    with pytest.raises(seedfarmer.errors.InvalidManifestError):
+        pc.load_parameter_values(
+            deployment_name="mlops",
+            deployment_manifest=dep,
+            parameters=dep.groups[0].modules[0].parameters,
+            target_account="123456789012",
+            target_region="us-east-1",
+        )
+
+
+@pytest.mark.commands
+@pytest.mark.commands_parameters
+def test_load_parameter_values_missing_param_value(session_manager, mocker):
+    mocker.patch(
+        "seedfarmer.commands._parameter_commands.get_module_metadata",
+        return_value={
+            "IsolatedSubnetIds": [],
+            "PrivateSubnetIds": ["subnet-0758c0b5ba97e0fc9", "subnet-0dc60fe4557261145"],
+            "PublicSubnetIds": ["subnet-089b632dada2c71e8", "subnet-0296fff0ba0fa48c0"],
+            "VpcId": "vpc-01e556d052f429282",
+        },
+    )
+    faulty_d = deployment_manifest_fail_json
+    faulty_d["groups"][0]["modules"][0]["parameters"][4] = (
+        {"name": "test-regional-param", "value_from": {"parameterValue": "regParamMissing"}},
+    )
+    dep = DeploymentManifest(**faulty_d)
+    dep.validate_and_set_module_defaults()
+    with pytest.raises(seedfarmer.errors.InvalidManifestError):
+        pc.load_parameter_values(
+            deployment_name="mlops",
+            deployment_manifest=dep,
+            parameters=dep.groups[0].modules[0].parameters,
+            target_account="123456789012",
+            target_region="us-east-1",
+        )