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

fix(ec2): 'encoded list token' error using Vpc imported from deploy-time lists #12040

Merged
merged 4 commits into from
Dec 14, 2020

Commits on Dec 9, 2020

  1. fix(ec2): Vpcs can't be imported from Token arrays

    Adding a test.
    rix0rrr committed Dec 9, 2020
    Configuration menu
    Copy the full SHA
    fbd9063 View commit details
    Browse the repository at this point in the history

Commits on Dec 12, 2020

  1. fix(ec2): 'encoded list token' error using Vpc imported from deploy-t…

    …ime lists
    
    Even though using `Vpc.fromVpcAttributes()` using deploy-time lists like
    from `Fn.importValue()`s and `CfnParameters` was never really supposed
    to work, it accidentally did.
    
    The reason for that is:
    
    ```ts
    
    // Encoded list token
    const subnetIds = Token.asList(Fn.importValue('someValue'))
    // [ '#{Token[1234]}' ]
    
    // Gets parsed to a singleton `Subnet` list:
    const subnets = subnetIds.map(s => Subnet.fromSubnetAttributes({ subnetId: s }));
    // [ Subnet({ subnetId: '#{Token[1234]}' }) ]
    
    // This 'subnetId' is illegal by itself, and if yould try to use it for,
    // say, an ec2.Instance it would fail. However, if you treat this single
    // subnet as a GROUP of subnets:
    new CfnAutoScalingGroup({ subnetIds: subnets.map(s => s.subnetId) })
    // [ '#{Token[1234]}' ]
    
    // And this resolves back to:
    resolve(cfnSubnetIds)
    // SubnetIds: { Fn::ImportValue: 'someValue' }
    ```
    
    --------
    
    We introduced an additional check in #11899 to make sure that the
    list-element token that represents an encoded list (`'#{Token[1234]}'`)
    never occurs in a non-list context, because it's illegal there.
    
    However, because:
    
    * `Subnet.fromSubnetAttributes()` logs the subnetId as a *warning*
      to its own metadata (which will log a string like `"there's something
      wrong with '#{Token[1234]}' ..."`).
    * All metadata is resolved just the same as the template expressions
      are.
    
    The `resolve()` function encounters that orphaned list token in the
    metadata and throws.
    
    --------
    
    The *proper* solution would be to handle unparseable list tokens
    specially to never get into this situation, but doing that requires
    introducing classes and new concepts that will be a large effort and
    not be backwards compatible. Tracking in #4118.
    
    Another possible solution is to stop resolving metadata. I don't
    know if we usefully use this feature; I think we don't. However,
    we have tests enforcing that it is being done, and I'm scared
    to break something there.
    
    The quick solution around this for now is to have
    `Subnet.fromSubnetAttributes()` recognize when it's about to log
    a problematic identifier to metadata, and don't do it.
    
    Fixes #11945.
    rix0rrr committed Dec 12, 2020
    Configuration menu
    Copy the full SHA
    e5a9e88 View commit details
    Browse the repository at this point in the history
  2. Fix test

    rix0rrr committed Dec 12, 2020
    Configuration menu
    Copy the full SHA
    55db5ed View commit details
    Browse the repository at this point in the history

Commits on Dec 14, 2020

  1. Configuration menu
    Copy the full SHA
    e1b27b4 View commit details
    Browse the repository at this point in the history