Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create staging release candidate #540

Closed
eloquence opened this issue Apr 22, 2020 · 11 comments · Fixed by #550 or freedomofpress/securedrop-yum-test#10
Closed

Create staging release candidate #540

eloquence opened this issue Apr 22, 2020 · 11 comments · Fixed by #550 or freedomofpress/securedrop-yum-test#10

Comments

@eloquence
Copy link
Member

eloquence commented Apr 22, 2020

To ensure everyone can run the same trusted artifacts during QA, we'd like to upload RCs for testing that supersede the nightly RPM. To begin with, we'll want to do a 0.3.0-rpm-rc1 (or whatever naming convention we end up using) release.

@emkll
Copy link
Contributor

emkll commented May 8, 2020

I was trying to set a pre-release version scheme for these dom0 RPMs, but running into some issues.
Fedora's guidelines suggest we can use tilde to specify prerelease versions [1] . It seems like we can preserve the scheme used by SecureDrop (in this case, version string being 0.3.0~rc1).

However, to build the RPM, we first build a tarball using setup.py: https://github.com/freedomofpress/securedrop-workstation/blob/master/scripts/build-dom0-rpm#L39

It seems like setup.py does not support ~ for versions:

user@dev:~/src/securedrop-workstation$ python3 setup.py --version
Warning: 'classifiers' should be a list, got type 'tuple'
/usr/lib/python3/dist-packages/setuptools/dist.py:484: UserWarning: The version specified ('0.3.0~rc1') is an invalid version, this may not work as expected with newer versions of setuptools, pip, and PyPI. Please see PEP 440 for more details.
  "details." % self.metadata.version
0.3.0~rc1

The sdist operation will overwrite the ~ to a -. Based on the docs for sdist [2], it appears that we can't override the target filename (only destination folder)

Setting Source0 in the RPM spec doesn't seem to work, and the build fails at the %prep phase:

+ cd securedrop-workstation-dom0-config-0.3.0~rc1
/var/tmp/rpm-tmp.gqojFJ: line 37: cd: securedrop-workstation-dom0-config-0.3.0~rc1: No such file or directory
error: Bad exit status from /var/tmp/rpm-tmp.gqojFJ (%prep)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.gqojFJ (%prep)
make: *** [Makefile:33: dom0-rpm] Error 1

It seems like the source folder path at build time is not derived from the Source field in the spec. @kushaldas any ideas here?

[1] https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_versioning_prereleases_with_tilde
[2] https://docs.python.org/3/distutils/sourcedist.html

@emkll
Copy link
Contributor

emkll commented May 11, 2020

@kushaldas suggested in standup we use %setup -n <path> in the %prep section of the spec, and this appears to resolve the issue, with some minor changes to update_version.sh

@emkll
Copy link
Contributor

emkll commented May 13, 2020

Unfortunately, the above versioning scheme used by SecureDrop core will not work here. The qubes-dom0-update mechanism [1] uses sys-firewall to download the rpm packages, and sends them back to dom0 for installation.
Prior to being installed, the qubes-receive-updates script will validate the RPM package, including gpg signature but also the rpm filename. In this validation script, the ~ character is not an accepted character for the rpm filename regex [2].

At this stage, i see two options to unblock us on the short term:

  • The most straightforward way is to simply increment the minor version from 0.2.4 to 0.2.5, not releasing 0.2.5 rpms if there are bugs, and incrementing to 0.3.0 prior to release.
  • increment the last release, for example 0.3.0~rc1 becomes 0.2.4-next-rc1 or something of the sort, and updating the version to 0.3.0 once qa is complete.

We could also coordinate with upstream to consider/allow the use of the ~ character in package names (perhaps there could be further implications due to it's expansion properties in bash). However, since since it's part of qubes-core-admin-linux package, it take naturally take a while for the change to get propagated

[1] : https://www.qubes-os.org/doc/dom0-secure-updates/
[2] : https://github.com/QubesOS/qubes-core-admin-linux/blob/2695a6ec9030a9a073f5bddd3202950707bd5e5a/dom0-updates/qubes-receive-updates#L42

@eloquence
Copy link
Member Author

increment the last release, for example 0.3.0~rc1 becomes 0.2.4-next-rc1 or something of the sort

I'm a bit confused by this, if 0.2.4-next-rc1 works, wouldn't 0.3.0-rc1 (instead of 0.3.0~rc1) also work?

@emkll
Copy link
Contributor

emkll commented May 13, 2020

When comparing version strings, my understanding is that 0.3.0-rc1 > 0.3.0. That means we wont be able to update staging environments to the latest release, unless the release version is incremented to 0.3.1

@eloquence
Copy link
Member Author

Ah, that makes sense. I defer to y'all of course, but I personally think that a scheme like the one you propose, <current-production-release>-next-rc<current-rc-level>, sounds reasonable for now. I'm pretty sure I've seen similar uses of the next pragma before.

conorsch pushed a commit to freedomofpress/securedrop-yum-test that referenced this issue May 18, 2020
The tilde in the version string doesn't play nicely with the regex
validation logic in `qubes-receive-updates`; see discussion in [0].
Removing the package for now, we'll follow up with a subsequent package
that works with the tooling.

[0] freedomofpress/securedrop-workstation#540 (comment)
@conorsch
Copy link
Contributor

According to https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_prerelease_versions :

In the Version: tag, use the version that upstream has determined the next release will be. For the field of the Release: tag, use a number of the form "0.N" where N is an integer beginning with 1 and increasing for each revision of the package. Prerelease versions MUST use a Release: tag strictly less than 1, as this is the sole indicator that a prerelease has been packaged.

Following that recommendation, we should be setting Version: 0.3.0 and Release: 0.1%{?dist} for 0.3.0~rc1. When it comes time to build 0.3.0~rc2, then the fields should be Version 0.3.0 and Release: 0.2%{?dist}. It'd require changes to the update-version logic, naturally, but if confirmed working, it'll be more consistent with the rc-versioning practices used in other SD components.

@kushaldas
Copy link
Contributor

kushaldas commented May 18, 2020

https://fedoraproject.org/wiki/Package_Versioning_Examples @conorsch this also has good examples.

For 0.3.0~rc2 it should have Version: 0.3.0~rc2 and Release: 0.1%{?dist}. That release number increases only when we rebuilt the rpm for release (for the same source).

Version: 0.3.0 and Revision: 0.rc2.1 is what I have been suggested now from the Fedora devels.

@conorsch
Copy link
Contributor

Based on comments by @kushaldas, I implemented changes to update-version.sh that supports the following invocations:

  • ./update-version.sh 0.3.0-rc1
  • ./update-version.sh 0.3.0-rc2
  • ./update-version.sh 0.3.0

After each step, make dom0-rpm passes successfully. A bit more manual testing is required to ensure the upgrade/downgrade behavior is as expected, but all in all, I'm optimistic about the approach, and pleased with how well it mirrors versioning logic for other SD components.

@emkll
Copy link
Contributor

emkll commented May 19, 2020

@conorsch @kushaldas those changes look great to me after testing locally. If we'd like to preserve f25 in the Release Candidate case, I would recommend a minor change (see [1]).

Otherwise, I think that the branch @conorsch should be cleaned up and replace #552

[1]: Preserve dist string in Revision for RCs:

diff --git a/update_version.sh b/update_version.sh
index 01273dc..e4b52cc 100755
--- a/update_version.sh
+++ b/update_version.sh
@@ -32,7 +32,7 @@ elif [[ $NEW_VERSION == *-rc* ]]; then
         echo "$rc_error_msg"
         exit 2
     fi
-    RELEASE_FIELD="0.rc${RC_VERSION}.1"
+    RELEASE_FIELD="0.rc${RC_VERSION}.1%{?dist}"
 fi
 
 # Remove hyphens to ensure tarball filepath is built correctly

@conorsch
Copy link
Contributor

@emkll Yes, the dist string should absolutely be preserved! I'll amend the commit to include, then append the commit to #540. Let's discuss the specifics of the test plan as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment