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

timer: make jobs-status file world readable #2796

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

lucasmoura
Copy link
Contributor

@lucasmoura lucasmoura commented Oct 16, 2023

Why is this needed?

When running collect-logs as non-root, we might fail trying to collect the jobs-status information. Since there is no private date in this file, we are making it world readable now.

Fixes: #2601

Test Steps

Run the modified integration test and the manual test in this PR

Checklist

  • I have updated or added any unit tests accordingly
  • I have updated or added any integration tests accordingly
  • Changes here need to be documented, and this was done in:

Does this PR require extra reviews?

  • Yes
  • No

@github-actions
Copy link

Jira: This PR is not related to a Jira item. (The PR title does not include a SC-#### reference)

GitHub Issues:

Launchpad Bugs: No Launchpad bugs are fixed by this PR. (No commits have LP: #### references)

Documentation: The changes in this PR do not require documentation changes.

👍 this comment to confirm that this is correct.

Comment on lines 427 to 432
if dpkg --compare-versions "$PREVIOUS_PKG_VER" lt "31~"; then
if [ -f /var/lib/ubuntu-advantage/jobs-status.json ]; then
chmod 0644 /var/lib/ubuntu-advantage/jobs-status.json
fi
fi

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this change deserve a postinst transition? I think the next time the file is written it will get the new permissions anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think you are right. My concern was that if the user disabled the timer jobs, we would not update the file, but this is not the case. I will remove this bit together with the manual test

@@ -1,5 +1,6 @@
Feature: Command behaviour when attached to an Ubuntu Pro subscription

@wip
Copy link
Collaborator

Choose a reason for hiding this comment

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

some @wips left behind 🙃

When running collect-logs as non-root, we might fail
trying to collect the jobs-status information. Since there
is no private date in this file, we are making it world readable
now.

Fixes: #2601
@lucasmoura lucasmoura force-pushed the make-jobs-status-non-root-readable branch from 175d763 to 46e11e6 Compare October 20, 2023 17:26
@lucasmoura
Copy link
Contributor Author

@orndorffgrant updated

Copy link
Collaborator

@orndorffgrant orndorffgrant left a comment

Choose a reason for hiding this comment

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

LGTM thank you!

Copy link
Contributor

@CalvoM CalvoM left a comment

Choose a reason for hiding this comment

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

LGTM

@renanrodrigo renanrodrigo merged commit 4c5604b into main Oct 25, 2023
22 of 25 checks passed
@renanrodrigo renanrodrigo deleted the make-jobs-status-non-root-readable branch October 25, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-root user getting errors collecting jobs-status.json
4 participants