Proof of Concept Changes to support filtering flags by CIDR ranges

[adamhaney]

I wanted to sketch out what an API/changeset might look like for #37

TODO:

• The ipaddress library is added in Python 3.3 and a backport is available on PyPi for 2.7 users, what should the policy for optional dependencies be for 2.7 users?
• django-ipware (https://github.com/un33k/django-ipware) is definitely an external dependency but would only be needed if the CIDR filtering was desired, would it make sense to do a try/except on import to allow it to optionally be installed?
• Testing plan, I hacked this together pretty quickly to get the conversation going, it definitely needs more thought before being merged, testing being the highest on the list of things to think through.
• Generate/Write migrations for field changes

[adamhaney]

Oh, one important one that I left out, there aren't any migrations yet. It doesn't look like there's a test project in the repo so I was wondering if any of the contributors could provide any guidance/docs on how to generate migrations? Or does everyone usually just hand code them?

[clintonb]

extra_requires is useful for specifying additional dependencies. See https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies. For ipaddress, update the docs noting the use of the dependent library for Python 2.7, and informing users to install with pip install django-waffle[ip] (or something along those lines). A similar setup extra could be used for ipware, but this would be applicable to all versions of Python.

Definite 👍 to more testing. I'm not sure if RequestFactory can mock IP addresses. Although, if I understand this correctly, it's a header, so it should be doable.

test_settings.py can be used to generate new migrations. Run the command below in the repo root:

PYTHONPATH=. DJANGO_SETTINGS_MODULE=test_settings django-admin makemigrations

[clintonb]

It would be ideal if we had some method of determining when the external dependencies are missing, and notifying the user via admin UI and management command output.

[clintonb]

Add validation on these fields to ensure the ranges are real.

[clintonb]

Remove.

[jsocol]

I'd love to see CIDR ranges supported. I've dealt with client IP a lot in django-ratelimit as well, and I'd consider possibly moving this to something like waffle.ext....

As ipware's readme indicates, correctly determining client IP behind a proxy is super environment-specific (this is why Django itself dropped the SetRemoteAddrFromForwardedFor middleware all the way back in 1.1). I'd be strongly tempted to defer that to application code (or another library if you want to configure ipware).

Maybe make the protocol something like: set a value on the request (possibly request.META['WAFFLE_IP'] or something) and Waffle will use that value, falling back to request.META['REMOTE_ADDR'].

Then you could provide middleware e.g. waffle.ext.middleware.WaffleClientIPMiddleware that depends on ipware (but only add it as an optional dependency, and add a system check to the middleware module for it) and sets request.META['WAFFLE_IP'].

That way users could also optionally write their own middleware to set the value based on their environments. For example, on my own projects I usually like to use something like X-Client-IP/Proto or Cluster-Client-IP/Proto from the edge, to avoid dealing with parsing XFF correctly, so I could write a small middleware to copy HTTP_X_CLIENT_IP to WAFFLE_IP.

It's also probably worth scoping these fields to IPv4 to enable expansion to IPv6 ranges in the future