Cfn-python-lint: I1022 Subject to false positives when using intrinstics in Fn::Join

[trav-c]

*cfn-lint version: 0.25.0

Description of issue.
The recently introduced I1022 which flags the use of Fn::Join with empty delimiter flags use cases which are not necessarily sensible to replace with Fn:Sub.

For example parts of a join can be conditional values, or other intrinsic functions like an Fn::Select on an Fn::Split.

I would suggest that the rule should not trigger if the Fn:Join arguments include any intrinstics other than Fn:Ref or Fn:GetAtt

Here is a simple standalone stack that somewhat questionably triggers I1022, I say questionable in this case because you could argue that it _might_ be clearer to use a conditional outside of two Fn:Sub's

AWSTemplateFormatVersion: 2010-09-09
Description: "cfn-lint I1022 false positive"

Conditions:
    Cond1: !Equals [ !Ref 'AWS::Region', 'us-east-1' ]

Resources:

    # Simple Conditional In Join with empty delimiter
    Bucket:
        Type: AWS::S3::Bucket
        Properties:
            Tags:
              - Key: Foo
                Value: !Join [ '', [
                     'Prefix/',
                     !If [ Cond1, 'Infix-', '' ],
                     '/Suffix',
                ]]

Here is a more complex real-world example of a situation that would probably very difficult to sensibly convert o Fn:::Sub but triggers I1022

!Join [ '', [ 'ecs-alb-', !Select [ 2, !Split [ '/', !GetAtt ECSALB.LoadBalancerFullName ] ], '.', !Ref ServicesDomainSuffix, '.' ] ]

[kddejong]

I always appreciate another set of eyes. @PatMyron opinions here?

I think the preferred way of doing this would be. Since sub will automatically do the Ref/GetAtt for you the only thing you would need to define is the Select sub parameter.

Fn::Sub:
  - ecs-alb-${albName}.${ServicesDomainSuffix}.
  - albName:  !Select [ 2, !Split [ '/', !GetAtt ECSALB.LoadBalancerFullName ] ]

[trav-c]

Ah, I hadn't actually realised that Fn::Sub supported variable maps, I'm inclined to agree that the Sub option is probably clearer / more readable here given that.

The only potential confusion issue I can see with it is that I don't see any information on the Fn::Sub documentation as to whether mapped variables take precedence (or even triggers an error) if the name conflicts with a !Ref name eg:

Parameters:
    Foo:
        Type: String
        Default: Bar
   NotFoo:
        Type: String
        Default: NotBar
Resources:
...
        Fn::Sub:
            - Replace-${Foo}
            - Foo: !Ref NotFoo

I don't think that impacts which option is preferable too much though.

[kddejong]

Good question. I may have to do some testing on that.

I've always wondered the same thing about naming a resource and parameter by the same name. What is used when you do a Ref to that similar name.

[trav-c]

While addressing the I1022 warnings in our stack I came across an example, which may be a false positive, basically it uses !Sub inside a !Base64, I'm not sure if this is permitted by !Sub I can see that !Sub is not listed in the permitted intrinsics for !Sub variable maps:

ViewOriginalText: !Join
  - ''
  - - '/*Presto view: "
    - "Fn::Base64":
        "Fn::Sub": '
           <Long multiline Text of AWS::Glue::Table View with ${Var} replacents>
        '
    - ' */'

I have yet to experiment with converting this to a !Sub, but the docs suggest that !Sub can't be nested within the variable map (at least it's not listed in the supported functions for VarName / VarValue), I'm not sure if the fact that it's wrapped in Fn::Base64 changes that though

[kddejong]

I believe this will also work.

ViewOriginalText:
  Fn::Sub:
    - "/*Presto view: ${var}*/"
    - var:
         Fn::Base64: !Sub "<Long multiline Text of AWS::Glue::Table View with ${Var} replacents>"

I didn't test with Glue but with Tags just to get a string out.

[trav-c]

While I haven't tested the above AWS::Glue::Table in situ yet, I'm going to close this ticket, since there are as yet no examples of FN:Join's with empty delimiters that can't at least as neatly, if not more so be expressed using Fn::Sub.