stacker icon indicating copy to clipboard operation
stacker copied to clipboard

environments used in dictionary variables that don't exist fail as an unknown lookup

Open phobologic opened this issue 7 years ago • 4 comments

[2018-07-31T13:01:07] Unknown lookup type: "None"
Traceback (most recent call last):
  File "stacker/stacker/plan.py", line 93, in _run_once
    status = self.fn(self.stack, status=self.status)
  File "stacker/stacker/actions/build.py", line 321, in _launch_stack
    stack.resolve(self.context, self.provider)
  File "stacker/stacker/stack.py", line 195, in resolve
    resolve_variables(self.variables, context, provider)
  File "stacker/stacker/variables.py", line 80, in resolve_variables
    variable.resolve(context, provider)
  File "stacker/stacker/variables.py", line 145, in resolve
    resolved_lookups = resolve_lookups(self, context, provider)
  File "stacker/stacker/lookups/registry.py", line 71, in resolve_lookups
    raise UnknownLookupType(lookup)
UnknownLookupType: Unknown lookup type: "None"

This was in a stack config like:

  - name: wrc-service
    class_path: blueprints.wrc.ecs_service.WRCEcsService
    variables:
      << : *private_subnets
      ServiceName: ${environment}-wrc-worker
      Image: ${image_id}:${envvar WRC_SERVICE_GIT_SHA}
      Cluster: ${output wrc-cluster::ClusterId}
      CPU: 2048
      Memory: 4096
      Count: 1
      LogGroup: ${output logGroup::LogGroupId}
      Environment:
        DEPLOYMENT_ENV: ${environment}
        KAILAN_VRSCENE_CONVERSION_QUEUE: ${wrc_worker_queue_name}
        WRC_QUEUE_NORMAL_NAME: VrsceneConversionQueue
        WRC_QUEUE_NORMAL_ARN: ${wrc_worker_queue_arn}
        VROL_HOST: ${vrls_hostname}.${internal_zone}
        VROL_PORT: 30304
        WAREHOUSE_USERNAME: ${wrc_warehouse_username}
        WAREHOUSE_PASSWORD: ${wrc_warehouse_password|
        WAREHOUSE_URL: ${warehouse_url}
        S3_BUCKET_NAME: ${s3_bucket_name}
        GIT_SHA: ${envvar WRC_SERVICE_GIT_SHA}
        AWS_REGION: us-west-2
        AWS_DEFAULT_REGION: us-west-2

Where vrls_hostname was not set in the environment.

This small patch allowed me to find the bad variable, but it was messy - might be able to use this later though:

diff --git a/stacker/lookups/__init__.py b/stacker/lookups/__init__.py
index fddaeb8..e7fb80e 100644
--- a/stacker/lookups/__init__.py
+++ b/stacker/lookups/__init__.py
@@ -64,6 +64,8 @@ def extract_lookups(value):
         for v in value:
             lookups = lookups.union(extract_lookups(v))
     elif isinstance(value, dict):
-        for v in value.values():
-            lookups = lookups.union(extract_lookups(v))
+        for k, v in value.items():
+            lookup = extract_lookups(v)
+            print("%s: %s" % (k, lookup))
+            lookups = lookups.union(lookup)

phobologic avatar Jul 31 '18 20:07 phobologic

I guess I'm more inclined to see a solution by changing the regular expression in lookups/__init__.py.

Think about this:

> import re
> LOOKUP_REGEX = re.compile("""\$\{((?P<type>[._\-a-zA-Z0-9]*(?=\s))?\s*(?P<input>[@\+\/,\._\-a-zA-Z0-9\:\s=\[\]\*]+))\}""", re.VERBOSE)
> LOOKUP_REGEX.match("${vrls_hostname}")
<_sre.SRE_Match object at 0x1060353c0>

Why would this ${vrls_hostname} be treated as a lookup in the first place? A lookup should contain some lookup input, right?

xiaket avatar Jul 31 '18 23:07 xiaket

@xiaket It's been a while, but I think there is a reason why that regex matches - just can't remember what it is. I think a bigger issue has always been that the env vars get replaced first - but we do it in a "safe" way that allows unfound variables to fail through to the next step, which is lookups.

Again, I think there's some tricky reason for this, but I have to think we can do better. I plan to dig into this soon. If you (or anyone else!)has a chance/is interested in figuring it out, go for it too :)

phobologic avatar Jul 31 '18 23:07 phobologic

btw, one thing I've been working on for some time now is removing the use of the regex altogether and using an actual parser... pyparsing to be specific. I think I finally had the breakthrough that will make it possible to use, and it seems fairly clean.

https://gist.github.com/phobologic/ca573f973e9f77ac89a580a12b65f1da

phobologic avatar Jul 31 '18 23:07 phobologic

This looks awesome! We have need for nested lookups here and it's always a headache for us.

xiaket avatar Jul 31 '18 23:07 xiaket