hashicorp / terraform-config-inspect

A helper library for shallow inspection of Terraform configurations
Mozilla Public License 2.0
383 stars 76 forks source link

Panic when calling LoadModuleFromFile #61

Open zzxwill opened 3 years ago

zzxwill commented 3 years ago

Terraform Configuration rds.tf is as below.

module "rds" {
  source = "terraform-alicloud-modules/rds/alicloud"
  engine = "MySQL"
  engine_version = "8.0"
  instance_type = "rds.mysql.c1.large"
  instance_storage = "20"
  instance_name = var.instance_name
  account_name = var.account_name
  password = var.password
}

output "DB_NAME" {
  value = module.rds.this_db_instance_name
}
output "DB_USER" {
  value = module.rds.this_db_database_account
}
output "DB_PORT" {
  value = module.rds.this_db_instance_port
}
output "DB_HOST" {
  value = module.rds.this_db_instance_connection_string
}
output "DB_PASSWORD" {
  value = module.rds.this_db_instance_port
}

variable "instance_name" {
  default = "poc"
  type = string
}

variable "account_name" {
  default = "oam"
}

variable "password" {
  default = "Xyfff83jfewGGfaked"
}

Try to get all variable information but panic happended.

func main()  {
    p := hclparse.NewParser()
    hclFile, _ := p.ParseHCLFile("./rds.tf")
    mod := tfconfig.Module{Variables: map[string]*tfconfig.Variable{}}
    tfconfig.LoadModuleFromFile(hclFile, &mod)
    for _, v := range mod.Variables {
        fmt.Println(v.Name, v.Type)
    }
}

It seems that all fields of tfconfig.Module has to be set even though I just variable from it, or panic will happen.

panic: assignment to entry in nil map

goroutine 1 [running]:
github.com/hashicorp/terraform-config-inspect/tfconfig.LoadModuleFromFile(0xc00010bd00, 0xc0001b7f00, 0x0, 0x0, 0x0)
    /Users/zhouzhengxi/go/pkg/mod/github.com/hashicorp/terraform-config-inspect@v0.0.0-20210318070130-9a80970d6b34/tfconfig/load_hcl.go:336 +0x1d12
main.main()
    /Users/zhouzhengxi/Programming/golang/src/github.com/zzxwill/try-cloudnative/homework/hcl.go:66 +0x246
radeksimko commented 3 years ago

If you replace this

mod := tfconfig.Module{Variables: map[string]*tfconfig.Variable{}}

with

mod := tfconfig.NewModule()

then your example should not panic.

zzxwill commented 3 years ago

@radeksimko Thanks for the reply. But there is no path for .tf files as the configuration file is from an API and I don't want to write it to a local path in order to speed the process.

I think nil checking won't do anything bad in the PR, right?

func NewModule(path string) *Module {
radeksimko commented 3 years ago

I don't know how does your API work, but I believe that LoadModuleFromFilesystem(fs, dir) is the function you are looking for.

Assuming that you can expose the directory through that interface you should be able to make it work:

https://github.com/hashicorp/terraform-config-inspect/blob/9a80970d6b348d0c23719ed60bccb83d4bfc2ccd/tfconfig/filesystem.go#L8-L22

The FS interface is basically a superset of the Go 1.16 io/fs.FS and File is an exact copy of io/fs.File. This is partially because these interfaces were introduced prior to Go 1.16 being released.

We use that exact function inside Terraform LS where files (and their content) also come via API.

I think nil checking won't do anything bad in the PR, right?

AFAICT with the break statements as-is it would effectively prevent parsing anything when the Module has uninitialised fields - which - again should not happen as long as you use that constructor. I admit that panic is not ideal, but IMO it's a better side effect than silently not parsing anything.

In Go constructors usually follow a particular naming convention - so I'd expect people to look for New* functions before attempting to create the struct on their own and running into similar problems. I'm not aware of any decent/easy way of communicating that convention however.