From 62fe392617734a4a8985bb9471fbf9dafcac89d3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 25 Jan 2026 12:36:43 +0000 Subject: [PATCH 1/2] feat: Add YAML configuration file support (diffdash.yml) Adds support for a diffdash.yml configuration file that can be committed to the repository, allowing teams to share configuration consistently. Changes: - Add ConfigLoader class for YAML file loading with env var overrides - Update Config class to use ConfigLoader as its backend - Add --config flag to CLI for specifying custom config file paths - Update FileFilter to support configurable ignore_paths, include_paths, excluded_suffixes, and excluded_directories - Pass config through ChangeSet.from_git to FileFilter - Add app_name and pr_deploy_annotation_expr to Config for Grafana output - Add comprehensive tests for ConfigLoader and updated Config/FileFilter Configuration precedence (highest to lowest): 1. Environment variables 2. --config flag specified file 3. diffdash.yml in current directory 4. diffdash.yml in git repository root 5. Default values Security: API tokens are only loaded from environment variables. Example configuration file provided as diffdash.example.yml. Co-authored-by: r.buddie --- diffdash.example.yml | 65 ++++++ lib/diffdash.rb | 1 + lib/diffdash/cli/runner.rb | 194 +++++++++++------- lib/diffdash/config.rb | 72 +++++-- lib/diffdash/config_loader.rb | 231 ++++++++++++++++++++++ lib/diffdash/engine/change_set.rb | 8 +- lib/diffdash/file_filter.rb | 117 +++++++++-- lib/diffdash/outputs/grafana.rb | 11 +- spec/diffdash/config_loader_spec.rb | 293 ++++++++++++++++++++++++++++ spec/diffdash/config_spec.rb | 133 ++++++++++++- spec/diffdash/file_filter_spec.rb | 216 +++++++++++++++----- 11 files changed, 1185 insertions(+), 156 deletions(-) create mode 100644 diffdash.example.yml create mode 100644 lib/diffdash/config_loader.rb create mode 100644 spec/diffdash/config_loader_spec.rb diff --git a/diffdash.example.yml b/diffdash.example.yml new file mode 100644 index 0000000..47f0eec --- /dev/null +++ b/diffdash.example.yml @@ -0,0 +1,65 @@ +# Example diffdash configuration file +# Copy this to diffdash.yml and customize for your project +# +# Configuration is loaded from the following sources in order of precedence: +# 1. Environment variables (highest priority) +# 2. Path specified via --config flag +# 3. diffdash.yml in current directory +# 4. diffdash.yml in git repository root +# 5. Default values (lowest priority) +# +# NOTE: API tokens should ONLY be set via environment variables for security. + +# Grafana configuration +grafana: + # Grafana instance URL (also accepts DIFFDASH_GRAFANA_URL env var) + url: https://grafana.mycompany.com + + # Grafana folder ID to store dashboards (also accepts DIFFDASH_GRAFANA_FOLDER_ID) + # Use `diffdash folders` to list available folders + folder_id: 42 + +# Output adapters to use (also accepts DIFFDASH_OUTPUTS env var as comma-separated list) +# Available: grafana, json +outputs: + - grafana + - json + +# Default environment filter for Loki/Prometheus queries +# (also accepts DIFFDASH_DEFAULT_ENV env var) +default_env: production + +# Whether to post dashboard link as a comment on the PR +# (also accepts DIFFDASH_PR_COMMENT env var) +pr_comment: true + +# Application name for dashboard queries (also accepts DIFFDASH_APP_NAME env var) +# If not set, will be inferred from git remote URL +app_name: my-service + +# Custom PromQL expression for PR deployment annotations +# (also accepts DIFFDASH_PR_DEPLOY_ANNOTATION_EXPR env var) +# pr_deploy_annotation_expr: changes(deploy_timestamp{branch="$branch"}[5m]) > 0 + +# File filtering configuration +# Paths to ignore when scanning for observability signals +ignore_paths: + - vendor/ + - lib/legacy/ + - tmp/ + +# Only scan files within these paths (empty = scan all paths) +# include_paths: +# - app/ +# - lib/ + +# File suffixes to exclude (defaults to test files) +excluded_suffixes: + - _spec.rb + - _test.rb + +# Directories to exclude (defaults to test and config directories) +excluded_directories: + - spec + - test + - config diff --git a/lib/diffdash.rb b/lib/diffdash.rb index d494507..13dd888 100644 --- a/lib/diffdash.rb +++ b/lib/diffdash.rb @@ -15,6 +15,7 @@ end require_relative "diffdash/version" +require_relative "diffdash/config_loader" require_relative "diffdash/config" require_relative "diffdash/git_context" require_relative "diffdash/file_filter" diff --git a/lib/diffdash/cli/runner.rb b/lib/diffdash/cli/runner.rb index 4f8d3c6..6940f64 100644 --- a/lib/diffdash/cli/runner.rb +++ b/lib/diffdash/cli/runner.rb @@ -4,7 +4,7 @@ module Diffdash module CLI # Thin CLI glue. Orchestrates engine + output adapters. class Runner - VALID_OPTIONS = %w[--dry-run --verbose -v --help -h --version].freeze + VALID_OPTIONS = %w[--dry-run --verbose -v --help -h --version --config].freeze VALID_SUBCOMMANDS = %w[folders rspec].freeze def self.run(args) @@ -13,11 +13,12 @@ def self.run(args) def initialize(args) @args = args - @config = Config.new - @dry_run = ENV["DIFFDASH_DRY_RUN"] == "true" || args.include?("--dry-run") - @help = args.include?("--help") || args.include?("-h") - @version = args.include?("--version") - @verbose = args.include?("--verbose") || args.include?("-v") + @config_path = extract_config_path(args) + @config = Config.new(config_path: @config_path) + @dry_run = ENV['DIFFDASH_DRY_RUN'] == 'true' || args.include?('--dry-run') + @help = args.include?('--help') || args.include?('-h') + @version = args.include?('--version') + @verbose = args.include?('--verbose') || args.include?('-v') @subcommand = extract_subcommand(args) @dynamic_metrics = [] end @@ -37,23 +38,24 @@ def execute # Validate arguments (after version/help checks) invalid_args = find_invalid_arguments if invalid_args.any? - warn "ERROR: Unknown argument(s): #{invalid_args.join(", ")}" - warn "" + warn "ERROR: Unknown argument(s): #{invalid_args.join(', ')}" + warn '' warn "Run 'diffdash --help' for usage information." return 1 end # Handle subcommands case @subcommand - when "folders" + when 'folders' return list_grafana_folders - when "rspec" + when 'rspec' return run_rspec end warn "[diffdash] v#{VERSION}" + log_config_info - change_set = Engine::ChangeSet.from_git + change_set = Engine::ChangeSet.from_git(config: @config) log_verbose("Branch: #{change_set.branch_name}") log_verbose("Changed files: #{change_set.changed_files.size}") log_verbose("Filtered Ruby files: #{change_set.filtered_files.size}") @@ -65,9 +67,9 @@ def execute log_verbose("Total signals extracted: #{bundle.logs.size + bundle.metrics.size}") if bundle.empty? - warn "[diffdash] No observability signals found in changed files" + warn '[diffdash] No observability signals found in changed files' warn_dynamic_metrics_summary - warn "[diffdash] Dashboard not created" + warn '[diffdash] Dashboard not created' return 0 end @@ -89,9 +91,7 @@ def execute print_signal_summary(bundle, url: dashboard_url, grafana_failed: grafana_failed) # Post PR comment with dashboard link and signal summary - if dashboard_url && @config.pr_comment? - post_pr_comment(dashboard_url, bundle) - end + post_pr_comment(dashboard_url, bundle) if dashboard_url && @config.pr_comment? warn_limit_warnings if @limit_warnings.any? warn_output_errors(errors) if errors.any? @@ -112,14 +112,42 @@ def extract_subcommand(args) args.find { |arg| VALID_SUBCOMMANDS.include?(arg) } end + def extract_config_path(args) + idx = args.index('--config') + return nil unless idx + + args[idx + 1] + end + def find_invalid_arguments + skip_next = false @args.reject do |arg| + if skip_next + skip_next = false + next true # Skip the value for --config + end + + if arg == '--config' + skip_next = true + next true + end + VALID_OPTIONS.include?(arg) || VALID_SUBCOMMANDS.include?(arg) end end + def log_config_info + return unless @verbose + + if @config.loaded_from + log_verbose("Config loaded from: #{@config.loaded_from}") + else + log_verbose('No config file found, using environment variables and defaults') + end + end + def rspec_args - idx = @args.index("rspec") + idx = @args.index('rspec') return [] unless idx @args[(idx + 1)..] || [] @@ -136,7 +164,9 @@ def build_outputs(change_set) folder_id: @config.grafana_folder_id, dry_run: @dry_run, verbose: @verbose, - default_env: @config.default_env + default_env: @config.default_env, + app_name: @config.app_name, + pr_deploy_annotation_expr: @config.pr_deploy_annotation_expr ) when :json Outputs::Json.new @@ -180,27 +210,28 @@ def adapter_key(adapter) else class_name = adapter.class.name return :adapter if class_name.nil? || class_name.empty? - class_name.split("::").last.downcase.to_sym + + class_name.split('::').last.downcase.to_sym end end def warn_limit_warnings - warn "" - warn "[diffdash] ⚠️ Some signals were excluded:" + warn '' + warn '[diffdash] ⚠️ Some signals were excluded:' @limit_warnings.each do |warning| warn " • #{warning}" end - warn "" + warn '' end def warn_output_errors(errors) - warn "" - warn "[diffdash] Some outputs failed:" + warn '' + warn '[diffdash] Some outputs failed:' errors.each do |entry| warn " • #{entry[:adapter]}: #{entry[:error].message}" warn entry[:error].backtrace.first(3).join("\n") if @verbose end - warn "" + warn '' end def list_grafana_folders @@ -208,15 +239,15 @@ def list_grafana_folders folders = client.list_folders if folders.empty? - puts "No folders found (dashboards will be created in General folder)" + puts 'No folders found (dashboards will be created in General folder)' else - puts "Available Grafana folders:" - puts "" + puts 'Available Grafana folders:' + puts '' folders.each do |folder| puts " ID: #{folder['id'].to_s.ljust(6)} Title: #{folder['title']}" end - puts "" - puts "Set DIFFDASH_GRAFANA_FOLDER_ID in your .env file to use a specific folder" + puts '' + puts 'Set DIFFDASH_GRAFANA_FOLDER_ID in your .env file to use a specific folder' end 0 rescue Clients::Grafana::ConnectionError => e @@ -228,8 +259,8 @@ def list_grafana_folders end def run_rspec - cmd = ["bundle", "exec", "rspec", *rspec_args] - warn "[diffdash] Running: #{cmd.join(" ")}" + cmd = ['bundle', 'exec', 'rspec', *rspec_args] + warn "[diffdash] Running: #{cmd.join(' ')}" system(*cmd) $?.success? ? 0 : 1 end @@ -244,34 +275,34 @@ def print_signal_summary(bundle, url: nil, grafana_failed: false) # Build signal breakdown signal_parts = [] - signal_parts << pluralize(log_count, "log") if log_count > 0 - signal_parts << pluralize(counter_count, "counter") if counter_count > 0 - signal_parts << pluralize(gauge_count, "gauge") if gauge_count > 0 - signal_parts << pluralize(histogram_count, "histogram") if histogram_count > 0 - signal_parts << pluralize(summary_count, "summary") if summary_count > 0 + signal_parts << pluralize(log_count, 'log') if log_count > 0 + signal_parts << pluralize(counter_count, 'counter') if counter_count > 0 + signal_parts << pluralize(gauge_count, 'gauge') if gauge_count > 0 + signal_parts << pluralize(histogram_count, 'histogram') if histogram_count > 0 + signal_parts << pluralize(summary_count, 'summary') if summary_count > 0 # Build panel count total_panels = [log_count, counter_count, gauge_count, histogram_count, summary_count].count { |c| c > 0 } - warn "" - warn "[diffdash] Dashboard created with #{pluralize(total_panels, "panel")}: #{signal_parts.join(", ")}" + warn '' + warn "[diffdash] Dashboard created with #{pluralize(total_panels, 'panel')}: #{signal_parts.join(', ')}" if url warn "[diffdash] Uploaded to: #{url}" elsif grafana_failed - warn "[diffdash] Upload failed (see errors below)" + warn '[diffdash] Upload failed (see errors below)' elsif @dry_run - warn "[diffdash] Mode: dry-run (not uploaded)" + warn '[diffdash] Mode: dry-run (not uploaded)' else - warn "[diffdash] Dashboard JSON printed to stdout" + warn '[diffdash] Dashboard JSON printed to stdout' end if dynamic_count > 0 - warn "[diffdash] Note: #{pluralize(dynamic_count, "dynamic metric")} could not be added" + warn "[diffdash] Note: #{pluralize(dynamic_count, 'dynamic metric')} could not be added" warn_dynamic_metrics_details if @verbose end - warn "" + warn '' end def pluralize(count, word) @@ -282,23 +313,25 @@ def warn_dynamic_metrics_summary dynamic_count = @dynamic_metrics&.size || 0 return if dynamic_count == 0 - warn "[diffdash] Note: #{dynamic_count} dynamic metric#{"s" unless dynamic_count == 1} detected but cannot be added to dashboard" + warn "[diffdash] Note: #{dynamic_count} dynamic metric#{unless dynamic_count == 1 + 's' + end} detected but cannot be added to dashboard" warn_dynamic_metrics_details if @verbose end def warn_dynamic_metrics_details return if @dynamic_metrics.nil? || @dynamic_metrics.empty? - warn "" - warn "[diffdash] ⚠️ Dynamic metrics use runtime values and cannot be added to the dashboard:" + warn '' + warn '[diffdash] ⚠️ Dynamic metrics use runtime values and cannot be added to the dashboard:' @dynamic_metrics.each do |m| warn " • #{m[:file]}:#{m[:line]} - #{m[:receiver]}.#{m[:type]} in #{m[:class]}" end - warn "" - warn "[diffdash] Tip: Use static metric names with labels instead:" - warn " Prometheus.counter(:my_metric).increment(labels: { entity_id: id })" + warn '' + warn '[diffdash] Tip: Use static metric names with labels instead:' + warn ' Prometheus.counter(:my_metric).increment(labels: { entity_id: id })' end def log_verbose(message) @@ -307,9 +340,9 @@ def log_verbose(message) def post_pr_comment(dashboard_url, signal_bundle) commenter = Services::PrCommenter.new(verbose: @verbose, default_env: @config.default_env) - if commenter.post(dashboard_url: dashboard_url, signal_bundle: signal_bundle) - log_verbose("Posted dashboard link to PR") - end + return unless commenter.post(dashboard_url: dashboard_url, signal_bundle: signal_bundle) + + log_verbose('Posted dashboard link to PR') end def print_help @@ -324,29 +357,54 @@ def print_help (none) Run analysis and generate/upload dashboard Options: - --dry-run Generate JSON only, skip Grafana connection - --verbose Print detailed progress information - --version Show version number - --help Show this help message - - Environment Variables (set in .env file): - DIFFDASH_GRAFANA_URL Grafana instance URL (required) - DIFFDASH_GRAFANA_TOKEN Grafana API token (required) - DIFFDASH_GRAFANA_FOLDER_ID Target folder ID (optional) + --config FILE Path to diffdash.yml configuration file + --dry-run Generate JSON only, skip Grafana connection + --verbose Print detailed progress information + --version Show version number + --help Show this help message + + Configuration File (diffdash.yml): + You can create a diffdash.yml file in your repository root to share + configuration across your team. Environment variables always take + precedence over file values (except for secrets like API tokens, + which should ONLY be set via environment variables). + + Config file is searched in this order: + 1. Path specified via --config flag + 2. diffdash.yml or .diffdash.yml in current directory + 3. diffdash.yml or .diffdash.yml in git repository root + + Example diffdash.yml: + grafana: + url: https://myorg.grafana.net + folder_id: 42 + + ignore_paths: + - vendor/ + - lib/legacy/ + + outputs: + - grafana + - json + + default_env: production + pr_comment: true + app_name: my-service + + Environment Variables: + DIFFDASH_GRAFANA_URL Grafana instance URL + DIFFDASH_GRAFANA_TOKEN Grafana API token (required, env only) + DIFFDASH_GRAFANA_FOLDER_ID Target folder ID DIFFDASH_OUTPUTS Comma-separated outputs (default: grafana) DIFFDASH_DRY_RUN Set to 'true' to force dry-run mode - DIFFDASH_DEFAULT_ENV Default environment filter (default: production) + DIFFDASH_DEFAULT_ENV Default environment filter DIFFDASH_PR_COMMENT Set to 'false' to disable PR comments + DIFFDASH_APP_NAME Application name for dashboard DIFFDASH_PR_DEPLOY_ANNOTATION_EXPR PromQL for PR deployment annotation Output: Prints output JSON to STDOUT (Grafana first if configured). Errors and progress info go to STDERR. - - Example .env file: - DIFFDASH_GRAFANA_URL=https://myorg.grafana.net - DIFFDASH_GRAFANA_TOKEN=glsa_xxxxxxxxxxxx - DIFFDASH_GRAFANA_FOLDER_ID=42 HELP end end diff --git a/lib/diffdash/config.rb b/lib/diffdash/config.rb index 6bcd6a2..a73195f 100644 --- a/lib/diffdash/config.rb +++ b/lib/diffdash/config.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative 'config_loader' + module Diffdash class Config # Hard guard rail limits - not configurable for PoC @@ -8,48 +10,86 @@ class Config MAX_EVENTS = 5 MAX_PANELS = 12 - attr_reader :max_logs, :max_metrics, :max_events, :max_panels + attr_reader :max_logs, :max_metrics, :max_events, :max_panels, :loader - def initialize + # Initialize Config with optional YAML file support. + # + # @param config_path [String, nil] explicit path to diffdash.yml + # @param working_dir [String, nil] working directory to search for config files + def initialize(config_path: nil, working_dir: nil) @max_logs = MAX_LOGS @max_metrics = MAX_METRICS @max_events = MAX_EVENTS @max_panels = MAX_PANELS + @loader = ConfigLoader.new(config_path: config_path, working_dir: working_dir) + end + + # Returns the path to the loaded config file, or nil if none was loaded. + def loaded_from + @loader.loaded_from end def grafana_url - ENV["DIFFDASH_GRAFANA_URL"] || ENV["GRAFANA_URL"] + @loader.grafana_url end def grafana_token - ENV["DIFFDASH_GRAFANA_TOKEN"] || ENV["GRAFANA_TOKEN"] + @loader.grafana_token end def grafana_folder_id - ENV["DIFFDASH_GRAFANA_FOLDER_ID"] || ENV["GRAFANA_FOLDER_ID"] + @loader.grafana_folder_id end def outputs - raw = ENV["DIFFDASH_OUTPUTS"].to_s - return [:grafana] if raw.strip.empty? - - raw.split(",") - .map(&:strip) - .reject(&:empty?) - .map(&:downcase) - .map(&:to_sym) + @loader.outputs end def dry_run? - ENV["DIFFDASH_DRY_RUN"] == "true" + @loader.dry_run? end def default_env - ENV["DIFFDASH_DEFAULT_ENV"] || "production" + @loader.default_env end def pr_comment? - ENV["DIFFDASH_PR_COMMENT"] != "false" + @loader.pr_comment? + end + + def app_name + @loader.app_name + end + + def pr_deploy_annotation_expr + @loader.pr_deploy_annotation_expr + end + + # File filtering configuration (new YAML-configurable options) + def ignore_paths + @loader.ignore_paths + end + + def include_paths + @loader.include_paths + end + + def excluded_suffixes + @loader.excluded_suffixes + end + + def excluded_directories + @loader.excluded_directories + end + + # Returns the full configuration as a hash (useful for debugging) + def to_h + @loader.to_h.merge( + max_logs: max_logs, + max_metrics: max_metrics, + max_events: max_events, + max_panels: max_panels + ) end end end diff --git a/lib/diffdash/config_loader.rb b/lib/diffdash/config_loader.rb new file mode 100644 index 0000000..805ff2e --- /dev/null +++ b/lib/diffdash/config_loader.rb @@ -0,0 +1,231 @@ +# frozen_string_literal: true + +require 'yaml' + +module Diffdash + # Loads configuration from diffdash.yml files with environment variable overrides. + # + # Configuration is loaded from the following sources in order of precedence: + # 1. Environment variables (highest priority) + # 2. Explicitly specified config file via --config flag + # 3. diffdash.yml in the current working directory + # 4. diffdash.yml in the git repository root + # 5. Default values (lowest priority) + # + # Example diffdash.yml: + # + # grafana: + # url: https://grafana.mycompany.com + # folder_id: 42 + # + # ignore_paths: + # - vendor/ + # - tmp/ + # - lib/legacy/ + # + # outputs: + # - grafana + # - json + # + # default_env: production + # pr_comment: true + # app_name: my-service + # + class ConfigLoader + CONFIG_FILE_NAMES = %w[diffdash.yml diffdash.yaml .diffdash.yml .diffdash.yaml].freeze + + attr_reader :config_path, :loaded_from + + def initialize(config_path: nil, working_dir: nil) + @explicit_config_path = config_path + @working_dir = working_dir || Dir.pwd + @file_config = {} + @loaded_from = nil + load_config_file + end + + # Grafana configuration + def grafana_url + env_value('DIFFDASH_GRAFANA_URL') || + env_value('GRAFANA_URL') || + file_value('grafana', 'url') + end + + def grafana_token + # Token should NOT be loaded from file for security - only env vars + env_value('DIFFDASH_GRAFANA_TOKEN') || env_value('GRAFANA_TOKEN') + end + + def grafana_folder_id + env_value('DIFFDASH_GRAFANA_FOLDER_ID') || + env_value('GRAFANA_FOLDER_ID') || + file_value('grafana', 'folder_id')&.to_s + end + + # Output configuration + def outputs + raw_env = env_value('DIFFDASH_OUTPUTS') + return parse_outputs_string(raw_env) if raw_env && !raw_env.strip.empty? + + file_outputs = file_value('outputs') + return file_outputs.map { |o| o.to_s.downcase.to_sym } if file_outputs.is_a?(Array) && file_outputs.any? + + [:grafana] # default + end + + # General settings + def dry_run? + env_value('DIFFDASH_DRY_RUN') == 'true' + end + + def default_env + env_value('DIFFDASH_DEFAULT_ENV') || + file_value('default_env') || + 'production' + end + + def pr_comment? + env_val = env_value('DIFFDASH_PR_COMMENT') + return env_val != 'false' unless env_val.nil? + + file_val = file_value('pr_comment') + return file_val != false unless file_val.nil? + + true # default + end + + def app_name + env_value('DIFFDASH_APP_NAME') || file_value('app_name') + end + + def pr_deploy_annotation_expr + env_value('DIFFDASH_PR_DEPLOY_ANNOTATION_EXPR') || + file_value('pr_deploy_annotation_expr') + end + + # File filtering configuration + def ignore_paths + file_value('ignore_paths') || [] + end + + def include_paths + file_value('include_paths') || [] + end + + def excluded_suffixes + file_value('excluded_suffixes') || %w[_spec.rb _test.rb] + end + + def excluded_directories + file_value('excluded_directories') || %w[spec test config] + end + + # Returns the full configuration as a hash (useful for debugging) + def to_h + { + loaded_from: @loaded_from, + grafana: { + url: grafana_url, + token: grafana_token ? '[REDACTED]' : nil, + folder_id: grafana_folder_id + }, + outputs: outputs, + default_env: default_env, + dry_run: dry_run?, + pr_comment: pr_comment?, + app_name: app_name, + ignore_paths: ignore_paths, + include_paths: include_paths, + excluded_suffixes: excluded_suffixes, + excluded_directories: excluded_directories + } + end + + private + + def load_config_file + @config_path = find_config_file + return unless @config_path + + begin + @file_config = YAML.load_file(@config_path, permitted_classes: [Symbol]) || {} + @loaded_from = @config_path + + unless @file_config.is_a?(Hash) + warn "[diffdash] Warning: #{@config_path} is not a valid configuration (expected hash)" + @file_config = {} + end + rescue Psych::SyntaxError => e + warn "[diffdash] Warning: Failed to parse #{@config_path}: #{e.message}" + @file_config = {} + rescue Errno::ENOENT + @file_config = {} + end + end + + def find_config_file + # 1. Explicit config path takes highest priority + if @explicit_config_path + return @explicit_config_path if File.exist?(@explicit_config_path) + + warn "[diffdash] Warning: Config file not found: #{@explicit_config_path}" + return nil + end + + # 2. Check working directory + CONFIG_FILE_NAMES.each do |name| + path = File.join(@working_dir, name) + return path if File.exist?(path) + end + + # 3. Check git root (if different from working directory) + git_root = find_git_root + if git_root && git_root != @working_dir + CONFIG_FILE_NAMES.each do |name| + path = File.join(git_root, name) + return path if File.exist?(path) + end + end + + nil + end + + def find_git_root + dir = @working_dir + loop do + return dir if File.directory?(File.join(dir, '.git')) + + parent = File.dirname(dir) + return nil if parent == dir # reached filesystem root + + dir = parent + end + end + + def env_value(key) + val = ENV[key] + val.nil? || val.empty? ? nil : val + end + + def file_value(*keys) + keys.reduce(@file_config) do |hash, key| + return nil unless hash.is_a?(Hash) + + # Use key? to check existence since || doesn't work with false values + if hash.key?(key.to_s) + hash[key.to_s] + elsif hash.key?(key.to_sym) + hash[key.to_sym] + end + end + end + + def parse_outputs_string(raw) + raw.split(',') + .map(&:strip) + .reject(&:empty?) + .map(&:downcase) + .map(&:to_sym) + end + end +end diff --git a/lib/diffdash/engine/change_set.rb b/lib/diffdash/engine/change_set.rb index 9f6d4f6..ca49db4 100644 --- a/lib/diffdash/engine/change_set.rb +++ b/lib/diffdash/engine/change_set.rb @@ -13,10 +13,14 @@ def initialize(branch_name:, changed_files:, filtered_files:) @filtered_files = filtered_files end - def self.from_git + # Create a ChangeSet from git context. + # + # @param config [Config, nil] optional config for file filtering rules + def self.from_git(config: nil) git_context = GitContext.new changed_files = git_context.changed_files - filtered_files = FileFilter.filter(changed_files) + file_filter = FileFilter.new(config: config) + filtered_files = file_filter.filter(changed_files) new( branch_name: git_context.branch_name, diff --git a/lib/diffdash/file_filter.rb b/lib/diffdash/file_filter.rb index 1a56bb7..2760f5b 100644 --- a/lib/diffdash/file_filter.rb +++ b/lib/diffdash/file_filter.rb @@ -1,33 +1,114 @@ # frozen_string_literal: true module Diffdash + # Filters files based on configurable rules. + # + # By default, includes only Ruby files and excludes test files and directories. + # Can be customized via diffdash.yml: + # + # ignore_paths: + # - vendor/ + # - lib/legacy/ + # + # include_paths: + # - app/ + # - lib/ + # + # excluded_suffixes: + # - _spec.rb + # - _test.rb + # + # excluded_directories: + # - spec + # - test + # - config + # class FileFilter - EXCLUDED_SUFFIXES = %w[_spec.rb _test.rb].freeze - EXCLUDED_DIRECTORIES = %r{/(spec|test|config)/}.freeze - RUBY_EXTENSION = ".rb" + DEFAULT_EXCLUDED_SUFFIXES = %w[_spec.rb _test.rb].freeze + DEFAULT_EXCLUDED_DIRECTORIES = %w[spec test config].freeze + RUBY_EXTENSION = '.rb' - class << self - def filter(files) - files.select { |f| include_file?(f) } - end + attr_reader :excluded_suffixes, :excluded_directories, :ignore_paths, :include_paths - def include_file?(file_path) - return false unless file_path.end_with?(RUBY_EXTENSION) - return false if excluded_suffix?(file_path) - return false if excluded_directory?(file_path) + # Initialize with optional config for customizable filtering rules. + # + # @param config [Config, nil] configuration object with filtering rules + def initialize(config: nil) + @excluded_suffixes = config&.excluded_suffixes || DEFAULT_EXCLUDED_SUFFIXES + @excluded_directories = config&.excluded_directories || DEFAULT_EXCLUDED_DIRECTORIES + @ignore_paths = config&.ignore_paths || [] + @include_paths = config&.include_paths || [] + @excluded_directories_regex = build_directories_regex(@excluded_directories) + end - true - end + def filter(files) + files.select { |f| include_file?(f) } + end + + def include_file?(file_path) + return false unless file_path.end_with?(RUBY_EXTENSION) + return false if excluded_suffix?(file_path) + return false if excluded_directory?(file_path) + return false if ignored_path?(file_path) + return false if include_paths_configured? && !included_path?(file_path) + + true + end + + # Class-level convenience method for backwards compatibility. + # Uses default filtering rules (no config). + def self.filter(files) + new.filter(files) + end + + # Class-level convenience method for backwards compatibility. + # Uses default filtering rules (no config). + def self.include_file?(file_path) + new.include_file?(file_path) + end + + private - private + def excluded_suffix?(file_path) + @excluded_suffixes.any? { |suffix| file_path.end_with?(suffix) } + end + + def excluded_directory?(file_path) + file_path.match?(@excluded_directories_regex) + end + + def ignored_path?(file_path) + @ignore_paths.any? do |pattern| + match_path_pattern?(file_path, pattern) + end + end - def excluded_suffix?(file_path) - EXCLUDED_SUFFIXES.any? { |suffix| file_path.end_with?(suffix) } + def included_path?(file_path) + @include_paths.any? do |pattern| + match_path_pattern?(file_path, pattern) end + end + + def include_paths_configured? + @include_paths.any? + end - def excluded_directory?(file_path) - file_path.match?(EXCLUDED_DIRECTORIES) + def match_path_pattern?(file_path, pattern) + # Support glob-style patterns and simple prefix matching + if pattern.include?('*') + File.fnmatch?(pattern, file_path, File::FNM_PATHNAME) + else + # Normalize pattern to ensure consistent matching + normalized = pattern.chomp('/') + file_path.start_with?(normalized) || file_path.include?("/#{normalized}/") end end + + def build_directories_regex(directories) + return /(?!)/ if directories.empty? # Never match + + escaped = directories.map { |d| Regexp.escape(d) } + Regexp.new("/(#{escaped.join('|')})/") + end end end diff --git a/lib/diffdash/outputs/grafana.rb b/lib/diffdash/outputs/grafana.rb index 2da841c..64e0848 100644 --- a/lib/diffdash/outputs/grafana.rb +++ b/lib/diffdash/outputs/grafana.rb @@ -5,12 +5,15 @@ module Outputs # Grafana output adapter. # Translates SignalQuery intent into Grafana dashboard JSON. class Grafana < Base - def initialize(title:, folder_id:, dry_run: false, verbose: false, default_env: "production") + def initialize(title:, folder_id:, dry_run: false, verbose: false, default_env: "production", + app_name: nil, pr_deploy_annotation_expr: nil) @title = title @folder_id = folder_id @dry_run = dry_run @verbose = verbose @default_env = default_env + @app_name = app_name + @pr_deploy_annotation_expr = pr_deploy_annotation_expr end # Render SignalBundle into Grafana dashboard payload @@ -123,8 +126,8 @@ def app_variable end def infer_app_name - # Priority: ENV var > Git repo name > wildcard - return ENV["DIFFDASH_APP_NAME"] if ENV["DIFFDASH_APP_NAME"] + # Priority: Configured app_name > Git repo name > wildcard + return @app_name if @app_name && !@app_name.empty? # Try to get repo name from git remote begin @@ -420,7 +423,7 @@ def pr_deployment_annotation(signal_bundle) end def pr_deployment_expr(signal_bundle) - override = ENV["DIFFDASH_PR_DEPLOY_ANNOTATION_EXPR"].to_s.strip + override = @pr_deploy_annotation_expr.to_s.strip return override unless override.empty? branch = signal_bundle.metadata.dig(:change_set, :branch_name).to_s.strip diff --git a/spec/diffdash/config_loader_spec.rb b/spec/diffdash/config_loader_spec.rb new file mode 100644 index 0000000..0f11d0f --- /dev/null +++ b/spec/diffdash/config_loader_spec.rb @@ -0,0 +1,293 @@ +# frozen_string_literal: true + +require "spec_helper" +require "tempfile" +require "tmpdir" + +RSpec.describe Diffdash::ConfigLoader do + let(:temp_dir) { Dir.mktmpdir } + + after do + FileUtils.remove_entry(temp_dir) if temp_dir && Dir.exist?(temp_dir) + end + + def create_config_file(content, filename: "diffdash.yml", dir: temp_dir) + path = File.join(dir, filename) + File.write(path, content) + path + end + + describe "file discovery" do + it "finds diffdash.yml in the working directory" do + create_config_file("default_env: staging") + loader = described_class.new(working_dir: temp_dir) + expect(loader.loaded_from).to eq(File.join(temp_dir, "diffdash.yml")) + end + + it "finds diffdash.yaml as an alternative extension" do + create_config_file("default_env: staging", filename: "diffdash.yaml") + loader = described_class.new(working_dir: temp_dir) + expect(loader.loaded_from).to eq(File.join(temp_dir, "diffdash.yaml")) + end + + it "finds .diffdash.yml (hidden file)" do + create_config_file("default_env: staging", filename: ".diffdash.yml") + loader = described_class.new(working_dir: temp_dir) + expect(loader.loaded_from).to eq(File.join(temp_dir, ".diffdash.yml")) + end + + it "prefers diffdash.yml over .diffdash.yml" do + create_config_file("default_env: one", filename: "diffdash.yml") + create_config_file("default_env: two", filename: ".diffdash.yml") + loader = described_class.new(working_dir: temp_dir) + expect(loader.loaded_from).to eq(File.join(temp_dir, "diffdash.yml")) + expect(loader.default_env).to eq("one") + end + + it "uses explicit config_path when provided" do + custom_path = create_config_file("default_env: custom", filename: "custom.yml") + loader = described_class.new(config_path: custom_path) + expect(loader.loaded_from).to eq(custom_path) + expect(loader.default_env).to eq("custom") + end + + it "warns when explicit config_path does not exist" do + expect do + described_class.new(config_path: "/nonexistent/config.yml") + end.to output(/Warning: Config file not found/).to_stderr + end + + it "returns nil for loaded_from when no config file is found" do + empty_dir = Dir.mktmpdir + loader = described_class.new(working_dir: empty_dir) + expect(loader.loaded_from).to be_nil + FileUtils.remove_entry(empty_dir) + end + end + + describe "grafana configuration" do + it "loads grafana.url from config file" do + create_config_file(<<~YAML) + grafana: + url: https://grafana.example.com + YAML + loader = described_class.new(working_dir: temp_dir) + expect(loader.grafana_url).to eq("https://grafana.example.com") + end + + it "loads grafana.folder_id from config file" do + create_config_file(<<~YAML) + grafana: + folder_id: 42 + YAML + loader = described_class.new(working_dir: temp_dir) + expect(loader.grafana_folder_id).to eq("42") + end + + it "prioritizes DIFFDASH_GRAFANA_URL over file config" do + create_config_file(<<~YAML) + grafana: + url: https://file.example.com + YAML + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DIFFDASH_GRAFANA_URL").and_return("https://env.example.com") + loader = described_class.new(working_dir: temp_dir) + expect(loader.grafana_url).to eq("https://env.example.com") + end + + it "only allows grafana_token from environment variables (security)" do + create_config_file(<<~YAML) + grafana: + token: should-be-ignored + YAML + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DIFFDASH_GRAFANA_TOKEN").and_return(nil) + allow(ENV).to receive(:[]).with("GRAFANA_TOKEN").and_return(nil) + loader = described_class.new(working_dir: temp_dir) + expect(loader.grafana_token).to be_nil + end + end + + describe "outputs configuration" do + it "loads outputs array from config file" do + create_config_file(<<~YAML) + outputs: + - grafana + - json + YAML + loader = described_class.new(working_dir: temp_dir) + expect(loader.outputs).to eq([:grafana, :json]) + end + + it "defaults to [:grafana] when not configured" do + create_config_file("") + loader = described_class.new(working_dir: temp_dir) + expect(loader.outputs).to eq([:grafana]) + end + + it "prioritizes DIFFDASH_OUTPUTS env var over file config" do + create_config_file(<<~YAML) + outputs: + - grafana + YAML + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DIFFDASH_OUTPUTS").and_return("json") + loader = described_class.new(working_dir: temp_dir) + expect(loader.outputs).to eq([:json]) + end + end + + describe "general settings" do + it "loads default_env from config file" do + create_config_file("default_env: staging") + loader = described_class.new(working_dir: temp_dir) + expect(loader.default_env).to eq("staging") + end + + it "defaults default_env to 'production'" do + create_config_file("") + loader = described_class.new(working_dir: temp_dir) + expect(loader.default_env).to eq("production") + end + + it "loads pr_comment from config file" do + create_config_file("pr_comment: false") + loader = described_class.new(working_dir: temp_dir) + expect(loader.pr_comment?).to be false + end + + it "defaults pr_comment to true" do + create_config_file("") + loader = described_class.new(working_dir: temp_dir) + expect(loader.pr_comment?).to be true + end + + it "loads app_name from config file" do + create_config_file("app_name: my-service") + loader = described_class.new(working_dir: temp_dir) + expect(loader.app_name).to eq("my-service") + end + + it "loads pr_deploy_annotation_expr from config file" do + create_config_file('pr_deploy_annotation_expr: changes(deploy_ts[5m]) > 0') + loader = described_class.new(working_dir: temp_dir) + expect(loader.pr_deploy_annotation_expr).to eq("changes(deploy_ts[5m]) > 0") + end + end + + describe "file filtering configuration" do + it "loads ignore_paths from config file" do + create_config_file(<<~YAML) + ignore_paths: + - vendor/ + - lib/legacy/ + YAML + loader = described_class.new(working_dir: temp_dir) + expect(loader.ignore_paths).to eq(["vendor/", "lib/legacy/"]) + end + + it "defaults ignore_paths to empty array" do + create_config_file("") + loader = described_class.new(working_dir: temp_dir) + expect(loader.ignore_paths).to eq([]) + end + + it "loads include_paths from config file" do + create_config_file(<<~YAML) + include_paths: + - app/ + - lib/ + YAML + loader = described_class.new(working_dir: temp_dir) + expect(loader.include_paths).to eq(["app/", "lib/"]) + end + + it "loads excluded_suffixes from config file" do + create_config_file(<<~YAML) + excluded_suffixes: + - _spec.rb + - _integration.rb + YAML + loader = described_class.new(working_dir: temp_dir) + expect(loader.excluded_suffixes).to eq(["_spec.rb", "_integration.rb"]) + end + + it "defaults excluded_suffixes to _spec.rb and _test.rb" do + create_config_file("") + loader = described_class.new(working_dir: temp_dir) + expect(loader.excluded_suffixes).to eq(%w[_spec.rb _test.rb]) + end + + it "loads excluded_directories from config file" do + create_config_file(<<~YAML) + excluded_directories: + - spec + - test + - features + YAML + loader = described_class.new(working_dir: temp_dir) + expect(loader.excluded_directories).to eq(["spec", "test", "features"]) + end + + it "defaults excluded_directories to spec, test, config" do + create_config_file("") + loader = described_class.new(working_dir: temp_dir) + expect(loader.excluded_directories).to eq(%w[spec test config]) + end + end + + describe "error handling" do + it "handles invalid YAML gracefully" do + create_config_file("invalid: yaml: content: [") + expect do + described_class.new(working_dir: temp_dir) + end.to output(/Warning: Failed to parse/).to_stderr + end + + it "handles non-hash YAML content gracefully" do + create_config_file("- just\n- a\n- list") + expect do + described_class.new(working_dir: temp_dir) + end.to output(/Warning.*not a valid configuration/).to_stderr + end + + it "returns defaults when config file has parsing errors" do + create_config_file("invalid: yaml: content: [") + loader = nil + expect do + loader = described_class.new(working_dir: temp_dir) + end.to output(/Warning/).to_stderr + expect(loader.default_env).to eq("production") + expect(loader.outputs).to eq([:grafana]) + end + end + + describe "#to_h" do + it "returns a hash representation of the configuration" do + create_config_file(<<~YAML) + grafana: + url: https://grafana.example.com + folder_id: 42 + default_env: staging + app_name: my-app + YAML + loader = described_class.new(working_dir: temp_dir) + hash = loader.to_h + + expect(hash[:grafana][:url]).to eq("https://grafana.example.com") + expect(hash[:grafana][:folder_id]).to eq("42") + expect(hash[:default_env]).to eq("staging") + expect(hash[:app_name]).to eq("my-app") + expect(hash[:loaded_from]).to include("diffdash.yml") + end + + it "redacts the grafana token in the hash output" do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DIFFDASH_GRAFANA_TOKEN").and_return("secret-token") + loader = described_class.new(working_dir: temp_dir) + hash = loader.to_h + + expect(hash[:grafana][:token]).to eq("[REDACTED]") + end + end +end diff --git a/spec/diffdash/config_spec.rb b/spec/diffdash/config_spec.rb index 58ef293..f8e6e98 100644 --- a/spec/diffdash/config_spec.rb +++ b/spec/diffdash/config_spec.rb @@ -1,7 +1,20 @@ # frozen_string_literal: true +require "tmpdir" + RSpec.describe Diffdash::Config do - subject(:config) { described_class.new } + let(:temp_dir) { Dir.mktmpdir } + + after do + FileUtils.remove_entry(temp_dir) if temp_dir && Dir.exist?(temp_dir) + end + + def create_config_file(content, dir: temp_dir) + File.write(File.join(dir, "diffdash.yml"), content) + end + + # Use a working directory with no config file for ENV-based tests + subject(:config) { described_class.new(working_dir: temp_dir) } describe "guard rail limits" do it "has MAX_LOGS of 10" do @@ -42,6 +55,18 @@ allow(ENV).to receive(:[]).with("GRAFANA_URL").and_return(nil) expect(config.grafana_url).to be_nil end + + it "loads from config file when env vars not set" do + create_config_file(<<~YAML) + grafana: + url: https://file.example.com + YAML + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DIFFDASH_GRAFANA_URL").and_return(nil) + allow(ENV).to receive(:[]).with("GRAFANA_URL").and_return(nil) + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.grafana_url).to eq("https://file.example.com") + end end describe "#grafana_token" do @@ -74,20 +99,35 @@ allow(ENV).to receive(:[]).with("GRAFANA_FOLDER_ID").and_return("123") expect(config.grafana_folder_id).to eq("123") end + + it "loads from config file when env vars not set" do + create_config_file(<<~YAML) + grafana: + folder_id: 99 + YAML + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DIFFDASH_GRAFANA_FOLDER_ID").and_return(nil) + allow(ENV).to receive(:[]).with("GRAFANA_FOLDER_ID").and_return(nil) + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.grafana_folder_id).to eq("99") + end end describe "#dry_run?" do it "returns true when DIFFDASH_DRY_RUN is 'true'" do + allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).with("DIFFDASH_DRY_RUN").and_return("true") expect(config.dry_run?).to be true end it "returns false when DIFFDASH_DRY_RUN is not 'true'" do + allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).with("DIFFDASH_DRY_RUN").and_return("false") expect(config.dry_run?).to be false end it "returns false when DIFFDASH_DRY_RUN is not set" do + allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).with("DIFFDASH_DRY_RUN").and_return(nil) expect(config.dry_run?).to be false end @@ -95,30 +135,121 @@ describe "#default_env" do it "returns DIFFDASH_DEFAULT_ENV when set" do + allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).with("DIFFDASH_DEFAULT_ENV").and_return("staging") expect(config.default_env).to eq("staging") end it "defaults to 'production' when not set" do + allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).with("DIFFDASH_DEFAULT_ENV").and_return(nil) expect(config.default_env).to eq("production") end + + it "loads from config file when env var not set" do + create_config_file("default_env: development") + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DIFFDASH_DEFAULT_ENV").and_return(nil) + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.default_env).to eq("development") + end end describe "#pr_comment?" do it "returns true by default" do + allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).with("DIFFDASH_PR_COMMENT").and_return(nil) expect(config.pr_comment?).to be true end it "returns false when DIFFDASH_PR_COMMENT is 'false'" do + allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).with("DIFFDASH_PR_COMMENT").and_return("false") expect(config.pr_comment?).to be false end it "returns true for any other value" do + allow(ENV).to receive(:[]).and_call_original allow(ENV).to receive(:[]).with("DIFFDASH_PR_COMMENT").and_return("true") expect(config.pr_comment?).to be true end + + it "loads from config file when env var not set" do + create_config_file("pr_comment: false") + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with("DIFFDASH_PR_COMMENT").and_return(nil) + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.pr_comment?).to be false + end + end + + describe "#loaded_from" do + it "returns nil when no config file is found" do + expect(config.loaded_from).to be_nil + end + + it "returns the path to the loaded config file" do + create_config_file("default_env: staging") + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.loaded_from).to eq(File.join(temp_dir, "diffdash.yml")) + end + end + + describe "file filtering configuration" do + it "returns ignore_paths from config file" do + create_config_file(<<~YAML) + ignore_paths: + - vendor/ + - lib/legacy/ + YAML + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.ignore_paths).to eq(["vendor/", "lib/legacy/"]) + end + + it "returns include_paths from config file" do + create_config_file(<<~YAML) + include_paths: + - app/ + YAML + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.include_paths).to eq(["app/"]) + end + + it "returns excluded_suffixes from config file" do + create_config_file(<<~YAML) + excluded_suffixes: + - _spec.rb + - _integration.rb + YAML + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.excluded_suffixes).to eq(["_spec.rb", "_integration.rb"]) + end + + it "returns excluded_directories from config file" do + create_config_file(<<~YAML) + excluded_directories: + - spec + - features + YAML + new_config = described_class.new(working_dir: temp_dir) + expect(new_config.excluded_directories).to eq(["spec", "features"]) + end + end + + describe "#to_h" do + it "returns a hash representation of the configuration" do + create_config_file(<<~YAML) + grafana: + url: https://grafana.example.com + default_env: staging + YAML + new_config = described_class.new(working_dir: temp_dir) + hash = new_config.to_h + + expect(hash[:grafana][:url]).to eq("https://grafana.example.com") + expect(hash[:default_env]).to eq("staging") + expect(hash[:max_logs]).to eq(10) + expect(hash[:max_panels]).to eq(12) + end end end diff --git a/spec/diffdash/file_filter_spec.rb b/spec/diffdash/file_filter_spec.rb index ceaa665..82c13b4 100644 --- a/spec/diffdash/file_filter_spec.rb +++ b/spec/diffdash/file_filter_spec.rb @@ -1,73 +1,195 @@ # frozen_string_literal: true RSpec.describe Diffdash::FileFilter do - describe ".filter" do - it "includes regular Ruby files" do - files = ["/app/models/user.rb", "/app/services/payment.rb"] - expect(described_class.filter(files)).to eq(files) - end + describe "class methods (backwards compatibility)" do + describe ".filter" do + it "includes regular Ruby files" do + files = ["/app/models/user.rb", "/app/services/payment.rb"] + expect(described_class.filter(files)).to eq(files) + end - it "excludes _spec.rb files" do - files = ["/app/models/user.rb", "/spec/models/user_spec.rb"] - expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) - end + it "excludes _spec.rb files" do + files = ["/app/models/user.rb", "/spec/models/user_spec.rb"] + expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) + end - it "excludes _test.rb files" do - files = ["/app/models/user.rb", "/test/models/user_test.rb"] - expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) - end + it "excludes _test.rb files" do + files = ["/app/models/user.rb", "/test/models/user_test.rb"] + expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) + end - it "excludes files in /spec/ directory" do - files = ["/app/models/user.rb", "/spec/support/helpers.rb"] - expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) - end + it "excludes files in /spec/ directory" do + files = ["/app/models/user.rb", "/spec/support/helpers.rb"] + expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) + end - it "excludes files in /test/ directory" do - files = ["/app/models/user.rb", "/test/fixtures/data.rb"] - expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) - end + it "excludes files in /test/ directory" do + files = ["/app/models/user.rb", "/test/fixtures/data.rb"] + expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) + end - it "excludes files in /config/ directory" do - files = ["/app/models/user.rb", "/config/initializers/sidekiq.rb"] - expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) - end + it "excludes files in /config/ directory" do + files = ["/app/models/user.rb", "/config/initializers/sidekiq.rb"] + expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) + end - it "excludes non-Ruby files" do - files = ["/app/models/user.rb", "/README.md", "/config.yml", "/data.json"] - expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) - end + it "excludes non-Ruby files" do + files = ["/app/models/user.rb", "/README.md", "/config.yml", "/data.json"] + expect(described_class.filter(files)).to eq(["/app/models/user.rb"]) + end + + it "returns empty array when all files are excluded" do + files = ["/spec/models/user_spec.rb", "/test/models/user_test.rb"] + expect(described_class.filter(files)).to eq([]) + end - it "returns empty array when all files are excluded" do - files = ["/spec/models/user_spec.rb", "/test/models/user_test.rb"] - expect(described_class.filter(files)).to eq([]) + it "handles empty input" do + expect(described_class.filter([])).to eq([]) + end end - it "handles empty input" do - expect(described_class.filter([])).to eq([]) + describe ".include_file?" do + it "returns true for regular Ruby application files" do + expect(described_class.include_file?("/app/models/user.rb")).to be true + expect(described_class.include_file?("/lib/diffdash/parser.rb")).to be true + end + + it "returns false for spec files" do + expect(described_class.include_file?("/spec/models/user_spec.rb")).to be false + end + + it "returns false for test files" do + expect(described_class.include_file?("/test/models/user_test.rb")).to be false + end + + it "returns false for config files" do + expect(described_class.include_file?("/config/application.rb")).to be false + end + + it "returns false for non-Ruby files" do + expect(described_class.include_file?("/README.md")).to be false + expect(described_class.include_file?("/config.yml")).to be false + end end end - describe ".include_file?" do - it "returns true for regular Ruby application files" do - expect(described_class.include_file?("/app/models/user.rb")).to be true - expect(described_class.include_file?("/lib/diffdash/parser.rb")).to be true + describe "instance methods with config" do + let(:config) { double("Config") } + + before do + allow(config).to receive(:excluded_suffixes).and_return(%w[_spec.rb _test.rb]) + allow(config).to receive(:excluded_directories).and_return(%w[spec test config]) + allow(config).to receive(:ignore_paths).and_return([]) + allow(config).to receive(:include_paths).and_return([]) end - it "returns false for spec files" do - expect(described_class.include_file?("/spec/models/user_spec.rb")).to be false + describe "#filter" do + subject(:filter) { described_class.new(config: config) } + + it "includes regular Ruby files" do + files = ["/app/models/user.rb", "/app/services/payment.rb"] + expect(filter.filter(files)).to eq(files) + end + + it "excludes files based on configured suffixes" do + allow(config).to receive(:excluded_suffixes).and_return(%w[_spec.rb _integration.rb]) + files = ["/app/models/user.rb", "/spec/user_spec.rb", "/test/user_integration.rb"] + expect(filter.filter(files)).to eq(["/app/models/user.rb"]) + end + + it "excludes files based on configured directories" do + allow(config).to receive(:excluded_directories).and_return(%w[spec vendor]) + files = ["/app/models/user.rb", "/spec/user_spec.rb", "/vendor/bundle/gems.rb"] + expect(filter.filter(files)).to eq(["/app/models/user.rb"]) + end end - it "returns false for test files" do - expect(described_class.include_file?("/test/models/user_test.rb")).to be false + describe "#include_file? with ignore_paths" do + subject(:filter) { described_class.new(config: config) } + + it "excludes files matching ignore_paths patterns" do + allow(config).to receive(:ignore_paths).and_return(["vendor/", "lib/legacy/"]) + expect(filter.include_file?("/vendor/bundle/active_record.rb")).to be false + expect(filter.include_file?("/lib/legacy/old_code.rb")).to be false + expect(filter.include_file?("/app/models/user.rb")).to be true + end + + it "supports glob patterns in ignore_paths" do + allow(config).to receive(:ignore_paths).and_return(["lib/**/*_old.rb"]) + expect(filter.include_file?("lib/utils/parser_old.rb")).to be false + expect(filter.include_file?("lib/utils/parser.rb")).to be true + end + + it "matches paths containing the pattern" do + allow(config).to receive(:ignore_paths).and_return(["legacy"]) + expect(filter.include_file?("/app/legacy/code.rb")).to be false + expect(filter.include_file?("/lib/legacy/utils.rb")).to be false + expect(filter.include_file?("/app/models/user.rb")).to be true + end end - it "returns false for config files" do - expect(described_class.include_file?("/config/application.rb")).to be false + describe "#include_file? with include_paths" do + subject(:filter) { described_class.new(config: config) } + + it "only includes files matching include_paths when configured" do + allow(config).to receive(:include_paths).and_return(["app/", "lib/"]) + expect(filter.include_file?("/app/models/user.rb")).to be true + expect(filter.include_file?("/lib/utils/parser.rb")).to be true + expect(filter.include_file?("/bin/script.rb")).to be false + end + + it "includes all files when include_paths is empty" do + allow(config).to receive(:include_paths).and_return([]) + expect(filter.include_file?("/app/models/user.rb")).to be true + expect(filter.include_file?("/bin/script.rb")).to be true + end + + it "supports glob patterns in include_paths" do + allow(config).to receive(:include_paths).and_return(["app/**/*.rb"]) + expect(filter.include_file?("app/models/user.rb")).to be true + expect(filter.include_file?("lib/utils/parser.rb")).to be false + end end - it "returns false for non-Ruby files" do - expect(described_class.include_file?("/README.md")).to be false - expect(described_class.include_file?("/config.yml")).to be false + describe "#include_file? with combined filters" do + subject(:filter) { described_class.new(config: config) } + + it "applies all filters in order" do + allow(config).to receive(:ignore_paths).and_return(["vendor/"]) + allow(config).to receive(:include_paths).and_return(["app/", "lib/"]) + + # Included by include_paths, not in ignore_paths + expect(filter.include_file?("/app/models/user.rb")).to be true + + # Not in include_paths + expect(filter.include_file?("/bin/script.rb")).to be false + + # In ignore_paths (even though it might match include_paths) + expect(filter.include_file?("/vendor/bundle/active.rb")).to be false + + # Excluded by suffix + expect(filter.include_file?("/app/models/user_spec.rb")).to be false + + # Excluded by directory + expect(filter.include_file?("/spec/models/user.rb")).to be false + end + end + + describe "custom excluded_directories regex building" do + it "handles empty excluded_directories" do + allow(config).to receive(:excluded_directories).and_return([]) + filter = described_class.new(config: config) + expect(filter.include_file?("/spec/user_spec.rb")).to be false # Still excluded by suffix + expect(filter.include_file?("/spec/support/helpers.rb")).to be true # Directory not excluded + end + + it "escapes special regex characters in directory names" do + allow(config).to receive(:excluded_directories).and_return(["spec.d", "test+files"]) + filter = described_class.new(config: config) + expect(filter.include_file?("/spec.d/helpers.rb")).to be false + expect(filter.include_file?("/test+files/data.rb")).to be false + expect(filter.include_file?("/specd/helpers.rb")).to be true # Not matching unescaped + end end end end From e1e3eacf065f675c602e7d7fa56b18250399b565 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 25 Jan 2026 12:41:35 +0000 Subject: [PATCH 2/2] docs: Update README and CHANGELOG for YAML configuration support - Add Configuration File section with full example - Update Quick Start with both YAML and env var options - Add --config flag to CLI options - Update Environment Variables table with config file equivalents - Expand File Filtering section with customization options - Simplify GitHub Actions example to use config file - Add YAML configuration to CHANGELOG Co-authored-by: r.buddie --- CHANGELOG.md | 5 ++ README.md | 189 +++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 164 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 004bb11..095ad3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- YAML configuration file support (`diffdash.yml`) for shared team configuration +- New `--config` CLI flag to specify custom config file path +- Configurable file filtering: `ignore_paths`, `include_paths`, `excluded_suffixes`, `excluded_directories` +- Configuration file discovery (current dir, git root, explicit path) +- Example configuration file (`diffdash.example.yml`) - Grafana v1 golden fixture contract test - Adapter isolation spec - Hard-fail on missing branch name diff --git a/README.md b/README.md index 26565cf..d2b9605 100644 --- a/README.md +++ b/README.md @@ -23,16 +23,39 @@ gem install diffdash-*.gem ## Quick Start -### 1. Add these to your application's `.env` (dotenv) file +### Option A: Configuration File (Recommended for Teams) + +Create a `diffdash.yml` in your repository root: + +```yaml +grafana: + url: https://myorg.grafana.net + folder_id: 42 + +ignore_paths: + - vendor/ + - lib/legacy/ + +default_env: production +``` + +Then set your API token via environment variable (never commit tokens): + +```bash +export DIFFDASH_GRAFANA_TOKEN=glsa_xxxxxxxxxxxx +``` + +### Option B: Environment Variables Only + +Add to your `.env` file: ```bash DIFFDASH_GRAFANA_URL=https://myorg.grafana.net DIFFDASH_GRAFANA_TOKEN=glsa_xxxxxxxxxxxx DIFFDASH_GRAFANA_FOLDER_ID=42 # optional -DIFFDASH_OUTPUTS=grafana,json ``` -### 2. Find your folder ID (optional) +### Find Your Folder ID (Optional) ```bash diffdash folders @@ -49,7 +72,7 @@ Available Grafana folders: Set DIFFDASH_GRAFANA_FOLDER_ID in your .env file to use a specific folder ``` -### 3. Generate Dashboard +### Generate Dashboard ```bash # From your repo with changed files @@ -57,6 +80,9 @@ diffdash # Or dry-run to see JSON without uploading diffdash --dry-run + +# Use a custom config file +diffdash --config path/to/config.yml ``` ## CLI Usage @@ -70,26 +96,87 @@ diffdash [command] [options] - *(none)* - Run analysis and generate/upload dashboard **Options:** +- `--config FILE` - Path to configuration file (default: `diffdash.yml` in repo root) - `--dry-run` - Generate JSON only, don't upload to Grafana -- `--verbose` - Show detailed progress and dynamic metric warnings +- `--verbose` - Show detailed progress, config source, and dynamic metric warnings - `--version` - Show version number - `--help` - Show help +## Configuration File + +Create a `diffdash.yml` (or `.diffdash.yml`) in your repository root to share configuration across your team. + +### Example Configuration + +```yaml +# Grafana connection +grafana: + url: https://myorg.grafana.net + folder_id: 42 + +# Output adapters (grafana, json) +outputs: + - grafana + - json + +# General settings +default_env: production +pr_comment: true +app_name: my-service + +# File filtering +ignore_paths: + - vendor/ + - lib/legacy/ + - tmp/ + +include_paths: # Optional whitelist (empty = scan all) + - app/ + - lib/ + +excluded_suffixes: # Defaults: _spec.rb, _test.rb + - _spec.rb + - _test.rb + +excluded_directories: # Defaults: spec, test, config + - spec + - test + - config +``` + +### Configuration Precedence + +Configuration is loaded from multiple sources (highest to lowest priority): + +1. **Environment variables** - Always take precedence +2. **`--config` flag** - Explicitly specified config file +3. **Config file in current directory** - `diffdash.yml`, `.diffdash.yml`, `diffdash.yaml`, `.diffdash.yaml` +4. **Config file in git root** - If different from current directory +5. **Default values** + +### Security Note + +**API tokens are only loaded from environment variables** — never from config files. This prevents accidental commits of secrets. The `grafana.token` key is intentionally ignored if present in the YAML. + ## Environment Variables -Set these in a `.env` file in your project root: +Environment variables can be set in a `.env` file or exported directly. They **always override** config file values. -| Variable | Required | Description | -|----------|----------|-------------| -| `DIFFDASH_GRAFANA_URL` | Yes | Grafana instance URL (e.g., `https://myorg.grafana.net`) | -| `DIFFDASH_GRAFANA_TOKEN` | Yes | Grafana API token (Service Account token with Editor role) | -| `DIFFDASH_GRAFANA_FOLDER_ID` | No | Target folder ID for dashboards | -| `DIFFDASH_OUTPUTS` | No | Comma-separated outputs (default: `grafana`) | -| `DIFFDASH_DRY_RUN` | No | Set to `true` to force dry-run mode | -| `DIFFDASH_DEFAULT_ENV` | No | Default environment filter (default: `production`) | -| `DIFFDASH_APP_NAME` | No | Override app name in dashboard (defaults to Git repo name) | -| `DIFFDASH_PR_COMMENT` | No | Set to `false` to disable PR comments with dashboard link | -| `DIFFDASH_PR_DEPLOY_ANNOTATION_EXPR` | No | PromQL expr for PR deployment annotation | +| Variable | Required | Config File Equivalent | Description | +|----------|----------|------------------------|-------------| +| `DIFFDASH_GRAFANA_URL` | Yes* | `grafana.url` | Grafana instance URL | +| `DIFFDASH_GRAFANA_TOKEN` | Yes | *(env only)* | Grafana API token (never in config file) | +| `DIFFDASH_GRAFANA_FOLDER_ID` | No | `grafana.folder_id` | Target folder ID for dashboards | +| `DIFFDASH_OUTPUTS` | No | `outputs` | Comma-separated outputs (default: `grafana`) | +| `DIFFDASH_DRY_RUN` | No | — | Set to `true` to force dry-run mode | +| `DIFFDASH_DEFAULT_ENV` | No | `default_env` | Default environment filter (default: `production`) | +| `DIFFDASH_APP_NAME` | No | `app_name` | Override app name (defaults to Git repo name) | +| `DIFFDASH_PR_COMMENT` | No | `pr_comment` | Set to `false` to disable PR comments | +| `DIFFDASH_PR_DEPLOY_ANNOTATION_EXPR` | No | `pr_deploy_annotation_expr` | PromQL expr for PR deployment annotation | + +*Required unless set in config file. + +**Legacy fallbacks:** `GRAFANA_URL`, `GRAFANA_TOKEN`, `GRAFANA_FOLDER_ID` are also supported if the `DIFFDASH_` versions aren't set. ## Output @@ -203,14 +290,45 @@ If any limit is exceeded, the gem aborts with a clear error message and exits wi ## File Filtering -**Included:** -- Files ending with `.rb` -- Ruby application code +By default, diffdash scans Ruby files while excluding test and config directories. + +**Default behavior:** -**Excluded:** -- `*_spec.rb`, `*_test.rb` -- Files in `/spec/`, `/test/`, `/config/` -- Non-Ruby files +| Filter | Default Value | Config Key | +|--------|---------------|------------| +| Included extensions | `.rb` | — | +| Excluded suffixes | `_spec.rb`, `_test.rb` | `excluded_suffixes` | +| Excluded directories | `spec`, `test`, `config` | `excluded_directories` | +| Ignored paths | *(none)* | `ignore_paths` | +| Included paths | *(all)* | `include_paths` | + +**Customizing in `diffdash.yml`:** + +```yaml +# Ignore additional paths +ignore_paths: + - vendor/ + - lib/legacy/ + - tmp/ + +# Only scan specific paths (optional whitelist) +include_paths: + - app/ + - lib/ + +# Add custom excluded suffixes +excluded_suffixes: + - _spec.rb + - _test.rb + - _integration.rb + +# Add custom excluded directories +excluded_directories: + - spec + - test + - config + - features +``` ## Inheritance & Module Support @@ -256,12 +374,23 @@ When `PaymentProcessor` is changed, signals from `BaseProcessor` and `Loggable` ### Setup -1. **Add secrets to your repository:** - - `DIFFDASH_GRAFANA_URL` - Your Grafana instance URL +1. **Create `diffdash.yml`** in your repository root with shared settings: + +```yaml +grafana: + url: https://myorg.grafana.net + folder_id: 42 + +ignore_paths: + - vendor/ + +default_env: staging +``` + +2. **Add the API token as a repository secret:** - `DIFFDASH_GRAFANA_TOKEN` - Service Account token with Editor role - - `DIFFDASH_GRAFANA_FOLDER_ID` (optional) - Folder ID for dashboards -2. **Create workflow file** `.github/workflows/pr-dashboard.yml`: +3. **Create workflow file** `.github/workflows/pr-dashboard.yml`: ```yaml name: PR Observability Dashboard @@ -287,12 +416,12 @@ jobs: - name: Generate dashboard env: - DIFFDASH_GRAFANA_URL: ${{ secrets.DIFFDASH_GRAFANA_URL }} DIFFDASH_GRAFANA_TOKEN: ${{ secrets.DIFFDASH_GRAFANA_TOKEN }} - DIFFDASH_GRAFANA_FOLDER_ID: ${{ secrets.DIFFDASH_GRAFANA_FOLDER_ID }} run: diffdash --verbose ``` +The workflow reads `grafana.url` and `grafana.folder_id` from the committed `diffdash.yml`, while the token is injected securely via environment variable. + ## Development ```bash