Ubuntu
ubuntu-advantage-tools package

Merge ~renanrodrigo/ubuntu/+source/ubuntu-advantage-tools:upload-27.11-kinetic into ubuntu/+source/ubuntu-advantage-tools:ubuntu/devel

  1. Git
  2. lp:~renanrodrigo/ubuntu/+source/ubuntu-advantage-tools
  3. upload-27.11-kinetic
  4. Merge into ubuntu/devel
Proposed by Renan Rodrigo
Status: Merged
Merge reported by: Renan Rodrigo
Merged at revision: 69ddad099a3f8a03070499406e4fd02dfcc2fb91
Proposed branch: ~renanrodrigo/ubuntu/+source/ubuntu-advantage-tools:upload-27.11-kinetic
Merge into: ubuntu/+source/ubuntu-advantage-tools:ubuntu/devel
Diff against target: 34580 lines (+14190/-5940)
218 files modified
.github/workflows/ci-integration.yaml (+8/-3)
.github/workflows/cla-check.yaml (+12/-0)
.readthedocs.yaml (+14/-0)
CONTRIBUTING.md (+6/-0)
Makefile (+1/-0)
README.md (+33/-32)
apport/source_ubuntu-advantage-tools.py (+29/-0)
apt-hook/hook.cc (+2/-6)
apt-hook/json-hook.cc (+3/-12)
apt-hook/json-hook.test.cc (+22/-22)
debian/changelog (+60/-0)
debian/control (+2/-2)
debian/ubuntu-advantage-tools.links (+2/-0)
debian/ubuntu-advantage-tools.postinst (+26/-3)
dev-docs/explanations/how_auto_attach_works.md (+57/-0)
dev-docs/howtoguides/how_to_use_magic_attach_endpoints.md (+168/-0)
dev-docs/howtoguides/testing.md (+12/-4)
dev-docs/howtoguides/use_staging_environment.md (+13/-0)
dev-docs/references/directory_layout.md (+11/-0)
dev-docs/references/what_happens_during_attach.md (+2/-1)
dev/null (+0/-9)
docs-requirements.txt (+4/-0)
docs/README.md (+11/-0)
docs/conf.py (+52/-0)
docs/explanations/apt_messages.md (+19/-29)
docs/explanations/how_to_interpret_the_security_status_command.md (+7/-0)
docs/explanations/motd_messages.md (+15/-18)
docs/explanations/status_columns.md (+2/-3)
docs/explanations/what_are_the_timer_jobs.md (+1/-1)
docs/howtoguides/create_pro_golden_image.md (+7/-17)
docs/howtoguides/enable_cc.md (+28/-0)
docs/howtoguides/enable_realtime_kernel.md (+62/-0)
docs/howtoguides/enable_ua_in_dockerfile.md (+1/-1)
docs/howtoguides/get_token_and_attach.md (+1/-1)
docs/howtoguides/how_to_run_ua_fix_in_dry_run_mode.md (+1/-1)
docs/howtoguides/how_to_simulate_attach.md (+1/-1)
docs/index.rst (+58/-0)
docs/references/support_matrix.md (+3/-1)
docs/tutorials/basic_ua_commands.md (+6/-7)
docs/tutorials/create_a_fips_updates_pro_cloud_image.md (+65/-0)
docs/tutorials/ua_fix_scenarios.md (+9/-9)
features/_version.feature (+8/-8)
features/api.feature (+52/-0)
features/api_full_auto_attach.feature (+39/-0)
features/api_magic_attach.feature (+74/-0)
features/apt_messages.feature (+328/-0)
features/attach_invalidtoken.feature (+10/-10)
features/attach_validtoken.feature (+37/-170)
features/attached_commands.feature (+189/-240)
features/attached_enable.feature (+251/-183)
features/attached_status.feature (+168/-3)
features/cloud.py (+7/-15)
features/cloud_pro_clone.feature (+65/-0)
features/collect_logs.feature (+4/-14)
features/daemon.feature (+16/-16)
features/detached_auto_attach.feature (+7/-7)
features/docker.feature (+3/-3)
features/enable_fips_cloud.feature (+36/-36)
features/enable_fips_container.feature (+11/-11)
features/enable_fips_pro.feature (+15/-15)
features/enable_fips_vm.feature (+68/-66)
features/environment.py (+104/-77)
features/fix.feature (+58/-60)
features/install_uninstall.feature (+2/-2)
features/motd_messages.feature (+163/-0)
features/proxy_config.feature (+127/-127)
features/realtime_kernel.feature (+49/-12)
features/schemas/api_response.json (+64/-0)
features/schemas/magic_attach.json (+24/-0)
features/schemas/ua_security_status.json (+19/-0)
features/security_status.feature (+542/-0)
features/staging_commands.feature (+6/-53)
features/steps/steps.py (+216/-74)
features/ubuntu_pro.feature (+167/-59)
features/ubuntu_pro_fips.feature (+100/-113)
features/ubuntu_upgrade.feature (+25/-23)
features/ubuntu_upgrade_unattached.feature (+3/-5)
features/unattached_commands.feature (+135/-103)
features/unattached_status.feature (+467/-176)
features/util.py (+67/-42)
help_data.yaml (+10/-11)
integration-requirements.txt (+1/-1)
lib/auto_attach.py (+69/-0)
lib/daemon.py (+2/-2)
lib/patch_status_json.py (+4/-4)
lib/reboot_cmds.py (+10/-8)
lib/timer.py (+7/-2)
lib/upgrade_lts_contract.py (+16/-10)
setup.py (+5/-1)
systemd/ua-auto-attach.service (+2/-1)
tools/create-lp-release-branches.sh (+2/-3)
tools/refresh-aws-pro-ids (+3/-3)
tools/run-integration-tests.py (+37/-27)
tools/test-in-lxd.sh (+2/-2)
tools/ua.bash (+1/-1)
tox.ini (+4/-3)
uaclient-devel.conf (+1/-1)
uaclient.conf (+3/-3)
uaclient/actions.py (+170/-10)
uaclient/api/__init__.py (+0/-0)
uaclient/api/api.py (+144/-0)
uaclient/api/data_types.py (+69/-0)
uaclient/api/errors.py (+61/-0)
uaclient/api/exceptions.py (+40/-0)
uaclient/api/tests/__init__.py (+0/-0)
uaclient/api/tests/test_api.py (+310/-0)
uaclient/api/tests/test_api_u_pro_attach_auto_should_auto_attach.py (+49/-0)
uaclient/api/tests/test_api_u_pro_attach_magic_wait_v1.py (+147/-0)
uaclient/api/tests/test_api_u_pro_version.py (+26/-0)
uaclient/api/tests/test_u_pro_attach_auto_full_auto_attach_v1.py (+161/-0)
uaclient/api/u/__init__.py (+0/-0)
uaclient/api/u/pro/__init__.py (+0/-0)
uaclient/api/u/pro/attach/__init__.py (+0/-0)
uaclient/api/u/pro/attach/auto/__init__.py (+0/-0)
uaclient/api/u/pro/attach/auto/full_auto_attach/__init__.py (+0/-0)
uaclient/api/u/pro/attach/auto/full_auto_attach/v1.py (+149/-0)
uaclient/api/u/pro/attach/auto/should_auto_attach/__init__.py (+0/-0)
uaclient/api/u/pro/attach/auto/should_auto_attach/v1.py (+41/-0)
uaclient/api/u/pro/attach/magic/__init__.py (+0/-0)
uaclient/api/u/pro/attach/magic/initiate/__init__.py (+0/-0)
uaclient/api/u/pro/attach/magic/initiate/v1.py (+55/-0)
uaclient/api/u/pro/attach/magic/revoke/__init__.py (+0/-0)
uaclient/api/u/pro/attach/magic/revoke/v1.py (+39/-0)
uaclient/api/u/pro/attach/magic/wait/__init__.py (+0/-0)
uaclient/api/u/pro/attach/magic/wait/v1.py (+118/-0)
uaclient/api/u/pro/version/__init__.py (+0/-0)
uaclient/api/u/pro/version/v1.py (+39/-0)
uaclient/apt.py (+111/-43)
uaclient/cli.py (+244/-279)
uaclient/clouds/aws.py (+5/-5)
uaclient/clouds/azure.py (+4/-4)
uaclient/clouds/gcp.py (+9/-7)
uaclient/clouds/identity.py (+4/-4)
uaclient/clouds/tests/test_aws.py (+3/-3)
uaclient/clouds/tests/test_azure.py (+6/-6)
uaclient/clouds/tests/test_gcp.py (+9/-7)
uaclient/clouds/tests/test_identity.py (+9/-9)
uaclient/config.py (+56/-257)
uaclient/conftest.py (+30/-9)
uaclient/contract.py (+209/-34)
uaclient/contract_data_types.py (+312/-0)
uaclient/daemon.py (+8/-8)
uaclient/data_types.py (+63/-8)
uaclient/defaults.py (+11/-2)
uaclient/entitlements/__init__.py (+45/-0)
uaclient/entitlements/base.py (+58/-36)
uaclient/entitlements/cc.py (+15/-6)
uaclient/entitlements/cis.py (+3/-2)
uaclient/entitlements/entitlement_status.py (+4/-3)
uaclient/entitlements/esm.py (+36/-12)
uaclient/entitlements/fips.py (+37/-25)
uaclient/entitlements/livepatch.py (+33/-23)
uaclient/entitlements/realtime.py (+16/-8)
uaclient/entitlements/repo.py (+46/-23)
uaclient/entitlements/ros.py (+0/-1)
uaclient/entitlements/tests/conftest.py (+18/-6)
uaclient/entitlements/tests/test_base.py (+49/-14)
uaclient/entitlements/tests/test_cc.py (+30/-18)
uaclient/entitlements/tests/test_cis.py (+5/-4)
uaclient/entitlements/tests/test_esm.py (+32/-37)
uaclient/entitlements/tests/test_fips.py (+68/-48)
uaclient/entitlements/tests/test_livepatch.py (+243/-124)
uaclient/entitlements/tests/test_repo.py (+72/-32)
uaclient/event_logger.py (+8/-16)
uaclient/exceptions.py (+82/-29)
uaclient/files.py (+433/-0)
uaclient/jobs/eol_status.py (+13/-0)
uaclient/jobs/tests/__init__.py (+0/-0)
uaclient/jobs/tests/test_update_messaging.py (+111/-136)
uaclient/jobs/update_messaging.py (+102/-85)
uaclient/lock.py (+2/-2)
uaclient/messages.py (+234/-84)
uaclient/security.py (+66/-31)
uaclient/security_status.py (+498/-74)
uaclient/serviceclient.py (+16/-5)
uaclient/snap.py (+7/-7)
uaclient/status.py (+74/-41)
uaclient/system.py (+479/-0)
uaclient/tests/test_apt.py (+134/-70)
uaclient/tests/test_cli.py (+122/-28)
uaclient/tests/test_cli_api.py (+64/-0)
uaclient/tests/test_cli_attach.py (+30/-26)
uaclient/tests/test_cli_auto_attach.py (+26/-117)
uaclient/tests/test_cli_collect_logs.py (+39/-54)
uaclient/tests/test_cli_config_set.py (+20/-9)
uaclient/tests/test_cli_config_show.py (+24/-10)
uaclient/tests/test_cli_config_unset.py (+18/-7)
uaclient/tests/test_cli_detach.py (+7/-8)
uaclient/tests/test_cli_disable.py (+42/-26)
uaclient/tests/test_cli_enable.py (+38/-17)
uaclient/tests/test_cli_fix.py (+8/-4)
uaclient/tests/test_cli_refresh.py (+17/-14)
uaclient/tests/test_cli_security_status.py (+70/-33)
uaclient/tests/test_cli_status.py (+184/-98)
uaclient/tests/test_config.py (+201/-192)
uaclient/tests/test_contract.py (+457/-22)
uaclient/tests/test_daemon.py (+10/-10)
uaclient/tests/test_data_types.py (+296/-1)
uaclient/tests/test_eol_status.py (+83/-0)
uaclient/tests/test_event_logger.py (+20/-2)
uaclient/tests/test_files.py (+175/-0)
uaclient/tests/test_gpg.py (+5/-5)
uaclient/tests/test_lib_auto_attach.py (+98/-0)
uaclient/tests/test_lock.py (+2/-2)
uaclient/tests/test_patch_status_json.py (+8/-8)
uaclient/tests/test_reboot_cmds.py (+3/-3)
uaclient/tests/test_security.py (+109/-79)
uaclient/tests/test_security_status.py (+149/-53)
uaclient/tests/test_snap.py (+5/-5)
uaclient/tests/test_status.py (+117/-129)
uaclient/tests/test_system.py (+953/-0)
uaclient/tests/test_ua_timer.py (+3/-1)
uaclient/tests/test_upgrade_lts_contract.py (+6/-1)
uaclient/tests/test_util.py (+29/-795)
uaclient/tests/test_version.py (+70/-2)
uaclient/util.py (+67/-449)
uaclient/version.py (+53/-5)
ubuntu-advantage.1 (+41/-39)
Reviewer Review Type Date Requested Status
Robie Basak ubuntu-sru Approve
Paride Legovini (community) Approve
Review via email: mp+429778@code.staging.launchpad.net

