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

Single and Multiple provider definitions #18

Open
jpreese opened this issue Sep 19, 2020 · 4 comments
Open

Single and Multiple provider definitions #18

jpreese opened this issue Sep 19, 2020 · 4 comments

Comments

@jpreese
Copy link
Contributor

jpreese commented Sep 19, 2020

Thanks a ton for exposing this functionality, it'll allow us to stop maintaining a local version of this library. Now onto the issue at hand!

@sarcasticadmin explained it best in this comment: open-policy-agent/conftest#266 (comment)

The overall issue is that when an HCL file has a single provider definition, the resulting JSON is just a single object.

"provider": {
    "aws": { "version": "=2.46.0", "alias": "one" }
}

But when an HCL file has multiple provider definitons of the same type, aliased, the JSON is turned into a set.

"provider" : {
    "aws": [
        { "version": "=2.46.0", "alias": "one" },
        { "version": "=2.46.0", "alias": "two" }
    ]
}

We would like to always return a set for the provider block for consistency.

Question being, do you agree with this approach as well and is something that hcl2json should do? If so, again would be more than happy to do a PR! Otherwise, we'll handle this specific conversion ourselves.

Context: open-policy-agent/conftest#266

@tmccombs
Copy link
Owner

That seems reasonable. My big question is how does it decide if it should use a set/array or not? Should it always wrap objects in arrays, or have a list of keys that should always be treated as sets or arrays? If the latter, I think it's prefer if the set of keys was configurable.

@jpreese
Copy link
Contributor Author

jpreese commented Sep 19, 2020

The thought would be that if specifically a provider block only has a single entry for that provider, we would force it to be a set by putting it into a slice during the conversion process. The case where there are multiple providers already puts it into a set so we're good there.

I haven't dug into the conversion logic a whole lot yet, but that would more or less be the idea.

@tmccombs
Copy link
Owner

I'd rather have something more general, because currently this tool could be used for more than just terraform, and even in the context of terraform, this same issue could arise in other situations. For example, if you have something like:

resource aws_security_group "test" {
  name = "test"
  ingress {
    from_port = 22
    to_port = 22
   protocol = "tcp"
  cidr_blocks = "10.0.0.0/24"
  }
}

you'll get

{
    "resource": {
        "aws_security_group": {
            "test": {
                "ingress": {
                    "cidr_blocks": "10.0.0.0/24",
                    "from_port": 22,
                    "protocol": "tcp",
                    "to_port": 22
                },
                "name": "test"
            }
        }
    }
}

but if you had two ingress rules, you would get an array for resource.aws_security_group.test.ingress rather than a single object.

@jpreese
Copy link
Contributor Author

jpreese commented Sep 20, 2020

Agreed, that makes sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants