💡 make org config issues clearer (#85)

* This adds a new response state- bad_config- so that those impacted by
this application can more easily identify what problem their org sync is
running into.

Author: JoshuaTheMiller

src/services/gitHub.ts

@@ -1,7 +1,7 @@
 import { Octokit } from "octokit";
 import { createAppAuth } from "@octokit/auth-app";
 import { Config } from "../config";
-import { AddMemberResponse, CopilotAddResponse, GitHubClient, GitHubId, GitHubTeamId, InstalledClient, Org, OrgInvite, OrgRoles, RemoveMemberResponse, Response } from "./gitHubTypes";
+import { AddMemberResponse, CopilotAddResponse, GitHubClient, GitHubId, GitHubTeamId, InstalledClient, Org, OrgConfigResponse, OrgInvite, OrgRoles, RemoveMemberResponse, Response } from "./gitHubTypes";
 import { AppConfig } from "./appConfig";
 import yaml from "js-yaml";
 import { throttling } from "@octokit/plugin-throttling";
@@ -615,7 +615,7 @@ class InstalledGitHubClient implements InstalledClient {
 
     }
 
-    public async GetConfigurationForInstallation(): Response<OrgConfig> {
+    public async GetConfigurationForInstallation(): OrgConfigResponse {
         // TODO: this function doesn't really belong on this class...
         // i.e., it doesn't fit with a "GitHub Facade"
         const getContentRequest = {
@@ -631,15 +631,17 @@ class InstalledGitHubClient implements InstalledClient {
         }
         catch {
             return {
-                successful: false
+                successful: false,
+                state: "NoConfig"            
             }
         }
 
         const potentialFiles = filesResponse.data;
 
         if (!Array.isArray(potentialFiles)) {
             return {
-                successful: false
+                successful: false,
+                state: "NoConfig"                
             }
         }
 
@@ -649,7 +651,9 @@ class InstalledGitHubClient implements InstalledClient {
 
         if (onlyConfigFiles.length != 1) {
             return {
-                successful: false
+                successful: false,
+                state: "BadConfig",
+                message: "Multiple configuration files are not supported at this point in time."
             }
         }
 
@@ -664,15 +668,24 @@ class InstalledGitHubClient implements InstalledClient {
 
         if (Array.isArray(contentData) || contentData.type != "file") {
             return {
-                successful: false
+                successful: false,
+                state: "BadConfig"                
             }
         }
 
-        const configuration = yaml.load(Buffer.from(contentData.content, 'base64').toString()) as OrgConfigurationOptions;
-
-        return {
-            successful: true,
-            data: new OrgConfig(configuration)
+        try {
+            const configuration = yaml.load(Buffer.from(contentData.content, 'base64').toString()) as OrgConfigurationOptions;
+            return {
+                successful: true,
+                data: new OrgConfig(configuration)
+            }
+        }     
+        catch {
+            return {
+                successful: false,
+                state: "BadConfig",
+                message: "Error parsing configuration- check configuration file for validity: https://github.com/cloudpups/github-teams-user-sync/blob/main/docs/OrganizationConfiguration.md"
+            }
         }
     }
 }

src/services/gitHubCache.ts

@@ -1,6 +1,6 @@
 import { CacheClient } from "../app";
 import { ILogger } from "../logging";
-import { AddMemberResponse, CopilotAddResponse, GitHubId, GitHubTeamId, InstalledClient, OrgInvite, OrgRoles, RemoveMemberResponse, Response } from "./gitHubTypes";
+import { AddMemberResponse, CopilotAddResponse, GitHubId, GitHubTeamId, InstalledClient, OrgConfigResponse, OrgInvite, OrgRoles, RemoveMemberResponse, Response } from "./gitHubTypes";
 import { OrgConfig } from "./orgConfig";
 
 export class GitHubClientCache implements InstalledClient {
@@ -146,7 +146,7 @@ export class GitHubClientCache implements InstalledClient {
         return this.client.AddSecurityManagerTeam(team);
     }
 
-    GetConfigurationForInstallation(): Response<OrgConfig> {
+    GetConfigurationForInstallation(): OrgConfigResponse {
         return this.client.GetConfigurationForInstallation();
     }    
 }

src/services/gitHubTypes.ts

@@ -19,6 +19,19 @@ export type OrgInvite = {
     GitHubUser: string
 }
 
+export type OrgConfigResponseSuccess = {
+    successful: true,
+    data: OrgConfig
+}
+
+export type OrgConfigResponseBad = {
+    successful: false,
+    state: "NoConfig" | "BadConfig",
+    message?: string
+}
+
+export type OrgConfigResponse = Promise<OrgConfigResponseSuccess | OrgConfigResponseBad>;
+
 export interface InstalledClient {
     GetCurrentOrgName(): string
     GetCurrentRateLimit(): Promise<{ remaining: number }>
@@ -32,7 +45,7 @@ export interface InstalledClient {
     RemoveTeamMemberAsync(team: GitHubTeamName, user: GitHubId): RemoveMemberResponse
     UpdateTeamDetails(team: GitHubTeamName, description: string): Response
     AddSecurityManagerTeam(team: GitHubTeamName): Promise<unknown>
-    GetConfigurationForInstallation(): Response<OrgConfig>        
+    GetConfigurationForInstallation(): OrgConfigResponse        
     SetOrgRole(id: GitHubId, role: OrgRoles): Response
     GetPendingOrgInvites():Response<OrgInvite[]>
     CancelOrgInvite(invite:OrgInvite): Response    

src/services/githubSync.ts

@@ -1,5 +1,6 @@
 // REMEMBER TO REPLACE '_' with '-' for GitHub Names! 🤦‍♂️
 
+import e from "express";
 import { Log, LogError } from "../logging";
 import { AppConfig } from "./appConfig";
 import { CopilotAddResponse, FailedResponse, GitHubId, InstalledClient, OrgInvite } from "./gitHubTypes";
@@ -167,7 +168,7 @@ async function SynchronizeGitHubTeam(installedGitHubClient: InstalledClient, tea
 
 type ReturnTypeOfSyncOrg = {
     message?: string;
-    status: "failed" | "completed" | "no_config";
+    status: "failed" | "completed" | "no_config" | "bad_config";
     orgName: string;
     syncedSecurityManagerTeams: string[];
     orgOwnersGroup: string;
@@ -261,13 +262,14 @@ async function syncOrg(installedGitHubClient: InstalledClient, appConfig: AppCon
     }
     const setOfExistingTeams = new Set(existingTeamsResponse.data.map(t => t.Name.toUpperCase()));
 
-    const orgConfigResponse = await installedGitHubClient.GetConfigurationForInstallation();
+    const orgConfigResponse = await installedGitHubClient.GetConfigurationForInstallation();    
 
-    const orgConfig = orgConfigResponse.successful ? orgConfigResponse.data : undefined;    
+    const securityManagersFromOrgConfig = orgConfigResponse.successful ? orgConfigResponse.data.AdditionalSecurityManagerGroups : [];
+    const securityManagersDisplayNameSourceMap = orgConfigResponse.successful ? orgConfigResponse.data.DisplayNameToSourceMap : new Map<string,string>();
 
     const securityManagerTeams = [
         ...appConfig.SecurityManagerTeams,
-        ...orgConfig?.AdditionalSecurityManagerGroups ?? []
+        ...securityManagersFromOrgConfig
     ];        
 
     if (securityManagerTeams.length > 0) {
@@ -278,7 +280,7 @@ async function syncOrg(installedGitHubClient: InstalledClient, appConfig: AppCon
             securityManagerTeams,            
             setOfExistingTeams,            
             shortLink: appConfig.Description.ShortLink,
-            displayNameToSourceMap: orgConfig?.DisplayNameToSourceMap ?? new Map<string,string>()
+            displayNameToSourceMap: securityManagersDisplayNameSourceMap
         });
 
         if (!syncManagersResponse.Success) {
@@ -293,15 +295,31 @@ async function syncOrg(installedGitHubClient: InstalledClient, appConfig: AppCon
             ...response,
             syncedSecurityManagerTeams: syncManagersResponse.SyncedSecurityManagerTeams
         }
-    }
+    }    
 
-    if (orgConfig == undefined) {
+    if (!orgConfigResponse.successful && orgConfigResponse.state == "NoConfig") {
         return {
             ...response,
             message: "Cannot access/fetch organization config",
             status: "no_config"
         }
     }
+    else if(!orgConfigResponse.successful && orgConfigResponse.state == "BadConfig") {
+        return {
+            ...response,
+            message: orgConfigResponse.message,
+            status: "bad_config"
+        }
+    } 
+    else if (!orgConfigResponse.successful) {
+        return {
+            ...response,
+            message: orgConfigResponse.message,
+            status: "bad_config"
+        }
+    }
+
+    const orgConfig = orgConfigResponse.data;
 
     Log(JSON.stringify(orgConfig));