Support for multiple providers of the same type

[mgood]

This is incomplete, but I'm submitting to start getting feedback. This will enable support for #451.

Adds an "alias" field to the provider which allows creating multiple instances
of a provider under different names. This provides support for configurations
such as multiple AWS providers for different regions. In each resource, the
provider can be set with the "provider" field.

Known issues:

• input prompts for missing config keys aren't being propagated, so you still get a key error during validation (opened a new issue since it turned out to be broken in master: User input variables ignored #1282)
• needs more tests

Tentatively, the config syntax looks like this:

provider "aws" {
    alias = "asia"
    region = "ap-northeast-1"
}

provider "aws" {
    alias = "us"
    region = "us-west-1"
}

resource "aws_instance" "foo" {
    provider = "aws.asia"
}

resource "aws_instance" "bar" {
    provider = "aws.us"
}

(thanks to Cisco Cloud for sponsoring development)

[mgood]

It looks like the issue with user input I mentioned under "known issues" already existed on the current master, so I've opened issue #1282.

[bitglue]

I think it would be a good exercise to see what this would look like in the documentation. Currently, it says:

Every resource in Terraform is mapped to a provider based on longest-prefix matching. For example the aws_instance resource type would map to the aws provider (if that exists).

I'm not sure this is actually true, since providers map whole resource names:

        ResourcesMap: map[string]*schema.Resource{
            "aws_autoscaling_group":            resourceAwsAutoscalingGroup(),
            "aws_db_instance":                  resourceAwsDbInstance(),
            "aws_db_parameter_group":           resourceAwsDbParameterGroup(),
            "aws_db_security_group":            resourceAwsDbSecurityGroup(),
            // [...]
        },

Anyhow, how does the documentation get amended to describe the proposed behavior?

...unless the resource has a provider attribute, in which case (something totally different happens)

I think it's difficult to describe. Especially the edge cases. For example, what's this do?

resource "google_compute_instance" "bar" {
    provider = "aws.us"
}

I'd also say there's already a syntax for naming things, the "bar" in:

resource "aws_instance" "bar" {
    ...
}

so if we need a way to name providers, why not one of these:

provider "aws" "west" {
    region = "us-west-1"
}

# option 1
resource "west_instance" "bar" {
    ...
}

# option 2
resource "aws.west_instance" "bar" {
    ...
}

# option 3
resource "aws.west.instance" "bar" {
    ...
}

# option 4
resource "aws_west_instance" "bar" {
    ...
}

There is a bit of inconsistency in option 1. Consider, an aws_instance named bar does not conflict with a google_compute_instance also named bar: each resource type has its own namespace as in ${aws_instance.bar.id}.

Option 2 is a bit weird since a . serves as a delimiter between the provider type and the name, while a _ serves as a delimiter between the provider and the instance type. I've never tried it, but based on the semantics described in the documentation the _ isn't even really necessary, so aws.westinstance should work also. That's just plain weird.

Option 3 resolves this inconsistency by always using . as a delimiter. But that probably messes up the variable interpolation syntax, because now you need ${aws.west.instance.bar.id}. It might take some hackery to implement this in a backwards compatible way, but at least it's consistent, both with itself and most other languages that use . for attribute reference and whatnot. This is my favorite option, and I'd rather see a clean break in compatibility than an inconsistent syntax. I imagine I could patch all my configurations with sed for the new syntax in under 15 minutes.

Option 4 resolves the ambiguity by always using _ as a delimiter. However there's still some internal inconsistency since . is used as a delimiter in variable interpolations.

[mgood]

Thanks for the feedback.

Every resource in Terraform is mapped to a provider based on longest-prefix matching. For example the aws_instance resource type would map to the aws provider (if that exists).

Yeah, as you said, that description doesn't seem to match what I'm seeing in the code. In terraform/util.go resourceProvider() it splits on the first underscore, and uses the prefix as the provider.

