From 57681fd345271734de2370b9038328186d060b7b Mon Sep 17 00:00:00 2001 From: Pawel Zmarzly Date: Thu, 21 Nov 2024 04:12:51 -0800 Subject: [PATCH] [antlir] Make uid/gid required Summary: Making sure I didn't miss a spot. Test Plan: Signals Reviewed By: wujj123456 Differential Revision: D66168664 fbshipit-source-id: aecaff71fe1304ae841b678bddec8137a6e6515c --- antlir/antlir2/antlir2_users/src/lib.rs | 2 - .../antlir2_users/src/next_available.rs | 120 ------------------ antlir/antlir2/features/group/group.bzl | 6 +- antlir/antlir2/features/group/group.rs | 12 +- antlir/antlir2/features/user/user.bzl | 8 +- antlir/antlir2/features/user/user.rs | 12 +- 6 files changed, 9 insertions(+), 151 deletions(-) delete mode 100644 antlir/antlir2/antlir2_users/src/next_available.rs diff --git a/antlir/antlir2/antlir2_users/src/lib.rs b/antlir/antlir2/antlir2_users/src/lib.rs index eaf8d1c8496..b44d22b5a2a 100644 --- a/antlir/antlir2/antlir2_users/src/lib.rs +++ b/antlir/antlir2/antlir2_users/src/lib.rs @@ -9,10 +9,8 @@ use std::fmt::Display; use std::fmt::Formatter; pub mod group; -mod next_available; pub mod passwd; pub mod shadow; -pub use next_available::NextAvailableId; #[derive(Debug, thiserror::Error)] pub enum Error { diff --git a/antlir/antlir2/antlir2_users/src/next_available.rs b/antlir/antlir2/antlir2_users/src/next_available.rs deleted file mode 100644 index 0c6a1f0be9e..00000000000 --- a/antlir/antlir2/antlir2_users/src/next_available.rs +++ /dev/null @@ -1,120 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -use std::collections::HashSet; - -use crate::Id; - -pub trait NextAvailableId { - type Id: Id; - type UsedIds: Iterator; - - fn used_ids(&self) -> Self::UsedIds; - - fn next_available_id(&self) -> Option { - let used: HashSet<_> = self.used_ids().map(Self::Id::into_raw).collect(); - // these numbers is semi-magical, but matches the defaults in - // /etc/login.defs for choosing new ids - for candidate in 1000..60000 { - if !used.contains(&candidate) { - return Some(Self::Id::from_raw(candidate)); - } - } - None - } -} - -impl NextAvailableId for crate::passwd::EtcPasswd<'_> { - type Id = crate::UserId; - type UsedIds = as IntoIterator>::IntoIter; - - fn used_ids(&self) -> Self::UsedIds { - self.records() - .map(|r| r.uid) - .collect::>() - .into_iter() - } -} - -impl NextAvailableId for crate::group::EtcGroup<'_> { - type Id = crate::GroupId; - type UsedIds = as IntoIterator>::IntoIter; - - fn used_ids(&self) -> Self::UsedIds { - self.records() - .map(|r| r.gid) - .collect::>() - .into_iter() - } -} - -#[cfg(test)] -mod tests { - use std::path::Path; - - use super::*; - use crate::group::EtcGroup; - use crate::group::GroupRecord; - use crate::passwd::EtcPasswd; - use crate::passwd::UserRecord; - use crate::GroupId; - use crate::Password; - use crate::UserId; - - #[test] - fn ids_from_correct_range() { - let passwd = EtcPasswd::parse( - r#"root:x:0:0:root:/root:/bin/bash -bin:x:1:1:bin:/bin:/sbin/nologin -nobody:x:65534:65534:Kernel Overflow User:/:/sbin/nologin -"#, - ) - .expect("failed to parse passwd"); - assert_eq!(Some(UserId::from(1000)), passwd.next_available_id()); - let groups = EtcGroup::parse( - r#"root:x:0: -bin:x:1:root,daemon -nobody:x:65534: -"#, - ) - .expect("failed to parse group"); - assert_eq!(Some(GroupId::from(1000)), groups.next_available_id()); - } - - #[test] - fn id_space_exhausted() { - let mut passwd = EtcPasswd::default(); - let mut group = EtcGroup::default(); - for id in 0..=60000 { - passwd.push(UserRecord { - name: format!("u{id}").into(), - password: Password::Shadow, - uid: id.into(), - gid: id.into(), - comment: "".into(), - homedir: Path::new("/").into(), - shell: Path::new("/bin/nologin").into(), - }); - group.push(GroupRecord { - name: format!("u{id}").into(), - password: Password::Shadow, - gid: id.into(), - users: Vec::new(), - }); - } - assert_eq!( - passwd.next_available_id(), - None, - "id space was exhausted, no uid should be possible" - ); - assert_eq!( - group.next_available_id(), - None, - "id space was exhausted, no gid should be possible" - ); - } -} diff --git a/antlir/antlir2/features/group/group.bzl b/antlir/antlir2/features/group/group.bzl index 1004260cbac..fa354c29b9d 100644 --- a/antlir/antlir2/features/group/group.bzl +++ b/antlir/antlir2/features/group/group.bzl @@ -7,8 +7,8 @@ load("//antlir/antlir2/features:feature_info.bzl", "ParseTimeFeature", "data_onl def group_add( *, - groupname: str | Select, - gid: int | Select | None = None): + gid: int | Select, + groupname: str | Select): """ Add a group entry to /etc/group @@ -26,7 +26,7 @@ def group_add( group_rule = data_only_feature_rule( feature_attrs = { - "gid": attrs.option(attrs.int(), default = None), + "gid": attrs.int(), "groupname": attrs.string(), }, feature_type = "group", diff --git a/antlir/antlir2/features/group/group.rs b/antlir/antlir2/features/group/group.rs index c13f0536073..91a76231e37 100644 --- a/antlir/antlir2/features/group/group.rs +++ b/antlir/antlir2/features/group/group.rs @@ -13,7 +13,6 @@ use antlir2_depgraph_if::Requirement; use antlir2_depgraph_if::Validator; use antlir2_features::types::GroupName; use antlir2_users::group::GroupRecord; -use antlir2_users::NextAvailableId; use antlir2_users::Password; use anyhow::Context; use serde::Deserialize; @@ -46,16 +45,7 @@ impl antlir2_compile::CompileFeature for Group { #[tracing::instrument(skip(ctx), ret, err)] fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> { let mut groups_db = ctx.groups_db()?; - let gid = match self.gid { - Some(gid) => gid.into(), - None => { - let gid = groups_db - .next_available_id() - .context("no more gids available")?; - tracing::trace!("next available gid = {gid}"); - gid - } - }; + let gid = self.gid.context("gid is required")?.into(); let record = GroupRecord { name: self.groupname.to_owned().into(), password: Password::Shadow, diff --git a/antlir/antlir2/features/user/user.bzl b/antlir/antlir2/features/user/user.bzl index 53dd9039307..1501c4072ed 100644 --- a/antlir/antlir2/features/user/user.bzl +++ b/antlir/antlir2/features/user/user.bzl @@ -13,11 +13,11 @@ SHELL_NOLOGIN = "/sbin/nologin" def user_add( *, + uid: int | Select, username: str | Select, primary_group: str | Select, home_dir: str | Select, shell: str | Select = SHELL_NOLOGIN, - uid: int | Select | None = None, supplementary_groups: list[str | Select] | Select = [], comment: str | None = None): """ @@ -67,12 +67,12 @@ def user_add( ) def standard_user( + uid: int, username: str, + gid: int, groupname: str, home_dir: str | None = None, shell: str = SHELL_BASH, - uid: int | None = None, - gid: int | None = None, supplementary_groups: list[str] = []): """ A convenient function that wraps `group_add`, `user_add`, @@ -110,7 +110,7 @@ user_rule = data_only_feature_rule( "primary_group": attrs.string(), "shell": attrs.string(), "supplementary_groups": attrs.list(attrs.string()), - "uid": attrs.option(attrs.int(), default = None), + "uid": attrs.int(), "username": attrs.string(), }, feature_type = "user", diff --git a/antlir/antlir2/features/user/user.rs b/antlir/antlir2/features/user/user.rs index d805fb194de..aa284d5c326 100644 --- a/antlir/antlir2/features/user/user.rs +++ b/antlir/antlir2/features/user/user.rs @@ -19,7 +19,6 @@ use antlir2_features::types::GroupName; use antlir2_features::types::PathInLayer; use antlir2_features::types::UserName; use antlir2_users::passwd::UserRecord; -use antlir2_users::NextAvailableId; use antlir2_users::Password; use anyhow::Context; use serde::Deserialize; @@ -75,16 +74,7 @@ impl antlir2_compile::CompileFeature for User { #[tracing::instrument(name = "user", skip(ctx), ret, err)] fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> { let mut user_db = ctx.user_db()?; - let uid = match self.uid { - Some(uid) => uid.into(), - None => { - let uid = user_db - .next_available_id() - .context("no more uids available")?; - tracing::trace!("next available uid = {uid}"); - uid - } - }; + let uid = self.uid.context("uid is required")?.into(); let record = UserRecord { name: self.username.clone().into(), password: Password::Shadow,