Skip to content

[WIP] Platform/oci golden vm#13

Closed
rstarmer wants to merge 12 commits intoartificialwisdomai:mainfrom
rstarmer:platform/oci-golden-vm
Closed

[WIP] Platform/oci golden vm#13
rstarmer wants to merge 12 commits intoartificialwisdomai:mainfrom
rstarmer:platform/oci-golden-vm

Conversation

@rstarmer
Copy link
Member

adds cloud-init (needed for ssh key additions)
removes packer user password
adds serial interface grub config
checks for persistent network (mac) and removes it
minor tweaks to packer build (shutdown path)

rstarmer and others added 3 commits May 9, 2023 12:15
add cloud-init to support auto-import of ssh keys
add setup script to remove injected password
add ssh-keys from @sdake and @rstarmer for packer user

add draft script to upload built image to Oracle
@rstarmer rstarmer requested a review from sdake as a code owner May 14, 2023 15:54
@cla-bot cla-bot bot added the CLA CLA signed? label May 14, 2023
@rstarmer
Copy link
Member Author

Fixes #11 and #12

Copy link
Member

@sdake sdake left a comment

Choose a reason for hiding this comment

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

No problem merging this now, once the --delete problem is fixed. I do think we should be using Ansible as a provisioner (rather than bash). I'll submit a few PRs for this.

mkdir /home/packer/.ssh
chmod 700 /home/packer/.ssh
curl -sLo - https://github.com/sdake.keys >> /home/packer/.ssh/authorized_keys
curl -sLo - https://github.com/rstarmer.keys >> /home/packer/.ssh/authorized_keys
Copy link
Member

Choose a reason for hiding this comment

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

I am not really sure how we should handle dynamic secret management. For now, I have started a wiki entry so we know where our secrets are stored or consumed in our repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can enable this as a provisioning step. If we're goign to use Ansible, then it would likely be easier to add an ansible provisioner that loads the right keys at build time, or even better at image deploy time so that there is nothing static in the base/golden image.

Copy link
Member

Choose a reason for hiding this comment

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

Right. I think some things should be dynamic, and the rest should be baked (with packer). Dynamically loaded credentials make sense, far more sense then static.

But, I spent like 5+ years working with people at Red Hat that battled and battled over this credential issue. My recommendation - tech debt it for now, we will sort out the workflow later.

Another way to introduce SSH credentials is via the kernel command line.

cat > /tmp/fixes.sh <<EOF
#!/bin/bash
set -x
echo 'GRUB_CMDLINE_LINUX="console=ttyS0"' >> /etc/default/grub
Copy link
Member

Choose a reason for hiding this comment

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

better to put in /etc/default/grub.d. I will submit an update.

Copy link
Member Author

Choose a reason for hiding this comment

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

