diff --git a/README.md b/README.md index dc9c4054f..6887f438c 100644 --- a/README.md +++ b/README.md @@ -431,6 +431,14 @@ You can pass environment variables; the easiest way to do it is via a `.env` fil ``` BLOCK_REPO_RENAME_BY_HUMAN=true ``` +1. Specify a custom branch for configuration files using `SAFE_SETTINGS_BRANCH`. This is useful for testing configuration changes from a PR branch before merging. For e.g. + ``` + SAFE_SETTINGS_BRANCH=feature/my-branch + ``` + Alternative environment variables that are automatically detected (in order of priority): + - `GITHUB_HEAD_REF` (GitHub Actions PR head branch) + - `GITHUB_REF_NAME` (GitHub Actions branch name) + - `GITHUB_REF` (GitHub Actions full ref, will be converted to branch name) ### Runtime Settings diff --git a/docs/github-action.md b/docs/github-action.md index dcff491f7..0a5a3aa65 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -8,7 +8,10 @@ Follow the [Create the GitHub App](deploy.md#create-the-github-app) guide to cre ## Defining the GitHub Action Workflow -Running a full-sync with `safe-settings` can be done via `npm run full-sync`. This requires installing Node, such as with [actions/setup-node](https://github.com/actions/setup-node) (see example below). When doing so, the appropriate environment variables must be set (see the [Environment variables](#environment-variables) document for more details). +Running a full-sync with `safe-settings` can be done via `npm run full-sync`. This requires installing Node, such as with [actions/setup-node](https://github.com/actions/setup-node) (see example below). When doing so, the appropriate environment variables must be set (see the [Environment variables](../README.md#environment-variables) document for more details). + +### Testing Configuration Changes from PR Branches +You can test configuration changes from a PR branch before merging by setting the `SAFE_SETTINGS_BRANCH` environment variable or using one of the automatic GitHub Actions environment variables (`GITHUB_HEAD_REF`, `GITHUB_REF_NAME`, or `GITHUB_REF`). This allows you to see what changes would be applied without merging the PR first. ### Example GHA Workflow @@ -55,3 +58,49 @@ jobs: CONFIG_PATH: safe-settings DEPLOYMENT_CONFIG_FILE: ${{ github.workspace }}/safe-settings/deployment-settings.yml ``` + +### Example: Testing PR Changes +To test configuration changes from a specific branch (useful for testing PR changes before merging): + +```yaml +name: Test Safe Settings PR Changes +on: + workflow_dispatch: + inputs: + branch: + description: 'Branch to test configuration from' + required: true + default: 'main' + +jobs: + testSafeSettingsChanges: + runs-on: ubuntu-latest + env: + SAFE_SETTINGS_VERSION: 2.1.13 + SAFE_SETTINGS_CODE_DIR: ${{ github.workspace }}/.safe-settings-code + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.inputs.branch }} + - uses: actions/checkout@v4 + with: + repository: github/safe-settings + ref: ${{ env.SAFE_SETTINGS_VERSION }} + path: ${{ env.SAFE_SETTINGS_CODE_DIR }} + - uses: actions/setup-node@v4 + - run: npm install + working-directory: ${{ env.SAFE_SETTINGS_CODE_DIR }} + - run: npm run full-sync + working-directory: ${{ env.SAFE_SETTINGS_CODE_DIR }} + env: + GH_ORG: ${{ vars.SAFE_SETTINGS_GH_ORG }} + APP_ID: ${{ vars.SAFE_SETTINGS_APP_ID }} + PRIVATE_KEY: ${{ secrets.SAFE_SETTINGS_PRIVATE_KEY }} + GITHUB_CLIENT_ID: ${{ vars.SAFE_SETTINGS_GITHUB_CLIENT_ID }} + GITHUB_CLIENT_SECRET: ${{ secrets.SAFE_SETTINGS_GITHUB_CLIENT_SECRET }} + ADMIN_REPO: .github + CONFIG_PATH: safe-settings + DEPLOYMENT_CONFIG_FILE: ${{ github.workspace }}/safe-settings/deployment-settings.yml + # Test configuration from the specified branch + SAFE_SETTINGS_BRANCH: ${{ github.event.inputs.branch }} +``` diff --git a/docs/sample-settings/sample-deployment-settings.yml b/docs/sample-settings/sample-deployment-settings.yml index 6164d4389..6c3839fe7 100644 --- a/docs/sample-settings/sample-deployment-settings.yml +++ b/docs/sample-settings/sample-deployment-settings.yml @@ -4,15 +4,22 @@ # If no file is specified, the following repositories are excluded by default # restrictedRepos: ['admin', '.github', 'safe-settings'] +# restrictedRepos supports both glob patterns and regex patterns +# Glob patterns: use * for wildcards (e.g., admin-*, core-*) +# Regex patterns: use regex syntax (e.g., ^admin-.*$, ^\\.github$) +# The system automatically detects the pattern type based on regex indicators restrictedRepos: exclude: - admin - .github - safe-settings - - admin-* + - admin-* # Glob pattern: matches admin-test, admin-config, etc. + - ^test-.*$ # Regex pattern: matches test- prefix + - ^\\.github$ # Regex pattern: matches exactly .github include: - docs - - core-* + - core-* # Glob pattern: matches core-api, core-service, etc. + - ^production-.*env$ # Regex pattern: matches production-*env configvalidators: - plugin: collaborators diff --git a/index.js b/index.js index f6af26b5a..4e664ef4b 100644 --- a/index.js +++ b/index.js @@ -234,7 +234,22 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => log: robot.log, repo: () => { return { repo: env.ADMIN_REPO, owner: installation.account.login } } } - return syncAllSettings(nop, context) + + // Use environment variable for branch reference, fallback to undefined (main branch) + let ref = process.env.SAFE_SETTINGS_BRANCH || process.env.GITHUB_HEAD_REF || process.env.GITHUB_REF_NAME || process.env.GITHUB_REF + + // If we have a ref and it doesn't start with refs/, assume it's a branch name and add refs/heads/ + if (ref && !ref.startsWith('refs/')) { + ref = `refs/heads/${ref}` + } + // If it starts with refs/heads/ already, use as-is + // If it's undefined, will use main branch + + if (ref) { + robot.log.info(`Using branch reference: ${ref}`) + } + + return syncAllSettings(nop, context, context.repo(), ref) } return null } diff --git a/lib/glob.js b/lib/glob.js index f78cee03e..9c2f28791 100644 --- a/lib/glob.js +++ b/lib/glob.js @@ -7,7 +7,29 @@ class Glob { } test (input) { - return minimatch(input, this.pattern, this.options) + // Detect if pattern looks like regex (has regex-specific characters) + if (this.isRegexPattern(this.pattern)) { + try { + const regex = new RegExp(this.pattern) + return regex.test(input) + } catch (e) { + // If regex parsing fails, fall back to glob + result = minimatch(input, this.pattern, this.options) + } + } else { + // Use glob pattern matching + result = minimatch(input, this.pattern, this.options) + } + return result + } + + isRegexPattern (pattern) { + // Check for common regex indicators + return pattern.includes('^') || + pattern.includes('$') || + pattern.includes('.*') || + pattern.includes('\\') || + (pattern.includes('[') && pattern.includes(']') && pattern.includes('^')) } } diff --git a/lib/settings.js b/lib/settings.js index 20e711672..e94e0e7df 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -66,7 +66,7 @@ class Settings { constructor (nop, context, repo, config, ref, suborg) { this.ref = ref this.context = context - this.installation_id = context.payload.installation.id + this.installation_id = context.payload?.installation?.id || context.installation?.id this.github = context.octokit this.repo = repo this.config = config @@ -151,8 +151,8 @@ class Settings { logError (msg) { this.log.error(msg) this.errors.push({ - owner: this.repo.owner, - repo: this.repo.repo, + owner: this.repo?.owner || 'unknown', + repo: this.repo?.repo || 'unknown', msg, plugin: this.constructor.name }) @@ -168,6 +168,12 @@ class Settings { return } + // Only proceed with PR comment/check run updates if we have webhook context + if (!payload || !payload.check_run) { + this.log.debug('Not in webhook context, skipping PR comment and check run updates') + return + } + // remove duplicate rows in this.results this.results = this.results.filter((thing, index, self) => { return index === self.findIndex((t) => { @@ -234,7 +240,7 @@ class Settings { const renderedCommentMessage = await eta.renderString(commetMessageTemplate, stats) - if (env.CREATE_PR_COMMENT === 'true') { + if (env.CREATE_PR_COMMENT === 'true' && payload.check_run?.check_suite?.pull_requests?.length > 0) { const summary = ` #### :robot: Safe-Settings config changes detected: @@ -261,16 +267,16 @@ ${this.results.reduce((x, y) => { const pullRequest = payload.check_run.check_suite.pull_requests[0] await this.github.issues.createComment({ - owner: payload.repository.owner.login, - repo: payload.repository.name, + owner: payload.repository?.owner?.login || this.repo?.owner || 'unknown', + repo: payload.repository?.name || this.repo?.repo || 'unknown', issue_number: pullRequest.number, body: summary.length > 55536 ? `${summary.substring(0, 55536)}... (too many changes to report)` : summary }) } const params = { - owner: payload.repository.owner.login, - repo: payload.repository.name, + owner: payload.repository?.owner?.login || this.repo?.owner || 'unknown', + repo: payload.repository?.name || this.repo?.repo || 'unknown', check_run_id: payload.check_run.id, status: 'completed', conclusion: error ? 'failure' : 'success', @@ -494,8 +500,10 @@ ${this.results.reduce((x, y) => { return null } - const { owner, name } = repository - return this.updateRepos({ owner: owner.login, repo: name }) + // Handle both string and object repository owner formats + const owner = typeof repository.owner === 'string' ? repository.owner : repository.owner.login + const { name } = repository + return this.updateRepos({ owner, repo: name }) }) ) })