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

Data-driven Terraform Configuration #4169

Closed
apparentlymart opened this issue Dec 4, 2015 · 35 comments
Closed

Data-driven Terraform Configuration #4169

apparentlymart opened this issue Dec 4, 2015 · 35 comments

Comments

@apparentlymart
Copy link
Contributor

A common design pattern has emerged of using read-only resources like consul_keys (when used without explicit value arguments), terraform_remote_state and atlas_artifact to produce Terraform configurations from dynamically-loaded data.

Relatedly, there are also now several "logical resources" that don't even fetch data: instead, they just transform a set of inputs into a set of outputs entirely within Terraform. These include template_file, tls_private_key and tls_cert_request.

The current resource workflow requires that a resource must always be "created" before it can be used as a dependency, even though "create" is not a meaningful action for either read-only resources or logical resources. This creates a few different issues, which I will enumerate in the sections that follow.

This proposal is to make data-driven configuration a first-class feature of Terraform, with the goal of making Terraform's behavior more intuitive, predicable and explicit.


Problems with Data-as-Resources

Incomplete Diffs

Consider the following configuration:

resource "consul_keys" "deploy_config" {
    key {
        name = "instance_type"
        path = "myapp/ec2_instance_type"
    }
    key {
        name = "instance_ami"
        path = "myapp/ec2_source_ami"
    }
}

resource "aws_instance" "app_server" {
    instance_type = "${consul_keys.deploy_config.var.instance_type}"
    ami = "${consul_keys.deploy_config.var.instance_ami}"
}

Here we use values from Consul to populate some arguments of an aws_instance. This results in the following plan:

+ aws_instance.app_server
    ami:                      "" => "${consul_keys.deploy_config.var.instance_ami}"
    availability_zone:        "" => "<computed>"
    ebs_block_device.#:       "" => "<computed>"
    ephemeral_block_device.#: "" => "<computed>"
    instance_type:            "" => "${consul_keys.deploy_config.var.instance_type}"
    key_name:                 "" => "<computed>"
    placement_group:          "" => "<computed>"
    private_dns:              "" => "<computed>"
    private_ip:               "" => "<computed>"
    public_dns:               "" => "<computed>"
    public_ip:                "" => "<computed>"
    root_block_device.#:      "" => "<computed>"
    security_groups.#:        "" => "<computed>"
    source_dest_check:        "" => "1"
    subnet_id:                "" => "<computed>"
    tenancy:                  "" => "<computed>"
    vpc_security_group_ids.#: "" => "<computed>"

+ consul_keys.deploy_config
    datacenter:             "" => "<computed>"
    key.#:                  "" => "2"
    key.2539070124.default: "" => ""
    key.2539070124.delete:  "" => "0"
    key.2539070124.name:    "" => "instance_type"
    key.2539070124.path:    "" => "myapp/ec2_instance_type"
    key.2539070124.value:   "" => "<computed>"
    key.2621654296.default: "" => ""
    key.2621654296.delete:  "" => "0"
    key.2621654296.name:    "" => "instance_ami"
    key.2621654296.path:    "" => "myapp/ec2_source_ami"
    key.2621654296.value:   "" => "<computed>"
    var.#:                  "" => "<computed>"

In this diff the ami and instance_type arguments are shown as their raw, uninterpolated values because the consul_keys resource hasn't been "created" yet. This makes the diff almost useless: we can see that an instance will be created, but the diff tells us nothing about is characteristics or its cost profile.

It would be better for Terraform to look up the data in Consul before making the plan, and then show the true values in the diff to make clearer what will happen on terraform apply.

Counts are not resource-configurable

Consider the following desired configuration, which does not work on current versions of Terraform:

resource "consul_keys" "deploy_config" {
    key {
        name = "instance_count"
        path = "myapp/ec2_instance_count"
    }
}

resource "aws_instance" "app_server" {
    // ... usual instance arguments ...

    count = "${consul_keys.deploy_config.var.instance_count}"
}

Here the user wants to use a Consul key to specify how many app_server instances are to be created, but this fails because the count meta-argument supports interpolation only from variables.

This limitation on count exists because the value of count must be known during the plan phase, and thus it is impossible to complete the plan if the count value were a resource property that is not known until after apply.

It would be ideal if Terraform could look up the instance_count value before creating the plan, thus supporting the above use-case.
#1497 describes a variant of this problem where the count data comes from within a Terraform module, but in the process demonstrates various use-cases for populating count from retrieved data.

