🐛 Correctly determine member roles

over-generous "or". oops. - add a debug endpoint - catch exception when fetching non-existent config files - cache collaborators
commonhaus · Jun 10, 2024 · 51be6d7 · 51be6d7
1 parent 36b1c11
commit 51be6d7
Show file tree

Hide file tree

Showing 12 changed files with 121 additions and 32 deletions.
diff --git a/bot-github-core/src/main/java/org/commonhaus/automation/github/context/BaseQueryCache.java b/bot-github-core/src/main/java/org/commonhaus/automation/github/context/BaseQueryCache.java
@@ -13,6 +13,7 @@ public enum BaseQueryCache {
     LABELS(b -> b.expireAfterWrite(1, TimeUnit.DAYS)),
 
     TEAM_MEMBERS(b -> b.expireAfterWrite(1, TimeUnit.DAYS)),
+    COLLABORATORS(b -> b.expireAfterWrite(1, TimeUnit.DAYS)),
 
     BOT_LOGIN(b -> b.expireAfterWrite(6, TimeUnit.HOURS)),
 

diff --git a/bot-github-core/src/main/java/org/commonhaus/automation/github/context/QueryContext.java b/bot-github-core/src/main/java/org/commonhaus/automation/github/context/QueryContext.java
@@ -1,6 +1,7 @@
 package org.commonhaus.automation.github.context;
 
 import static org.commonhaus.automation.github.context.BaseQueryCache.BOT_LOGIN;
+import static org.commonhaus.automation.github.context.BaseQueryCache.COLLABORATORS;
 import static org.commonhaus.automation.github.context.BaseQueryCache.LABELS;
 import static org.commonhaus.automation.github.context.BaseQueryCache.RECENT_BOT_CONTENT;
 import static org.commonhaus.automation.github.context.BaseQueryCache.TEAM_MEMBERS;
@@ -638,9 +639,16 @@ public DataLabel createLabel(String labelName, String color) {
 
     public boolean isTeamMember(GHUser user, String teamFullName) {
         Set<GHUser> members = teamMembers(teamFullName);
+        Log.debugf("%s members: %s", teamFullName, members.stream().map(GHUser::getLogin).toList());
         return members != null && members.contains(user);
     }
 
+    public boolean isCollaborator(GHUser user, String repoName) {
+        Set<String> collaborators = collaborators(repoName);
+        Log.debugf("%s members: %s", repoName, collaborators);
+        return collaborators != null && collaborators.contains(user.getLogin());
+    }
+
     public TeamList getTeamList(String teamFullName) {
         Set<GHUser> members = teamMembers(teamFullName);
         TeamList teamList = new TeamList(teamFullName, members);
@@ -679,13 +687,32 @@ public Set<GHUser> teamMembers(String teamFullName) {
                 return ghTeam == null ? Set.of() : ghTeam.getMembers();
             });
             if (hasErrors() || members == null) {
+                clearNotFound();
                 return null;
             }
             TEAM_MEMBERS.put(teamFullName, members);
         }
         return members;
     }
 
+    public Set<String> collaborators(String repoFullName) {
+        Set<String> collaborators = COLLABORATORS.get(repoFullName);
+        if (collaborators == null) {
+            collaborators = execGitHubSync((gh, dryRun) -> {
+                GHRepository repo = gh.getRepository(repoFullName);
+                return repo == null
+                        ? null
+                        : repo.getCollaboratorNames();
+            });
+            if (hasErrors() || collaborators == null) {
+                clearNotFound();
+                return null;
+            }
+            COLLABORATORS.put(repoFullName, collaborators);
+        }
+        return collaborators;
+    }
+
     public String[] getErrorAddresses() {
         return ctx.botErrorEmailAddress();
     }

