From 73b3f6aa6e34993fe7d9d099b143f2e98304b1e7 Mon Sep 17 00:00:00 2001 From: hirosassa Date: Mon, 2 Jan 2023 15:33:47 +0900 Subject: [PATCH] add patch option to update comment instead of create a new comment (#31) * add patch option * bump version and skip ci on beta --- .github/workflows/test.yaml | 8 ++--- Cargo.toml | 2 +- README.md | 6 ++++ src/config.rs | 10 +++++++ src/main.rs | 20 +++++++++---- src/notifier.rs | 4 ++- src/notifier/gitlab.rs | 42 +++++++++++++++++++++++++-- src/template.rs | 58 +++++++++++++++++++++++++++++++++++++ 8 files changed, 136 insertions(+), 14 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 57a4e9d..9212898 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -29,19 +29,19 @@ jobs: - name: Run cargo check run: cargo check - continue-on-error: ${{ matrix.rust == 'nightly' }} + continue-on-error: ${{ matrix.rust == 'nightly' || matrix.rust == 'beta' }} - name: Run cargo fmt run: cargo fmt --all -- --check - continue-on-error: ${{ matrix.rust == 'nightly' }} + continue-on-error: ${{ matrix.rust == 'nightly' || matrix.rust == 'beta' }} - name: Run cargo clippy run: cargo clippy -- -D warnings -W clippy::nursery - continue-on-error: ${{ matrix.rust == 'nightly' }} + continue-on-error: ${{ matrix.rust == 'nightly' || matrix.rust == 'beta' }} - name: Run cargo test run: cargo test --release --all-features - continue-on-error: ${{ matrix.rust == 'nightly' }} + continue-on-error: ${{ matrix.rust == 'nightly' || matrix.rust == 'beta' }} coverage: runs-on: ubuntu-latest diff --git a/Cargo.toml b/Cargo.toml index 977a241..a614d6e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "ksnotify" authors = ["hirosassa "] description = "A CLI command to parse kubectl diff result and notify it to GitLab" -version = "0.1.7" +version = "0.1.8" edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/README.md b/README.md index 0e09f18..2efd632 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,12 @@ Basic usage is as follows: skaffold render -p dev | kubectl diff -f - 2> /dev/null | | ksnotify --notifier gitlab --ci gitlab ``` +If you want to update existing comment instead of create a new comment, you should add `--patch` flag like + +```console +skaffold render -p dev | kubectl diff -f - 2> /dev/null | | ksnotify --notifier gitlab --ci gitlab --patch +``` + To suppress `skaffold` labels like `skaffold.dev/run-id: 1234` automatically added by `skaffold`, you should add `--suppress-skaffold` flag like ```console diff --git a/src/config.rs b/src/config.rs index 83f5ba4..705444d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -23,6 +23,7 @@ pub struct Config { pub ci: ci::CIKind, pub notifier: notifier::NotifierKind, pub suppress_skaffold: bool, + pub patch: bool, } impl Config { @@ -34,10 +35,12 @@ impl Config { let ci = ci::CIKind::from_str(ci_kind)?; let notifier = notifier::NotifierKind::from_str(notifier_kind)?; let suppress_skaffold = cli.suppress_skaffold; + let patch = cli.patch; return Ok(Self { ci, notifier, suppress_skaffold, + patch, }); } @@ -59,11 +62,16 @@ impl Config { .get("suppress_skaffold") .expect("failed to load the suppress_skaffold flag") .parse::()?; + let patch = doc + .get("patch") + .expect("failed to load the patch flag") + .parse::()?; Ok(Self { ci, notifier, suppress_skaffold, + patch, }) } @@ -72,10 +80,12 @@ impl Config { let ci = ci::CIKind::from_str(&env::var("KSNOTIFY_CI")?)?; let notifier = notifier::NotifierKind::from_str(&env::var("KSNOTIFY_NOTIFIER")?)?; let suppress_skaffold = matches!(env::var("KSNOTIFY_SUPPRESS_SKAFFOLD"), Ok(_)); + let patch = matches!(env::var("KSNOTIFY_PATCH"), Ok(_)); Ok(Self { ci, notifier, suppress_skaffold, + patch, }) } } diff --git a/src/main.rs b/src/main.rs index 66cc773..2e5e251 100755 --- a/src/main.rs +++ b/src/main.rs @@ -27,6 +27,10 @@ pub struct Cli { #[arg(long)] pub notifier: Option, + /// Update an existing comment instead of creating a new comment. If there is no existing comment, a new comment is created. + #[arg(long)] + pub patch: bool, + /// Target component name to distinguish for each environments or product. #[arg(long)] pub target: Option, @@ -63,8 +67,8 @@ fn run() -> Result<()> { // Local PC (for debug) if config.ci == ci::CIKind::Local { - let content = process(config, None, cli.target)?; - println!("{}", content); + let content = process(&config, None, cli.target)?; + println!("{}", content.render()?); return Ok(()); } @@ -77,19 +81,23 @@ fn run() -> Result<()> { } .with_context(|| format!("failed to create notifier: {:?}", ci))?; - let content = process(config, Some(ci.job_url().to_string()), cli.target)?; + let template = process(&config, Some(ci.job_url().to_string()), cli.target)?; notifier - .notify(content) + .notify(template, config.patch) .with_context(|| "failed to notify".to_string())?; Ok(()) } -fn process(config: config::Config, url: Option, target: Option) -> Result { +fn process( + config: &config::Config, + url: Option, + target: Option, +) -> Result { let mut body = String::new(); io::stdin().read_to_string(&mut body)?; let parser = parser::DiffParser::new(config.suppress_skaffold)?; let result = parser.parse(&body)?; let link = url.unwrap_or_default(); let template = template::Template::new(result.kind_result, link, target); - template.render() + Ok(template) } diff --git a/src/notifier.rs b/src/notifier.rs index ff2e174..758e3bb 100644 --- a/src/notifier.rs +++ b/src/notifier.rs @@ -1,4 +1,6 @@ pub mod gitlab; +use crate::template; + use anyhow::Result; use strum_macros::EnumString; @@ -11,5 +13,5 @@ pub enum NotifierKind { } pub trait Notifiable { - fn notify(&self, body: String) -> Result<()>; + fn notify(&self, body: template::Template, patch: bool) -> Result<()>; } diff --git a/src/notifier/gitlab.rs b/src/notifier/gitlab.rs index 621eefe..49a3971 100644 --- a/src/notifier/gitlab.rs +++ b/src/notifier/gitlab.rs @@ -1,7 +1,10 @@ use crate::ci::CI; +use crate::template::Template; use anyhow::{Context, Result}; +use gitlab::api::projects::merge_requests::notes::{EditMergeRequestNote, MergeRequestNotes}; use gitlab::api::{self, projects::merge_requests::notes::CreateMergeRequestNote, Query}; +use gitlab::types::Note; use gitlab::Gitlab; use log::info; use std::env; @@ -44,16 +47,51 @@ impl GitlabNotifier { fn get_project() -> Result { Ok(env::var("CI_PROJECT_ID")?.parse::()?) } + + fn retrive_same_build_comment(&self, template: &Template) -> Result> { + info!("retrieve same build comment"); + let endpoint = MergeRequestNotes::builder() + .project(self.project) + .merge_request(*self.ci.merge_request().number()) + .build() + .map_err(anyhow::Error::msg)?; + let comments: Vec = endpoint.query(&self.client)?; + + for comment in comments { + if template.is_same_build(&comment.body)? { + return Ok(Some(comment)); + } + } + Ok(None) + } } impl Notifiable for GitlabNotifier { - fn notify(&self, body: String) -> Result<()> { + fn notify(&self, template: Template, patch: bool) -> Result<()> { info!("notify to GitLab"); + let same_build_comment = self.retrive_same_build_comment(&template)?; + + // update comment if existed + if patch { + if let Some(same_build_comment) = same_build_comment { + let note = EditMergeRequestNote::builder() + .project(self.project) + .merge_request(*self.ci.merge_request().number()) + .note(same_build_comment.id.value()) + .body(template.render()?) + .build() + .map_err(anyhow::Error::msg)?; + api::ignore(note).query(&self.client)?; + return Ok(()); + } + } + + // create new comment let note = CreateMergeRequestNote::builder() .project(self.project) .merge_request(*self.ci.merge_request().number()) - .body(body) + .body(template.render()?) .build() .map_err(anyhow::Error::msg)?; api::ignore(note).query(&self.client)?; diff --git a/src/template.rs b/src/template.rs index a40b386..d819a90 100644 --- a/src/template.rs +++ b/src/template.rs @@ -57,6 +57,28 @@ No changes. Kubernetes configurations are up-to-date. Ok(format!("{}{}", title, body)) } + pub fn is_same_build(&self, rendered_string: &str) -> Result { + if self.target.is_none() { + return Ok(false); + } + + let old_title = match rendered_string.lines().next() { + // take first line (it should be title) + Some(title) => title, + None => return Ok(false), + }; + + let reg = Handlebars::new(); + let j = serde_json::to_value(self).unwrap(); + let current_title = reg.render_template(Self::DEFAULT_BUILD_TITLE_TEMPLATE, &j)?; + + if current_title == old_title { + return Ok(true); + } + + Ok(false) + } + fn generate_changed_kinds_markdown(results: &HashMap) -> String { let kinds: Vec = results.keys().map(|e| e.to_string()).sorted().collect(); format!(" * {}", kinds.join("\n * ")) @@ -194,4 +216,40 @@ No changes. Kubernetes configurations are up-to-date. .to_string(); assert_eq!(actual, expected); } + + #[test] + fn test_is_same_build_with_target_none() { + let template = Template { + target: None, + changed_kinds: "test".to_string(), + details: "test".to_string(), + link: "http://example.com".to_string(), + is_no_changes: false, + }; + assert!(!template.is_same_build("test").unwrap()); + } + + #[test] + fn test_is_same_build_with_same_build() { + let template = Template { + target: Some("test".to_string()), + changed_kinds: "test".to_string(), + details: "test".to_string(), + link: "http://example.com".to_string(), + is_no_changes: false, + }; + assert!(template.is_same_build("## Plan result (test)").unwrap()) + } + + #[test] + fn test_is_same_build_with_different_build() { + let template = Template { + target: Some("test1".to_string()), + changed_kinds: "test".to_string(), + details: "test".to_string(), + link: "http://example.com".to_string(), + is_no_changes: false, + }; + assert!(!template.is_same_build("## Plan result (test2)").unwrap()) + } }