search

hashicorp / packer-plugin-sdk

Packer Plugin SDK enables building Packer plugins (builders, provisioners, or post-processors) to manage any service providers or custom in-house solutions
Mozilla Public License 2.0
34 stars 48 forks source link

fix: confighelper.Trilean to show as boolean #244

Closed cpwc closed 5 months ago

cpwc commented 5 months ago

Users do not need to know what's confighelper.Trilean similar to https://github.com/hashicorp/packer/pull/8673 to avoid confusion.

cpwc commented 5 months ago

Seems like packer-plugin-amazon aliased config to confighelper instead.

https://github.com/hashicorp/packer-plugin-amazon/blob/dae26ee80cbdf0977c990e81a8cf258b4b9d6b1f/builder/common/step_network_info.go#L18

lbajolet-hashicorp commented 5 months ago

I see.

Not sure we should handle every alias possible here, the approach to determine the type should probably be revisited then. If this is a major pain point I can see us accepting this in the meantime, but the fact that we're missing something as simple as a package alias means this is in sore need of refactoring.

@nywilken thoughts?

cpwc commented 5 months ago

@lbajolet-hashicorp @nywilken alias might change but the actual type won't change. A potential change we can look at is see if it contains Trilean?

lbajolet-hashicorp commented 5 months ago

Hi @cpwc,

It's another heuristic that I think will be sufficient in most cases, but can still fall short in some cases. I would like for us to take our time and address this in a way that will be rather robust if possible.

I've opened a PR on the AWS plugin to scrub the references to confighelper.Trilean over there, so the name is processed adequately for this plugin, if you have experienced this on other plugins I would suggest opening an issue there if possible in order to fix usage.

I'll convert this PR into an issue so we can track an plan for this work directly in the SDK, hopefully we can use type analysis to figure out what we're dealing with.