Interpolation within provider configuration

Finally, consider the following configuration:

resource "consul_keys" "deploy_config" {
    key {
        name = "deploy_region"
        path = "deploy_config/region"
    }
}
provider "aws" {
    region = "${consul_keys.deploy_config.var.deploy_region}"
}
// ...and then any resource that comes from the aws provider...

Terraform permits this configuration and correctly notes within the graph that the aws provider depends on the consul_keys resource. However, this fails during terraform plan:

Error refreshing state: 1 error(s) occurred:

* 1 error(s) occurred:

* Not a valid region: 

The issue here is that the provider must be instantiated in order to produce the plan, but the consul_keys resource has not yet been "created" and so the region argument interpolates to the empty string, or in some cases to the raw source string ${consul_keys.deploy_config.var.deploy_region}.

Conceptually Confusing

Terraform's conceptual model currently overloads the idea of a "resource" to support three different use-cases:

  • Creating and managing an object within a system or service ("traditional" resources)
  • Reading data from a system or service ("read-only" resources)
  • Transforming some data into another form or computing values on-the-fly ("logical" resources)

Given that all resource names are just nouns, the detailed documentation for each resource is the only place to discover which of the above categories a given resource belongs to, and thus understand its effect.

For example, atlas_artifact could either create an artifact within Atlas or it could read the data about an existing one. The only way to know which -- e.g. when you see one in a Terraform diff -- is to study its documentation. (It's the latter, incidentally.)

For another example, consul_keys is a rather-strange resource that is used for both reading and writing data to Consul, a combination that is unprecedented elsewhere in Terraform's builtin providers. Again the difference between these cases is very subtle when shown in a Terraform diff, and this hurts Terraform's predictability, reduces user confidence, and increases the risk of misunderstandings and mistakes.


Proposed Solution: First-class Data Sources

To address the above concerns I propose introducing the concept of "data sources" to represent the retrieval of external data or the computation of local data for use elsewhere in the configuration.

Here are some example data source declarations to illustrate the proposed UX while simultaneously showing examples of potential data sources:

// Read values from a Consul key/value store
data "consul_keys" "deploy_config" {
    key {
        name = "region"
        path = "deploy_config/region"
    }
    key {
        name = "instance_count"
        path = "deploy_config/instance_count"
    }
}

// Read artifacts from atlas
data "atlas_artifact" "app_server" {
    name = "example/web"
    type = "amazon.image"
    build = "latest"
}

// Remote state
data "terraform_remote_state" "environment" {
    backend = "consul"
    config = { ... }
}

// Locally-computed Template
data "template_file" "foo" {
    template = "${file("${path.module/hello.tmpl}")}"
    vars = {
        name = "Stephen"
    }
}

// Locally-created TLS private key
data "tls_private_key" "not_so_secret" {
    algorithm = "ECDSA"
}

// Find an AMI matching the given criteria using the AWS API
data "aws_ami" "deploy_image" {
    tags = {
        Name = "my-awesome-app"
        QAStatus = "approved"
    }
    // Take the most recently-created AMI matching the above tags
    select = "latest"

    // exposes "id" as an attribute
}

// Retrieve a public key from Rundeck's key store in order to create
// a corresponding AWS key pair object that will allow Rundeck to
// access an EC2 instance.
data "rundeck_public_key" "admin" {
    path = "anvils/id_rsa.pub"

    // exposes "key_material" as an attribute
}

Just like resources, data source belong to providers and are prefixed with the provider name.

They exist in a separate namespace, so for example we can have both an aws_ami resource (to create and manage an AMI within Terraform) and an aws_ami data source (to retrieve data about an AMI that was created separately). As a convention, the attributes exposed by a data source would correspond to arguments or computed attributes on the same-named resource, so that an object is used in a similar way regardless of whether Terraform is directly managing it.

Data source attributes can be interpolated:

provider "aws" {
    region = "${data.consul_keys.deploy_config.var.region}"
}

resource "aws_instance" "app_server" {
    instance_type = "t2.micro"
    ami = "${lookup(data.atlas_artifact.app_server.metadata_full, "region-${data.consul_keys.deploy_config.var.region}")}"
    count = "${data.consul_keys.deploy_config.var.instance_count}"
}

Data interpolations are valid anywhere that variable interpolations are valid, although dependency cycles between them are a fatal error as always.

A data interpolation may be used within a provider block or a resource count only if the data source instance doesn't (directly or indirectly) depend on a resource instance, since that would cause its value to be computed at plan time and thus prevent the plan from completing. (If #4149 were also implemented, the restrictions here would be loosened but some configurations would require progressive application.)

By introducing data as a distinct concept we clarify the model such that users can expect that data blocks will never change infrastructure and that resource blocks will always change infrastructure. During refresh the data source is immediately read and its values stored in the state. These values can then be interpolated immediately during the plan phase, showing the final values in any resulting diffs.

A data source instance will never appear in a plan, just like variables and providers do not appear in plans. This will make Terraform's behavior more predictable and intuitive, as the plan will include only changes to infrastructure and not no-op create/replace actions.


Implementation Details

Internally, data sources work a lot like resources except that Refresh is the only lifecycle function that provider plugins must implement for them.

helper/schema is extended to understand data sources. Schema-based data sources re-use all the same schema definition mechanisms that resources do, including the idea that attributes may be Required, Optional and Computed in various combinations, except that it doesn't make sense for a data source attribute to be both Optional and Computed, unlike resource attributes.

A new RefreshDataSource method is required for providers that expose data sources. This has an equivalent signature to the Refresh function for resources.

In order to allow data interpolations into providers and resource count with predictable behavior, the definitions of both of these is changed to allow any interpolation as long as the resulting value is not computed.

Backward Compatibility

Existing read-only and logical resources

As noted above, there are several existing read-only and logical resources that must be preserved for compatibility.

To achieve this, a utility is created in helper/schema that wraps a data source implementation and implements the resource interface in terms of it, by providing the usual stub implementations of Create, Update and Delete.

Making use of a resource that is implemented with this wrapper generates a deprecation warning, encouraging the user to switch to the equivalent data block.

consul_keys continues to exist in its current form but any key blocks that do not set value result in a deprecation warning encouraging the use of a data "consul_keys" block instead.

Interpolation of resources into provider blocks

The above proposal suggests that it would become invalid to use computed values (from not-yet-created resources) in provider blocks, but that is a breaking change from the current behavior. Currently such configuration is accepted but requires manual -target workarounds to apply a configuration from scratch.

To avoid breaking existing configurations that do this in spite of the sub-optimal behavior that results, using computed values inside providers would initially just yield a deprecation warning rather than a hard failure.

One use-case remains unsupported: the use of a "true" resource to configure a provider. For example, it would become deprecated to use the computed properties of an aws_instance running Rundeck to configure a rundeck provider within the same configuration. The separate proposal #4149 attempts to address this with a more significant change to Terraform's workflow, but in the absense of that change such use-cases would be effectively unsupported.

Changing the provider plugin interface

Adding a new RefreshDataSource method to the provider plugin interface requires some care to avoid breaking compatibility with plugin binaries built against earlier versions.

RefreshDataSource will only get called in the presence of a data block referencing the given provider, so this should be fine as long as users don't try to use data sources from provider binaries that predate the concept.

If they do, the call to RefreshDataSource will fail on the "server" side of the plugin RPC channel, causing an error to be returned to the "client" side. The client would ideally detect this error case and present it to the user as a "provider does not support this data source"-type error.


Relationship to Other Proposals

  • In Pre-refreshed resources #2976 I previously suggested that Terraform support the concept of "pre-refreshed resources" as a way to solve many of the same problems. That model was trickier to explain and less explicit, so as far as I'm concerned this proposal supersedes that one.
  • In Partial/Progressive Configuration Changes #4149 I propose a change to Terraform's workflow to better support tricky inter-dependency cases and allow ideas like a truly-dynamic count for resources. If Partial/Progressive Configuration Changes #4149 were implemented without this proposal then many of my earlier motivating examples would begin to work but would require progressive application.
  • In terraform_synthetic_state resource #3164 I suggested a terraform_synthetic_state resource as a complement to terraform_remote_state that allows one configuration to explicitly pass configuration to another, piggybacking on the remote state mechanism. I would retract that proposal in favor of this one if this resulted in the implementation of a data "aws_s3_bucket_object" counterpart to resource "aws_s3_bucket_object", and similar pairings allowing data to be passed between configurations with equivalent flexibility as offered by the remote state mechanism.
  • #2756 suggests that provisioners also become a provider-owned concept. This does not really interact with the data source proposal except that together these proposals allow providers to support a coherent family of related operations: read data (data sources), manage objects (resources), perform ad-hoc actions (provisioners). These three concepts should together provide reasonable mappings for almost all of the API surface of any given service, helping Terraform to expose the full functionality of the services it supports.
@phinze
Copy link
Contributor

phinze commented Dec 4, 2015

@apparentlymart Just did a first read through here. I am going to need to process this more, but allow me to share my initial reaction:

Yes! This! 😄

I too have been mentally feeling out this need for a new top level concept, and your thinking is clearly in nearly the exact same vein.

A more detailed review to follow next week. In the meantime, have a wonderful weekend!

@sl1pm4t
Copy link
Contributor

sl1pm4t commented Dec 5, 2015

Yes, this would be a big help.
You touched on it already @apparentlymart - but I think it's important to reiterate IMO the data should not be refreshed, or only under provocation, for existing resources. So in essence the state of data source ( specific to each resource) should only be read at resource creation time. Otherwise configurations can't be evolved without potentially impacting existing deployed resources if a global value is changed in the data source.

@apparentlymart
Copy link
Contributor Author

@sl1pm4t that's an interesting point, since I actually had exactly the opposite perspective!

For me, changing the upstream data source should cause the corresponding changes in the resources that are derived from the data source. In all of my use-cases that is a requirement. For example:

  • The idea of the atlas_artifact data source is to find the latest artifact for a given application. If I build a new artifact then I want Terraform to deploy it on the next run.
  • If I'm using terraform_remote_state and the upstream state changes, I need to adapt my downstream state to fit with it on the next deploy.
  • If I use template_file to render a template and then I change the template, I want all uses of that template to be updated.

So for me, the idea was that you choose data sources that only change at times when you want your Terraform config to change. If your data source changes more often than you want to change Terraform-based infrastructure then you've chosen the wrong data source.

My initial impression is that you're asking for an orthogonal feature to be able to "freeze" a resource after it's created, so rather than re-applying all of the interpolations on the next run Terraform would instead use the literal values from the state, perhaps until you explicitly "unfreeze" the resource?

Supporting that sort of thing only for data sources is conceptually rather difficult since it requires the data source data to be stored separately for every single resource in the state, so that each resource retains its "snapshot" of what the data looked like at the time it was created. I believe that it could be done, but that feels like an "opt-in" sort of thing to me, rather than the default behavior. I also wonder if it should generalized to include all of a resource's dependencies, rather than just data sources.

If you'd be willing, I'd love to hear some more detail about your use cases to see how they might fit in to this design. If have some time to write up some example configurations of what you're thinking and how you'd like Terraform to behave with those configurations, that'd be very useful to help me understand what you're trying to achieve and how it fits into my conception of this feature.

@sean-bennett112
Copy link

This single issue sums up most of my problems with Terraform, and really cleanly addresses them. First class data sources would be a huge help in a number of ways. Currently I'm plugging data(amis, snapshots, etc) into Terraform via various shell/Python scripts, and it works, but it feels incredibly kludgy.

Many(all?) of the existing 'logical' resources would fit better conceptually under this umbrella. The aws_ami example is also excellent, as would a number of other resources that frequently change outside of Terraform.

I tend to agree that I would want, by default, changes to the upstream data source to cause corresponding downstream changes to resources, though I can think of examples where I wouldn't want that.

For example, we currently deploy instances with a number of ebs volumes attached, each of which is based off a given snapshot on deployment. These snapshots get regularly rotated, so that grabbing the latest snapshot(matching a specific set of tags, let's say) is very likely to be different on a subsequent Terraform run.

Now, those snapshots were taken from the currently running instance, and changes in those snapshot ids don't really reflect any changes that should require the instance to be reprovisioned. However, I would still definitely prefer to have that information specified in a Terraform data source, rather then grabbed from a script and passed in via a variable. So I'm thinking it might be useful to be able to specify whether you want changes to this data source to, by itself, cause updates to downstream resources that use it.

Does that use case make sense?

@phinze
Copy link
Contributor

phinze commented Dec 7, 2015

Chiming in on the "pinning" / "freezing" data sources question: I think this should be a per-resource question rather than a core feature. atlas_artifact already supports this via its version attribute. If we tried to address this universally we'd have to start internally versioning every data source, which is not complexity that the Terraform state should be responsible for.

The use case of explicit pinning / promotion is important - this is how we use atlas_artifact in production at HashiCorp after all! - but the implementation belongs outside of core.

@mtougeron
Copy link
Contributor

I'm really looking forward to this feature.

Our primary use case (at the moment) would be to load a set of credentials using the Vault provider to use for connecting to AWS and other providers. In this example, I would want the provider definition to refresh based on changes to those credentials, but I wouldn't want the related resources to automatically refresh. It is also important to be able to taint the provider definition or the resource that was used to configure it without forcing a refresh of the related resources. In our use case, the AWS credentials from Vault would be using the dynamic AWS backend that expire after X hours. This would require that the developer taint the Vault resource in order to reconnect to AWS when they next run Terraform.

Hope this use case helps,
Mike

p.s., Yes, I'm aware of the security implications of the Vault secrets being stored in the tfstate file(s) but since they are short-lived, temporary credentials we are willing to accept that risk.

@apparentlymart
Copy link
Contributor Author

@mtougeron Thanks for that use-case! Let's see how that might look under this proposal, with a hypothetical vault provider and data source:

provider "vault" {
    address = "https://vault.service.consul:8200"
    // "token" from VAULT_TOKEN environment variable, hopefully
    ssl {
        enabled = true
        verify = true
        cert = "/path/to/cert.pem"
        ca_cert = "/path/to/ca_cert.pem"
    }
}

data "vault" "aws" {
    path = "aws/creds/deploy"

    // Attributes:
    // data: map of key/value pairs from the vault response
    // lease_id: the id of the lease in vault
    // lease_duration: the duration of the lease in seconds
    // renewable: bool for whether the lease is renewable
}

provider "aws" {
    access_key = "${data.vault.aws.data.access_key}"
    secret_key = "${data.vault.aws.data.secret_key}"
}

resource "aws_instance" "foo" {
    instance_type = "t2.micro"
    ami = "ami-12345"
}

Since values from the vault data source are interpolated into the aws provider configuration, each time new credentials are issued the AWS provider credentials will immediately change for the next Terraform run. It is the user's responsibility to make sure that the provided credentials always refer to the same AWS account, since Terraform isn't able to support automatically migrating resources from one account to another.

Assuming the user preserves that invariant, the aws_instance resource depends on the aws provider and thus in turn the vault data source, but its values do not depend on any attributes from the vault provider, so the constantly-shifting Vault credentials will have no effect on the configuration of that instance, so no unusual actions are required to prevent it from being updated or replaced on future updates. In particular, there is no need to manually taint anything because Terraform is able to infer automatically what actions are required here.

Please let me know if I've missed part of your use-case in the above example.

@mtougeron
Copy link
Contributor

That looks good to me.

@sl1pm4t
Copy link
Contributor

sl1pm4t commented Dec 7, 2015

Sorry for the delayed response.
Our use case is perhaps a little unconventional and goes something like this:
We are not using terraform for production infrastructure, rather to automate deployment of lab environments for our developers. Our software is a large monolithic beast that can take any where from 20mins to an hour to install, and then requires configuration. Once up and running it is treated like pets rather than cattle.
If a developer needs to deploy the software on a new server, they can SSH's into their home directory on the "terraform server", run a script to generate a module stub that references some terraform module stored in git, and then runs terraform apply to execute the change. They are not expected to know the ins and out of terraform. Unexpected resource destruction and recreation will not be received happily, but I'd like to be able to evolve the configuration that gets used for new resources (e.g. point to a new base image if we run a new packer build), without impacting existing deployed resources.

There are many ways to achieve what I need, so I'm not going to push hard for "one shot" datasource reads, or "frozen" resources etc, and will still follow this thread with interest.
Thanks!

@mitchellh
Copy link
Contributor

Sorry for taking awhile to come and take a look at this. I've read the original proposal as well as all the comments, so let me give my thoughts on this so we can continue to move the needle forward.

Overall, the idea is great. Terraform core knowing that a resource is logical would help in a lot of ways. A separate concept to ensure this makes complete sense. So I'm interested in making this work and so consider the rest of this comment questions towards making it work, rather than skepticism of the overall idea.

The main problem I see here, which is a feature of your request, is that data sources themselves are computed at refresh time. It appears that this would restrict data sources to not have any inputs that are computed. I can imagine scenarios where you would want a computed data source: you use a module which does stuff and you use its output as a key to Consul to grab data. This would necessitate "computed" and would have to show up in the plan.

Conceptually, this fits into plans already: if a field is not computed, we can show the value immediately in the plan. If it is computed, we show <computed>. I don't think major changes are necessary to the plan to make this happen. In fact, in core, I think this requires very little changes to accomodate data sources as a whole.

So, to sum up the above thoughts: I think data sources should show up in the plan, we should just try to refresh the value if its not computed. We have the data in the plan of what that thing is, so it becomes an output/UX issue that we just highlight the fact that something is a data source rather than a resource with a lifecycle. One way we can do this quickly is the icon before, the "+, -, ~". We can use a different icon for a data source since its not a create, but a fetch.

For implementation, I agree with your steps. I don't think any changes to helper/schema.Resource is necessary. To nitpick the new method on ResourceProvider, I'd name the function RefreshData as the config key is data. Just a nit!

Next, freeze/pin values: there is value in this idea. Let's not feature creep this with enhancements. Let's implement it in the way Terraform views the state of the world today: when something changes it trickles down. We can introduce freezing/pinning in a future version and let's not cause more discussion here. I agree its a good idea, but let's not block this feature on that.

@milsonian
Copy link

This would be a very useful feature. As a follow-on, it'd be great if we could tap into Consul's service discovery endpoint through this mechanism (in addition to kv).

@apparentlymart
Copy link
Contributor Author

@mitchellh Thanks for all this feedback!

Digging in a bit more into the situation where data source arguments are computed: My intent for the sake of this issue alone was to make this work the same way as computed provider configuration, which is to say that it would fail in the same sort of broken way that Terraform fails today. That's not great on its own, but then #4149 would provide the building-block necessary to fix it, just as it would fix computed provider configurations.

However, thinking this over again I see that there are situations where Terraform could make a complete plan even when given a computed data source argument:

  • The provider of the data source is not computed.
  • The data source has an argument that depends on a resource that does not yet exist, but can be planned.
  • The data source values are not used in any of the scenarios that would trigger the "partial plan" scenario as outlined in Partial/Progressive Configuration Changes #4149.

In a world where progressive application is possible we could just require the above to be applied in two steps, but if we allow refreshing the data source during apply time then we could do it in a single step.

So here's what a resource plan might look like for that scenario, assuming both an atlas_artifact data source and a consul_keys data source, both of which have computed arguments at plan time:

+ aws_instance.example
    ami:                               "" => "${lookup(data.atlas_artifact.app_server.metadata_full, "region-${data.consul_keys.deploy_config.var.region}")}"
    associate_public_ip_address:       "" => "1"
    availability_zone:                 "" => "<computed>"
    ebs_block_device.#:                "" => "<computed>"
    ephemeral_block_device.#:          "" => "<computed>"
    instance_type:                     "" => "${data.consul_keys.primary_dc.var.instance_type}"
    key_name:                          "" => "example"
    placement_group:                   "" => "<computed>"
    private_dns:                       "" => "<computed>"
    private_ip:                        "" => "<computed>"
    public_dns:                        "" => "<computed>"
    public_ip:                         "" => "<computed>"
    security_groups.#:                 "" => "<computed>"
    source_dest_check:                 "" => "1"
    subnet_id:                         "" => "subnet-deadbeef"
    tags.#:                            "" => "0"
    tenancy:                           "" => "<computed>"
    user_data:                         "" => "0bec99db19ddc75d2cc509e4bc3e9a47f3348ca4"
    vpc_security_group_ids.#:          "" => "1"
    vpc_security_group_ids.1651759775: "" => "sg-beefbeef"

The interpolations like ${data.consul_keys.primary_dc.var.instance_type} shown here already give the user some feedback that this value will come from the data source once it's loaded. It doesn't give the user feedback on why that data source can't be resolved yet. We could potentially address that by including a "data source" block in the plan as you suggested:

<- data.consul_keys.primary_dc
    datacenter:                        "${aws_instance.main.availability_zone}"
    keys.#:                            "1"
    keys.1234.name:                    "instance_type"
    keys.1234.path:                    "ec2_instance/type"

I think you were suggesting that only data sources with computed arguments would be included in the plan and handled during apply, with the un-computed ones still handled during refresh as I originally proposed. It'd be important in this case to make sure that we don't "re-refresh" statically-configured providers during apply, for consistency with the idea that we don't refresh resources during apply either, unless the resource implementation happens to call its Read function as part of its Create or Update.

@mitchellh
Copy link
Contributor

Correct in all cases. I think data fetching only shows in the plan if its computed itself, which is very possible if we allow the inputs themselves to be computed, which I think is a useful thing to allow.

But as a correction we do usually call refresh during apply, if you're talking about the CLI command apply unless -refresh=false. Though, if you're talking about the internal core Apply function, then you're correct. The logic around when we refresh should be interesting, and may pose some weird edge cases in the core. I'll think about this more since edge cases can be some sort of impl. smell.

@apparentlymart
Copy link
Contributor Author

I think we're in agreement, since I was talking about the internal passes rather than the user commands, but let's just make sure. Here is my understanding of the relationship between the CLI commands and the internal passes, with the rows showing some user commands and the columns representing the internal passes:

User Command Refresh Diff Apply
terraform refresh
terraform plan
terraform plan -refresh=false
terraform apply
terraform apply -refresh=false
terraform apply tfplan

So in my statement when I was talking about refreshing during apply, I was talking about the internal function Refresh during the user command terraform apply tfplan... which is to say, when the user already ran the plan in a previous step and is now just running the Apply phase without implicitly re-running Refresh and Diff.

In my conception of the solution we're describing, non-computed data sources are read during Refresh, while computed ones are read during Apply, once their arguments are no longer computed.

I think a key edge case to consider here is when a data source has already been read on a previous run, but in the current config one of its arguments has become computed somehow: perhaps a dependency resource has a -/+ diff on it, or the user just edited the config. In this case, I think we must be sure not to use the "stale" values of the data source that are already saved in the state, and instead plan to refresh it during the Apply phase and then propagate its values dynamically to its dependents.

@mitchellh
Copy link
Contributor

Correct yes. I think this would hopefully just naturally get represented into the graph.

@vancluever
Copy link
Contributor

Thanks for referring me to me this @apparentlymart on #4396. The only concern I would have is how this would tackle data sources tied to a provider - ie: to pull up a AMI with aws_ami/aws_ami_descripton, the provider connection needs to be set up first, but supporting interpolation from the data sources. Right now, providers are pretty non-intrusive, but say what happens when you want to configure an AWS provider with a consul key, look up an image with the configured provider, and then pass that down to a resource. Hopefully there's an elegant way around this that does not require configuration duplication.

Apologies if this was already covered, I admit to mainly only skimming thru all the issue content right now!

@lbernail
Copy link

This proposal would really help for several of our use cases. We currently tend to wrap terraform commands within shell/python scripts to retrieve data from external sources (I feel we are in a very similar situation as @sean-bennett112 ) but it would be a lot more natural if all logic was kept within terraform.

A very simple use-case that has not been mentioned yet is a simple data "local-exec" xxx which would use the output of a script (json for instance) to setup key/value pairs. This provider could be used as a workaround when there is not native data source.

@ketzacoatl
Copy link
Contributor

I would like to send out a big Thank you! to all of you who have contributed to get this proposal to where it is at today.

glasser added a commit to meteor/terraform that referenced this issue Apr 7, 2016
(Initial draft. This commit does not include doc updates, and all of the
code is in a single file instead of being split into provider/resource
files as is the terraform standard.)

Unlike the 'docker' provider, which talks to your local Docker daemon to
read and write your local image stores, the 'dockerregistry' provider
talks to a Docker Registry V1 server and confirms that the named
repository/tag exists.

It is a read-only provider: it doesn't push to the registry; it just
helps you assert that the docker tag push via your build process
actually exists before creating other configuration that uses it.
Ideally this would use the `data` feature from hashicorp#4169 instead.
@ketzacoatl
Copy link
Contributor

ketzacoatl commented Apr 15, 2016

The mangled nature of variables and data sources seriously limits our ability to bring Terraform to larger teams with high-security requirements (eg, Atlas would only be considered if it were on-prem). Considering the overwhelming support for this proposal, has this been included in a concrete roadmap, or still an ephemeral future todo?

@coen-hyde
Copy link

@ketzacoatl As a work around you could wrap the terraform execution and pull data from other sources then set them as ENV vars. This is what we do. This allows us to have environment specific configs, latest ami's and secrets.

@ketzacoatl
Copy link
Contributor

Right, I'm completely aware of that possibility (see https://groups.google.com/forum/#!topic/terraform-tool/5WInahdIQm4), but a work-around like this is cumbersome and kludgy (as others have noted). It's also a bit difficult for me to get developer/engineering attention on creating a tool for that (and which would sufficiently meet the needs of our different environments) when this issue is an obvious improvement/enhancement that will be coming in a future release. That is why I am asking a concrete question about whether this issue is a todo on the actual roadmap, or still unknown in that respect.

@apparentlymart
Copy link
Contributor Author

apparentlymart commented Apr 15, 2016

@ketzacoatl, @jen20 and I are currently working on this, and it's slated to be in the 0.7 release. I gather it's the last significant feature for that release so my assumption would be that 0.7 lands shortly after this does, though @jen20 (as a Hashicorp staffer) would know better than me on that front.

This feature is a tricky one because it touches basically every layer of Terraform, so it'll probably take a little time to work through all the layers and get it working right. I'm totally with you on the frustration of not having this feature, and I can promise you that I want it in a release at least as much as you do!

@ketzacoatl
Copy link
Contributor

That sounds wonderful, and I will (patiently) look forward to testing the RC when it is available. Thanks for your confirmation @apparentlymart, not to mention all your hard work on this!

@mtougeron
Copy link
Contributor

@apparentlymart woohoo! I'm super looking forward to this functionality. :) Thanks for the update.

@GreenEA
Copy link

GreenEA commented Apr 16, 2016

ahh… thank goodness for apparentlymart. Sometimes he makes me wish my rain was bigger.

On Apr 15, 2016, at 1:59 PM, Mike Tougeron <notifications@gh.neting.ccmailto:notifications@github.com> wrote:

@apparentlymarthttps://github.com/apparentlymart woohoo! I'm super looking forward to this functionality. :) Thanks for the update.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHubhttps://github.com//issues/4169#issuecomment-210565644

@vancluever
Copy link
Contributor

vancluever commented May 11, 2016

@apparentlymart - looks like you are close to making this happen 100% - awesome!

Just curious - did you manage to get data sources to interpolate in count as per the proposal? I merged your changes from #6598 last night and tried out terraform_remote_state and got the same error I did on my current build of 0.6.16.

I'm trying to investigate to see if it may have come from a path where that's not possible, but I'm pretty sure it didn't, unless that datasource needs to use the AWS provider when it does S3. Also, the state values were passed in to a module, where the error occurred, for what it's worth.

@apparentlymart
Copy link
Contributor Author

@vancluever sorry, we discussed that in a different forum and I realize now that I didn't mention it explicitly: after some back and forth we concluded that data sources in count was better split into a separate bit of work, because it requires some pretty big changes in its own right.

I would still like to make this happen, but want to get the basic functionality landed first since the patch is already rather large.

The current set of changes in #6598 should, however, already allow data source attributes (for non-computed data source configurations) to be used in provider configuration... apart from the caveat that you currently need to force -input=false so that it won't try to instantiate the provider to prompt for input.

@vancluever
Copy link
Contributor

Thanks @apparentlymart. Yeah, since this is a big thing for me and would make things that I'm working on a lot easier, I've been poring over the source myself and have not seen a silver bullet or anything. It seems so close!

The code does indeed work otherwise - if I add a static value for the variable in question, the rest of the data source stuff marches along just fine.

@jen20
Copy link
Contributor

jen20 commented May 23, 2016

This was merged in #6598 (for Terraform 0.7), so I'm going to close this issue in favour of new issues for regressions and new features in the implementation. Thanks to @apparentlymart for a huge amount of effort in specifying and implementing it!

@jen20 jen20 closed this as completed May 23, 2016
@idntfy
Copy link

idntfy commented Jun 28, 2016

This was merged in #6598 (for Terraform 0.7)

when Terraform 0.7 is going to be released? thanks

@catsby
Copy link
Contributor

catsby commented Jun 28, 2016

Hey @idntfy – we don't have a set date as we're still hammering out some last mile things, but we do have release candidates available:

@dtolnay
Copy link
Contributor

dtolnay commented Jul 20, 2016

@apparentlymart

sorry, we discussed that in a different forum and I realize now that I didn't mention it explicitly: after some back and forth we concluded that data sources in count was better split into a separate bit of work, because it requires some pretty big changes in its own right.

Where can I find the ticket tracking that?

@dtolnay
Copy link
Contributor

dtolnay commented Jul 27, 2016

Looks like there was no ticket but now there is: #7762.

@ghost
Copy link

ghost commented Apr 24, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests