feat: add shellcheck to circleci #2835

hirasawayuki · 2021-11-03T03:17:37Z

Add shellcheck to the ci pipeline for testing and security improvements.

Related issue(s)

Checklist

I have read the contributing guidelines.
I have referenced an issue containing the design document if my change
introduces a new feature.
I am following the
contributing code guidelines.
I have read the security policy.
I confirm that this pull request does not address a security
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
security@ory.sh) from the maintainers to push
the changes.
I have added tests that prove my fix is effective or that my feature
works.
I have added or changed the documentation.

Further Comments

I have confirmed that all the parts pointed out by shellcheck have been fixed in my local environment.

$ shellcheck ./**/*.sh

CLAassistant · 2021-11-03T03:17:40Z

All committers have signed the CLA.

…ual redirects

…...`

hirasawayuki · 2021-11-03T07:31:03Z

scripts/run-bench.sh

@@ -94,6 +93,7 @@ hey -n $numReqs -c $numParallel -m POST \
    | tee -a BENCHMARKS.md


+# shellcheck disable=SC2006


I set it to disable because it is a backtick that is written in markdown.
https://github.com/koalaman/shellcheck/wiki/SC2006

hirasawayuki · 2021-11-03T07:37:38Z

install.sh

@@ -368,7 +368,6 @@ EOF
 PROJECT_NAME="hydra"
 OWNER=ory
 REPO="hydra"
-BINARY=hydra


It was an unused variable, so it was removed.
https://github.com/koalaman/shellcheck/wiki/SC2034

hirasawayuki · 2021-11-03T07:38:41Z

scripts/run-bench.sh

@@ -56,7 +56,6 @@ hydra clients create \
    --endpoint http://localhost:9001

 echo "Generating initial access tokens for token introspection benchmark"
-authToken=$(hydra token client --endpoint http://localhost:9000 --client-id $clientId --client-secret $clientSecret)


It was an unused variable, so it was removed.
https://github.com/koalaman/shellcheck/wiki/SC2034

hirasawayuki · 2021-11-03T08:05:24Z

test/conformance/start.sh

@@ -3,4 +3,5 @@
 set -euxo pipefail
 cd "$( dirname "${BASH_SOURCE[0]}" )/../.."

-docker-compose -f quickstart.yml -f quickstart-postgres.yml -f test/conformance/docker-compose.yml up "${1:-}" -d
+# shellcheck disable=SC2086
+docker-compose -f quickstart.yml -f quickstart-postgres.yml -f test/conformance/docker-compose.yml up ${1:-} -d


I used double-quote, but it had unnecessary empty characters, so I disabled SC2086.

↓ this docker-compose -f quickstart.yml -f quickstart-postgres.yml -f test/conformance/docker-compose.yml up '' -d

hirasawayuki · 2021-11-03T10:43:17Z

internal/certification/scripts/configure.sh

@@ -1,6 +1,7 @@
 #!/bin/bash

-source $HOME/.profile
+# shellcheck disable=SC1090,SC1091


I set it to disable because it was a non-existent file when shellcheck is running.
https://github.com/koalaman/shellcheck/wiki/SC1090
https://github.com/koalaman/shellcheck/wiki/SC1091

hirasawayuki · 2021-11-03T10:52:46Z

Shellcheck checks all passed, but the prettier checks don't seem to pass 🤔

Error Log

#!/bin/bash -eo pipefail
cd docs/
npx prettier --check $(jq -r '.config.prettierTarget' package.json)

Checking formatting...
[warn] docs/guides/oauth2-public-spa-mobile.mdx
[warn] Code style issues found in the above file(s). Forgot to run Prettier?

Exited with code exit status 1
CircleCI received exit code 1

Can I ignore the prettier check, since it seems to have failed in other PRs?

aeneasr · 2021-11-04T10:37:16Z

Run make format to format all files :)

codecov · 2021-11-04T12:42:08Z

Codecov Report

Merging #2835 (7ae40aa) into master (f15f339) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2835   +/-   ##
=======================================
  Coverage   49.87%   49.87%           
=======================================
  Files         236      236           
  Lines       14974    14974           
=======================================
  Hits         7469     7469           
  Misses       6843     6843           
  Partials      662      662

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50f9dc8...7ae40aa. Read the comment docs.

hirasawayuki · 2021-11-04T12:45:12Z

Run make format to format all files :)

Thankyou @aeneasr
I ran make format and it passed all the checks!

aeneasr

Great job!

hirasawayuki requested a review from aeneasr as a code owner November 3, 2021 03:17

hirasawayuki force-pushed the add-shellcheck-to-circleci branch from bc900ac to 8417616 Compare November 3, 2021 05:41

feat: add shellcheck to circleci

08bd06d

hirasawayuki force-pushed the add-shellcheck-to-circleci branch from 8417616 to 08bd06d Compare November 3, 2021 05:56

hirasawayuki added 7 commits November 3, 2021 15:22

fix: SC2034 unused valiable

c147839

fix: SC2086 double quote to prevent globbing and word splitting

99c8d4a

fix: ignore SC1091

33dcf88

fix: SC2129 consider using { cmd1; cmd2; } >> file instead of individ…

7d4f3d3

…ual redirects

fix: ignore SC2006 Use $(...) notation instead of legacy backticked `…

