Skip to content

Commit

Permalink
🐛 TeamSync fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@ebullient
ebullient committed Jun 7, 2024
1 parent b03df90 commit c70c58f
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 17 deletions.
9 changes: 5 additions & 4 deletions cf-admin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberAliasesResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
@Authenticated
@ApplicationScoped
public class MemberAliasesResource {
final static String ID = "email";
@Inject
AppContextService ctx;

Expand All @@ -47,14 +48,14 @@ public Response getAliases(@DefaultValue("false") @QueryParam("refresh") boolean
if (!user.status().mayHaveEmail()) {
return Response.status(Response.Status.FORBIDDEN).build();
}
if (!ctx.validAttestation("email")) {
if (!ctx.validAttestation(ID)) {
return Response.status(Response.Status.BAD_REQUEST).build();
}

Services services = user.services();
ForwardEmail forwardEmail = services.forwardEmail();

boolean possibleMissingActive = !forwardEmail.configured && ctx.validAttestation("email");
boolean possibleMissingActive = !forwardEmail.configured && ctx.validAttestation(ID);
boolean checkAlias = forwardEmail.configured || possibleMissingActive;

Map<String, Alias> aliasMap = Map.of();
Expand Down Expand Up @@ -94,7 +95,7 @@ public Response updateAliases(Map<String, Set<String>> aliases) {
if (!user.status().mayHaveEmail()) {
return Response.status(Response.Status.FORBIDDEN).build();
}
if (!ctx.validAttestation("email")) {
if (!ctx.validAttestation(ID)) {
return Response.status(Response.Status.BAD_REQUEST).build();
}

Expand Down Expand Up @@ -151,7 +152,7 @@ public Response generatePassword(AliasRequest alias) {
// ForwardEmail forwardEmail = user.services().forwardEmail();

// boolean possibleMissingActive = !forwardEmail.active &&
// ctx.validAttestation("email");
// ctx.validAttestation(ID);
// boolean generatePassword = (forwardEmail.active || possibleMissingActive);
// if (generatePassword && !forwardEmail.validAddress(alias.email(),
// memberSession.login(), ctx.getDefaultDomain())) {
Expand Down
33 changes: 31 additions & 2 deletions cf-admin-bot/src/main/java/org/commonhaus/automation/admin/config/TeamSourceConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
public record TeamSourceConfig(
String path,
String repo,
Defaults defaults,
Map<String, SyncToTeams> sync,
@JsonAlias("dry_run") Boolean dryRun) {

Expand Down Expand Up @@ -45,6 +46,26 @@ public int hashCode() {
return Objects.hash(path, repo);
}

public record Defaults(
String field,
@JsonAlias("preserve_users") List<String> preserveUsers) {
@Override
public String field() {
return field == null ? "login" : field;
}

@Override
public List<String> preserveUsers() {
return preserveUsers == null ? List.of() : preserveUsers;
}

@Override
public String toString() {
return "SyncToTeams{field='%s', preserveUsers=%s}"
.formatted(field, preserveUsers);
}
}

public record SyncToTeams(
String field,
List<String> teams,
Expand All @@ -60,15 +81,23 @@ public List<String> preserveUsers() {
return preserveUsers == null ? List.of() : preserveUsers;
}

public List<String> preserveUsers(Defaults defaults) {
return preserveUsers == null ? defaults.preserveUsers() : preserveUsers;
}

@Override
public String field() {
return field == null ? "login" : field;
}

public String field(Defaults defaults) {
return field == null ? defaults.field() : field;
}

@Override
public String toString() {
return "SyncToTeams{field='%s', teams=%s}"
.formatted(field, teams);
return "SyncToTeams{field='%s', preserveUsers=%s, teams=%s}"
.formatted(field, preserveUsers, teams);
}
}
}
12 changes: 8 additions & 4 deletions cf-admin-bot/src/main/java/org/commonhaus/automation/admin/github/TeamMemberSync.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.commonhaus.automation.admin.config.AdminConfigFile;
import org.commonhaus.automation.admin.config.TeamManagementConfig;
import org.commonhaus.automation.admin.config.TeamSourceConfig;
import org.commonhaus.automation.admin.config.TeamSourceConfig.Defaults;
import org.commonhaus.automation.admin.config.TeamSourceConfig.SyncToTeams;
import org.commonhaus.automation.github.discovery.RepositoryDiscoveryEvent;
import org.kohsuke.github.GHEventPayload;
Expand Down Expand Up @@ -179,11 +180,12 @@ void syncTeamMembership(ScopedQueryContext qc, TeamSourceConfig source) {
return;
}

Defaults defaults = source.defaults();
// sync team membership
for (Map.Entry<String, SyncToTeams> entry : source.sync().entrySet()) {
String groupName = entry.getKey();
SyncToTeams sync = entry.getValue();
String field = sync.field();
String field = sync.field(defaults);

JsonNode sourceTeamData = data.get(groupName);
if (sourceTeamData != null && sourceTeamData.isArray()) {
Expand All @@ -206,7 +208,7 @@ void syncTeamMembership(ScopedQueryContext qc, TeamSourceConfig source) {
if (!targetTeam.contains("/")) {
targetTeam = repo.getFullName() + "/" + targetTeam;
}
doSyncTeamMembers(source, targetTeam, logins, qc.dryRunEmailAddress());
doSyncTeamMembers(source, targetTeam, logins, sync.preserveUsers(defaults), qc.dryRunEmailAddress());
} catch (Throwable t) {
qc.logAndSendEmail("doSyncTeamMembers", "Error syncing team members", t);
}
Expand All @@ -218,7 +220,7 @@ void syncTeamMembership(ScopedQueryContext qc, TeamSourceConfig source) {
}

void doSyncTeamMembers(TeamSourceConfig config, String fullTeamName, List<String> sourceLogins,
String[] dryRunEmail) {
List<String> preserveUsers, String[] dryRunEmail) {
boolean productionRun = !config.dryRun();

String orgName = ScopedQueryContext.toOrganizationName(fullTeamName);
Expand All @@ -244,11 +246,13 @@ void doSyncTeamMembers(TeamSourceConfig config, String fullTeamName, List<String
Set<GHUser> removed = new HashSet<>();

Set<String> toAdd = new HashSet<>(sourceLogins);
toAdd.addAll(preserveUsers);

original.forEach(user -> {
if (sourceLogins.contains(user.getLogin())) {
toAdd.remove(user.getLogin()); // already in team
finalLogins.add(user);
} else {
} else if (!preserveUsers.contains(user.getLogin())) {
Log.infof("doSyncTeamMembers: removing %s from %s", user.getLogin(), relativeName);
removed.add(user);
if (productionRun) {
Expand Down
6 changes: 5 additions & 1 deletion cf-admin-bot/src/test/java/org/commonhaus/automation/admin/github/ContextHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,16 @@ public GitHub setupMockTeam(GitHubMockSetupContext mocks) throws IOException {
if (i % 3 == 0) {
council.add(user);
}
if (i % 4 == 0) {
if (i % 6 == 0) {
voting.add(user);
}
when(gh.getUser(login)).thenReturn(user);
}

System.out.println("testQuorum " + testQuorum.stream().map(GHUser::getLogin).toList());
System.out.println("council " + council.stream().map(GHUser::getLogin).toList());
System.out.println("voting " + voting.stream().map(GHUser::getLogin).toList());

setupMockTeam("team-quorum-default", org, testQuorum);
setupMockTeam("cf-council", org, council);
setupMockTeam("cf-voting", org, voting);
Expand Down
36 changes: 35 additions & 1 deletion cf-admin-bot/src/test/java/org/commonhaus/automation/admin/github/TeamMemberSyncTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;

import jakarta.inject.Inject;

Expand All @@ -22,6 +23,7 @@
import org.kohsuke.github.GitHub;

import io.quarkiverse.githubapp.testing.GitHubAppTest;
import io.quarkus.mailer.Mail;
import io.quarkus.mailer.MockMailbox;
import io.quarkus.test.junit.QuarkusTest;

Expand Down Expand Up @@ -67,7 +69,39 @@ void testDiscoveryWhenInstallationAdded() throws Exception {
await().atMost(15, SECONDS).until(() -> mailbox.getTotalMessagesSent() >= 3);
assertThat(mailbox.getMailsSentTo("[email protected]")).hasSize(0);
assertThat(mailbox.getMailsSentTo("[email protected]")).hasSize(0);
assertThat(mailbox.getMailsSentTo("[email protected]")).hasSize(3);
List<Mail> mailList = mailbox.getMailsSentTo("[email protected]");
assertThat(mailList).hasSize(3);

for (Mail m : mailList) {
String subject = m.getSubject();
String body = m.getText();
String finalGroup = body.replaceAll("(?s).*?(Final:.*)", "$1");
System.out.println("final group: " + finalGroup);

// see test CONTACTS.yaml for group members
// see cf-admin.yml for sync parameters
if (subject.contains("commonhaus-test/cf-council") || subject.contains("commonhaus-test/cf-voting")) {
// cf-council and cf-voting should be the same
assertTeamLogins(finalGroup,
List.of("user1 ()", "user2 ()", "user3 ()", "user9 ()"));
}
if (subject.contains("commonhaus-test/team-quorum-default")) {
// team quorum has different membership + other preserved value
assertTeamLogins(finalGroup,
List.of("user4 ()", "user5 ()", "user6 ()", "user7 ()", "user8 ()", "user9 ()", "user12 ()"));
}
}
}

void assertTeamLogins(String text, List<String> logins) {
for (int i = 1; i < 15; i++) {
String login = "user" + i + " ()";
if (logins.contains(login)) {
assertThat(text).contains(login);
} else {
assertThat(text).doesNotContain(login);
}
}
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion cf-admin-bot/src/test/resources/CONTACTS.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ egc:
- project: jreleaser
login: user5
- project: morphia
login: user6
login: user12
- project: openrewrite
login: user7
- project: jackson
Expand Down
10 changes: 6 additions & 4 deletions cf-admin-bot/src/test/resources/cf-admin.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@ groupManagement:
sources:
- path: CONTACTS.yaml
repo: commonhaus/foundation
defaults:
field: login
preserveUsers:
- user6
sync:
cf-council:
field: login
preserve_users:
- user12
preserveUsers:
- user9
teams:
- commonhaus-test/cf-council
- commonhaus-test/cf-voting
egc:
field: login
teams:
- commonhaus-test/team-quorum-default
dryRun: true
Expand Down

0 comments on commit c70c58f

Please sign in to comment.