diff --git a/lib/settings.js b/lib/settings.js index fd7ca2693..2f73b55b3 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -314,7 +314,7 @@ ${this.results.reduce((x, y) => { } async loadConfigs (repo) { - this.subOrgConfigs = await this.getSubOrgConfigs() + this.subOrgConfigs = await this.getSubOrgConfigs(repo) this.repoConfigs = await this.getRepoConfigs(repo) } @@ -329,7 +329,7 @@ ${this.results.reduce((x, y) => { } async updateRepos (repo) { - this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs() + this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs(repo) // Create a new object to avoid mutating the shared this.config.repository // This prevents race conditions when multiple repos are processed concurrently via Promise.all let repoConfig = this.config.repository @@ -753,13 +753,21 @@ ${this.results.reduce((x, y) => { * @param params Params to fetch the file with * @return The parsed YAML file */ - async getSubOrgConfigs () { + async getSubOrgConfigs (repo) { try { // Get all suborg configs even though we might be here becuase of a suborg config change // we will filter them out if request is due to a suborg config change const overridePaths = await this.getSubOrgConfigMap() const subOrgConfigs = {} + // When syncing a single repo (and not processing a suborg config change), + // resolve suborg membership by inspecting only this repo's teams/custom + // properties instead of enumerating every repo of every suborg across the + // whole org. See getSubOrgConfigsForRepo. + if (repo && !this.subOrgConfigMap) { + return await this.getSubOrgConfigsForRepo(repo, overridePaths || []) + } + for (const override of overridePaths) { const data = await this.loadYaml(override.path) this.log.debug(`data = ${JSON.stringify(data)}`) @@ -830,6 +838,92 @@ ${this.results.reduce((x, y) => { } } + /** + * Repo-scoped variant of suborg config resolution. + * + * Instead of expanding every suborg's `suborgteams`/`suborgproperties` into the + * full set of matching repos across the org (many paginated, org-wide API calls), + * this inspects only the repo being synced: it fetches the repo's own teams and + * custom property values at most once each (and only if some suborg config needs + * them), then matches locally. Result: a single-repo event costs at most one + * `GET /repos/{owner}/{repo}/teams` and one `GET /repos/{owner}/{repo}/properties/values` + * regardless of how many suborgs/teams/properties are defined. + * + * @param {*} repo the repo being synced ({ owner, repo }) + * @param {*} overridePaths list of suborg config files ({ name, path }) + * @returns subOrgConfigs keyed by repo name for the suborg (if any) this repo belongs to + */ + async getSubOrgConfigsForRepo (repo, overridePaths) { + const subOrgConfigs = {} + const repoName = repo.repo + + // Lazily fetched and cached for this repo; only queried when a suborg config + // actually references teams/properties. + let repoTeamSlugs + let repoProperties + + for (const override of overridePaths) { + const data = await this.loadYaml(override.path) + this.log.debug(`data = ${JSON.stringify(data)}`) + if (!data) { continue } + + let matched = false + + if (data.suborgrepos) { + matched = data.suborgrepos.some(pattern => new Glob(pattern).test(repoName)) + } + + if (!matched && data.suborgteams) { + if (repoTeamSlugs === undefined) { + repoTeamSlugs = (await this.getReposTeams(repo)).map(team => team.slug) + } + matched = data.suborgteams.some(teamslug => repoTeamSlugs.includes(teamslug)) + } + + if (!matched && data.suborgproperties) { + if (repoProperties === undefined) { + repoProperties = await this.getRepoCustomPropertyValues(repo) + } + matched = this.repoMatchesProperties(repoProperties, data.suborgproperties) + } + + if (matched) { + this.storeSubOrgConfigIfNoConflicts(subOrgConfigs, override.path, repoName, data) + } + } + + return subOrgConfigs + } + + /** + * Returns true if the repo's custom property values satisfy any of the suborg + * property filters. Mirrors getRepositoriesByProperty by only considering the + * first key/value of each filter entry (e.g. `- EDP: true`). Values are compared + * as strings so YAML booleans/numbers match the API's string representation, and + * multi-select property values (arrays) match if they contain the expected value. + */ + repoMatchesProperties (repoProperties, subOrgProperties) { + // Property names are case-insensitive in GitHub (the org-wide `props.` search + // relied on this, and the custom_properties plugin lowercases names for the same + // reason), so normalize names to lowercase on both sides before comparing. + const propertyValues = new Map( + (repoProperties || []).map(property => [String(property.property_name).toLowerCase(), property.value]) + ) + return subOrgProperties.some(filter => { + const [name] = Object.keys(filter) + const expected = filter[name] + const key = String(name).toLowerCase() + if (!propertyValues.has(key)) { + return false + } + const actual = propertyValues.get(key) + if (Array.isArray(actual)) { + return actual.map(String).includes(String(expected)) + } + return String(actual) === String(expected) + }) + } + storeSubOrgConfigIfNoConflicts (subOrgConfigs, overridePath, repoName, data) { const existingConfigForRepo = subOrgConfigs[repoName] if (existingConfigForRepo && existingConfigForRepo.source !== overridePath) { @@ -933,6 +1027,27 @@ ${this.results.reduce((x, y) => { return this.github.paginate(options) } + // Repo-scoped inverse of getReposForTeam: the teams a single repo belongs to. + async getReposTeams (repo) { + const options = this.github.rest.repos.listTeams.endpoint.merge({ + owner: repo.owner, + repo: repo.repo, + per_page: 100 + }) + return this.github.paginate(options) + } + + // Repo-scoped inverse of getRepositoriesByProperty: the custom property values + // set on a single repo. Returns an array of { property_name, value }. + // Uses the same typed endpoint as the custom_properties plugin (getCustomPropertiesValues). + async getRepoCustomPropertyValues (repo) { + return this.github.paginate(this.github.rest.repos.getCustomPropertiesValues, { + owner: repo.owner, + repo: repo.repo, + per_page: 100 + }) + } + async getRepositoriesByProperty (organizationName, propertyFilter) { if (!organizationName || !propertyFilter) { throw new Error('Organization name and property filter are required') diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index e102379a6..f006c9eec 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -303,6 +303,139 @@ repository: // } }) }) + + describe('repo-scoped suborg config resolution', () => { + // When syncing a single repo (not a suborg config change), getSubOrgConfigs(repo) + // should resolve membership by inspecting only that repo's teams/properties, + // instead of enumerating every repo of every suborg across the org. + beforeEach(() => { + // No suborg => subOrgConfigMap is not set => repo-scoped path is eligible + mockSubOrg = undefined + stubConfig = { restrictedRepos: {} } + subOrgConfig = yaml.load(` + suborgrepos: + - new-repo + + suborgteams: + - core + + suborgproperties: + - EDP: true + - do_no_delete: true + + repository: + topics: + - frontend + `) + }) + + function createRepoScopedSettings () { + settings = createSettings(stubConfig) + jest.spyOn(settings, 'loadConfigMap').mockImplementation(() => [{ name: 'frontend', path: '.github/suborgs/frontend.yml' }]) + jest.spyOn(settings, 'loadYaml').mockImplementation(() => subOrgConfig) + // org-wide resolvers should NOT be used on the repo-scoped path + jest.spyOn(settings, 'getReposForTeam').mockResolvedValue([{ name: 'repo-test' }]) + jest.spyOn(settings, 'getSubOrgRepositories').mockResolvedValue([{ repository_name: 'repo-for-property' }]) + return settings + } + + it('matches by suborgrepos glob without any repo API calls', async () => { + settings = createRepoScopedSettings() + const getReposTeams = jest.spyOn(settings, 'getReposTeams').mockResolvedValue([]) + const getRepoProps = jest.spyOn(settings, 'getRepoCustomPropertyValues').mockResolvedValue([]) + + const subOrgConfigs = await settings.getSubOrgConfigs({ owner: 'test', repo: 'new-repo' }) + + expect(subOrgConfigs['new-repo']).toBeDefined() + expect(subOrgConfigs['new-repo'].source).toEqual('.github/suborgs/frontend.yml') + // glob matched first, so teams/properties are never queried + expect(getReposTeams).not.toHaveBeenCalled() + expect(getRepoProps).not.toHaveBeenCalled() + // org-wide resolvers are never used + expect(settings.getReposForTeam).not.toHaveBeenCalled() + expect(settings.getSubOrgRepositories).not.toHaveBeenCalled() + }) + + it('matches by team membership using the repo-scoped teams endpoint', async () => { + settings = createRepoScopedSettings() + const getReposTeams = jest.spyOn(settings, 'getReposTeams').mockResolvedValue([{ slug: 'core' }]) + const getRepoProps = jest.spyOn(settings, 'getRepoCustomPropertyValues').mockResolvedValue([]) + + const subOrgConfigs = await settings.getSubOrgConfigs({ owner: 'test', repo: 'some-repo' }) + + expect(subOrgConfigs['some-repo']).toBeDefined() + expect(getReposTeams).toHaveBeenCalledTimes(1) + // team matched, so properties are never queried + expect(getRepoProps).not.toHaveBeenCalled() + expect(settings.getReposForTeam).not.toHaveBeenCalled() + }) + + it('matches by custom property using the repo-scoped properties endpoint', async () => { + settings = createRepoScopedSettings() + const getReposTeams = jest.spyOn(settings, 'getReposTeams').mockResolvedValue([]) + const getRepoProps = jest.spyOn(settings, 'getRepoCustomPropertyValues').mockResolvedValue([{ property_name: 'EDP', value: 'true' }]) + + const subOrgConfigs = await settings.getSubOrgConfigs({ owner: 'test', repo: 'some-repo' }) + + expect(subOrgConfigs['some-repo']).toBeDefined() + expect(getReposTeams).toHaveBeenCalledTimes(1) + expect(getRepoProps).toHaveBeenCalledTimes(1) + expect(settings.getSubOrgRepositories).not.toHaveBeenCalled() + }) + + it('returns no config when the repo matches nothing', async () => { + settings = createRepoScopedSettings() + jest.spyOn(settings, 'getReposTeams').mockResolvedValue([{ slug: 'other-team' }]) + jest.spyOn(settings, 'getRepoCustomPropertyValues').mockResolvedValue([{ property_name: 'EDP', value: 'false' }]) + + const subOrgConfigs = await settings.getSubOrgConfigs({ owner: 'test', repo: 'some-repo' }) + + expect(Object.keys(subOrgConfigs)).toHaveLength(0) + }) + + it('falls back to the org-wide path when processing a suborg config change', async () => { + settings = createRepoScopedSettings() + settings.subOrgConfigMap = [{ path: '.github/suborgs/frontend.yml' }] + const getReposTeams = jest.spyOn(settings, 'getReposTeams').mockResolvedValue([]) + const getRepoProps = jest.spyOn(settings, 'getRepoCustomPropertyValues').mockResolvedValue([]) + + await settings.getSubOrgConfigs({ owner: 'test', repo: 'new-repo' }) + + // org-wide resolvers are used; repo-scoped ones are not + expect(settings.getReposForTeam).toHaveBeenCalled() + expect(settings.getSubOrgRepositories).toHaveBeenCalled() + expect(getReposTeams).not.toHaveBeenCalled() + expect(getRepoProps).not.toHaveBeenCalled() + }) + }) + + describe('repoMatchesProperties', () => { + beforeEach(() => { + mockSubOrg = undefined + settings = createSettings({ restrictedRepos: {} }) + }) + + it('coerces YAML booleans/numbers to match the API string values', () => { + expect(settings.repoMatchesProperties([{ property_name: 'EDP', value: 'true' }], [{ EDP: true }])).toBe(true) + expect(settings.repoMatchesProperties([{ property_name: 'tier', value: '2' }], [{ tier: 2 }])).toBe(true) + }) + + it('matches property names case-insensitively', () => { + // GitHub may return the property name in a different case than the config declares + expect(settings.repoMatchesProperties([{ property_name: 'edp', value: 'true' }], [{ EDP: true }])).toBe(true) + expect(settings.repoMatchesProperties([{ property_name: 'EDP', value: 'true' }], [{ edp: true }])).toBe(true) + }) + + it('returns false when the property is absent or the value differs', () => { + expect(settings.repoMatchesProperties([{ property_name: 'EDP', value: 'false' }], [{ EDP: true }])).toBe(false) + expect(settings.repoMatchesProperties([], [{ EDP: true }])).toBe(false) + }) + + it('matches multi-select property values that contain the expected value', () => { + expect(settings.repoMatchesProperties([{ property_name: 'envs', value: ['dev', 'prod'] }], [{ envs: 'prod' }])).toBe(true) + expect(settings.repoMatchesProperties([{ property_name: 'envs', value: ['dev'] }], [{ envs: 'prod' }])).toBe(false) + }) + }) }) // loadConfigs describe('loadYaml', () => {