why not one of these:
provider "aws" "west" {

Yes, I would actually prefer that syntax for consistency with the configuration for resource names. This was in fact the first format suggested by @phinze on IRC, but I haven't found a practical way to support it.

The problem is that Terraform first parses the config into simple data-structures where these two formats are equivalent nested maps:

provider "aws" "west" {
  foo = "bar"
}
provider "aws" {
  west = { foo = "bar" }
}

For resources, the name is required, so it always recurses one level to extract the name, but for providers it would be optional. Since those structures are equivalent, there's no straightforward way to distinguish when the key is intended as a name for the provider, or part of its config. I'll keep thinking about that one, since I agree it would be preferable.

As for changing the resource type name to map it to a provider instance, that's an interesting suggestion, though the current use of the provider key is based on input from @phinze and @mitchellh. This syntax seemed fairly consistent with how depends_on works.

For example, what's this do?

resource "google_compute_instance" "bar" {
    provider = "aws.us"
}

This gives a validation error, since the "aws" provider doesn't support that type of resource:

  * google_compute_instance.bar: Provider doesn't support resource: google_compute_instance

[bitglue]

Since those structures are equivalent, there's no straightforward way to distinguish when the key is intended as a name for the provider, or part of its config.

You could probably make a pretty good heuristic guess. I'd wager that there are no providers where all their configuration keys are maps (if there are any maps at all), so if you are looking at the root object, inspect the keys. If all of them are maps, then guess that the keys are names of providers to instantiate, using the new proposed syntax.

Or, if some of the values are not maps, then emit a deprecation warning and assume it's using the old syntax. Instantiate just one provider using the provider type as the name.

You can make an even better guess if you know the schema of the provider, but I'm guessing that may not be easily accomplished.

This gives a validation error, since the "aws" provider doesn't support that type of resource:

I was sure it did something, and this was one of my guesses. The point is that the semantics are complicated and hard to describe, and while you could elaborate on them in the documentation, people probably won't read it.

[mgood]

@bitglue yes, the built-in providers do not use nested maps at this time, but it's still valid for a provider's schema to do that. There might be third-party plugins that do, or this might be useful in the future, so I didn't want to make those assumptions about the schema. Using the schema validation to make a better guess does seem like a good idea, but I think would require some more complicated restructuring of the config processing, since the alias is needed a few steps before validation takes place.

@phinze or @mitchellh I'd love to hear your thoughts on the "alias" field, or other ways to specify that in the configuration.

[bitglue]

I just came across another use case in my configurations that's probably relevant here. In a few places, I create IAM policies. IAM policies frequently need to contain the region and the account number in ARNs.

Currently I just paste these values into the policy, but that makes my configuration quite unportable. I could also push them up to variables the user must provide, but it seems undesirable to burden the user with providing more information that could be calculate from information they've already provided. (The account number can be found, given an AWS API key).

Each of these things has the same scope as an AWS connection, represented by an AWS provider. It would be very elegant if I could interpolate these values by referencing a provider instance, much as variables from resource instances can be interpolated now.

Another reason, I think, to make provider syntax identical to resource syntax.

[mgood]

I just updated the PR with some unit tests and an AWS acceptance test.

@bitglue do you have an example of what you're doing? It sounds like you're asking for the interpolation syntax to support referencing provider attributes, which sounds useful, but beyond the scope of this feature.

[mgood]

@bitglue oh BTW, I talked with @phinze on IRC yesterday. He mentioned that they have some plans in mind to improve the config parser to be able to support the provider "aws" "west" { syntax in the future, but that he thought it would be ok to proceed with this syntax in order to get this feature included.

[bitglue]

Here's what I'm doing specifically:

resource "aws_iam_user_policy" "lb_ro" {
    name = "test"
    user = "${aws_iam_user.lb.name}"
    # TODO: the ARN in this policy has an account number and region embedded in it.
    policy = <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": [
        "elasticloadbalancing:RegisterInstancesWithLoadBalancer",
        "elasticloadbalancing:DeregisterInstancesFromLoadBalancer"
      ],
      "Effect": "Allow",
      "Resource": "arn:aws:elasticloadbalancing:us-east-1:1234567890:loadbalancer/${aws_elb.www.name}"
    }
  ]
}
EOF
}

It would much better if I could have

provider "aws" "main" {
    region = "us-east-1"
}

and then later:

      "Resource": "arn:aws:elasticloadbalancing:${aws.main.region}:${aws.main.account_number}:loadbalancer/${aws_elb.www.name}"

account_number would be a calculated attribute of the provider, just as some resources have calculated attributes now.

Agreed, interpolation like this is out of scope here, and by adopting a provider syntax that's consistent with resource syntax, it's immediately obvious how interpolation would work when it is implemented.

I also still think you should just go ahead with implementing that syntax now. You don't need any changes to the parser because you can know that someone is using the new syntax because all of the keys will be maps, which I think is guaranteed to never happen with any existent provider because all providers have at least one attribute which is not a map. Deprecate the old syntax now and remove it in a few releases.

[keithchambers]

We would really like to see this for 4.0. It blocks our move to Atlas for @ https://github.com/CiscoCloud/microservices-infrastructure