Description of the change

Release 27.11 of ubuntu-advantage-tools.

More information about the changes can be found in the SRU bug:
https://bugs.launchpad.net/ubuntu/+source/ubuntu-advantage-tools/+bug/1989279

Packages built from this branch can be found in the `staging` PPA:
https://launchpad.net/~ua-client/+archive/ubuntu/staging

To post a comment you must log in.
Revision history for this message
Robie Basak (racb) wrote :

Has the namespace grab of /usr/bin/pro been approved by somebody considering the archive side - such as an AA? Notably /usr/bin/ua remains "grabbed". Are we OK to continue to grow a list of short names that will potentially collide with Debian essentially forever?

review: Needs Information
Revision history for this message
Robie Basak (racb) wrote :

Somebody also needs to search the archives for anything that might collide with "pro" in PATH please. And another risk that should be noted is that users may have installed "pro" into their PATH either via a local customisation or a third party software source, and this would then collide.

Revision history for this message
Paride Legovini (paride) wrote :

Hi, this diff is quite big, I tried to do a commit by commit review to get a sense of the changes, paying more attention to packaging changes and changes in behavior and less attention to changes that do not affect how to package behaves for users (e.g. tests). The gist of it is:

- New features: they look safe to me from a SRU perspective. The code looks good, so I'm +1 for inclusion in the stable releases (provided that the tests pass).

