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

Always set a VM's internalness #1219

Merged
merged 1 commit into from
Dec 17, 2024
Merged

Always set a VM's internalness #1219

merged 1 commit into from
Dec 17, 2024

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Dec 11, 2024

Status

Ready for review

Description of Changes

To ensure we correctly flip the status of a VM's internal feature or not, ensure the property is always being set, but conditional the value on the environment.

And then explicitly set sd-app and sd-gpg as non-internal, so that salt will update them from when they used to be internal (see e754b40).

Refs #1214

Testing

  • provision a prod workstation using 1.0.0 or anything before e754b40; sd-app, sd-gpg, sd-proxy are internal
  • re-provision that prod workstation using this PR; sd-app and sd-gpg should be non-internal, while sd-proxy stays internal.
  • re-provision as a dev workstation using this PR; sd-proxy should switch to being non-internal

Deployment

Any special considerations for deployment? This is to implement handling for upgrades.

Checklist

  • All tests (make test) pass in dom0

@legoktm legoktm added this to the 1.1.0 milestone Dec 11, 2024
@legoktm
Copy link
Member Author

legoktm commented Dec 11, 2024

I don't have my normal SDW testing machine with me so I haven't been able to test this at all.

To ensure we correctly flip the status of a VM's internal feature or
not, ensure the property is always being set, but conditional the value
on the environment.

And then explicitly set sd-app and sd-gpg as non-internal, so that salt
will update them from when they used to be internal (see e754b40).

Refs #1214
Copy link
Contributor

@deeplow deeplow left a comment

Choose a reason for hiding this comment

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

Thanks! Visually looks good to me. I'll setup a fresh install tomorrow (was going to do it anyways) and follow the test plan if nobody gets there first.

Copy link
Contributor

@deeplow deeplow left a comment

Choose a reason for hiding this comment

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

Works as expected. Good to merge.

@legoktm legoktm added this pull request to the merge queue Dec 17, 2024
Merged via the queue into main with commit dedddd0 Dec 17, 2024
7 checks passed
deeplow added a commit to deeplow/securedrop-workstation that referenced this pull request Jan 16, 2025
With freedomofpress#1219 sd-app and sd-proxy were explicitly as "internal: 0".
However, this does not work, as the developer docs warn [1]:

> Some extensions interpret the values as boolean. In this case,
> the empty string means False and non-empty string (commonly '1')
> means True.

To fix this, the internalness of a qube is explicitly set to "".

[1]: https://dev.qubes-os.org/projects/core-admin-client/en/latest/manpages/qvm-features.html#description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants