Nil refence fix for issue #464

[madenwala]

This nil check is needed to prevent a terraform crash that occurs when a user tries to update a network rule that was manually changed through the Azure Portal. Fix is to check if properties on the props object is nil before you try to assign those string values to the sgRule dictionary.

[thiagocaiubi]

Hi @madenwala . Thanks for the work!

Why this PR was self-approved and why it lacks tests?

[madenwala]

Sorry about that, I'm new to this. I didn't know that I shouldn't submit the merge. Not exactly sure how to create a test case around this as the crash only occurs if you manually edit the fields through the Azure portal and then try to run your TF script again. The fix is very simple, just adding a nil check before you try to ready the property value.

[tombuildsstuff]

Taking a look at this retroactively, this change LGTM - however as discussed on Slack this shouldn't have been merged without a review and an acceptance test to cover it. I've enabled Branch Protection for the moment which requires a 👍 before merging - which should mean this can't happen in the future.

In addition - as discussed it'd be good to add an acceptance test for this in a follow-up PR - if you can reach out we can talk through the best way to achieve this :)

Thanks!

[tombuildsstuff]

Hey @thiagocaiubi

Why this PR was self-approved and why it lacks tests?

Thanks for raising this - in addition to @madenwala's comments I've followed up internally and have enabled Branch Protection on the master branch (which requires a 👍 from an admin) which should avoid this issue going forward.

Regarding the missing tests - I've reached out to @madenwala and we're going to discuss the options here and add these in a follow up PR.

Thanks!

[thiagocaiubi]

Awesome @tombuildsstuff! Great work! 👏

Thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!