diff --git a/.github/workflows/TESTS/internal_on_merge_tag_versions.unit.test.js b/.github/workflows/TESTS/internal_on_merge_tag_versions.unit.test.js new file mode 100644 index 0000000..4aa363a --- /dev/null +++ b/.github/workflows/TESTS/internal_on_merge_tag_versions.unit.test.js @@ -0,0 +1,129 @@ +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +const { extractRunScript, readWorkflow, runScript } = require('./workflow-test-helpers'); + +const workflowFile = 'internal_on_merge_tag_versions.yml'; + +const gitEnv = { + GIT_CONFIG_GLOBAL: '/dev/null', + GIT_CONFIG_NOSYSTEM: '1', + GIT_AUTHOR_NAME: 'Test', + GIT_AUTHOR_EMAIL: 'test@example.com', + GIT_COMMITTER_NAME: 'Test', + GIT_COMMITTER_EMAIL: 'test@example.com', +}; + +function git(cwd, args) { + const result = spawnSync('git', args, { + cwd, + encoding: 'utf8', + env: { ...process.env, ...gitEnv }, + }); + if (result.status !== 0) { + throw new Error(`git ${args.join(' ')} failed: ${result.stderr}`); + } + return result.stdout.trim(); +} + +describe('internal_on_merge_tag_versions trigger', () => { + const workflow = readWorkflow(workflowFile); + + test('runs only for pull requests into main', () => { + expect(workflow).toMatch(/pull_request:\n {4}branches: \[main\]\n {4}types: \[closed\]/); + }); + + test('tags the merge commit of the pull request', () => { + expect(workflow).toContain('ref: ${{ github.event.pull_request.merge_commit_sha }}'); + }); + + test('runs steps in bash with pipefail, so a failed tag lookup fails the step', () => { + expect(workflow).toContain('defaults:\n run:\n shell: bash\n'); + }); +}); + +describe('internal_on_merge_tag_versions Create Version Tags', () => { + let directory; + let remote; + let checkout; + + beforeEach(() => { + directory = fs.mkdtempSync(path.join(os.tmpdir(), 'tag-versions-')); + remote = path.join(directory, 'remote.git'); + checkout = path.join(directory, 'checkout'); + git(directory, ['init', '--quiet', '--bare', remote]); + git(directory, ['init', '--quiet', checkout]); + fs.writeFileSync(path.join(checkout, 'file.txt'), 'first\n'); + git(checkout, ['add', 'file.txt']); + git(checkout, ['commit', '--quiet', '-m', 'first']); + git(checkout, ['remote', 'add', 'origin', remote]); + git(checkout, ['tag', 'actions/existing/1.0.0']); + git(checkout, ['push', '--quiet', 'origin', 'refs/tags/actions/existing/1.0.0']); + git(checkout, ['tag', '--delete', 'actions/existing/1.0.0']); + fs.writeFileSync(path.join(checkout, 'file.txt'), 'second\n'); + git(checkout, ['commit', '--quiet', '-am', 'second']); + }); + + afterEach(() => { + fs.rmSync(directory, { recursive: true, force: true }); + }); + + const createTags = componentTags => + runScript(extractRunScript(workflowFile, 'set-version-tags', 'Create Version Tags'), { + cwd: checkout, + env: { ...gitEnv, COMPONENT_TAGS: componentTags }, + }); + + const remoteTags = () => + git(checkout, ['ls-remote', '--tags', '--refs', 'origin']) + .split('\n') + .filter(Boolean) + .map(line => line.split('\t')[1].replace('refs/tags/', '')) + .sort(); + + test('pushes every tag on the checked-out commit', () => { + const result = createTags('actions/new-action/1.0.0,workflows/new_workflow/2.0.0'); + + expect(result.status).toBe(0); + expect(remoteTags()).toEqual([ + 'actions/existing/1.0.0', + 'actions/new-action/1.0.0', + 'workflows/new_workflow/2.0.0', + ]); + const head = git(checkout, ['rev-parse', 'HEAD']); + expect(git(remote, ['rev-parse', 'workflows/new_workflow/2.0.0'])).toBe(head); + expect(result.summary).toContain('✅ Created and pushed tag: actions/new-action/1.0.0'); + }); + + test('pushes no tags when any of them already exists', () => { + const result = createTags('actions/new-action/1.0.0,actions/existing/1.0.0'); + + expect(result.status).toBe(1); + expect(result.stdout).toContain( + '::error title=Tags Already Exist::These tags already exist, so no tags were created: actions/existing/1.0.0' + ); + expect(remoteTags()).toEqual(['actions/existing/1.0.0']); + }); + + test('reports a rejected push and pushes no tags', () => { + const hook = path.join(remote, 'hooks', 'pre-receive'); + fs.writeFileSync(hook, '#!/bin/sh\nexit 1\n', { mode: 0o755 }); + + const result = createTags('actions/new-action/1.0.0,workflows/new_workflow/2.0.0'); + + expect(result.status).toBe(1); + expect(result.stdout).toContain( + '::error title=Tag Push Rejected::The atomic push was rejected, so no tags were pushed.' + ); + expect(result.summary).not.toContain('✅'); + expect(remoteTags()).toEqual(['actions/existing/1.0.0']); + }); + + test('does nothing when there are no component tags', () => { + const result = createTags(''); + + expect(result.status).toBe(0); + expect(remoteTags()).toEqual(['actions/existing/1.0.0']); + }); +}); diff --git a/.github/workflows/TESTS/internal_on_pr_validate_component_version.unit.test.js b/.github/workflows/TESTS/internal_on_pr_validate_component_version.unit.test.js new file mode 100644 index 0000000..4e6b312 --- /dev/null +++ b/.github/workflows/TESTS/internal_on_pr_validate_component_version.unit.test.js @@ -0,0 +1,81 @@ +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +const { extractRunScript, readWorkflow, runScript } = require('./workflow-test-helpers'); + +const workflowFile = 'internal_on_pr_validate_component_version.yml'; + +const gitEnv = { + GIT_CONFIG_GLOBAL: '/dev/null', + GIT_CONFIG_NOSYSTEM: '1', + GIT_AUTHOR_NAME: 'Test', + GIT_AUTHOR_EMAIL: 'test@example.com', + GIT_COMMITTER_NAME: 'Test', + GIT_COMMITTER_EMAIL: 'test@example.com', +}; + +function git(cwd, args) { + const result = spawnSync('git', args, { + cwd, + encoding: 'utf8', + env: { ...process.env, ...gitEnv }, + }); + if (result.status !== 0) { + throw new Error(`git ${args.join(' ')} failed: ${result.stderr}`); + } + return result.stdout.trim(); +} + +describe('internal_on_pr_validate_component_version shell', () => { + test('runs steps in bash with pipefail, so a failed tag lookup fails the step', () => { + expect(readWorkflow(workflowFile)).toContain('defaults:\n run:\n shell: bash\n'); + }); +}); + +describe('internal_on_pr_validate_component_version Get released version tags', () => { + let directory; + let checkout; + + beforeEach(() => { + directory = fs.mkdtempSync(path.join(os.tmpdir(), 'released-tags-')); + const remote = path.join(directory, 'remote.git'); + checkout = path.join(directory, 'checkout'); + git(directory, ['init', '--quiet', '--bare', remote]); + git(directory, ['init', '--quiet', checkout]); + fs.writeFileSync(path.join(checkout, 'file.txt'), 'first\n'); + git(checkout, ['add', 'file.txt']); + git(checkout, ['commit', '--quiet', '-m', 'first']); + git(checkout, ['remote', 'add', 'origin', remote]); + git(checkout, ['tag', 'actions/pr-open-check/1.0.0']); + git(checkout, ['tag', 'workflows/run_semgrep_scan/2.0.0']); + git(checkout, ['push', '--quiet', 'origin', '--tags']); + }); + + afterEach(() => { + fs.rmSync(directory, { recursive: true, force: true }); + }); + + const getReleasedTags = () => + runScript( + extractRunScript(workflowFile, 'validate-version-labels', 'Get released version tags'), + { cwd: checkout, env: gitEnv } + ); + + test('writes the remote tag names to released_tags.txt', () => { + const result = getReleasedTags(); + + expect(result.status).toBe(0); + expect(fs.readFileSync(path.join(checkout, 'released_tags.txt'), 'utf8')).toBe( + 'actions/pr-open-check/1.0.0\nworkflows/run_semgrep_scan/2.0.0\n' + ); + }); + + test('fails when the remote cannot be read', () => { + git(checkout, ['remote', 'set-url', 'origin', path.join(directory, 'missing.git')]); + + const result = getReleasedTags(); + + expect(result.status).not.toBe(0); + }); +}); diff --git a/.github/workflows/internal_on_merge_tag_versions.yml b/.github/workflows/internal_on_merge_tag_versions.yml index ed90223..1a1a19d 100644 --- a/.github/workflows/internal_on_merge_tag_versions.yml +++ b/.github/workflows/internal_on_merge_tag_versions.yml @@ -2,10 +2,15 @@ name: Set Version Tags on: pull_request: + branches: [main] types: [closed] permissions: {} +defaults: + run: + shell: bash + jobs: no-merge: if: github.event.pull_request.merged == false @@ -27,12 +32,14 @@ jobs: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + ref: ${{ github.event.pull_request.merge_commit_sha }} persist-credentials: true # Required for git push of the version tags. - name: Set up Node.js uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 with: node-version-file: .nvmrc + package-manager-cache: false - name: Get all labels from PR id: get_labels @@ -73,8 +80,37 @@ jobs: COMPONENT_TAGS: ${{ steps.get_tags.outputs.componentTags }} run: | IFS=',' read -r -a tagsArray <<< "$COMPONENT_TAGS" + if [ "${#tagsArray[@]}" -eq 0 ]; then + exit 0 + fi + + released_tags=$(git ls-remote --tags --refs origin | cut -f2) + existing=() + refs=() + for tag in "${tagsArray[@]}"; do + if grep -Fxq "refs/tags/$tag" <<< "$released_tags"; then + existing+=("$tag") + fi + refs+=("refs/tags/$tag") + done + + if [ "${#existing[@]}" -gt 0 ]; then + message="These tags already exist, so no tags were created: ${existing[*]}" + echo "❌ $message" >> "$GITHUB_STEP_SUMMARY" + echo "::error title=Tags Already Exist::$message" + exit 1 + fi + for tag in "${tagsArray[@]}"; do git tag "$tag" - git push origin "$tag" + done + if ! git push --atomic origin "${refs[@]}"; then + message="The atomic push was rejected, so no tags were pushed. Another run may have created one of these tags since the check: ${tagsArray[*]}" + echo "❌ $message" >> "$GITHUB_STEP_SUMMARY" + echo "::error title=Tag Push Rejected::$message" + exit 1 + fi + + for tag in "${tagsArray[@]}"; do echo "✅ Created and pushed tag: $tag" >> "$GITHUB_STEP_SUMMARY" done diff --git a/.github/workflows/internal_on_pr_validate_component_version.yml b/.github/workflows/internal_on_pr_validate_component_version.yml index 34cd5f7..68ff3ca 100644 --- a/.github/workflows/internal_on_pr_validate_component_version.yml +++ b/.github/workflows/internal_on_pr_validate_component_version.yml @@ -11,8 +11,16 @@ on: - unlabeled - reopened +concurrency: + group: validate-version-labels-${{ github.event.pull_request.number }} + cancel-in-progress: true + permissions: {} +defaults: + run: + shell: bash + jobs: validate-version-labels: name: Validate PR Version Labels @@ -83,13 +91,20 @@ jobs: echo "$all_labels" > all_labels.txt + - name: Get released version tags + run: | + git ls-remote --tags --refs origin | cut -f2 | sed 's#^refs/tags/##' > released_tags.txt + - name: Set up Node.js uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 with: node-version-file: .nvmrc + package-manager-cache: false - name: Label Validation id: validate + env: + RELEASED_TAGS_FILE: released_tags.txt run: node scripts/internal-ci/validate-version-labels/index.js all_labels.txt - name: Validation Job Summary @@ -102,8 +117,10 @@ jobs: INVALID_VERSION_LABELS: ${{ steps.validate.outputs.invalidVersionLabels }} HAS_UNTRACKED_VERSION: ${{ steps.validate.outputs.hasUntrackedVersion }} COMPONENT_VERSION_LABELS: ${{ steps.validate.outputs.componentVersionLabels }} + DUPLICATE_COMPONENTS: ${{ steps.validate.outputs.duplicateComponents }} INVALID_COMPONENTS: ${{ steps.validate.outputs.invalidComponents }} MISSING_CHANGELOGS: ${{ steps.validate.outputs.missingChangelogs }} + RELEASED_VERSIONS: ${{ steps.validate.outputs.releasedVersions }} run: | { echo "## Validation Outcome" @@ -125,12 +142,18 @@ jobs: if [ -n "$INVALID_VERSION_LABELS" ]; then echo "**Invalid version labels**: $INVALID_VERSION_LABELS" fi + if [ -n "$DUPLICATE_COMPONENTS" ]; then + echo "**Components with more than one version label**: $DUPLICATE_COMPONENTS" + fi if [ -n "$INVALID_COMPONENTS" ]; then echo "**Nonexistent/Invalid components**: $INVALID_COMPONENTS" fi if [ -n "$MISSING_CHANGELOGS" ]; then echo "**Components missing changelogs/entries**: $MISSING_CHANGELOGS" fi + if [ -n "$RELEASED_VERSIONS" ]; then + echo "**Versions already released**: $RELEASED_VERSIONS" + fi elif [ "$IS_VALID" == "true" ]; then echo "✅ Version label validation passed" fi @@ -146,13 +169,11 @@ jobs: HAS_UNTRACKED_VERSION: ${{ steps.validate.outputs.hasUntrackedVersion }} VALID_COMPONENTS: ${{ steps.validate.outputs.validComponents }} run: | - set -x summary="## Tags\n" if [ "${HAS_UNTRACKED_VERSION}" = "true" ]; then summary+="- No Tags will be created on main" else summary+="The following tags will be created on main after merge\n\n" - echo "$VALID_COMPONENTS" IFS=',' read -ra components <<< "$VALID_COMPONENTS" for component_version in "${components[@]}"; do summary+="🏷️ \`$component_version\`\n" diff --git a/VERSIONING.md b/VERSIONING.md index 900bbc2..8c1732c 100644 --- a/VERSIONING.md +++ b/VERSIONING.md @@ -81,7 +81,7 @@ v:a/pr-open-check/1.1.0 v:wf/run_semgrep_scan/1.0.0 ``` -Multiple version labels are allowed on a PR. +Multiple version labels are allowed on a PR, one per component. ### Special Case: Untracked Versions @@ -109,7 +109,9 @@ A PR will fail automated validation if any of the following are true: - There are multiple version labels and one of them is `v:untracked` - A version label uses an invalid component name - X.Y.Z version does not appear in the component’s CHANGELOG.md -- There are duplicate or malformed version labels +- A component has more than one version label +- A version label's tag already exists, so the version is already released +- There are malformed version labels A PR cannot merge until version validation passes. @@ -163,7 +165,8 @@ Each released version must have a heading that includes the version label Minimum requirements: -- The header must contain `## X.Y.Z` exactly (this is what validation looks for). +- The header must be a line that is exactly `## X.Y.Z`, optionally followed by ` - title` or + ` — title` (this is what validation looks for). - The PR must add or update an entry for the version used in the label (`v:a/{component-name}/X.Y.Z` or `v:wf/{component-name}/X.Y.Z`). diff --git a/scripts/internal-ci/get-version-tags/index.integration.test.js b/scripts/internal-ci/get-version-tags/index.integration.test.js index 4006ca9..ee3770b 100644 --- a/scripts/internal-ci/get-version-tags/index.integration.test.js +++ b/scripts/internal-ci/get-version-tags/index.integration.test.js @@ -50,4 +50,21 @@ describe('get-version-tags main module integration', () => { fs.unlinkSync(labelFile); fs.unlinkSync(outputFile); }); + + it('fails without writing tags when a component has more than one version label', () => { + const labelInput = `${versionLabelPrefix}a/pr-open-check/1.0.0\n${versionLabelPrefix}a/pr-open-check/1.1.0`; + const unique = Date.now() + Math.random(); + const labelFile = path.join(tmp, `labels-${unique}.txt`); + const outputFile = path.join(tmp, `gha_output-${unique}.txt`); + fs.writeFileSync(labelFile, labelInput, 'utf8'); + const result = spawnSync('node', [scriptPath, labelFile], { + cwd: projectRoot, + encoding: 'utf8', + env: { ...process.env, GITHUB_OUTPUT: outputFile }, + }); + expect(result.status).toBe(1); + expect(result.stderr).toContain('More than one version label for: actions/pr-open-check'); + expect(fs.existsSync(outputFile)).toBe(false); + fs.unlinkSync(labelFile); + }); }); diff --git a/scripts/internal-ci/get-version-tags/index.js b/scripts/internal-ci/get-version-tags/index.js index 64ce4f1..51e9a63 100644 --- a/scripts/internal-ci/get-version-tags/index.js +++ b/scripts/internal-ci/get-version-tags/index.js @@ -1,32 +1,41 @@ -/// Reads labels from all_labels.txt and outputs whether untracked is present +/// Reads labels from a file and outputs whether untracked is present /// and a comma-separated list of component version tags for GitHub Actions. /// /// Outputs: -/// HAS_UNTRACKED=true|false +/// hasUntracked=true|false /// componentTags=type/name/X.Y.Z,... +/// Exits with code 1, writing no outputs, when a component has more than one version label. /// Usage: /// node scripts/internal-ci/get-version-tags/index.js const fs = require('fs'); const { getLabelArray, + getVersionLabelArray, + getComponentVersionLabels, + getDuplicateComponents, untrackedLabel, - componentVersionRegEx, expandComponentVersionLabel, } = require('../validate-version-labels/index.js'); // Main entry if (require.main === module) { - // Accept labels from stdin or as a file argument const rawLabels = fs.readFileSync(process.argv[2], 'utf8'); - const labels = getLabelArray(rawLabels); + const versionLabels = getVersionLabelArray(getLabelArray(rawLabels)); - const hasUntracked = labels.includes(untrackedLabel); + const hasUntracked = versionLabels.includes(untrackedLabel); - const componentTags = labels - .filter(label => componentVersionRegEx.test(label)) - .map(expandComponentVersionLabel); + const componentVersionLabels = getComponentVersionLabels(versionLabels); + const duplicateComponents = getDuplicateComponents(componentVersionLabels); + if (duplicateComponents.length > 0) { + console.error( + `::error title=Duplicate Version Labels::More than one version label for: ${duplicateComponents.join(', ')}` + ); + process.exit(1); + } + + const componentTags = componentVersionLabels.map(expandComponentVersionLabel); // write outputs for use in later steps const githubOutput = process.env.GITHUB_OUTPUT; diff --git a/scripts/internal-ci/validate-version-labels/index.integration.test.js b/scripts/internal-ci/validate-version-labels/index.integration.test.js index a2b7d66..d540c7e 100644 --- a/scripts/internal-ci/validate-version-labels/index.integration.test.js +++ b/scripts/internal-ci/validate-version-labels/index.integration.test.js @@ -46,6 +46,49 @@ describe('validate-version-labels main module integration', () => { fs.unlinkSync(outputFile); }); + it('fails when a component has more than one version label', () => { + const labelInput = `${versionLabelPrefix}a/pr-open-check/1.0.0\n${versionLabelPrefix}a/pr-open-check/1.1.0`; + const outputFile = path.join(tmp, 'gha_duplicate_output.txt'); + const result = spawnSync('node', [scriptPath], { + cwd: projectRoot, + input: labelInput, + encoding: 'utf8', + env: { ...process.env, GITHUB_OUTPUT: outputFile }, + }); + const outputs = parseGithubOutput(outputFile); + expect(outputs.duplicateComponents).toBe('actions/pr-open-check'); + expect(outputs.validComponents).toBe(''); + expect(outputs.validationMessage).toContain('More than one version label'); + expect(outputs.isValid).toBe('false'); + expect(result.status).not.toBe(0); + fs.unlinkSync(outputFile); + }); + + it('fails when the tag listed in RELEASED_TAGS_FILE already exists', () => { + const labelInput = `${versionLabelPrefix}a/pr-open-check/1.0.0`; + const releasedTagsFile = path.join(tmp, 'released_tags.txt'); + fs.writeFileSync( + releasedTagsFile, + 'actions/pr-open-check/1.0.0\nworkflows/run_semgrep_scan/1.0.0\n', + 'utf8' + ); + const outputFile = path.join(tmp, 'gha_released_output.txt'); + const result = spawnSync('node', [scriptPath], { + cwd: projectRoot, + input: labelInput, + encoding: 'utf8', + env: { ...process.env, GITHUB_OUTPUT: outputFile, RELEASED_TAGS_FILE: releasedTagsFile }, + }); + const outputs = parseGithubOutput(outputFile); + expect(outputs.releasedVersions).toBe('actions/pr-open-check/1.0.0'); + expect(outputs.validComponents).toBe(''); + expect(outputs.validationMessage).toContain('already released'); + expect(outputs.isValid).toBe('false'); + expect(result.status).not.toBe(0); + fs.unlinkSync(releasedTagsFile); + fs.unlinkSync(outputFile); + }); + it('rejects the previous version label format', () => { const labelInput = 'version:actions/pr-open-check/1.0.0'; const outputFile = path.join(tmp, 'gha_old_format_output.txt'); diff --git a/scripts/internal-ci/validate-version-labels/index.js b/scripts/internal-ci/validate-version-labels/index.js index 63cc021..644bad8 100644 --- a/scripts/internal-ci/validate-version-labels/index.js +++ b/scripts/internal-ci/validate-version-labels/index.js @@ -1,12 +1,13 @@ -// validate-version-labels.js -// Script to validate version labels for CI workflows +// Validates the version labels on a pull request. // // Usage: -// node validate-version-labels.js -// echo -e "v:a/foo/1.2.3\nv:wf/bar/2.0.0\nother" | node scripts/internal-ci/validate-version-labels.js +// node scripts/internal-ci/validate-version-labels/index.js +// echo -e "v:a/foo/1.2.3\nv:wf/bar/2.0.0\nother" | node scripts/internal-ci/validate-version-labels/index.js // // Inputs: // - Newline-separated list of label strings (from file or stdin) +// - RELEASED_TAGS_FILE (optional): newline-separated list of existing release tags. When set, +// a label whose tag is listed fails validation. // // Outputs: // - Writes to $GITHUB_OUTPUT variable: @@ -15,8 +16,10 @@ // invalidVersionLabels: comma-separated list of invalid version labels // hasUntrackedVersion: true/false if untracked version label is present // componentVersionLabels: comma-separated list of valid component version labels +// duplicateComponents: comma-separated list of components with more than one version label // invalidComponents: comma-separated list of invalid component paths // missingChangelogs: comma-separated list of components missing changelog entries +// releasedVersions: comma-separated list of component versions that already have a tag // validComponents: comma-separated list of valid components with versions in format component-path/X.Y.Z // - Exits with code 1 on validation failure, 0 on success @@ -36,7 +39,7 @@ const componentTypeAliases = { a: 'actions', wf: 'workflows', }; -const semverPattern = '\\d+\\.\\d+\\.\\d+'; // Semantic versioning pattern e.g., v1.2.3 +const semverPattern = '\\d+\\.\\d+\\.\\d+'; // Semantic versioning pattern e.g., 1.2.3 // Pattern for valid version label: v:/component-name/X.Y.Z const componentVersionRegEx = new RegExp( `^${versionLabelPrefix}(${Object.keys(componentTypeAliases).join('|')})/([a-z0-9_-]+)/(${semverPattern})$` @@ -84,6 +87,26 @@ function parseComponentVersionLabels(componentVersionLabels) { return componentVersionMap; } +/// Returns the components that have more than one version label +/// It is assumed that component version labels are valid when this function is called +function getDuplicateComponents(componentVersionLabels) { + const labelCounts = {}; + componentVersionLabels.forEach(label => { + const componentTag = expandComponentVersionLabel(label); + const component = componentTag.substring(0, componentTag.lastIndexOf('/')); + labelCounts[component] = (labelCounts[component] || 0) + 1; + }); + return Object.keys(labelCounts).filter(component => labelCounts[component] > 1); +} + +/// Returns the component versions, as component-path/X.Y.Z, that already have a release tag +function getReleasedVersions(componentVersionMap, releasedTags) { + const tagSet = new Set(releasedTags); + return Object.entries(componentVersionMap) + .map(([component, version]) => `${component}/${version}`) + .filter(tag => tagSet.has(tag)); +} + /// Checks for existence of paths in the repo matching the component function getInvalidComponents(componentVersionMap) { if (Object.keys(componentVersionMap).length == 0) { @@ -99,7 +122,7 @@ function getInvalidComponents(componentVersionMap) { } } else if (path.startsWith('workflows/')) { const expectedFile = `.github/${path}.yml`; - if (!fs.existsSync(expectedFile) || !fs.lstatSync(expectedFile).isFile) { + if (!fs.existsSync(expectedFile) || !fs.lstatSync(expectedFile).isFile()) { invalidComponents.push(path); } } else { @@ -110,6 +133,14 @@ function getInvalidComponents(componentVersionMap) { return invalidComponents; } +/// Checks for a `## X.Y.Z` heading, optionally followed by ` - title` or ` — title` +function hasChangelogEntry(changelogContent, version) { + const escapedVersion = version.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return new RegExp(`^## ${escapedVersion}(?:[ \\t]+[-—][ \\t].*)?[ \\t\\r]*$`, 'm').test( + changelogContent + ); +} + /// Checks for CHANGELOG.md entry matching the version for each component /// It is assumed that the components exist when this function is called function getMissingChangelogs(componentVersionMap) { @@ -133,8 +164,7 @@ function getMissingChangelogs(componentVersionMap) { continue; } const changelogContent = fs.readFileSync(changelogPath, 'utf8'); - const expectedVersionHeader = `## ${componentVersionMap[path]}`; - if (!changelogContent.includes(expectedVersionHeader)) { + if (!hasChangelogEntry(changelogContent, componentVersionMap[path])) { missingChangelogs.push(path); } } @@ -147,7 +177,9 @@ function validate( hasComponentVersionLabels, hasUntrackedVersion, hasInvalidComponents, - hasMissingChangelogs + hasMissingChangelogs, + hasDuplicateComponents = false, + hasReleasedVersions = false ) { const hasVersionLabels = hasInvalidVersionLabels || hasComponentVersionLabels || hasUntrackedVersion; @@ -165,12 +197,18 @@ function validate( validationMessage += '- Conflicting version labels found: both untracked and component version labels present\n'; } + if (hasDuplicateComponents) { + validationMessage += '- More than one version label found for a component\n'; + } if (hasInvalidComponents) { validationMessage += '- Non-existent components found (invalid paths)\n'; } if (hasMissingChangelogs) { validationMessage += '- Missing or incomplete CHANGELOG.md entries found\n'; } + if (hasReleasedVersions) { + validationMessage += '- Versions that are already released found (the tag exists)\n'; + } if (validationMessage === '') { validationMessage = 'Version validation passed'; @@ -186,12 +224,7 @@ function validate( // Main entry if (require.main === module) { // Accept labels from stdin or as a file argument - let rawLabels = ''; - if (process.argv.length > 2) { - rawLabels = fs.readFileSync(process.argv[2], 'utf8'); - } else { - rawLabels = fs.readFileSync(0, 'utf8'); // Read from stdin - } + const rawLabels = fs.readFileSync(process.argv.length > 2 ? process.argv[2] : 0, 'utf8'); const labels = getLabelArray(rawLabels); console.log('Normalized Labels:', labels); @@ -203,30 +236,45 @@ if (require.main === module) { const hasUntrackedVersion = versionLabels.includes(untrackedLabel); const componentVersionMap = parseComponentVersionLabels(componentVersionLabels); + const duplicateComponents = getDuplicateComponents(componentVersionLabels); const invalidComponents = getInvalidComponents(componentVersionMap); - let validComponentMap = Object.fromEntries( - Object.entries(componentVersionMap).filter( - ([component]) => !invalidComponents.includes(component) - ) + const withoutComponents = (componentMap, excluded) => + Object.fromEntries( + Object.entries(componentMap).filter(([component]) => !excluded.includes(component)) + ); + + let validComponentMap = withoutComponents( + componentVersionMap, + duplicateComponents.concat(invalidComponents) ); const missingChangelogs = getMissingChangelogs(validComponentMap); - + validComponentMap = withoutComponents(validComponentMap, missingChangelogs); + + const releasedTagsFile = process.env.RELEASED_TAGS_FILE; + const releasedVersions = releasedTagsFile + ? getReleasedVersions( + validComponentMap, + getLabelArray(fs.readFileSync(releasedTagsFile, 'utf8')) + ) + : []; validComponentMap = Object.fromEntries( Object.entries(validComponentMap).filter( - ([component]) => !missingChangelogs.includes(component) + ([component, version]) => !releasedVersions.includes(`${component}/${version}`) ) ); // Validate - let { isValid, validationMessage } = validate( + const { isValid, validationMessage } = validate( invalidVersionLabels.length > 0, componentVersionLabels.length, hasUntrackedVersion, invalidComponents.length > 0, - missingChangelogs.length > 0 + missingChangelogs.length > 0, + duplicateComponents.length > 0, + releasedVersions.length > 0 ); // write outputs for use in later steps @@ -239,8 +287,10 @@ if (require.main === module) { githubOutput, `componentVersionLabels=${componentVersionLabels.join(', ')}\n` ); + fs.appendFileSync(githubOutput, `duplicateComponents=${duplicateComponents.join(', ')}\n`); fs.appendFileSync(githubOutput, `invalidComponents=${invalidComponents.join(', ')}\n`); fs.appendFileSync(githubOutput, `missingChangelogs=${missingChangelogs.join(', ')}\n`); + fs.appendFileSync(githubOutput, `releasedVersions=${releasedVersions.join(', ')}\n`); fs.appendFileSync( githubOutput, `validationMessage=${validationMessage.replace(/\n/g, '\\n')}\n` @@ -270,7 +320,10 @@ module.exports = { getComponentVersionLabels, expandComponentVersionLabel, parseComponentVersionLabels, + getDuplicateComponents, + getReleasedVersions, getInvalidComponents, + hasChangelogEntry, getMissingChangelogs, validate, versionLabelPrefix, diff --git a/scripts/internal-ci/validate-version-labels/index.unit.test.js b/scripts/internal-ci/validate-version-labels/index.unit.test.js index ead4567..b19c5dd 100644 --- a/scripts/internal-ci/validate-version-labels/index.unit.test.js +++ b/scripts/internal-ci/validate-version-labels/index.unit.test.js @@ -5,7 +5,10 @@ const { getComponentVersionLabels, expandComponentVersionLabel, parseComponentVersionLabels, + getDuplicateComponents, + getReleasedVersions, getInvalidComponents, + hasChangelogEntry, getMissingChangelogs, validate, versionLabelPrefix, @@ -104,6 +107,45 @@ describe('parseComponentVersionLabels', () => { }); }); +describe('getDuplicateComponents', () => { + it('should return components with more than one version label', () => { + const componentLabels = [ + `${versionLabelPrefix}a/component-a/1.2.3`, + `${versionLabelPrefix}wf/component-b/2.3.4`, + `${versionLabelPrefix}a/component-a/1.2.4`, + `${versionLabelPrefix}wf/component-a/1.0.0`, + ]; + expect(getDuplicateComponents(componentLabels)).toEqual(['actions/component-a']); + }); + + it('should return an empty array when each component has one label', () => { + const componentLabels = [ + `${versionLabelPrefix}a/component-a/1.2.3`, + `${versionLabelPrefix}wf/component-a/1.2.3`, + ]; + expect(getDuplicateComponents(componentLabels)).toEqual([]); + }); +}); + +describe('getReleasedVersions', () => { + const releasedTags = ['actions/component-a/1.0.0', 'workflows/component-b/2.0.0']; + + it('should return component versions that already have a tag', () => { + const componentVersionMap = { + 'actions/component-a': '1.0.0', + 'workflows/component-b': '2.0.1', + }; + expect(getReleasedVersions(componentVersionMap, releasedTags)).toEqual([ + 'actions/component-a/1.0.0', + ]); + }); + + it('should not match a tag of the same version for another component type', () => { + const componentVersionMap = { 'workflows/component-a': '1.0.0' }; + expect(getReleasedVersions(componentVersionMap, releasedTags)).toEqual([]); + }); +}); + // It is assumed that path is correctly formatted when this function is called (dir1/dir2) describe('getInvalidComponents', () => { const fs = require('fs'); @@ -117,6 +159,7 @@ describe('getInvalidComponents', () => { '.github/actions/not-a-directory': 'file', // invalid action, extensionless file instead of directory '.github/workflows/not-a-file': 'directory', // invalid workflow - subdirectory instead of file '.github/workflows/not-yaml.txt': 'file', // invalid workflow - wrong file type + '.github/workflows/directory-workflow.yml': 'directory', // invalid workflow - directory named like a workflow '.github/foo/bar.yml': 'file', // invalid type }; @@ -190,6 +233,38 @@ describe('getInvalidComponents', () => { const invalidComponents = getInvalidComponents(componentVersionMap); expect(invalidComponents).toEqual(['workflows/not-a-file', 'workflows/not-yaml']); }); + + it('should return workflows whose .yml path is a directory', () => { + const componentVersionMap = { + 'workflows/directory-workflow': '1.0.0', + }; + const invalidComponents = getInvalidComponents(componentVersionMap); + expect(invalidComponents).toEqual(['workflows/directory-workflow']); + }); +}); + +describe('hasChangelogEntry', () => { + it.each([ + ['## 1.0.0\n- Initial release', '1.0.0'], + ['# Changelog\n\n## 1.0.1\n\n## 1.0.0', '1.0.0'], + ['## 1.0.0 - Initial Release', '1.0.0'], + ['## 1.0.0 — Initial Release', '1.0.0'], + ['## 1.0.0\r\n- Windows line endings', '1.0.0'], + ])('should find the entry in %j for %s', (content, version) => { + expect(hasChangelogEntry(content, version)).toBe(true); + }); + + it.each([ + ['## 1.0.10', '1.0.1'], + ['### 1.0.0', '1.0.0'], + ['## 11.0.0', '1.0.0'], + ['## 1x0x0', '1.0.0'], + ['## v1.0.0', '1.0.0'], + ['## 1.0.0-beta', '1.0.0'], + ['Released in ## 1.0.0', '1.0.0'], + ])('should not find the entry in %j for %s', (content, version) => { + expect(hasChangelogEntry(content, version)).toBe(false); + }); }); // It is assumed that path is correctly formatted when this function is called (dir1/dir2) @@ -340,6 +415,31 @@ describe('validate', () => { }, ]; + scenarios.push( + { + description: 'a component has more than one version label', + hasUntrackedLabel: false, + hasInvalidLabels: false, + hasComponentLabels: true, + hasInvalidComponents: false, + hasMissingChangelogs: false, + hasDuplicateComponents: true, + expectedIsValid: false, + expectedMessageFragment: '- More than one version label found for a component', + }, + { + description: 'a version is already released', + hasUntrackedLabel: false, + hasInvalidLabels: false, + hasComponentLabels: true, + hasInvalidComponents: false, + hasMissingChangelogs: false, + hasReleasedVersions: true, + expectedIsValid: false, + expectedMessageFragment: '- Versions that are already released', + } + ); + scenarios.forEach(scenario => { it(`should ${scenario.expectedIsValid ? 'pass' : 'fail'} when ${scenario.description}`, () => { const result = validate( @@ -347,7 +447,9 @@ describe('validate', () => { scenario.hasComponentLabels, scenario.hasUntrackedLabel, scenario.hasInvalidComponents, - scenario.hasMissingChangelogs + scenario.hasMissingChangelogs, + scenario.hasDuplicateComponents, + scenario.hasReleasedVersions ); expect(result.isValid).toBe(scenario.expectedIsValid); expect(result.validationMessage).toContain(scenario.expectedMessageFragment);