Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to go 1.17 and bumps golangci-lint from 1.18.0 to 1.50.1 #1370

Merged
merged 3 commits into from
Nov 17, 2022

Conversation

nickfloyd
Copy link
Contributor

The dependency golangci-lint requires go v 1.17. This PR bumps that version as well as updates the vulnerable package.

NOTE: I'm not sure if this is a breaking change, but since CI uses go 1.19 I think we're ok. maintainers will need to update Go if they are currently @ 1.16


Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features) - N/A
  • Docs have been reviewed and added / updated if needed (for bug fixes / features) - N/A, release notes will reflect this change
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

@kfcampbell please verify that this would not be considered a breaking change

If Yes, what's the impact:

  • N/A

@nickfloyd nickfloyd added Priority: Normal Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR dependencies Pull requests that update a dependency file labels Nov 17, 2022
@kfcampbell
Copy link
Member

Confirming that switching to Go 1.17 will not be a breaking change for consumers of this plugin. Developers will need to have Go 1.17 at a minimum installed, which they should anyways. Go supports the last two minor versions, so 1.16 isn't receiving patches anymore.

This lint failure is obnoxious. I can reproduce it locally using the currently-specified version of golangci/golangci-lint and the latest version. There's a thread about similar issues here, here and here. Those threads seem to suggest that there's a missing dependency or perhaps a C library expected, though I don't see how that's possible given that the project builds just fine and the error is reproduceable with CGO_ENABLED=0.

Another thing I tried is running go clean -modcache and then re-running go mod tidy and go mod vendor to see if we had a corrupted go.sum file somehow. Doing so does not show any file changes. I also thought the change might be related to Go 1.19 support, but that was added to golangci-lint in v1.48.0.

I'll have to keep thinking about this one.

@kfcampbell
Copy link
Member

Aha! The error is a real one, but it's in the testing code and not the building code which is why builds work fine still.

 sh$ make test
go test ./...
# github.com/hashicorp/terraform-plugin-sdk/internal/configs/configload
vendor/github.com/hashicorp/terraform-plugin-sdk/internal/configs/configload/loader_snapshot.go:52:30: cannot use fs (variable of type snapshotFS) as type afero.Fs in argument to configs.NewParser:
	snapshotFS does not implement afero.Fs (missing Chown method)
vendor/github.com/hashicorp/terraform-plugin-sdk/internal/configs/configload/loader_snapshot.go:57:32: cannot use fs (variable of type snapshotFS) as type afero.Fs in struct literal:
	snapshotFS does not implement afero.Fs (missing Chown method)
vendor/github.com/hashicorp/terraform-plugin-sdk/internal/configs/configload/loader_snapshot.go:200:18: cannot use snapshotFS{} (value of type snapshotFS) as type afero.Fs in variable declaration:
	snapshotFS does not implement afero.Fs (missing Chown method)
?   	github.com/integrations/terraform-provider-github/v5	[no test files]
FAIL	github.com/integrations/terraform-provider-github/v5/github [build failed]
FAIL
make: *** [GNUmakefile:27: test] Error 2

I'm still exploring how we might fix this.

@kfcampbell
Copy link
Member

Okay, so here's the deal: we're dependent on an old version of hashicorp/terraform-provider-sdk. We can and should update that in the near future.

That version of hashicorp/terraform-provider-sdk is dependent on v1.4.1 of spf13/afero. v1.5.0 of spf13/afero introduced the chown method to an interface that the terraform-plugin-sdk depends on.

We can pin to v1.4.1 of spf13/afero (released on 6 October 2020) by running go get github.com/spf13/[email protected] and subsequently go mod tidy && go mod vendor. In the near future, I think that's the best way forward until we can update hashicorp/terraform-provider-sdk to the next major version.

@nickfloyd would you like me to push the changes up to this branch or would you like to do it?

@nickfloyd
Copy link
Contributor Author

@nickfloyd would you like me to push the changes up to this branch or would you like to do it?

Solid sleuthing! Feel free to make the change given you're already in there. Thanks for helping me get this one sorted!

@kfcampbell kfcampbell merged commit bb0f993 into main Nov 17, 2022
@kfcampbell kfcampbell deleted the update-go-1-17 branch November 17, 2022 22:48
kazaker pushed a commit to auto1-oss/terraform-provider-github that referenced this pull request Dec 28, 2022
…egrations#1370)

* Updates to go 1.17 and bumps golangci-lint from 1.18.0 to 1.50.1

* Pin to spf13/afero v1.4.1

* README now reflects updated Go version

Co-authored-by: Keegan Campbell <[email protected]>
avidspartan1 pushed a commit to avidspartan1/terraform-provider-github that referenced this pull request Feb 5, 2024
…egrations#1370)

* Updates to go 1.17 and bumps golangci-lint from 1.18.0 to 1.50.1

* Pin to spf13/afero v1.4.1

* README now reflects updated Go version

Co-authored-by: Keegan Campbell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants