From eae82ee0ac3437492db4bf9ff9201c7af490cb39 Mon Sep 17 00:00:00 2001 From: Nan Jiang Date: Wed, 10 May 2023 10:09:23 -0400 Subject: [PATCH] [DISCO-2383] feat: Use SHA commit hash as version (#551) --- .circleci/config.yml | 5 +++- Dockerfile | 6 ++++- src/lib.rs | 23 +++++++++++++++++++ src/logging.rs | 7 ++---- src/main.rs | 8 +++++-- src/server/mod.rs | 6 ++--- src/web/dockerflow.rs | 7 ++---- .../partner/app/record_keeper.py | 7 +++++- .../volumes/client/scenarios.yml | 2 +- .../volumes/client/scenarios_tiles_cache.yml | 6 ++--- 10 files changed, 54 insertions(+), 23 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 97eb7d96..6844a41e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -148,7 +148,10 @@ jobs: - write-version - run: name: Build Docker image - command: docker build -t app:build . + command: | + docker build \ + -t app:build \ + --build-arg VERSION="$(echo ${CIRCLE_SHA1} | cut -c -7)" . # save the built docker container into CircleCI's cache. This is # required since Workflows do not have the same remote docker instance. - run: diff --git a/Dockerfile b/Dockerfile index e4422983..245983a8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,10 +2,14 @@ # Change this to be your application's name ARG APPNAME=contile +# This build arg is used to pass the version (e.g. the commit SHA1 hash) from CI +# when building the application. +ARG VERSION=unset # NOTE: Ensure builder's Rust version matches CI's in .circleci/config.yml FROM rust:1.68-slim-bullseye as builder ARG APPNAME +ARG VERSION ADD . /app WORKDIR /app @@ -19,7 +23,7 @@ RUN \ cargo --version && \ rustc --version && \ mkdir -m 755 bin && \ - cargo build --release && \ + CONTILE_VERSION=${VERSION} cargo build --release && \ cp /app/target/release/${APPNAME} /app/bin diff --git a/src/lib.rs b/src/lib.rs index b9ab20a8..debdda21 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,3 +23,26 @@ pub mod server; pub mod settings; pub mod tags; pub mod web; + +/// Create the version string (e.g. "contile/1.0.0") with the given separator. +/// It expects an environment variable `CONTILE_VERSION` as the version and +/// falls back to `CARGO_PKG_VERSION` if it's not present. +pub fn create_app_version(separator: &str) -> String { + let app = env!("CARGO_PKG_NAME"); + let version = option_env!("CONTILE_VERSION").unwrap_or(env!("CARGO_PKG_VERSION")); + + format!("{app}{separator}{version}") +} + +#[cfg(test)] +mod tests { + use crate::create_app_version; + + #[test] + fn test_create_app_version_fallback() { + assert_eq!( + create_app_version("/"), + concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION")) + ); + } +} diff --git a/src/logging.rs b/src/logging.rs index e7f3b4e1..ca7fbbef 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -2,6 +2,7 @@ use std::io; +use crate::create_app_version; use crate::error::HandlerResult; use slog::{self, slog_o, Drain}; @@ -21,11 +22,7 @@ pub fn init_logging(json: bool) -> HandlerResult<()> { .expect("Couldn't get hostname"); let drain = MozLogJson::new(io::stdout()) - .logger_name(format!( - "{}-{}", - env!("CARGO_PKG_NAME"), - env!("CARGO_PKG_VERSION") - )) + .logger_name(create_app_version("-")) .msg_type(format!("{}:log", env!("CARGO_PKG_NAME"))) .hostname(hostname) .build() diff --git a/src/main.rs b/src/main.rs index 05f48391..62fd46db 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,6 @@ //! Main application entry point #![forbid(unsafe_code)] +use std::borrow::Cow; use std::error::Error; #[macro_use] @@ -24,7 +25,7 @@ struct Args { flag_debug_settings: Option, } -use contile::{logging, server, settings}; +use contile::{create_app_version, logging, server, settings}; #[actix_web::main] async fn main() -> Result<(), Box> { @@ -53,7 +54,10 @@ async fn main() -> Result<(), Box> { let mut opts = sentry::apply_defaults(sentry::ClientOptions { // Note: set "debug: true," to diagnose sentry issues // transport: Some(Arc::new(curl_transport_factory)), - release: sentry::release_name!(), + + // Use "@" as the separator to be consistent with the default `release` + // string generated by Sentry. + release: Some(Cow::Owned(create_app_version("@"))), ..sentry::ClientOptions::default() }); opts.integrations.retain(|i| i.name() != "debug-images"); diff --git a/src/server/mod.rs b/src/server/mod.rs index d0cd214e..f5a96330 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -15,6 +15,7 @@ use cadence::StatsdClient; use crate::{ adm::{spawn_updater, AdmFilter}, + create_app_version, error::{HandlerError, HandlerResult}, metrics::metrics_from_opts, server::{img_storage::ImageStore, location::location_config_from_settings}, @@ -30,9 +31,6 @@ pub mod location; /// adjust const TILES_CACHE_INITIAL_CAPACITY: usize = 768; -/// User-Agent sent to adM -const REQWEST_USER_AGENT: &str = concat!(env!("CARGO_PKG_NAME"), "/", env!("CARGO_PKG_VERSION"),); - /// This is the global HTTP state object that will be made available to all /// HTTP API calls. pub struct ServerState { @@ -111,7 +109,7 @@ impl Server { let req = reqwest::Client::builder() .connect_timeout(Duration::from_secs(settings.connect_timeout)) .timeout(Duration::from_secs(settings.request_timeout)) - .user_agent(REQWEST_USER_AGENT) + .user_agent(create_app_version("/")) .build()?; let storage_client = cloud_storage::Client::builder() .client(req.clone()) diff --git a/src/web/dockerflow.rs b/src/web/dockerflow.rs index 38491e73..5861d628 100644 --- a/src/web/dockerflow.rs +++ b/src/web/dockerflow.rs @@ -12,7 +12,7 @@ use actix_web_location::Location; use serde::Deserialize; use serde_json::{json, Value}; -use crate::{error::HandlerError, server::ServerState}; +use crate::{create_app_version, error::HandlerError, server::ServerState}; /// Well Known DockerFlow commands for Ops callbacks pub const DOCKER_FLOW_ENDPOINTS: [&str; 4] = [ @@ -52,10 +52,7 @@ async fn version() -> HttpResponse { /// Returns a status message indicating the current state of the server async fn heartbeat(state: web::Data) -> HttpResponse { let mut checklist = HashMap::new(); - checklist.insert( - "version".to_owned(), - Value::String(env!("CARGO_PKG_VERSION").to_owned()), - ); + checklist.insert("version".to_owned(), Value::String(create_app_version("/"))); if state.settings.test_mode != crate::settings::TestModes::NoTest { checklist.insert( "test_mode".to_owned(), diff --git a/test-engineering/contract-tests/partner/app/record_keeper.py b/test-engineering/contract-tests/partner/app/record_keeper.py index 4f1f7337..67ddfc98 100644 --- a/test-engineering/contract-tests/partner/app/record_keeper.py +++ b/test-engineering/contract-tests/partner/app/record_keeper.py @@ -17,7 +17,12 @@ def add(self, request: Request) -> None: """Create record from Fast API Request and add record to the record keeper.""" headers: tuple[Header, ...] = tuple( - Header(name=name, value=value) for name, value in request.headers.items() + Header( + name=name, + # Strip the version from "user-agent" as it's volatile in CI. + value=value if name != "user-agent" else value.split("/")[0], + ) + for name, value in request.headers.items() ) query_parameters: tuple[QueryParameter, ...] = tuple( diff --git a/test-engineering/contract-tests/volumes/client/scenarios.yml b/test-engineering/contract-tests/volumes/client/scenarios.yml index 8af9d177..9bd7fa52 100644 --- a/test-engineering/contract-tests/volumes/client/scenarios.yml +++ b/test-engineering/contract-tests/volumes/client/scenarios.yml @@ -535,7 +535,7 @@ scenarios: - name: accept value: '*/*' - name: user-agent - value: 'contile/1.11.3' + value: 'contile' - name: host value: 'partner:5000' path: '/tilesp/desktop' diff --git a/test-engineering/contract-tests/volumes/client/scenarios_tiles_cache.yml b/test-engineering/contract-tests/volumes/client/scenarios_tiles_cache.yml index 3c3aa35e..cd94778b 100644 --- a/test-engineering/contract-tests/volumes/client/scenarios_tiles_cache.yml +++ b/test-engineering/contract-tests/volumes/client/scenarios_tiles_cache.yml @@ -88,7 +88,7 @@ scenarios: - name: accept value: '*/*' - name: user-agent - value: 'contile/1.11.3' + value: 'contile' - name: host value: 'partner:5000' path: '/tilesp/desktop' @@ -201,7 +201,7 @@ scenarios: - name: accept value: '*/*' - name: user-agent - value: 'contile/1.11.3' + value: 'contile' - name: host value: 'partner:5000' path: '/tilesp/desktop' @@ -235,7 +235,7 @@ scenarios: - name: accept value: '*/*' - name: user-agent - value: 'contile/1.11.3' + value: 'contile' - name: host value: 'partner:5000' path: '/tilesp/desktop'