Validate/test all default configs #1268

MisterTimn · 2022-04-19T12:34:12Z

📁 Related issues

✍️ Description

A bash script has been added /test/deploy/validate-configs.sh
This script will test run CSS with all the different default configs by looping over config/*.json
For https (example-https-file.json and https-cli-file.json) certs are set (through cli params or config file overwrites) and base URL is changed.

This script is run in the test-configs job. The job has a tenforce/virtuoso service for sparql config validation and also generates a self-signed cert/key for https. All configs get tested, in case of failure the logs from CSS get printed and the script moves on to the next config. At the end an overview printout is provided:

If one of the tests has failed, the overall job will fail.

Improvements

Conditional command changes

Additional exceptions like in the case of https can be set up but this might become cumbersome in the future as configs get added or changed. Perhaps we might move to a solution where default configs are accompanied with an entry in a separate validation config file that provides a string to append to the community-solid-server command. With the addition #1264 and dynamic CLI parameters, we can also enforce all future configs to have parameters that allow it to be run without editing config files (like in the case of example-https-file.json).

Validation / deployment testing / config testing?

Right now, the job name is test-configs and the script is called validate-configs, I'm not sure how we best name this step and where we position it in the pipeline. It's more a form of deployment testing with different configs than a true test...

✅ PR check list

Before this pull request can be merged, a core maintainer will check whether

this PR is labeled with the correct semver label
- semver.patch: Backwards compatible bug fixes.
- semver.minor: Backwards compatible feature additions.
- semver.major: Breaking changes. This includes changing interfaces or configuration behaviour.
the correct branch is targeted. Patch updates can target main, other changes should target the latest versions/* branch.
the RELEASE_NOTES.md document in case of relevant feature or config changes.
any relevant documentation was updated to reflect the changes in this PR.

RubenVerborgh · 2022-04-19T19:01:32Z

Great!

Is this the right target branch though? I see a couple of unrelated commits.

MisterTimn · 2022-04-20T07:00:38Z

Is this the right target branch though? I see a couple of unrelated commits.

Oh, yea I rebased off the version/4.0.0 branch before opening the PR but then targeted main. I think this best targets main since its unrelated to the CSS code itself, maybe after the 4.0.0 release this can be cleaned up.

EDIT: fixed with rebase off main after release

joachimvh · 2022-04-20T09:37:01Z

.github/workflows/ci.yml

+          node-version: '16.x'
+      - name: Check out repository
+        uses: actions/checkout@v3
+      - name: Generate certificates https config


Suggested change

- name: Generate certificates https config

- name: Generate certificates for HTTPS configurations

test/deploy/validate-configs.sh

joachimvh · 2022-04-20T09:46:44Z

test/deploy/validate-configs.sh

+  if [[ $CONFIG_NAME =~ "https" ]]; then
+    sed -i -E "s/(\W+\"options_key\".*\").+(\".*)/\1server.key\2/" $CONFIG_PATH
+    sed -i -E "s/(\W+\"options_cert\".*\").+(\".*)/\1server.cert\2/" $CONFIG_PATH
+    CSS_OPTS="$CSS_OPTS -b https://localhost:8888 --httpsKey server.key --httpsCert server.cert"
+    CURL_STATEMENT='-k https://localhost:8888'
+  fi


I would have 2 separate if-statements for the 2 different HTTPS configs so only one of them gets the CLI params, and only one of them gets the changed config. You can also changes the paths in example-https-file.json should that make testing easier.

joachimvh · 2022-04-20T09:47:07Z

test/deploy/validate-configs.sh

+echo "----------------------------------------"
+echo -e $SUCCESSES
+echo -e $FAILURES
+exit $VALIDATION_FAILURE


Missing EOL.

test/deploy/validate-configs.sh

RubenVerborgh · 2022-04-20T09:51:40Z

test/deploy/validate-configs.sh

+  FAILURE=1
+  if [ -z $PID ]; then
+    echo "$TEST_NAME($CONFIG_NAME) - FAILURE: Server did not start"
+    cat logs/$CONFIG_NAME
+  else
+    echo "$TEST_NAME($CONFIG_NAME) - Attempting HTTP access to the server"
+    for i in {1..20}; do
+      sleep 1
+      if curl -s -f $CURL_STATEMENT > logs/$CONFIG_NAME-curl; then
+        echo "$TEST_NAME($CONFIG_NAME) - SUCCESS: server reached (after ~${i}s)"
+        FAILURE=0
+        break
+      fi
+    done


Since writing this I found https://stackoverflow.com/a/71522504/476820

RubenVerborgh · 2022-04-20T09:52:58Z

test/deploy/validate-configs.sh

+echo -e $SUCCESSES
+echo -e $FAILURES


Brutal honesty there.

MisterTimn · 2022-04-22T12:10:09Z

The script can now be handed one or multiple arguments, either full path, name, name.json or config/name (see example below).

$ npm run test:deploy default file.json config/example-https-file config/https-file-cli.json

> @solid/community-server@4.0.0 test:deploy
> test/deploy/validate-configs.sh "default" "file.json" "config/example-https-file" "config/https-file-cli.json"

Deployment testing 4 configs
 - config/default.json
 - config/file.json
 - config/example-https-file.json
 - config/https-file-cli.json

No arguments will start test for all configs.

$ npm run test:deploy

> @solid/community-server@4.0.0 test:deploy
> test/deploy/validate-configs.sh

Deployment testing all configs!
 - config/default.json
 - config/dynamic.json
 - config/example-https-file.json
 - config/file-no-setup.json
 - config/file.json
 - config/https-file-cli.json
 - config/memory-subdomains.json
 - config/path-routing.json
 - config/quota-file.json
 - config/restrict-idp.json
 - config/sparql-endpoint-no-setup.json
 - config/sparql-endpoint.json
 - config/sparql-file-storage.json

Added -f argument to css run with a folder for automated cleanup of all files after tests, so it can be used locally.

! When you run locally, sparql configs will fail unless you have an endpoint on https://localhost:4000/sparql

Certificate generation was moved inside the script (instead of from CI run) to allow local deploy:test of https (now it will get generated twice in case all configs are run, don't think this is an issue but might be better to generate at start script (not in the https conditional)

The example-https-file gets copied and placed back so original values are restored. Alternatively, we just change the paths in the file like @joachimvh suggested, but maybe best to keep it in a dir still (to indicate clearly it needs a path). Something like ssl/server.key, then either create/remove that folder everytime or leave it.

Perhaps I should look into writing some small Developer docs on what to do/check when new default configs get added?

MisterTimn · 2022-04-22T12:27:00Z

Im seeing the unit tests on ubuntu fail because they are running deploy:test as well. Remove this step from all CI unit tests as we can depend on the separate deploy-configs job for this?

RubenVerborgh · 2022-04-24T16:14:54Z

Im seeing the unit tests on ubuntu fail because they are running deploy:test as well. Remove this step from all CI unit tests as we can depend on the separate deploy-configs job for this?

Absolutely, yes!

RubenVerborgh

Super cool stuff!

.github/workflows/ci.yml

RubenVerborgh · 2022-04-24T16:18:21Z

test/deploy/validate-configs.sh

+
+TEST_NAME="Deployment testing"
+declare -a CONFIG_ARRAY
+if [[ $# -gt 0 ]]; then


Some comments perhaps regarding expected arguments?

RubenVerborgh · 2022-04-24T16:18:54Z

test/deploy/validate-configs.sh

+# Script to validate the packaged module and configs
+
+# Ensure our workdir is that of the project root
+cd "${0%/*}/../.." || exit 


Exit with warning / error code?

test/deploy/validate-configs.sh

MisterTimn · 2022-04-25T11:42:01Z

Requested changes are implemented

All files needed are generated/cleaned from test/tmp folder which is already in gitignore
test:deploy only called in its separate job and needs updated
fixed an issue with parameter expansion
Now only takes full config path as optional parameter (relative from project directory or absolute)

RubenVerborgh · 2022-04-25T12:24:48Z

So nice to see this pass. Great work! @joachimvh Let's make test-configs mandatory in GitHub after this is merged.

RubenVerborgh · 2022-04-25T12:26:30Z

@joachimvh Do we have graceful shutdown instead of kill? Like a HUP? Or is this the safe way?

joachimvh · 2022-04-25T13:12:24Z

@joachimvh Do we have graceful shutdown instead of kill? Like a HUP? Or is this the safe way?

We don't. The only way to gracefully shutdown is if you start the server through code.

MisterTimn added the semver.patch Does not require a minor or major version bump label Apr 19, 2022

MisterTimn changed the title ~~Chore/test deploy configs~~ Validate/test all default configs Apr 19, 2022

MisterTimn added 6 commits April 20, 2022 09:17

chore: add base script to test-run all configs

4ea079b

chore: job for test-deploy

4fd999e

chore: fine-tuning config validation

961269b

chore: config validation fully functional

3c5f588

chore: fix https-file-cli validation (missing var)

45e2067

fix: generate self-signed CA through openSSL

a9bda29

MisterTimn force-pushed the chore/test-deploy-configs branch from bb074da to a9bda29 Compare April 20, 2022 07:17

joachimvh reviewed Apr 20, 2022

View reviewed changes

RubenVerborgh approved these changes Apr 20, 2022

View reviewed changes

MisterTimn added 2 commits April 20, 2022 17:40

chore: streamlining test script + review

ee73598

chore: validate-configs accepts config args

6c655e9

MisterTimn requested a review from RubenVerborgh April 22, 2022 12:24

chore: cleanup and best-practices

afb8968

RubenVerborgh approved these changes Apr 24, 2022

View reviewed changes

joachimvh reviewed Apr 25, 2022

View reviewed changes

test/deploy/validate-configs.sh Outdated Show resolved Hide resolved

test/deploy/validate-configs.sh Outdated Show resolved Hide resolved

test/deploy/validate-configs.sh Outdated Show resolved Hide resolved

test/deploy/validate-configs.sh Outdated Show resolved Hide resolved

MisterTimn added 4 commits April 25, 2022 10:52

chore: test-configs as precond + needs consistency

39732ac

chore: changes after review

25ceb35

chore: fix argument variable expansion

4eb81bf

chore: more tweaks to script

1358acc

joachimvh approved these changes Apr 25, 2022

View reviewed changes

joachimvh merged commit 6dd77cf into main Apr 25, 2022

joachimvh deleted the chore/test-deploy-configs branch April 25, 2022 13:44

joachimvh mentioned this pull request Apr 25, 2022

Validate all of our configs and deploy them #354

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate/test all default configs #1268

Validate/test all default configs #1268

MisterTimn commented Apr 19, 2022

RubenVerborgh commented Apr 19, 2022

MisterTimn commented Apr 20, 2022 •

edited

Loading

joachimvh Apr 20, 2022

joachimvh Apr 20, 2022

joachimvh Apr 20, 2022

RubenVerborgh Apr 20, 2022

RubenVerborgh Apr 20, 2022

MisterTimn commented Apr 22, 2022

MisterTimn commented Apr 22, 2022

RubenVerborgh commented Apr 24, 2022

RubenVerborgh left a comment

RubenVerborgh Apr 24, 2022

RubenVerborgh Apr 24, 2022

MisterTimn commented Apr 25, 2022 •

edited

Loading

RubenVerborgh commented Apr 25, 2022

RubenVerborgh commented Apr 25, 2022

joachimvh commented Apr 25, 2022

	- name: Generate certificates https config
	- name: Generate certificates for HTTPS configurations

Validate/test all default configs #1268

Validate/test all default configs #1268

Conversation

MisterTimn commented Apr 19, 2022

📁 Related issues

✍️ Description

Improvements

Conditional command changes

Validation / deployment testing / config testing?

✅ PR check list

RubenVerborgh commented Apr 19, 2022

MisterTimn commented Apr 20, 2022 • edited Loading

joachimvh Apr 20, 2022

Choose a reason for hiding this comment

joachimvh Apr 20, 2022

Choose a reason for hiding this comment

joachimvh Apr 20, 2022

Choose a reason for hiding this comment

RubenVerborgh Apr 20, 2022

Choose a reason for hiding this comment

RubenVerborgh Apr 20, 2022

Choose a reason for hiding this comment

MisterTimn commented Apr 22, 2022

MisterTimn commented Apr 22, 2022

RubenVerborgh commented Apr 24, 2022

RubenVerborgh left a comment

Choose a reason for hiding this comment

RubenVerborgh Apr 24, 2022

Choose a reason for hiding this comment

RubenVerborgh Apr 24, 2022

Choose a reason for hiding this comment

MisterTimn commented Apr 25, 2022 • edited Loading

RubenVerborgh commented Apr 25, 2022

RubenVerborgh commented Apr 25, 2022

joachimvh commented Apr 25, 2022

MisterTimn commented Apr 20, 2022 •

edited

Loading

MisterTimn commented Apr 25, 2022 •

edited

Loading