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

@@ -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

Cargo.toml

@@ -2,7 +2,7 @@
 name = "ksnotify"
 authors = ["hirosassa <[email protected]>"]
 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

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

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::<bool>()?;
+ let patch = doc
+ .get("patch")
+ .expect("failed to load the patch flag")
+ .parse::<bool>()?;
 
  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,
  })
  }
 }

src/main.rs

@@ -27,6 +27,10 @@ pub struct Cli {
  #[arg(long)]
  pub notifier: Option<String>,
 
+ /// 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<String>,
@@ -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<String>, target: Option<String>) -> Result<String> {
+fn process(
+ config: &config::Config,
+ url: Option<String>,
+ target: Option<String>,
+) -> Result<template::Template> {
  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)
 }

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<()>;
 }

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<u64> {
  Ok(env::var("CI_PROJECT_ID")?.parse::<u64>()?)
  }
+
+ fn retrive_same_build_comment(&self, template: &Template) -> Result<Option<Note>> {
+ 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<Note> = 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)?;

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<bool> {
+ 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, String>) -> String {
  let kinds: Vec<String> = 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())
+ }
 }