Skip to content

Commit

Permalink
Add more YAML verification
Browse files Browse the repository at this point in the history
* Validate YAML files using actionlint
* Check that tests in packages/datadog-instrumentations/tests/ are
  actually run.
* Ensure all tests have the same triggers.
  • Loading branch information
bengl committed Jan 23, 2025
1 parent c28765a commit 75b9f4b
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 16 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/all-green.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ on:
push:
branches:
- master
schedule:
- cron: "0 4 * * *"

jobs:

Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/datadog-static-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ on:
pull_request:
push:
branches: [master]
schedule:
- cron: "0 4 * * *"

jobs:
static-analysis:
Expand Down
61 changes: 61 additions & 0 deletions .github/workflows/instrumentations.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
name: Instrumentations

on:
pull_request:
push:
branches: [master]
schedule:
- cron: '0 4 * * *'

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

# TODO: upstream jobs


jobs:

# These ones don't have a plugin directory, but exist in the
# instrumentations directory, so they need to be run somewhere. This seems to
# be a reasonable place to run them for now.

check_require_cache:
runs-on: ubuntu-latest
env:
PLUGINS: check_require_cache
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/plugins/test

fs:
runs-on: ubuntu-latest
env:
PLUGINS: fs
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/plugins/test

multer:
runs-on: ubuntu-latest
env:
PLUGINS: multer
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/plugins/test

passport-http:
runs-on: ubuntu-latest
env:
PLUGINS: passport-http
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/plugins/test

passport-local:
runs-on: ubuntu-latest
env:
PLUGINS: passport-local
steps:
- uses: actions/checkout@v4
- uses: ./.github/actions/plugins/test
19 changes: 19 additions & 0 deletions .github/workflows/project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,22 @@ jobs:
- uses: ./.github/actions/node/setup
- uses: ./.github/actions/install
- run: node scripts/verify-ci-config.js

- name: actionlint
id: actionlint #optional, id required only when outputs are used in the workflow steps later
uses: raven-actions/actionlint@v2
with:
matcher: true
fail-on-error: true

- name: actionlint Summary
if: ${{ steps.actionlint.outputs.exit-code != 0 }}
run: |
echo "Used actionlint version ${{ steps.actionlint.outputs.version-semver }}"
echo "Used actionlint release ${{ steps.actionlint.outputs.version-tag }}"
echo "actionlint ended with ${{ steps.actionlint.outputs.exit-code }} exit code"
echo "actionlint ended because '${{ steps.actionlint.outputs.exit-message }}'"
echo "actionlint found ${{ steps.actionlint.outputs.total-errors }} errors"
echo "actionlint checked ${{ steps.actionlint.outputs.total-files }} files"
echo "actionlint cache used: ${{ steps.actionlint.outputs.cache-hit }}"
exit ${{ steps.actionlint.outputs.exit-code }}
2 changes: 2 additions & 0 deletions .github/workflows/serverless-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ on:
pull_request:
push:
branches: [master]
schedule:
- cron: "0 4 * * *"

jobs:
integration:
Expand Down
4 changes: 1 addition & 3 deletions .github/workflows/system-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ name: System Tests

on:
pull_request:
branches:
- "**"
push:
branches: [master]
workflow_dispatch: {}
schedule:
- cron: '00 04 * * 2-6'
- cron: "0 4 * * *"

jobs:
build-artifacts:
Expand Down
81 changes: 68 additions & 13 deletions scripts/verify-ci-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@ const { execSync } = require('child_process')
const Module = require('module')
const { getAllInstrumentations } = require('../packages/dd-trace/test/setup/helpers/load-inst')

function errorMsg (title, ...message) {
console.log('===========================================')
console.log(title)
console.log('-------------------------------------------')
console.log(...message)
console.log('\n')
process.exitCode = 1
}

/// /
/// / Verifying plugins.yml and appsec.yml that plugins are consistently tested
/// /

if (!Module.isBuiltin) {
Module.isBuiltin = mod => Module.builtinModules.includes(mod)
}
Expand All @@ -20,7 +33,9 @@ const instrumentations = getAllInstrumentations()

const versions = {}