[mitchellh]

@keithchambers This won't make it in 0.4 since it just isn't ready, and we're releasing 0.4 right now. But it is our priority for the next version which will have a much shorter release cycle.

[mgood]

@bitglue well as I mentioned before, the method you suggested would seem to work for the built-in providers, but providers can be extended by plugins. Some plugins could use maps in their config, so I wouldn't want to introduce a change that would potentially break a third-party plugin due to a new interpretation of the config syntax.

I still like the syntax, but at this point I don't see a way to do it without some possible breaking changes. I'll leave it to the Hashicorp developers to decide how or when they'd want to make that change.

[bitglue]

As I proposed, it would only be a problem if the provider was configured only by maps. I think it's extremely unlikely that someone implemented a provider which doesn't have a single scalar configuration value, or no configuration at all. This very tiny hypothetical breakage seems insignificant in comparison with the inherent instability of a 0.x software. I already have to patch bugs in Terraform almost every time I update it -- if I had to open up my manifests and change provider "foo" to provider "foo" "foo", I'd call that an easy day, especially for someone who's familiar enough with Terraform to have tackled writing a custom, wacky provider that is configured exclusively by maps. I'll take a slim chance of needing to change my syntax now over a certainty of changing it later when the syntax changes again.

[mgood]

@bitglue I'll leave that judgement to the Hashicorp team, who knows a lot more about the plugin community than I do. I've found them quite helpful on the IRC channel, so I'm sure you could chat with them more about those ideas.

One issue is that they seemed to want the alias to be an optional extra parameter in the provider block, rather than deprecating the current syntax. In which case, there would need to be a predictable way to distinguish the two going forward, not just during a deprecation period.

Also, while I agree that it's unlikely that a provider's schema would only contain maps, consider this config:

provider "foo" {
  region = "us-east"
  extra_params { foo = "bar" }
}

Your proposed solution would correctly distinguish this config, however, the region key may be optional. The provider might fall back on a default, read the default from an environment variable, or prompt the user for input. Even though that provider accepts scalar config parameters, they might not all be included in each config file. With your suggestion, removing the region key in the above would change the meaning of the extra_params, and the error messages would probably be confusing the the user.

While the chances of this might be slim, I don't know the Terraform community well enough to determine the risk myself. So, I'd recommend talking it over with the Hashicorp team for more feedback.

[bitglue]

Talked to @phinze on IRC. Sounds like he's in agreement that provider "type" "name" is the eventual goal, which works well enough for me. He did say:

the plan is that we'd land both within a release cycle to avoid too much backcompat sadness

[progrium]

Is this going to make it into the next release?

[mitchellh]

@progrium Yes

[mgood]

Thanks @mitchellh, let me know if there some additional tests or anything you'd like to see.

[metahertz]

Hi @mgood Thanks for this!
Before I start using it in anger (with the openstack provider) how 'incomplete' is it as you mention above?
Ie, will a breakdown of what does/doesn't work be helpful at this point, or too early to try out?

Also i agree with @bitglue that the format within resource types needs to flow better with the existing, top level naming decleration.

Also, what is the expectation in the below scenario:

provider: aws
alias: 1

provider: aws
alias: 2

resource security_group "SGname"
  stuff

resource vm "instance 1"
  provider: 1
  security_group: SGname

resource vm "instance 2"
  provider: 2
  security_group: SGname

In my mind, Terraform should deploy the security group to ALL configured providers (of type AWS in this case) (with each provider then getting one instance each as configured with the provider: option).

I guess in @bitglue 's naming format i'm saying resource aws.* should be the default for resources that don't explicitly define which sub-provider to use and multiple providers of the same type are configured.

This would allow multi-region deployments to share common resources without the need for duplication in the TF file (example, keys, security groups, image uploads (when supported)).

Thoughts?
Matt

[bitglue]

@matjohn2 That's a use case I hadn't considered, though I think it would be better if the syntax to do it on all matching providers were explicit. Otherwise you forget to type something, and you get a big surprise.

Having an explicit syntax would also allow richer syntax about what matches. Globs, regexes, etc. to apply to many, but not all the providers.

[mgood]

@matjohn2 the issues I'd mentioned initially have since been addressed. The Hashicorp devs might notice something else while reviewing the change, but I think it's complete at this point.

The provider syntax will eventually be changed to provider "aws" "name" for consistency with how resources are named. @phinze has indicated that they plan some improvements to the config parser to handle that without the heuristics discussed above. At this point it sounds like both are planned as part of the next release.

