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 constants crate #1709

Merged
merged 3 commits into from
Aug 18, 2021
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 sources/Cargo.lock

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

2 changes: 2 additions & 0 deletions sources/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ members = [
"updater/updog",

"webpki-roots-shim",

"constants"
]

[profile.release]
Expand Down
1 change: 1 addition & 0 deletions sources/api/apiclient/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ build = "build.rs"
exclude = ["README.md"]

[dependencies]
constants = { path = "../../constants" }
datastore = { path = "../datastore" }
futures = { version = "0.3", default-features = false }
http = "0.2"
Expand Down
6 changes: 3 additions & 3 deletions sources/api/apiclient/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ use std::env;
use std::process;
use std::str::FromStr;
use unindent::unindent;
use constants;

const DEFAULT_API_SOCKET: &str = "/run/api.sock";
const DEFAULT_METHOD: &str = "GET";

/// Stores user-supplied global arguments.
Expand All @@ -33,7 +33,7 @@ impl Default for Args {
fn default() -> Self {
Self {
log_level: LevelFilter::Info,
socket_path: DEFAULT_API_SOCKET.to_string(),
socket_path: constants::API_SOCKET.to_string(),
}
}
}
Expand Down Expand Up @@ -151,7 +151,7 @@ fn usage() -> ! {

update cancel options:
None."#,
socket = DEFAULT_API_SOCKET,
socket = constants::API_SOCKET,
method = DEFAULT_METHOD,
);
eprintln!("{}", unindent(msg));
Expand Down
2 changes: 1 addition & 1 deletion sources/api/apiserver/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ mod error;
pub use error::Error;

use actix_web::{
body::Body, error::ResponseError, web, App, BaseHttpResponse, FromRequest, HttpRequest,
body::Body, error::ResponseError, web, App, FromRequest, HttpRequest,
HttpResponse, HttpServer, Responder,
};
use bottlerocket_release::BottlerocketRelease;
Expand Down
1 change: 1 addition & 0 deletions sources/api/bootstrap-containers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ exclude = ["README.md"]

[dependencies]
apiclient = { path = "../apiclient" }
constants = { path = "../../constants" }
datastore = { path = "../datastore" }
base64 = "0.13"
http = "0.2"
Expand Down
26 changes: 10 additions & 16 deletions sources/api/bootstrap-containers/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,15 @@ use std::fs;
use std::path::Path;
use std::process::{self, Command};
use std::str::FromStr;
use constants;
Copy link
Contributor

@webern webern Aug 12, 2021

Choose a reason for hiding this comment

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

I would probably import the constant names instead of qualifying them everywhere. (Same in all places...)

Suggested change
use constants;
use constants::{API_SOCKET, HOST_CTR_BIN, SYSTEMCTL_BIN};

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 this is a case where the opposite is justifiable - qualified constants make it clear that they're not crate-specific, where unqualified ones look like "standard" constants you'd expect in the same file.

Copy link
Contributor

Choose a reason for hiding this comment

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

qualified constants make it clear that they're not crate-specific, where unqualified ones look like "standard" constants you'd expect in the same file.

I guess I don't have the expectation that constants are in the same file. To me they're like all other symbols. That is, I have to jump to definition to know where they are. Not a big deal though.


use model::modeled_types::{BootstrapContainerMode, Identifier};

// FIXME Get from configuration in the future
const DEFAULT_API_SOCKET: &str = "/run/api.sock";
const API_SETTINGS_URI: &str = "/settings";

const ENV_FILE_DIR: &str = "/etc/bootstrap-containers";
const DROPIN_FILE_DIR: &str = "/etc/systemd/system";
const PERSISTENT_STORAGE_DIR: &str = "/local/bootstrap-containers";
const DROP_IN_FILENAME: &str = "overrides.conf";

const SYSTEMCTL_BIN: &str = "/bin/systemctl";
const HOST_CTR_BIN: &str = "/bin/host-ctr";

/// Stores user-supplied global arguments
#[derive(Debug)]
struct Args {
Expand All @@ -113,7 +107,7 @@ impl Default for Args {
fn default() -> Self {
Self {
log_level: LevelFilter::Info,
socket_path: DEFAULT_API_SOCKET.to_string(),
socket_path: constants::API_SOCKET.to_string(),
}
}
}
Expand Down Expand Up @@ -150,7 +144,7 @@ fn usage() {
--mode MODE

Socket path defaults to {}",
program_name, DEFAULT_API_SOCKET,
program_name, constants::API_SOCKET,
);
}

