Add `codeql-action/start-proxy` #2376

aibaars · 2024-07-17T13:30:09Z

This pull request adds a new experimental action, codeql-action/start-proxy, which starts the HTTP proxy server that is also used by dependabot update jobs. The Action is used for an experimental implementation leveraging Dependabot's configurations and secrets by CodeQL Default Setup jobs that require access to private maven repositories.

Merge / deployment checklist

Confirm this change is backwards compatible with existing workflows.
Confirm the readme has been updated if necessary.
Confirm the changelog has been updated if necessary.

github-actions · 2024-07-18T11:36:01Z

Pushed a commit to update the checked-in dependencies. Please mark the PR as ready for review to trigger PR checks.

angelapwen

Overall looks good to me 👍 added a few in-line comments.

Additional nit: could you make all the variables in the TS files camelCase to match the repo convention? Thanks 🙇

angelapwen · 2024-07-23T10:07:31Z

start-proxy/action.yml

@@ -0,0 +1,22 @@
+name: "CodeQL: Start proxy"
+description: "Start HTTP proxy server"


Suggested change

description: "Start HTTP proxy server"

description: "[Experimental] Start HTTP proxy server"

to match the other experimental actions.

angelapwen · 2024-07-23T10:10:01Z

src/start-proxy-action.ts

+  },
+  {
+    name: "organizationName",
+    value: "GitHub ic.",


Gut check: is this meant to be GitHub inc or something like that? I wasn't able to find the expected value in our internal issues.

Well spotted, it comes from the following place, which has the same spelling mistake ;-)

https://github.com/github/dependabot-action/blob/5d0fcfd7688762c2f00f2c9176acc5b185ec6a60/src/proxy.ts#L21-L23

angelapwen · 2024-07-23T10:13:40Z

src/start-proxy-action.ts

