[BUG] Make NetworkID and FirewallID Mandatory for Instance resource

[uzaxirr]

Description

context: https://civo-community.slack.com/archives/C0186KTRLPP/p1728269229482269?thread_ts=1728189777.364919&cid=C0186KTRLPP

Acceptance Criteria

• If any instance is being created without network or firewall id then it should error out.

[dipu989]

hello @uzaxirr @alessandroargentieri
I would like to pick this up.

I went through the codebase of terraform-provider-civo. Here's my understanding of this issue and how I am approaching this problem.

My Understanding -

I see in the resource_instance.go file, network_id is marked as Optional.

While firewall_id is marked as a required field. In case it is not set, the default firewall will be used. (As per description)

Then, I check the resourceInstanceCreate() method, in case network_id is not passed, we fetch the default network and set the networkID.

While in case of firewall_id, we just check if it's present, we set the instance firewall.

My proposed solution and doubts -

1. Make the network_id as Required : true instead of Optional : true
2. Doubt - We are checking for the firewall_id after createInstance(config) call is made (See the below attached screenshot) So should we perform this check before calling the createInstance() method? Will that help? And in case firewall_id is not set, do we return an error as per the issue description?

Another question - If we were already setting default values for both firewall_id and network_id when they weren't passed, why did this issue occur in the first place?

Sorry for these many doubts, want to have full context for my understanding.

[uzaxirr]

hey @dipu989
Yes you can Make the network_id as Required : true instead of Optional : true
coming on to your doubt, we are setting up firewall_id after instance creation bcz its an separate API call which is kind of update instance call, it does not happen during instance creation https://api.civo.com/v2/instances/:id/firewall this API call happens behind the scene. refer to https://www.civo.com/api/instances for more details.

[dipu989]

Will take a look at the docs 🫡
Thanks for reverting back @uzaxirr