add note about how we update the builder image #137
Conversation
| tells us that our builder image needs to be updated to ensure we are using the latest | ||
| security packages. You can ignore this test failure for release candidates, but | ||
| create a ticket in the ``securedrop`` repository so that we can update the builder | ||
| image before building the packages for the final release. |
There was a problem hiding this comment.
It it necessary to create an issue to update the builder image (historically it doesn't look like we've done so), or is it standard operating procedure to always update it before building the packages for the final release?
There was a problem hiding this comment.
I think you're right. Perhaps creating an issue to update the builder image is overkill. We could just add in the "Release Process" section below a step for updating the builder image if this test fails. So the note here could say something more like:
.. note:: Since we don't update the container that builds the debian packages at runtime, you
might end up seeing ``test_ensure_no_updates_avail`` fail in the build output. This
tells us that our builder image needs to be updated to ensure we are using the latest
security packages. You can ignore this test failure for release candidates, because
it will be updated as part of the final release (linking to the release process section).
There was a problem hiding this comment.
That sounds right to me -- @emkll, does that make sense to you as well?
There was a problem hiding this comment.
Yes, @creviera that language looks good to me. We could also link to the build instructions, since the release management documentation is generally developer-facing: docs/development/dockerbuildmaint.rst
Note that in the "Final Release" section of Release issues, we have a check box in place to ensure the builder is up-to-date, so i agree that opening an issue may be redundant here:
Ensure builder in release branch is updated and/or update builder image
There was a problem hiding this comment.
okay i also update docs/development/dockerbuildmaint.rst so this is ready again for review
sssoleileraaa
force-pushed
the
release-management
branch
from
607d7cd to
d29a24a
Compare
sssoleileraaa
force-pushed
the
release-management
branch
from
d29a24a to
511db58
Compare
| **test_ensure_no_updates_avail** . If you have access rights to push to quay.io, | ||
| here is the process to build and push a new container: | ||
| .. note:: The reason we don't update the container at runtime is that we use the container image as | ||
| a way of recording our build environment. |
There was a problem hiding this comment.
Could equivalent security properties be achieved by including the exact version information in the build logs automatically, as suggested in freedomofpress/securedrop#4533 (comment) ? If so, it does not seem a compelling argument to include in favor of the current process.
There was a problem hiding this comment.
The docs presented here are definitely accurate in terms of the process we actually adhere to. I remain interested in getting the SD core debs fully reproducible, so we can stop fussing with the build container so much. After Focal, that seems a reasonable goal again, consistent with discussion in freedomofpress/securedrop#4533.
All that to say, the "note" as written here LGTM!
|
Looks good to me, but since I'm not directly familiar with the builder image update process, I would appreciate review from a SecureDrop maintainer before merging. |
| Docker repository at **quay.io/freedomofpress**. The images are organized by Ubuntu release | ||
| version. For instance, you can find the images for Xenial at | ||
| **quay.io/freedomofpress/sd-docker-builder-xenial** and, for Focal, at | ||
| **quay.io/freedomofpress/sd-docker-builder-focal**. |
There was a problem hiding this comment.
We could make these bolded URLs actual URLs to be a bit easier on folks, but that's not a change that came in via this PR.
| **test_ensure_no_updates_avail** . If you have access rights to push to quay.io, | ||
| here is the process to build and push a new container: | ||
| .. note:: The reason we don't update the container at runtime is that we use the container image as | ||
| a way of recording our build environment. |
There was a problem hiding this comment.
The docs presented here are definitely accurate in terms of the process we actually adhere to. I remain interested in getting the SD core debs fully reproducible, so we can stop fussing with the build container so much. After Focal, that seems a reasonable goal again, consistent with discussion in freedomofpress/securedrop#4533.
All that to say, the "note" as written here LGTM!
| `stable releases <https://blog.torproject.org/category/tags/stable-release>`_. Let the team | ||
| know about any new release candidates during the SecureDrop release process in case there are | ||
| critical bug fixes. For a new stable release, file an issue and upgrade Tor following these | ||
| steps: |
There was a problem hiding this comment.
Much clearer, thank you for including the explicit URLs!
| .. warning:: Only commit deb packages with an incremented version number: do not clobber | ||
| existing packages. That is, if there is already a deb called e.g. | ||
| ``ossec-agent-3.6.0-amd64.deb`` in ``main``, do not commit a new version of this | ||
| deb. |
There was a problem hiding this comment.
Locally, the way I determine this is by running git status and checking if a file was modified by my changes. If so, I run git reset <file> on each of them, and commit only the new packages. That's something CI could check for us, even in the dev-lfs repo, that might be a little friendlier to developers. Weighed against continued progress toward reproducibility, when we can trust a bot to file the PR, I very much prefer these docs you've got here.
Status
Ready for review
Description of Changes
Add a note in the docs about how we ignore the test failure that checks if the container that builds our debian packages is up-to-date with the latest packages. The test failure is expected and future improvements to this process can be made after our Focal upgrade, see see freedomofpress/securedrop#4533.