Skip to content

Bugfix/ua assume yes 1954842#1158

Merged
blackboxsw merged 2 commits into
canonical:mainfrom
j5awry:bugfix/ua-assume-yes-1954842
Jan 4, 2022
Merged

Bugfix/ua assume yes 1954842#1158
blackboxsw merged 2 commits into
canonical:mainfrom
j5awry:bugfix/ua-assume-yes-1954842

Conversation

@j5awry
Copy link
Copy Markdown
Contributor

@j5awry j5awry commented Dec 15, 2021

Proposed Commit Message

   Update cc_ubuntu_advantage calls to assume-yes
    
   cloud-init currently makes calls to ubuntu_advantage without assume-yes.
   some ua enable commands, such as ua enable fips, have prompts. In an
   automated environment, calling ua enable without --assume-yes will
   result in errors and not applying the change. This sets --assume-yes by
   default for all enable commands. This capability was added two years ago
   in ua commit 576e605ceb5f so should be safe for use in all systems at
   this time.

   LP: #1954842

Additional Context

This was raised by a Canonical Public Cloud partner as a possible bug when suggested to an end-user as a possible way of running FIPS enablement automatically in their cloud.

Test Steps

cloud-init snippet that needs added for a test:

ubuntu-advantage:
  token: <ua_contract_token>
  enable:
  - fips

This will require a valid token.

Old behaviour -- cloud-init should fail as ua enable fips will raise a prompt and no interaction will happen.

New behaviour -- cloud-init will finish and fips will be enabled on the image. note because ua enable fips requires a restart to take affect, the image itself may not be in fips mode until a restart. However this PR only addresses the failure caused by raising an interactive prompt in a non-interactive setting.

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly
    • As a bugfix, there is no documentation to change. It's documented as working already :)

Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Thanks for this @j5awry as a Canonical contribution. Changeset looks fine. We've just blackened/isorted cloud-init now, so you'll probably need a git fetch upstream; git rebase -i upstream/main to sort merge conflicts and a final tox -e format now to make any formatting changes to your commit as well.

I can confirm failure without your patch: using the following

$ cat > test.yaml <<EOF
#cloud-config   
ubuntu_advantage:
   token: <REDACTED>
   enable: [fips, cis]
EOF
$ lxc launch ubuntu-daily:bionic -p pycloudlib-vm-bionic-v4 --vm test-it -c user.user-data="$(cat 1.yaml)"
$ lxc exec test-it -- cloud-init status --long --wait
status: error
time: Thu, 16 Dec 2021 02:32:36 +0000
detail:
('ubuntu-advantage', RuntimeError('Failure enabling Ubuntu Advantage service(s): "fips"',))

And confirmed success with the patch:

lxc exec test-it -- cloud-init status --long --wait
.......................................................................................................................................................status: done
time: Thu, 16 Dec 2021 03:09:24 +0000
detail:
DataSourceNoCloud [seed=/var/lib/cloud/seed/nocloud-net][dsmode=net]

# commands passed to UA with --assume-yes to every enabled service, even CIS which doesn't have prompts
$ sudo grep assume /var/log/ubuntu-advantage.log  | grep -v apt
2021-12-15 20:07:05,209 - cli.py:(1529) [DEBUG]: Executed with sys.argv: ['/usr/bin/ua', 'enable', '--assume-yes', 'fips']
2021-12-15 20:09:07,033 - cli.py:(1529) [DEBUG]: Executed with sys.argv: ['/usr/bin/ua', 'enable', '--assume-yes', 'cis']
$ ua status
ubuntu@test-it:~$ sudo ua status
SERVICE       ENTITLED  STATUS    DESCRIPTION
cc-eal        yes       n/a       Common Criteria EAL2 Provisioning Packages
cis           yes       enabled   Center for Internet Security Audit Tools
esm-apps      yes       enabled   UA Apps: Extended Security Maintenance (ESM)
esm-infra     yes       enabled   UA Infra: Extended Security Maintenance (ESM)
fips          yes       enabled   NIST-certified core packages
fips-updates  yes       disabled  NIST-certified core packages with priority security updates
livepatch     yes       n/a       Canonical Livepatch service

NOTICES
FIPS support requires system reboot to complete configuration.

Enable services with: ua enable <service>

                Account: Canonical - staff
           Subscription: UA Applications - Essential (Virtual)
            Valid until: 3999-12-31 00:00:00+00:00
Technical support level: essential

Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Just tagging this to wait on resolving git merge conflicts.

j5awry is John Chittum, employee at Canonical as of time of signing.
Addresses LP: 1954842
cloud-init currently makes calls to ubuntu_advantage without assume-yes.
some ua enable commands, such as ua enable fips, have prompts. In an
automated environment, calling ua enable without --assume-yes will
result in errors and not applying the change. This sets --assume-yes by
default for all enable commands. This capability was added two years ago
in ua commit 576e605ceb5f so should be safe for use in all systems at
this time.
@j5awry j5awry force-pushed the bugfix/ua-assume-yes-1954842 branch from ccaf97c to dd14001 Compare December 16, 2021 20:54
@j5awry
Copy link
Copy Markdown
Contributor Author

j5awry commented Dec 16, 2021

force pushed a rebase after the formatting and linting change. Also includes running the formatting and linting on the branch.

@github-actions
Copy link
Copy Markdown

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging mitechie, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag mitechie to reopen it.)

@github-actions github-actions Bot added the stale-pr Pull request is stale; will be auto-closed soon label Dec 31, 2021
@TheRealFalcon TheRealFalcon removed the stale-pr Pull request is stale; will be auto-closed soon label Dec 31, 2021
Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Thanks for this @j5awry! Welcome to cloud-init.

@blackboxsw blackboxsw merged commit 137c9b0 into canonical:main Jan 4, 2022
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.

3 participants