+      });
+      subprocess.on("exit", (code) => {
+        if (code !== 0) {
+          port = Math.floor(Math.random() * (65535 - 49152) + 49152);


Just curious: where does this rand calculation come from?

I just wanted to write port = random_int_between(49152, 65535), but I could not find such a function in typescript. The range [49152 .. 65535] is the dynamic ports range, meant for temporary/client stuff. The randomness is to avoid collisions.

Ah got it — I wasn't sure why those numbers were chosen but makes sense that it's the range for dynamic ports 👍

Co-pilot generated the following comment for the code:

// If the proxy failed to start, try a different port from the ephemeral range [49152, 65535]

angelapwen · 2024-07-23T10:15:59Z

src/start-proxy-action.ts

+      );
+      subprocess.unref();
+      if (subprocess.pid) {
+        core.saveState("proxy-process-pid", `${subprocess.pid}`);


TIL about saveState and getState 👍

angelapwen · 2024-07-23T11:11:59Z

src/start-proxy-action-post.ts

+      `start-proxy post-action step failed: ${wrapError(error).message}`,
+    );
+  }
+  if (core.isDebug()) {


You might want to use config.debugMode instead here; we use this throughout to take into account the debug input parameter in the init action:

codeql-action/src/init-action.ts

Line 285 in 0e346f2

debugMode: getOptionalInput("debug") === "true" || core.isDebug(),

Nice! Note, the start proxy action may have failed before the init step has run, so I added some code that tries to get the config and falls back on core.isDebug() if it is undefined.

Makes sense to me 👍

angelapwen · 2024-07-23T13:20:16Z

We may want to add a changelog note indicating this is a new experimental action, probably something similar to this one resolve-environment: https://github.com/github/codeql-action/blob/main/CHANGELOG.md#2201---21-jun-2023.

angelapwen

Thanks for addressing all the comments — looks great to me!

angelapwen

Nice!

henrymercer · 2024-07-24T16:02:23Z

@angelapwen When we do the next release, we'll probably want to add a commit to the v3 -> v2 backport PR to run this Action on Node 16. I wonder whether it's worth doing a release now so we don't forget — what do you think?

angelapwen · 2024-07-24T16:26:44Z

Good point @henrymercer — I'll release earlier in the workday tomorrow.

angelapwen · 2024-07-25T08:04:02Z

Kicking off the release now 🙏

angelapwen · 2024-07-25T08:57:39Z

Done in #2390!

aibaars force-pushed the aibaars/start-proxy branch from 37674bb to 19fdcff Compare July 18, 2024 11:24

aibaars added the Update dependencies Trigger PR workflow to update dependencies label Jul 18, 2024

github-actions bot removed the Update dependencies Trigger PR workflow to update dependencies label Jul 18, 2024

aibaars marked this pull request as ready for review July 18, 2024 12:02

aibaars requested a review from a team as a code owner July 18, 2024 12:02

aibaars marked this pull request as draft July 18, 2024 12:02

aibaars marked this pull request as ready for review July 19, 2024 14:21

angelapwen previously approved these changes Jul 23, 2024

View reviewed changes

angelapwen reviewed Jul 23, 2024

View reviewed changes

aibaars dismissed angelapwen’s stale review via 58e34d6 July 24, 2024 09:39

aibaars added 4 commits July 24, 2024 11:42

Add node-forge to package.json

5669f66

Add codeql-action/start-proxy

7b43b7c

Print proxy log when debugging is enabled

6186179

Address comments

4733419

angelapwen previously approved these changes Jul 24, 2024

View reviewed changes

start-proxy: get binary from toolcache

77e4172

aibaars dismissed angelapwen’s stale review via 77e4172 July 24, 2024 11:53

aibaars force-pushed the aibaars/start-proxy branch from 58e34d6 to 77e4172 Compare July 24, 2024 11:53

angelapwen approved these changes Jul 24, 2024

View reviewed changes

aibaars merged commit f67c9cd into main Jul 24, 2024
310 checks passed

aibaars deleted the aibaars/start-proxy branch July 24, 2024 12:26

This was referenced Jul 25, 2024

Merge main into releases/v3 #2388

Merged

Merge releases/v3 into releases/v2 #2390

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `codeql-action/start-proxy` #2376

Add `codeql-action/start-proxy` #2376

aibaars commented Jul 17, 2024 •

edited by angelapwen

Loading

github-actions bot commented Jul 18, 2024

angelapwen left a comment

angelapwen Jul 23, 2024

angelapwen Jul 23, 2024

aibaars Jul 23, 2024

angelapwen Jul 23, 2024

aibaars Jul 23, 2024

angelapwen Jul 23, 2024

aibaars Jul 24, 2024

angelapwen Jul 23, 2024

angelapwen Jul 23, 2024

aibaars Jul 24, 2024

angelapwen Jul 24, 2024

angelapwen commented Jul 23, 2024

angelapwen left a comment

angelapwen left a comment

henrymercer commented Jul 24, 2024

angelapwen commented Jul 24, 2024

angelapwen commented Jul 25, 2024

angelapwen commented Jul 25, 2024

		@@ -0,0 +1,22 @@
		name: "CodeQL: Start proxy"
		description: "Start HTTP proxy server"

	description: "Start HTTP proxy server"
	description: "[Experimental] Start HTTP proxy server"

Add codeql-action/start-proxy #2376

Add codeql-action/start-proxy #2376

Conversation

aibaars commented Jul 17, 2024 • edited by angelapwen Loading

Merge / deployment checklist

github-actions bot commented Jul 18, 2024

angelapwen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angelapwen commented Jul 23, 2024

angelapwen left a comment

Choose a reason for hiding this comment

angelapwen left a comment

Choose a reason for hiding this comment

henrymercer commented Jul 24, 2024

angelapwen commented Jul 24, 2024

angelapwen commented Jul 25, 2024

angelapwen commented Jul 25, 2024

Add `codeql-action/start-proxy` #2376

Add `codeql-action/start-proxy` #2376

aibaars commented Jul 17, 2024 •

edited by angelapwen

Loading