Fix validation of notification endpoint to raise when empty

docs/installation-guide.md

@@ -391,7 +391,7 @@ Guide](./admin-guide.md#adfconfig).
 
 #### Parameter MainNotificationEndpoint
 
-Optional, default value: (empty)
+Required on installation, optional on update, default value: (empty)
 
 Example: `[email protected]`
 

src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/config.py

@@ -76,19 +76,30 @@ def _validate(self):
                 "adfconfig.yml is missing required properties. "
                 "Please see the documentation."
             ) from None
-
-        try:
-            if self.config.get("scp"):
-                assert self.config.get("scp").get("keep-default-scp") in [
-                    "enabled",
-                    "disabled",
-                ]
-        except AssertionError:
+        if "" in (
+            self.cross_account_access_role,
+            self.deployment_account_region,
+            self.notification_endpoint,
+        ):
+            raise InvalidConfigError(
+                "adfconfig.yml is missing required properties, set as ''. "
+                "Please see the documentation."
+            ) from None
+        if self.notification_type == "email" and "@" not in self.notification_endpoint:
             raise InvalidConfigError(
-                "Configuration settings for organizations should be either "
-                "enabled or disabled"
+                "The main-notification-endpoint configured in adfconfig.yml, "
+                "is configured as an email, but lacks the '@' character. "
+                "Please see the documentation."
             ) from None
 
+        if self.config.get("scp"):
+            valid_options = ["enabled", "disabled"]
+            if self.config.get("scp").get("keep-default-scp") not in valid_options:
+                raise InvalidConfigError(
+                    "Configuration settings for organizations should be either "
+                    "enabled or disabled"
+                ) from None
+
         if isinstance(self.deployment_account_region, list):
             if len(self.deployment_account_region) > 1:
                 raise InvalidConfigError(
@@ -134,18 +145,15 @@ def _parse_config(self):
 
         # TODO Investigate why this only considers the first notification
         # endpoint. Seems like a bug, it should support multiple.
-        adf_config_notification_type = self.config.get("main-notification-endpoint")[
-            0
-        ].get("type")
+        main_notification_endpoint = (self.config.get("main-notification-endpoint") or [{}])[0]
         self.notification_type = (
-            "lambda" if adf_config_notification_type == "slack" else "email"
+            "lambda" if main_notification_endpoint.get("type") == "slack" else "email"
         )
-        self.notification_endpoint = self.config.get("main-notification-endpoint")[
-            0
-        ].get("target")
+        self.notification_endpoint = main_notification_endpoint.get("target", "")
         self.notification_channel = (
             None if self.notification_type == "email" else self.notification_endpoint
         )
+
         self.extensions = self.config_contents.get("extensions", {})
         self._configure_default_extensions_behavior()
 

src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/tests/test_config.py

@@ -60,6 +60,12 @@ def test_raise_validation_remove_deployment_target_region(cls):
         assert cls._parse_config()
 
 
+def test_raise_validation_no_notification_endpoint(cls):
+    cls.config.pop("main-notification-endpoint", None)
+    with raises(InvalidConfigError):
+        assert cls._parse_config()
+
+
 def test_raise_validation_length_deployment_target_region(cls):
     cls.config_contents["regions"]["deployment-account"] = [
         "region1",
@@ -69,6 +75,44 @@ def test_raise_validation_length_deployment_target_region(cls):
         assert cls._parse_config()
 
 
+def test_raise_validation_empty_roles(cls):
+    cls.config_contents["roles"]["cross-account-access"] = ""
+    with raises(InvalidConfigError):
+        assert cls._parse_config()
+
+
+def test_raise_validation_empty_deployment_region(cls):
+    cls.config_contents["regions"]["deployment-account"] = ""
+    with raises(InvalidConfigError):
+        assert cls._parse_config()
+
+
+def test_raise_validation_zero_notification_endpoint(cls):
+    cls.config["main-notification-endpoint"] = []
+    with raises(InvalidConfigError):
+        assert cls._parse_config()
+
+
+def test_raise_validation_empty_obj_notification_endpoint(cls):
+    cls.config["main-notification-endpoint"] = [{}]
+    with raises(InvalidConfigError):
+        assert cls._parse_config()
+
+
+def test_raise_validation_empty_email_notification_endpoint(cls):
+    cls.config["main-notification-endpoint"] = [{"target": ""}]
+    with raises(InvalidConfigError):
+        assert cls._parse_config()
+
+
+def test_raise_validation_no_at_in_email_notification_endpoint(cls):
+    cls.config["main-notification-endpoint"] = [
+        {"target": "some-str", "type": "email"},
+    ]
+    with raises(InvalidConfigError):
+        assert cls._parse_config()
+
+
 def test_sorted_regions(cls):
     cls.config_contents["regions"]["deployment-account"] = [
         "us-east-1",