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

Implement global images #915

Merged
merged 21 commits into from
Apr 21, 2022
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
23 changes: 23 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

79 changes: 77 additions & 2 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ pub enum ResourceType {
Silo,
SiloUser,
ConsoleSession,
GlobalImage,
Organization,
Project,
Dataset,
Expand Down Expand Up @@ -1691,15 +1692,65 @@ pub struct NetworkInterface {
// V6 address, at least one of which must be specified.
}

#[derive(
Clone,
Debug,
Deserialize,
Serialize,
JsonSchema,
Eq,
PartialEq,
Ord,
PartialOrd,
)]
pub enum Digest {
Sha256(String),
}

impl FromStr for Digest {
type Err = anyhow::Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
if s.starts_with("sha256:") {
let parts: Vec<&str> = s.split(':').collect();
if parts.len() != 2 {
anyhow::bail!("digest string {} should have two parts", s);
}

if parts[1].len() != 64 {
anyhow::bail!("sha256 length must be 64");
}

return Ok(Digest::Sha256(parts[1].to_string()));
}

anyhow::bail!("invalid digest string {}", s);
}
}

impl std::fmt::Display for Digest {
fn fmt(
&self,
f: &mut std::fmt::Formatter<'_>,
) -> Result<(), std::fmt::Error> {
write!(
f,
"{}",
match self {
Digest::Sha256(value) => format!("sha256:{}", value),
}
)
}
}

