Change documentation CI to push to new S3-based cloudfront infra instead of the legacy docs infrastructure#100
Conversation
…oc.powerdns.com using the new Cloudfront-based system
|
Currently waiting for the harbor password before this will pass tests. |
Luit
left a comment
Luit
left a comment
There was a problem hiding this comment.
I think the bash scripts could use a full ShellCheck, but I'll leave it up to you whether to do that right now or later.
|
|
||
| docker run -v "${PWD}:${PWD}" $image sh -c "pip install mkdocs-swagger-ui-tag && mkdocs build -f $mkdocs_file -d ${PWD}/output/${version}" | ||
|
|
||
| latestVersion=`aws s3 ls s3://"${AWS_S3_BUCKET_DOCS}"/docs.powerdns.com/$subdir/ | awk '{print $2}' | grep -v latest | awk -F '/' '/\// {print $1}' | sort -V | tail -1` |
There was a problem hiding this comment.
I know this all comes verbatim from a different repo, but this line I didn't even fire up ShellCheck for to know it'll complain. Please use $(...) instead of the legacy backtick style command substitution.
| @@ -3,60 +3,47 @@ name: 'Documentation' | |||
| on: | |||
| push: | |||
| # docs is the branch on which we are developing this, can be removed later | |||
| # - AWS_SECRET_ACCESS_KEY: The AWS secret access key | ||
| # - AWS_REGION: The AWS region where resources are located | ||
| # - AWS_S3_BUCKET_DOCS: The name of the S3 bucket for documentation | ||
| # - BUILD_PATH: The root of the lightningstream directory |
There was a problem hiding this comment.
I often prefer something like the following for finding my script dir, which seems to be the only reason for this required var:
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
There was a problem hiding this comment.
This var disappears in the GH Action version anyway
| while read -r docsVersion; do | ||
| if [ "$docsVersion" != "" ] && [ "$docsVersion" != "latest" ]; then | ||
| if [ $docsVersion == $latestVersion ]; then | ||
| versionsData=$(echo $versionsData | jq ". += [{\"title\": \"${docsVersion}\", \"version\": \"${latestVersion}\", \"aliases\": [\"latest\"]}]") | ||
| else | ||
| versionsData=$(echo $versionsData | jq ". += [{\"title\": \"${docsVersion}\", \"version\": \"${docsVersion}\", \"aliases\": []}]") | ||
| fi | ||
| fi | ||
| done < <(aws s3 ls s3://"${AWS_S3_BUCKET_DOCS}"/docs.powerdns.com/$subdir/ | awk '{print $2}' | awk -F '/' '/\// {print $1}') |
There was a problem hiding this comment.
This seems a bit convoluted when the aws s3 ls command has an --output json option.
There was a problem hiding this comment.
The --output flag is ignored for the ls command. From the help:
Note that the --output and --no-paginate arguments are ignored for this command.
There was a problem hiding this comment.
Strange, there's no mention of that on https://docs.aws.amazon.com/cli/latest/reference/s3/ls.html
There's the option to use aws s3api though, I found when looking up this issue...
There was a problem hiding this comment.
Oh my, it is there, but Ctrl+F "--output" won't find it, because the -- is turned into a – by their docs 🤦🏻
|
I think all the review comments are valid, however I'm going to apply them in the new repo for the GH Action, and then refactor this PR to just use that. |
2 participants
No description provided.