[release-5.24] Backport #2418 + #2453 #2548
Conversation
mtrmac
force-pushed
the
digest-unmarshal-5.24
branch
3 times, most recently
from
2c3acb6 to
810e473
Compare
module-unaware installation is now failing because it is trying to use the latest version of golang.org/x/tools, and that doesn't build against Go 1.18 used on this branch. Instead, use a module-aware installation, to use the golint-designated version of the dependency. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This is causing test failures with recent toolchains. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This makes the test a tiny bit more realistic, and avoids a linter warning. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
If doing it makes sense at all, it should happen before the values are used. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to prevent panics if the value does not contain a :, or other unexpected values (e.g. a path traversal). Don't bother on paths where we computed the digest ourselves, or it is already trusted for other reasons. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use defer() to remove the temporary file, instead of duplicating the call. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use defer, a nested function, and early returns. Besides being a bit more directly related to what we want to achieve, this now does not call decompressed.Close() on a nil value if DecompressStream fails. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... to prevent unexpected behavior on invalid values. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Per containers/skopeo#2346, that happens in the wild. Signed-off-by: Miloslav Trmač <mitr@redhat.com> Signed-off-by: tomsweeneyredhat <tsweeney@redhat.com>
mtrmac
force-pushed
the
digest-unmarshal-5.24
branch
from
b1a6279 to
a92d2c7
Compare
|
LGTM |
| @@ -23,7 +24,7 @@ func TestReadAtMost(t *testing.T) { | |||
| {bytes.MinRead*5 + 1, bytes.MinRead * 5, false}, | |||
| } { | |||
| input := make([]byte, c.input) | |||
| _, err := rand.Read(input) | |||
| _, err := rng.Read(input) | |||
| require.NoError(t, err) | |||
| result, err := ReadAtMost(bytes.NewReader(input), c.limit) | |||
| if c.shouldSucceed { | |||
There was a problem hiding this comment.
In 521155f, the Makefile had a slight change at line 58, bumping lint from v1.49.0 to v1.51.0. Is that not needed here?
There was a problem hiding this comment.
The CI environment we have is running Go 1.18., so the linter update to support Go 1.20 is not necessary.
(I’m not claiming that Go 1.18 is the right Go version to require on this branch, I didn’t investigate that in detail — it’s just a version that happens to be used right now, based on IMAGE_SUFFIX in .cirrus.yml I think.)
It’s quite possible that various backported lint cleanups are not necessary either, they were just making my life easier when backporting (because my development system has golangci-lint 1.60.2 installed).
There was a problem hiding this comment.
… ah, perhaps a better way to actually answer the question:
I backported the lint warning cleanup because it was annoying me in local development with a recent linter. I didn’t backport the linter version bump itself because I thought that’s undesirable on a stable branch.
Arguably my local development setup which motivates me to backport unnecessary cleanups is a problem that should be fixed; and strictly speaking, maybe this lint backport should be reverted as unnecessary on the stable branch as well.
There was a problem hiding this comment.
If you've run into lint issues in your development branch, I'd rather have them in the stable branch in case they help someone else later. I'd leave them as is.
I'm fine with not bumping the version; I wasn't certain it was necessary, given that this is an older branch.
|
Just one question on the Makefile at line 58 for the Golint bump from 1.49 to 1.51. Otherwise LGTM. TYVM @mtrmac ! I'll create tags and releases in the morning. |
#2547 + #2546 exist on later branches to maintain continuity.