#[cfg(test)]
mod test {
use super::RouteDestination;
use super::RouteTarget;
use super::VpcFirewallRuleHostFilter;
use super::VpcFirewallRuleTarget;
use super::{
ByteCount, L4Port, L4PortRange, Name, RoleName, VpcFirewallRuleAction,
VpcFirewallRuleDirection, VpcFirewallRuleFilter,
ByteCount, Digest, L4Port, L4PortRange, Name, RoleName,
VpcFirewallRuleAction, VpcFirewallRuleDirection, VpcFirewallRuleFilter,
VpcFirewallRulePriority, VpcFirewallRuleProtocol,
VpcFirewallRuleStatus, VpcFirewallRuleUpdate,
VpcFirewallRuleUpdateParams,
Expand Down Expand Up @@ -2176,4 +2227,28 @@ mod test {
assert!("foo:foo".parse::<VpcFirewallRuleHostFilter>().is_err());
assert!("foo".parse::<VpcFirewallRuleHostFilter>().is_err());
}

#[test]
fn test_digest() {
// No prefix
assert!(
"5cc9d1620911c280b0b1dad1413603702baccf340a1e74ade9d0521bcd826acf"
.parse::<Digest>()
.is_err()
);

// Valid sha256
let actual: Digest =
"sha256:5cc9d1620911c280b0b1dad1413603702baccf340a1e74ade9d0521bcd826acf".to_string().parse().unwrap();
assert_eq!(
actual,
Digest::Sha256("5cc9d1620911c280b0b1dad1413603702baccf340a1e74ade9d0521bcd826acf".to_string()),
);

// Too short for sha256
assert!("sha256:5cc9d1620911c280b".parse::<Digest>().is_err());

// Bad prefix
assert!("hash:super_random".parse::<Digest>().is_err());
}
}
35 changes: 30 additions & 5 deletions common/src/sql/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,13 @@ CREATE TABLE omicron.public.image (
/* Indicates that the object has been deleted */
time_deleted TIMESTAMPTZ,

/* Optional project UUID: Images may or may not be global */
project_id UUID,
/* Optional volume ID: Images may exist without backing volumes */
volume_id UUID,
/* Optional URL: Images may be backed by either a URL or a volume */
project_id UUID NOT NULL,
volume_id UUID NOT NULL,

url STRING(8192),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see you removed the "optional" comments here but the fields are still optional. Is that the intent or are they supposed to be NOT NULL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are supposed to be optional

version STRING(64),
digest TEXT,
block_size omicron.public.block_size NOT NULL,
size_bytes INT NOT NULL
);

Expand All @@ -454,6 +455,30 @@ CREATE UNIQUE INDEX on omicron.public.image (
) WHERE
time_deleted is NULL;

CREATE TABLE omicron.public.global_image (
davepacheco marked this conversation as resolved.
Show resolved Hide resolved
/* Identity metadata (resource) */
id UUID PRIMARY KEY,
name STRING(63) NOT NULL,
description STRING(512) NOT NULL,
time_created TIMESTAMPTZ NOT NULL,
time_modified TIMESTAMPTZ NOT NULL,
/* Indicates that the object has been deleted */
time_deleted TIMESTAMPTZ,

volume_id UUID NOT NULL,

url STRING(8192),
version STRING(64),
digest TEXT,
block_size omicron.public.block_size NOT NULL,
size_bytes INT NOT NULL
);

CREATE UNIQUE INDEX on omicron.public.global_image (
name
) WHERE
time_deleted is NULL;

CREATE TABLE omicron.public.snapshot (
/* Identity metadata (resource) */
id UUID PRIMARY KEY,
Expand Down
1 change: 1 addition & 0 deletions nexus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ openapiv3 = "1.0"
regex = "1.5.5"
subprocess = "0.2.8"
term = "0.7"
httptest = "0.15.4"

[dev-dependencies.openapi-lint]
git = "https://github.com/oxidecomputer/openapi-lint"
Expand Down
69 changes: 69 additions & 0 deletions nexus/src/authz/api_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,67 @@ impl AuthorizedResource for ConsoleSessionList {
}
}

#[derive(Clone, Copy, Debug)]
pub struct GlobalImageList;
/// Singleton representing the [`GlobalImageList`] itself for authz purposes
pub const GLOBAL_IMAGE_LIST: GlobalImageList = GlobalImageList;

impl Eq for GlobalImageList {}
impl PartialEq for GlobalImageList {
fn eq(&self, _: &Self) -> bool {
// There is only one GlobalImageList.
true
}
}

impl oso::PolarClass for GlobalImageList {
fn get_polar_class_builder() -> oso::ClassBuilder<Self> {
oso::Class::builder()
.with_equality_check()
.add_attribute_getter("fleet", |_x: &GlobalImageList| FLEET)
}
}

impl AuthorizedResource for GlobalImageList {
fn load_roles<'a, 'b, 'c, 'd, 'e, 'f>(
&'a self,
opctx: &'b OpContext,
datastore: &'c DataStore,
authn: &'d authn::Context,
roleset: &'e mut RoleSet,
) -> futures::future::BoxFuture<'f, Result<(), Error>>
where
'a: 'f,
'b: 'f,
'c: 'f,
'd: 'f,
'e: 'f,
{
// there's no roles related to GlobalImageList, just permissions but we
// still need to load the fleet related roles to find if the actor has
// the "admin" role on the fleet
load_roles_for_resource(
opctx,
datastore,
authn,
ResourceType::Fleet,
*FLEET_ID,
roleset,
)
.boxed()
}

fn on_unauthorized(
&self,
_: &Authz,
error: Error,
_: AnyActor,
_: Action,
) -> Error {
error
}
}

// Main resource hierarchy: Organizations, Projects, and their resources

authz_resource! {
Expand Down Expand Up @@ -389,3 +450,11 @@ authz_resource! {
roles_allowed = false,
polar_snippet = FleetChild,
}

authz_resource! {
name = "GlobalImage",
parent = "Fleet",
primary_key = Uuid,
roles_allowed = false,
polar_snippet = FleetChild,
}
21 changes: 19 additions & 2 deletions nexus/src/authz/omicron.polar
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ has_role(actor: AuthenticatedActor, role: String, resource: Resource)
#
# - fleet.admin (superuser for the whole system)
# - fleet.collaborator (can create and own silos)
# - fleet.viewer (can read fleet-wide data)
# - fleet.viewer (can read fleet-wide data)
# - silo.admin (superuser for the silo)
# - silo.collaborator (can create and own orgs)
# - silo.viewer (can read silo-wide data)
Expand All @@ -91,7 +91,7 @@ has_role(actor: AuthenticatedActor, role: String, resource: Resource)
# the project, but cannot modify or delete the project
# itself)
# - project.viewer (can see everything in the project, but cannot modify
# anything)
# anything)
#

# At the top level is the "Fleet" resource.
Expand Down Expand Up @@ -215,6 +215,23 @@ resource Project {
has_relation(organization: Organization, "parent_organization", project: Project)
if project.organization = organization;

resource GlobalImageList {
permissions = [
"list_children",
"modify",
"create_child",
];

# Only admins can create or modify the global images list
relations = { parent_fleet: Fleet };
"modify" if "admin" on "parent_fleet";
"create_child" if "admin" on "parent_fleet";

# Anyone with viewer can list global images
"list_children" if "viewer" on "parent_fleet";
}
has_relation(fleet: Fleet, "parent_fleet", global_image_list: GlobalImageList)
if global_image_list.fleet = fleet;

# ConsoleSessionList is a synthetic resource used for modeling who has access
# to create sessions.
Expand Down
2 changes: 2 additions & 0 deletions nexus/src/authz/oso_generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result<Oso, anyhow::Error> {
AuthenticatedActor::get_polar_class(),
Database::get_polar_class(),
Fleet::get_polar_class(),
GlobalImageList::get_polar_class(),
ConsoleSessionList::get_polar_class(),
];
for c in classes {
Expand Down Expand Up @@ -70,6 +71,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result<Oso, anyhow::Error> {
Sled::init(),
UpdateAvailableArtifact::init(),
UserBuiltin::init(),
GlobalImage::init(),
];

let polar_config = std::iter::once(OMICRON_AUTHZ_CONFIG_BASE)
Expand Down
Loading