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

Add host container persistent storage & persist admin ctr ssh host keys #450

Merged
merged 3 commits into from
Oct 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions extras/admin-container/sshd_config
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
HostKey /etc/ssh/ssh_host_rsa_key
HostKey /etc/ssh/ssh_host_ecdsa_key
HostKey /etc/ssh/ssh_host_ed25519_key
HostKey /.thar/host-containers/admin/etc/ssh/ssh_host_rsa_key
HostKey /.thar/host-containers/admin/etc/ssh/ssh_host_ecdsa_key
HostKey /.thar/host-containers/admin/etc/ssh/ssh_host_ed25519_key

PasswordAuthentication no

Expand Down
4 changes: 2 additions & 2 deletions extras/admin-container/start_admin_sshd.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ set -e

mkdir -p /home/ec2-user/.ssh/
chmod 700 /home/ec2-user/.ssh/
ssh_host_key_dir="/etc/ssh"
ssh_host_key_dir="/.thar/host-containers/admin/etc/ssh"
ssh_config_dir="/home/ec2-user/.ssh"

# Populate authorized_keys with all the public keys found in instance meta-data
Expand Down Expand Up @@ -39,6 +39,7 @@ fi
chown ec2-user -R "${ssh_config_dir}"

# Generate the server keys
mkdir -p "${ssh_host_key_dir}"
for key in rsa ecdsa ed25519; do
# If both of the keys exist, don't overwrite them
if [ -s "${ssh_host_key_dir}/ssh_host_${key}_key" ] && [ -s "${ssh_host_key_dir}/ssh_host_${key}_key.pub" ]; then
Expand All @@ -49,7 +50,6 @@ for key in rsa ecdsa ed25519; do
rm -rf \
"${ssh_host_key_dir}/ssh_host_${key}_key" \
"${ssh_host_key_dir}/ssh_host_${key}_key.pub"
# Generate new host keys. This would happen every time this script is invoked when we start the admin container.
if ssh-keygen -t "${key}" -f "${ssh_host_key_dir}/ssh_host_${key}_key" -q -N ""; then
chmod 600 "${ssh_host_key_dir}/ssh_host_${key}_key"
chmod 644 "${ssh_host_key_dir}/ssh_host_${key}_key.pub"
Expand Down
3 changes: 3 additions & 0 deletions packages/workspaces/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ BindsTo=host-containerd.service
[Service]
Type=simple
EnvironmentFile=/etc/host-containers/%i.env
Environment=LOCAL_DIR=/local
# Create directories for container persistent storage
ExecStartPre=/usr/bin/mkdir -m 1777 -p ${LOCAL_DIR}/host-containers/%i
ExecStart=/usr/bin/host-ctr -ctr-id='%i' -source='${CTR_SOURCE}' -superpowered='${CTR_SUPERPOWERED}'
Restart=always
RestartSec=10
Expand Down
4 changes: 2 additions & 2 deletions workspaces/api/apiserver/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use std::net::Ipv4Addr;

use crate::modeled_types::{SingleLineString, ValidBase64};
use crate::modeled_types::{SingleLineString, ValidBase64, Identifier};

///// Primary user-visible settings

Expand All @@ -32,7 +32,7 @@ pub struct Settings {
pub updates: Option<UpdatesSettings>,

#[serde(skip_serializing_if = "Option::is_none")]
pub host_containers: Option<HashMap<SingleLineString, ContainerImage>>,
pub host_containers: Option<HashMap<Identifier, ContainerImage>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this require a migration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe so? Both SingleLineString and Identifier will accept what we're passing currently for container names, e.g. admin and control.

Copy link
Contributor

Choose a reason for hiding this comment

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

They accept what we're currently passing, but Identifier is more restrictive than SingleLineString, and we don't know what our customers are passing to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're right, a sufficiently interesting value here could fail to parse.

Copy link
Contributor

Choose a reason for hiding this comment

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

While technically right, I'm not sure it's worth writing a migration here; we don't expect this has been used yet, and we'd have to mangle the user's naming tremendously to have a backwards-capable migration.


#[serde(skip_serializing_if = "Option::is_none")]
pub ntp: Option<NtpSettings>,
Expand Down
100 changes: 100 additions & 0 deletions workspaces/api/apiserver/src/modeled_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,103 @@ mod test_single_line_string {
// paragraph separator
}
}

// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^=

