Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E0002/W1001 Edge Case: Fn::Transform in Fn::Sub.String #2587

Closed
LaikaN57 opened this issue Feb 14, 2023 · 9 comments
Closed

E0002/W1001 Edge Case: Fn::Transform in Fn::Sub.String #2587

LaikaN57 opened this issue Feb 14, 2023 · 9 comments
Assignees
Labels
bug Something isn't working

Comments

@LaikaN57
Copy link

LaikaN57 commented Feb 14, 2023

CloudFormation Lint Version

0.73.1

What operating system are you using?

macOS 13.1 (22C65)

Describe the bug

Context

W1001 Checks for Ref's to conditional resources in Fn::Sub's String parameter (ie. 0th list index).

🐛 Bug

Using an Fn::Transform breaks this check during a regex scan for Ref tokens in the Fn::Sub's String parameter as the regex is expecting a string type.

🔍 Stack Trace

E0002 Unknown exception while processing rule W1001: Traceback (most recent call last):
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/rules/__init__.py", line 320, in run_check
    return check(*args)
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/rules/__init__.py", line 44, in wrapper
    results = match_function(self, filename, cfn, *args, **kwargs)
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/rules/__init__.py", line 202, in matchall
    return self.match(cfn)  # pylint: disable=E1102
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/rules/functions/RelationshipConditions.py", line 99, in match
    string_params = cfn.get_sub_parameters(sub_string[0])
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/template.py", line 627, in get_sub_parameters
    string_params = regex.findall(sub_string)
TypeError: expected string or bytes-like object

image

Expected behavior

I would expect that Fn::Sub's which contain an Fn::Transform as the String parameter to be evaluated "post-transform" for W1001.

Alternatively, I would expect that using an Fn::Transform as the String parameter in an Fn::Sub would result in a different warning as some type of "bad practice".

📖 Doc Findings

For the String parameter, you can't use any functions. You must specify a string value.

AWS CloudFormation User Guide - Fn::Sub Supported functions

I believe the doc is making this statement as it applies to "post-transform". I will open a related docfix issue against that page to make this entry more correct.

Reproduction template

Partial

Resources:
  Resource1:
    Type: Custom::ResourceType1
    Properties:
      ServiceToken:
        Fn::ImportValue:
          Fn::Sub: ${Stage1Stack}-ServiceToken1
      Data:
        Fn::Sub:
          - Fn::Transform:
              Name: Jinja
              Parameters:
                OutputFormat: String
                JinjaTemplate:
                  # [REDACTED - This template has been heavily redacted but
                  # one could image complex Jinja template logic.]
                  Fn::Base64: |
                    - rolearn: "${InstanceRole}"
                      username: {% raw %}'system:node:{{EC2PrivateDNSName}}'{%- endraw %}
                    - rolearn: arn:aws:iam::${AWS::AccountId}:role/AWSServiceRoleForAmazonEMRContainers
                      username: emr-containers
          - InstanceRole:
              Fn::ImportValue:
                Fn::Sub: '${Stage1Stack}-InstanceRole'
@LaikaN57
Copy link
Author

Looks like we might have regressed? #920

@LaikaN57
Copy link
Author

This seems to be a non-fatal issue here as well:

if isinstance(value, list):
if not value:
continue
if len(value) == 2:
sub_parameter_values = value[1]
sub_parameters = self._find_parameter(value[0])
elif isinstance(value, (str)):
sub_parameters = self._find_parameter(value)

🔍 Stack Trace

2023-02-14 01:36:53,275 - cfnlint.template - ERROR - Encountered unknown error while building graph: expected string or bytes-like object
Traceback (most recent call last):
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/template.py", line 48, in __init__
    self.graph = Graph(self)
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/graph.py", line 92, in __init__
    self._add_subs(cfn)
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/graph.py", line 223, in _add_subs
    sub_parameters = self._find_parameter(value[0])
  File "/Users/akennedy/src/catapult/.venv/lib/python3.10/site-packages/cfnlint/graph.py", line 268, in _find_parameter
    return regex.findall(string)
TypeError: expected string or bytes-like object

@PatMyron
Copy link
Contributor

Looks like we might have regressed? #920


That PR is a separate rule, but likely regressed in #2525


More generally, tracking transform support in #476. It's been open for a while, but I still think some general solution will be more sustainable than if-statements across all individual rules that could be affected by transforms

@kddejong kddejong self-assigned this Feb 14, 2023
@kddejong kddejong added the bug Something isn't working label Feb 14, 2023
@kddejong
Copy link
Contributor

I'm seeing errors across the board. I think what makes sense is to validate that the get_sub_parameters regex match validate that the sub_string is actually a string before running regex on it. It should be on the rule to validate the structure of the Fn::Sub to know if this properly configured.

E0002 Unknown exception while processing rule W2001: expected string or bytes-like object
local/issue/2587.yaml:1:1

E0002 Unknown exception while processing rule W1001: expected string or bytes-like object
local/issue/2587.yaml:1:1

E0002 Unknown exception while processing rule E1019: expected string or bytes-like object
local/issue/2587.yaml:1:1

with that change:

E1019 Parameter Stage1Stack for Fn::Sub not found at Resources/Resource1/Properties/ServiceToken/Fn::ImportValue/Fn::Sub
local/issue/2587.yaml:7:11

E1019 Subs first element should be of type string for Resources/Resource1/Properties/Data/Fn::Sub
local/issue/2587.yaml:10:13

E1019 Parameter Stage1Stack for Fn::Sub not found at Resources/Resource1/Properties/Data/Fn::Sub/1/InstanceRole/Fn::ImportValue/Fn::Sub
local/issue/2587.yaml:24:17

@LaikaN57
Copy link
Author

@kddejong Any insights on the documentation discrepancy? (Not sure where/what the source of truth here is.)

@kddejong
Copy link
Contributor

@LaikaN57 I'll look into it

@kddejong
Copy link
Contributor

So talked to @TheDanBlanco and transforms are run before anything else is processed. So Transform can be any place in a template including in a sub. I think the documentation around what is supported in a Sub is meant to represent what is supported once all the Transforms are complete.

@LaikaN57
Copy link
Author

ok I think my doc fix request still stands in that case.

Thanks for the quick turn-around on this!

ref: awsdocs/aws-cloudformation-user-guide#1306

@kddejong
Copy link
Contributor

We will have this fix in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants