-
Notifications
You must be signed in to change notification settings - Fork 68
Sample "x-l7xlb" causes an invalid key lookup #137
Description
Using the samples/x-l7xlb template, I tried to setup an Apigee environment in my GCP organization.
I used the sample settings in https://github.com/apigee/terraform-modules/blob/main/samples/x-l7xlb/x-demo.tfvars, only filling the necessary project variables to use my existing project.
Running terraform apply showed an error after creating the Apigee instance, saying:
Error: Invalid index
on main.tf line 75, in module "apigee-x-bridge-mig":
75: endpoint_ip = module.apigee-x-core.instance_endpoints[each.key]
├────────────────
│ each.key is "euw1-instance"
│ module.apigee-x-core.instance_endpoints is map of string with 1 element
The given key does not identify an element in this collection value.
Looking how the values flow and that the phrase "euw1-instance" is only mentioned as key in the apigee_instances map variable, this key is apparently lost in the process of creating the instance and is not a valid key in the map of instances eventually returned by apigee-x-core module.
Checking my GCP Console, I noticed that the instance is called instance-europe-west1. which is weird, since obviously, the instance is expected to be names like the key here. Digging into the code, I think I found a bug in where the name is retrieved from, after creating the instance.
- x-l7xlb#main.tf#L64 passes the
apigee_instancesobject right into the module, so the map isname => instance object - apigee-x-core's main.tf#L19 transforms this into a map of
region => instance object. Do note, that thisinstance objectdoes NOT transfer the key ofapigee_instancesinto anamefield, which becomes important, once the data is passed into theapigeemodule. local.instancesis passed into theapigeemodule here in line 83- Module
apigeemain.tf#L78 is a loop to create one Apigee instance per object in thevar.instancesretrieved from outside. Usingcoalesce()is checks whether theeach.valueobject has anameattribute and if not, assigns the default nameinstance-${each.key}, which causes the default nameinstance-europe-west1observed earlier. - Instance creation works fine nonetheless, since the module accounts for the lack of an explicit instance name - however, at this point, the instances all retrieved a standard name instead of the key value "euw1-instance" previously set in the configuration file, so these pieces of information diverged.
- output.tf of
apigee-x-corenow returns the instance objects (correctly containing the standard names in the instance objects) as output.
Bug happens now:x-l7xlb's main.tf looks like this:
module "apigee-x-bridge-mig" {
for_each = var.apigee_instances
source = "../../modules/apigee-x-bridge-mig"
project_id = module.project.project_id
network = module.vpc.network.id
subnet = module.vpc.subnet_self_links[local.subnet_region_name[each.value.region]]
region = each.value.region
endpoint_ip = module.apigee-x-core.instance_endpoints[each.key]
}It utilizes var.apigee_instances to perform a loop. The desired instance names (like "euw1-instance") are the keys and the instance object is the value. However, the endpoint_ip line now tries to retrieve the endpoint_ip of the actual instances, which were just created, by their name in the return value of module.apigee-x-core whose keys are filled with the instances' names - the standard names.
Since the desired instance names from the configuration file were omitted between the apigee-x-core and apigee modules, causing the names to be set by the apigee module itself, the keys in var.apigee_instances cannot be used to retrieve the actual instance objects from the map.
There are multiple ways to fix this.
for_eachusesmodule.apigee-x-core.instance_map(region =>instance object) as iterator. This changes thesubnetandregionlines, aseach.value.regionis now retrievable aseach.key(may also be contained in the instance object; didn't check). Also,endpoint_ipusesmodule.apigee-x-core.instance_endpoints[each.value.name]as index.- Make
apigee-x-corepass the keys of theapigee_instancesobject into the instance objects asnamefield. This should causeapigeeto properly recognize the explicit assignment of an instance name and prevent it from falling back to the default instance name. In addition, this prevents the desired instance name fromapigee_instances' keys and the actual instance names from diverging.
I prefer approach 1, since it sources the instance names from the module that creates the instances itself and returns their final names, instead of relying on the instances having a specific name from a config file key. It's just receiving the data directly from the source.
Edit: After testing this, I realized that terraform plan will fail, since the for_each argument is dynamic and cannot be determined at plan-time. So I prefer approach 2 of adding name = key in apigee-x-core#main.tf @ line 20 to pass the keys of apigee_instances as name field in the local.instances values.
Edit2: I also noticed that the output value instance_endpoints of apigee-x-core is documented to be a map of instance name => instance endpoint IP (see here). However, after applying solution 2 and using the hostname as a key, I still received the error saying that "the key doesn't identify an object in the map". Printing the object revealed that instance_endpoints is actually mapping region => instance endpoint IP, contrary to what the documentation says.
I stumbled upon this when testing for a project of mine, so I'll add a pull request, when I have the time and can properly test it. As I'm quite new to Terraform with GCP and this module family, I couldn't provide one along with this report.
If I overlooked something and caused the bug myself, I'm happy to be corrected :)
Edit3: Pull request added.