Expand Down Expand Up @@ -313,7 +307,7 @@ where
if host_containerd_unit.is_active()? {
debug!("Cleaning up container '{}'", name);
command(
HOST_CTR_BIN,
constants::HOST_CTR_BIN,
&[
"clean-up",
"--container-id",
Expand All @@ -327,7 +321,7 @@ where
// Clean up any left over tasks, before the container is enabled
if host_containerd_unit.is_active()? && !systemd_unit.is_enabled()? {
command(
HOST_CTR_BIN,
constants::HOST_CTR_BIN,
&[
"clean-up",
"--container-id",
Expand Down Expand Up @@ -397,7 +391,7 @@ where
debug!("Querying the API for settings");

let method = "GET";
let uri = API_SETTINGS_URI;
let uri = constants::API_SETTINGS_URI;
let (_code, response_body) = apiclient::raw_request(&socket_path, uri, method, None)
.await
.context(error::APIRequest { method, uri })?;
Expand All @@ -422,7 +416,7 @@ impl<'a> SystemdUnit<'a> {
}

fn is_enabled(&self) -> Result<bool> {
match command(SYSTEMCTL_BIN, &["is-enabled", &self.unit]) {
match command(constants::SYSTEMCTL_BIN, &["is-enabled", &self.unit]) {
Ok(()) => Ok(true),
Err(e) => {
// If the systemd unit is not enabled, then `systemctl is-enabled` will return a
Expand All @@ -439,7 +433,7 @@ impl<'a> SystemdUnit<'a> {
}

fn is_active(&self) -> Result<bool> {
match command(SYSTEMCTL_BIN, &["is-active", &self.unit]) {
match command(constants::SYSTEMCTL_BIN, &["is-active", &self.unit]) {
Ok(()) => Ok(true),
Err(e) => {
// If the systemd unit is not active(running), then `systemctl is-active` will
Expand All @@ -458,13 +452,13 @@ impl<'a> SystemdUnit<'a> {
fn enable(&self) -> Result<()> {
// Only enable the unit, since it will be started once systemd reaches the `preconfigured`
// target
command(SYSTEMCTL_BIN, &["enable", &self.unit])
command(constants::SYSTEMCTL_BIN, &["enable", &self.unit])
}

fn disable(&self) -> Result<()> {
// Bootstrap containers won't be up by the time the user sends configurations through
// `apiclient`, so there is no need to add `--now` to stop them
command(SYSTEMCTL_BIN, &["disable", &self.unit])
command(constants::SYSTEMCTL_BIN, &["disable", &self.unit])
}
}

Expand Down
1 change: 1 addition & 0 deletions sources/api/certdog/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ exclude = ["README.md"]
apiclient = { path = "../apiclient" }
argh = "0.1.3"
base64 = "0.13"
constants = { path = "../../constants" }
# x509-parser depends on der-parser ^5.0. 5.1.1 contains breaking changes.
# The 5.1.1 release isn't in the master branch; those changes are instead in a
# 6.0.0 release, more clearly implying breaking changes. Lock to 5.1.0.
Expand Down
8 changes: 3 additions & 5 deletions sources/api/certdog/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@ use std::io::{BufRead, Seek};
use std::path::Path;
use std::process;
use x509_parser;
use constants;

use model::modeled_types::Identifier;

// FIXME Get from configuration in the future
const DEFAULT_API_SOCKET: &str = "/run/api.sock";
const API_SETTINGS_URI: &str = "/settings";
// Read from the source in `/usr/share/factory` not the copy in `/etc`
const DEFAULT_SOURCE_BUNDLE: &str = "/usr/share/factory/etc/pki/tls/certs/ca-bundle.crt";
// This file is first created with tmpfilesd configurations
Expand All @@ -42,7 +40,7 @@ struct Args {
#[argh(option, default = "LevelFilter::Info", short = 'l')]
/// log-level trace|debug|info|warn|error
log_level: LevelFilter,
#[argh(option, default = "DEFAULT_API_SOCKET.to_string()", short = 's')]
#[argh(option, default = "constants::API_SOCKET.to_string()", short = 's')]
/// socket-path path to apiserver socket
socket_path: String,
#[argh(option, default = "DEFAULT_TRUSTED_STORE.to_string()", short = 't')]
Expand All @@ -67,7 +65,7 @@ where
debug!("Querying the API for settings");

let method = "GET";
let uri = API_SETTINGS_URI;
let uri = constants::API_SETTINGS_URI;
let (_code, response_body) = apiclient::raw_request(&socket_path, uri, method, None)
.await
.context(error::APIRequest { method, uri })?;
Expand Down
1 change: 1 addition & 0 deletions sources/api/corndog/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ exclude = ["README.md"]

[dependencies]
apiclient = { path = "../apiclient" }
constants = { path = "../../constants" }
http = "0.2"
log = "0.4"
models = { path = "../../models" }
Expand Down
6 changes: 3 additions & 3 deletions sources/api/corndog/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::string::String;
use std::{env, process};
use constants;

const DEFAULT_API_SOCKET: &str = "/run/api.sock";
const SYSCTL_PATH_PREFIX: &str = "/proc/sys";
const LOCKDOWN_PATH: &str = "/sys/kernel/security/lockdown";

Expand Down Expand Up @@ -183,7 +183,7 @@ fn usage() -> ! {
--log-level trace|debug|info|warn|error

Socket path defaults to {}",
program_name, DEFAULT_API_SOCKET,
program_name, constants::API_SOCKET,
);
process::exit(2);
}
Expand Down Expand Up @@ -228,7 +228,7 @@ fn parse_args(args: env::Args) -> Args {
Args {
subcommand: subcommand.unwrap_or_else(|| usage_msg("Must specify a subcommand.")),
log_level: log_level.unwrap_or_else(|| LevelFilter::Info),
socket_path: socket_path.unwrap_or_else(|| DEFAULT_API_SOCKET.to_string()),
socket_path: socket_path.unwrap_or_else(|| constants::API_SOCKET.to_string()),
}
}

Expand Down
1 change: 1 addition & 0 deletions sources/api/early-boot-config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ exclude = ["README.md"]
apiclient = { path = "../apiclient" }
async-trait = "0.1.36"
base64 = "0.13"
constants = { path = "../../constants" }
flate2 = { version = "1.0", default-features = false, features = ["rust_backend"] }
http = "0.2"
imdsclient = { path = "../../imdsclient" }
Expand Down
Loading