- Bug fixes: LGTM.

- Renaming from ua to pro: it's a bit confusing in some places, for example the service files and "true" filenames of the cli tool and manpage are still `ubuntu-advantage`, but then users are expected to use "pro", and then `ua` still appears here and there. In any case, given that we had to stay backwards compatible, I think you did the best that was possible to do with the rename.

- File name clash: I share Robie's concern on taking the "pro" name. I searched the archive and I don't think we have conflicts at the moment, but this is a "forever" thing. We should at least think of a way to free the "ua" name at some point, not straightforward as we want to SRU the same package to all the supported releases, but doable.

- File name clash 2: if a stable release user is using "pro" for a locally installed script/package/alias, the new "pro" command may not work as expected, or their script may stop working. Given how PATH is ordered and that `ua` will still be available this risk should be low.

- Packaging changes: the only notable is handling PUBLIC_MACHINE_TOKEN_FILE in postinst. The logic looks good, I tested a couple of use cases and it worked fine.

Overall +1 from me, ready for the next review step!

review: Approve
Revision history for this message
Renan Rodrigo (renanrodrigo) wrote :

Paride, thanks for your review! The renaming to `pro` is indeed the task with most changes, mainly in messaging, and further iterations may come in the future. About the file name, we have now the open discussion with AAs to make sure we are doing it without too much risk.

Revision history for this message
Renan Rodrigo (renanrodrigo) wrote :

Robie, thanks for the comments as well. As we have been discussing, AAs should sign off in the SRU bug that this change is feasible. Besides that, the SRU bug has also been updated ([Discussion] section) to bring the known risks about the new /usr/bin/pro symlink, and it includes a topic about the expectation to not add more names in the future.

Revision history for this message
Renan Rodrigo (renanrodrigo) wrote :

Robie, a new commit has been added disabling the EOL-Check job, as discussed earlier.

Revision history for this message
Robie Basak (racb) wrote :

+1 for SRU. I reviewed up to commit 69ddad0, tree 31ab7922fa25bcae9b4954131d592a12ae3261ff.

Review comments:

Łukasz has signed off on the use of /usr/bin/pro, and Steve has indicated his agreement that it's OK privately. Risk and mitigations have been discussed in the SRU information in the bug. So +1 to proceed on that.

> - "From Ubuntu 20.04 and onwards 'ua enable cis' has been",
> - "replaced by 'ua enable usg'. See more information at:",
> + "From Ubuntu 20.04 and onwards 'pro enable cis' has been",
> + "replaced by 'pro enable usg'. See more information at:",

[a passing comment; not within the scope of my SRU review] Isn't this going to be confusing for a user who invokes "ua enable cis"?

