Repository: onicagroup/runway

Issue:
update `runway.cfngin.lookups.handlers.output` to use the standardized query syntax
Subtask of #1184 

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/onicagroup/runway/pull/1306?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup) Report
> Merging [#1306](https://codecov.io/gh/onicagroup/runway/pull/1306?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup) (18c9bfd) into [master](https://codecov.io/gh/onicagroup/runway/commit/1afdae6d38b8c9a3fa79bb3983e80e7a250363b7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup) (1afdae6) will **increase** coverage by `0.03%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/onicagroup/runway/pull/1306/graphs/tree.svg?width=650&height=150&src=pr&token=Ku28I0RY80&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup)](https://codecov.io/gh/onicagroup/runway/pull/1306?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup)

```diff
@@            Coverage Diff             @@
##           master    #1306      +/-   ##
==========================================
+ Coverage   90.10%   90.13%   +0.03%     
==========================================
  Files         247      247              
  Lines       15391    15412      +21     
==========================================
+ Hits        13868    13892      +24     
+ Misses       1523     1520       -3     
```


| [Impacted Files](https://codecov.io/gh/onicagroup/runway/pull/1306?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup) | Coverage Δ | |
|---|---|---|
| [runway/cfngin/lookups/handlers/split.py](https://codecov.io/gh/onicagroup/runway/pull/1306/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup#diff-cnVud2F5L2Nmbmdpbi9sb29rdXBzL2hhbmRsZXJzL3NwbGl0LnB5) | `100.00% <ø> (ø)` | |
| [runway/cfngin/lookups/handlers/output.py](https://codecov.io/gh/onicagroup/runway/pull/1306/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup#diff-cnVud2F5L2Nmbmdpbi9sb29rdXBzL2hhbmRsZXJzL291dHB1dC5weQ==) | `96.42% <100.00%> (+10.31%)` | :arrow_up: |
| [runway/exceptions.py](https://codecov.io/gh/onicagroup/runway/pull/1306/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup#diff-cnVud2F5L2V4Y2VwdGlvbnMucHk=) | `99.13% <100.00%> (+<0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/onicagroup/runway/pull/1306?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/onicagroup/runway/pull/1306?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup). Last update [1afdae6...18c9bfd](https://codecov.io/gh/onicagroup/runway/pull/1306?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onicagroup).


Patch:
diff --git a/docs/source/cfngin/configuration.rst b/docs/source/cfngin/configuration.rst
index 08872f53..0c1791a3 100644
--- a/docs/source/cfngin/configuration.rst
+++ b/docs/source/cfngin/configuration.rst
@@ -823,7 +823,7 @@ To do so, use the :ref:`output lookup` in the :attr:`~cfngin.stack.variables` on
 
 .. code-block:: yaml
 
-  MyParameter: ${output OtherStack::OutputName}
+  MyParameter: ${output OtherStack.OutputName}
 
 For more information see :ref:`Lookups <cfngin-lookups>`.
 
@@ -843,7 +843,7 @@ If the **vpc** stack provides an Output called **VpcId**, you can reference it e
       class_path: blueprints.asg.AutoscalingGroup
       variables:
         DomainName: *domain
-        VpcId: ${output vpc::VpcId} # gets the VpcId Output from the vpc stack
+        VpcId: ${output vpc.VpcId} # gets the VpcId Output from the vpc stack
 
 Doing this creates an implicit dependency from the **webservers** stack to the **vpc** stack, which will cause CFNgin to submit the **vpc** stack, and then wait until it is complete until it submits the **webservers** stack.
 This would be the same as adding **vpc** to the :attr:`~cfngin.stack.requires` field of the **webservers** stack.
diff --git a/docs/source/cfngin/lookups/index.rst b/docs/source/cfngin/lookups/index.rst
index 9d3c3068..1023d496 100644
--- a/docs/source/cfngin/lookups/index.rst
+++ b/docs/source/cfngin/lookups/index.rst
@@ -24,7 +24,7 @@ They can be nested in any part of a YAML data structure and within another looku
 
   e.g. if ``custom`` returns a list, this would raise an exception::
 
-    Variable: ${custom something}, ${output otherStack::Output}
+    Variable: ${custom something}, ${output otherStack.Output}
 
   This is valid::
 
@@ -40,11 +40,11 @@ For example, given the following:
       class_path: some.stack.blueprint.Blueprint
       variables:
         Roles:
-          - ${output otherStack::IAMRole}
+          - ${output otherStack.IAMRole}
         Values:
           Env:
-            Custom: ${custom ${output otherStack::Output}}
-            DBUrl: postgres://${output dbStack::User}@${output dbStack::HostName}
+            Custom: ${custom ${output otherStack.Output}}
+            DBUrl: postgres://${output dbStack.User}@${output dbStack.HostName}
 
 The |Blueprint| would have access to the following resolved variables dictionary:
 
diff --git a/docs/source/cfngin/lookups/output.rst b/docs/source/cfngin/lookups/output.rst
index 4e87f98e..f886c73a 100644
--- a/docs/source/cfngin/lookups/output.rst
+++ b/docs/source/cfngin/lookups/output.rst
@@ -4,7 +4,7 @@
 output
 ######
 
-:Query Syntax: ``<stack-name>::<output-name>``
+:Query Syntax: ``<relative-stack-name>.<output-name>[::<arg>=<arg-val>, ...]``
 
 
 The output_ lookup retrieves an Output from the given Stack name within the current |namespace|.
@@ -12,6 +12,20 @@ The output_ lookup retrieves an Output from the given Stack name within the curr
 CFNgin treats output lookups differently than other lookups by auto adding the referenced stack in the lookup as a requirement to the stack whose variable the output value is being passed to.
 
 
+.. versionchanged:: 2.7.0
+  The ``<relative-stack-name>::<output-name>`` syntax is deprecated to comply with Runway's lookup syntax.
+
+
+
+*********
+Arguments
+*********
+
+This Lookup supports all :ref:`Common Lookup Arguments` but, the following have limited or no effect:
+
+- region
+
+
 
 *******
 Example
@@ -21,4 +35,9 @@ You can specify an output lookup with the following syntax:
 
 .. code-block:: yaml
 
-  ConfVariable: ${output someStack::SomeOutput}
+  namespace: example
+
+  stacks:
+    - ...
+      variables:
+        ConfVariable: ${output stack-name.OutputName}
diff --git a/infrastructure/blueprints/prevent_privilege_escalation.py b/infrastructure/blueprints/prevent_privilege_escalation.py
index 5a4081ea..466a11da 100644
--- a/infrastructure/blueprints/prevent_privilege_escalation.py
+++ b/infrastructure/blueprints/prevent_privilege_escalation.py
@@ -129,7 +129,7 @@ class AdminPreventPrivilegeEscalation(Blueprint):
                 Action("account", "*"),
                 Action("aws-portal", "*"),
                 Action("ce", "*"),
-                Action("cur", ""),
+                Action("cur", "*"),
                 Action("savingsplans", "*"),
             ],
             Effect=Deny,
diff --git a/runway/cfngin/lookups/handlers/output.py b/runway/cfngin/lookups/handlers/output.py
index 75542a4c..521fe82b 100644
--- a/runway/cfngin/lookups/handlers/output.py
+++ b/runway/cfngin/lookups/handlers/output.py
@@ -2,18 +2,23 @@
 # pyright: reportIncompatibleMethodOverride=none
 from __future__ import annotations
 
+import logging
 import re
-from typing import TYPE_CHECKING, Any, NamedTuple, Set
+from typing import TYPE_CHECKING, Any, Dict, NamedTuple, Set, Tuple
 
 from typing_extensions import Final, Literal
 
+from ....exceptions import OutputDoesNotExist
 from ....lookups.handlers.base import LookupHandler
+from ....utils import DOC_SITE
 from ...exceptions import StackDoesNotExist
 
 if TYPE_CHECKING:
     from ....context import CfnginContext
     from ....variables import VariableValue
 
+LOGGER = logging.getLogger(__name__)
+
 
 class OutputQuery(NamedTuple):
     """Output query NamedTuple."""
@@ -25,9 +30,25 @@ class OutputQuery(NamedTuple):
 class OutputLookup(LookupHandler):
     """AWS CloudFormation Output lookup."""
 
+    DEPRECATION_MSG = (
+        'lookup query syntax "<relative-stack-name>::<OutputName>" has been deprecated; '
+        "to learn how to use the new lookup query syntax visit "
+        f"{DOC_SITE}/page/cfngin/lookups/output.html"
+    )
     TYPE_NAME: Final[Literal["output"]] = "output"
     """Name that the Lookup is registered as."""
 
+    @classmethod
+    def legacy_parse(cls, value: str) -> Tuple[OutputQuery, Dict[str, str]]:
+        """Retain support for legacy lookup syntax.
+
+        Format of value:
+            <relative-stack-name>::<OutputName>
+
+        """
+        LOGGER.warning("${output %s}: %s", value, cls.DEPRECATION_MSG)
+        return deconstruct(value), {}
+
     @classmethod
     def handle(  # pylint: disable=arguments-differ
         cls, value: str, context: CfnginContext, **_: Any
@@ -36,21 +57,40 @@ class OutputLookup(LookupHandler):
 
         Args:
             value: Parameter(s) given to this lookup.
-                ``<stack_name>::<output_name>``
+                ``<relative-stack-name>.<OutputName>``
             context: Context instance.
 
         Returns:
             Output from the specified stack.
 
         Raises:
+            OutputDoesNotExist: Output not found for Stack.
             StackDoesNotExist: Stack not found for the name provided.
 
         """
-        decon = deconstruct(value)
-        stack = context.get_stack(decon.stack_name)
-        if stack:
-            return stack.outputs[decon.output_name]
-        raise StackDoesNotExist(context.get_fqn(decon.stack_name))
+        try:
+            raw_query, args = cls.parse(value)
+            query = OutputQuery(*raw_query.split("."))
+        except ValueError:
+            query, args = cls.legacy_parse(value)
+
+        stack = context.get_stack(query.stack_name)
+        if not stack:
+            if "default" in args:
+                return cls.format_results(args["default"], **args)
+            raise StackDoesNotExist(context.get_fqn(query.stack_name))
+
+        if "default" in args:  # handle falsy default
+            return cls.format_results(
+                stack.outputs.get(query.output_name, args["default"]), **args
+            )
+
+        try:
+            return cls.format_results(stack.outputs[query.output_name], **args)
+        except KeyError:
+            raise OutputDoesNotExist(
+                stack_name=context.get_fqn(query.stack_name), output=query.output_name
+            ) from None
 
     @classmethod
     def dependencies(cls, lookup_query: VariableValue) -> Set[str]:
@@ -71,11 +111,11 @@ class OutputLookup(LookupHandler):
             if not data_item.resolved:
                 # We encountered an unresolved substitution.
                 # StackName is calculated dynamically based on context:
-                #  e.g. ${output ${default var::source}::name}
+                #  e.g. ${output ${default var::source}.name}
                 # Stop here
                 return set()
             stack_name += data_item.value
-            match = re.search(r"::", stack_name)
+            match = re.search(r"(::|\.)", stack_name)
             if match:
                 stack_name = stack_name[0 : match.start()]
                 return {stack_name}
@@ -86,7 +126,7 @@ class OutputLookup(LookupHandler):
         return set()
 
 
-def deconstruct(value: str) -> OutputQuery:
+def deconstruct(value: str) -> OutputQuery:  # TODO remove in next major release
     """Deconstruct the value."""
     try:
         stack_name, output_name = value.split("::")
diff --git a/runway/cfngin/lookups/handlers/split.py b/runway/cfngin/lookups/handlers/split.py
index 4747675e..421e7586 100644
--- a/runway/cfngin/lookups/handlers/split.py
+++ b/runway/cfngin/lookups/handlers/split.py
@@ -42,7 +42,7 @@ class SplitLookup(LookupHandler):
             (``PublicSubnets``, ``PrivateSubnets``) that are comma separated,
             so you could use this in your config::
 
-                Subnets: ${split ,::${output vpc::PrivateSubnets}}
+                Subnets: ${split ,::${output vpc.PrivateSubnets}}
 
         """
         try:
diff --git a/runway/exceptions.py b/runway/exceptions.py
index e274d5a6..42d5d26e 100644
--- a/runway/exceptions.py
+++ b/runway/exceptions.py
@@ -21,6 +21,7 @@ class RunwayError(Exception):
     """Base class for custom exceptions raised by Runway."""
 
     message: str
+    """Error message."""
 
     def __init__(self, *args: Any, **kwargs: Any) -> None:
         """Instantiate class."""
@@ -259,7 +260,11 @@ class NpmNotFound(RunwayError):
 class OutputDoesNotExist(RunwayError):
     """Raised when a specific stack output does not exist."""
 
-    message: str
+    output: str
+    """Name of the CloudFormation Stack's Output that does not exist."""
+
+    stack_name: str
+    """Name of a CloudFormation Stack."""
 
     def __init__(self, stack_name: str, output: str, *args: Any, **kwargs: Any) -> None:
         """Instantiate class.


Test patch:
diff --git a/infrastructure/blueprints/test_runner_boundary.py b/infrastructure/blueprints/test_runner_boundary.py
index c8859c10..f2f66313 100644
--- a/infrastructure/blueprints/test_runner_boundary.py
+++ b/infrastructure/blueprints/test_runner_boundary.py
@@ -127,10 +127,10 @@ class TestRunnerBoundary(AdminPreventPrivilegeEscalation):
                     "arn:aws:cloudformation:*:${AWS::AccountId}:stack/"
                     f"{self.namespace}-*"
                 ),
-                f"arn:aws:s3:::${self.namespace}",
-                f"arn:aws:s3:::${self.namespace}/*",
-                f"arn:aws:s3:::${self.namespace}-*",
-                f"arn:aws:s3:::${self.namespace}-*/*",
+                f"arn:aws:s3:::{self.namespace}",
+                f"arn:aws:s3:::{self.namespace}/*",
+                f"arn:aws:s3:::{self.namespace}-*",
+                f"arn:aws:s3:::{self.namespace}-*/*",
             ],
         )
 
diff --git a/infrastructure/test-alt/common/iam.cfn/cfngin.yml b/infrastructure/test-alt/common/iam.cfn/cfngin.yml
index 686c78c8..0aa0da92 100644
--- a/infrastructure/test-alt/common/iam.cfn/cfngin.yml
+++ b/infrastructure/test-alt/common/iam.cfn/cfngin.yml
@@ -15,18 +15,18 @@ stacks:
     class_path: blueprints.AdminPreventPrivilegeEscalation
     variables:
       ApprovedPermissionBoundaries:
-        - ${output test-runner-boundary::Policy}
+        - ${output test-runner-boundary.Policy}
   - name: deploy-role
     class_path: blueprints.AdminRole
     variables:
       CrossAccountAccessAccountIds:
         - ${test_account_id}
-      PermissionsBoundary: ${output admin-prevent-privilege-escalation::PolicyArn}
+      PermissionsBoundary: ${output admin-prevent-privilege-escalation.PolicyArn}
       RoleName: ${namespace}-gh-action-deploy
   - name: test-runner-role
     class_path: blueprints.AdminRole
     variables:
       CrossAccountAccessAccountIds:
         - ${test_account_id}
-      PermissionsBoundary: ${output test-runner-boundary::PolicyArn}
+      PermissionsBoundary: ${output test-runner-boundary.PolicyArn}
       RoleName: ${namespace}-gh-action-runner
diff --git a/infrastructure/test/common/iam.cfn/cfngin.yml b/infrastructure/test/common/iam.cfn/cfngin.yml
index 64689e10..60b53c17 100644
--- a/infrastructure/test/common/iam.cfn/cfngin.yml
+++ b/infrastructure/test/common/iam.cfn/cfngin.yml
@@ -20,16 +20,16 @@ stacks:
     variables:
       <<: *DenyAssumeRoleNotResources
       ApprovedPermissionBoundaries:
-        - ${output test-runner-boundary::Policy}
+        - ${output test-runner-boundary.Policy}
   - name: deploy-user
     class_path: blueprints.AdminUser
     variables:
-      PermissionsBoundary: ${output admin-prevent-privilege-escalation::PolicyArn}
+      PermissionsBoundary: ${output admin-prevent-privilege-escalation.PolicyArn}
       UserName: ${namespace}-gh-action-deploy
   - name: test-runner-user
     class_path: blueprints.TestRunnerUser
     variables:
       DenyAssumeRoleNotResources:
         - arn:aws:iam::${test_alt_account_id}:role/${namespace}-gh-action-runner
-      PermissionsBoundary: ${output test-runner-boundary::PolicyArn}
+      PermissionsBoundary: ${output test-runner-boundary.PolicyArn}
       UserName: ${namespace}-gh-action-runner
diff --git a/tests/functional/cfngin/test_locked_stack/01-locked-stack.yaml b/tests/functional/cfngin/test_locked_stack/01-locked-stack.yaml
index 5403141f..bc7dfa88 100644
--- a/tests/functional/cfngin/test_locked_stack/01-locked-stack.yaml
+++ b/tests/functional/cfngin/test_locked_stack/01-locked-stack.yaml
@@ -9,4 +9,4 @@ stacks:
   - name: locked-stack-bastion
     class_path: blueprints.Dummy
     variables:
-      StringVariable: ${output locked-stack-vpc::DummyId}
+      StringVariable: ${output locked-stack-vpc.DummyId}
diff --git a/tests/unit/cfngin/actions/test_deploy.py b/tests/unit/cfngin/actions/test_deploy.py
index 82b8fd5f..671bcc77 100644
--- a/tests/unit/cfngin/actions/test_deploy.py
+++ b/tests/unit/cfngin/actions/test_deploy.py
@@ -182,14 +182,14 @@ class TestBuildAction(
                 {
                     "name": "bastion",
                     "template_path": ".",
-                    "variables": {"test": "${output vpc::something}"},
+                    "variables": {"test": "${output vpc.something}"},
                 },
                 {
                     "name": "db",
                     "template_path": ".",
                     "variables": {
-                        "test": "${output vpc::something}",
-                        "else": "${output bastion::something}",
+                        "test": "${output vpc.something}",
+                        "else": "${output bastion.something}",
                     },
                 },
                 {"name": "other", "template_path": ".", "variables": {}},
diff --git a/tests/unit/cfngin/factories.py b/tests/unit/cfngin/factories.py
index d0df3b2d..e8f47a2b 100644
--- a/tests/unit/cfngin/factories.py
+++ b/tests/unit/cfngin/factories.py
@@ -69,11 +69,11 @@ def mock_context(
 
 
 def generate_definition(
-    base_name: str, stack_id: Any, **overrides: Any
+    base_name: str, stack_id: Any = None, **overrides: Any
 ) -> CfnginStackDefinitionModel:
     """Generate definitions."""
     definition: Dict[str, Any] = {
-        "name": f"{base_name}.{stack_id}",
+        "name": f"{base_name}-{stack_id}" if stack_id else base_name,
         "class_path": f"tests.unit.cfngin.fixtures.mock_blueprints.{base_name.upper()}",
         "requires": [],
     }
diff --git a/tests/unit/cfngin/lookups/handlers/test_output.py b/tests/unit/cfngin/lookups/handlers/test_output.py
index 316393ec..b6bb4465 100644
--- a/tests/unit/cfngin/lookups/handlers/test_output.py
+++ b/tests/unit/cfngin/lookups/handlers/test_output.py
@@ -1,29 +1,114 @@
 """Tests for runway.cfngin.lookups.handlers.output."""
-# pyright: basic, reportUnknownArgumentType=none, reportUnknownVariableType=none
-import unittest
+# pylint: disable=no-self-use,protected-access
+from __future__ import annotations
 
+from typing import TYPE_CHECKING
+
+import pytest
 from mock import MagicMock
 
+from runway._logging import LogLevels
+from runway.cfngin.exceptions import StackDoesNotExist
 from runway.cfngin.lookups.handlers.output import OutputLookup
 from runway.cfngin.stack import Stack
+from runway.exceptions import OutputDoesNotExist
+from runway.variables import VariableValueLiteral
 
 from ...factories import generate_definition
 
+if TYPE_CHECKING:
+    from pytest import LogCaptureFixture
+    from pytest_mock import MockerFixture
+
+    from ....factories import MockCFNginContext
+
+MODULE = "runway.cfngin.lookups.handlers.output"
+
+
+class TestOutputLookup:
+    """Test OutputLookup."""
+
+    @pytest.mark.parametrize("provided", ["stack-name.Output", "stack-name::Output"])
+    def test_dependencies(self, provided: str) -> None:
+        """Test dependencies."""
+        data_item = VariableValueLiteral(provided)
+        var_val = MagicMock()
+        var_val.__iter__.return_value = iter([data_item])
+        assert OutputLookup.dependencies(var_val) == {"stack-name"}
+
+    @pytest.mark.parametrize("provided", ["stack-name:Output", "foobar"])
+    def test_dependencies_none(self, provided: str) -> None:
+        """Test dependencies none found."""
+        data_item = VariableValueLiteral(provided)
+        var_val = MagicMock()
+        var_val.__iter__.return_value = iter([data_item])
+        assert OutputLookup.dependencies(var_val) == set()
+
+    def test_dependencies_not_resolved(self) -> None:
+        """Test dependencies."""
+        data_item = MagicMock(resolved=False)
+        var_val = MagicMock()
+        var_val.__iter__.return_value = iter([data_item])
+        assert OutputLookup.dependencies(var_val) == set()
+
+    @pytest.mark.parametrize(
+        "provided, expected",
+        [
+            ("stack-name::Output", "output-val"),
+            ("stack-name.Output", "output-val"),
+            ("stack-name.Output::default=bar", "output-val"),
+            ("foo.Output::default=bar", "bar"),
+            ("stack-name.foo::default=bar", "bar"),
+        ],
+    )
+    def test_handle(
+        self, cfngin_context: MockCFNginContext, expected: str, provided: str
+    ) -> None:
+        """Test handle."""
+        stack = Stack(
+            definition=generate_definition("stack-name"), context=cfngin_context
+        )
+        stack.set_outputs({"Output": "output-val"})
+        cfngin_context.stacks_dict[cfngin_context.get_fqn(stack.name)] = stack
+        assert OutputLookup.handle(provided, context=cfngin_context) == expected
 
-class TestOutputHandler(unittest.TestCase):
-    """Tests for runway.cfngin.lookups.handlers.output.OutputLookup."""
+    @pytest.mark.parametrize(
+        "provided", ["stack-name.MissingOutput", "stack-name::MissingOutput"]
+    )
+    def test_handle_raise_output_does_not_exist(
+        self, cfngin_context: MockCFNginContext, provided: str
+    ) -> None:
+        """Test handle raise OutputDoesNotExist."""
+        stack = Stack(
+            definition=generate_definition("stack-name"), context=cfngin_context
+        )
+        stack.set_outputs({"Output": "output-val"})
+        cfngin_context.stacks_dict[cfngin_context.get_fqn(stack.name)] = stack
+        with pytest.raises(
+            OutputDoesNotExist,
+            match="Output MissingOutput does not exist on stack "
+            + cfngin_context.get_fqn(stack.name),
+        ):
+            OutputLookup.handle(provided, context=cfngin_context)
 
-    def setUp(self) -> None:
-        """Run before tests."""
-        self.context = MagicMock()
+    @pytest.mark.parametrize("provided", ["stack-name.Output", "stack-name::Output"])
+    def test_handle_raise_stack_does_not_exist(
+        self, cfngin_context: MockCFNginContext, provided: str
+    ) -> None:
+        """Test handle raise StackDoesNotExist."""
+        with pytest.raises(
+            StackDoesNotExist,
+            match=rf'Stack: "{cfngin_context.get_fqn("stack-name")}" does not exist .*',
+        ):
+            OutputLookup.handle(provided, context=cfngin_context)
 
-    def test_output_handler(self) -> None:
-        """Test output handler."""
-        stack = Stack(definition=generate_definition("vpc", 1), context=self.context)
-        stack.set_outputs({"SomeOutput": "Test Output"})
-        self.context.get_stack.return_value = stack
-        value = OutputLookup.handle("stack-name::SomeOutput", context=self.context)
-        self.assertEqual(value, "Test Output")
-        self.assertEqual(self.context.get_stack.call_count, 1)
-        args = self.context.get_stack.call_args
-        self.assertEqual(args[0][0], "stack-name")
+    def test_legacy_parse(
+        self, caplog: LogCaptureFixture, mocker: MockerFixture
+    ) -> None:
+        """Test legacy_parse."""
+        query = "foo"
+        caplog.set_level(LogLevels.WARNING, MODULE)
+        deconstruct = mocker.patch(f"{MODULE}.deconstruct", return_value="success")
+        assert OutputLookup.legacy_parse(query) == (deconstruct.return_value, {})
+        deconstruct.assert_called_once_with(query)
+        assert f"${{output {query}}}: {OutputLookup.DEPRECATION_MSG}" in caplog.messages
diff --git a/tests/unit/cfngin/test_plan.py b/tests/unit/cfngin/test_plan.py
index ffeb50db..9ce0d8e4 100644
--- a/tests/unit/cfngin/test_plan.py
+++ b/tests/unit/cfngin/test_plan.py
@@ -178,7 +178,7 @@ class TestPlan(unittest.TestCase):
         graph = Graph.from_steps([Step(vpc, fn=None), Step(bastion, fn=None)])
         plan = Plan(description="Test", graph=graph)
 
-        self.assertEqual(plan.graph.to_dict(), {"bastion.1": {"vpc.1"}, "vpc.1": set()})
+        self.assertEqual(plan.graph.to_dict(), {"bastion-1": {"vpc-1"}, "vpc-1": set()})
 
     def test_plan_reverse(self) -> None:
         """Test plan reverse."""
@@ -192,8 +192,8 @@ class TestPlan(unittest.TestCase):
 
         # order is different between python2/3 so can't compare dicts
         result_graph_dict = plan.graph.to_dict()
-        self.assertEqual(set(), result_graph_dict.get("bastion.1"))
-        self.assertEqual({"bastion.1"}, result_graph_dict.get("vpc.1"))
+        self.assertEqual(set(), result_graph_dict.get("bastion-1"))
+        self.assertEqual({"bastion-1"}, result_graph_dict.get("vpc-1"))
 
     def test_plan_targeted(self) -> None:
         """Test plan targeted."""
@@ -245,17 +245,17 @@ class TestPlan(unittest.TestCase):
         plan.execute(walk)
 
         # the order these are appended changes between python2/3
-        self.assertIn("namespace-vpc.1", calls)
-        self.assertIn("namespace-bastion.1", calls)
-        self.assertIn("namespace-removed.1", calls)
+        self.assertIn("namespace-vpc-1", calls)
+        self.assertIn("namespace-bastion-1", calls)
+        self.assertIn("namespace-removed-1", calls)
         context.put_persistent_graph.assert_called()
 
         # order is different between python2/3 so can't compare dicts
         result_graph_dict = context.persistent_graph.to_dict()  # type: ignore
         self.assertEqual(2, len(result_graph_dict))
-        self.assertEqual(set(), result_graph_dict.get("vpc.1"))
-        self.assertEqual({"vpc.1"}, result_graph_dict.get("bastion.1"))
-        self.assertIsNone(result_graph_dict.get("namespace-removed.1"))
+        self.assertEqual(set(), result_graph_dict.get("vpc-1"))
+        self.assertEqual({"vpc-1"}, result_graph_dict.get("bastion-1"))
+        self.assertIsNone(result_graph_dict.get("namespace-removed-1"))
 
     def test_execute_plan_no_persist(self) -> None:
         """Test execute plan with no persistent graph."""
@@ -280,7 +280,7 @@ class TestPlan(unittest.TestCase):
 
         plan.execute(walk)
 
-        self.assertEqual(calls, ["namespace-vpc.1", "namespace-bastion.1"])
+        self.assertEqual(calls, ["namespace-vpc-1", "namespace-bastion-1"])
         context.put_persistent_graph.assert_not_called()
 
     def test_execute_plan_locked(self) -> None:
@@ -307,7 +307,7 @@ class TestPlan(unittest.TestCase):
         plan = Plan(description="Test", graph=graph)
         plan.execute(walk)
 
-        self.assertEqual(calls, ["namespace-vpc.1", "namespace-bastion.1"])
+        self.assertEqual(calls, ["namespace-vpc-1", "namespace-bastion-1"])
 
     def test_execute_plan_filtered(self) -> None:
         """Test execute plan filtered."""
@@ -329,12 +329,12 @@ class TestPlan(unittest.TestCase):
 
         context = mock.MagicMock()
         context.persistent_graph_locked = False
-        context.stack_names = ["db.1"]
+        context.stack_names = ["db-1"]
         graph = Graph.from_steps([Step(vpc, fn=fn), Step(db, fn=fn), Step(app, fn=fn)])
         plan = Plan(context=context, description="Test", graph=graph)
         plan.execute(walk)
 
-        self.assertEqual(calls, ["namespace-vpc.1", "namespace-db.1"])
+        self.assertEqual(calls, ["namespace-vpc-1", "namespace-db-1"])
 
     def test_execute_plan_exception(self) -> None:
         """Test execute plan exception."""
@@ -361,7 +361,7 @@ class TestPlan(unittest.TestCase):
         with self.assertRaises(PlanFailed):
             plan.execute(walk)
 
-        self.assertEqual(calls, ["namespace-vpc.1"])
+        self.assertEqual(calls, ["namespace-vpc-1"])
         self.assertEqual(vpc_step.status, FAILED)
 
     def test_execute_plan_skipped(self) -> None:
@@ -387,7 +387,7 @@ class TestPlan(unittest.TestCase):
         plan = Plan(description="Test", graph=graph)
         plan.execute(walk)
 
-        self.assertEqual(calls, ["namespace-vpc.1", "namespace-bastion.1"])
+        self.assertEqual(calls, ["namespace-vpc-1", "namespace-bastion-1"])
 
     def test_execute_plan_failed(self) -> None:
         """Test execute plan failed."""
@@ -417,7 +417,7 @@ class TestPlan(unittest.TestCase):
 
         calls.sort()
 
-        self.assertEqual(calls, ["namespace-db.1", "namespace-vpc.1"])
+        self.assertEqual(calls, ["namespace-db-1", "namespace-vpc-1"])
 
     def test_execute_plan_cancelled(self) -> None:
         """Test execute plan cancelled."""
@@ -442,7 +442,7 @@ class TestPlan(unittest.TestCase):
         plan = Plan(description="Test", graph=graph)
         plan.execute(walk)
 
-        self.assertEqual(calls, ["namespace-vpc.1", "namespace-bastion.1"])
+        self.assertEqual(calls, ["namespace-vpc-1", "namespace-bastion-1"])
 
     def test_execute_plan_graph_locked(self) -> None:
         """Test execute plan with locked persistent graph."""
@@ -456,16 +456,16 @@ class TestPlan(unittest.TestCase):
     def test_build_graph_missing_dependency(self) -> None:
         """Test build graph missing dependency."""
         bastion = Stack(
-            definition=generate_definition("bastion", 1, requires=["vpc.1"]),
+            definition=generate_definition("bastion", 1, requires=["vpc-1"]),
             context=self.context,
         )
 
         with self.assertRaises(GraphError) as expected:
             Graph.from_steps([Step(bastion)])
         message_starts = (
-            "Error detected when adding 'vpc.1' as a dependency of 'bastion.1':"
+            "Error detected when adding 'vpc-1' as a dependency of 'bastion-1':"
         )
-        message_contains = "dependent node vpc.1 does not exist"
+        message_contains = "dependent node vpc-1 does not exist"
         self.assertTrue(str(expected.exception).startswith(message_starts))
         self.assertTrue(message_contains in str(expected.exception))
 
@@ -473,19 +473,19 @@ class TestPlan(unittest.TestCase):
         """Test build graph cyclic dependencies."""
         vpc = Stack(definition=generate_definition("vpc", 1), context=self.context)
         db = Stack(
-            definition=generate_definition("db", 1, requires=["app.1"]),
+            definition=generate_definition("db", 1, requires=["app-1"]),
             context=self.context,
         )
         app = Stack(
-            definition=generate_definition("app", 1, requires=["db.1"]),
+            definition=generate_definition("app", 1, requires=["db-1"]),
             context=self.context,
         )
 
         with self.assertRaises(GraphError) as expected:
             Graph.from_steps([Step(vpc), Step(db), Step(app)])
         message = (
-            "Error detected when adding 'db.1' "
-            "as a dependency of 'app.1': graph is "
+            "Error detected when adding 'db-1' "
+            "as a dependency of 'app-1': graph is "
             "not acyclic"
         )
         self.assertEqual(str(expected.exception), message)
diff --git a/tests/unit/cfngin/test_stack.py b/tests/unit/cfngin/test_stack.py
index c4dfe014..59dcd67a 100644
--- a/tests/unit/cfngin/test_stack.py
+++ b/tests/unit/cfngin/test_stack.py
@@ -53,11 +53,11 @@ class TestStack(unittest.TestCase):
             variables={
                 "Var1": "${noop fakeStack3::FakeOutput}",
                 "Var2": (
-                    "some.template.value:${output fakeStack2::FakeOutput}:"
-                    "${output fakeStack::FakeOutput}"
+                    "some.template.value:${output fakeStack2.FakeOutput}:"
+                    "${output fakeStack.FakeOutput}"
                 ),
-                "Var3": "${output fakeStack::FakeOutput},"
-                "${output fakeStack2::FakeOutput}",
+                "Var3": "${output fakeStack.FakeOutput},"
+                "${output fakeStack2.FakeOutput}",
             },
             requires=["fakeStack"],
         )
@@ -71,7 +71,7 @@ class TestStack(unittest.TestCase):
         definition = generate_definition(
             base_name="vpc",
             stack_id=1,
-            variables={"Var1": "${output vpc.1::FakeOutput}"},
+            variables={"Var1": "${output vpc-1.FakeOutput}"},
         )
         stack = Stack(definition=definition, context=self.context)
         with self.assertRaises(ValueError):
@@ -82,7 +82,7 @@ class TestStack(unittest.TestCase):
         definition = generate_definition(
             base_name="vpc",
             stack_id=1,
-            variables={"Param1": "${output fakeStack::FakeOutput}"},
+            variables={"Param1": "${output fakeStack.FakeOutput}"},
         )
         stack = Stack(definition=definition, context=self.context)
         # pylint: disable=protected-access