puppetlabs / puppetlabs-terraform

Bolt Terraform plugin
Apache License 2.0
6 stars 18 forks source link

(DO NOT MERGE)(GH-1405) Convert resolve_reference task to use ruby_plugin_helper #6

Closed beechtom closed 4 years ago

beechtom commented 4 years ago

This updates the resolve_reference task to use the ruby plugin helper to perform operations common to Bolt plugins, such as mapping resource values to target data.

It replaces several methods that were part of the task with those in the plugin helper.

It also replaces the statefile parameter for the resolve_reference task with a state parameter to maintain consistency among the tasks and plans in the module.

TODO

donoghuc commented 4 years ago

Could you add an integration test for this?

beechtom commented 4 years ago

Updated with an integration test that runs a command on a discovered container.

donoghuc commented 4 years ago

I think we forgot to update dependencies in metadata.json. Those should include ruby_{task,plugin}_helper I think. For reference: https://github.com/puppetlabs/puppetlabs-puppet_agent/blob/4d0be4c630bba952f4086a9f28c6a80c5be2daf5/metadata.json#L119-L124

donoghuc commented 4 years ago

Could be a problem here: I noticed that executing resolve_reference in inventory plugin my computer would completely freeze up (requiring a reboot to regain control). It occurs with an inventory v3 statefile with something like:

"ingress.2541437006.cidr_blocks.#": "1",
"ingress.2541437006.cidr_blocks.0": "0.0.0.0/0",
"ingress.2541437006.description": "",
"ingress.2541437006.from_port": "22",
"ingress.2541437006.ipv6_cidr_blocks.#": "0",
"ingress.2541437006.prefix_list_ids.#": "0",
"ingress.2541437006.protocol": "tcp",
"ingress.2541437006.security_groups.#": "0",
"ingress.2541437006.self": "false",
"ingress.2541437006.to_port": "22",

Without digging in to the code, it could be a problem with an assumption that anything that can be coerced to an integer is assumed to be an array index. But it appears that something causes the task to consume enough resources to completely freeze my laptop.

donoghuc commented 4 years ago

Are you planning on squashing all the commits? The first two make sense, but then it looks like a bunch of stuff got added in to the Update changelog commit.

beechtom commented 4 years ago

Squashed and updated.