Any resources without an explicit provider key will use the default provider which is defined without an alias. Each resource still only corresponds to one provider instance. I think for common resources created in all regions you would still want to put those in a module that you can reference multiple times.

Although, I believe that AWS security groups are "global" and not region-specific. So for security groups, you could define them just once, with any valid AWS provider and they would be usable by instances created in other regions.

[metahertz]

Thanks @mgood
I did some testing today using the openstack provider against two different openstack installations.
It works, assets are created to labelled providers, however, I have an issue with how TF handles resource tracking/dependancies and creation which i'd like to work through with you. (To rule out my configuration vs bug).

I aliased both provider sections (there is no provider in my configuration without an alias line).
Resources refer to eachother (ie, keypair, securitygroup are needed by the VM instance)

When each of the resources are assigned to the same 'provider:' everything is fine, but if I assign the security group (for example), to another aliased provider, i'd expect $terraform plan to fail with unmet dependancies for the region.

Instead, it succeeds, as if the planning/dependancy logic is aware only of the high level provider vs sub-region requirements.

This then fails at the apply stage with an openstack error response, as we have attempted creation of an instance, using a security group that doesn't exist in that region.

* Error creating OpenStack server: Expected HTTP response code [201 202] when accessing [POST https://cloud:8774/v2/b32f38ccf8834fd99427b3415a406295/servers], but got 400 instead
{"badRequest": {"message": "Unable to find security_group with name 'mj_tf_sec_1'", "code": 400}}

I've also tested this on other resource types (ie, the Openstack Quantum network i'm creating) with a similar dependancy-related issue, resulting in an apply-time error.

My configuration is below, minus credentials. In the example, you can see the security group is in a different provider region, tx1 vs int1.

Shout if it's incorrect and i'm bringing this upon myself!
If it's a bug, i'd say the the namespaceing change proposals you discussed above should make this much easier to deal with, as the dependancy map is then based on resource naming vs checking an extra field.

Also, it's really easy to 'lose a resource' by changing the 'provider:' field, TF plan shows the resource needs creating in the new region, but forgets about your old instance. This seems to be another side affect of the same issue.

# Testing the OpenStack provider from merge of 
# https://github.com/hashicorp/terraform/pull/1281


provider "openstack" {
     alias = "int1"
     user_name = "blah"
     tenant_name = "INT1-TENANT"
     auth_url = "https://INT1-api:5000/v2.0"
     password = "indeed"
}

provider "openstack" {
     alias = "tx1"
     user_name = "blah"
     tenant_name = "TX-TENANT"
     auth_url = "https://TX-API:5000/v2.0"
     password = "indeed"
}


#
# Create a new keypair in our openstack tenant
#

resource "openstack_compute_keypair_v2" "mj-tf-keypair-1" {
    name = "mj-tf-keypair-1"
    region = ""
    provider = "openstack.int1"
    public_key = "ssh-rsa AAAAB3C1yc2EAAAADAQABAAACAaJNEz6lnPY9kLPs6MUED6Lb61pojol8yBc/fE/8fSp9s25kO2p37RvhsNs9kIiKZvu7vgNc/lVw0doU4LQL0jJn1uW6yVleECmi6bH1FK+nBkI+5dut6eduE5vXsPLpTwsCZDQ5RiW/iVO5puQdTq30hlfHSKEr5OuVYZG1aFzTR/OWdt6LHWZD2bSnBp1tbw/Dr4LEnDYGx3s0ZrZwmXvSveMt+HpOER/c8d+AgkU/IALchNuqv47ZIBAguGqX0ZolzJfNmytznU+cDmsvHHKN556bth8mr5SZuxor1mN1QgcokZ2+m3c41iCVGI8QPMpmWURaxp6PHj646l8Jogox24+jGyuhnJy2iWDD5xxVvMgWcE5dayn7iacpRV51EbbBRlmyGcCVhWZZdm6Cw=="
}


#
# Create a security group
#

resource "openstack_compute_secgroup_v2" "mj_tf_sec_1" {
    region = ""
    name = "mj_tf_sec_1"
    provider = "openstack.tx1"
    description = "Security Group Via Terraform"
    rule {
        from_port = 22
        to_port = 22
        ip_protocol = "tcp"
        cidr = "0.0.0.0/0"
    }
    rule {
        from_port = 1
        to_port = 65535
        ip_protocol = "tcp"
        self = true
    }
    rule {
        from_port = 443
        to_port = 443
        ip_protocol = "tcp"
        cidr = "0.0.0.0/0"
    }
}


#
# Create a Network 
#
resource "openstack_networking_network_v2" "mj_tf_network" {
    region = ""
    provider = "openstack.int1"
    name = "mj_tf_network"
    admin_state_up = "true"
}

#
# Create a subnet in our new network
# Notice here we use a TF variable for the name of our network above.
#
resource "openstack_networking_subnet_v2" "mj_tf_net_sub1" {
    region = ""
    provider = "openstack.int1"
    network_id = "${openstack_networking_network_v2.mj_tf_network.id}"
    cidr = "192.168.1.0/24"
    ip_version = 4
}

#
# Create a router for our network
#
resource "openstack_networking_router_v2" "mj_tf_router1" {
    region = ""
    provider = "openstack.int1"
    name = "mj_tf_router1"
    admin_state_up = "true"
    external_gateway = "ca80ff29-4f29-49a5-aa22-549f31b09268"
}

#
# Attach the Router to our Network via an Interface
#
resource "openstack_networking_router_interface_v2" "mj_tf_rtr_if_1" {
    region = ""
    provider = "openstack.int1"
    router_id = "${openstack_networking_router_v2.mj_tf_router1.id}"
    subnet_id = "${openstack_networking_subnet_v2.mj_tf_net_sub1.id}"
}

#
# Create a VM Instance on CentOS7 Image by name
#

resource "openstack_compute_instance_v2" "mj_instance_1" {
    region = ""
    provider = "openstack.int1"
    name = "mj_tf_test_1"
    image_name = "centos-7_x86_64-2015-01-27-v6"
    flavor_name = "GP2-Xlarge"
    key_pair = "mj-tf-keypair-1"
    security_groups = ["${openstack_compute_secgroup_v2.mj_tf_sec_1.name}"]
    metadata {
        foo = "bar"
    }
    network {
        uuid = "${openstack_networking_network_v2.mj_tf_network.id}"
        fixed_ip = "192.168.1.100"
    }
}

#
# Create a VM Instance on CentOS7 Image by ID
#
resource "openstack_compute_instance_v2" "mj_instance_2" {
    region = ""
    provider = "openstack.int1"
    name = "mj_tf_test_2"
    image_id = "43b247f3-8d79-4945-8f03-76cf0e8e7008"
    flavor_name = "GP2-Xlarge"
    key_pair = "mj-tf-keypair-1"
    security_groups = ["${openstack_compute_secgroup_v2.mj_tf_sec_1.name}"]
    metadata {
        bar = "baz"
    }
    network {
        uuid = "${openstack_networking_network_v2.mj_tf_network.id}"
        fixed_ip = "192.168.1.101"
    } 
}

#
# Create a VM Instance on CentOS7 Image by ID
#
resource "openstack_compute_instance_v2" "mj_instance_3" {
    region = ""
    provider = "openstack.int1"
    name = "mj_tf_test_3"
    image_id = "43b247f3-8d79-4945-8f03-76cf0e8e7008"
    flavor_name = "GP2-Medium"
    key_pair = "mj-tf-keypair-1"
    security_groups = ["${openstack_compute_secgroup_v2.mj_tf_sec_1.name}"]
    metadata {
        bob = "zzwaa"
    }
    network {
        uuid = "${openstack_networking_network_v2.mj_tf_network.id}"
        fixed_ip = "192.168.1.110"
    } 

Matt

[sparkprime]

This is interesting as I had a long term plan to add region to all google resources that did not already have a zone. This PR would make that unnecessary, although it seems like a bit of a hack to create a whole new provider config just to make it possible to deploy multi-region.

[mgood]

@matjohn2 I think that's just a configuration issue, not a bug. When you use the syntax "${openstack_compute_secgroup_v2.mj_tf_sec_1.name}" it's just a string, and Terraform doesn't know that it's not valid to use that name across different providers. Plenty of keys could be valid to use across providers. For example, you could pass the IP address of one instance to the config for an instance in another region.

[sparkprime]

Yes the resource model is not type-checked. Failures always come from the APIs behind the providers, so this is WAI. Lack of knowledge within Terraform of these kinds of constraints reminds me of #178

[mitchellh]

Pulling this down to do a full review now. Please don't make any more changes, or notify me if you need to!

[mitchellh]

Great job. I finished the review, fixed the merge conflict, and I'm merging this in. There are some additional tests and bug fixes I need to make but I'll do so via another upcoming PR.

[mitchellh]

Merged: 21b0a03

[mgood]

Thanks @mitchellh and @phinze

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.