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

TypeError in overrides.py with ResourceGroup #290

Closed
eightnoneone opened this issue Apr 26, 2024 · 7 comments
Closed

TypeError in overrides.py with ResourceGroup #290

eightnoneone opened this issue Apr 26, 2024 · 7 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@eightnoneone
Copy link

I'm Doing a test conversion on a CF template produced by the OpsWorks-to-SystemsManager script that AWS provides.

The ResourceGroup is:

  ResourceGroup:
    Properties:
      Description: Resource group for all resources deployed by CFN stack.
      Name:
        Ref: AWS::StackName
      ResourceQuery:
        Query:
          TagFilters:
          - Key: ComponentName
            Values:
            - Ref: AWS::StackName
        Type: TAG_FILTERS_1_0
      Tags:
      - Fn::If:
        - IsApplicationNameProvided
        - Key: ApplicationName
          Value:
            Ref: ApplicationName
        - Ref: AWS::NoValue
    Type: AWS::ResourceGroups::Group

Getting this TypeError:

debug: Converting Cloudformation resource ResourceGroup to Terraform.
debug: Converted name to resource_group
debug: Converted CF type AWS::ResourceGroups::Group to search term resource groups group.
debug: Searcing for resource groups group in terraform docs...
debug: Best match was inspector resource group at /var/folders/p1/5g9g_2kx5gv8hk4nhkr0b4400000gp/T/terraform_src/website/docs/r/inspector_resource_group.html.markdown with score of 71.
debug: Found documentation file /var/folders/p1/5g9g_2kx5gv8hk4nhkr0b4400000gp/T/terraform_src/website/docs/r/inspector_resource_group.html.markdown
debug: Converted type from AWS::ResourceGroups::Group to aws_inspector_resource_group
debug: We have ran out out attributes to parse
debug: Parsed the following arguments from the documentation: 
debug: ['tags']
debug: Parsed the following attributes from the documentation: 
debug: ['arn']
debug: Converting the intrinsic functions to Terraform expressions...
debug: Overiding Properties
debug: Overiding params for {tf_type}
debug: Performing global overrides for {tf_type}
Traceback (most recent call last):
  File "/opt/homebrew/bin/cf2tf", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/cf2tf/app.py", line 44, in cli
    config = TemplateConverter(tmpl_path.stem, cf_template, search_manger).convert()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/cf2tf/convert.py", line 97, in convert
    tf_resources = self.convert_to_tf(self.manifest)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/cf2tf/convert.py", line 144, in convert_to_tf
    tf_resources.extend(converter(resources))
                        ^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/cf2tf/convert.py", line 352, in convert_resources
    overrided_values = perform_global_overrides(
                       ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/cf2tf/convert.py", line 645, in perform_global_overrides
    params = override(tc, params)
             ^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/cf2tf/0.7.1/libexec/lib/python3.12/site-packages/cf2tf/conversion/overrides.py", line 55, in tag_conversion
    new_tags = {LiteralType(tag["Key"]): tag["Value"] for tag in orginal_tags}
                            ~~~^^^^^^^
TypeError: string indices must be integers, not 'str'
@shadycuz
Copy link
Member

Thanks for reporting this issue @eightnoneone

I think I understand what is happening here. Cloudformation tags are normally in list like this:

Tags:
  - Key: Foo
    Value: Bar

We have to convert these into the form Terraform uses:

tags = {
    Foo = "Bar"
}

The problem here is that we are attempting to do the conversion but the Fn::If: seems to be messing it up. Which makes sense. There isn't really a way to convert this properly. The actual terraform code should look something like:

tags = locals.IsApplicationNameProvided == true ? { Key = "ApplicationName", Value = aws_some_resouce.ApplicationName.arn } : {}

Let me do some digging into the code and see if I can come up with a fix. At the very least, we shouldn't stop the conversion for every error we encounter.

@shadycuz shadycuz added bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed labels Apr 27, 2024
@eightnoneone
Copy link
Author

I'm also seeing what looks like a repeating problem in dealing with this overly complicated CF that the stack_exporter.py migration produces. There's lots of this ] : [pattern in the commented CF fields. This seems to then mess with the structure of the output. I'll get things like an ALB with no required load_balancer_port defined.

I know it's a weird case, but with the May 26 2024 deadline for OpsWorks sunset, you might be getting more and more users trying what I'm trying (Opsworks->CF->TF). I appreciate the tool and I can easily edit around the output it's produced. Just a "head's up."

For an ASG in the CF template:

  ApplicationAsg:
    DependsOn: CustomTerminationLambdaPermission
    Properties:
      HealthCheckGracePeriod: 30
      HealthCheckType:
        Fn::If:
        - NoLBResources
        - EC2
        - ELB
      LaunchTemplate:
        LaunchTemplateId:
          Ref: LaunchTemplateId
        Version:
          Ref: LaunchTemplateVersion
      LoadBalancerNames:
        Fn::If:
        - CreateClassicELBResources
        - - Ref: ClassicELB
        - - Ref: AWS::NoValue
      MaxSize: '0'
      MinSize: '0'
      ServiceLinkedRoleARN:
        Fn::Sub: arn:${AWS::Partition}:iam::${AWS::AccountId}:role/aws-service-role/autoscaling.amazonaws.com/${ASGServiceLinkedRole}
      Tags:
      - Key: ComponentName
        PropagateAtLaunch: true
        Value:
          Ref: AWS::StackName
      TargetGroupARNs:
        Fn::If:
        - CreateALBResources
        - - Ref: ALBTargetGroup1
        - - Ref: AWS::NoValue
      TerminationPolicies:
        Fn::If:
        - UseASGCustomTerminationPolicy
        - - Fn::GetAtt:
            - CustomTerminationLambda
            - Arn
          - Default
        - - Default
      VPCZoneIdentifier:
        Ref: SubnetIds
    Type: AWS::AutoScaling::AutoScalingGroup

Resulting TF:

resource "aws_autoscalingplans_scaling_plan" "application_asg" {
  // CF Property(HealthCheckGracePeriod) = 30
  predefined_load_metric_type = local.NoLBResources ? "EC2" : "ELB"
  // CF Property(LaunchTemplate) = {
  //   LaunchTemplateId = var.launch_template_id
  //   Version = var.launch_template_version
  // }
  name = local.CreateClassicELBResources ? [
  aws_load_balancer_listener_policy.classic_elb[0].id
  max_capacity = "0"
  min_capacity = "0"
  // CF Property(ServiceLinkedRoleARN) = "arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role/aws-service-role/autoscaling.amazonaws.com/${aws_iam_service_linked_role.asg_service_linked_role.id}"
  target_tracking_configuration = local.CreateALBResources ? [
  aws_lb_target_group_attachment.alb_target_group1[0].id
  // CF Property(TerminationPolicies) = local.UseASGCustomTerminationPolicy ? [
  //   aws_lambda_function.custom_termination_lambda.arn,
  //   "Default"
  // ] : [
  //   "Default"
  // ]
  // CF Property(VPCZoneIdentifier) = var.subnet_ids
  // CF Property(tags) = {
  //   ComponentName = local.stack_name
  // }
}

@shadycuz
Copy link
Member

shadycuz commented May 1, 2024

@eightnoneone I thought I had responded but I guess I forgot to hit send.

When you have a commented-out property like // CF Property(VPCZoneIdentifier, it's because cf2tf could not find a Terraform attribute (property) that matched.

In your case, you have so many because cf2tf converted AWS::AutoScaling::AutoScalingGroup to aws_autoscalingplans_scaling_plan, which is similar but not the correct resource. It should have converted it to aws_autoscaling_group.

We use thefuzz to help us do fuzzy string matching, but obviously, our searching could be a lot better. There is also an "overrides" system. This is the same system that is breaking the ability to convert tags from one format to another 🙃. When needed, we can use an override to force a better match on a resource.

I just opened my laptop to take a look at this, but I have to do updates first =/

@shadycuz
Copy link
Member

shadycuz commented May 1, 2024

Here it is with debug logging:

debug: Converted CF type AWS::AutoScaling::AutoScalingGroup to search term auto scaling auto scaling group.
debug: Searcing for auto scaling auto scaling group in terraform docs...
debug: Best match was autoscalingplans scaling plan at /tmp/terraform_src/website/docs/r/autoscalingplans_scaling_plan.html.markdown with score of 63.

I think it didn't like the camel case split. When I remove it we get better results:

debug: Searcing for autoscaling autoscalinggroup in terraform docs...
debug: Best match was autoscaling group at /tmp/terraform_src/website/docs/r/autoscaling_group.html.markdown with score of 76.
debug: Found documentation file /tmp/terraform_src/website/docs/r/autoscaling_group.html.markdown
debug: Converted type from AWS::AutoScaling::AutoScalingGroup to aws_autoscaling_group

But there seems to be some additional problems...

debug: Unable to find items in section Argument Reference of /tmp/terraform_src/website/docs/r/autoscaling_group.html.markdown
debug: Unable to find items in section Attribute Reference of /tmp/terraform_src/website/docs/r/autoscaling_group.html.markdown
debug: Parsed the following arguments from the documentation: 
debug: []
debug: Parsed the following attributes from the documentation: 
debug: []

@shadycuz
Copy link
Member

shadycuz commented May 1, 2024

Now we are getting somewhere

debug: Parsed the following arguments from the documentation: 
debug: ['name', 'name_prefix', 'max_size', 'min_size', 'availability_zones', 'capacity_rebalance', 'context', 'default_cooldown', 'default_instance_warmup', 'launch_configuration', 'launch_template', 'mixed_instances_policy', 'ignore_failed_scaling_activities', 'initial_lifecycle_hook', 'health_check_grace_period', 'health_check_type', 'instance_maintenance_policy', 'desired_capacity', 'desired_capacity_type', 'force_delete', 'load_balancers', 'traffic_source', 'vpc_zone_identifier', 'target_group_arns', 'termination_policies', 'suspended_processes', 'tag', 'placement_group', 'metrics_granularity', 'enabled_metrics', 'wait_for_capacity_timeout', 'min_elb_capacity', 'wait_for_elb_capacity', 'protect_from_scale_in', 'service_linked_role_arn', 'max_instance_lifetime', 'instance_refresh', 'warm_pool', 'force_delete_warm_pool']
debug: Parsed the following attributes from the documentation: 
debug: ['id', 'arn', 'availability_zones', 'min_size', 'max_size', 'default_cooldown', 'default_instance_warmup', 'name', 'health_check_grace_period', 'health_check_type', 'desired_capacity', 'launch_configuration', 'predicted_capacity', 'vpc_zone_identifier', 'warm_pool_size']

This looks better:

data "aws_caller_identity" "current" {}

data "aws_partition" "current" {}

resource "aws_autoscaling_group" "application_asg" {
  health_check_grace_period = 30
  health_check_type = "ELB"
  launch_template {
    // CF Property(LaunchTemplateId) = "LaunchTemplateId"
    version = "LaunchTemplateVersion"
  }
  name = "ClassicELB"
  max_size = "0"
  min_size = "0"
  service_linked_role_arn = "arn:${data.aws_partition.current.partition}:iam::${data.aws_caller_identity.current.account_id}:role/aws-service-role/autoscaling.amazonaws.com/foo"
  target_group_arns = "ALBTargetGroup1"
  termination_policies = "Default"
  vpc_zone_identifier = "Subnet1,Subnet2"
}

but it's not perfect:

debug: Searching for Load Balancer Names instead of LoadBalancerNames
debug: Converted LoadBalancerNames to name with 90% match.

That seems right but this:

debug: Searching for Launch Template Id instead of LaunchTemplateId
debug: No match found for LaunchTemplateId, commenting out this argument.

Seems like it should have matched =/

@shadycuz
Copy link
Member

shadycuz commented May 1, 2024

@eightnoneone Can you install the latest "pre-release" version and confirm that things are better?

pip install -U cf2tf --pre

Successfully installed cf2tf-0.8.0a0

@shadycuz
Copy link
Member

shadycuz commented May 2, 2024

Closing this now that v0.8.0 is released. Feel free to reopen this issue or create new ones =)

Thansk!

@shadycuz shadycuz closed this as completed May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants