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

private apt env (SC-1346) #2342

Merged
merged 11 commits into from Jan 14, 2023
Merged

private apt env (SC-1346) #2342

merged 11 commits into from Jan 14, 2023

Conversation

renanrodrigo
Copy link
Member

@renanrodrigo renanrodrigo commented Dec 4, 2022

Stop relying on the unauthenticated and pinned-to-never ESM repositories in the apt config and use a private separate apt cache for that instead.

Reference is spec US028.

LP: #1990378

todos:

  • remove postinst code that would create files in the system apt configuration
  • remove timer job that would do the same
  • Implement functionality to create the internal cache
  • create/update internal cache from the apt update hook
  • make sure the internal cache and system cache are fine when enabling/disabling esm-[infra/apps]
  • change security status to read from both caches
  • change the C++ hook to read from both caches
  • create/update internal cache when running dist-upgrade (decided to remove it and let be recreated on update)
  • make sure all integration tests pass and make sense

Desired commit type

  • Squash and merge: Using the "Proposed Commit Message"
  • Create a merge commit and use "Proposed Commit Message"
  • Rebase and merge, no merge commit, rely only on existing commit messages

Checklist

  • I have updated or added any unit tests accordingly
  • I have updated or added any integration tests accordingly
  • I have updated or added any documentation accordingly

Does this PR require extra reviews?

  • Yes: Julian Klode, Robie Basak (or other SRU member for pre-review)
  • No

@renanrodrigo renanrodrigo marked this pull request as draft December 4, 2022 18:04
@renanrodrigo renanrodrigo changed the title wip: private apt env wip: private apt env (SC-1346) Dec 7, 2022
uaclient/apt.py Outdated Show resolved Hide resolved
uaclient/entitlements/esm.py Outdated Show resolved Hide resolved
debian/ubuntu-advantage-tools.postinst Show resolved Hide resolved
uaclient/security_status.py Outdated Show resolved Hide resolved
uaclient/security_status.py Outdated Show resolved Hide resolved
@orndorffgrant
Copy link
Collaborator

cpp hook portion of this is mostly done and sitting at #2358 for now. we can decide how to rebase/merge all of this together in the new year

@renanrodrigo
Copy link
Member Author

Rebased this branch on main, then addressed the review comments here and did small changes for things to work as expected

@renanrodrigo
Copy link
Member Author

Imported the commit from @orndorffgrant's branch here for completeness

uaclient/apt.py Show resolved Hide resolved
uaclient/apt.py Outdated Show resolved Hide resolved
uaclient/entitlements/esm.py Show resolved Hide resolved
uaclient/jobs/update_messaging.py Show resolved Hide resolved
uaclient/apt.py Outdated Show resolved Hide resolved
@renanrodrigo renanrodrigo marked this pull request as ready for review January 4, 2023 20:25
@renanrodrigo renanrodrigo force-pushed the private-apt branch 3 times, most recently from aef2c4c to 74c2eff Compare January 5, 2023 16:57
@renanrodrigo renanrodrigo force-pushed the private-apt branch 2 times, most recently from 40cea43 to becd0ac Compare January 6, 2023 13:30
@orndorffgrant orndorffgrant changed the title wip: private apt env (SC-1346) private apt env (SC-1346) Jan 6, 2023
@renanrodrigo renanrodrigo force-pushed the private-apt branch 2 times, most recently from d713171 to 172a6e3 Compare January 9, 2023 07:20
Copy link
Contributor

@julian-klode julian-klode left a comment

Choose a reason for hiding this comment

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

I've only reviewed the apt related bits and not very deeply but I did not see anything wrong there.

Copy link
Contributor

@basak basak left a comment

Choose a reason for hiding this comment

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

This is not a full review. I've taken the code at face value and have assumed that it does what it's intended to do and your testing is adequate.

My comments are all optional - I'd like you to consider them as suggestions for improvement but nothing is a hard blocker. Some of them relate to style that wouldn't be appropriate to fix in this PR anyway.

I appreciate how nicely this is laid out in separate commits. This made it straightforward to review. Thanks!

uaclient/security_status.py Show resolved Hide resolved
lib/upgrade_lts_contract.py Outdated Show resolved Hide resolved
debian/ubuntu-advantage-tools.postinst Outdated Show resolved Hide resolved
uaclient/apt.py Outdated Show resolved Hide resolved
uaclient/entitlements/esm.py Outdated Show resolved Hide resolved
uaclient/apt.py Show resolved Hide resolved
uaclient/entitlements/esm.py Outdated Show resolved Hide resolved
apt-hook/20apt-esm-hook.conf Show resolved Hide resolved
systemd/esm-cache.service Show resolved Hide resolved
apt-hook/json-hook.cc Show resolved Hide resolved
@renanrodrigo renanrodrigo force-pushed the private-apt branch 2 times, most recently from 2ef82d9 to fccf893 Compare January 13, 2023 03:30
Remove all the logic to setup unauthenticated repos
LP: #1990378

Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
renanrodrigo and others added 10 commits January 13, 2023 21:36
Other logic will be used to ensure the esm packages will be advertised

Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
This service is executed as part of the pre-invoke apt hook.

Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
Remove all enable/disable/logic status relying on it

Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
When the user does not have esm enabled, we will need the security_status
module to read esm package information from the private cache.
We are updating the code to support that path
When running the apt dist-upgrade command, we want to remove
the private esm apt cache, since it will be targeting the old
release. When the user runs apt update again, the private apt
cache will be created again for the right release

Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
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.

None yet

5 participants