Skip to content

Commit

Permalink
Fix new rust fmt and cargo clippy failures on main with the latest ru…
Browse files Browse the repository at this point in the history
…st (#365)

* Clippy auto format

* Cargo fmt error fix module docs

* Refactor due to clippy

This suggestion was hard to apply so I moved the code:

```
57 |   fn fetch_secret_key_base_from_store(store: &Option<Store>) -> (String, Store) {
   |   ^                                          -------------- help: change this to: `Option<&Store>`
   |  _|
   | |
58 | |     let mut store = store.clone().unwrap_or_default();
59 | |     let default_secret_key_base = store
60 | |         .metadata
...  |
72 | |     (default_secret_key_base, store)
73 | | }
   | |_^
```

I also added an integration test to assert that the SECRET_KEY_BASE is preserved
  • Loading branch information
schneems authored Dec 11, 2024
1 parent 187770e commit a3f5834
Show file tree
Hide file tree
Showing 10 changed files with 148 additions and 146 deletions.
20 changes: 10 additions & 10 deletions buildpacks/ruby/src/bin/agentmon_loop.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
//! Agentmon Loop
//!
//! Boots agentmon (a statsd server) in a loop
//!
//! Example:
//!
//! ```shell
//! $ cargo run --bin agentmon_loop -- --path <path/to/agentmon/binary>
//! ```
// Required due to: https://github.com/rust-lang/rust/issues/95513
#![allow(unused_crate_dependencies)]

Expand All @@ -19,16 +29,6 @@ static HEROKU_METRICS_URL: &str = "HEROKU_METRICS_URL";

const SLEEP_FOR: Duration = Duration::from_secs(1);

/// Agentmon Loop
///
/// Boots agentmon (a statsd server) in a loop
///
/// Example:
///
/// ```shell
/// $ cargo run --bin agentmon_loop -- --path <path/to/agentmon/binary>
/// ```
/// Turn CLI arguments into a Rust struct
#[derive(Parser, Debug)]
struct Args {
Expand Down
4 changes: 2 additions & 2 deletions buildpacks/ruby/src/bin/launch_daemon.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! Schedules agentmon to run as a background daemon
// Required due to: https://github.com/rust-lang/rust/issues/95513
#![allow(unused_crate_dependencies)]

Expand All @@ -8,8 +10,6 @@ use std::process::Command;

static AGENTMON_DEBUG: &str = "AGENTMON_DEBUG";

/// Schedules agentmon to run as a background daemon
/// CLI argument parser
///
/// ```shell
Expand Down
33 changes: 14 additions & 19 deletions buildpacks/ruby/src/steps/default_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,20 @@ pub(crate) fn default_env(
env.insert(k, v);
}

let (default_secret_key_base, store) = fetch_secret_key_base_from_store(&context.store);
let mut store = context.store.clone().unwrap_or_default();
let default_secret_key_base = store
.metadata
.entry("SECRET_KEY_BASE")
.or_insert_with(|| {
let mut rng = rand::thread_rng();

(0..64)
.map(|_| rng.sample(rand::distributions::Alphanumeric) as char)
.collect::<String>()
.into()
})
.to_string();

let layer_ref = context.uncached_layer(
layer_name!("env_defaults"),
UncachedLayerDefinition {
Expand Down Expand Up @@ -53,21 +66,3 @@ pub(crate) fn default_env(

Ok((env, store))
}

fn fetch_secret_key_base_from_store(store: &Option<Store>) -> (String, Store) {
let mut store = store.clone().unwrap_or_default();
let default_secret_key_base = store
.metadata
.entry("SECRET_KEY_BASE")
.or_insert_with(|| {
let mut rng = rand::thread_rng();

(0..64)
.map(|_| rng.sample(rand::distributions::Alphanumeric) as char)
.collect::<String>()
.into()
})
.to_string();

(default_secret_key_base, store)
}
9 changes: 9 additions & 0 deletions buildpacks/ruby/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ fn test_default_app_latest_distro() {

assert_contains!(context.pack_stdout, "Installing puma");

let secret_key_base = context.run_shell_command("echo \"${SECRET_KEY_BASE:?No SECRET_KEY_BASE set}\"").stdout;
assert!(!secret_key_base.trim().is_empty(), "Expected {secret_key_base:?} to not be empty but it is");

let config = context.config.clone();
context.rebuild(config, |rebuild_context| {
println!("{}", rebuild_context.pack_stdout);
Expand All @@ -114,6 +117,12 @@ fn test_default_app_latest_distro() {
assert_contains!(body, "ruby_version");
},
);

// Assert SECRET_KEY_BASE is preserved between invocations
assert_eq!(
secret_key_base,
rebuild_context.run_shell_command("echo \"${SECRET_KEY_BASE:?No SECRET_KEY_BASE set}\"").stdout
);
});
},
);
Expand Down
4 changes: 2 additions & 2 deletions commons/src/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl<'a, L: AsRef<str>> SentenceList<'a, L> {
}
}

impl<'a, L: AsRef<str>> Default for SentenceList<'a, L> {
impl<L: AsRef<str>> Default for SentenceList<'_, L> {
fn default() -> Self {
Self {
list: Default::default(),
Expand All @@ -63,7 +63,7 @@ impl<'a, L: AsRef<str>> Default for SentenceList<'a, L> {
}
}

impl<'a, L: AsRef<str>> Display for SentenceList<'a, L> {
impl<L: AsRef<str>> Display for SentenceList<'_, L> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let SentenceList {
list,
Expand Down
6 changes: 3 additions & 3 deletions commons/src/output/background_timer.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//! This module is responsible for the logic involved in the printing to output while
//! other work is being performed.
//!
use std::io::Write;
use std::sync::mpsc::Sender;
use std::sync::{mpsc, Arc, Mutex};
use std::thread::JoinHandle;
use std::time::{Duration, Instant};

/// This module is responsible for the logic involved in the printing to output while
/// other work is being performed.
/// Prints a start, then a tick every second, and an end to the given `Write` value.
///
/// Returns a struct that allows for manually stopping the timer or will automatically stop
Expand Down
3 changes: 1 addition & 2 deletions commons/src/output/fmt.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
//! Helpers for formatting and colorizing your output
use crate::output::util::LinesWithEndings;
use const_format::formatcp;
use std::fmt::Write;

/// Helpers for formatting and colorizing your output
/// Decorated str for prefixing "Help:"
#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")]
pub const HELP: &str = formatcp!("{IMPORTANT_COLOR}! HELP{RESET}");
Expand Down
14 changes: 7 additions & 7 deletions commons/src/output/interface.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
//! Consuming stateful logger interface
//!
//! The log pattern used by `BuildLog` is a consuming state machine that is designed to minimize
//! the amount of mistakes that can result in malformed build output.
//!
//! The interface isn't stable and may need to change.
//!
use std::fmt::Debug;
use std::io::Write;

/// Consuming stateful logger interface
///
/// The log pattern used by `BuildLog` is a consuming state machine that is designed to minimize
/// the amount of mistakes that can result in malformed build output.
///
/// The interface isn't stable and may need to change.
pub trait Logger: Debug {
fn buildpack_name(self, s: &str) -> Box<dyn StartedLogger>;
fn without_buildpack_name(self) -> Box<dyn StartedLogger>;
Expand Down
69 changes: 34 additions & 35 deletions commons/src/output/section_log.rs
Original file line number Diff line number Diff line change
@@ -1,44 +1,43 @@
//! Write to the build output in a `Box<dyn SectionLogger>` format with functions
//!
//! ## What
//!
//! Logging from within a layer can be difficult because calls to the layer interface are not
//! mutable nor consumable. Functions can be used at any time with no restrictions. The
//! only downside is that the buildpack author (you) is now responsible for:
//!
//! - Ensuring that `Box<dyn StartedLogger>::section()` was called right before any of these
//! functions are called.
//! - Ensuring that you are not attempting to log while already logging i.e. calling `step()` within a
//! `step_timed()` call.
//!
//! ## Use
//!
//! The main use case is logging inside of a layer:
//!
//! ```no_run
//! use commons::output::section_log::log_step_timed;
//!
//! // fn create(
//! // &self,
//! // context: &libcnb::build::BuildContext<Self::Buildpack>,
//! // layer_path: &std::path::Path,
//! // ) -> Result<
//! // libcnb::layer::LayerResult<Self::Metadata>,
//! // <Self::Buildpack as libcnb::Buildpack>::Error,
//! // > {
//! log_step_timed("Installing", || {
//! // Install logic here
//! todo!()
//! })
//! // }
//! ```
use crate::output::build_log::{state, BuildData, BuildLog};
#[allow(clippy::wildcard_imports)]
pub use crate::output::interface::*;
use std::io::Stdout;
use std::marker::PhantomData;

/// Write to the build output in a `Box<dyn SectionLogger>` format with functions
///
/// ## What
///
/// Logging from within a layer can be difficult because calls to the layer interface are not
/// mutable nor consumable. Functions can be used at any time with no restrictions. The
/// only downside is that the buildpack author (you) is now responsible for:
///
/// - Ensuring that `Box<dyn StartedLogger>::section()` was called right before any of these
/// functions are called.
/// - Ensuring that you are not attempting to log while already logging i.e. calling `step()` within a
/// `step_timed()` call.
///
/// ## Use
///
/// The main use case is logging inside of a layer:
///
/// ```no_run
/// use commons::output::section_log::log_step_timed;
///
/// // fn create(
/// // &self,
/// // context: &libcnb::build::BuildContext<Self::Buildpack>,
/// // layer_path: &std::path::Path,
/// // ) -> Result<
/// // libcnb::layer::LayerResult<Self::Metadata>,
/// // <Self::Buildpack as libcnb::Buildpack>::Error,
/// // > {
/// log_step_timed("Installing", || {
/// // Install logic here
/// todo!()
/// })
/// // }
/// ```
/// Output a message as a single step, ideally a short message
///
/// ```
Expand Down
Loading

0 comments on commit a3f5834

Please sign in to comment.