locations: everything else

[dogboat]

This PR introduces Locations as a replacement for Endpoints. To migrate to using locations, first the migrate_endpoints_to_locations management command should be run, and then the V3_FEATURE_LOCATIONS setting should be set to True. This should be considered an irreversible process.

Once enabled, things should generally feel like business as usual, with a few changes to the UI and API. Most notably, the current /api/v2/endpoints and /api/v2/endpoint_status endpoints will return read-only versions of the data.

Some behind-the-scenes work has been done to migrate to the new terminology vis-à-vis variables, method names, etc., but a good amount of work remains to be done there; some renames cascade into the plumbing of Dojo's testing framework, and this PR is huge enough already.

Tests have been updated to run both against Endpoints and Locations.

Even though this feature took only one commit (amazing, right‽) a lot of the work done here actually belongs to Cody. For anything good, give him the credit; anything bad is probably my fault.

[mtesauro]

Approved

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[valentijnscholten]

Anything left TODO here?

[dogboat]

I skipped this test for Locations, it's not implemented for those; part of the reason being that I didn't apply the performance changes you'd been working on to locations, which I imagine we'll do here soon. At that point I would add test coverage for locations performance, so this is still a TODO. If you'd like me to, I could add the test now, but the numbers might make everybody sad hahaha.

[valentijnscholten]

If you add the bad numbers now you can feel good when you forward port the performance improvements and check the numbers again :-D
If you want me to extend the performance test I can do that.

[valentijnscholten]

The old code has the problem that the Endpoint table doesn't have a unique constraint. This leads to kind of complicated and sometimes expensive logic just to create endpoints during imports. There are situations in which duplicate can or could be created. There's also a warning in the code to log if duplicates are found. In the new code this warning seems to have been removed.

I would say that now is the time to fix the URL model to have a uniqueconstraint. I believe concatting all the fields would not work because they all have a high max length. I think a hash of all fields in a new "url_hash" field could work to make everything around inserts straightforward again. Updates might still be problematic if someone causes a URL to become identical to another URL. But I belivee this problem is already present right now. The update problem is also something that could be fixed later. The insert problem should be done right now as the migration also provides a way to cleanup/deduplicate.

[valentijnscholten]

I think the below also needs looking in to:

• The settings.dist.py still contains only settings for endpoints (hashcode/equality related). Should these be renamed/aliased for their Location counterparts? And/or docs/comments updated?
• dojo/jira/helper.py contains some code that uses/update endpoints which will no longer work when enabling locations
• dojo/tasks.py displays endpoint count which will no longer be accurate when enabling locations
• view_test.html, view_finding.html reference test_import.import_settings.endpoint instead of test_import.import_settings.locations
• display_tags.py reference import_settings.get("endpoints", get("endpoint")) but should reference locations

[dogboat]

The old code has the problem that the Endpoint table doesn't have a unique constraint. This leads to kind of complicated and sometimes expensive logic just to create endpoints during imports. There are situations in which duplicate can or could be created. There's also a warning in the code to log if duplicates are found. In the new code this warning seems to have been removed.

I would say that now is the time to fix the URL model to have a uniqueconstraint. I believe concatting all the fields would not work because they all have a high max length. I think a hash of all fields in a new "url_hash" field could work to make everything around inserts straightforward again. Updates might still be problematic if someone causes a URL to become identical to another URL. But I belivee this problem is already present right now. The update problem is also something that could be fixed later. The insert problem should be done right now as the migration also provides a way to cleanup/deduplicate.

I've added this as you've described: URLs (well, their hashes) are unique now. For updates I just throw a "hey this exists" message if there's a collision.

[dogboat]

* The `settings.dist.py` still contains only settings for `endpoints` (hashcode/equality related). Should these be renamed/aliased for their `Location` counterparts? And/or docs/comments updated?>

We wanted to keep the same settings names defined so users don't have to tweak those at all when migrating (compatibility). Behind the scenes, it uses the new locations stuff instead when appropriate.

It would certainly be nice to change all the terminology fully, and maybe we could somehow do it ("if v3 and hash_field_name in {'endpoints', 'locations'} do x" with warnings about the old one as we go forward?), but for now this is simpler.

* dojo/jira/helper.py contains some code that uses/update endpoints which will no longer work when enabling locations

* dojo/tasks.py displays endpoint count which will no longer be accurate when enabling locations

* view_test.html, view_finding.html reference test_import.import_settings.endpoint instead of test_import.import_settings.locations

* display_tags.py reference import_settings.get("endpoints", get("endpoint")) but should reference locations

Argh, sorry! Thanks! Fixed!

[valentijnscholten]

We wanted to keep the same settings names defined so users don't have to tweak those at all when migrating (compatibility). Behind the scenes, it uses the new locations stuff instead when appropriate.

It would certainly be nice to change all the terminology fully, and maybe we could somehow do it ("if v3 and hash_field_name in {'endpoints', 'locations'} do x" with warnings about the old one as we go forward?), but for now this is simpler.
Maybe add some (words to the)comment to make it clear it also applies to Locations (or URLs)?

[dogboat]

Maybe add some (words to the)comment to make it clear it also applies to Locations (or URLs)?

Good idea, done!