Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 44 additions & 6 deletions lib/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ ${this.results.reduce((x, y) => {
}
}

async updateRepos (repo) {
async updateRepos (repo, archived) {
this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs()
// 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
Expand Down Expand Up @@ -362,14 +362,31 @@ ${this.results.reduce((x, y) => {
if (overrideRepoConfig) {
repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, overrideRepoConfig)
}

const archivePlugin = new Archive(this.nop, this.github, repo, repoConfig, this.log)

// Archived repos get skipped further down, but only after
// archivePlugin.getState() has spent a repos.get on each one. When the caller
// already knows — eachRepositoryRepos gets `archived` for free from
// GET /installation/repositories — skip before issuing any request. On orgs
// where most repos are archived this is where the bulk of a full sync's
// rate-limit budget goes.
//
// An explicit `archived: false` in config is a request to unarchive and must
// still be processed, so the desired state decides. getDesiredArchiveState()
// reads config only and issues no request.
if (archived === true && archivePlugin.getDesiredArchiveState() !== false) {
this.log.debug(`Skipping archived repo ${repo.repo} without fetching it`)
return
}

if (repoConfig) {
try {
this.log.debug(`found a matching repoconfig for this repo ${JSON.stringify(repoConfig)}`)

const childPlugins = this.childPluginsList(repo)
const RepoPlugin = Settings.PLUGINS.repository

const archivePlugin = new Archive(this.nop, this.github, repo, repoConfig, this.log)
const { isArchived, shouldArchive, shouldUnarchive } = await archivePlugin.getState()

if (shouldUnarchive) {
Expand Down Expand Up @@ -410,6 +427,20 @@ ${this.results.reduce((x, y) => {
}
} else {
this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`)

// This branch has no repoConfig, so getState() was never reached and the
// isArchived guard above did not run — child plugins would still issue
// forbidden writes against an archived repo. Only verify when the archived
// state is not already known: `false` from the listing needs no request,
// and `true` already returned above unless an unarchive was requested.
if (archived !== false) {
const { isArchived, shouldUnarchive } = await archivePlugin.getState()
if (isArchived && !shouldUnarchive) {
this.log.debug(`Skipping child plugin updates for archived repo ${repo.repo}`)
return
}
}

const childPlugins = this.childPluginsList(repo)
return Promise.all(childPlugins.map(([Plugin, config]) => {
return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync().then(res => {
Expand Down Expand Up @@ -540,18 +571,25 @@ ${this.results.reduce((x, y) => {
log.debug('Fetching repositories')
return github.paginate('GET /installation/repositories').then(repositories => {
return Promise.all(repositories.map(repository => {
const { owner, name } = repository
return this.checkAndProcessRepo(owner.login, name)
// `archived` is already part of the listing payload, so passing it down
// lets updateRepos skip archived repos without spending an API call.
const { owner, name, archived } = repository
return this.checkAndProcessRepo(owner.login, name, archived)
})
)
})
}

async checkAndProcessRepo (owner, name) {
async checkAndProcessRepo (owner, name, archived) {
if (this.isRestricted(name)) {
return null
}
return this.updateRepos({ owner, repo: name })
// `archived` travels as its own argument, never merged into the repo ref: the
// Repository plugin does Object.assign({}, settings, repo) and later
// repos.update(settings), so an `archived` key on the ref would land in the
// update payload — and would re-archive a repo in the middle of unarchiving
// it. Plugins must keep receiving a bare { owner, repo }.
return this.updateRepos({ owner, repo: name }, archived)
}

/**
Expand Down
82 changes: 82 additions & 0 deletions test/unit/lib/settings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,88 @@ repository:
})
})
}) // repoOverrideConfig

describe('updateRepos with a known-archived repo', () => {
let settings

beforeEach(() => {
stubConfig = { repository: { has_wiki: true }, restrictedRepos: { exclude: [] } }
// Built without a suborg on purpose: passing one sets subOrgConfigMap, and
// updateRepos then returns early for any repo outside that suborg.
settings = new Settings(false, stubContext, mockRepo, stubConfig, mockRef)
settings.subOrgConfigs = {}
settings.repoConfigs = {}
// repos.get is what archivePlugin.getState() calls. Asserting on it proves
// whether the archived repo was skipped before any request was made.
settings.github.rest.repos.get = jest.fn().mockResolvedValue({ data: { archived: true } })
settings.github.rest.repos.update = jest.fn().mockResolvedValue({ data: {} })
})

it('Skips without fetching the repo when the caller reports it archived', async () => {
await settings.updateRepos({ owner: 'test', repo: 'archived-repo' }, true)
expect(settings.github.rest.repos.get).not.toHaveBeenCalled()
})

it('Still processes the repo when config asks to unarchive it', async () => {
settings.config.repository.archived = false
await settings.updateRepos({ owner: 'test', repo: 'archived-repo' }, true)
expect(settings.github.rest.repos.get).toHaveBeenCalled()
})

it('Still processes the repo when the caller does not report archived state', async () => {
await settings.updateRepos({ owner: 'test', repo: 'some-repo' })
expect(settings.github.rest.repos.get).toHaveBeenCalled()
})

it('Guards the no-repoConfig path too, so child plugins do not write to an archived repo', async () => {
// Label-only config: repoConfig is absent, so getState() is never reached
// by the main branch and the child plugins would run unguarded.
settings.config = { restrictedRepos: { exclude: [] }, labels: [{ name: 'bug' }] }
// Realistic stub: the labels plugin builds options via
// listLabelsForRepo.endpoint.merge and then calls github.paginate. Stubbing
// both means the plugin would run cleanly if it were reached, so the
// assertion below fails loudly instead of passing by accident.
settings.github.rest.issues = {
listLabelsForRepo: Object.assign(jest.fn(), { endpoint: { merge: jest.fn(() => ({})) } })
}
settings.github.paginate = jest.fn().mockResolvedValue([])

await settings.updateRepos({ owner: 'test', repo: 'archived-repo' })

expect(settings.github.rest.repos.get).toHaveBeenCalled()
expect(settings.github.paginate).not.toHaveBeenCalled()
})

it('Does not leak archived onto the repo ref handed to plugins', async () => {
// The Repository plugin does Object.assign({}, settings, repo) and later
// repos.update(settings): an `archived` key on the ref would land in the
// update payload and re-archive the repo mid-unarchive.
settings.github.paginate = jest.fn().mockResolvedValue([
{ name: 'archived-repo', archived: true, owner: { login: 'test' } }
])
const seen = []
settings.updateRepos = jest.fn(async (repo, archived) => { seen.push({ repo, archived }) })

await settings.eachRepositoryRepos(settings.github, settings.log)

expect(seen).toEqual([{ repo: { owner: 'test', repo: 'archived-repo' }, archived: true }])
expect(seen[0].repo).not.toHaveProperty('archived')
})

it('Passes the archived flag from the repository listing as a separate argument', async () => {
settings.github.paginate = jest.fn().mockResolvedValue([
{ name: 'active-repo', archived: false, owner: { login: 'test' } },
{ name: 'archived-repo', archived: true, owner: { login: 'test' } }
])
const seen = []
settings.updateRepos = jest.fn(async (repo, archived) => { seen.push([repo.repo, archived]) })

await settings.eachRepositoryRepos(settings.github, settings.log)

expect(seen).toEqual([['active-repo', false], ['archived-repo', true]])
})
}) // updateRepos with a known-archived repo

describe('loadConfigs', () => {
describe('load suborg configs', () => {
beforeEach(() => {
Expand Down