1bbb1d3

…...`

fix: SC2164 cd ... || exit in case cd fails

9e657f7

fix: SC3040 in POSIX sh, set option pipefail is undefined

a0cb34d

hirasawayuki commented Nov 3, 2021

View reviewed changes

fix: test/conformance/start.sh ignore SC2086

a27e282

hirasawayuki force-pushed the add-shellcheck-to-circleci branch from 93ba97f to a27e282 Compare November 3, 2021 10:09

hirasawayuki added 2 commits November 3, 2021 19:27

fix: run shellcheck before prettier

9b00598

fix: ignore SC1090 cant follow non-constant source

fc5cb2a

hirasawayuki commented Nov 3, 2021

View reviewed changes

fix: Run make format

7ae40aa

aeneasr approved these changes Nov 9, 2021

View reviewed changes

aeneasr merged commit 38cbcc0 into ory:master Nov 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add shellcheck to circleci #2835

feat: add shellcheck to circleci #2835

hirasawayuki commented Nov 3, 2021 •

edited

Loading

CLAassistant commented Nov 3, 2021 •

edited

Loading

hirasawayuki Nov 3, 2021 •

edited

Loading

hirasawayuki Nov 3, 2021 •

edited

Loading

hirasawayuki Nov 3, 2021

hirasawayuki Nov 3, 2021 •

edited

Loading

hirasawayuki Nov 3, 2021

hirasawayuki commented Nov 3, 2021 •

edited

Loading

aeneasr commented Nov 4, 2021

codecov bot commented Nov 4, 2021 •

edited

Loading

hirasawayuki commented Nov 4, 2021

aeneasr left a comment

		@@ -94,6 +93,7 @@ hey -n $numReqs -c $numParallel -m POST \
		\| tee -a BENCHMARKS.md


		# shellcheck disable=SC2006

feat: add shellcheck to circleci #2835

feat: add shellcheck to circleci #2835

Conversation

hirasawayuki commented Nov 3, 2021 • edited Loading

Related issue(s)

Checklist

Further Comments

CLAassistant commented Nov 3, 2021 • edited Loading

hirasawayuki Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

hirasawayuki Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

hirasawayuki Nov 3, 2021

Choose a reason for hiding this comment

hirasawayuki Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

hirasawayuki Nov 3, 2021

Choose a reason for hiding this comment

hirasawayuki commented Nov 3, 2021 • edited Loading

aeneasr commented Nov 4, 2021

codecov bot commented Nov 4, 2021 • edited Loading

Codecov Report

hirasawayuki commented Nov 4, 2021

aeneasr left a comment

Choose a reason for hiding this comment

hirasawayuki commented Nov 3, 2021 •

edited

Loading

CLAassistant commented Nov 3, 2021 •

edited

Loading

hirasawayuki Nov 3, 2021 •

edited

Loading

hirasawayuki Nov 3, 2021 •

edited

Loading

hirasawayuki Nov 3, 2021 •

edited

Loading

hirasawayuki commented Nov 3, 2021 •

edited

Loading

codecov bot commented Nov 4, 2021 •

edited

Loading