[Already addressed] Commit 8a5091e introduces an entry in sources.list even if a user has not opted-in. This will cause an automated "apt-get update" to start failing for users who operate production services isolated from direct access to our public apt sources. On further discussion, apparently running the postinst after Bionic reaches standard support will do this as well, on the version of ubuntu-advantage-tools already in the archive. I understand this functionality has been disabled, but the existing code that will activate this behaviour in the postinst when Bionic reaches ESM remains. With the functionality disabled, this SRU is not touching the bug, and there's no requirement to block this SRU to fix a bug introduced in a previous SRU, so this is OK for now. However it does need fixing in a future SRU so I filed bug 1990378 to track that.

[Continue if you think it's OK; no need for re-review] Commit 767dfdc: maybe overreaching a little with all timers rather than just ua-tools ones? Are all files being sent definitely bounded in size (via logrotate or equivalent?) Otherwise free disk space might become an issue. Maybe apport itself has that issue too, but no reason to double it.

[Not a blocker for SRU] Commit bd095d4: IMHO, it's preferable to guard all upgrade path code with a test against the version being upgraded from. By doing this from the beginning, it's less risky to remove it in the future. One can simply confirm that no supported users remain on the old version because the guard ensures that newer users are not relying on its behaviour. Easier future cleanup helps minimise complexity in upgrade path code.

review: Approve (ubuntu-sru)
Revision history for this message
Renan Rodrigo (renanrodrigo) wrote :

About the CIS comment, the same would happen to help or CLI hints. We discussed that with Lech and got response that it is fine to have `pro` in the text, as it is the preferred name, even if the user calls `ua`. `ua` should be there just for the backwards compatibility, so people don't need to rush changing scripts and such.

Thanks for filing the APT/ESM bug!

About the logs:
- we do logrotate our logs
- the timers list won't add that much to the tarball size
I'm adding a task to revisit this in the future, but so far we think this is ok.

For the postinst change, we added the integration test for the expected controlled situation where the file would not exist - showing the buggy output, then reconfiguring, and seeing the correct output - and we can't really expect people to not be in old versions... This code will, unfortunately, be there until Jammy goes EOL :/

Revision history for this message
Robie Basak (racb) wrote :

On Wed, Sep 21, 2022 at 01:41:04PM -0000, Renan Rodrigo wrote:
> About the CIS comment, the same would happen to help or CLI hints. We discussed that with Lech and got response that it is fine to have `pro` in the text, as it is the preferred name, even if the user calls `ua`. `ua` should be there just for the backwards compatibility, so people don't need to rush changing scripts and such.

Sure - entirely up to you and Lech as to what's acceptable to you.

> About the logs:
> - we do logrotate our logs
> - the timers list won't add that much to the tarball size
> I'm adding a task to revisit this in the future, but so far we think this is ok.

Some users might see this as outside the scope of debugging ua-tools
though, and therefore a privacy leak. It's a really minor thing but one
that erodes trust. I think the boundary of what is gathered should be
limited to what can be justified as actually needed.

> For the postinst change, we added the integration test for the expected controlled situation where the file would not exist - showing the buggy output, then reconfiguring, and seeing the correct output - and we can't really expect people to not be in old versions... This code will, unfortunately, be there until Jammy goes EOL :/

Sure, but my point is that by that time nobody will remember the
details; it's easier to remove legacy upgrade path code if the code
itself is very obviously gated on an upgrade from an (at the time)
obsolete version. That saves developers at that time from having the
spend the time to ensure that removal won't have any side effect.

Revision history for this message
Renan Rodrigo (renanrodrigo) wrote :

> Some users might see this as outside the scope of debugging ua-tools
> though, and therefore a privacy leak. It's a really minor thing but one
> that erodes trust. I think the boundary of what is gathered should be
> limited to what can be justified as actually needed.

We have a card there to revisit this in the future, and in further releases we can remove what is essentially not needed.

> Sure, but my point is that by that time nobody will remember the
> details; it's easier to remove legacy upgrade path code if the code
> itself is very obviously gated on an upgrade from an (at the time)
> obsolete version. That saves developers at that time from having the
> spend the time to ensure that removal won't have any side effect.

To best effort we can add a comment there stating in which version the change happened.

Revision history for this message
Renan Rodrigo (renanrodrigo) wrote :

This has been merged with a commit matching the same as HEAD from this changeset

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: