Add better handling for suborgproperties and suborgteams

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')

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', () => {