Fix/types for idempotence

[Maarc-D]

Description

Linked to https://github.com/hashicorp/terraform-provider-google/issues/9353

Release Note Template for Downstream PRs (will be copied)

Fix(types for idempotence): transform advertised_routes to Set instead of List to keep idempotence on each plan when you have a dynamic block. But it will create a change displayed on plan for all user even the ones not using dynamic. this change should not have any impact as gcp side at the routes will remain the same

ToDo

Need to test it to be sure their is no impacts with https://registry.terraform.io/providers/public-cloud-wl/google/5.17.4 from https://github.com/public-cloud-wl/terraform-provider-google/commit/1a4d7e6ecb001cf279c502512133edda167f6f4a

[github-actions[bot]]

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@SarahFrench, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

[SarahFrench]

Just a heads up - this would be a breaking change for the field (see our guidance on this here) and would need to be included in an upcoming major release. We make exceptions if the 'breaking change' fixes something that is broken for all users but I don't think that is the case here.

[Maarc-D]

@SarahFrench In fact I would need to update the label because I tested It is not breaking change it will may say that it want to reorder everything so 1change but on apply everything is fine, tested and approved on my side, I will use my fork in the meaning time this one can be merge

What do you think I need to put, keep breaking-change or put something else like : bug ?

[SarahFrench]

@Maarc-D sorry for the delay, this PR slipped off my radar!

We'd need to double check what users will experience when upgrading from a previous version of the provider to a version including this change. Even if there's a new diff that's resolved with a single apply, navigating that diff might not work well for people with more complex configurations.

I'll trigger tests to run: /gcbrun

---

Edit: there is currently a build error occurring:

bundler: failed to load command: compiler.rb (compiler.rb)
/usr/local/lib/ruby/3.1.0/psych/class_loader.rb:99:in `find': Tried to load unspecified class: Api::Type::Set (Psych::DisallowedClass)

[Maarc-D]

@Maarc-D sorry for the delay, this PR slipped off my radar!

We'd need to double check what users will experience when upgrading from a previous version of the provider to a version including this change. Even if there's a new diff that's resolved with a single apply, navigating that diff might not work well for people with more complex configurations.

I'll trigger tests to run: /gcbrun

no problem I putted back as draft because we experienced few cases where this one does not resolve completely the idempotency issue :/ sometime still prompt more changes that it should. No working impact but it is on this cases still hard to make review before apply as it is now with list.

[LucaPrete]

Hello @SarahFrench this seems a huge pain for many of our large Google customers. Is there anything we can do to speed up the development/review? Happy to help!

BTW should we start to "undraft" this? :)

Thanks!

[SarahFrench]

Hi @LucaPrete - My review comments above describes the two routes forward for this PR/the original issue. @Maarc-D hasn't had enough time to respond to the review yet, but if this PR was to become stale then someone could open another PR informed from the discussion here.

[LucaPrete]

Thank you @SarahFrench ! It makes absolutely sense to me.

I'd proceed with both paths in parallel:

• I opened a new PR that uses the flatten function. Waiting for the tests to complete. I'll add you in cc.

• @Maarc-D can you please address the comments of @SarahFrench. We can then think to merge this once we cut a new major? @SarahFrench do we have timelines for it (assuming the PR will be complete)?

[SarahFrench]

@Maarc-D The 6.0.0 major release is coming up later this year. Do you want to proceed with this PR for that release?

[LucaPrete]

@SarahFrench @Maarc-D I opened this one since I've seen this wasn't moving forward and I wasn't able to commit to it. Please let me know if you change your mind and you're somehow able to finish this instead. In that case I'll abandon it.

[SarahFrench]

Given the lack of response I'll close this PR as stale, and we can move forward with the replacement PR

[Maarc-D]

Thank you @SarahFrench ! It makes absolutely sense to me.

I'd proceed with both paths in parallel:

• I opened a new PR that uses the flatten function. Waiting for the tests to complete. I'll add you in cc.
• @Maarc-D can you please address the comments of @SarahFrench. We can then think to merge this once we cut a new major? @SarahFrench do we have timelines for it (assuming the PR will be complete)?

I did not get time to deep dive on it with my day to day job stuffs increasing, I seen you've creating your own PR, great, hope it will fix.

But this problem is recurrent on multiple resources I have the same mess with consumer_accept_lists on google_compute_service_attachment

[SarahFrench]

Hi @Maarc-D - please open new GH issue(s) for any problems you're experiencing so that they can go through the triage process. Thanks!