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

helper/schema: arbitrary JSON attributes #3026

Closed
wants to merge 1 commit into from
Closed

helper/schema: arbitrary JSON attributes #3026

wants to merge 1 commit into from

Conversation

apparentlymart
Copy link
Contributor

Since provisioners implement directly their own configuration decoding, it's possible for them to have parameters that take arbitrary data structures, as we see in the attributes parameter of the chef provisioner:

    provisioner "chef"  {
        attributes {
            "key" = "value"
            "app" {
                "cluster1" {
                    "nodes" = ["webserver1", "webserver2"]
                }
            }
        }
    }
    // ...

For the OpsWorks support I've been working on in #2162 I'd like to be able to include a similar structure in a resource, since OpsWorks is essentially an interface to running Chef and thus I'd like it to follow the conventions of the Chef provisioner as much as possible.

Thus I'd like to propose a new schema type called TypeJSON, which represents any arbitrary JSON-friendly data structure as long as it is rooted in an object (a map[string]interface{}). When writing a config file, the user may either assign a native HCL data structure like in the Chef example above, or they may assign a string containing a valid JSON-serialized data structure.

Allowing both a native structure and a string has a few different advantages:

  • When assigning a string it's possible to use the file() interpolation function, in cases where the JSON structure is fixed and more convenient to maintain in a separate file.
  • When assigning a native structure it's possible to do normal Terraform interpolations such as list globbing.
  • Accepting a string containing JSON also provides backward-compatibility with existing resources that currently accept JSON strings, like aws_iam_policy, allowing them to be retroactively switched to TypeJSON without breaking existing configurations.

I've attached a prototype of this. It's has some known omissions, such as native structures containing nested computed interpolations, and there are no tests. However, my main goal here was to make this proposal and see what folks think about it before I try to push too much further in this direction.

resource "opsworks_stack" "foo" {
    chef_attributes {
        something {
            arbitrary = true
        }
    }
}

Such an attribute can either be set from a string containing JSON or
from an arbitrary data structure within the Terraform config itself.

This prototype is incomplete and in particular doesn't support some
important cases like the data structure containing computed
interpolations.
@mitchellh
Copy link
Contributor

Thanks for all the work in doing this. Unfortunately, while I understand the use case, I'm not a fan.

This seems like a really heavy and forced approach with the only use case I can think of is to support Chef attributes. It makes the configuration relatively arbitrary as well. More specifically: it is annoying to document that in chef_attributes you can do anything in HCL, and how that turns into JSON. Another way to think about it: JSON isn't a type, it is a format. "int" is a type, "string" is a type, "json" is a complex format. This feels wrong.

I understand the reasoning that you can use globbing + interpolations + file(). I think the better approach instead is to just make Chef attributes accept a string and then make new functions that make the globbing easier to print arrays into the string.

I also saw Radek's issue in #3014 and I see what he is doing there, but again it is very complicated road to go down and adds a new level of complexity to helper/schema that I'd prefer to avoid.

For Radek, he wanted to fix #2589. I think the better approach for that is to modify helper/schema to have a custom equality check function that the diff function in helper/schema will use instead and we can do a JSON parse and reflect.DeepEqual to compare.

It may be possible of course that this TypeJSON is the correct approach, but I'm not comfortable merging this in for these two use cases right now. I'd like to try alternative approach more within the current framework before we take a hammer and put a new type in to solve this.

@mitchellh mitchellh closed this Aug 19, 2015
@radeksimko
Copy link
Member

@mitchellh Thanks for the extensive explanation and tips.
I understand your concerns and I will see if there's something that could be done via reflect.DeepEqual.

@apparentlymart
Copy link
Contributor Author

It's true that the chef provisioner is the only case where we have so far where the structure is completely arbitrary, and maybe the provisioner is the thing that is "wrong" and it should be changed to accept a JSON string instead, for all of the same reasons.

@ghost
Copy link

ghost commented May 1, 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 May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants