Merge pull request #22 from trunk-io/eli/verify_token

Verify that if trigger is API that the trunk token is set

Author: EliSchleifer

src/config.rs

@@ -326,7 +326,7 @@ impl Conf {
         Ok(())
     }
 
-    pub fn is_valid(&self) -> Result<(), &'static str> {
+    pub fn is_valid(&self, cli: Option<&crate::cli::Cli>) -> Result<(), &'static str> {
         if self.test.flake_rate <= 0.0 || self.test.flake_rate > 1.0 {
             return Err("flake_rate must be between 0.0 and 1.0");
         }
@@ -362,8 +362,15 @@ impl Conf {
                 }
             }
             EnqueueTrigger::Api => {
-                // For API trigger, we don't validate here as the token comes from CLI/env
-                // The actual validation happens at runtime when the API call is made
+                if let Some(cli) = cli {
+                    if cli.trunk_token.is_empty() {
+                        return Err(
+                            "merge trigger is set to 'api' but TRUNK_TOKEN is not available",
+                        );
+                    }
+                } else {
+                    return Err("merge trigger is set to 'api' but CLI context is required for token validation");
+                }
             }
         }
 

src/main.rs

@@ -497,7 +497,7 @@ fn run() -> anyhow::Result<()> {
             std::process::exit(1);
         });
 
