Task #905 - Add scan-licenses.yml GA workflow

[anamarijabalaban]

Task: #905

Aim

Add GA workflow that will run license scanning in the CI/CD pipeline for projects that already have configured license_finder gem.

Solution

Added scan-licenses.yml GA workflow that will be triggered when the Gemfile or package.json file has been changed.

First I tried to run license_finder only by using this command: bundle exec license_finder but I got some errors with missing nokogiri so I found this solution that includes adding x86_64-linux platform to the bundler. License finder issue with this solution.

[anamarijabalaban]

Later, I figured out that this would be more useful if a workflow could be run on any project no matter if it has configured license_finder or not, so maybe I could prepare a workflow that will install, configure license_finder gem and run license scanning. What do you think?

[lovro-bikic]

I'd actually like to have this as a script in the project, similar to e.g. bin/lint:

default_rails_template/template.rb

Lines 164 to 178 in ce3d228

	BIN_LINT = <<~HEREDOC.strip_heredoc
	#!/usr/bin/env bash
	set -o errexit
	set -o pipefail
	set -o nounset
	echo "=========== zeitwerk check ==========="
	time bundle exec rails zeitwerk:check
	echo "=========== rubocop ==========="
	time bundle exec rubocop --format simple --format github --color --parallel
	HEREDOC
	create_file 'bin/lint', BIN_LINT, force: true
	chmod 'bin/lint', 0755, verbose: false

That way developers can run the script locally and modify it if needed.

[vr4b4c]

For simple one/two-liners, I find it's more complex when CI workflow is referencing project scripts. GH workflow is coupled with the default_rails_template, and it makes it less portable.

In this case, I'm OK with both approaches, but in general, I'm in favor of workflow being self-sustainable, and project scripts should be used only where necessary.

[lovro-bikic]

How are the developers notified that there are issues with some licenses?

[nikajukic]

Agree, we should setup a Slack notification here.

DevOps used https://github.com/rtCamp/action-slack-notify

[vr4b4c]

I don't think Slack notification is necessary for this step.

[anamarijabalaban]

@nikajukic I tried to use https://github.com/rtCamp/action-slack-notify for sending slack notifications but it looks like they require us to install this https://slack.com/apps/A0F7XDUAZ-incoming-webhooks?tab=settings&next_id=0

[nikajukic]

@uncoverd @vr4b4c please leave your review

[uncoverd]

@lovro-bikic 's suggestions are great, I think once we resolve those we're good to push this 👍

As far as the Slack notifications are concerned, we'd get a failing build notification and a visual notification on the PR. On top of that we have after commit hooks defined in the other PR so I don't think another Slack notification is required

[vr4b4c]

Did we consider running this check as part of the build workflow?

[vr4b4c]

This block looks like it's trying to perform a trick/hack. Why do we need this? Is there a more conventional way of achieving the same result?

[anamarijabalaban]

First, I tried just to call bundle exec license_finder but I got some missing nokogiri error #145 (comment)

[vr4b4c]

Seems like you uncovered a problem with our setup. This is a native extension compilation problem, which originates from our template.rb specifically here.

Nokogiri installation docs contains compelling arguments to explicitly list platforms making bundler install precompiled binaries instead of compiling deps natively.

@lovro-bikic your comment here was the trigger to move from explicitly listing platforms to using ruby. Can you please take a look at this problem and add your thoughts on the topic?

[vr4b4c]

For simple one/two-liners, I find it's more complex when CI workflow is referencing project scripts. GH workflow is coupled with the default_rails_template, and it makes it less portable.

In this case, I'm OK with both approaches, but in general, I'm in favor of workflow being self-sustainable, and project scripts should be used only where necessary.