From 9e3a769bcd5a8e5b8d27aab6396658d67be7b449 Mon Sep 17 00:00:00 2001 From: Jon Ruskin Date: Thu, 25 May 2023 15:54:50 -0700 Subject: [PATCH 1/4] capture the result value from command blocks --- lib/licensed/commands/cache.rb | 1 + lib/licensed/commands/command.rb | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/licensed/commands/cache.rb b/lib/licensed/commands/cache.rb index 6dc5fc66..18ed54fa 100644 --- a/lib/licensed/commands/cache.rb +++ b/lib/licensed/commands/cache.rb @@ -23,6 +23,7 @@ def default_reporter(options) def run_command(report) super do |result| clear_stale_cached_records if result + result end ensure cache_paths.clear diff --git a/lib/licensed/commands/command.rb b/lib/licensed/commands/command.rb index ed63872f..0f08f20e 100644 --- a/lib/licensed/commands/command.rb +++ b/lib/licensed/commands/command.rb @@ -69,7 +69,7 @@ def run_command(report) result = results.all? - yield(result) if block_given? + result = yield(result) if block_given? result ensure @@ -103,7 +103,7 @@ def run_app(app, report) result = results.all? - yield(result) if block_given? + result = yield(result) if block_given? result end @@ -142,7 +142,7 @@ def run_source(app, source, report) result = results.all? - yield(result) if block_given? + result = yield(result) if block_given? result rescue Licensed::Shell::Error => err @@ -175,7 +175,7 @@ def run_dependency(app, source, dependency, report) result = evaluate_dependency(app, source, dependency, report) - yield(result) if block_given? + result = yield(result) if block_given? result rescue Licensed::DependencyRecord::Error, Licensed::Shell::Error => err From 38245e410fad1167179688c33d10b73c28444128 Mon Sep 17 00:00:00 2001 From: Jon Ruskin Date: Thu, 25 May 2023 15:55:34 -0700 Subject: [PATCH 2/4] expose root level configuration options --- lib/licensed/configuration.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/licensed/configuration.rb b/lib/licensed/configuration.rb index 5d481631..e8d0ae43 100644 --- a/lib/licensed/configuration.rb +++ b/lib/licensed/configuration.rb @@ -274,6 +274,7 @@ def self.load_from(path) end def initialize(options = {}) + @options = options apps = options.delete("apps") || [] apps << default_options.merge(options) if apps.empty? @@ -285,6 +286,10 @@ def initialize(options = {}) @apps = apps.map { |app| AppConfiguration.new(app, options) } end + def [](key) + @options&.fetch(key, nil) + end + private def self.expand_app_source_path(app_config) From cf3173f00819e103e3e2f2195585c0c3cddd98a3 Mon Sep 17 00:00:00 2001 From: Jon Ruskin Date: Thu, 25 May 2023 17:11:46 -0700 Subject: [PATCH 3/4] notify on stale metadata files in status command --- docs/commands/status.md | 15 +++++ docs/configuration.md | 9 +++ lib/licensed/commands/status.rb | 63 ++++++++++++++++++- lib/licensed/reporters/status_reporter.rb | 5 ++ test/commands/status_test.rb | 74 +++++++++++++++++++++-- test/reporters/status_reporter_test.rb | 12 ++++ 6 files changed, 173 insertions(+), 5 deletions(-) diff --git a/docs/commands/status.md b/docs/commands/status.md index d0264854..6e6b4da3 100644 --- a/docs/commands/status.md +++ b/docs/commands/status.md @@ -31,6 +31,21 @@ A dependency will fail the status checks if: - If `license: other` is specified and all of the `licenses` entries match an `allowed` license a failure will not be logged - A `reviewed` entry must reference a specific version of the depdency, e.g. `@`. The version identifier must specify a specific dependency version, ranges are not allowed. +## Detect and alert on stale cached metadata files + +Licensed can alert on any metadata files that don't correlate to a currently used dependency when `licensed status` is run. To configure this behavior, set a root-level `stale_records_action` value in your [licensed configuration file](./../configuration.md). + +Available values are: + +1. `'error'`: Treat stale cached records as errors. Licensed will output errors for any stale metadata files and will cause `licensed status` to fail. +1. `'warn'`, `''`, or unset (default): Treat stale cached records as warnings. Licensed will output warnings for any stale metadata files but will not cause `licensed status` to fail. +1. `'ignore'`, any other value: Ignore stale cached records. Licensed will not output any notifications about stale metadata files. + +```yaml +# in the licensed configuration file +stale_records_action: 'warn' +``` + ## Options - `--config`/`-c`: the path to the licensed configuration file diff --git a/docs/configuration.md b/docs/configuration.md index 9204d64f..ba9f0e7c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -26,6 +26,15 @@ cache_path: 'relative/path/to/cache' # Defaults to current directory when running `licensed` source_path: 'relative/path/to/source' +# Whether to take any action when records are detected in the cache paths that don't map to evaluated +# dependencies. +# Available values are: +# - 'error': treat stale cached records as errors. Notify the user and fail status checks +# - 'warn', '', unset: treat stale cached records as warnings. Notify the user but do not fail status checks +# - 'ignore': Ignore stale cached records. Do not notify the user and do not fail status checks +# Optional, when not set this defaults to 'warn' behavior +stale_records_action: 'warn' + # Sources of metadata sources: bower: true diff --git a/lib/licensed/commands/status.rb b/lib/licensed/commands/status.rb index 2a2cc99b..79297177 100644 --- a/lib/licensed/commands/status.rb +++ b/lib/licensed/commands/status.rb @@ -23,10 +23,48 @@ def default_reporter(options) # Returns whether the command succeeded based on the call to super def run_command(report) super do |result| - next if result + stale_records = stale_cached_records + if stale_records.any? + messages = stale_records.map { |f| "Stale dependency record found: #{f}" } + messages << "Please run the licensed cache command to clean up stale records" + + case config["stale_records_action"].to_s + when "error" + report.errors.concat messages + result = false + when "warn", "" + report.warnings.concat messages + end + end + + next result if result report.errors << "Licensed found errors during source enumeration. Please see https://github.com/github/licensed/tree/master/docs/commands/status.md#status-errors-and-resolutions for possible resolutions." + + result end + ensure + cache_paths.clear + files.clear + end + + # Run the command for all enumerated dependencies found in a dependency source, + # recording results in a report. + # Enumerating dependencies in the source is skipped if a :sources option + # is provided and the evaluated `source.class.type` is not in the :sources values + # + # app - The application configuration for the source + # source - A dependency source enumerator + # + # Returns whether the command succeeded for the dependency source enumerator + def run_source(app, source, report) + result = super + + # add the full cache path to the list of cache paths + # that should be checked for extra files after the command run + cache_paths << app.cache_path.join(source.class.type) unless result == :skipped + + result end # Evaluates a dependency for any compliance errors. @@ -49,6 +87,9 @@ def evaluate_dependency(app, source, dependency, report) filename = app.cache_path.join(source.class.type, "#{dependency.name}.#{DependencyRecord::EXTENSION}") report["filename"] = filename record = cached_record(filename) + + # add the absolute dependency file path to the list of files seen during this licensed run + files << filename.to_s end if record.nil? @@ -133,6 +174,26 @@ def license_from_text(text) licenses.sort_by { |license| license != "other" ? 0 : 1 }.first end + + # Check for cached files that don't match current dependencies + # + # Returns an array of any cached records that do not match a currently used dependency + def stale_cached_records + cache_paths.flat_map do |cache_path| + record_search_glob_pattern = cache_path.join("**/*.#{DependencyRecord::EXTENSION}") + Dir.glob(record_search_glob_pattern).select { |file| !files.include?(file) } + end.uniq + end + + # Set of unique cache paths that are evaluted during the run + def cache_paths + @cache_paths ||= Set.new + end + + # Set of unique absolute file paths of cached records evaluted during the run + def files + @files ||= Set.new + end end end end diff --git a/lib/licensed/reporters/status_reporter.rb b/lib/licensed/reporters/status_reporter.rb index f5594556..5d922ea9 100644 --- a/lib/licensed/reporters/status_reporter.rb +++ b/lib/licensed/reporters/status_reporter.rb @@ -8,6 +8,11 @@ class StatusReporter < Reporter # command - The command being run # report - A report object containing information about the command run def end_report_command(command, report) + if report.warnings.any? + shell.newline + report.warnings.each { |e| shell.warn e } + end + if report.errors.any? shell.newline report.errors.each { |e| shell.error e } diff --git a/test/commands/status_test.rb b/test/commands/status_test.rb index 051e0ad6..d31a1498 100644 --- a/test/commands/status_test.rb +++ b/test/commands/status_test.rb @@ -9,7 +9,8 @@ let(:reporter) { TestReporter.new } let(:apps) { [] } let(:source_config) { {} } - let(:config) { Licensed::Configuration.new("apps" => apps, "cache_path" => cache_path, "sources" => { "test" => true }, "test" => source_config) } + let(:command_config) { { "apps" => apps, "cache_path" => cache_path, "sources" => { "test" => true }, "test" => source_config } } + let(:config) { Licensed::Configuration.new(command_config) } let(:fixtures) { File.expand_path("../../fixtures", __FILE__) } let(:command) { Licensed::Commands::Status.new(config: config) } @@ -24,11 +25,15 @@ def dependency_errors(app, source, dependency_name = "dependency") dependency_report&.errors || [] end + def generate_metadata_files + generator_config = Marshal.load(Marshal.dump(config)) + generator = Licensed::Commands::Cache.new(config: generator_config) + generator.run(force: true, reporter: TestReporter.new) + end + describe "with cached metadata data source" do before do - generator_config = Marshal.load(Marshal.dump(config)) - generator = Licensed::Commands::Cache.new(config: generator_config) - generator.run(force: true, reporter: TestReporter.new) + generate_metadata_files end after do @@ -776,4 +781,65 @@ def run_command(**opts) end end end + + describe "with stale cached records" do + let(:unused_record_file_path) do + app = config.apps.first + source = app.sources.first + File.join(app.cache_path, source.class.type, "unused.#{Licensed::DependencyRecord::EXTENSION}") + end + + before do + # generate artifacts needed for the status command to normally pass + # in order to validate that the command passes or fails depending on + # the stale_records_action config setting + generate_metadata_files + config.apps.each do |app| + app.allow "mit" + end + + FileUtils.mkdir_p File.dirname(unused_record_file_path) + File.write(unused_record_file_path, "") + end + + after do + config.apps.each do |app| + FileUtils.rm_rf app.cache_path + end + end + + it "reports an error on stale cached records when configured" do + command_config["stale_records_action"] = "error" + + refute run_command + + assert reporter.report.errors.include?("Stale dependency record found: #{unused_record_file_path}") + refute reporter.report.warnings.include?("Stale dependency record found: #{unused_record_file_path}") + end + + it "reports a warning on stale cached records when unconfigured" do + assert run_command + + refute reporter.report.errors.include?("Stale dependency record found: #{unused_record_file_path}") + assert reporter.report.warnings.include?("Stale dependency record found: #{unused_record_file_path}") + end + + it "reports a warning on stale cached records when configured" do + command_config["stale_records_action"] = "warning" + + assert run_command + + refute reporter.report.errors.include?("Stale dependency record found: #{unused_record_file_path}") + assert reporter.report.warnings.include?("Stale dependency record found: #{unused_record_file_path}") + end + + it "ignores stale cached records when configured" do + command_config["stale_records_action"] = "ignore" + + assert run_command + + refute reporter.report.errors.include?("Stale dependency record found: #{unused_record_file_path}") + refute reporter.report.warnings.include?("Stale dependency record found: #{unused_record_file_path}") + end + end end diff --git a/test/reporters/status_reporter_test.rb b/test/reporters/status_reporter_test.rb index 91fa1ae8..c69a9623 100644 --- a/test/reporters/status_reporter_test.rb +++ b/test/reporters/status_reporter_test.rb @@ -24,6 +24,18 @@ style: :error } end + + it "reports any warnings specified on the report object" do + report.warnings << "command warning" + reporter.end_report_command(command, report) + + assert_includes shell.messages, + { + message: "command warning", + newline: true, + style: :warn + } + end end describe "#begin_report_app" do From 80d1f459c731c17c471b28917e13db98c671bcd0 Mon Sep 17 00:00:00 2001 From: Jon Ruskin Date: Thu, 25 May 2023 17:26:58 -0700 Subject: [PATCH 4/4] update test for renamed value this value was originally "warning" but updated to "warn" while writing documentation I missed updating the test before committing changes --- test/commands/status_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/commands/status_test.rb b/test/commands/status_test.rb index d31a1498..a81d34d5 100644 --- a/test/commands/status_test.rb +++ b/test/commands/status_test.rb @@ -825,7 +825,7 @@ def run_command(**opts) end it "reports a warning on stale cached records when configured" do - command_config["stale_records_action"] = "warning" + command_config["stale_records_action"] = "warn" assert run_command