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.

Sign up for GitHub

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

Jump to bottom

Fix an overflow on retries on container name conflicts #4752

Merged
merged 2 commits into from May 2, 2023
Merged

Fix an overflow on retries on container name conflicts #4752

merged 2 commits into from May 2, 2023

Commits on Apr 24, 2023

  1. Rename conflict to suffixDigitsModulo 

    to reflect a bit better what this does.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Apr 24, 2023
    Copy the full SHA
    07fda81 View commit details
    Browse the repository at this point in the history

  2. Cap suffixDigitsModulo to a 9-digits suffix. 

    Because Go integers silently overflow, and 10 is multiple of 2,
repeatedly multiplying by 2 eventually turns suffixDigitsModulo
into all-bits-zero, triggering a division by zero.

So, cap the suffix to a 9-digit one. That is very likely to be sufficient.

(But don't actually limit the number of retries. We could do that as well,
for extra robustness. OTOH that would be another bit of code that we don't
test...)

[NO NEW TESTS NEEDED]
We could plausibly unit-test the loop just to see that it doesn't crash
on 100 conflicts in a row, but that's an awfully specific test that would
require moving the loop to a separate function with a closure parameter,
a complex test that makes the code less readable for a very specific crash.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Apr 24, 2023
    Copy the full SHA
    4775b24 View commit details
    Browse the repository at this point in the history