Automate mirror submission #448

Torxed · 2023-02-17T10:51:54Z

Concept so far:

Public endpoint that doesn't require an arch account
Captcha to combat most spam
When submitted, the mirror is default Active: False and Public: False. These change to True when:
- ~~Upon (automated?) verification of Completion: 100.0%~~
- After email to maintainers is sent and they manually change the two values after inspection.
Tier 1 requests gets automatically rejected (with a nice email explaining why) if:
- No valid Tier 2 in the database matching the same name and URL endpoints.
- Tier 2 is newer than registered < MIN days
- Tier 2 has a log containing multiple suspicious errors

…ng pypi instead

…ecks

jelly

Cool! I've made some minor comments in the PR. Would be nice to also add a test later, happy to help if you have some questions.

.gitignore

requirements.txt

templates/mirrors/mirror_submit.html

mirrors/views/mirrorlist.py

jelly · 2023-02-18T13:40:22Z

templates/mirrors/mirror_submit.html

+
+    <ul>
+        <li><a href="/mirrors/tier/1/">Tier 1 mirrors</a></li>
+        <li><a href="/mirrors/tier/2/">Tier 2 mirrors</a></li>


ideally this uses the url template tag

Good idea, I'll learn how those work and see if I can get it working.

Not sure how to work around the fact that the tier url's use re_path(r'^tier/(?P<tier>\d+)/$', mirrors, name='mirror-list-tier'), with one common name (I assume that's how those links work after reading up).

templates/mirrors/mirror_submit.html

Torxed · 2023-02-18T17:11:34Z

Cool! I've made some minor comments in the PR. Would be nice to also add a test later, happy to help if you have some questions.

Thanks for the feedback, me and the misses will get onto fixing the last few things before I mark this as ready instead of WIP.

The whole Django thing is still new to me and I have no idea where things are stored in the db 🙈
Tests are coming too, once I know it's all working in manual tests.

Torxed · 2023-02-18T17:57:33Z

Oh, and I will run a code linter once I feel ready. Just can't stand the syntax while developing :)

Torxed · 2023-02-18T20:33:09Z

Mail function looks good:

Aside from automatic check to make the mirror public after certain criterias, I think the PR is mostly ready.

… should be ok as some errors such as faulty DNS lookups do occur on some hosts momentarily. That we have to account for

Torxed · 2023-02-18T20:36:57Z

@jelly I consider the PR ready, as I might need help with writing tests for this.
But as mentioned, I can't figure out why the pip+git breaks (even in the runner):

Obtaining cssmin from git+git://github.com/fredj/cssmin.git@master#egg=cssmin (from -r requirements.txt (line 1))
  Cloning git://github.com/fredj/cssmin.git (to revision master) to ./src/cssmin
  Running command git clone --filter=blob:none --quiet git://github.com/fredj/cssmin.git /home/runner/work/archweb/archweb/src/cssmin
  fatal: unable to connect to github.com:
  github.com[0: 140.82.112.3]: errno=Connection timed out

  error: subprocess-exited-with-error
  
  × git clone --filter=blob:none --quiet git://github.com/fredj/cssmin.git /home/runner/work/archweb/archweb/src/cssmin did not run successfully.
  │ exit code: 128
  ╰─> See above for output.
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× git clone --filter=blob:none --quiet git://github.com/fredj/cssmin.git /home/runner/work/archweb/archweb/src/cssmin did not run successfully.
│ exit code: 128
╰─> See above for output.

Torxed · 2023-02-18T21:36:16Z

mirrors/tests/test_mirror_submission.py