function checkYaml (yamlPath) {
const allTestedPlugins = new Set()

function checkPlugins (yamlPath) {
const yamlContent = yaml.parse(fs.readFileSync(yamlPath, 'utf8'))

const rangesPerPluginFromYaml = {}
Expand All @@ -30,6 +45,9 @@ function checkYaml (yamlPath) {
if (!job.env || !job.env.PLUGINS) continue

const pluginName = job.env.PLUGINS
if (!yamlPath.includes('appsec')) {
pluginName.split('|').forEach(plugin => allTestedPlugins.add(plugin))
}
if (Module.isBuiltin(pluginName)) continue
const rangesFromYaml = getRangesFromYaml(job)
if (rangesFromYaml) {
Expand All @@ -42,6 +60,7 @@ function checkYaml (yamlPath) {
rangesPerPluginFromInst[pluginName] = allRangesForPlugin
}
}

for (const pluginName in rangesPerPluginFromYaml) {
const yamlRanges = Array.from(rangesPerPluginFromYaml[pluginName])
const instRanges = Array.from(rangesPerPluginFromInst[pluginName])
Expand All @@ -50,7 +69,7 @@ function checkYaml (yamlPath) {
if (!util.isDeepStrictEqual(yamlVersions, instVersions)) {
const opts = { colors: true }
const colors = x => util.inspect(x, opts)
errorMsg(pluginName, 'Mismatch', `
pluginErrorMsg(pluginName, 'Mismatch', `
Valid version ranges from YAML: ${colors(yamlRanges)}
Valid version ranges from INST: ${colors(instRanges)}
${mismatching(yamlVersions, instVersions)}
Expand All @@ -67,7 +86,7 @@ Note that versions may be dependent on Node.js version. This is Node.js v${color
function getRangesFromYaml (job) {
// eslint-disable-next-line no-template-curly-in-string
if (job.env && job.env.PACKAGE_VERSION_RANGE && job.env.PACKAGE_VERSION_RANGE !== '${{ matrix.range }}') {
errorMsg(job.env.PLUGINS, 'ERROR in YAML', 'You must use matrix.range instead of env.PACKAGE_VERSION_RANGE')
pluginErrorMsg(job.env.PLUGINS, 'ERROR in YAML', 'You must use matrix.range instead of env.PACKAGE_VERSION_RANGE')
process.exitCode = 1
}
if (job.strategy && job.strategy.matrix && job.strategy.matrix.range) {
Expand All @@ -94,9 +113,6 @@ function getMatchingVersions (name, ranges) {
return versions[name].filter(version => ranges.some(range => semver.satisfies(version, range)))
}

checkYaml(path.join(__dirname, '..', '.github', 'workflows', 'plugins.yml'))
checkYaml(path.join(__dirname, '..', '.github', 'workflows', 'appsec.yml'))

function mismatching (yamlVersions, instVersions) {
const yamlSet = new Set(yamlVersions)
const instSet = new Set(instVersions)
Expand All @@ -111,11 +127,50 @@ function mismatching (yamlVersions, instVersions) {
].join('\n')
}

function errorMsg (pluginName, title, message) {
console.log('===========================================')
console.log(title + ' for ' + pluginName)
console.log('-------------------------------------------')
console.log(message)
console.log('\n')
process.exitCode = 1
function pluginErrorMsg (pluginName, title, message) {
errorMsg(title + ' for ' + pluginName, message)
}

checkPlugins(path.join(__dirname, '..', '.github', 'workflows', 'plugins.yml'))
checkPlugins(path.join(__dirname, '..', '.github', 'workflows', 'instrumentations.yml'))
checkPlugins(path.join(__dirname, '..', '.github', 'workflows', 'appsec.yml'))
{
const testDir = path.join(__dirname, '..', 'packages', 'datadog-instrumentations', 'test')
const testedInstrumentations = fs.readdirSync(testDir)
.filter(file => file.endsWith('.spec.js'))
.map(file => file.replace('.spec.js', ''))
for (const instrumentation of testedInstrumentations) {
if (!allTestedPlugins.has(instrumentation)) {
pluginErrorMsg(instrumentation, 'ERROR', 'Instrumentation is tested but not in plugins.yml')
}
}
}

/// /
/// / Verifying that tests run on correct triggers
/// /

const workflows = fs.readdirSync(path.join(__dirname, '..', '.github', 'workflows'))
.filter(file =>
!['release', 'codeql', 'pr-labels']
.reduce((contained, name) => contained || file.includes(name), false)
)

function triggersError (workflow, ...text) {
errorMsg('ERROR in ' + workflow, ...text)
}

for (const workflow of workflows) {
const yamlPath = path.join(__dirname, '..', '.github', 'workflows', workflow)
const yamlContent = yaml.parse(fs.readFileSync(yamlPath, 'utf8'))
const triggers = yamlContent.on
if (triggers?.pull_request !== null) {
triggersError(workflow, 'The `pull_request` trigger should be blank')
}
if (workflow !== 'package-size.yml' && triggers?.push?.branches?.[0] !== 'master') {
triggersError(workflow, 'The `push` trigger should run on master')
}
if (triggers?.schedule?.[0]?.cron !== '0 4 * * *') {
triggersError(workflow, 'The `cron` trigger should be \'0 4 * * *\'')
}
}

0 comments on commit 75b9f4b

Please sign in to comment.