-
Notifications
You must be signed in to change notification settings - Fork 46
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
Fixes #15 removes our logging plugin for sd-log #479
Conversation
This PR removes the sdlog.conf for rsyslog via rc.local file for the sd-log vm. Fixes #15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kushaldas for the fix, works well for me locally,
-
make all
-
make test
-
sd-log
vm should not have any error at/var/log/syslog
file. -
sd-log
vm/rw/config/rc.local
file has the following lines: - logs are flowing
Prior to merge, I think we should update the comments in sd-log-disable-plugin.sls
(see inline comments)
# vim: set syntax=yaml ts=2 sw=2 sts=2 et : | ||
|
||
## | ||
# Disable securedrop-log rsyslog plugin in sd-log vm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also link to the issue in the comments here? It would be useful to note, for future maintainers, that this is the side effect of using the same package for both the client and server component of the log: we want to make sure that sd-log does not send logs.
dom0/sd-log-disable-plugin.sls
Outdated
- marker_start: "### BEGIN securedrop-workstation ###" | ||
- marker_end: "### END securedrop-workstation ###" | ||
- content: | | ||
# Add sd-rsyslog.conf file for syslog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are removing here, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will update the comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kushaldas, looks good to me!
Noting that you are adding a new file but since it's prefixed by sd, it will be caught by make clean, and since it's in the dom0 folder it is included in MANIFEST.in [1] and dom0 rpm spec [2]
[1]
securedrop-workstation/MANIFEST.in
Line 1 in 76ba862
include dom0/*.sls |
[2]
install -m 644 dom0/*.sls %{buildroot}/srv/salt/ |
Status
Ready for review
Description of Changes
This PR removes the sdlog.conf for rsyslog via rc.local file
for the sd-log vm.
Fixes freedomofpress/securedrop-log#15
Testing
make all
sd-log
vm should not have any error at/var/log/syslog
file.sd-log
vm/rw/config/rc.local
file has the following lines:Checklist
If you have made code changes
make flake8
) passes in the development environment (this box maybe left unchecked, as
flake8
also runs in CI)If you have made changes to the provisioning logic
All tests (
make test
) pass indom0
of a Qubes installThis PR adds/removes files, and includes required updates to the packaging
logic in
MANIFEST.in
andrpm-build/SPECS/securedrop-workstation-dom0-config.spec