/// Identifier can only be created by deserializing from a string that contains
/// ASCII alphanumeric characters, plus hyphens, which we use as our standard word separator
/// character in user-facing identifiers. It stores the original form and makes it accessible
/// through standard traits. Its purpose is to validate input for identifiers like container names
/// that might be used to create files/directories.
#[derive(Debug, Clone, Eq, PartialEq, Hash)]
pub struct Identifier {
inner: String,
}

impl<'de> Deserialize<'de> for Identifier {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let original = String::deserialize(deserializer)?;

if !original
.chars()
.all(|c| (c.is_ascii() && c.is_alphanumeric()) || c == '-')
{
Err(D::Error::custom(format!(
"Identifiers may only contain ASCII alphanumerics plus hyphens; received '{}'",
original,
)))
} else {
Ok(Identifier { inner: original })
}
}
}

/// Serialize the original string back out.
impl Serialize for Identifier {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_str(&self.inner)
}
}

impl Deref for Identifier {
type Target = str;
fn deref(&self) -> &Self::Target {
&self.inner
}
}

impl Borrow<String> for Identifier {
fn borrow(&self) -> &String {
&self.inner
}
}

impl Borrow<str> for Identifier {
fn borrow(&self) -> &str {
&self.inner
}
}

impl AsRef<str> for Identifier {
fn as_ref(&self) -> &str {
&self.inner
}
}

impl fmt::Display for Identifier {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.inner)
}
}

#[cfg(test)]
mod test_valid_identifier {
use super::Identifier;

#[test]
fn valid_identifier() {
assert!(serde_json::from_str::<Identifier>("\"hello-world\"").is_ok());
assert!(serde_json::from_str::<Identifier>("\"helloworld\"").is_ok());
assert!(serde_json::from_str::<Identifier>("\"123321hello\"").is_ok());
assert!(serde_json::from_str::<Identifier>("\"hello-1234\"").is_ok());
assert!(serde_json::from_str::<Identifier>("\"--------\"").is_ok());
assert!(serde_json::from_str::<Identifier>("\"11111111\"").is_ok());
}

#[test]
fn invalid_identifier() {
assert!(serde_json::from_str::<Identifier>("\"../\"").is_err());
assert!(serde_json::from_str::<Identifier>("\"{}\"").is_err());
assert!(serde_json::from_str::<Identifier>("\"hello|World\"").is_err());
assert!(serde_json::from_str::<Identifier>("\"hello\nWorld\"").is_err());
assert!(serde_json::from_str::<Identifier>("\"hello_world\"").is_err());
assert!(serde_json::from_str::<Identifier>("\"タール\"").is_err());
assert!(serde_json::from_str::<Identifier>("\"💝\"").is_err());
}
}
4 changes: 2 additions & 2 deletions workspaces/api/host-containers/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::path::{Path, PathBuf};
use std::process::{self, Command};

use apiserver::model;
use apiserver::modeled_types::SingleLineString;
use apiserver::modeled_types::Identifier;

#[macro_use]
extern crate tracing;
Expand Down Expand Up @@ -111,7 +111,7 @@ mod error {
type Result<T> = std::result::Result<T, error::Error>;

/// Query the API for the currently defined host containers
fn get_host_containers<P>(socket_path: P) -> Result<HashMap<SingleLineString, model::ContainerImage>>
fn get_host_containers<P>(socket_path: P) -> Result<HashMap<Identifier, model::ContainerImage>>
where
P: AsRef<Path>,
{
Expand Down
2 changes: 1 addition & 1 deletion workspaces/api/storewolf/defaults.toml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ val = "bork seed"

[settings.host-containers.admin]
enabled = false
source = "328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-admin:v0.1"
source = "328549459982.dkr.ecr.us-west-2.amazonaws.com/thar-admin:v0.2"
superpowered = true

[settings.host-containers.control]
Expand Down
6 changes: 6 additions & 0 deletions workspaces/host-ctr/cmd/host-ctr/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ func _main() int {
Options: []string{"bind", "ro"},
Destination: "/usr/local/bin/apiclient",
Source: "/usr/bin/apiclient",
},
// Mount in the persistent storage location for this container
{
Options: []string{"rbind", "rw"},
Destination: "/.thar/host-containers/" + targetCtr,
Source: "/local/host-containers/" + targetCtr,
}}),
withSuperpowered(superpowered),
)
Expand Down