@@ -0,0 +1,3 @@
+def test_mirror_registration(client, mirrorurl):
+    response = client.get('/mirrorlist/submit/?name=test3&tier=2&upstream=1&admin_email=anton%40hvornum.se&alternate_email=&isos=on&rsync_user=&rsync_password=&notes=&active=True&public=True&url1-url=rsync%3A%2F%2Ftest3.com%2Farchlinux&url1-country=SE&url1-bandwidth=1234&url1-active=on&url2-url=&url2-country=&url2-bandwidth=&url2-active=on&url3-url=&url3-country=&url3-bandwidth=&url3-active=on&ip=&captcha_0=d5a017cc3851fb59898167f666759c99b42afd52&captcha_1=tdof')


It's a crude test, but will at least test that the endpoint exists and that it accepts the request data (captcha will fail)

Torxed · 2023-02-19T17:02:25Z

A thought that hit me while administrating the manual process was that adding a bunch of URL's to the same mirror might be desirable.

That currently won't work with this approach, but very well could in v2.0 of this endpoint.
I feel it's a bit of an undertaking adding support for 3+ URL's, especially dynamically added URL's.
So if no one objects, I'd prefer if this goes in before I start working on v2.0 of this.

jelly · 2023-03-12T14:26:16Z

This PR still has CI issues.

jelly

Please / rebase cleanup the comments and I'll take another look

jelly · 2023-03-12T14:27:23Z

requirements.txt

@@ -1,4 +1,4 @@
-e git+git://github.com/fredj/cssmin.git@master#egg=cssmin
+-e git+https://github.com/fredj/cssmin.git@master#egg=cssmin


Are you on an old branch? On master it's

1 │ -e git+https://github.com/fredj/cssmin.git@master#egg=cssmin

jelly · 2023-03-12T14:30:39Z

.gitignore

@@ -6,6 +6,7 @@ local_settings.py
 archweb.db
 archweb.db-*
 database.db
+/*.tar.gz


Please drop this, this shouldn't be required anymore. It's also unrelated to the other changes.

Sure it's "unrelated", but the fact remains that whenever you work from the project folder and follow some instructions about downloading a database or something similar you run the risk of pushing it globally.

If the project does not need .tar.gz files to be built or executed, there's really no risk of adding it to the ignore list. And I thought while I'm at it, I'll add it.

jelly · 2023-03-12T14:40:56Z

mirrors/urls_mirrorlist.py

@@ -3,6 +3,7 @@

 urlpatterns = [
    path('', views.generate_mirrorlist, name='mirrorlist'),
+    path('submit/', views.submit_mirror, name='mirrorsubmit'),


This is not linked anywhere on archlinux.org, maybe we should add a link with some text on https://archlinux.org/mirrors.

jelly · 2023-03-12T14:46:26Z

requirements.txt

@@ -1,5 +1,5 @@
 cssmin==0.2.0
-Django==4.0.1
+Django==4.1.6


This should not be in here? Please rebase and drop it.

Sure, but saw an opportunity to update old stuff seeing as they have a few vulns in 4.0 that should be fixed in 4.1. But I'll revert it and someone can update later.

jelly · 2023-03-12T14:47:19Z

AUTHORS

@@ -37,3 +37,5 @@ Thayer Williams <thayer@archlinux.org>
 Thomas Bächler <thomas@archlinux.org>
 Tom Willemsen <tom.willemsen@archlinux.us>
 Tyler Dence <tyzoid.d@gmail.com>
+Anton Hvornum <anton@hvornum.se>
+Nina Nick <5041877+ninchester@users.noreply.github.com>


Please add this in a separate commit.

Separate commit to the same PR, or separate PR with a separate commit?

jelly · 2023-03-12T14:48:04Z

mirrors/tests/test_mirror_submission.py

@@ -0,0 +1,3 @@
+def test_mirror_registration(client, mirrorurl):
+    response = client.get('/mirrorlist/submit/?name=test3&tier=2&upstream=1&admin_email=anton%40hvornum.se&alternate_email=&isos=on&rsync_user=&rsync_password=&notes=&active=True&public=True&url1-url=rsync%3A%2F%2Ftest3.com%2Farchlinux&url1-country=SE&url1-bandwidth=1234&url1-active=on&url2-url=&url2-country=&url2-bandwidth=&url2-active=on&url3-url=&url3-country=&url3-bandwidth=&url3-active=on&ip=&captcha_0=d5a017cc3851fb59898167f666759c99b42afd52&captcha_1=tdof')


This looks wrong, a form should never be able to be posted with a GET, it should always be a POST.

Also please create the url programmatically so you can easily assert the data when expanding the test.

Same as below:

archweb/templates/mirrors/mirrorlist_generate.html

Lines 38 to 41 in ad3b182

<form id="list-generator" method="get">

{{ mirrorlist_form.as_div }}

<p><label></label> <input type="submit" value="Generate List" /></p>

</form>

jelly · 2023-03-12T14:49:22Z

mirrors/views/mirrorlist.py

@@ -19,7 +19,6 @@

 import random

-


Unneeded change

jelly · 2023-03-12T14:49:28Z

mirrors/views/mirrorlist.py

@@ -234,41 +233,64 @@ def find_mirrors_simple(request, protocol):
    proto = get_object_or_404(MirrorProtocol, protocol=protocol)
    return find_mirrors(request, protocols=[proto])

+


Same as above.

I'm sorry, but PEP8#Blank-Lines dictates two blank lines between function definitions. The rest of the code is also spaced out this way.

For reference:

archweb/mirrors/views/mirrorlist.py

Lines 191 to 194 in 3904bac

return urls

def find_mirrors(request, countries=None, protocols=None, use_status=False,

jelly · 2023-03-12T14:53:15Z

templates/mirrors/mirror_submit.html

+
+    <p>Before you can submit a <b>Tier 1</b> request the mirror in question must first be a registered <b>Tier 2</b> for a certain amount of time with proven reliablity. Once the submitted information is verified the mirror will be visible under the appropriate tier list above. This process usually takes 5 minutes.</p>
+
+    <form id="list-generator" method="get">


A Form should never be a GET, but always a POST and should have:

<form method="post">{% csrf_token %}

And just for reference, I took the method from this form:

archweb/templates/mirrors/mirrorlist_generate.html

Lines 38 to 41 in ad3b182

<form id="list-generator" method="get">

{{ mirrorlist_form.as_div }}

<p><label></label> <input type="submit" value="Generate List" /></p>

</form>

jelly · 2023-03-12T15:02:35Z

requirements.txt

@@ -5,6 +5,7 @@ Markdown==3.3.7
 bencode.py==4.0.0
 django-countries==7.3.2
 django-extensions==3.1.3
+django-simple-captcha==0.5.17


Not sure if I like the captch dependency, we don't have it for flagging packages. Do you think it's really required?

Well this is what people said I should put in when I was asking in the arch channels, I don't mind dropping it but to reduce spam this is what was suggested.

To quote: "Just make a captcha thing"

Torxed and others added 5 commits February 17, 2023 11:42

Created the view for submitting mirrors

ad3b182

Fixed merge issue with requirements. The git link appears broken, usi…

642b77d

…ng pypi instead

Added POST

7dc2168

Added save functionality for mirrors

3904bac

Added sending of email on new mirror requests, improved validation ch…

b02b312

…ecks

jelly reviewed Feb 18, 2023

View reviewed changes

Added automatic deny if ERRORS > LIMIT during TIME_FRAME.

0937f13

Shortened URL's according to comments

ea4d9bc

Torxed added 8 commits February 18, 2023 20:15

Removed hardcoded emails

46cf8e0

Added MIN_DAYS as Tier 2 before Tier 1 requests being accepted.

32689a9

Added captcha on submission

594545f

Added captcha validation (forgot)

001d6e4

Added entries to authors

9ea8db5

Reverted requirements change for cssmin

7ef39df

Changed the wording on the HTML page, no promised time frame is given.

0825b1b

Removed debug code and fixed linting

365a14e

Changed to a more sane error limit for Tier 1, 10 errors over 30 days…

5846eea

… should be ok as some errors such as faulty DNS lookups do occur on some hosts momentarily. That we have to account for

Torxed marked this pull request as ready for review February 18, 2023 20:37

Torxed added 2 commits February 18, 2023 21:38

Reverting .gitignore - See archlinux#449 instead

120f5e4

Added a test using GET values

cd7ccc2

Torxed commented Feb 18, 2023

View reviewed changes

Fixed requirements?

0da5082

jelly requested changes Mar 12, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automate mirror submission #448

Automate mirror submission #448

Torxed commented Feb 17, 2023 •

edited

jelly left a comment

jelly Feb 18, 2023

Torxed Feb 18, 2023

Torxed Feb 18, 2023

Torxed commented Feb 18, 2023 •

edited

Torxed commented Feb 18, 2023

Torxed commented Feb 18, 2023

Torxed commented Feb 18, 2023

Torxed Feb 18, 2023

Torxed commented Feb 19, 2023

jelly commented Mar 12, 2023

jelly left a comment

jelly Mar 12, 2023

Torxed Mar 12, 2023

jelly Mar 12, 2023

Torxed Mar 12, 2023

jelly Mar 12, 2023

jelly Mar 12, 2023

Torxed Mar 12, 2023

jelly Mar 12, 2023

Torxed Mar 12, 2023

jelly Mar 12, 2023

jelly Mar 12, 2023

Torxed Mar 12, 2023

jelly Mar 12, 2023

jelly Mar 12, 2023

Torxed Mar 12, 2023

jelly Mar 12, 2023

Torxed Mar 12, 2023

jelly Mar 12, 2023

Torxed Mar 12, 2023

		@@ -0,0 +1,3 @@
		def test_mirror_registration(client, mirrorurl):
		response = client.get('/mirrorlist/submit/?name=test3&tier=2&upstream=1&admin_email=anton%40hvornum.se&alternate_email=&isos=on&rsync_user=&rsync_password=&notes=&active=True&public=True&url1-url=rsync%3A%2F%2Ftest3.com%2Farchlinux&url1-country=SE&url1-bandwidth=1234&url1-active=on&url2-url=&url2-country=&url2-bandwidth=&url2-active=on&url3-url=&url3-country=&url3-bandwidth=&url3-active=on&ip=&captcha_0=d5a017cc3851fb59898167f666759c99b42afd52&captcha_1=tdof')

		@@ -1,4 +1,4 @@
		-e git+git://github.com/fredj/cssmin.git@master#egg=cssmin
		-e git+https://github.com/fredj/cssmin.git@master#egg=cssmin

	<form id="list-generator" method="get">
	{{ mirrorlist_form.as_div }}
	<p><label></label> <input type="submit" value="Generate List" /></p>
	</form>

		@@ -234,41 +233,64 @@ def find_mirrors_simple(request, protocol):
		proto = get_object_or_404(MirrorProtocol, protocol=protocol)
		return find_mirrors(request, protocols=[proto])

	return urls


	def find_mirrors(request, countries=None, protocols=None, use_status=False,


		<p>Before you can submit a <b>Tier 1</b> request the mirror in question must first be a registered <b>Tier 2</b> for a certain amount of time with proven reliablity. Once the submitted information is verified the mirror will be visible under the appropriate tier list above. This process usually takes 5 minutes.</p>

		<form id="list-generator" method="get">

Automate mirror submission #448

Are you sure you want to change the base?

Automate mirror submission #448

Conversation

Torxed commented Feb 17, 2023 • edited

jelly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Torxed commented Feb 18, 2023 • edited

Torxed commented Feb 18, 2023

Torxed commented Feb 18, 2023

Torxed commented Feb 18, 2023

Choose a reason for hiding this comment

Torxed commented Feb 19, 2023

jelly commented Mar 12, 2023

jelly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Torxed commented Feb 17, 2023 •

edited

Torxed commented Feb 18, 2023 •

edited