Make exclude_folders and exclude_projects use sets and use for_each to avoid unnecessary resource updates

[MajorBreakfast]

Currently, when adding new excluded folders or projects in to the middle of the list, Terraform shows unnecessary changes due to index shifts. With this change, this no longer happens. This makes it possible to order the exclusions alphabetically without getting unnecessary changes in the plan.

[MajorBreakfast]

Could you provide me the details why this does not pass? I don't have access

[MajorBreakfast]

@bharathkkb could you maybe have a look again? ^^'

[bharathkkb]

@MajorBreakfast thanks for the contribution

It is failing due to a problem on our end that we are working to resolve. @morgante PTAL

[MajorBreakfast]

Any progress on this? The reshuffling makes it so that we get 409 errors whenever we add an exclusion to the middle of the list. That's inconvenient

[bharathkkb]

@MajorBreakfast CI problems on our end have been resolved and since tests were not passing I tried debugging- left some comments with my findings.
you can execute the tests locally as described here

[MajorBreakfast]

Tests are green now

[bharathkkb]

@MajorBreakfast LGTM overall. nit: I assume the locals like exclude_folders_list_length are not used anymore? They are still in main.tf

[MajorBreakfast]

Yep. The search confirms that they‘re unused. I removed them

[MajorBreakfast]

@morgante can you have a look?

[MajorBreakfast]

@bharathkkb ping

[bharathkkb]

@morgante PTAL

[morgante]

Sorry for the delay on this.

My main concern here is that by switching to for_each we will make it impossible to reference dynamically created folders/projects. ie it will be impossible to create a folder and apply an org policy to it in the same execution.

[MajorBreakfast]

@morgante The old code computed the length via length(compact(var.exclude_folders)) which means the length was also computed before. Are you sure that this change could break in more scenarios? And if so, could you provide an example?

[morgante]

You're right, it should have the same behavior now.

[MajorBreakfast]

@morgante This PR might have a problem. The size of the set depends on how many unique values there are. This means the size cannot be known statically if one or more entires are only known dynamically (e.g. in case it comes from a resource attribute). This makes the value not allowed in a for_each. I haven't looked deeply into this, but it I just came across a similar scenario and wanted to mention it here.

[morgante]

Yeah that's unfortunately true. I don't think there's any easy fix though.