search

PaloAltoNetworks / terraform-aws-vmseries-modules

Terraform Reusable Modules for VM-Series on AWS
https://registry.terraform.io/modules/PaloAltoNetworks/vmseries-modules/aws
MIT License
40 stars 49 forks source link

Expose api termination protection as variable #322

Closed wouldd closed 1 year ago

wouldd commented 1 year ago

Is your feature request related to a problem?

We're looking at building out our firewall instances and would really like to enable termination protection given the importance these will have in our network. Currently your terraform is hardcoding that value to false in: modules/vmseries/main.tf:73

I don't think we can override this anyway that won't have terraform trying to flip it back to false everytime we run it.

Describe the solution you'd like

As with other settings, please expose a new input variable that defaults to false but can be set, eg 'enable_instance_termination_protection'

Describe alternatives you've considered.

Since you're explicitly setting this value I don't think there is anything else I can do short of manually setting the protection on after creation and never running the terraform again. this feels likely to cause problems in the long term. I guess I could fork this repo but I'd rather not.

Additional context

Whilst not a huge risk, it is certianly possible for someone to accidentally issue a termination command, either because they briefly confuse it with shutdown, or because they don't realise they have more than once instance selected when they are intentionally terminating another (ask me how I know ;-))

I'd obviously be happy to just default to forcing termination protection on if that is easier, but a variable should be straight forward. I would be happy to submit a PR for it if that helps

welcome-to-palo-alto-networks[bot] commented 1 year ago

:tada: Thanks for opening your first issue here! Welcome to the community!

pimielowski commented 1 year ago

Hello @wouldd, thanks for addressing this issue! It makes sense to lease this option to a variable with default value. We can do it or as you said at the end of your message you can create PR to this issue to contribute!

wouldd commented 1 year ago

Great, I have a branch with the change on but I don't have permission to publish it. I'm not sure exactly what your process would normally be? I would normally publish a branch and raise a PR from that. But if you let me know the best way to submit the change I'll happily do that. Many thanks

wouldd commented 1 year ago

nevermind, sorry first time raising a PR against a repo that isn't mine already ;-) I've created a fork and raised the PR. let me know if I needed to update anything else.