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

reference building doesn't handle multiple refs on the same block #99

Open
kapilt opened this issue Apr 2, 2023 · 3 comments
Open

reference building doesn't handle multiple refs on the same block #99

kapilt opened this issue Apr 2, 2023 · 3 comments

Comments

@kapilt
Copy link
Contributor

kapilt commented Apr 2, 2023

resource "aws_db_subnet_group" "subnetGroup" {
  name       = "main"
  subnet_ids = [aws_subnet.frontend.id, aws_subnet.backend.id]

  tags = {
    Name = "My DB subnet group"
  }
}

gets serialized out with a single __ref__ when there are in fact two refs.

                          'subnet_ids': {'__attributes__': ['aws_subnet.frontend.id',
                                                            'aws_subnet.backend.id'],
                                         '__name__': 'frontend',
                                         '__ref__': '3a23e2cd-b8ce-405f-87dc-2aca548ed72f',
                                         '__type__': 'aws_subnet'},
@kapilt
Copy link
Contributor Author

kapilt commented Apr 3, 2023

from tfdump it looks like the backend subnet is an unknown cty ref.

@ajkerrigan
Copy link
Member

I think we have the same root issue when multiple variable references appear in an interpolated string:

variable "subscription_id" {
  type    = string
  default = "00000000-0000-0000-0000-000000000000"
}

resource "azurerm_resource_group" "example" {
  name     = "example"
  location = "West Europe"
}

resource "azurerm_storage_account" "example" {
  name                     = "example"
  resource_group_name      = azurerm_resource_group.example.name
  location                 = azurerm_resource_group.example.location
  account_tier             = "Standard"
  account_replication_type = "GRS"
}

output "interpolated_id" {
  value = "/subscriptions/${var.subscription_id}/resourceGroups/${azurerm_resource_group.example.name}/providers/Microsoft.Storage/storageAccounts/${azurerm_storage_account.example.name}"
}

The interpolated_id output shows up like this in a dump (where "__ref__": "fb0d7911-9474-4667-acff-b0e640c010c6" points to the resource group):

    "output": [                                                                    
      {                              
        "__tfmeta": {                                                              
          "filename": "main.tf",
          "label": "interpolated_id",                                              
          "line_end": 21,                                                          
          "line_start": 19,                                                        
          "path": "output.interpolated_id"                                         
        },                      
        "id": "2037ef97-ad12-42c7-a303-26025c49fbc8",                              
        "value": {                                                                 
          "__attributes__": [                                                      
            "var.subscription_id",                                                 
            "azurerm_resource_group.example.name",
            "azurerm_storage_account.example.name"                                 
          ],                                                                       
          "__name__": "example",
          "__ref__": "fb0d7911-9474-4667-acff-b0e640c010c6",                       
          "__type__": "azurerm_resource_group"
        }
      }
    ]

@ajkerrigan
Copy link
Member

ajkerrigan commented Oct 9, 2024

"What do we expect to see in the resource graph?" is a fair question. Given a module like the following...

`main.tf` source
provider "azurerm" {
  features {}
  subscription_id = "11111111-1111-1111-1111-111111111111"
  resource_provider_registrations = "none"
}

data "azurerm_subscription" "current" {
}

locals {
  shortname = "example"
  single_ref = azurerm_storage_account.example.id
  multi_ref = "/subscriptions/${data.azurerm_subscription.current.id}/resourceGroups/${azurerm_resource_group.example.name}/providers/Microsoft.Storage/storageAccounts/${azurerm_storage_account.example.name}"
}

resource "azurerm_resource_group" "example" {
  name     = "example"
  location = "East US"
}

data "azurerm_subnet" "example" {
  name                 = "example"
  virtual_network_name = "example"
  resource_group_name  = azurerm_resource_group.example.name
}

resource "azurerm_storage_account" "example" {
  name                            = "${azurerm_resource_group.example.name}storage"
  resource_group_name             = azurerm_resource_group.example.name
  location                        = azurerm_resource_group.example.location
  account_tier                    = "Standard"
  account_replication_type        = "LRS"
  public_network_access_enabled   = "false"
  allow_nested_items_to_be_public = "false"
}

