search

hashicorp / packer-plugin-alicloud

Packer plugin for Alibaba Cloud Image Builder
https://www.packer.io/docs/builders/alicloud-ecs
Mozilla Public License 2.0
12 stars 21 forks source link

Fix naming conflict for ram_role_name #107

Closed bittopaz closed 1 year ago

bittopaz commented 1 year ago

Currently ram_role_name is used in two places:

  1. ram_role_name (string) - Alicloud RamRole must be provided for EcsRamRole mode unless profile is set.
  2. ram_role_name (string) - Ram Role to apply when launching the instance.

In reality, they don't need to be the same role. We should be able to use RAM role A to launch a running instance which has RAM role B attached to.

Meanwhile, I'm open to the naming of this field, instance_ram_role is the best name that I could think of for this field. Please feel free to make a suggestion.

Thank you!

bittopaz commented 1 year ago

Hello @nywilken, could you please review this small PR?

alexyueer commented 1 year ago

I think the same as you do about this. #105

bittopaz commented 1 year ago

@alexyueer ah, I wasn't aware of that PR. Thanks for referring it here.

bittopaz commented 1 year ago

Gentle ping @lbajolet-hashicorp

lbajolet-hashicorp commented 1 year ago

Hi @bittopaz, I'll continue this discussion in @alexyueer's PR if you don't mind, the contents of both your PRs are similar, I think we'll benefit from only having one.

I'll close this one so we can all focus on #105, feel free to reopen this one if you think we should continue work here.

Thanks!

bittopaz commented 1 year ago

@lbajolet-hashicorp Thank you for the clarification, it's fine as long as this issue gets fixed.

Besides, would you mind to create a release for this? I need this fix in my production.

lbajolet-hashicorp commented 1 year ago

Hi @bittopaz,

We'll probably release this week, because of the fix being a breaking change, we'll try to proceed carefully here. If possible, I'd like to see #110 merged before the release, this way we can address all the conflicts at once, and continue developments on a sane base for this plugin.

Sorry for making you wait, we'll let you know when the release is ready.