go get cloud.google.com/go/[email protected]

[SarahFrench]

Bumping the version of the cloud.google.com/go/storage dependency, used in the gcs backend.

This upgrade should allow users to use GCP identity federation, described in the linked issue below:

Possible fix for #29656

Replaces this out-dated PR: #30276

Target Release

1.3.x, 1.4.x

Draft CHANGELOG entry

ENHANCEMENTS

• backed/gcs: Updated storage package to v1.28.0

[SarahFrench]

Replacing this PR where I updated more dependencies than necessary: #32202

[SarahFrench]

I'm not sure what the solution is for the failing consistency test is (I've run make generate & make protobuf) - requested review for a bit of help!

[apparentlymart]

Hi @SarahFrench!

I'm just looking at the consistency check failure here first, since I don't think I have sufficient context to do a broader review of this (I'm not really familiar with the GCP backend).

I think we've run into something like this before when making changes to our protobuf tooling. I think what's happening here is that we have two different tools with different opinions about what this generated code ought to look like:

• The protobuf compiler is copying a comment verbatim from the source .proto file, leaving it formatted exactly how it was formatted in the source file.
• The go fmt command in recent Go versions has new opinions about how documentation comments should be formatted, and so it wants to rewrite some of those generated comments to meet the new style.

I think we'll need to eventually fix this properly by making the protobuf generation script also run the Go formatter on its results so that it's always generating code that will pass formatting.

In the meantime though, I looked back at the last PR where I encountered this (#31655) and it looks like I made it work by running go fmt ./... first and then running make protobuf so that the files in my PR were the style that the protobuf compiler thinks is correct, rather than the style that the go fmt tool thinks is correct. I must admit I'm not sure why that worked in retrospect, since I would expect the format check step to then fail instead, but since that did seem to work for me last time I'd suggest we try that first and see if it works. If not then we can consider other options.

[SarahFrench]

I just force pushed new commits after I checked out the latest commit on main (I preferred that idea vs handling merge conflicts in go.sum etc) and ran these commands using go1.19.3.

• go get cloud.google.com/go/[email protected]
• go mod tidy

I think me upgrading to go 1.19 helps with the issue above that led to failing checks- when I run make protobuf I get different comment formatting vs with go 1.18.

I'll wait to see if the checks on this PR want me to run make protobuf again

[SarahFrench]

@apparentlymart - Thanks for posting the above context, I wasn't aware of differences in Go 1.19's fmt! After I upgraded from 1.18 to 1.19 locally I've been able to get the current consistency checks to be happy 😀

Also, I believe that bumping this dependency is enough to address the issue I linked in the PR description and no further code changes are needed. The existing acceptance tests for the gcs backend pass when they are run off this PR's branch, so I don't think there's any need for a broader review that required knowledge of the gcs backend.

Is it ok for me to merge these changes into main?

[apparentlymart]

I'm not familiar enough with the GCP backend to be able to review this beyond the shallow level of "yes, this does indeed upgrade this dependency to v1.28.0" 😀 but I trust that you've tested it in the ways that make sense for this backend and so I've no objection to merging it.

[SarahFrench]

In terms of testing I've:

• Checked the package's release notes for breaking changes - saw none
• Run all the automated acceptance tests - all passed
• Built this branch locally and manually tested init/apply/import actions

I've edited the release note to be more conservative and just describe the release bump instead of the expected result that it helps with the issue linked in the PR description. I think there should still be a release note as the change is possibly user-facing if it fixes that issue.

With all this I'm as confident as I can be that the dependency bump isn't going to cause issues, and I've tried to avoid over promising the bump's effect on GCP identity federation (as I don't have the resources to test that fully).

[github-actions]

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.