Here's what the c7n-left dump output looks like today:

locals in tfparse 0.6.11
[
    {
        "__tfmeta": { "filename": "main.tf", "line_end": 14, "line_start": 10, "path": "locals" },
        "id": "4c5d137e-dfd2-4863-a064-60b6b36eae97",
        "multi_ref":
            {
                "__attributes__":
                    [
                        "data.azurerm_subscription.current.id",
                        "azurerm_resource_group.example.name",
                        "azurerm_storage_account.example.name",
                    ],
                "__name__": "current",
                "__ref__": "22fc31f5-41f1-422f-a1bb-108351c38980",
                "__type__": "azurerm_subscription",
            },
        "shortname": "example",
        "single_ref":
            {
                "__attribute__": "azurerm_storage_account.example.id",
                "__name__": "example",
                "__ref__": "4a338af7-a870-4ad9-8ff4-19cd9ce9f197",
                "__type__": "azurerm_storage_account",
            },
    },
]

We could pluralize __name__/__ref__/__type__ whenever we pluralize __attribute__...

locals with name/ref/type pluralized as needed
[
    {
        "__tfmeta": {
            "filename": "main.tf",
            "line_end": 14,
            "line_start": 10,
            "path": "locals"
        },
        "id": "4c5d137e-dfd2-4863-a064-60b6b36eae97",
        "multi_ref": {
            "__attributes__": [
                "data.azurerm_subscription.current.id",
                "azurerm_resource_group.example.name",
                "azurerm_storage_account.example.name",
            ],
            "__names__": [
                "current",
                "example",
                "example",
            ],
            "__refs__": [
                "22fc31f5-41f1-422f-a1bb-108351c38980",
                "fcc2cbc6-b893-4213-82b8-9c1a9cad9c3b",
                "4a338af7-a870-4ad9-8ff4-19cd9ce9f197",
            ],
            "__types__": [
                "azurerm_subscription",
                "azurerm_resource_group",
                "azurerm_storage_account",
            ]
        },
        "shortname": "example",
        "single_ref": {
            "__attribute__": "azurerm_storage_account.example.id",
            "__name__": "example",
            "__ref__": "4a338af7-a870-4ad9-8ff4-19cd9ce9f197",
            "__type__": "azurerm_storage_account",
        },
    },
]

Or perhaps tuck always-singular reference structures inside a top-level key like __refs__?

locals with an array of reference objects
[
    {
        "__tfmeta": {
            "filename": "main.tf",
            "line_end": 14,
            "line_start": 10,
            "path": "locals"
        },
        "id": "4c5d137e-dfd2-4863-a064-60b6b36eae97",
        "multi_ref": {
            "__refs__": [
                {
                    "__attribute__": "data.azurerm_subscription.current.id",
                    "__name__": "current",
                    "__ref__": "22fc31f5-41f1-422f-a1bb-108351c38980",
                    "__type__": "azurerm_subscription",
                },
                {
                    "__attribute__": "azurerm_resource_group.example.name",
                    "__name__": "example",
                    "__ref__": "fcc2cbc6-b893-4213-82b8-9c1a9cad9c3b",
                    "__type__": "azurerm_resource_group",
                },
                {
                    "__attribute__": "azurerm_storage_account.example.name",
                    "__name__": "example",
                    "__ref__": "4a338af7-a870-4ad9-8ff4-19cd9ce9f197",
                    "__type__": "azurerm_storage_account",
                },
            ]
        },
        "shortname": "example",
        "single_ref": {
            "__refs__": [
                {
                    "__attribute__": "azurerm_storage_account.example.id",
                    "__name__": "example",
                    "__ref__": "4a338af7-a870-4ad9-8ff4-19cd9ce9f197",
                    "__type__": "azurerm_storage_account",
                }
            ]
        },
    },
]

From an aesthetic/consistency perspective I like the idea of having an array of refs, but I'm not sure what makes the most sense from the implementation side and downstream effects on the c7n-left traverse filter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants