removes ioutil usage everywhere which was deprecated in go1.16 #15297

kschoche · 2022-11-08T21:25:49Z

Description

This is a cleanup PR that I'm testing, it would be sweet if it passed all the unit tests :-D

Deprecated: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details.
https://pkg.go.dev/io/ioutil

This PR also introduces a new rules to the golangci-lint tool which forbids the use of the ioutil package and will result in output like this:

demo $ make lint
--> Running golangci-lint
command/logout/logout_test.go:34:12: use of `ioutil.ReadFile` forbidden because "Use io and os packages instead of ioutil" (forbidigo)
        _, err := ioutil.ReadFile("abcd")
                  ^
make: *** [lint] Error 1

PR Checklist

updated test coverage
external facing docs updated
not a security concern

kschoche · 2022-11-09T16:01:27Z

@dhiaayachi I'm not sure if this one's been proposed before, but thought I'd see if there was any interest in this PR. Thoughts?

dhiaayachi

Thank you for working on this @kschoche!
Seems like your experiment is working 🚀
I added a comment about a commented code that I think we should remove.

I also have a suggestion to avoid regression for this (as long as ioutils is existing in golang). Maybe we can use something like this https://github.com/le0tk0k/noioutil to vet the code in the CI but feel free to merge this as is and have this in a subsequent PR.

command/logout/logout_test.go

rboyer · 2022-11-09T21:22:43Z

You might be able to use the forbidigo golanglint-ci configuration to prohibit calls: https://github.com/hashicorp/consul/blob/main/.golangci.yml#L85

kschoche · 2022-11-09T21:49:04Z

You might be able to use the forbidigo golanglint-ci configuration to prohibit calls: https://github.com/hashicorp/consul/blob/main/.golangci.yml#L85

Ahh this is a great find, thanks!

--> Running golangci-lint
command/logout/logout_test.go:34:12: use of `ioutil.ReadFile` forbidden because "Use io and os packages instead of ioutil" (forbidigo)
        _, err := ioutil.ReadFile("abcd")
                  ^
make: *** [lint] Error 1

rboyer · 2022-11-09T22:11:36Z

.changelog/15297.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+all: Remove the use of the deprecated ioutil package throughout the codebase in favour of io and os packages which are still maintained and also introduce a lint rule which forbids the use of ioutil in commits.


Many times this sort of fixup doesn't necessitate a changelog entry. Your call if you think we should have one.

Makes sense! I was on the fence about it, so I'll remove it then, I think the commit message should be good enough!

rboyer · 2022-11-09T22:15:35Z

api/acl.go

@@ -1047,7 +1046,7 @@ func (a *ACL) RulesTranslate(rules io.Reader) (string, error) {
 	parseQueryMeta(resp, qm)
 	qm.RequestTime = rtt

-	ruleBytes, err := ioutil.ReadAll(resp.Body)
+	ruleBytes, err := io.ReadAll(resp.Body)


Usually we handle go version compatibility differently in the api package so that folks using the library aren't forced to upgrade to a future version before they're ready. I think the policy seems to be assuming folks are using a go version that is still receiving security updates (which 1.16 is definitely NOT anymore).

So in this case it's fine to do the swap. I wonder if we should correlate this with bumping the go directive in api/go.mod from 1.12 to at least 1.16 or maybe all the way to 1.18 (which is the oldest version we test against in CI).

The same logic would apply to sdk/go.mod too.

👍🏽 Can do, I think 1.18 makes sense here then.
There is a ton of module tidying, save that for another PR?

* update go version to 1.18 for api and sdk, go mod tidy * removes ioutil usage everywhere which was deprecated in go1.16 in favour of io and os packages. Also introduces a lint rule which forbids use of ioutil going forward. Co-authored-by: R.B. Boyer <[email protected]>

…#17243) * no-op commit due to failed cherry-picking * security: update go version to 1.20.4 (#17240) * update go version to 1.20.3 * add changelog * rename changelog file to remove underscore * update to use 1.20.4 * update change log entry to reflect 1.20.4 * upgrading to 1.20 * [OSS] security: update go to 1.20.1 (#16263) * security: update go to 1.20.1 * fixing auto_config_endpoint_test that was merged incorrectly * go mod tidy * fixing auto_config_endpoint_test that was merged incorrectly * updating linter to 1.51.1 * go mod tidy on api * go mod tidy * removes ioutil usage everywhere which was deprecated in go1.16 (#15297) * update go version to 1.18 for api and sdk, go mod tidy * removes ioutil usage everywhere which was deprecated in go1.16 in favour of io and os packages. Also introduces a lint rule which forbids use of ioutil going forward. Co-authored-by: R.B. Boyer <[email protected]> * go mod tidy * getting rd of net in imports * get rid of use of math.rand * get rid of use of math/rand in audo_config_endpoint_test.go * update leader test --------- Co-authored-by: temp <[email protected]> Co-authored-by: John Murret <[email protected]> Co-authored-by: Dan Stough <[email protected]> Co-authored-by: Kyle Schochenmaier <[email protected]>

removes ioutil usage everywhere which was deprecated in go1.16

54ce366

Merge branch 'main' into remove_deprecated_ioutil

dc94561

kschoche added the pr/no-changelog PR does not need a corresponding .changelog entry label Nov 8, 2022

vercel bot deployed to Preview – consul November 8, 2022 21:32 View deployment

fix merge mess

868cc06

kschoche requested review from dhiaayachi and DanStough November 9, 2022 16:00

dhiaayachi approved these changes Nov 9, 2022

View reviewed changes

command/logout/logout_test.go Outdated Show resolved Hide resolved

Merge branch 'main' into remove_deprecated_ioutil

b06df06

remove unnecessary comment and update golangci.yml

3526de4

kschoche changed the title ~~[wip] removes ioutil usage everywhere which was deprecated in go1.16~~ removes ioutil usage everywhere which was deprecated in go1.16 Nov 9, 2022

kschoche marked this pull request as ready for review November 9, 2022 21:54

add a changelog

068b70b

kschoche removed the pr/no-changelog PR does not need a corresponding .changelog entry label Nov 9, 2022

kschoche requested a review from rboyer November 9, 2022 22:02

rboyer reviewed Nov 9, 2022

View reviewed changes

remove changelog message

ae8daa0

kschoche added the pr/no-changelog PR does not need a corresponding .changelog entry label Nov 9, 2022

rboyer reviewed Nov 9, 2022

View reviewed changes

kschoche added 3 commits November 9, 2022 16:46

update go to 1.18

fe593bf

go mod tidy

0540f9e

add changelog again

123e36a

kschoche removed the pr/no-changelog PR does not need a corresponding .changelog entry label Nov 10, 2022

Update 15297.txt

232a245

rboyer approved these changes Nov 10, 2022

View reviewed changes

Merge branch 'main' into remove_deprecated_ioutil

70a4329

vercel bot deployed to Preview – consul November 10, 2022 16:04 View deployment

kschoche merged commit bf0f61a into main Nov 10, 2022

kschoche deleted the remove_deprecated_ioutil branch November 10, 2022 16:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removes ioutil usage everywhere which was deprecated in go1.16 #15297

removes ioutil usage everywhere which was deprecated in go1.16 #15297

kschoche commented Nov 8, 2022 •

edited

Loading

kschoche commented Nov 9, 2022

dhiaayachi left a comment

rboyer commented Nov 9, 2022

kschoche commented Nov 9, 2022

rboyer Nov 9, 2022

kschoche Nov 9, 2022

rboyer Nov 9, 2022 •

edited

Loading

rboyer Nov 9, 2022

kschoche Nov 9, 2022

		@@ -0,0 +1,3 @@
		```release-note:improvement
		all: Remove the use of the deprecated ioutil package throughout the codebase in favour of io and os packages which are still maintained and also introduce a lint rule which forbids the use of ioutil in commits.

removes ioutil usage everywhere which was deprecated in go1.16 #15297

removes ioutil usage everywhere which was deprecated in go1.16 #15297

Conversation

kschoche commented Nov 8, 2022 • edited Loading

Description

PR Checklist

kschoche commented Nov 9, 2022

dhiaayachi left a comment

Choose a reason for hiding this comment

rboyer commented Nov 9, 2022

kschoche commented Nov 9, 2022

rboyer Nov 9, 2022

Choose a reason for hiding this comment

kschoche Nov 9, 2022

Choose a reason for hiding this comment

rboyer Nov 9, 2022 • edited Loading

Choose a reason for hiding this comment

rboyer Nov 9, 2022

Choose a reason for hiding this comment

kschoche Nov 9, 2022

Choose a reason for hiding this comment

kschoche commented Nov 8, 2022 •

edited

Loading

rboyer Nov 9, 2022 •

edited

Loading