This may not even work, still trying to get a testable environment for dev (I think I can do this in OCI now, but I haven't yet managed to get a connection through the UI or via oci cli yet).


echo 'packer ALL=(ALL) NOPASSWD:ALL'>> /etc/sudoers.d/91-packer

passwd -d packer
Copy link
Member

Choose a reason for hiding this comment

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

I had to read the man page, and I was still not sure what --delete does. So, I tried it, and the result was

sdake@beast08 ~ [1]> sudo passwd -d sdake
passwd: password expiry information changed.

in this case, it appears to have expired my password, which I guess is reasonable.

Is the goal your really after to remove the ability to login with the password (but with ssh?)

In that case, I think you want

passwd --lock packer

from man page:

       -l, --lock
           Lock the password of the named account. This option disables a password
           by changing it to a value which matches no possible encrypted value (it
           adds a ´!´ at the beginning of the password).

           Note that this does not disable the account. The user may still be able
           to login using another authentication token (e.g. an SSH key). To disable
           the account, administrators should use usermod --expiredate 1 (this set
           the account's expire date to Jan 2, 1970).

           Users with a locked password are not allowed to change their password.

Copy link
Member

Choose a reason for hiding this comment

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

@rstarmer this needs a change to resolve. I recommend --lock.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that makes sense. I looked at the passwd/shadow files, and it "erases" the password, which would presumably make it impossible to login via a password, but would not expressly prohibit setting a password. And with passwordless Sudo, you could potentially set a password via su anyway.

Copy link
Member

Choose a reason for hiding this comment

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

this is important, but not worth slowing down the PR. Can you file a tech debt bug?

@@ -0,0 +1,28 @@
cd build
Copy link
Member

Choose a reason for hiding this comment

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

comments here and there would be cool - not. really sure what this does without a deep dive into the man pages..

hard_drive_interface = "virtio"
rtc_time_base = "UTC"
shutdown_command = "echo '${var.ssh_password}' | sudo -S shutdown -P now"
shutdown_command = "echo '${var.ssh_password}' | sudo -S /usr/sbin/shutdown -P now"
Copy link
Member

Choose a reason for hiding this comment

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

/usr/sbin not in the root path? Doesn't matter for the purposes of merging the PR, just seems odd. :)

Copy link
Member

Choose a reason for hiding this comment

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

@rstarmer can you respond on these questions? I am trying to learn along the way (this feels wrong, which means /etc/environment may not be right).

Copy link
Member Author

Choose a reason for hiding this comment

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

This was to resolve an issue with running shutdown, and I'm not sure why the environment worked initially, and then didn't. I'll do a bit more debugging, and I agree, I've not seen a single example where the full path to shutdown was required.

Copy link
Member

Choose a reason for hiding this comment

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

all good then. Perhaps a debt tech bug would help us keep track of it for new developer low hanging fruit.

rstarmer and others added 9 commits May 14, 2023 15:17
WORK_REQUEST_ID needed to have ""s stripped
Work Request matches SUCCEEDED not SUCCESS
The provisioners need not be this finegrained, just that tasks are.
add cloud-init to support auto-import of ssh keys
add setup script to remove injected password
add ssh-keys from @sdake and @rstarmer for packer user

add draft script to upload built image to Oracle
WORK_REQUEST_ID needed to have ""s stripped
Work Request matches SUCCEEDED not SUCCESS
@sdake
Copy link
Member

sdake commented May 15, 2023

@rstarmer was this rdy for review?

cheers,
-steve

hard_drive_interface = "virtio"
rtc_time_base = "UTC"
shutdown_command = "echo '${var.ssh_password}' | sudo -S shutdown -P now"
shutdown_command = "echo '${var.ssh_password}' | sudo -S /usr/sbin/shutdown -P now"
Copy link
Member

Choose a reason for hiding this comment

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

@rstarmer can you respond on these questions? I am trying to learn along the way (this feels wrong, which means /etc/environment may not be right).

@@ -0,0 +1,7 @@
- hosts: all
Copy link
Member

Choose a reason for hiding this comment

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

strange, I think this shouldn't be here.

@@ -0,0 +1,10 @@
- name: "Get installed packages"
Copy link
Member

Choose a reason for hiding this comment

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

try removing.

@@ -0,0 +1,157 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

this looks like a rebase gone wrong. Probably as a result of my force push error to main...


echo 'packer ALL=(ALL) NOPASSWD:ALL'>> /etc/sudoers.d/91-packer

passwd -d packer
Copy link
Member

Choose a reason for hiding this comment

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

@rstarmer this needs a change to resolve. I recommend --lock.

Copy link
Member

@sdake sdake left a comment

Choose a reason for hiding this comment

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

pasword needs --lock I believe.

@rstarmer rstarmer changed the title Platform/oci golden vm WIP: Platform/oci golden vm May 15, 2023
@rstarmer rstarmer changed the title WIP: Platform/oci golden vm [WIP] Platform/oci golden vm May 15, 2023
Copy link
Member

@sdake sdake left a comment

Choose a reason for hiding this comment

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

line 106 may be broken. (after reading the virtualbox provisioner and vbox source code for hours), I believe this is correct. This uses the natnetwork created by vbox/start.sh to expose an ssh port, that can then be used by the ssh connector.

  vboxmanage = [
    [ "modifyvm", "{{.Name}}", "--recording", "on" ],
    [ "modifyvm", "{{.Name}}", "--nic1", "natnetwork" ]
  ]

hard_drive_interface = "virtio"
rtc_time_base = "UTC"
shutdown_command = "echo '${var.ssh_password}' | sudo -S shutdown -P now"
shutdown_command = "echo '${var.ssh_password}' | sudo -S /usr/sbin/shutdown -P now"
Copy link
Member

Choose a reason for hiding this comment

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

all good then. Perhaps a debt tech bug would help us keep track of it for new developer low hanging fruit.


echo 'packer ALL=(ALL) NOPASSWD:ALL'>> /etc/sudoers.d/91-packer

passwd -d packer
Copy link
Member

Choose a reason for hiding this comment

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

this is important, but not worth slowing down the PR. Can you file a tech debt bug?

mkdir /home/packer/.ssh
chmod 700 /home/packer/.ssh
curl -sLo - https://github.com/sdake.keys >> /home/packer/.ssh/authorized_keys
curl -sLo - https://github.com/rstarmer.keys >> /home/packer/.ssh/authorized_keys
Copy link
Member

Choose a reason for hiding this comment

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

Right. I think some things should be dynamic, and the rest should be baked (with packer). Dynamically loaded credentials make sense, far more sense then static.

But, I spent like 5+ years working with people at Red Hat that battled and battled over this credential issue. My recommendation - tech debt it for now, we will sort out the workflow later.

Another way to introduce SSH credentials is via the kernel command line.

@rstarmer rstarmer closed this May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA CLA signed?

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants