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

Reorganizes mimetype declarations for template consolidation #198

Merged
merged 7 commits into from
Oct 27, 2020

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Oct 5, 2020

Towards freedomofpress/securedrop-workstation#471

Closes #197

Includes updates to packages for the Workstation, in order to achieve template consolidation. These packages are currently being served via a "template-consolidation" channel on apt-test: freedomofpress/securedrop-apt-test#65 Specifically, the changes presented here are:

Testing

Check out freedomofpress/securedrop-workstation#619, which will configure both of these new packages via a custom apt-test channel. There's also a detailed test plan at https://github.com/freedomofpress/securedrop-workstation/wiki/0.5.0-Test-Plan

conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 5, 2020
conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 6, 2020
Strictly for use with template-consolidation channel, for debugging.
Again, see related PR in:

freedomofpress/securedrop-builder#198
conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 6, 2020
conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 6, 2020
Strictly for use with template-consolidation channel, for debugging.
Again, see related PR in:

freedomofpress/securedrop-builder#198
@eloquence eloquence added this to Ready for Review in SecureDrop Team Board Oct 8, 2020
@emkll
Copy link
Contributor

emkll commented Oct 9, 2020

Noting that the mimetype config for the sd-export package is still in the system volume [1] (through overriden by the private volume configation after running the salt logic). Ideally, we would want /usr/share/applications/mimeapps.list to default to open-in-dvm for all mimetypes, but this may require two passes (first the debian packages, then the salt logic), similar to what's required for the proxy configuration changes.

[1] https://github.com/freedomofpress/securedrop-debian-packaging/blob/mimetype-reorg-for-all/securedrop-export/debian/securedrop-export.install#L4-L5
[2] freedomofpress/securedrop-proxy#79 (comment)

@conorsch
Copy link
Contributor Author

conorsch commented Oct 9, 2020

Noting that the mimetype config for the sd-export package is still in the system volume

Good eye. The need to update securedrop-export wasn't tracked in on the epic freedomofpress/securedrop-workstation#471, but that's an oversight. I've added it, and will circle back to update both the packaging logic and the corresponding salt states.

@eloquence eloquence moved this from Ready for Review to In Development in SecureDrop Team Board Oct 13, 2020
@conorsch
Copy link
Contributor Author

There are a few more changes that should be made before final review. For reference:

$ grep -rIF mimeapps.list *
securedrop-client/debian/securedrop-client.install:files/mimeapps.list usr/share/applications/
securedrop-export/debian/securedrop-export.install:files/mimeapps.list usr/share/applications/
securedrop-workstation-config/debian/securedrop-workstation-config.install:mimeapps.list.sd-viewer opt/sdw/
securedrop-workstation-config/debian/securedrop-workstation-config.install:mimeapps.list.sd-app opt/sdw/
securedrop-workstation-config/debian/securedrop-workstation-config.install:mimeapps.list.sd-app opt/sdw/mimeapps.list.default
securedrop-workstation-config/debian/securedrop-workstation-config.install:mimeapps.list.sd-devices-dvm opt/sdw/
$ grep -rIP 'files/.*\.desktop' *
securedrop-client/debian/securedrop-client.install:files/open-in-dvm.desktop usr/share/applications/
securedrop-client/debian/securedrop-client.install:files/securedrop-client.desktop usr/share/applications/
securedrop-export/debian/securedrop-export.install:files/send-to-usb.desktop usr/share/applications
securedrop-export/debian/securedrop-export.install:files/open-in-dvm.desktop usr/share/applications/

Remaining tasks:

  1. Remove mimeapps.list from securedrop-client package (it was moved to securedrop-workstation-config in Provide mime and paxctld configuration for template consolidation #190)
  2. Remove mimeapps.list from securedrop-export package (it was moved to securedrop-workstation-config in Provide mime and paxctld configuration for template consolidation #190)
  3. Ship the open-in-dvm.desktop file only once, rather than twice. Perhaps move to securedrop-workstation-config so it's available everywhere? cc @emkll

The above changes should eventually get PRs in the respective components repos, but are not strictly necessary to further testing here: as long as the files in question are excised from the package spec, we can follow up with more housekeeping to remove from the tarball generated in the component repos.

conorsch pushed a commit to freedomofpress/securedrop-export that referenced this pull request Oct 15, 2020
Follows packaging PR, just to evaluate CI behavior. This commit should
not be merged.

freedomofpress/securedrop-builder#198
@eloquence eloquence moved this from In Development to Ready for Review in SecureDrop Team Board Oct 19, 2020
conorsch pushed a commit to freedomofpress/securedrop-client that referenced this pull request Oct 19, 2020
As part of template consolidation [0], we've moved all mimetype
associations into the "securedrop-workstation-config" package [1].
The "open-in-dvm" desktop file now resides in the same package [2].

[0] freedomofpress/securedrop-workstation#471
[1] freedomofpress/securedrop-builder#190
[2] freedomofpress/securedrop-builder#198
conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 22, 2020
Removes mimetypes, given that they're now provided in
`securedrop-workstation-config`. See corresponding changes in:

  * freedomofpress/securedrop-client#1160
  * freedomofpress/securedrop-builder#198
conorsch pushed a commit to freedomofpress/securedrop-client that referenced this pull request Oct 22, 2020
For testing template consolidation, we want a newer version
to install via a custom channel. See related changes in

freedomofpress/securedrop-builder#198
@@ -1,5 +1,6 @@
mimeapps.list.sd-viewer opt/sdw/
mimeapps.list.sd-app opt/sdw/
mimeapps.list.sd-app opt/sdw/mimeapps.list.default
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on local testing, this will create a folder /opt/sdw/mimeapps.list.default/ and copy mimeapps.list.sd-app in that folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good catch. Pulled in your commit, added another, and built the package: freedomofpress/securedrop-apt-test#70

@emkll emkll moved this from Ready for Review to Under Review in SecureDrop Team Board Oct 22, 2020
@eloquence eloquence moved this from Under Review to In Development in SecureDrop Team Board Oct 22, 2020
conorsch pushed a commit to freedomofpress/securedrop-export that referenced this pull request Oct 22, 2020
Follows packaging PR, just to evaluate CI behavior. This commit should
not be merged.

freedomofpress/securedrop-builder#198
conorsch pushed a commit to freedomofpress/securedrop-apt-test that referenced this pull request Oct 22, 2020
Adjusts mimetypes handling, consolidating into a single package as much
as possible. See:

freedomofpress/securedrop-builder#198
@eloquence eloquence moved this from In Development to Under Review in SecureDrop Team Board Oct 26, 2020
@eloquence
Copy link
Member

(This is being tested as part of testing freedomofpress/securedrop-workstation#619 , let's not forget to give it the approval stamp when ready, so it can be merged along with the other consolidation PRs.)

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, packages are already on apt test and working as expected and have had ample testing as part of review of freedomofpress/securedrop-workstation#619

Two minor comments that shouldn't block merge (but we should be mindful prior to merge)

@@ -1,3 +1,9 @@
securedrop-client (0.2.2+buster) unstable; urgency=medium
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version bump in freedomofpress/securedrop-client#1160 is marked as temporary, we should either bump the client version and add the changelog as part of 1160 or in a separate PR, prior to merging this changelog addition here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll remove the version bump in freedomofpress/securedrop-client#1160, as well as remove the version bump in this PR. For nightlies, that'll be good enough: the consolidation logic additions we wish to QA will be available in apt-test main.

@@ -1,2 +1 @@
mimeapps.list usr/share/applications/
paxctld.conf.svsdisp opt/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the pax flags are also handled by the config package[1] , we can also remove this and relevant postinst chances

Given that this package will not be installed in consolidated template, we don't strictly need to add this here, we can just remove all the workstation-svs-disp related stuff after we release consolidated templates.

[1] https://github.com/freedomofpress/securedrop-debian-packaging/pull/190/files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can just remove all the workstation-svs-disp related stuff after we release consolidated templates.

Right, that was my plan: we'll circle back and delete all these packages once the release has shipped. I don't see a pressing need to do that right now.

@emkll emkll moved this from Under Review to Approved for template consolidation in SecureDrop Team Board Oct 27, 2020
conorsch pushed a commit to freedomofpress/securedrop-client that referenced this pull request Oct 27, 2020
As part of template consolidation [0], we've moved all mimetype
associations into the "securedrop-workstation-config" package [1].
The "open-in-dvm" desktop file now resides in the same package [2].

[0] freedomofpress/securedrop-workstation#471
[1] freedomofpress/securedrop-builder#190
[2] freedomofpress/securedrop-builder#198
Conor Schaefer and others added 7 commits October 27, 2020 13:06
The sd-proxy VM was using a legacy "do-not-open-here" warning,
a vestigate from early Workstation prototypes before DispVMs were used
to handle submissions. It's no longer required. A saner default is
actually the open-in-dvm, since the SDW default DispVM is netless, and
therefore quite safe. Let's use that.
Moves the `securedrop-workstation-svs-disp` mimeapps.list config into the
`securedrop-workstation-config` package, same as for the other
Workstation components. The same file was already referenced in the
config, so don't need to readd it.
Adds new metapackage with a clean history, to aid in template
consolidation. Attempts to override the old package, causing it
automatically be removed, via Replaces/Conflicts [0].

The dependencies are the same, although reordered, with one exception:
"evince" has been pulled in so we can remove it from the salt config.

[0] https://www.debian.org/doc/debian-policy/ch-relationships.html#s-replaces

Bumps securedrop-workstation-viewer to 0.1.1

Fixes a typo in the control file, which was pointing to a non-existent
package and trying to supersede it.
Updates the RPC file in the code repo, then rebuilds based on new
version. See related PR in:

freedomofpress/securedrop-proxy#79
Pointed out by @emkll during review, the ".install" declarations use
directories as dest, so we can't write a file that way. Added to
postinst, along with the other setups.

Technically we could reuse the ".sd-app" mimetype config elsewhere, but
using a ".default" name for it keeps the door open on divergence in the
future, and matches what we've been testing as part of template
consolidation.
@conorsch
Copy link
Contributor Author

@emkll I removed the version bump commits for all packages with other PRs, so we can adhere to standard procedure and bump separately. I've left in the commits bumping version for packages that are maintained entirely within this repo, however. If you'd like those removed too, just say so.

Ready for final review.

@conorsch conorsch marked this pull request as ready for review October 27, 2020 20:11
@conorsch conorsch moved this from Approved for template consolidation to Under Review in SecureDrop Team Board Oct 27, 2020
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@conorsch
Copy link
Contributor Author

Thanks for careful review, @emkll. Merging now. Let's make sure to nudge CI in the component repos, so this new build logic is picked up post-merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Create new metapackage to track application dependencies for sd-viewer based dispVMs
3 participants