Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Refactor gh integration #288

Merged
merged 2 commits into from
Jan 22, 2025
Merged

fix: Refactor gh integration #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
217011a
fix: Refactor gh integration
adityachoudhari26 Jan 22, 2025
9fcf3ca
cleanup
adityachoudhari26 Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 97 additions & 53 deletions apps/event-worker/src/job-dispatch/github.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { and, eq, takeFirstOrNull } from "@ctrlplane/db";
import { db } from "@ctrlplane/db/client";
import {
environment,
githubOrganization,
githubEntity,
job,
releaseJobTrigger,
runbook,
Expand All @@ -18,29 +18,15 @@ import { JobStatus } from "@ctrlplane/validators/jobs";

import { getInstallationOctokit } from "../github-utils.js";

export const dispatchGithubJob = async (je: Job) => {
logger.info(`Dispatching github job ${je.id}...`);

const config = je.jobAgentConfig;
const parsed = configSchema.safeParse(config);
if (!parsed.success) {
logger.error(
`Invalid job agent config for job ${je.id}: ${parsed.error.message}`,
);
await db
.update(job)
.set({
status: JobStatus.InvalidJobAgent,
message: `Invalid job agent config for job ${je.id}: ${parsed.error.message}`,
})
.where(eq(job.id, je.id));
return;
}

const releaseGhOrgResult = await db
const getGithubEntity = async (
jobId: string,
installationId: number,
owner: string,
) => {
const releaseGhEntityPromise = db
.select()
.from(githubOrganization)
.innerJoin(workspace, eq(githubOrganization.workspaceId, workspace.id))
.from(githubEntity)
.innerJoin(workspace, eq(githubEntity.workspaceId, workspace.id))
.innerJoin(system, eq(system.workspaceId, workspace.id))
.innerJoin(environment, eq(environment.systemId, system.id))
.innerJoin(
Expand All @@ -49,43 +35,77 @@ export const dispatchGithubJob = async (je: Job) => {
)
.where(
and(
eq(githubOrganization.installationId, parsed.data.installationId),
eq(githubOrganization.organizationName, parsed.data.owner),
eq(releaseJobTrigger.jobId, je.id),
eq(githubEntity.installationId, installationId),
eq(githubEntity.slug, owner),
eq(releaseJobTrigger.jobId, jobId),
),
)
.then(takeFirstOrNull);

const runbookGhOrgResult = await db
const runbookGhEntityPromise = db
.select()
.from(githubOrganization)
.innerJoin(workspace, eq(githubOrganization.workspaceId, workspace.id))
.from(githubEntity)
.innerJoin(workspace, eq(githubEntity.workspaceId, workspace.id))
.innerJoin(system, eq(system.workspaceId, workspace.id))
.innerJoin(runbook, eq(runbook.systemId, system.id))
.innerJoin(runbookJobTrigger, eq(runbookJobTrigger.runbookId, runbook.id))
.where(
and(
eq(githubOrganization.installationId, parsed.data.installationId),
eq(githubOrganization.organizationName, parsed.data.owner),
eq(runbookJobTrigger.jobId, je.id),
eq(githubEntity.installationId, installationId),
eq(githubEntity.slug, owner),
eq(runbookJobTrigger.jobId, jobId),
),
)
.then(takeFirstOrNull);
const ghOrgResult = releaseGhOrgResult ?? runbookGhOrgResult;

if (ghOrgResult == null) {
const [releaseGhEntityResult, runbookGhEntityResult] = await Promise.all([
releaseGhEntityPromise,
runbookGhEntityPromise,
]);

return (
releaseGhEntityResult?.github_entity ?? runbookGhEntityResult?.github_entity
);
};

export const dispatchGithubJob = async (je: Job) => {
logger.info(`Dispatching github job ${je.id}...`);

const config = je.jobAgentConfig;
const parsed = configSchema.safeParse(config);
if (!parsed.success) {
logger.error(
`Invalid job agent config for job ${je.id}: ${parsed.error.message}`,
);
await db
.update(job)
.set({
status: JobStatus.InvalidJobAgent,
message: `Invalid job agent config for job ${je.id}: ${parsed.error.message}`,
})
.where(eq(job.id, je.id));
return;
}

const { data: parsedConfig } = parsed;

const ghEntity = await getGithubEntity(
je.id,
parsedConfig.installationId,
parsedConfig.owner,
);
if (ghEntity == null) {
await db
.update(job)
.set({
status: JobStatus.InvalidIntegration,
message: `GitHub organization not found for job ${je.id}`,
message: `GitHub entity not found for job ${je.id}`,
})
.where(eq(job.id, je.id));
return;
}
const ghOrg = ghOrgResult.github_organization;

const octokit = getInstallationOctokit(parsed.data.installationId);
const octokit = getInstallationOctokit(ghEntity.installationId);
if (octokit == null) {
logger.error(`GitHub bot not configured for job ${je.id}`);
await db
Expand All @@ -100,28 +120,52 @@ export const dispatchGithubJob = async (je: Job) => {

const installationToken = (await octokit.auth({
type: "installation",
installationId: parsed.data.installationId,
installationId: parsedConfig.installationId,
})) as { token: string };

const headers = {
"X-GitHub-Api-Version": "2022-11-28",
authorization: `Bearer ${installationToken.token}`,
};
Comment on lines +126 to +129
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use Authorization: token instead of Bearer for GitHub authentication.

GitHub API expects the Authorization header to use the token scheme for personal access tokens and installation tokens, not Bearer. Using Bearer might result in authentication errors.

Apply this diff to correct the authorization header:

const headers = {
  "X-GitHub-Api-Version": "2022-11-28",
- authorization: `Bearer ${installationToken.token}`,
+ authorization: `token ${installationToken.token}`,
};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const headers = {
"X-GitHub-Api-Version": "2022-11-28",
authorization: `Bearer ${installationToken.token}`,
};
const headers = {
"X-GitHub-Api-Version": "2022-11-28",
authorization: `token ${installationToken.token}`,
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure, using Bearer currently works correctly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adityachoudhari26 You're right - I apologize for my earlier suggestion. The GitHub API accepts both Bearer and token schemes for authentication. Since you've confirmed that Bearer works correctly in your implementation, we should keep it as is.

🔗 Analysis chain

@adityachoudhari26 Thank you for the feedback. If the current implementation with Bearer scheme is working correctly, I'll retract my suggestion. Let's keep the existing code as is.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for GitHub's official documentation about installation token authentication
curl -s "https://docs.github.com/en/rest/authentication/authenticating-to-the-rest-api?apiVersion=2022-11-28" | grep -A 5 -B 5 "Authorization.*token"

# Search for examples in the codebase showing GitHub API authentication
rg -A 2 -B 2 "authorization.*token.*installationToken" --type ts

Length of output: 65785

Script:

#!/bin/bash
# Search GitHub's REST API documentation for installation token authentication
curl -s "https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28" | grep -A 5 -B 5 "Authorization.*token"

# Search for examples in GitHub's official Octokit documentation
curl -s "https://github.com/octokit/octokit.js/blob/master/README.md" | grep -A 5 -B 5 "authorization.*token"

# Search for more examples in the codebase
rg -A 2 -B 2 "authorization.*token" --type ts

Length of output: 131070

Script:

#!/bin/bash
# Search for GitHub API authentication examples in the codebase
rg -A 2 -B 2 "octokit.*auth.*installation" --type ts

# Search for GitHub's official examples
curl -s "https://raw.githubusercontent.com/octokit/octokit.js/master/README.md" | grep -A 5 -B 5 "createAppAuth"

Length of output: 801

✏️ Learnings added 
Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#288
File: apps/event-worker/src/job-dispatch/github.ts:126-129
Timestamp: 2025-01-22T07:38:08.672Z
Learning: GitHub API accepts both 'Bearer' and 'token' schemes for authentication with installation tokens.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!


const ref =
parsedConfig.ref ??
(await octokit.rest.repos
.get({ ...parsedConfig, headers })
.then((r) => r.data.default_branch)
.catch((e) => {
logger.error(`Failed to get ref for github action job ${je.id}`, {
error: e,
});
return null;
}));

if (ref == null) {
logger.error(`Failed to get ref for github action job ${je.id}`);
await db
.update(job)
.set({
status: JobStatus.InvalidJobAgent,
message: "Failed to get ref for github action job",
})
.where(eq(job.id, je.id));
return;
}

logger.info(`Creating workflow dispatch for job ${je.id}...`, {
owner: parsed.data.owner,
repo: parsed.data.repo,
workflow_id: parsed.data.workflowId,
ref: ghOrg.branch,
inputs: {
job_id: je.id,
},
owner: parsedConfig.owner,
repo: parsedConfig.repo,
workflow_id: parsedConfig.workflowId,
ref,
inputs: { job_id: je.id },
});

await octokit.actions.createWorkflowDispatch({
owner: parsed.data.owner,
repo: parsed.data.repo,
workflow_id: parsed.data.workflowId,
ref: ghOrg.branch,
owner: parsedConfig.owner,
repo: parsedConfig.repo,
workflow_id: parsedConfig.workflowId,
ref,
inputs: { job_id: je.id },
headers: {
"X-GitHub-Api-Version": "2022-11-28",
authorization: `Bearer ${installationToken.token}`,
},
headers,
});
};
5 changes: 0 additions & 5 deletions apps/webservice/src/app/[workspaceSlug]/(app)/_components/CreateDeployment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,6 @@ export const CreateDeploymentDialog: React.FC<{
jobAgent={selectedJobAgent}
value={value ?? {}}
onChange={onChange}
githubFormStyleConfig={{
className:
"flex flex-col gap-2 items-center w-[450px]",
buttonWidth: "w-[450px]",
}}
/>
</FormControl>
<FormMessage />
Expand Down
4 changes: 0 additions & 4 deletions ...eSlug]/(app)/systems/[systemSlug]/deployments/[deploymentSlug]/hooks/CreateHookDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,6 @@ export const CreateHookDialog: React.FC<CreateHookDialogProps> = ({
jobAgent={jobAgent}
value={value ?? {}}
onChange={onChange}
githubFormStyleConfig={{
className: "flex flex-col gap-2 items-center w-[450px]",
buttonWidth: "w-[450px]",
}}
/>
</FormControl>
<FormMessage />
Expand Down
4 changes: 0 additions & 4 deletions ...aceSlug]/(app)/systems/[systemSlug]/deployments/[deploymentSlug]/hooks/EditHookDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,6 @@ export const EditHookDialog: React.FC<EditHookDialogProps> = ({
jobAgent={jobAgent}
value={value ?? {}}
onChange={onChange}
githubFormStyleConfig={{
className: "flex flex-col gap-2 items-center w-[450px]",
buttonWidth: "w-[450px]",
}}
/>
</FormControl>
<FormMessage />
Expand Down
1 change: 0 additions & 1 deletion .../src/app/[workspaceSlug]/(app)/systems/[systemSlug]/runbooks/create/CreateRunbookForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ export const CreateRunbook: React.FC<{
workspace={workspace}
value={value}
onChange={onChange}
githubFormStyleConfig={{ buttonWidth: "w-72" }}
/>
</FormControl>
<FormMessage />
Expand Down
31 changes: 14 additions & 17 deletions ...integration)/github/OrgActionDropdown.tsx → ...egration)/github/EntityActionDropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,22 @@ import {
} from "@ctrlplane/ui/dropdown-menu";

import { DisconnectDropdownActionButton } from "./DisconnectDropdownActionButton";
import { GithubRemoveOrgDialog } from "./GithubRemoveOrgDialog";
import { GithubRemoveEntityDialog } from "./GithubRemoveEntityDialog";

type OrgActionDropdownProps = {
githubConfig: {
url: string;
botName: string;
clientId: string;
};
org: schema.GithubOrganization;
type EntityActionDropdownProps = {
githubConfig: { url: string; botName: string; clientId: string };
entity: schema.GithubEntity;
};

export const OrgActionDropdown: React.FC<OrgActionDropdownProps> = ({
export const EntityActionDropdown: React.FC<EntityActionDropdownProps> = ({
githubConfig,
org,
entity,
}) => {
const { type, slug, installationId } = entity;
const link =
type === "organization"
? `${githubConfig.url}/organizations/${slug}/settings/installations/${installationId}`
: `${githubConfig.url}/settings/installations/${installationId}`;
return (
<DropdownMenu>
<DropdownMenuTrigger asChild>
Expand All @@ -36,19 +37,15 @@ export const OrgActionDropdown: React.FC<OrgActionDropdownProps> = ({
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent>
<Link
href={`${githubConfig.url}/organizations/${org.organizationName}/settings/installations/${org.installationId}`}
target="_blank"
rel="noopener noreferrer"
>
<Link href={link} target="_blank" rel="noopener noreferrer">
<DropdownMenuItem>
Configure <IconExternalLink className="ml-2 h-4 w-4" />
</DropdownMenuItem>
</Link>

<GithubRemoveOrgDialog githubOrganization={org}>
<GithubRemoveEntityDialog githubEntity={entity}>
<DisconnectDropdownActionButton />
</GithubRemoveOrgDialog>
</GithubRemoveEntityDialog>
</DropdownMenuContent>
</DropdownMenu>
);
Expand Down
32 changes: 16 additions & 16 deletions ...ntegration)/github/GithubAddOrgDialog.tsx → ...gration)/github/GithubAddEntityDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,26 @@ import {
DialogTrigger,
} from "@ctrlplane/ui/dialog";

import type { GithubOrg } from "./SelectPreconnectedOrgDialogContent";
import { SelectPreconnectedOrgDialogContent } from "./SelectPreconnectedOrgDialogContent";
import { SelectPreconnectedEntityDialogContent } from "./SelectPreconnectedEntityDialogContent";

type GithubAddOrgDialogProps = {
type GithubAddEntityDialogProps = {
githubUser: GithubUser;
children: React.ReactNode;
githubConfig: {
url: string;
botName: string;
clientId: string;
};
validOrgsToAdd: GithubOrg[];
githubConfig: { url: string; botName: string; clientId: string };
validEntitiesToAdd: {
installationId: number;
type: "user" | "organization";
slug: string;
avatarUrl: string;
}[];
workspaceId: string;
};

export const GithubAddOrgDialog: React.FC<GithubAddOrgDialogProps> = ({
export const GithubAddEntityDialog: React.FC<GithubAddEntityDialogProps> = ({
githubUser,
children,
githubConfig,
validOrgsToAdd,
validEntitiesToAdd,
workspaceId,
}) => {
const [dialogStep, setDialogStep] = useState<"choose-org" | "pre-connected">(
Expand All @@ -52,15 +52,15 @@ export const GithubAddOrgDialog: React.FC<GithubAddOrgDialogProps> = ({
<>
<DialogHeader>
<DialogTitle>Connect a new Organization</DialogTitle>
{validOrgsToAdd.length === 0 && (
{validEntitiesToAdd.length === 0 && (
<DialogDescription>
Install the ctrlplane Github app on an organization to connect
it to your workspace.
</DialogDescription>
)}
</DialogHeader>

{validOrgsToAdd.length > 0 && (
{validEntitiesToAdd.length > 0 && (
<Alert variant="secondary">
<IconBulb className="h-5 w-5" />
<AlertTitle>Connect an organization</AlertTitle>
Expand Down Expand Up @@ -99,7 +99,7 @@ export const GithubAddOrgDialog: React.FC<GithubAddOrgDialogProps> = ({
<Button variant="outline">Connect new organization</Button>
</Link>

{validOrgsToAdd.length > 0 && (
{validEntitiesToAdd.length > 0 && (
<div className="flex flex-grow justify-end">
<Button
className="w-fit"
Expand All @@ -115,8 +115,8 @@ export const GithubAddOrgDialog: React.FC<GithubAddOrgDialogProps> = ({
)}

{dialogStep === "pre-connected" && (
<SelectPreconnectedOrgDialogContent
githubOrgs={validOrgsToAdd}
<SelectPreconnectedEntityDialogContent
githubEntities={validEntitiesToAdd}
githubUser={githubUser}
workspaceId={workspaceId}
onNavigateBack={() => setDialogStep("choose-org")}
Expand Down
Loading
Loading