diff --git a/README.md b/README.md index 07a626748..ab8ecdee2 100644 --- a/README.md +++ b/README.md @@ -494,6 +494,54 @@ See [`docs/sample-settings/settings.yml`](docs/sample-settings/settings.yml) for > - '*-config' > ``` +#### Preserving custom properties that `safe-settings` does not manage + +Custom properties cannot be deleted through the API, so a property that exists on a +repository but is absent from `custom_properties` has its value set to `null`. By +default `safe-settings` therefore owns *every* custom property on the repository. Use +the object form to declare what it manages and leave everything else untouched: + +```yml +custom_properties: + include: + - name: ruleset-tier + value: strict + exclude: + # Never clear the value of any property starting with "app-" + - name: ^app- +``` + +To manage only the properties you declare and leave all others alone, exclude +everything with `.*`: + +```yml +custom_properties: + include: + - name: ruleset-tier + value: strict + exclude: + - name: .* +``` + +> [!NOTE] +> Unlike the repository patterns above, these are **regular expressions matched +> against the property name**, not globs — "match everything" is `.*`, and a bare `*` +> is not a valid pattern. Casing does not matter. +> +> - `exclude` only prevents clearing. A property in `include` is always applied, even +> if it also matches an `exclude` pattern +> - `exclude` on its own still means `safe-settings` manages this repository's custom +> properties — every property not matching a pattern is cleared. To manage nothing, +> omit the `custom_properties` section entirely +> - An invalid pattern, or an object form with neither `include` nor `exclude`, is +> reported as a config error and fails closed: every property on that repository is +> left untouched, so a typo protects values rather than clearing them +> - `exclude` patterns accumulate across scopes, so a repository can add patterns to +> the ones defined for the org or suborg without restating them + +See [`docs/sample-settings/settings.yml`](docs/sample-settings/settings.yml) for a +commented example. + ### Additional values In addition to the values in the file above, the settings file can have some additional values: diff --git a/docs/sample-settings/settings.yml b/docs/sample-settings/settings.yml index 1ede6a079..a43c716f4 100644 --- a/docs/sample-settings/settings.yml +++ b/docs/sample-settings/settings.yml @@ -203,10 +203,26 @@ branches: # Custom properties # See https://docs.github.com/en/rest/repos/custom-properties?apiVersion=2026-03-10 +# A property absent from this config has its value cleared. To leave properties +# owned by other automation untouched, use the object form shown below - see +# "Preserving custom properties that `safe-settings` does not manage" in the README. custom_properties: - name: test value: test +# custom_properties: +# include: +# - name: test +# value: test +# exclude: +# # Regexes - not globs - matched against the property name +# - name: ^app- +# # Or `.*` to manage only the properties listed under `include` +# - name: ^deploy-status$ +# * The object form must contain `include`, `exclude`, or both. A malformed +# `custom_properties` (`{}`, say) is reported and also fails closed, leaving +# every property on the repo untouched. + # See the docs (https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/configuring-autolinks-to-reference-external-resources) for a description of autolinks and replacement values. autolinks: - key_prefix: "JIRA-" diff --git a/lib/plugins/custom_properties.js b/lib/plugins/custom_properties.js index 35f0144da..7fad17901 100644 --- a/lib/plugins/custom_properties.js +++ b/lib/plugins/custom_properties.js @@ -1,15 +1,75 @@ const Diffable = require('./diffable') const NopCommand = require('../nopcommand') +// Config shapes, precedence and the fail-closed behavior below are documented in +// README.md, "Preserving custom properties that `safe-settings` does not manage", +// with a commented example in docs/sample-settings/settings.yml. +function isExcludeAwareConfig (entries) { + return !!entries && + typeof entries === 'object' && + !Array.isArray(entries) && + (Array.isArray(entries.include) || Array.isArray(entries.exclude)) +} + module.exports = class CustomProperties extends Diffable { - constructor (...args) { - super(...args) + constructor (nop, github, repo, entries, log, errors) { + let include = entries + let exclude = [] + let malformed = false + + if (isExcludeAwareConfig(entries)) { + include = Array.isArray(entries.include) ? entries.include : [] + exclude = Array.isArray(entries.exclude) ? entries.exclude : [] + } else if (entries !== null && entries !== undefined && !Array.isArray(entries)) { + // Neither config shape, e.g. `custom_properties: {}`. Fail closed rather than + // letting a TypeError escape the constructor and reject the org-wide sync. + include = [] + malformed = true + } + + super(nop, github, repo, include, log, errors) + + const { patterns, excludeAll } = this.compileExcludePatterns(exclude) + this.exclude = patterns + this.excludeAll = excludeAll || malformed + + if (malformed) { + this.logError('`custom_properties` must be a list of properties or an object with `include` and/or `exclude` keys. Ignoring it and excluding all custom properties for this repo so no values are cleared.') + } if (this.entries) { this.normalizeEntries() } } + // An invalid pattern is recorded as a config error rather than thrown, because + // child plugins are constructed outside any try/catch in `Settings.updateRepos`. + compileExcludePatterns (exclude) { + return exclude.reduce((state, item) => { + if (!item || typeof item.name !== 'string') { + return state + } + + try { + // Lowercased to match the normalized property names. + state.patterns.push(new RegExp(item.name.toLowerCase())) + } catch (e) { + this.logError(`Invalid custom property exclude pattern "${item.name}": ${e.message || e}. Excluding all custom properties for this repo so no values are cleared.`) + state.excludeAll = true + } + + return state + }, { patterns: [], excludeAll: false }) + } + + isExcluded (name) { + if (this.excludeAll) { + return true + } + + return typeof name === 'string' && this.exclude.some(rx => rx.test(name)) + } + // Force all names to lowercase to avoid comparison issues. normalizeEntries () { this.entries = this.entries.reduce((normalizedEntries, entry) => { @@ -90,6 +150,10 @@ module.exports = class CustomProperties extends Diffable { // Custom Properties on repository does not support deletion, so we set the value to null async remove ({ name }) { + if (this.isExcluded(name)) { + this.log.debug(`Custom Property "${name}" matches an exclude pattern; leaving its value untouched`) + return Promise.resolve([]) + } return this.modifyProperty('Delete', { name, value: null }) } diff --git a/schema/dereferenced/repos.json b/schema/dereferenced/repos.json index 9213456a0..4814fb41c 100644 --- a/schema/dereferenced/repos.json +++ b/schema/dereferenced/repos.json @@ -778,19 +778,69 @@ }, "custom_properties": { "description": "Custom properties", - "type": "array", - "items": { - "description": "A custom property entry", - "type": "object", - "properties": { - "name": { - "type": "string" - }, - "value": { - "type": "string" + "oneOf": [ + { + "type": "array", + "items": { + "description": "A custom property entry", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "value": { + "type": "string" + } + } } + }, + { + "type": "object", + "properties": { + "include": { + "description": "Custom properties managed by safe-settings", + "type": "array", + "items": { + "description": "A custom property entry", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "value": { + "type": "string" + } + } + } + }, + "exclude": { + "description": "Never clear the value of any custom property whose name matches one of these regexes", + "type": "array", + "items": { + "description": "A regex, matched against the lowercased custom property name, identifying properties safe-settings must not clear", + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + } + } + }, + "anyOf": [ + { + "required": [ + "include" + ] + }, + { + "required": [ + "exclude" + ] + } + ] } - } + ] }, "variables": { "description": "Repository or org-level Actions variables", diff --git a/schema/dereferenced/settings.json b/schema/dereferenced/settings.json index 4dcdf0eb6..25b483109 100644 --- a/schema/dereferenced/settings.json +++ b/schema/dereferenced/settings.json @@ -1956,19 +1956,69 @@ }, "custom_properties": { "description": "Custom properties", - "type": "array", - "items": { - "description": "A custom property entry", - "type": "object", - "properties": { - "name": { - "type": "string" - }, - "value": { - "type": "string" + "oneOf": [ + { + "type": "array", + "items": { + "description": "A custom property entry", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "value": { + "type": "string" + } + } } + }, + { + "type": "object", + "properties": { + "include": { + "description": "Custom properties managed by safe-settings", + "type": "array", + "items": { + "description": "A custom property entry", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "value": { + "type": "string" + } + } + } + }, + "exclude": { + "description": "Never clear the value of any custom property whose name matches one of these regexes", + "type": "array", + "items": { + "description": "A regex, matched against the lowercased custom property name, identifying properties safe-settings must not clear", + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + } + } + }, + "anyOf": [ + { + "required": [ + "include" + ] + }, + { + "required": [ + "exclude" + ] + } + ] } - } + ] }, "variables": { "description": "Repository or org-level Actions variables", diff --git a/schema/dereferenced/suborgs.json b/schema/dereferenced/suborgs.json index 0267bf7a8..56545572f 100644 --- a/schema/dereferenced/suborgs.json +++ b/schema/dereferenced/suborgs.json @@ -812,19 +812,69 @@ }, "custom_properties": { "description": "Custom properties", - "type": "array", - "items": { - "description": "A custom property entry", - "type": "object", - "properties": { - "name": { - "type": "string" - }, - "value": { - "type": "string" + "oneOf": [ + { + "type": "array", + "items": { + "description": "A custom property entry", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "value": { + "type": "string" + } + } } + }, + { + "type": "object", + "properties": { + "include": { + "description": "Custom properties managed by safe-settings", + "type": "array", + "items": { + "description": "A custom property entry", + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "value": { + "type": "string" + } + } + } + }, + "exclude": { + "description": "Never clear the value of any custom property whose name matches one of these regexes", + "type": "array", + "items": { + "description": "A regex, matched against the lowercased custom property name, identifying properties safe-settings must not clear", + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + } + } + }, + "anyOf": [ + { + "required": [ + "include" + ] + }, + { + "required": [ + "exclude" + ] + } + ] } - } + ] }, "variables": { "description": "Repository or org-level Actions variables", diff --git a/schema/repos.json b/schema/repos.json index 3a7c51301..159ffb096 100644 --- a/schema/repos.json +++ b/schema/repos.json @@ -57,10 +57,37 @@ }, "custom_properties": { "description": "Custom properties", - "type": "array", - "items": { - "$ref": "#/$defs/CustomPropertiesSettings" - } + "oneOf": [ + { + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesSettings" + } + }, + { + "type": "object", + "properties": { + "include": { + "description": "Custom properties managed by safe-settings", + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesSettings" + } + }, + "exclude": { + "description": "Never clear the value of any custom property whose name matches one of these regexes", + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesExcludeSettings" + } + } + }, + "anyOf": [ + { "required": ["include"] }, + { "required": ["exclude"] } + ] + } + ] }, "variables": { "description": "Repository or org-level Actions variables", @@ -300,6 +327,15 @@ } } }, + "CustomPropertiesExcludeSettings": { + "description": "A regex, matched against the lowercased custom property name, identifying properties safe-settings must not clear", + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, "VariablesSettings": { "description": "An Actions variable entry", "type": "object", diff --git a/schema/settings.json b/schema/settings.json index 59d662d50..8df78bb59 100644 --- a/schema/settings.json +++ b/schema/settings.json @@ -64,10 +64,37 @@ }, "custom_properties": { "description": "Custom properties", - "type": "array", - "items": { - "$ref": "#/$defs/CustomPropertiesSettings" - } + "oneOf": [ + { + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesSettings" + } + }, + { + "type": "object", + "properties": { + "include": { + "description": "Custom properties managed by safe-settings", + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesSettings" + } + }, + "exclude": { + "description": "Never clear the value of any custom property whose name matches one of these regexes", + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesExcludeSettings" + } + } + }, + "anyOf": [ + { "required": ["include"] }, + { "required": ["exclude"] } + ] + } + ] }, "variables": { "description": "Repository or org-level Actions variables", @@ -307,6 +334,15 @@ } } }, + "CustomPropertiesExcludeSettings": { + "description": "A regex, matched against the lowercased custom property name, identifying properties safe-settings must not clear", + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, "VariablesSettings": { "description": "An Actions variable entry", "type": "object", diff --git a/schema/suborgs.json b/schema/suborgs.json index 3a3c79def..4d7a90f7c 100644 --- a/schema/suborgs.json +++ b/schema/suborgs.json @@ -91,10 +91,37 @@ }, "custom_properties": { "description": "Custom properties", - "type": "array", - "items": { - "$ref": "#/$defs/CustomPropertiesSettings" - } + "oneOf": [ + { + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesSettings" + } + }, + { + "type": "object", + "properties": { + "include": { + "description": "Custom properties managed by safe-settings", + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesSettings" + } + }, + "exclude": { + "description": "Never clear the value of any custom property whose name matches one of these regexes", + "type": "array", + "items": { + "$ref": "#/$defs/CustomPropertiesExcludeSettings" + } + } + }, + "anyOf": [ + { "required": ["include"] }, + { "required": ["exclude"] } + ] + } + ] }, "variables": { "description": "Repository or org-level Actions variables", @@ -334,6 +361,15 @@ } } }, + "CustomPropertiesExcludeSettings": { + "description": "A regex, matched against the lowercased custom property name, identifying properties safe-settings must not clear", + "type": "object", + "properties": { + "name": { + "type": "string" + } + } + }, "VariablesSettings": { "description": "An Actions variable entry", "type": "object", diff --git a/test/unit/lib/plugins/custom_properties.test.js b/test/unit/lib/plugins/custom_properties.test.js index a9e878d58..fbc1e208d 100644 --- a/test/unit/lib/plugins/custom_properties.test.js +++ b/test/unit/lib/plugins/custom_properties.test.js @@ -1,29 +1,42 @@ const CustomProperties = require('../../../../lib/plugins/custom_properties') +const MergeDeep = require('../../../../lib/mergeDeep') describe('CustomProperties', () => { const nop = false let github let log + let errors const owner = 'test-owner' const repo = 'test-repo' function configure (config) { - return new CustomProperties(nop, github, { owner, repo }, config, log, []) + return new CustomProperties(nop, github, { owner, repo }, config, log, errors) + } + + function configureNop (config) { + return new CustomProperties(true, github, { owner, repo }, config, log, errors) } beforeEach(() => { + const createOrUpdateCustomPropertiesValues = jest.fn() + createOrUpdateCustomPropertiesValues.endpoint = jest.fn(params => ({ + url: `/repos/${params.owner}/${params.repo}/properties/values`, + body: params + })) + github = { paginate: jest.fn(), rest: { repos: { getCustomPropertiesValues: jest.fn(), - createOrUpdateCustomPropertiesValues: jest.fn() + createOrUpdateCustomPropertiesValues } } } - log = { debug: jest.fn(), error: console.error } + log = { debug: jest.fn(), error: jest.fn() } + errors = [] }) describe('Custom Properties plugin', () => { @@ -164,4 +177,468 @@ describe('CustomProperties', () => { // }) }) }) + + describe('include/exclude config shape', () => { + // Existing repo state shared by most of the exclude tests: one property + // managed by safe-settings, one owned by another app, one abandoned. + function mockExistingProperties (properties) { + github.paginate.mockResolvedValue(properties) + } + + function propertyUpdate (name, value) { + return { + owner, + repo, + properties: [{ property_name: name, value }] + } + } + + it('keeps the plain array shape working unchanged', () => { + mockExistingProperties([ + { property_name: 'jira-team', value: 'Search' }, + { property_name: 'stale-prop', value: 'whatever' } + ]) + + const plugin = configure([ + { name: 'jira-team', value: 'Platform' }, + { name: 'jira-project', value: 'ARCH' } + ]) + + expect(plugin.exclude).toEqual([]) + + return plugin.sync().then(() => { + // update + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .toHaveBeenCalledWith(propertyUpdate('jira-team', 'Platform')) + // add + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .toHaveBeenCalledWith(propertyUpdate('jira-project', 'ARCH')) + // remove + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .toHaveBeenCalledWith(propertyUpdate('stale-prop', null)) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledTimes(3) + }) + }) + + it('does not null out an unmanaged property matching an exclude pattern', () => { + mockExistingProperties([ + { property_name: 'jira-team', value: 'Search' }, + { property_name: 'app-deploy-ring', value: 'canary' } + ]) + + const plugin = configure({ + include: [ + { name: 'jira-team', value: 'Platform' }, + { name: 'jira-project', value: 'ARCH' } + ], + exclude: [ + { name: '^app-.*' } + ] + }) + + return plugin.sync().then(() => { + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .not.toHaveBeenCalledWith(propertyUpdate('app-deploy-ring', null)) + // Managed properties are still enforced. + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .toHaveBeenCalledWith(propertyUpdate('jira-team', 'Platform')) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .toHaveBeenCalledWith(propertyUpdate('jira-project', 'ARCH')) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledTimes(2) + }) + }) + + it('enforces a property that is in include even when it matches exclude', () => { + mockExistingProperties([ + { property_name: 'app-owner', value: 'unknown' } + ]) + + const plugin = configure({ + include: [ + { name: 'app-owner', value: 'platform-team' } + ], + exclude: [ + { name: '^app-.*' } + ] + }) + + return plugin.sync().then(() => { + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .toHaveBeenCalledWith(propertyUpdate('app-owner', 'platform-team')) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledTimes(1) + }) + }) + + it('still removes an unmanaged property that matches no exclude pattern', () => { + mockExistingProperties([ + { property_name: 'app-deploy-ring', value: 'canary' }, + { property_name: 'stale-prop', value: 'leftover' } + ]) + + const plugin = configure({ + include: [ + { name: 'jira-team', value: 'Platform' } + ], + exclude: [ + { name: '^app-.*' } + ] + }) + + return plugin.sync().then(() => { + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .toHaveBeenCalledWith(propertyUpdate('stale-prop', null)) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .not.toHaveBeenCalledWith(propertyUpdate('app-deploy-ring', null)) + }) + }) + + it('protects properties whose API casing differs from the pattern', () => { + // The API may return any casing; the plugin normalizes to lowercase, so + // exclude patterns are matched against the lowercased name. + mockExistingProperties([ + { property_name: 'APP-Thing', value: 'set-by-an-app' } + ]) + + const plugin = configure({ + include: [], + exclude: [{ name: '^app-.*' }] + }) + + return plugin.sync().then(() => { + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).not.toHaveBeenCalled() + }) + }) + + it('produces no Delete NopCommand for an excluded property in nop mode', () => { + mockExistingProperties([ + { property_name: 'app-deploy-ring', value: 'canary' }, + { property_name: 'stale-prop', value: 'leftover' } + ]) + + const plugin = configureNop({ + include: [{ name: 'jira-team', value: 'Platform' }], + exclude: [{ name: '^app-.*' }] + }) + + return plugin.sync().then(res => { + const commands = res.flat().filter(c => c && c.action) + const deletes = commands.filter(c => c.action.msg === 'Delete Custom Property') + + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).not.toHaveBeenCalled() + expect(deletes).toHaveLength(1) + expect(deletes[0].body.properties).toEqual([ + { property_name: 'stale-prop', value: null } + ]) + }) + }) + + describe('degenerate shapes', () => { + it('accepts include without exclude', () => { + const plugin = configure({ + include: [{ name: 'Jira-Team', value: 'Platform' }] + }) + + expect(plugin.entries).toEqual([{ name: 'jira-team', value: 'Platform' }]) + expect(plugin.exclude).toEqual([]) + }) + + it('accepts exclude without include, managing nothing', () => { + mockExistingProperties([ + { property_name: 'app-thing', value: 'a' }, + { property_name: 'other-thing', value: 'b' } + ]) + + const plugin = configure({ + exclude: [{ name: '^app-.*' }] + }) + + expect(plugin.entries).toEqual([]) + + // An empty include list means safe-settings owns the whole surface, so + // everything not protected by `exclude` is still nulled out. + return plugin.sync().then(() => { + expect(github.rest.repos.createOrUpdateCustomPropertiesValues) + .toHaveBeenCalledWith(propertyUpdate('other-thing', null)) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledTimes(1) + }) + }) + + it('does not throw on a null config', () => { + let plugin + expect(() => { plugin = configure(null) }).not.toThrow() + expect(plugin.entries).toBeNull() + expect(plugin.exclude).toEqual([]) + expect(plugin.sync()).toBeUndefined() + }) + + it('does not throw on an undefined config', () => { + let plugin + expect(() => { plugin = configure(undefined) }).not.toThrow() + expect(plugin.exclude).toEqual([]) + }) + + it('ignores exclude entries without a name string', () => { + const plugin = configure({ + include: [{ name: 'jira-team', value: 'Platform' }], + exclude: [{ name: '^app-.*' }, {}, null, { value: 'nope' }, { name: 42 }] + }) + + expect(plugin.exclude).toHaveLength(1) + expect(plugin.isExcluded('app-thing')).toBe(true) + }) + + it('ignores a non-array exclude', () => { + const plugin = configure({ + include: [{ name: 'jira-team', value: 'Platform' }], + exclude: 'app-' + }) + + expect(plugin.exclude).toEqual([]) + expect(plugin.entries).toEqual([{ name: 'jira-team', value: 'Platform' }]) + }) + }) + + describe('malformed object config', () => { + it('does not throw on an empty object', () => { + expect(() => configure({})).not.toThrow() + }) + + it('fails closed and reports a config error for an empty object', () => { + const plugin = configure({}) + + expect(plugin.entries).toEqual([]) + expect(plugin.excludeAll).toBe(true) + expect(errors).toHaveLength(1) + expect(errors[0].msg).toMatch(/must be a list of properties or an object with `include`/) + }) + + it('clears nothing when the config is an empty object', async () => { + github.paginate.mockResolvedValue([{ property_name: 'deploy-status', value: 'green' }]) + + const plugin = configure({}) + await plugin.sync() + + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).not.toHaveBeenCalled() + }) + + it('fails closed when include is present but not an array', async () => { + github.paginate.mockResolvedValue([{ property_name: 'deploy-status', value: 'green' }]) + + const plugin = configure({ include: 'not-a-list' }) + await plugin.sync() + + expect(plugin.excludeAll).toBe(true) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).not.toHaveBeenCalled() + }) + }) + + describe('pattern casing', () => { + it('matches an uppercase pattern against the lowercased property name', () => { + const plugin = configure({ include: [], exclude: [{ name: '^Deploy-Status$' }] }) + + expect(plugin.isExcluded('deploy-status')).toBe(true) + }) + + it('does not clear a property whose exclude pattern was written in uppercase', async () => { + github.paginate.mockResolvedValue([{ property_name: 'Deploy-Status', value: 'green' }]) + + const plugin = configure({ include: [], exclude: [{ name: '^Deploy-' }] }) + await plugin.sync() + + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).not.toHaveBeenCalled() + }) + }) + + describe('invalid exclude patterns', () => { + it('records a config error instead of throwing', () => { + let plugin + expect(() => { + plugin = configure({ + include: [{ name: 'jira-team', value: 'Platform' }], + exclude: [{ name: '^app-.*' }, { name: '[unterminated' }] + }) + }).not.toThrow() + + expect(plugin.exclude).toHaveLength(1) + expect(plugin.isExcluded('app-thing')).toBe(true) + + expect(errors).toHaveLength(1) + expect(errors[0]).toMatchObject({ + owner, + repo, + plugin: 'CustomProperties' + }) + expect(errors[0].msg).toMatch(/Invalid custom property exclude pattern "\[unterminated"/) + }) + + it('fails closed - an invalid pattern excludes every property', () => { + const plugin = configure({ include: [], exclude: [{ name: '[unterminated' }] }) + + expect(plugin.excludeAll).toBe(true) + expect(plugin.isExcluded('anything-at-all')).toBe(true) + }) + + it('clears nothing when a glob "*" is used instead of the regex ".*"', async () => { + github.paginate.mockResolvedValue([ + { property_name: 'deploy-status', value: 'green' }, + { property_name: 'cost-center', value: '4417' } + ]) + + const plugin = configure({ include: [], exclude: [{ name: '*' }] }) + await plugin.sync() + + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).not.toHaveBeenCalled() + expect(errors).toHaveLength(1) + }) + + it('still enforces included properties when an invalid pattern fails closed', async () => { + github.paginate.mockResolvedValue([ + { property_name: 'jira-team', value: 'OldTeam' }, + { property_name: 'deploy-status', value: 'green' } + ]) + + const plugin = configure({ + include: [{ name: 'jira-team', value: 'Platform' }], + exclude: [{ name: '*' }] + }) + await plugin.sync() + + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledTimes(1) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledWith( + expect.objectContaining({ + properties: [{ property_name: 'jira-team', value: 'Platform' }] + }) + ) + }) + }) + + // `Settings.childPluginsList` merges the org, suborg and repo configs with + // MergeDeep before handing the result to the plugin, so the config the plugin + // actually receives is not the config any single file declares. These use the + // real MergeDeep to assert against that merged shape. + describe('config merged across org, suborg and repo scopes', () => { + function mergeScopes (...scopes) { + const mergeDeep = new MergeDeep(log, github, ['id', 'node_id', 'default', 'url']) + const sources = scopes.map(scope => ({ custom_properties: scope })) + return mergeDeep.mergeDeep({}, ...sources).custom_properties + } + + it('leaves a plain array config untouched through the merge', () => { + const merged = mergeScopes( + [{ name: 'jira-team', value: 'Platform' }], + [{ name: 'jira-team', value: 'RepoTeam' }] + ) + + expect(Array.isArray(merged)).toBe(true) + expect(merged).toEqual([{ name: 'jira-team', value: 'RepoTeam' }]) + }) + + it('accumulates exclude patterns from every scope', () => { + const merged = mergeScopes( + { include: [{ name: 'jira-team', value: 'Platform' }], exclude: [{ name: '^app-' }] }, + { exclude: [{ name: '^deploy-' }] } + ) + + const plugin = configure(merged) + + expect(plugin.exclude).toHaveLength(2) + expect(plugin.isExcluded('app-owner')).toBe(true) + expect(plugin.isExcluded('deploy-status')).toBe(true) + expect(plugin.isExcluded('jira-team')).toBe(false) + }) + + it('lets a narrower scope override an included value while keeping org excludes', async () => { + github.paginate.mockResolvedValue([ + { property_name: 'jira-team', value: 'OldTeam' }, + { property_name: 'app-owner', value: 'team-a' } + ]) + + const merged = mergeScopes( + { include: [{ name: 'jira-team', value: 'Platform' }], exclude: [{ name: '^app-' }] }, + { include: [{ name: 'jira-team', value: 'RepoTeam' }] } + ) + + const plugin = configure(merged) + await plugin.sync() + + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledTimes(1) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledWith( + expect.objectContaining({ + properties: [{ property_name: 'jira-team', value: 'RepoTeam' }] + }) + ) + }) + + it('applies includes contributed by different scopes together', async () => { + github.paginate.mockResolvedValue([]) + + const merged = mergeScopes( + { include: [{ name: 'jira-team', value: 'Platform' }], exclude: [{ name: '.*' }] }, + { include: [{ name: 'tier', value: 'gold' }] } + ) + + const plugin = configure(merged) + await plugin.sync() + + const written = github.rest.repos.createOrUpdateCustomPropertiesValues.mock.calls + .map(call => call[0].properties[0]) + + expect(written).toEqual( + expect.arrayContaining([ + { property_name: 'jira-team', value: 'Platform' }, + { property_name: 'tier', value: 'gold' } + ]) + ) + }) + + it('keeps an org-level exclude protecting properties when a repo adds an include', async () => { + github.paginate.mockResolvedValue([ + { property_name: 'deploy-status', value: 'green' }, + { property_name: 'cost-center', value: '4417' } + ]) + + const merged = mergeScopes( + { exclude: [{ name: '.*' }] }, + { include: [{ name: 'tier', value: 'gold' }] } + ) + + const plugin = configure(merged) + await plugin.sync() + + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledTimes(1) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).toHaveBeenCalledWith( + expect.objectContaining({ + properties: [{ property_name: 'tier', value: 'gold' }] + }) + ) + }) + + // Mixing the two shapes across scopes merges the array into the object under + // numeric keys. The same happens for `labels`, which shares this array-or-object + // config duality. The plugin must not throw on it, and the declared + // include/exclude must still be honoured. + it('does not throw when scopes mix the array and object shapes', async () => { + github.paginate.mockResolvedValue([ + { property_name: 'app-owner', value: 'team-a' } + ]) + + const merged = mergeScopes( + [{ name: 'jira-team', value: 'Platform' }], + { include: [{ name: 'tier', value: 'gold' }], exclude: [{ name: '^app-' }] } + ) + + let plugin + expect(() => { plugin = configure(merged) }).not.toThrow() + + await expect(plugin.sync()).resolves.not.toThrow() + + expect(plugin.isExcluded('app-owner')).toBe(true) + expect(github.rest.repos.createOrUpdateCustomPropertiesValues).not.toHaveBeenCalledWith( + expect.objectContaining({ + properties: [{ property_name: 'app-owner', value: null }] + }) + ) + }) + }) + }) })