-    config.is_valid().unwrap_or_else(|err| {
+    config.is_valid(Some(&cli)).unwrap_or_else(|err| {
         eprintln!("Invalid config:\n    {}", err);
         std::process::exit(1);
     });

tests/config_tests.rs

@@ -256,7 +256,7 @@ fn test_deps_distribution_validation() {
             ..Default::default()
         });
 
-        let result = config.is_valid();
+        let result = config.is_valid(None);
         if let Err(e) = result {
             panic!(
                 "Valid distribution '{}' should pass validation, but got error: {}",
@@ -285,7 +285,7 @@ fn test_deps_distribution_validation() {
         });
 
         assert!(
-            config.is_valid().is_err(),
+            config.is_valid(None).is_err(),
             "Invalid distribution '{}' ({}) should fail validation",
             distribution,
             description
@@ -305,7 +305,7 @@ fn test_validate_deps_distribution_directly() {
             ..Default::default()
         });
 
-        let result = config.is_valid();
+        let result = config.is_valid(None);
         assert!(
             result.is_ok(),
             "Valid distribution '{}' should pass validation, but got error: {:?}",
@@ -328,3 +328,46 @@ fn test_simple_distribution() {
         assert_eq!(count, 2, "PR {} should have 2 dependencies", pr_num);
     }
 }
+
+#[test]
+fn test_api_trigger_trunk_token_validation_in_config() {
+    use gen::cli::Cli;
+    use gen::config::EnqueueTrigger;
+
+    // Test with API trigger and empty token - should fail validation
+    let mut config = create_test_config(PullRequestConf::default());
+    config.merge.trigger = EnqueueTrigger::Api;
+
+    let cli_empty_token = Cli {
+        subcommand: None,
+        gh_token: vec![],
+        trunk_token: String::new(), // Empty token
+        dry_run: false,
+    };
+
+    let result = config.is_valid(Some(&cli_empty_token));
+    assert!(result.is_err());
+    assert_eq!(
+        result.unwrap_err(),
+        "merge trigger is set to 'api' but TRUNK_TOKEN is not available"
+    );
+
+    // Test with API trigger and valid token - should pass validation
+    let cli_valid_token = Cli {
+        subcommand: None,
+        gh_token: vec![],
+        trunk_token: "valid_token_123".to_string(),
+        dry_run: false,
+    };
+
+    let result = config.is_valid(Some(&cli_valid_token));
+    assert!(result.is_ok());
+
+    // Test with API trigger but no CLI context - should fail validation
+    let result = config.is_valid(None);
+    assert!(result.is_err());
+    assert_eq!(
+        result.unwrap_err(),
+        "merge trigger is set to 'api' but CLI context is required for token validation"
+    );
+}

tests/github_tests.rs

@@ -0,0 +1,139 @@
+use gen::github::GitHubAction;
+use gen::trunk::get_targets;
+
+#[test]
+fn test_parse_deps_from_pr_body() {
+    // Test data that mimics a GitHub PR event JSON
+    let github_json = r#"{
+        "repository": "owner/repo",
+        "event": {
+            "pull_request": {
+                "number": 123,
+                "head": {
+                    "sha": "abc123def456"
+                },
+                "body": "This is a test PR\n\nSome description here\n\ndeps=[a,b]\n\nMore content"
+            }
+        }
+    }"#;
+
+    // Parse the GitHub action data
+    let ga = GitHubAction::from_json(github_json);
+
+    // Verify the PR body was parsed correctly
+    assert!(ga.event.pull_request.body.is_some());
+    let body = ga.event.pull_request.body.as_ref().unwrap();
+    assert!(body.contains("deps=[a,b]"));
+
+    // Test the actual get_targets function
+    let impacted_targets = get_targets(body);
+
+    // Verify the extracted dependencies
+    assert_eq!(impacted_targets.len(), 2);
+    assert_eq!(impacted_targets[0], "a");
+    assert_eq!(impacted_targets[1], "b");
+}
+
+#[test]
+fn test_parse_deps_with_spaces() {
+    // Test with spaces around the dependencies
+    let github_json = r#"{
+        "repository": "owner/repo",
+        "event": {
+            "pull_request": {
+                "number": 123,
+                "head": {
+                    "sha": "abc123def456"
+                },
+                "body": "deps=[ a , b , c ]"
+            }
+        }
+    }"#;
+
+    let ga = GitHubAction::from_json(github_json);
+    let body = ga.event.pull_request.body.as_ref().unwrap();
+
+    let impacted_targets = get_targets(body);
+
+    // Verify spaces are trimmed
+    assert_eq!(impacted_targets.len(), 3);
+    assert_eq!(impacted_targets[0], "a");
+    assert_eq!(impacted_targets[1], "b");
+    assert_eq!(impacted_targets[2], "c");
+}
+
+#[test]
+fn test_parse_deps_single_dependency() {
+    // Test with a single dependency
+    let github_json = r#"{
+        "repository": "owner/repo",
+        "event": {
+            "pull_request": {
+                "number": 123,
+                "head": {
+                    "sha": "abc123def456"
+                },
+                "body": "Some text\ndeps=[single-target]\nMore text"
+            }
+        }
+    }"#;
+
+    let ga = GitHubAction::from_json(github_json);
+    let body = ga.event.pull_request.body.as_ref().unwrap();
+
+    let impacted_targets = get_targets(body);
+
+    assert_eq!(impacted_targets.len(), 1);
+    assert_eq!(impacted_targets[0], "single-target");
+}
+
+#[test]
+fn test_parse_deps_no_match() {
+    // Test when no deps pattern is found
+    let github_json = r#"{
+        "repository": "owner/repo",
+        "event": {
+            "pull_request": {
+                "number": 123,
+                "head": {
+                    "sha": "abc123def456"
+                },
+                "body": "This PR has no deps information"
+            }
+        }
+    }"#;
+
+    let ga = GitHubAction::from_json(github_json);
+    let body = ga.event.pull_request.body.as_ref().unwrap();
+
+    let impacted_targets = get_targets(body);
+
+    // Should be empty when no match is found
+    assert_eq!(impacted_targets.len(), 0);
+}
+
+#[test]
+fn test_parse_deps_empty_brackets() {
+    // Test with empty deps brackets
+    let github_json = r#"{
+        "repository": "owner/repo",
+        "event": {
+            "pull_request": {
+                "number": 123,
+                "head": {
+                    "sha": "abc123def456"
+                },
+                "body": "deps=[]"
+            }
+        }
+    }"#;
+
+    let ga = GitHubAction::from_json(github_json);
+    let body = ga.event.pull_request.body.as_ref().unwrap();
+
+    let impacted_targets = get_targets(body);
+
+    // Should have one empty string when brackets are empty
+    assert_eq!(impacted_targets.len(), 1);
+    assert_eq!(impacted_targets[0], "");
+}

tests/trunk_tests.rs

@@ -137,3 +137,36 @@ fn test_parse_deps_empty_brackets() {
     assert_eq!(impacted_targets.len(), 1);
     assert_eq!(impacted_targets[0], "");
 }
+
+#[test]
+fn test_api_trigger_trunk_token_validation() {
+    use gen::cli::Cli;
+    use gen::config::{Conf, EnqueueTrigger};
+
+    // Create a config with API trigger
+    let mut config = Conf::default();
+    config.merge.trigger = EnqueueTrigger::Api;
+    config.trunk.api = "api.trunk.io".to_string();
+
+    // Test with empty trunk token
+    let cli_empty_token = Cli {
+        subcommand: None,
+        gh_token: vec![],
+        trunk_token: String::new(), // Empty token
+        dry_run: false,
+    };
+
+    // Test with valid trunk token
+    let cli_valid_token = Cli {
+        subcommand: None,
+        gh_token: vec![],
+        trunk_token: "valid_token_123".to_string(),
+        dry_run: false,
+    };
+
+    // Verify empty token is detected
+    assert!(cli_empty_token.trunk_token.is_empty());
+
+    // Verify valid token is not empty
+    assert!(!cli_valid_token.trunk_token.is_empty());
+}