feat(TPG>=6.7.0)!: promote secret_manager_config to GA

[wyardley]

Promote secret_manager_config (GA since 6.1.)
https://github.com/hashicorp/terraform-provider-google/releases/tag/v6.1.0

Fixes #2157

Note: this will likely fail now, but I imagine should be able to go in the following major after 34.0.0. After #2126 or another PR goes in that requires >= 6.1.0, this shouldn't be breaking on its own. Note: #2126 required >= 6.5.0 for autopilot, but because we'd excluded 6..5 and 6.6, this now is going to require 6.7 after all, I think, so making it a separate breaking change is the right thing.

[wyardley]

@apeabody do you want this marked as breaking or not (since it will already have to go out with a major release anyway)?

Is there an example this should be added to?

[apeabody]

@apeabody do you want this marked as breaking or not (since it will already have to go out with a major release anyway)?

Is there an example this should be added to?

Thanks @wyardley!

Let's mark as breaking (!) as it's always possible this PR could go in first. As it will go into a major release, the only real impact will be whether it is listed as a breaking change in the release notes.

Feel free to pick an appropriate existing example, I'd like to avoid adding new examples when possible give the impact to the CI test times.

[apeabody]

/gcbrun

[wyardley]

Let's mark as breaking (!) as it's always possible this PR could go in first

Just note that I didn't update the version specification in this PR -- do you want me to? I can easily enough, just will probably create conflicts with anything that requires a later 6.x, but I'll go ahead and do that - let me know if you want it rolled back.

Since we're probably excluding 6.6.0 and under already, this ends up needing >= 6.7.0 for non autopilot and >= 6.4.0 for autopilot, so the main change would be dropping 5.x. Should I update the title to >6.7.0?

[apeabody]

Let's mark as breaking (!) as it's always possible this PR could go in first

Just note that I didn't update the version specification in this PR -- do you want me to? I can easily enough, just will probably create conflicts with anything that requires a later 6.x, but I'll go ahead and do that - let me know if you want it rolled back.

Since we're probably excluding 6.6.0 and under already, this ends up needing >= 6.7.0 for non autopilot and >= 6.4.0 for autopilot, so the main change would be dropping 5.x. Should I update the title to >6.7.0?

If it's not a rush, we can just wait on it till one of the other >=6 PRs is merged. As you point out, that will be less merge conflicts to resolve. We can adjust the PR title at that point.

[wyardley]

Not a rush from my standpoint, especially since there's a workaround of using the beta variant for people who have this issue.

Might as well run it this way to see if the example test passes, and I'm happy to revert version changes or other stuff as you prefer.

One bad thing about adding this to a common example is that people might copy / paste it even if they don't need this value. I can put in a different section or add a comment if you think that would help, but this is an area where having some good examples of plan-based unit tests might help, as well as some way of running examples that aren't as prominently displayed in module docs?

[apeabody]

/gcbrun

[apeabody]

Not a rush from my standpoint, especially since there's a workaround of using the beta variant for people who have this issue.

Might as well run it this way to see if the example test passes, and I'm happy to revert version changes or other stuff as you prefer.

One bad thing about adding this to a common example is that people might copy / paste it even if they don't need this value. I can put in a different section or add a comment if you think that would help, but this is an area where having some good examples of plan-based unit tests might help, as well as some way of running examples that aren't as prominently displayed in module docs?

As a simple solution, I was thinking we could add a few new "complex" examples primarily for test coverage, these could even be done using test fixtures which are not prominently displayed in the docs. That said, probably the best starting place would be to curate the current examples (some may not longer be needed), and rewrite the remaining kitchen tests into CFT which can support plan-based tests.

[wyardley]

@apeabody looks like one of the modules in the safer_cluster_iap_bastion example is requiring a < 6.x google-beta provider, which I think is causing the lint error for that module. Something else that will probably have to get resolved before dropping 5.x provider support. The VPC module is lagging, but I don't think it's just that?

I think it may be the version specs in the instance_template modules for https://github.com/terraform-google-modules/terraform-google-bastion-host not getting updated here?

[apeabody]

@apeabody looks like one of the modules in the safer_cluster_iap_bastion example is requiring a < 6.x google-beta provider, which I think is causing the lint error for that module. Something else that will probably have to get resolved before dropping 5.x provider support. The VPC module is lagging, but I don't think it's just that?

I think it may be the version specs in the instance_template modules for https://github.com/terraform-google-modules/terraform-google-bastion-host not getting updated here?

Hi @wyardley - Might be solved in terraform-google-modules/terraform-google-bastion-host#213?

[wyardley]

Hi @wyardley - Might be solved in terraform-google-modules/terraform-google-bastion-host#213?

Ah, you're right! Took me a bit to untangle that reference, that those templates were coming from https://github.com/terraform-google-modules/terraform-google-vm

[apeabody]

Hi @wyardley - Might be solved in terraform-google-modules/terraform-google-bastion-host#213?

Ah, you're right! Took me a bit to untangle that reference, that those templates were coming from https://github.com/terraform-google-modules/terraform-google-vm

Yeah - I'm going to try and get that release out in the next day, a few simple PRs waiting.

[wyardley]

Was the failure here related to the test data not matching, or something else?

[apeabody]

Was the failure here related to the test data not matching, or something else?

Same as lint, during tf init failed to find a provider version to satisfy the constraints.

[apeabody]

/gcbrun

[apeabody]

@wyardley - Can you please rebase when you get a chance, thanks!

[wyardley]

Rebased

[apeabody]

/gcbrun

[apeabody]

Thanks for the contribution @wyardley!

[apeabody]

/gcbrun

[apeabody]

Thanks for the contribution @wyardley!