diff --git a/lib/entitlements/backend/github_team/controller.rb b/lib/entitlements/backend/github_team/controller.rb index c8402e4..97751cf 100644 --- a/lib/entitlements/backend/github_team/controller.rb +++ b/lib/entitlements/backend/github_team/controller.rb @@ -26,10 +26,8 @@ def initialize(group_name, config = nil) def prefetch teams = Entitlements::Data::Groups::Calculated.read_all(group_name, config) - teams.each do |team_slug| - entitlement_group = Entitlements::Data::Groups::Calculated.read(team_slug) - provider.read(entitlement_group) - end + entitlement_groups = teams.map { |team_slug| Entitlements::Data::Groups::Calculated.read(team_slug) } + provider.prefetch(entitlement_groups) end # Calculation routines. diff --git a/lib/entitlements/backend/github_team/provider.rb b/lib/entitlements/backend/github_team/provider.rb index f21841c..a3a9a9f 100644 --- a/lib/entitlements/backend/github_team/provider.rb +++ b/lib/entitlements/backend/github_team/provider.rb @@ -30,6 +30,20 @@ def initialize(config:) @github_team_cache = {} end + # Populate the provider cache for a collection of desired teams. + # + # entitlement_groups - Array of Entitlements::Models::Group objects. + # + # Returns nothing. + Contract C::ArrayOf[Entitlements::Models::Group] => nil + def prefetch(entitlement_groups) + github.read_teams(entitlement_groups).each do |team_name, github_team| + log_loaded_team(github_team) if github_team + @github_team_cache[team_name] = github_team + end + nil + end + # Read in a specific GitHub.com Team and enumerate its members. Results are cached # for future runs. # @@ -39,15 +53,12 @@ def initialize(config:) Contract Entitlements::Models::Group => C::Maybe[Entitlements::Models::Group] def read(entitlement_group) slug = Entitlements::Util::Util.any_to_cn(entitlement_group.cn.downcase) - return @github_team_cache[slug] if @github_team_cache[slug] + return @github_team_cache[slug] if @github_team_cache.key?(slug) github_team = github.read_team(entitlement_group) - # We should not cache a team which does not exist - return nil if github_team.nil? - - Entitlements.logger.debug "Loaded #{github_team.team_dn} (id=#{github_team.team_id}) with #{github_team.member_strings.count} member(s)" - @github_team_cache[github_team.team_name] = github_team + log_loaded_team(github_team) if github_team + @github_team_cache[slug] = github_team end # Dry run of committing changes. Returns a list of users added or removed. @@ -151,6 +162,12 @@ def auto_generate_ignored_users(entitlement_group) private + Contract Entitlements::Backend::GitHubTeam::Models::Team => nil + def log_loaded_team(github_team) + Entitlements.logger.debug "Loaded #{github_team.team_dn} (id=#{github_team.team_id}) with #{github_team.member_strings.count} member(s)" + nil + end + # Construct an Entitlements::Models::Group for a new group and team # # group - An Entitlements::Models::Group object representing the defined group diff --git a/lib/entitlements/backend/github_team/service.rb b/lib/entitlements/backend/github_team/service.rb index dba731d..4329ecd 100644 --- a/lib/entitlements/backend/github_team/service.rb +++ b/lib/entitlements/backend/github_team/service.rb @@ -4,6 +4,7 @@ require_relative "../../service/github" require "base64" +require "json" require "set" module Entitlements @@ -16,6 +17,9 @@ class Service < Entitlements::Service::GitHub class TeamNotFound < RuntimeError; end + GRAPHQL_TEAM_BATCH_SIZE = 10 + MAX_GRAPHQL_TEAM_PAGES = 100 + # Constructor. # # addr - Base URL a GitHub Enterprise API (leave undefined to use dotcom) @@ -47,86 +51,62 @@ def initialize(org:, token:, ou:, addr: nil, ignore_not_found: false) # Returns a Entitlements::Backend::GitHubTeam::Models::Team or nil if the team does not exist Contract Entitlements::Models::Group => C::Maybe[Entitlements::Backend::GitHubTeam::Models::Team] def read_team(entitlement_group) - team_identifier = entitlement_group.cn.downcase - @team_cache[team_identifier] ||= begin - dn = "cn=#{team_identifier},#{ou}" - begin - entitlement_metadata = entitlement_group.metadata - rescue Entitlements::Models::Group::NoMetadata - entitlement_metadata = nil - end - - if (cached_members = Entitlements::Data::Groups::Cached.members(dn)) - Entitlements.logger.debug "Loading GitHub team #{identifier}:#{org}/#{team_identifier} from cache" - - cached_metadata = Entitlements::Data::Groups::Cached.metadata(dn) - # If both the cached and entitlement metadata are nil, our team metadata is nil - # If one of the cached or entitlement metadata is nil, we use the other populated metadata hash as-is - # If both cached and entitlement metadata exist, we combine the two hashes with the cached metadata taking precedence - # - # The reason we do this is because an entitlement file should be 1:1 to a GitHub Team. However, - # entitlements files allow for metadata tags and the GitHub.com Team does not have a place to store those. - # Therefore, we must combine any existing entitlement metadata entries into the Team metadata hash - if cached_metadata.nil? - team_metadata = entitlement_metadata - elsif entitlement_metadata.nil? - team_metadata = cached_metadata - else - # Always merge the current state metadata (cached or API call) into the entitlement metadata, so that the current state takes precedent - team_metadata = entitlement_metadata.merge(cached_metadata) - end + read_teams([entitlement_group]).fetch(entitlement_group.cn.downcase) + end - team = Entitlements::Backend::GitHubTeam::Models::Team.new( - team_id: -1, - team_name: team_identifier, - members: cached_members, - ou:, - metadata: team_metadata - ) + # Read multiple teams, using predictive state where valid and bounded GraphQL + # batches for the teams that require authoritative state. + # + # entitlement_groups - Array of desired Entitlements::Models::Group objects. + # + # Returns a Hash keyed by lower-case team slug. + Contract C::ArrayOf[Entitlements::Models::Group] => C::HashOf[String => C::Maybe[Entitlements::Backend::GitHubTeam::Models::Team]] + def read_teams(entitlement_groups) + result = {} + authoritative_groups = [] + + entitlement_groups.each do |entitlement_group| + team_identifier = entitlement_group.cn.downcase + if @team_cache.key?(team_identifier) + result[team_identifier] = @team_cache.fetch(team_identifier)[:value] + next + end - { cache: true, value: team } + predictive_team = team_from_predictive_cache(entitlement_group) + if predictive_team + @team_cache[team_identifier] = { cache: true, value: predictive_team } + result[team_identifier] = predictive_team else Entitlements.logger.debug "Loading GitHub team #{identifier}:#{org}/#{team_identifier}" + authoritative_groups << entitlement_group + end + end - begin - teamdata = graphql_team_data(team_identifier) - # The entitlement metadata may have GitHub.com Team metadata which it wants to set, so we must - # overwrite that metadata with what we get from the API - if teamdata[:parent_team_name].nil? - team_metadata = entitlement_metadata - else - parent_team_metadata = { - "parent_team_name" => teamdata[:parent_team_name] - } - if entitlement_metadata.nil? - team_metadata = parent_team_metadata - else - # Always merge the current state metadata (cached or API call) into the entitlement metadata, so that the current state takes precedent - team_metadata = entitlement_metadata.merge(parent_team_metadata) - end - end - - maintainers = teamdata[:members].select { |u| teamdata[:roles][u] == "maintainer" } - team_metadata ||= {} - team_metadata = team_metadata.merge({ "team_maintainers" => maintainers.any? ? maintainers.join(",") : nil }) - - team = Entitlements::Backend::GitHubTeam::Models::Team.new( - team_id: teamdata[:team_id], - team_name: team_identifier, - members: Set.new(teamdata[:members]), - ou:, - metadata: team_metadata - ) - rescue TeamNotFound + unless authoritative_groups.empty? + team_data = if authoritative_groups.one? + team_identifier = authoritative_groups.first.cn.downcase + begin + { team_identifier => graphql_team_data(team_identifier) } + rescue TeamNotFound + { team_identifier => nil } + end + else + graphql_team_data_batch(authoritative_groups.map { |group| group.cn.downcase }) + end + + authoritative_groups.each do |entitlement_group| + team_identifier = entitlement_group.cn.downcase + data = team_data.fetch(team_identifier) + team = data && team_from_graphql_data(entitlement_group, data) + unless team Entitlements.logger.warn "Team #{team_identifier} does not exist in this GitHub.com organization. If applied, the team will be created." - return nil end - - { cache: false, value: team } + @team_cache[team_identifier] = { cache: false, value: team } + result[team_identifier] = team end end - @team_cache[team_identifier][:value] + result end # Determine whether the most recent entry came from the predictive cache or an actual @@ -361,65 +341,187 @@ def team_by_name(org_name:, team_name:) Contract String => { members: C::ArrayOf[String], team_id: Integer, parent_team_name: C::Or[String, nil], roles: C::HashOf[String => String] } def graphql_team_data(team_slug) - cursor = nil - team_id = nil - result = [] - roles = {} - sanity_counter = 0 - - while sanity_counter < 100 - sanity_counter += 1 - first_str = cursor.nil? ? "first: #{max_graphql_results}" : "first: #{max_graphql_results}, after: \"#{cursor}\"" - query = "{ - organization(login: \"#{org}\") { - team(slug: \"#{team_slug}\") { - databaseId - parentTeam { - slug - } - members(#{first_str}, membership: IMMEDIATE) { - edges { - node { - login - } - role - cursor - } - } - } - } - }".gsub(/\n\s+/, "\n") + result = graphql_team_data_batch([team_slug]).fetch(team_slug) + raise TeamNotFound, "Requested team #{team_slug} does not exist in #{org}!" if result.nil? - response = graphql_http_post(query) - unless response[:code] == 200 - Entitlements.logger.fatal "Abort due to GraphQL failure on #{query.inspect}" - raise "GraphQL query failure" - end + result + end + + def graphql_team_data_batch(team_slugs) + states = team_slugs.to_h do |team_slug| + [team_slug, { members: [], roles: {}, team_id: nil, parent_team_name: nil, cursor: nil, pages: 0 }] + end + pending_team_slugs = team_slugs + + until pending_team_slugs.empty? + next_pending_team_slugs = [] + + pending_team_slugs.each_slice(graphql_team_batch_size) do |batch| + alias_to_team = batch.each_with_index.to_h { |team_slug, index| ["team#{index}", team_slug] } + query = graphql_team_batch_query(alias_to_team, states) + response = graphql_http_post(query) + unless response[:code] == 200 + Entitlements.logger.fatal "Abort due to GraphQL failure on #{query.inspect}" + raise "GraphQL query failure" + end + + response_data = response[:data].fetch("data") + organization = response_data.fetch("organization") + raise "GraphQL response missing organization #{org}" if organization.nil? + + log_graphql_rate_limit(response_data["rateLimit"]) - team = response[:data].fetch("data").fetch("organization").fetch("team") - raise TeamNotFound, "Requested team #{team_slug} does not exist in #{org}!" if team.nil? + alias_to_team.each do |team_alias, team_slug| + state = states.fetch(team_slug) + team = organization.fetch(team_alias) + if team.nil? + raise "GitHub team #{team_slug} disappeared during pagination" if state[:pages].positive? - team_id = team.fetch("databaseId") - parent_team_name = team.dig("parentTeam", "slug") + states[team_slug] = nil + next + end + + state[:pages] += 1 + team_id = team.fetch("databaseId") + if state[:team_id] && state[:team_id] != team_id + raise "GitHub team #{team_slug} changed database ID during pagination" + end + state[:team_id] = team_id + state[:parent_team_name] = team.dig("parentTeam", "slug") + + edges = team.fetch("members").fetch("edges") + edges.each do |edge| + username = edge.fetch("node").fetch("login").downcase + state[:members] << username + state[:roles][username] = edge.fetch("role").downcase + end - edges = team.fetch("members").fetch("edges") - break unless edges.any? + next unless edges.size == max_graphql_results - buffer = edges.map { |e| e.fetch("node").fetch("login").downcase } - result.concat buffer + cursor = edges.last.fetch("cursor") + raise "GitHub team #{team_slug} returned a full page without a cursor" if cursor.nil? + if state[:pages] >= MAX_GRAPHQL_TEAM_PAGES + raise "GitHub team #{team_slug} exceeded the #{MAX_GRAPHQL_TEAM_PAGES}-page GraphQL limit" + end - edges.each do |e| - role = e.fetch("role").downcase - roles[e.fetch("node").fetch("login").downcase] = role + state[:cursor] = cursor + next_pending_team_slugs << team_slug + end end - cursor = edges.last.fetch("cursor") - next if cursor && buffer.size == max_graphql_results + pending_team_slugs = next_pending_team_slugs + end + + states.transform_values do |state| + next if state.nil? - break + state.slice(:members, :roles, :team_id, :parent_team_name) end + end + + def graphql_team_batch_query(alias_to_team, states) + team_fields = alias_to_team.map do |team_alias, team_slug| + cursor = states.fetch(team_slug)[:cursor] + pagination = "first: #{max_graphql_results}" + pagination += ", after: #{graphql_string_literal(cursor)}" if cursor + "#{team_alias}: team(slug: #{graphql_string_literal(team_slug)}) { + databaseId + parentTeam { + slug + } + members(#{pagination}, membership: IMMEDIATE) { + edges { + node { + login + } + role + cursor + } + } + }" + end.join("\n") + + "query { + rateLimit { + cost + remaining + resetAt + } + organization(login: #{graphql_string_literal(org)}) { + #{team_fields} + } + }".gsub(/\n\s+/, "\n") + end - { members: result, team_id:, parent_team_name:, roles: } + Contract String => String + def graphql_string_literal(value) + JSON.generate(value) + end + + def graphql_team_batch_size + GRAPHQL_TEAM_BATCH_SIZE + end + + def log_graphql_rate_limit(rate_limit) + return if rate_limit.nil? + + Entitlements.logger.debug( + "GitHub GraphQL team batch cost=#{rate_limit['cost']} remaining=#{rate_limit['remaining']} reset_at=#{rate_limit['resetAt']}" + ) + end + + def team_from_predictive_cache(entitlement_group) + team_identifier = entitlement_group.cn.downcase + dn = "cn=#{team_identifier},#{ou}" + cached_members = Entitlements::Data::Groups::Cached.members(dn) + return if cached_members.nil? + + Entitlements.logger.debug "Loading GitHub team #{identifier}:#{org}/#{team_identifier} from cache" + cached_metadata = Entitlements::Data::Groups::Cached.metadata(dn) + entitlement_metadata = metadata_from_entitlement(entitlement_group) + team_metadata = if cached_metadata.nil? + entitlement_metadata + elsif entitlement_metadata.nil? + cached_metadata + else + entitlement_metadata.merge(cached_metadata) + end + + Entitlements::Backend::GitHubTeam::Models::Team.new( + team_id: -1, + team_name: team_identifier, + members: cached_members, + ou:, + metadata: team_metadata + ) + end + + def team_from_graphql_data(entitlement_group, teamdata) + team_identifier = entitlement_group.cn.downcase + entitlement_metadata = metadata_from_entitlement(entitlement_group) + parent_team_name = teamdata[:parent_team_name] + team_metadata = if parent_team_name.nil? + entitlement_metadata + else + (entitlement_metadata || {}).merge("parent_team_name" => parent_team_name) + end + + maintainers = teamdata[:members].select { |username| teamdata[:roles][username] == "maintainer" } + team_metadata = (team_metadata || {}).merge("team_maintainers" => maintainers.any? ? maintainers.join(",") : nil) + + Entitlements::Backend::GitHubTeam::Models::Team.new( + team_id: teamdata[:team_id], + team_name: team_identifier, + members: Set.new(teamdata[:members]), + ou:, + metadata: team_metadata + ) + end + + def metadata_from_entitlement(entitlement_group) + entitlement_group.metadata + rescue Entitlements::Models::Group::NoMetadata + nil end # Ensure that the given team ID actually matches up to the team slug on GitHub. This is in place diff --git a/spec/unit/entitlements/backend/github_team/controller_spec.rb b/spec/unit/entitlements/backend/github_team/controller_spec.rb index 44a4869..0657b3c 100644 --- a/spec/unit/entitlements/backend/github_team/controller_spec.rb +++ b/spec/unit/entitlements/backend/github_team/controller_spec.rb @@ -121,8 +121,9 @@ expect(Entitlements::Backend::GitHubTeam::Service).to receive(:new).with(hash_including(addr: nil)).and_return(dotcom_obj) allow(dotcom_obj).to receive(:identifier).and_return("github.com") allow(dotcom_obj).to receive(:org).and_return("kittensinc") - allow(dotcom_obj).to receive(:read_team).with(russian_blue_group).and_return(russian_blue_team) - allow(dotcom_obj).to receive(:read_team).with(snowshoe_group).and_return(snowshoe_team) + allow(dotcom_obj).to receive(:read_teams) + .with([snowshoe_group, russian_blue_group]) + .and_return("snowshoes" => snowshoe_team, "russian-blues" => russian_blue_team) allow(dotcom_obj).to receive(:org_members).and_return(org_member_hash) allow(dotcom_obj).to receive(:from_predictive_cache?).and_return(false) @@ -168,7 +169,9 @@ expect(Entitlements::Backend::GitHubTeam::Service).to receive(:new).with(hash_including(addr: nil)).and_return(dotcom_obj) allow(dotcom_obj).to receive(:identifier).and_return("github.com") allow(dotcom_obj).to receive(:org).and_return("kittensinc") - allow(dotcom_obj).to receive(:read_team).with(russian_blue_group).and_return(russian_blue_team) + allow(dotcom_obj).to receive(:read_teams) + .with([russian_blue_group]) + .and_return("russian-blues" => russian_blue_team) allow(dotcom_obj).to receive(:org_members).and_return(org_member_hash) allow(dotcom_obj).to receive(:from_predictive_cache?).and_return(false) @@ -216,8 +219,9 @@ allow(dotcom_obj).to receive(:identifier).and_return("github.com") allow(dotcom_obj).to receive(:org).and_return("kittensinc") allow(dotcom_obj).to receive(:ou).and_return("GitHub") - allow(dotcom_obj).to receive(:read_team).with(russian_blue_group).and_return(nil) - allow(dotcom_obj).to receive(:read_team).with(snowshoe_group).and_return(snowshoe_team) + allow(dotcom_obj).to receive(:read_teams) + .with([snowshoe_group, russian_blue_group]) + .and_return("snowshoes" => snowshoe_team, "russian-blues" => nil) allow(dotcom_obj).to receive(:org_members).and_return(org_member_hash) allow(dotcom_obj).to receive(:from_predictive_cache?).and_return(false) diff --git a/spec/unit/entitlements/backend/github_team/provider_spec.rb b/spec/unit/entitlements/backend/github_team/provider_spec.rb index d3df3dc..bbfce1d 100644 --- a/spec/unit/entitlements/backend/github_team/provider_spec.rb +++ b/spec/unit/entitlements/backend/github_team/provider_spec.rb @@ -98,6 +98,28 @@ end end + describe "#prefetch" do + let(:missing_group) do + Entitlements::Models::Group.new( + dn: "cn=missing-cats,ou=Github,dc=github,dc=fake", + members: Set.new + ) + end + + it "populates the existing provider cache for present and missing teams" do + allow(subject).to receive(:github).and_return(github) + expect(github).to receive(:read_teams).with([group, missing_group]) + .and_return("cats" => team, "missing-cats" => nil) + expect(logger).to receive(:debug).with("Loaded cn=cats,ou=kittensinc,ou=GitHub,dc=github,dc=fake (id=1001) with 2 member(s)") + + subject.prefetch([group, missing_group]) + + expect(github).not_to receive(:read_team) + expect(subject.read(group)).to eq(team) + expect(subject.read(missing_group)).to be_nil + end + end + describe "#diff" do let(:team_identifier) { "grumpy-cats" } let(:team_dn) { "cn=#{team_identifier},ou=kittensinc,ou=GitHub,dc=github,dc=fake" } diff --git a/spec/unit/entitlements/backend/github_team/service_spec.rb b/spec/unit/entitlements/backend/github_team/service_spec.rb index bc430af..83e67fd 100644 --- a/spec/unit/entitlements/backend/github_team/service_spec.rb +++ b/spec/unit/entitlements/backend/github_team/service_spec.rb @@ -79,9 +79,7 @@ it "returns nil when the team does not exist" do graphql_response = '{"data":{"organization":{"team":null}}}' stub_request(:post, "https://github.fake/api/v3/graphql") - .with( - body: "{\"query\":\"{\\norganization(login: \\\"kittensinc\\\") {\\nteam(slug: \\\"team-does-not-exist\\\") {\\ndatabaseId\\nparentTeam {\\nslug\\n}\\nmembers(first: 100, membership: IMMEDIATE) {\\nedges {\\nnode {\\nlogin\\n}\\nrole\\ncursor\\n}\\n}\\n}\\n}\\n}\"}" - ).to_return(status: 200, body: graphql_response) + .to_return(status: 200, body: graphql_response.sub('"team"', '"team0"')) expect(logger).to receive(:debug).with("Setting up GitHub API connection to https://github.fake/api/v3/") expect(logger).to receive(:debug).with("Loading GitHub team github.fake:kittensinc/team-does-not-exist") @@ -92,9 +90,7 @@ it "returns a Entitlements::Backend::GitHubTeam::Models::Team object when the team exists" do stub_request(:post, "https://github.fake/api/v3/graphql") - .with( - body: "{\"query\":\"{\\norganization(login: \\\"kittensinc\\\") {\\nteam(slug: \\\"cuddly-kittens\\\") {\\ndatabaseId\\nparentTeam {\\nslug\\n}\\nmembers(first: 100, membership: IMMEDIATE) {\\nedges {\\nnode {\\nlogin\\n}\\nrole\\ncursor\\n}\\n}\\n}\\n}\\n}\"}" - ).to_return(status: 200, body: graphql_response(cuddly_kittens, 0, 100)) + .to_return(status: 200, body: graphql_response(cuddly_kittens, 0, 100)) expect(logger).to receive(:debug).with("Setting up GitHub API connection to https://github.fake/api/v3/") expect(logger).to receive(:debug).with("Loading GitHub team github.fake:kittensinc/cuddly-kittens") @@ -109,9 +105,7 @@ it "returns a Entitlements::Backend::GitHubTeam::Models::Team object with parent team when the team exists" do stub_request(:post, "https://github.fake/api/v3/graphql") - .with( - body: "{\"query\":\"{\\norganization(login: \\\"kittensinc\\\") {\\nteam(slug: \\\"cuddly-kittens\\\") {\\ndatabaseId\\nparentTeam {\\nslug\\n}\\nmembers(first: 100, membership: IMMEDIATE) {\\nedges {\\nnode {\\nlogin\\n}\\nrole\\ncursor\\n}\\n}\\n}\\n}\\n}\"}" - ).to_return(status: 200, body: graphql_response(cuddly_kittens, 0, 100, parent_team: "parent-cats")) + .to_return(status: 200, body: graphql_response(cuddly_kittens, 0, 100, parent_team: "parent-cats")) expect(logger).to receive(:debug).with("Setting up GitHub API connection to https://github.fake/api/v3/") expect(logger).to receive(:debug).with("Loading GitHub team github.fake:kittensinc/cuddly-kittens") @@ -128,9 +122,7 @@ it "returns a Entitlements::Backend::GitHubTeam::Models::Team object with parent team when the team exists but has empty entitlement metadata" do stub_request(:post, "https://github.fake/api/v3/graphql") - .with( - body: "{\"query\":\"{\\norganization(login: \\\"kittensinc\\\") {\\nteam(slug: \\\"cuddly-kittens\\\") {\\ndatabaseId\\nparentTeam {\\nslug\\n}\\nmembers(first: 100, membership: IMMEDIATE) {\\nedges {\\nnode {\\nlogin\\n}\\nrole\\ncursor\\n}\\n}\\n}\\n}\\n}\"}" - ).to_return(status: 200, body: graphql_response(cuddly_kittens_no_metadata, 0, 100, parent_team: "parent-cats")) + .to_return(status: 200, body: graphql_response(cuddly_kittens_no_metadata, 0, 100, parent_team: "parent-cats")) expect(logger).to receive(:debug).with("Setting up GitHub API connection to https://github.fake/api/v3/") expect(logger).to receive(:debug).with("Loading GitHub team github.fake:kittensinc/cuddly-kittens") @@ -225,6 +217,226 @@ end end + describe "#read_teams" do + let(:alpha_group) do + Entitlements::Models::Group.new( + dn: "cn=alpha-cats,ou=kittensinc,ou=GitHub,dc=github,dc=fake", + members: Set.new, + metadata: { "application_owner" => "snowshoe" } + ) + end + + let(:beta_group) do + Entitlements::Models::Group.new( + dn: "cn=beta-cats,ou=kittensinc,ou=GitHub,dc=github,dc=fake", + members: Set.new, + metadata: nil + ) + end + + let(:missing_group) do + Entitlements::Models::Group.new( + dn: "cn=missing-cats,ou=kittensinc,ou=GitHub,dc=github,dc=fake", + members: Set.new, + metadata: {} + ) + end + + it "batches aliases, maps reordered responses, and paginates only unfinished teams" do + allow(subject).to receive(:max_graphql_results).and_return(2) + queries = [] + responses = [ + { + "data" => { + "rateLimit" => { "cost" => 5, "remaining" => 4_995, "resetAt" => "later" }, + "organization" => { + "team2" => nil, + "team1" => { + "databaseId" => 202, + "parentTeam" => nil, + "members" => { "edges" => [] } + }, + "team0" => { + "databaseId" => 101, + "parentTeam" => { "slug" => "parent-cats" }, + "members" => { + "edges" => [ + { "node" => { "login" => "ALPHA" }, "role" => "MAINTAINER", "cursor" => "cursor-1" }, + { "node" => { "login" => "Beta" }, "role" => "MEMBER", "cursor" => "cursor-2" } + ] + } + } + } + } + }, + { + "data" => { + "organization" => { + "team0" => { + "databaseId" => 101, + "parentTeam" => { "slug" => "parent-cats" }, + "members" => { + "edges" => [ + { "node" => { "login" => "GAMMA" }, "role" => "MEMBER", "cursor" => "cursor-3" } + ] + } + } + } + } + } + ] + allow(subject).to receive(:graphql_http_post) do |query| + queries << query + { code: 200, data: responses.shift } + end + + expect(logger).to receive(:debug).with("Loading GitHub team github.fake:kittensinc/alpha-cats") + expect(logger).to receive(:debug).with("Loading GitHub team github.fake:kittensinc/beta-cats") + expect(logger).to receive(:debug).with("Loading GitHub team github.fake:kittensinc/missing-cats") + expect(logger).to receive(:debug).with("GitHub GraphQL team batch cost=5 remaining=4995 reset_at=later") + expect(logger).to receive(:warn).with("Team missing-cats does not exist in this GitHub.com organization. If applied, the team will be created.") + + result = subject.read_teams([alpha_group, beta_group, missing_group]) + + expect(queries.size).to eq(2) + expect(queries.first).to include('team0: team(slug: "alpha-cats")') + expect(queries.first).to include('team1: team(slug: "beta-cats")') + expect(queries.first).to include('team2: team(slug: "missing-cats")') + expect(queries.last).to include('team0: team(slug: "alpha-cats")') + expect(queries.last).to include('after: "cursor-2"') + expect(queries.last).not_to include("beta-cats") + expect(queries.last).not_to include("missing-cats") + + expect(result.fetch("alpha-cats").member_strings).to eq(Set.new(%w[alpha beta gamma])) + expect(result.fetch("alpha-cats").team_id).to eq(101) + expect(result.fetch("alpha-cats").metadata).to eq( + "application_owner" => "snowshoe", + "parent_team_name" => "parent-cats", + "team_maintainers" => "alpha" + ) + expect(result.fetch("beta-cats").member_strings).to eq(Set.new) + expect(result.fetch("missing-cats")).to be_nil + end + + it "excludes predictive-cache hits from authoritative batches" do + alpha_dn = "cn=alpha-cats,ou=kittensinc,ou=GitHub,dc=github,dc=fake" + cache[:predictive_state] = { + by_dn: { alpha_dn => { members: Set.new(%w[CachedCat]), metadata: nil } }, + invalid: Set.new + } + authoritative_data = { + members: ["api-cat"], + roles: { "api-cat" => "member" }, + team_id: 202, + parent_team_name: nil + } + + expect(subject).to receive(:graphql_team_data).with("beta-cats").and_return(authoritative_data) + result = subject.read_teams([alpha_group, beta_group]) + + expect(result.fetch("alpha-cats").team_id).to eq(-1) + expect(result.fetch("alpha-cats").member_strings).to eq(Set.new(%w[cachedcat])) + expect(result.fetch("beta-cats").team_id).to eq(202) + expect(subject.from_predictive_cache?(alpha_group)).to eq(true) + expect(subject.from_predictive_cache?(beta_group)).to eq(false) + end + + it "respects the configured batch boundary" do + allow(subject).to receive(:graphql_team_batch_size).and_return(2) + groups = %w[one two three].map do |slug| + Entitlements::Models::Group.new( + dn: "cn=#{slug},ou=kittensinc,ou=GitHub,dc=github,dc=fake", + members: Set.new, + metadata: nil + ) + end + queries = [] + allow(subject).to receive(:graphql_http_post) do |query| + queries << query + aliases = query.scan(/(team\d+): team\(slug: "([^"]+)"\)/) + organization = aliases.to_h do |team_alias, slug| + [team_alias, { "databaseId" => slug.length, "parentTeam" => nil, "members" => { "edges" => [] } }] + end + { code: 200, data: { "data" => { "organization" => organization } } } + end + + subject.read_teams(groups) + + expect(queries.size).to eq(2) + expect(queries.first.scan(/team\d+: team/).size).to eq(2) + expect(queries.last.scan(/team\d+: team/).size).to eq(1) + end + + it "escapes GraphQL string values" do + expect(subject.send(:graphql_string_literal, "cats\"\\\n")).to eq("\"cats\\\"\\\\\\n\"") + end + + it "fails when the organization is missing from an otherwise successful response" do + allow(subject).to receive(:graphql_http_post) + .and_return(code: 200, data: { "data" => { "organization" => nil } }) + + expect do + subject.read_teams([alpha_group, beta_group]) + end.to raise_error(RuntimeError, "GraphQL response missing organization kittensinc") + end + + it "enforces the per-team pagination sanity limit" do + stub_const("#{described_class}::MAX_GRAPHQL_TEAM_PAGES", 2) + allow(subject).to receive(:max_graphql_results).and_return(1) + allow(subject).to receive(:graphql_http_post).and_return( + code: 200, + data: { + "data" => { + "organization" => { + "team0" => { + "databaseId" => 101, + "parentTeam" => nil, + "members" => { + "edges" => [ + { "node" => { "login" => "cat" }, "role" => "MEMBER", "cursor" => "next" } + ] + } + } + } + } + } + ) + + expect do + subject.read_teams([alpha_group]) + end.to raise_error(RuntimeError, "GitHub team alpha-cats exceeded the 2-page GraphQL limit") + end + + it "fails when a team changes database ID during pagination" do + allow(subject).to receive(:max_graphql_results).and_return(1) + responses = [101, 202].map do |team_id| + { + code: 200, + data: { + "data" => { + "organization" => { + "team0" => { + "databaseId" => team_id, + "parentTeam" => nil, + "members" => { + "edges" => [ + { "node" => { "login" => "cat" }, "role" => "MEMBER", "cursor" => "next" } + ] + } + } + } + } + } + } + end + allow(subject).to receive(:graphql_http_post) { responses.shift } + + expect do + subject.read_teams([alpha_group]) + end.to raise_error(RuntimeError, "GitHub team alpha-cats changed database ID during pagination") + end + end + describe "#from_predictive_cache?" do let(:people) { Set.new(%w[blackmanx ragamuffin russianblue]) } @@ -699,7 +911,7 @@ end it "raises a custom exception when team is not found" do - empty = JSON.generate("data" => { "organization" => { "team" => nil } }) + empty = JSON.generate("data" => { "organization" => { "team0" => nil } }) stub_request(:post, "https://github.fake/api/v3/graphql").to_return(status: 200, body: empty) expect do subject.send(:graphql_team_data, "crying-cat-face") @@ -718,12 +930,11 @@ it "parses team data from a single page of results" do stub_request(:post, "https://github.fake/api/v3/graphql") .with( - body: "{\"query\":\"{\\norganization(login: \\\"kittensinc\\\") {\\nteam(slug: \\\"grumpy-cat\\\") {\\ndatabaseId\\nparentTeam {\\nslug\\n}\\nmembers(first: 100, membership: IMMEDIATE) {\\nedges {\\nnode {\\nlogin\\n}\\nrole\\ncursor\\n}\\n}\\n}\\n}\\n}\"}", headers: { "Authorization" => "bearer GoPackGo", "Content-Type" => "application/json" } - ).to_return(status: 200, body: graphql_dotcom_response) + ).to_return(status: 200, body: graphql_dotcom_response.sub('"team"', '"team0"')) result = subject.send(:graphql_team_data, "grumpy-cat") members = ["highlander", "blackmanx", "toyger", "ocicat", "hubot", "korat", "mainecoon", "russianblue", @@ -761,9 +972,9 @@ it "parses team data from paginated results" do stub_request(:post, "https://github.fake/api/v3/graphql") .to_return( - { status: 200, body: graphql_dotcom_response_1 }, - { status: 200, body: graphql_dotcom_response_2 }, - { status: 200, body: graphql_dotcom_response_3 } + { status: 200, body: graphql_dotcom_response_1.sub('"team"', '"team0"') }, + { status: 200, body: graphql_dotcom_response_2.sub('"team"', '"team0"') }, + { status: 200, body: graphql_dotcom_response_3.sub('"team"', '"team0"') } ) result = subject.send(:graphql_team_data, "grumpy-cat") @@ -802,9 +1013,9 @@ it "parses team data from paginated results" do stub_request(:post, "https://github.fake/api/v3/graphql") .to_return( - { status: 200, body: graphql_dotcom_response_1 }, - { status: 200, body: graphql_dotcom_response_2 }, - { status: 200, body: graphql_dotcom_response_3 } + { status: 200, body: graphql_dotcom_response_1.sub('"team"', '"team0"') }, + { status: 200, body: graphql_dotcom_response_2.sub('"team"', '"team0"') }, + { status: 200, body: graphql_dotcom_response_3.sub('"team"', '"team0"') } ) result = subject.send(:graphql_team_data, "grumpy-cat") diff --git a/spec/unit/spec_helper.rb b/spec/unit/spec_helper.rb index 2a4ee2a..db3d0ea 100644 --- a/spec/unit/spec_helper.rb +++ b/spec/unit/spec_helper.rb @@ -57,15 +57,14 @@ def default_filters end def graphql_response(team, slice_start, slice_length, parent_team: nil) - team_id = rand(1..10000) edges = team.member_strings.sort.to_a.slice(slice_start, slice_length).map do |m| { "node" => { "login" => m }, "role" => "MEMBER", "cursor" => Base64.encode64(m) } end struct = { "data" => { "organization" => { - "team" => { - "databaseId" => team_id, + "team0" => { + "databaseId" => team.team_id, "members" => { "edges" => edges },