Skip to content

Commit

Permalink
introduce QA pipeline for protobuf schemas (#385)
Browse files Browse the repository at this point in the history
current protobuf schema files are not perfect.

this PR aims to prevent mistakes in the future, while acknowledging
issues from the past.

fixes #384

----

## status
- [x] introduce protobuf QA tools and configure them to our needs
- [x] baseline existing protobuf QA violations - as acknowledgement
- [x] introduce the protobuf QA tools in automated pipeline
- [x] introduce tools that detect and prevent breaking changes (BCD) in
protobuf
- [x] introduce the protobuf BCD tools in automated pipeline
- [x] have our own protobuf test files checked against the schemas


## followup
- [ ] add the appropriate header to `*.textproto`  
see
#384 (comment)
- [ ] create ticket for BC: fix the proto3 schema enum value `0` -- they
are intended to be fallbacks, not actual values.

---------

Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
  • Loading branch information
jkowalleck authored Mar 2, 2024
1 parent 8a168a4 commit e20f63a
Show file tree
Hide file tree
Showing 11 changed files with 414 additions and 113 deletions.
13 changes: 11 additions & 2 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates

# https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file
version: 2
updates:
- package-ecosystem: 'github-actions'
Expand All @@ -12,3 +11,13 @@ updates:
prefix: 'chore' ## prefix maximum string length of 15
include: 'scope'
open-pull-requests-limit: 999
- package-ecosystem: 'docker'
directory: '/'
schedule:
interval: 'weekly'
day: 'saturday'
labels: [ 'dependencies' ]
commit-message:
prefix: 'chore' ## prefix maximum string length of 15
include: 'scope'
open-pull-requests-limit: 999
28 changes: 28 additions & 0 deletions .github/workflows/test_proto.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# docs: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions

name: CT ProtoBuf

on:
push:
branches: ['master', 'main']
pull_request:
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

defaults:
run:
working-directory: tools/src/test/proto

jobs:
test:
timeout-minutes: 30
runs-on: ubuntu-latest
steps:
- name: Checkout
# see https://github.com/actions/checkout
uses: actions/checkout@v4
- name: Run test
run: ./test.sh
270 changes: 168 additions & 102 deletions schema/bom-1.6.proto

Large diffs are not rendered by default.

6 changes: 0 additions & 6 deletions tools/src/test/proto-test.sh

This file was deleted.

7 changes: 7 additions & 0 deletions tools/src/test/proto/buf_breaking-remote.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This is the config for "Buf" - a ProtocolBuffer linter/checker/more
# see https://buf.build/docs/configuration/v1/buf-yaml
version: v1
breaking: # https://buf.build/docs/configuration/v1/buf-yaml#breaking
use: # see https://buf.build/docs/breaking/overview#rules-and-categories
- FILE
- WIRE_JSON
10 changes: 10 additions & 0 deletions tools/src/test/proto/buf_breaking-version.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# This is the config for "Buf" - a ProtocolBuffer linter/checker/more
# see https://buf.build/docs/configuration/v1/buf-yaml
version: v1
breaking: # https://buf.build/docs/configuration/v1/buf-yaml#breaking
use: # see https://buf.build/docs/breaking/overview#rules-and-categories
- FILE
- WIRE_JSON
except:
# scope is to detect changes from one version to the other -> so ignore "FILE_SAME_PACKAGE"
- FILE_SAME_PACKAGE
22 changes: 22 additions & 0 deletions tools/src/test/proto/buf_lint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# This is the config for "Buf" - a ProtocolBuffer linter/checker/more
# see https://buf.build/docs/configuration/v1/buf-yaml
version: v1
lint: # https://buf.build/docs/configuration/v1/buf-yaml#lint
use: # see https://buf.build/docs/lint/rules
- DEFAULT # https://buf.build/docs/lint/rules#default
except:
# directory/file layout does not match the recommendation/framework of the tool
- DIRECTORY_SAME_PACKAGE # https://buf.build/docs/lint/rules#directory_same_package
- PACKAGE_DIRECTORY_MATCH # https://buf.build/docs/lint/rules#package_lower_snake_case
- FILE_LOWER_SNAKE_CASE # https://buf.build/docs/lint/rules#file_lower_snake_case
# we do not stick to the following best-practices and recommendations:
# (shall be fixed with v2.0 of this very schema)
- PACKAGE_VERSION_SUFFIX # https://buf.build/docs/lint/rules#package_version_suffix
- FIELD_LOWER_SNAKE_CASE # https://buf.build/docs/lint/rules#field_lower_snake_case
ignore_only:
DEFAULT: # https://buf.build/docs/lint/rules#default
# legacy schema files may NOT stick to the rules -- this is acknowledged.
- "schema/bom-1.5.proto"
- "schema/bom-1.4.proto"
- "schema/bom-1.3.proto"
allow_comment_ignores: true # the so-called "baseline" is done by annotating exceptions
165 changes: 165 additions & 0 deletions tools/src/test/proto/test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
#!/usr/bin/env bash
set -ue

THIS_PATH="$(realpath "$(dirname "$0")")"
ROOT_PATH="$(realpath "${THIS_PATH}/../../../..")"

# paths relative to $ROOT_PATH
SCHEMA_DIR='schema'
TEST_RES_DIR='tools/src/test/resources'

REMOTE="https://github.com/${GITHUB_REPOSITORY:-CycloneDX/specification}.git"


## ----


function schema-lint () {
echo '> lint schema files' >&2

if [[ -n "${CI:-}" ]]
then
LOG_FORMAT='github-actions'
else
LOG_FORMAT='text'
fi

docker run --rm \
--volume "${ROOT_PATH}/${SCHEMA_DIR}:/workspace/${SCHEMA_DIR}:ro" \
--volume "${THIS_PATH}/buf_lint.yaml:/workspace/buf.yaml:ro" \
--workdir '/workspace' \
bufbuild/buf:1.29.0 \
lint --path "$SCHEMA_DIR" \
--config 'buf.yaml' \
--error-format "$LOG_FORMAT"

echo '>> OK.' >&2
}


function schema-breaking-version () {
echo '> test schema for breaking changes against previous version' >&2

if [[ -n "${GITHUB_WORKFLOW:-}" ]]
then
LOG_FORMAT='github-actions'
else
LOG_FORMAT='text'
fi

function compare() {
NEW="bom-${1}.proto"
OLD="bom-${2}.proto"
SCHEMA_DIR_OLD="${SCHEMA_DIR}_old"

echo ">> compare new:${NEW} -VS- old:${OLD}" >&2
# stick with the original path of "$NEW", so the reporting makes sense...
docker run --rm \
--volume "${ROOT_PATH}/${SCHEMA_DIR}/${NEW}:/workspace/${SCHEMA_DIR}/${NEW}:ro" \
--volume "${ROOT_PATH}/${SCHEMA_DIR}/${OLD}:/workspace/${SCHEMA_DIR_OLD}/${NEW}:ro" \
--volume "${THIS_PATH}/buf_breaking-version.yaml:/workspace/buf.yaml:ro" \
--workdir '/workspace' \
bufbuild/buf:1.29.0 \
breaking "$SCHEMA_DIR" \
--against "$SCHEMA_DIR_OLD" \
--config 'buf.yaml' \
--error-format "$LOG_FORMAT"
}

compare '1.6' '1.5'
echo '>> skip compare' '1.5' '1.4' >&2 # <-- had breaking changes, which is acknowledged
compare '1.4' '1.3'

echo '>> OK.' >&2
}

function schema-breaking-remote () {
echo '> test schema for breaking changes against remote' >&2

if [[ -n "${GITHUB_WORKFLOW:-}" ]]
then
LOG_FORMAT='github-actions'
else
LOG_FORMAT='text'
fi

docker run --rm \
--volume "${ROOT_PATH}/${SCHEMA_DIR}:/workspace/${SCHEMA_DIR}:ro" \
--volume "${THIS_PATH}/buf_breaking-remote.yaml:/workspace/buf.yaml:ro" \
--workdir '/workspace' \
bufbuild/buf:1.29.0 \
breaking "$SCHEMA_DIR" \
--against "${REMOTE}#subdir=${SCHEMA_DIR}" \
--config 'buf.yaml' \
--error-format "$LOG_FORMAT"

echo '>> OK.' >&2
}

function schema-functional () {
echo '> test all examples against the respective schema' >&2

function validate() {
FILE="$1"
SCHEMA_VERS="$2"
SCHEMA_FILE="bom-${SCHEMA_VERS}.proto"
MESSAGE="cyclonedx.v${SCHEMA_VERS/./_}.Bom"

echo ">> validate $(realpath --relative-to="$PWD" "$FILE") as ${MESSAGE} of ${SCHEMA_FILE}" >&2

docker run --rm \
--volume "${ROOT_PATH}/${SCHEMA_DIR}:/workspace/${SCHEMA_DIR}:ro" \
--volume "${FILE}:/workspace/test_res:ro" \
--workdir '/workspace' \
bufbuild/buf:1.29.0 \
convert "${SCHEMA_DIR}/${SCHEMA_FILE}" \
--type "$MESSAGE" \
--from 'test_res#format=txtpb' \
--to /dev/null
}

shopt -s globstar
for test_res in "$ROOT_PATH"/"$TEST_RES_DIR"/*/valid-*.textproto
do
SCHEMA_VERS="$(basename "$(dirname "$test_res")")"
validate "$test_res" "$SCHEMA_VERS"
done

echo '>> OK.' >&2
}


## ----


case "${1:-all}" in
'schema-lint')
schema-lint
;;
'schema-breaking-version')
schema-breaking-version
;;
'schema-breaking-remote')
schema-breaking-remote
;;
'schema-breaking')
schema-breaking-version
schema-breaking-remote
;;
'schema-functional')
schema-functional
;;
'all')
# all the above
schema-lint
schema-breaking-version
schema-breaking-remote
schema-functional
;;
*)
echo 'unexpected argument. known arguments:' \
'schema-lint,schema-breaking-version,schema-breaking-remote,schema-breaking,schema-functional,all' \
>&2
exit 1
;;
esac
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ services {
url: "http://api.partner.org/swagger"
}
releaseNotes: {
type: RELEASE_TYPE_MAJOR
type: "major"
title: "My new release"
featuredImage: "https://example.com/featured_image.png"
socialImage: "https://example.com/social_image.png"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ services {
url: "http://api.partner.org/swagger"
}
releaseNotes: {
type: RELEASE_TYPE_MAJOR
type: "major"
title: "My new release"
featuredImage: "https://example.com/featured_image.png"
socialImage: "https://example.com/social_image.png"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ services {
url: "http://api.partner.org/swagger"
}
releaseNotes: {
type: RELEASE_TYPE_MAJOR
type: "major"
title: "My new release"
featuredImage: "https://example.com/featured_image.png"
socialImage: "https://example.com/social_image.png"
Expand Down

0 comments on commit e20f63a

Please sign in to comment.