Ensure qlconfig file is created when config parsing in cli is on by aeisenberg · Pull Request #1527 · github/codeql-action · GitHub

Previously, with the config parsing in the cli feature flag turned on, the CLI was not able to download packs from other registries. This PR adds the codeql-action changes required for this. The CLI changes will be in a separate, internal PR.

This is a fix for a potential bug that has not yet been raised, so no changelog post is necessary.

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.

I expect that init-with-registries.yml will fail since in this case the config.yml was not being used. It should pass if the CODEQL_PASS_CONFIG_TO_CLI is explicitly set to false. It will also pass after the corresponding CLI work is merged.

Note that even though the PR Check - Packaging: Download using registries jobs are passing, they are not actually testing what they should be. Because we are not using extra private PATs in this repository, we can't test on other registries. I will be creating an internal PR with an integration test.

This PR also depends on a change to the CLI where we allow the --qlconfig-file option to be passed to database init.

Previously, with the config parsing in the cli feature flag turned on, the CLI was not able to download packs from other registries. This PR adds the codeql-action changes required for this. The CLI changes will be in a separate, internal PR.

This is ready for review, even though it is not complete until the associated change in the codeql cli is available.

…-in-cli

This looks mostly good. Comments are mainly around maintainability and filling a few gaps in my knowledge.

Co-authored-by: Henry Mercer <henry.mercer@me.com>

@henrymercer can you take another look? There is a minor semantic change. If a user explicitly sets a CODEQL_REGISTRIES_AUTH env var, then this will override the generated env var based on inputs.

I think this provides a nice back door so that in case there are issues with the action not generating or passing the qlconfig or CODEQL_REGISTRIES_AUTH, a workflow can do that explicitly.

…_AUTH

I'm going to hold off on merging this until the related PR on the CLI side is merged as well. The assumption here is that the feature will be enabled officially for v2.12.3, but we won't be certain until the CLI PR is merged.

v2.12.3 is now available. I am picking this PR up again.

@henrymercer, can you take another look? Nothing has changed since your last approval.

Suggested change

      
             * Versions 2.12.3+ of the CodeQL CLI support the `--qlconfig` flag in calls to `database init`.
          
             * Versions 2.12.4+ of the CodeQL CLI support the `--qlconfig` flag in calls to `database init`.

Thanks for the review. Addressed your comments and need another approvel.

aeisenberg requested a review from a team as a code owner February 6, 2023 21:28

aeisenberg force-pushed the aeisenberg/qlconfig-in-cli branch from 8a54dcb to c7ecff7 Compare February 6, 2023 21:30

aeisenberg force-pushed the aeisenberg/qlconfig-in-cli branch 2 times, most recently from b00e4e4 to 328773c Compare February 6, 2023 21:37

aeisenberg marked this pull request as draft February 6, 2023 21:37

aeisenberg removed the request for review from a team February 6, 2023 21:37

aeisenberg force-pushed the aeisenberg/qlconfig-in-cli branch 3 times, most recently from 75fa980 to 826edd9 Compare February 6, 2023 23:44

aeisenberg force-pushed the aeisenberg/qlconfig-in-cli branch from 826edd9 to bbe8d37 Compare February 7, 2023 18:41

aeisenberg marked this pull request as ready for review February 7, 2023 20:11

aeisenberg requested a review from henrymercer February 8, 2023 16:30

Merge remote-tracking branch 'upstream/main' into aeisenberg/qlconfig…

e2f72f1

…-in-cli

henrymercer reviewed Feb 9, 2023

View reviewed changes

aeisenberg commented Feb 9, 2023

View reviewed changes

Apply suggestions from code review

3c81243

Co-authored-by: Henry Mercer <henry.mercer@me.com>

aeisenberg force-pushed the aeisenberg/qlconfig-in-cli branch from 999f341 to 3c81243 Compare February 9, 2023 20:26

aeisenberg requested a review from henrymercer February 9, 2023 20:26

aeisenberg force-pushed the aeisenberg/qlconfig-in-cli branch 2 times, most recently from a5f7338 to 60afa75 Compare February 9, 2023 21:30

Add tests for generateRegistries with an existing CODEQL_REGISTRIES…

5492b7d

…_AUTH

aeisenberg force-pushed the aeisenberg/qlconfig-in-cli branch from 60afa75 to 5492b7d Compare February 9, 2023 21:37

henrymercer reviewed Feb 10, 2023

View reviewed changes

Comment thread src/config-utils.ts

henrymercer previously approved these changes Feb 10, 2023

View reviewed changes

aeisenberg commented Feb 17, 2023

View reviewed changes

Comment thread src/codeql.ts Outdated

Update src/codeql.ts

736263f

aeisenberg dismissed henrymercer’s stale review via 736263f February 17, 2023 21:22

Merge branch 'main' into aeisenberg/qlconfig-in-cli

8f19113

aeisenberg requested a review from henrymercer February 27, 2023 05:59

henrymercer previously approved these changes Feb 27, 2023

View reviewed changes

Address comments from PR

98d24e5

aeisenberg dismissed henrymercer’s stale review via 98d24e5 February 27, 2023 17:59

aeisenberg requested a review from henrymercer February 27, 2023 17:59

aeisenberg enabled auto-merge February 27, 2023 18:00

henrymercer approved these changes Feb 27, 2023

View reviewed changes

aeisenberg merged commit a589d40 into main Feb 27, 2023

aeisenberg deleted the aeisenberg/qlconfig-in-cli branch February 27, 2023 18:26

henrymercer mentioned this pull request Mar 6, 2023

Update bundle to 2.12.4 #1561

Merged

3 tasks

github-actions Bot mentioned this pull request Mar 10, 2023

Merge main into releases/v2 #1570

Merged

6 tasks

aliscco mentioned this pull request Apr 28, 2023

[Snyk] Fix for 3 vulnerabilities aliscco/codeql-action#351

Open

	* Versions 2.12.3+ of the CodeQL CLI support the `--qlconfig` flag in calls to `database init`.
	* Versions 2.12.4+ of the CodeQL CLI support the `--qlconfig` flag in calls to `database init`.

Ensure qlconfig file is created when config parsing in cli is on by aeisenberg · Pull Request #1527 · github/codeql-action · GitHub

Ensure qlconfig file is created when config parsing in cli is on - #1527

Ensure qlconfig file is created when config parsing in cli is on#1527aeisenberg merged 7 commits intomaingithub/codeql-action:mainfrom aeisenberg/qlconfig-in-cligithub/codeql-action:aeisenberg/qlconfig-in-cliCopy head branch name to clipboard

Conversation

aeisenberg commented Feb 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge / deployment checklist

Uh oh!

aeisenberg commented Feb 6, 2023

Uh oh!

aeisenberg commented Feb 7, 2023

Uh oh!

aeisenberg commented Feb 7, 2023

Uh oh!

henrymercer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aeisenberg commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

aeisenberg commented Feb 10, 2023

Uh oh!

Uh oh!

aeisenberg commented Feb 27, 2023

Uh oh!

aeisenberg commented Feb 27, 2023

Uh oh!

Uh oh!

henrymercer Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

aeisenberg commented Feb 27, 2023

Uh oh!

henrymercer left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Ensure qlconfig file is created when config parsing in cli is on#1527
aeisenberg merged 7 commits into
mainfrom
aeisenberg/qlconfig-in-cli

aeisenberg commented Feb 6, 2023 •
edited
Loading

aeisenberg commented Feb 9, 2023 •
edited
Loading