Add `codeql-action/start-proxy` by aibaars · Pull Request #2376 · github/codeql-action · GitHub

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.

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

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 🙇

Suggested change

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

to match the other experimental actions.

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

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]

TIL about saveState and getState 👍

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 👍

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.

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

Nice!

@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?

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

Kicking off the release now 🙏

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

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

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

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

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

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

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

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

Add `codeql-action/start-proxy` by aibaars · Pull Request #2376 · github/codeql-action · GitHub

Add codeql-action/start-proxy - #2376

Add codeql-action/start-proxy#2376aibaars merged 5 commits intomaingithub/codeql-action:mainfrom aibaars/start-proxygithub/codeql-action:aibaars/start-proxyCopy head branch name to clipboard

Conversation

aibaars commented Jul 17, 2024 • edited by angelapwen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge / deployment checklist

Uh oh!

github-actions Bot commented Jul 18, 2024

Uh oh!

angelapwen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

angelapwen commented Jul 23, 2024

Uh oh!

angelapwen left a comment

Choose a reason for hiding this comment

Uh oh!

angelapwen left a comment

Choose a reason for hiding this comment

Uh oh!

henrymercer commented Jul 24, 2024

Uh oh!

angelapwen commented Jul 24, 2024

Uh oh!

angelapwen commented Jul 25, 2024

Uh oh!

angelapwen commented Jul 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add codeql-action/start-proxy#2376
aibaars merged 5 commits into
mainfrom
aibaars/start-proxy

aibaars commented Jul 17, 2024 •
edited by angelapwen
Loading