Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify on stale cached records #657

Merged
merged 4 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions docs/commands/status.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. `<name>@<version>`. 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
Expand Down
9 changes: 9 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/licensed/commands/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions lib/licensed/commands/command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def run_command(report)

result = results.all?

yield(result) if block_given?
result = yield(result) if block_given?

result
ensure
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
63 changes: 62 additions & 1 deletion lib/licensed/commands/status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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?
Expand Down Expand Up @@ -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
5 changes: 5 additions & 0 deletions lib/licensed/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand All @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions lib/licensed/reporters/status_reporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
74 changes: 70 additions & 4 deletions test/commands/status_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand All @@ -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
Expand Down Expand Up @@ -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"] = "warn"

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
12 changes: 12 additions & 0 deletions test/reporters/status_reporter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down