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

Renaming IPWhiteList to IPAllowList #9457

Merged
merged 9 commits into from
Oct 26, 2022
Merged

Conversation

wxmbugu
Copy link
Contributor

@wxmbugu wxmbugu commented Oct 21, 2022

What does this PR do?

Renaming IpWhiteList to IpAllowList.

Motivation

Fixes #9437

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

@kevinpollet
Copy link
Member

Hello @Wambug and thanks for your contribution,

After a quick review, we saw that you missed renaming the ipwhitelist pkg itself and the corresponding source files. You also forgot to rename the menu item in the documentation (this should be done in the mkdocs.yaml).

Please, could you also ensure that the case is always the same as before? ipwhitelist should become ipallowlist, ipWhiteList should becomeipAllowList, and IPWhiteList should becomeIPAllowList.

@wxmbugu
Copy link
Contributor Author

wxmbugu commented Oct 21, 2022

sure lemme make the corrections👍

@kevinpollet
Copy link
Member

Thanks for updating the pull request, but you did not actually fix the case (e.g. https://github.com/traefik/traefik/pull/9457/files#diff-60c4d1d064d90b88b5a75e66f98620429a4a685cf91054164c96cb609d138b93R2 ipallowlist must be changed to IPAllowList).

Please, could you update the pull request?

@wxmbugu
Copy link
Contributor Author

wxmbugu commented Oct 24, 2022

ok

@kevinpollet kevinpollet changed the title Renaming IpWhiteList to IpAllowList Renaming IPWhiteList to IPAllowList Oct 26, 2022
Copy link
Member

@kevinpollet kevinpollet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@wxmbugu
Copy link
Contributor Author

wxmbugu commented Oct 26, 2022

anytime

Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@traefiker traefiker merged commit 1b9873c into traefik:master Oct 26, 2022
v2 automation moved this from To review to Done Oct 26, 2022
@peabnuts123

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@mrambossek
Copy link

may i ask why this was done? what new functionality does allowlist provide over whitelist?
it just caused me many hours of work because of broken systems.. primarily when watchtower auto-updated traefik to :latest on not-so-critical-systems

@rtribotte
Copy link
Member

Hello @mrambossek,

This is not expected, as you can see #10341 reintroduced the IpWhitelist middleware to ease migration.
Can you please open a dedicated issue and explain how to reproduce the issue?
Thanks!

@mrambossek
Copy link

unfortunately, i was not able to reconstruct this on the affected systems yet because .. well, they are in production :)
but i will check remaining systems and hopefully be able to get some logs out. i do remember seeing a message about ipwhitelist being deprecated, but i dont remember if it was a hard error or just a warning - i only remember routing didnt work. i will try to gather more data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v2
Done
Development

Successfully merging this pull request may close these issues.

rename(config/method): IPAllowList insteadof IPWhiteList
8 participants