Setting external-name annotation

[crossplane]

Setting external-name annotation

[muvaf]

@turkenh Right now, we assume that .id is the field to use in state and it seems that's what's used for terraform import as well. The only thing missing is that Terraform does not guarantee that the field will be named as id so we might need to override that default. So, I guess this issue can be closed if we provide a way to override the path to ID field in state, right?

[muvaf]

Oh actually we don't optimize for resources that can be named, for example GCP Network. If users do give an external-name, we don't use that for creation of the resource from what I can tell, right? They have to give the name in spec.forProvider.name?

[muvaf]

I just tested that giving id attribute in the input HCL/JSON is not enough for Terraform to figure out the rest of the fields during the creation. The following JSON returns error saying name is required:

{
  "terraform": {
      "required_providers": {
          "google": {
              "source": "hashicorp/google",
              "version": "3.5.0"
            }
        }
    },
  "provider": {
    "google": [
      {
        "project": "some-project",
        "region": "us-central1",
        "zone": "us-central1-c"
      }
    ]
  },
  "resource": {
    "google_compute_network": {
      "vpc_network": {
        "id": "projects/some-project/global/networks/muvaf-testing-network"
      }
    }
  }
}

$ terraform apply
╷
│ Error: Missing required argument
│
│   on main.tf.json line 23, in resource.google_compute_network.vpc_network:
│   23:       }
│
│ The argument "name" is required, but no definition was found.

So, the notion of id attribute is limited to what's returned from the cloud and how to identify the resource uniquely; it isn't used to actually name the resource during the creation, which is the difference between our external name annotation.

Crossplane external name annotation tries to do two things with the same string; input for some of the spec fields and ability to uniquely identify the resource in the cloud. We're experiencing some pain in manually written resources as well because of this unification, for example in Azure unique identifier contains resource group name as well which exists under spec so importing resources is not as easy as just putting the external name annotation like Terraform, you have to fill required fields as well and producing the unique ID is manually implemented because all this doesn't map to provider API. Terraform treats ID as only the unique identifier for it to find the resource so you can actually run a single command to import, it'll put the string as id of the state and run the Read function; no custom mapping because that's what's used in non-import runs as well, similar to us. But it doesn't treat it as input for parameters of the resources that happen to be included in the unique identification string (not always the case), hence all input always from the resource block.

We can ask for a function from provider authors that can take the Terraform ID (what's used during terraform import) and return a base JSON, i.e. take projects/some-project/global/networks/muvaf-testing-network and return a JSON whose name is set, i.e. func(id string) map[string]interface{} that will be base of the JSON input of terraform apply. Additionally, we'll need to know which fields to omit from the schema, so likely a comment marker will be needed.

It's a manual process that's coupled with number of resources which is something we're actively trying not to have unless we really have to. Though there could be some heuristics specific to providers. For example, TF GCP provider has a parser here to be able to not ask the full identifier but only the name of the network. Some just don't do anything at all. In some cases, we might be able to reuse the same parser. But in all cases, provider authors need to make the choice one by one.

I tried to think of ways to avoid this, including my earlier proposal of removing external name annotation, but nothing really saves us from some kind of manual input here. For example, if we remove the external name annotation, then import would mean that we need to take the spec and produce the id that Terraform wants. It's better than the current situation but the cost of removal of external name annotation, even if we keep propagating metadata.name to spec.forProvider.name as suggested in that issue, is too high to be worth the difference in Terrajet. In other words, I'd still want us to rethink it for the reasons mentioned in that issue but it's not worth doing just for the sake of Terrajet providers if all others are equal. cc @negz

I'll start the implementation of exposing an interface for accepting that function and the comment marker.