diff --git a/...ub-core/src/main/java/org/commonhaus/automation/github/discovery/RepositoryDiscovery.java b/...ub-core/src/main/java/org/commonhaus/automation/github/discovery/RepositoryDiscovery.java
@@ -159,9 +159,17 @@ protected void handleRepositoryChanges(GitHub github, DynamicGraphQLClient graph
     }
 
     Optional<?> fetchConfigFile(GHRepository ghRepository) {
-        return configProvider.fetchConfigFile(ghRepository,
-                ctx.getConfigFileName(),
-                Source.DEFAULT,
-                ctx.getConfigType());
+        try {
+            return configProvider.fetchConfigFile(ghRepository,
+                    ctx.getConfigFileName(),
+                    Source.DEFAULT,
+                    ctx.getConfigType());
+        } catch (Throwable t) {
+            if (Log.isDebugEnabled()) {
+                t.printStackTrace();
+            }
+            return Optional.empty();
+        }
+
     }
 }
diff --git a/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberSession.java b/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberSession.java
@@ -44,7 +44,7 @@ public boolean userIsKnown(AppContextService ctx) {
 
     public GitHub connect(AppContextService ctx, SecurityIdentity identity) {
         if (connection == null) {
-            connection = ctx.getConnection(nodeId, identity);
+            connection = ctx.getUserConnection(nodeId, identity);
         }
         return connection;
     }

diff --git a/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/config/AdminConfigFile.java b/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/config/AdminConfigFile.java
@@ -2,6 +2,7 @@
 
 import com.fasterxml.jackson.annotation.JsonAlias;
 
+import io.quarkus.runtime.LaunchMode;
 import io.quarkus.runtime.annotations.RegisterForReflection;
 
 @RegisterForReflection
@@ -10,5 +11,5 @@ public record AdminConfigFile(
         @JsonAlias("user_management") UserManagementConfig userManagement,
         @JsonAlias("email_address") EmailAddress emailAddress) {
 
-    public static final String NAME = "cf-admin.yml";
+    public static final String NAME = LaunchMode.current() == LaunchMode.DEVELOPMENT ? "cf-admin-dev.yml" : "cf-admin.yml";
 }
diff --git a/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/dev/DebugRoutes.java b/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/dev/DebugRoutes.java
@@ -0,0 +1,58 @@
+package org.commonhaus.automation.admin.dev;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import jakarta.inject.Inject;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.core.Response;
+
+import org.commonhaus.automation.admin.github.AppContextService;
+import org.commonhaus.automation.admin.github.ScopedQueryContext;
+import org.kohsuke.github.GHUser;
+
+import io.quarkus.arc.profile.IfBuildProfile;
+import io.quarkus.logging.Log;
+
+@IfBuildProfile("dev")
+@Path("/debug")
+public class DebugRoutes {
+    @Inject
+    AppContextService appCtx;
+
+    @GET
+    @Path("/{login}/roles")
+    public Response getRoles(@PathParam("login") String login) {
+        Log.infof("DEBUG: getRoles(%s)", login);
+
+        ScopedQueryContext qc = appCtx.getScopedQueryContext("commonhaus/sponsors");
+
+        GHUser user = qc.getUser(login);
+        isTeamMember(qc, user, "commonhaus/cf-council");
+        isTeamMember(qc, user, "commonhaus/cf-egc");
+        isTeamMember(qc, user, "commonhaus/members");
+        isCollaborator(qc, user, "commonhaus/sponsors");
+
+        Set<String> roles = new HashSet<>();
+        appCtx.userIsKnown(qc, login, roles);
+
+        // appCtx.userIsKnown(qc, login, roles);
+        return Response.ok().build();
+    }
+
+    private boolean isTeamMember(ScopedQueryContext qc, GHUser user, String fullTeamName) {
+        boolean result = qc.isTeamMember(user, fullTeamName);
+        Log.debugf("%s isTeamMember of %s -> %s",
+                user.getLogin(), fullTeamName, result);
+        return result;
+    }
+
+    private boolean isCollaborator(ScopedQueryContext qc, GHUser user, String repoName) {
+        boolean result = qc.isCollaborator(user, repoName);
+        Log.debugf("%s isCollaborator of %s -> %s",
+                user.getLogin(), repoName, result);
+        return result;
+    }
+}
diff --git a/...mmonhaus/automation/admin/TestRoutes.java → ...haus/automation/admin/dev/TestRoutes.java b/...mmonhaus/automation/admin/TestRoutes.java → ...haus/automation/admin/dev/TestRoutes.java
@@ -1,13 +1,13 @@
-package org.commonhaus.automation.admin;
+package org.commonhaus.automation.admin.dev;
 
-import io.quarkus.arc.profile.UnlessBuildProfile;
+import io.quarkus.arc.profile.IfBuildProfile;
 import io.quarkus.qute.Engine;
 import io.quarkus.vertx.web.Route;
 import io.quarkus.vertx.web.Route.HttpMethod;
 import io.quarkus.vertx.web.RoutingExchange;
 import io.vertx.ext.web.RoutingContext;
 
-@UnlessBuildProfile("prod")
+@IfBuildProfile("dev")
 public class TestRoutes {
     final String memberHome;
 

diff --git a/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/github/AppContextService.java b/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/github/AppContextService.java
@@ -42,7 +42,6 @@
 import org.commonhaus.automation.github.discovery.RepositoryDiscoveryEvent;
 import org.eclipse.microprofile.rest.client.inject.RestClient;
 import org.kohsuke.github.GHEventPayload;
-import org.kohsuke.github.GHOrganization;
 import org.kohsuke.github.GHRepository;
 import org.kohsuke.github.GHUser;
 import org.kohsuke.github.GitHub;
@@ -182,7 +181,7 @@ protected void repositoryDiscovered(@Observes RepositoryDiscoveryEvent repoEvent
         }
     }
 
-    public GitHub getConnection(String nodeId, SecurityIdentity identity) {
+    public GitHub getUserConnection(String nodeId, SecurityIdentity identity) {
         GitHub connect = AdminDataCache.USER_CONNECTION.get(nodeId);
 
         if (connect == null || !connect.isCredentialValid()) {
@@ -258,6 +257,7 @@ public boolean userIsKnown(QueryContext initQc, String login, Set<String> roles)
             return false;
         }
 
+        result = Boolean.FALSE;
         if (!userConfig.collaboratorRoles().isEmpty()) {
             Map<String, String> collabRoles = userConfig.collaboratorRoles();
             Log.debugf("collaborators: %s", collabRoles);
@@ -270,16 +270,9 @@ public boolean userIsKnown(QueryContext initQc, String login, Set<String> roles)
                 if (qc == null) {
                     Log.errorf("No context for %s", repoName);
                 } else {
-                    Set<String> names = qc.execGitHubSync((gh, dryRun) -> {
-                        GHRepository repo = gh.getRepository(repoName);
-                        return repo == null
-                                ? null
-                                : repo.getCollaboratorNames();
-                    });
-                    qc.clearNotFound();
-                    if (names != null && names.contains(login)) {
-                        result = or(result, Boolean.TRUE);
+                    if (qc.isCollaborator(ghUser, repoName)) {
                         roles.add(role);
+                        result = or(result, Boolean.TRUE);
                     }
                 }
             }
@@ -293,24 +286,16 @@ public boolean userIsKnown(QueryContext initQc, String login, Set<String> roles)
                 String role = entry.getValue();
 
                 String orgName = ScopedQueryContext.toOrganizationName(teamFullName);
-
                 ScopedQueryContext qc = getScopedQueryContext(orgName);
-                GHOrganization org = qc.getOrganization(orgName);
-                if (org == null) {
-                    Log.errorf("No organization for %s", orgName);
-                    continue;
-                }
 
-                boolean isMember = or(result, qc.isTeamMember(ghUser, teamFullName));
-                if (isMember) {
-                    result = or(result, Boolean.TRUE);
+                if (qc == null) {
+                    Log.errorf("No context for %s", orgName);
+                } else if (qc.isTeamMember(ghUser, teamFullName)) {
                     roles.add(role);
+                    result = or(result, Boolean.TRUE);
                 }
             }
         }
-        if (result == null) {
-            return false;
-        }
         AdminDataCache.KNOWN_USER.put(login, result);
         return result;
     }

diff --git a/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/github/TeamMemberSync.java b/cf-admin-bot/src/main/java/org/commonhaus/automation/admin/github/TeamMemberSync.java
@@ -104,6 +104,7 @@ public void repositoryDiscovered(@Observes RepositoryDiscoveryEvent repoEvent) {
 
     public void updateTeamMembers(@Push GHEventPayload.Push pushEvent, GitHub github) {
         GHRepository repo = pushEvent.getRepository();
+        Log.debugf("pushEvent: %s", repo.getFullName());
         for (Entry<MonitoredRepo, String> entry : monitoredRepos.entrySet()) {
             MonitoredRepo repoCfg = entry.getKey();
             if (repoCfg.repoFullName().equals(repo.getFullName())

diff --git a/cf-admin-bot/src/main/resources/application.properties b/cf-admin-bot/src/main/resources/application.properties
@@ -5,6 +5,7 @@ quarkus.log.category."io.quarkus.oidc".level=DEBUG
 quarkus.log.category."io.quarkiverse".level=TRACE
 quarkus.log.category."io.quarkus.cache".level=DEBUG
 quarkus.log.category."org.kohsuke.github".level=DEBUG
+quarkus.log.category."org.kohsuke.github.GitHubClient".level=INFO
 quarkus.log.category."jdk.event.security".level=WARN
 quarkus.log.category."org.jboss.resteasy.reactive.client.logging".level=DEBUG
 

diff --git a/cf-admin-bot/src/test/java/org/commonhaus/automation/admin/api/MemberDataTest.java b/cf-admin-bot/src/test/java/org/commonhaus/automation/admin/api/MemberDataTest.java
@@ -120,6 +120,8 @@ void testUnknownUser() throws Exception {
             @UserInfo(key = "avatar_url", value = "https://avatars.githubusercontent.com/u/156364140?v=4")
     })
     void testUserInfoEndpoint() throws Exception {
+
+        addCollaborator("commonhaus-test/sponsors-test", botLogin);
         // Make known user: add to sponsors-test repository
         GHRepository repo = mockContext.mocks.repository("commonhaus-test/sponsors-test");
         when(repo.getCollaboratorNames()).thenReturn(Set.of(botLogin));

diff --git a/cf-admin-bot/src/test/java/org/commonhaus/automation/admin/github/ContextHelper.java b/cf-admin-bot/src/test/java/org/commonhaus/automation/admin/github/ContextHelper.java
@@ -20,6 +20,7 @@
 import org.commonhaus.automation.admin.AdminDataCache;
 import org.commonhaus.automation.admin.config.AdminConfigFile;
 import org.commonhaus.automation.github.context.ActionType;
+import org.commonhaus.automation.github.context.BaseQueryCache;
 import org.commonhaus.automation.github.context.EventType;
 import org.commonhaus.automation.github.context.QueryContext;
 import org.commonhaus.automation.github.discovery.DiscoveryAction;
@@ -160,6 +161,10 @@ public void setUserAsUnknown(String login) {
         AdminDataCache.KNOWN_USER.put(login, Boolean.FALSE);
     }
 
+    public void addCollaborator(String repoName, String login) {
+        BaseQueryCache.COLLABORATORS.put(repoName, Set.of(login));
+    }
+
     public GitHub setupBotGithub(AppContextService ctx, GitHubMockSetupContext mocks) throws IOException {
         GitHub gh = mocks.installationClient(installationId);
         DynamicGraphQLClient dql = mocks.installationGraphQLClient(installationId);