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

Add missing files to rpmspec and manifest #425

Merged
merged 3 commits into from Jan 23, 2020
Merged

Conversation

emkll
Copy link
Contributor

@emkll emkll commented Jan 23, 2020

Towards #406

Some files were failing when trying to install the dom0 rpm (When attempting to apply state.highstate)

Also added a pull request template to increase awareness of adding files to manifest and spec files.

Test plan:

Note that I have successfully completed the test plan below. Given the complexity of testing this relatively small diff, I would suggest the reviewer just perform a visual diff of this PR, but ultimately will leave it to their discretion.

  • make clean, and ensure no lingering files in /srv/salt
  • On this branch, in a dev vm (with docker installed), make dom0-rpm and copy artifact to dom0
  • make clone (we will use the cloned repo for tests)
  • in dom0, where the rpm was copied: sudo dnf install securedrop-workstation-dom0-config-0.1.1-1.fc25.noarch.rpm
  • package installs successfully
  • copy your instance-specific config.json and sd-journalist.sec into /srv/salt/sd/ (needs sudo)
  • cd /usr/share/securedrop-workstation-dom0-config
  • ./scripts/provision-all
  • provision-all returns 0, no errors in stdout
  • cd into your cloned workstation repo (e.g. ~/securedrop-workstation)
  • run make test
  • all tests pass

Some files were failing when trying to install the dom0 rpm (When attempting to apply state.highstate)

sd-proxy/*, securedrop-login now found, securedrop-launcher.desktop, securedrop-login, sys-firewall/*, launcher files
Provides checklist to developers to ensure files in packaging logic are in sync with provisioning logic
@emkll emkll added this to Ready for Review in SecureDrop Team Board Jan 23, 2020
Add rpm files for thsv3 support
Also explicitly define file permissions
@emkll
Copy link
Contributor Author

emkll commented Jan 23, 2020

great catch, pushed a commit to fix. I've also provisioned with v3 and ran make test, and the client communication with the server is working as expected.

Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

thanks for the rapid changes, took another scan through the repo, this lgtm

since you have tested i'm approving based on visual review


- [ ] Linting (`make flake8`) and tests (`make test`) pass in dom0 of a Qubes install

- [ ] I have added/removed files, and have updated packaging logic in MANIFEST.in and rpm-build/SPECS/securedrop-workstation-dom0-config.spec
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@redshiftzero redshiftzero merged commit 73dfb8c into master Jan 23, 2020
SecureDrop Team Board automation moved this from Ready for Review to Done Jan 23, 2020
@redshiftzero redshiftzero deleted the add-files-rpmspec branch January 23, 2020 22:56
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.

None yet

2 participants