Mete Polat [Mon, 15 Jul 2019 14:50:28 +0000 (16:50 +0200)]
Add 'dumparchive' management command
Introduces a new management command which can export all patches in a
project as one mbox file. Export of multiple projects is supported.
Additionally allows to compress the output.
Signed-off-by: Mete Polat <metepolat2000@gmail.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
- Remove some newlines between terms and definitions that were causing
the latter to be rendered as blockquotes instead
- Order list of settings alphabetically
- Update URLs to use latest version of Django we support
Signed-off-by: Stephen Finucane <stephen@that.guru>
templatetags: Do not mark output of msgid tag as safe
The msgid template tag exists to remove angle brackets from either side of
the Message-ID header.
It also marks its output as safe, meaning it does not get autoescaped by
Django templating.
Its output is not safe. A maliciously crafted email can include HTML tags
inside the Message-ID header, and as long as the angle brackets are not at
the start and end of the header, we will quite happily render them.
Rather than using mark_safe(), use escape() to explicitly escape the
Message-ID.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Sun, 21 Oct 2018 11:05:58 +0000 (12:05 +0100)]
Remove pwclient
Let's start managing this via a separate project, which will allow the
client to evolve separately from the server. No redirect is added for
the old '/pwclient' URL as it seems wiser to return a HTTP 404 error
code.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Commit 753e4572d updated the parser to consider additional header lines
when deciding where a patch message ends and the diff begins. However,
these additional lines were not captured meaning these patches didn't
have a diff associated with them and they therefore weren't patches in
the Patchwork sense of the term. Correct this and add a test.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Petr Vorel [Sun, 5 May 2019 20:20:05 +0000 (22:20 +0200)]
parser: Add missing extended header lines
Patchwork didn't recognise some patches due missing some extended header
lines (e.g. "old mode" and "new mode" for renaming file mode, see [1]).
Thus adding all modes from git doc [2].
Stephen Finucane [Tue, 14 May 2019 14:56:53 +0000 (15:56 +0100)]
docs: Document backport criteria
Explain why we don't want to be in the business of backport certain
patches, in the long run. It took me a while to put this into words but
I was helped by a similar discussion ongoing in the OpenStack community
at the moment [1].
In July 2018, we received a report of OzLabs patchwork mangling
emails that have subjects containing words with internal commas,
like "Insert DT binding for foo,bar" (#197).
Stephen took a look and came up with the comment this reverts. Quoting
the commit message:
RFC2822 states that long headers can be wrapped using CRLF followed by
WSP [1]. For example:
Subject: Foo bar,
baz
Should be parsed as:
Foo bar,baz
As it turns out, this is not the case. Journey with me to
section 2.2.3 of RFC 2822:
2.2.3. Long Header Fields
Each header field is logically a single line of characters comprising
the field name, the colon, and the field body. For convenience
however, and to deal with the 998/78 character limitations per line,
the field body portion of a header field can be split into a multiple
line representation; this is called "folding". The general rule is
that wherever this standard allows for folding white space (not
simply WSP characters), a CRLF may be inserted before any WSP. For
example, the header field:
Subject: This is a test
can be represented as:
Subject: This
is a test
So the issue with the example in the reverted commit is that there is no
folding white space in "bar,baz", so it's not valid to split it.
These are valid:
Subject: Foo bar,baz
Subject: Foo
bar,baz
but splitting "bar,baz" into "bar,\n baz" is not valid.
What then is correct unfolding behaviour? Quoting the RFC again:
The process of moving from this folded multiple-line representation
of a header field to its single line representation is called
"unfolding". Unfolding is accomplished by simply removing any CRLF
that is immediately followed by WSP. Each header field should be
treated in its unfolded form for further syntactic and semantic
evaluation.
In other words, the unfolding rule requires you to strip the CRLF, but
it does not permit you to strip the WSP. Indeed, if "bar,\n baz" is
received, the correct unfolding is "bar, baz".
If you do strip the WSP, you end up mashing words together, such as in
https://patchwork.ozlabs.org/patch/1097852/
So revert the commit, restoring original behaviour, but keep a corrected
version of the test.
This presents a big question though: how did Rob's email up with a
mangled subject line?
To answer this question, you end up having to learn about OzLabs
Patchwork and how it differs from Patchwork the project.
OzLabs Patchwork (patchwork.ozlabs.org) is an installation of Patchwork.
Part of what makes it so useful for so many projects is a little
intervening layer that can massage some mail to make it end up in the
right project. Email that lands in the device tree project is an example
of email that goes through this process. I only learned about this
today and I haven't looked in any detail at precisely what is done to
the mail. The script is not part of the Patchwork project.
This intervening filter is a Python script that runs - and this is an
important detail - in Python 2.7.
Ignoring all the details, the filter basically operates in a pipe
between the mail program and patchwork's parsemail, like
(mail from system) | filter.py | parsemail
At it's very simplest, filter.py acts as follows:
import email
import sys
mail = email.parse_from_file(sys.stdin)
sys.stdout.write(mail.as_string())
Fascinatingly, if you take Rob's email from #197 and put it through this
process, you can see that it is getting mangled:
You can see that python27 has incorrectly wrapped the header, breaking
where there is not a foldable space. Python3 does not have this issue.
To summarise:
- part of the magic of OzLabs PW is a filter to make sure mail gets to
the right place. This isn't part of the Patchwork project and so is
usually invisible to patchwork developers.
- the filter is written in python27. The email module in py27 has a bug
that incorrectly breaks subjects around commas within words.
- patchwork correctly unfolds those broken subjects with a space after
the comma.
- the extra space was interpreted as a bug in patchwork, leading to a
misinterpretation of the spec to strip out the whitespace that was
believed to be in error.
- that broke other wrapped subjects.
To solve this, revert the commit and I'll work with jk to get the filter
script into py3 compatibility. (Given that py27 sunsets in ~7mo, trying
to fix it is not worth it.)
Closes: #273 Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
[stephenfin: Use a new release note instead of editing the original one]
Russell Currey [Wed, 1 May 2019 06:27:18 +0000 (16:27 +1000)]
docs: Mention Postgres for Docker development install
Might as well since it's there, and it gives some clue to anyone trying
to use Docker on non-x86. I figured it was best to leave this out of
the README since it's incredibly niche.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 06:27:17 +0000 (16:27 +1000)]
docker: Use Ubuntu ports repositories on non-x86 architectures
This should allow Patchwork to run "out of the box" in Docker on any
architecture with a) an Ubuntu port and b) support in the Postgres
multiarch Docker image, which includes at least arm64 and ppc64le.
It's a little gross hacking the Dockerfile like this, but I'm not sure
there's a more elegant way to do it. Unfortunately it doesn't seem like
there's any way to do conditional COPY, and anything in RUN is plain
/bin/sh, so that's why it looks like it does.
Tested on ppc64le and on x86_64.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 04:35:53 +0000 (14:35 +1000)]
docker: Install libpq-dev to fix psycopg2-binary build
psycopg2-binary fails if pg_config isn't installed, which is provided by
libpq-dev.
This seems strange to me since psycopg2-binary suggests that
you use psycopg2-binary instead (of itself) if you don't want to build
psycopg2 so you wouldn't need pg_config, which is very confusing.
It's possible that psycopg2-binary only needs to compile itself on
non-x86 platforms, since I hit this on ppc64le.
Anyway, it works when this is added.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 03:16:43 +0000 (13:16 +1000)]
README: add .env file to installation instructions
Creating the .env file is mentioned in the installation documentation
but not in the README, so following only the steps mentioned there will
fail. Add this and add a `cd patchwork` in there for good measure so
you could straight up copy paste the steps.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Mon, 29 Apr 2019 15:33:16 +0000 (01:33 +1000)]
REST: Handle regular form data requests for checks
08d1459a4a40 ("Add REST API validation using OpenAPI schema") moved
all API requests to JSON blobs rather than form data.
dc48fbce99ef ("REST: Handle JSON requests") attempted to change the
check serialiser to handle this. However, because both a JSON dict
and a QueryDict satisfy isinstance(data, dict), everything was handled
as JSON and the old style requests were broken.
Found in the process of debugging issues from the OzLabs PW & Snowpatch
crew - I'm not sure if they actually hit this one, but kudos to them
anyway as we wouldn't have found it without them.
Fixes: dc48fbce99ef ("REST: Handle JSON requests") Signed-off-by: Daniel Axtens <dja@axtens.net>
Jeremy Kerr [Sat, 27 Apr 2019 11:12:16 +0000 (19:12 +0800)]
notifications: fix notification expiry when no user is associated
It's possible that an EmailConfirmation object will have no associated
user (eg, for email opt-out, which does not require a user object). In
this case, we will see a NULL value for EmailConfirmation.user_id.
However, having a NULL value appear in a SQL 'IN' clause will match
every value. This means that once one of these null-user
EmailConfirmations is present, we will never expire any non-active user
accounts.
This change adds a filter for a valid user_id when we query for active
EmailConfirmation objects. This means we'll have a valid values set to
use in the pending_confs set.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
[dja: fix pep8 issue] Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Fri, 15 Mar 2019 06:27:40 +0000 (17:27 +1100)]
Fix YAML loader warning
In my tests I'm seeing:
/home/patchwork/patchwork/patchwork/tests/api/validator.py:229:
YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated,
as the default Loader is unsafe. Please read https://msg.pyyaml.org/load
for full details.
Fix this by using the safe loader in the tests.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Thu, 28 Feb 2019 04:29:53 +0000 (15:29 +1100)]
parser: recognise git commit consisting only of empty new file
Commits with only an empty new file are liable to be missed.
The parser state machine doesn't recognise the headers "new
file mode" and "index": teach it about them.
Add a test to demonstrate.
It's a little bit academic as you don't usually send patches like
that but sometimes you do, especially if you're a snowpatch dev :)
Closes: #256 Reported-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Ali Alnubani [Tue, 8 Jan 2019 12:38:47 +0000 (12:38 +0000)]
Beautify check counts in the patch list view
This patch [1] adds colors to the checks in the patch list view.
The colors are set based on the check's priority, with FAILURE
having the highest priority, followed by WARNING, and then SUCCESS.
Only the check with the highest priority and non-zero count
will be colored. This is to make failures and warnings more visible.
The patch also [2] replaces zero counts with a '-' for
FAILUREs and WARNINGs.
The SUCCESS count will only be replaced by a '-'
when all other checks have zero counts too.
Suggested-by: Thomas Monjalon <thomas@monjalon.net> Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
Ali Alnubani [Tue, 8 Jan 2019 12:38:45 +0000 (12:38 +0000)]
Fix return code when getting patch information fails
The `info` command always exits with success, even if
the patch didn't exist.
Modified to exit with a non-zero exit status and
print an error message in that case.
Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
It turns out it is possible to make PATCH requests with JSON bodies
rather than form-encoded data - you just need to include a Content-Type
header. Document this.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Django Admin seems to be doing something funky with how it's handling
the creation of a User's corresponding UserProfile instance when
modelled as an inline field. Re-setting the UserProfile.user attribute
seems to resolve the issue, so do just that.
Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #110
Stephen Finucane [Thu, 15 Nov 2018 12:57:58 +0000 (13:57 +0100)]
Add REST API validation using OpenAPI schema
Add validation using the rather excellent 'openapi_core' library. The
biggest issue we have to contend with is the fact that 'openapi_core'
expects us to be able to provide a templated URL string for each request
(e.g. '/api/patches/123/' would become '/api/patches/<id>/') and Django
doesn't provide a way to do this [*]. We work around this by
reverse-engineering some of the Django code to turn a URL to its
matching regex, which we can then easily convert into a template string.
It's kind of hacky and not at all portable but, crucially, it does work
and has highlighted some nice bugs in the API that have already merged.
Going forward, we can probably modify 'openapi_core' somewhat to remove
the need for the templated URL string. If and when this happens, most of
the funkier code here can happily go away.
[*] Django 2.0+ [1] does actually provide a way to do template
string-based URLs and in fact recommends them now, with regexes being
reserved for more advanced corner cases. However, we don't want to drop
support for the Django 1.11 yet as it is the most recent LTS release.
Stephen Finucane [Fri, 26 Oct 2018 23:49:17 +0000 (00:49 +0100)]
docs: Make API document versioned
OpenAPI doesn't appear to support versioning natively, suggesting
instead that separate documents are kept. Rather than doing this
manually, let's use a templating tool - Jinja2, in this case - to
generate these document for us from a single master document.
Note that while we can now auto-generate these whenever we need them
(and we tend to avoid storing auto-generated assets in VCS), these
change so rarely that it's easier to just store them. This also means we
can reference the schemas themselves online. We do this in a following
change.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 26 Oct 2018 21:12:57 +0000 (22:12 +0100)]
docs: Document the '/events' resource
This is the final resource to document and also the most complicated, on
account of the polymorphism of the responses. However, with this done,
our first pass at an OpenAPI 3.0 schema is completed.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 26 Oct 2018 20:57:40 +0000 (21:57 +0100)]
docs: Document the '/bundles' resource
This one's a little unusual too, in that we provide the embedded
serializer for resources we haven't defined the end resource for. That's
necessary in general, due to recursive references in the API
(series-patch, patch-series etc.) so might as well embrace it early.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 26 Oct 2018 20:42:02 +0000 (21:42 +0100)]
docs: Start documenting API using OpenAPI
When the REST API was first added, we attempted to document it using
OpenAPI 2.0 (formerly Swagger). This was mostly never completed because
(a) it was really tedious and (b) no one was that bothered. However, as
we expand the range of clients for the REST API, having a well
documented API becomes more and more of an asset.
Start doing this by adding a brand new schema, this time using OpenAPI.
This will entirely replace the older schema and, as such, is namespaced
separately. We start by documenting '/' (i.e. the index) page and will
add additional resources as we go.
Signed-off-by: Stephen Finucane <stephen@that.guru>
I'm not actually sure why this wasn't raising an error. Perhaps it's
because null validation for char fields happens in forms rather than at
the database level. In any case, this won't happen normally since we
only allow creation via the admin API so simply start setting this.
Signed-off-by: Stephen Finucane <stephen@that.guru>