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.

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

Add support for actions language #2572

Merged
merged 6 commits into from
Nov 1, 2024
Merged

Add support for actions language #2572

merged 6 commits into from
Nov 1, 2024

Conversation

dbartol
Copy link
Contributor

@dbartol dbartol commented Nov 1, 2024

This PR adds support for the new actions CodeQL language, which will be part of the upcoming 2.19.3 release of the CodeQL bundle. In addition to adding actions to the list of supported languages, this PR also fixes a couple of issues I encountered while testing the new language:

The CODEQL_ACTION_EXTRA_OPTIONS environment variable, used for setting additional CLI options, now supports YAML, rather than just JSON. This makes it easier to specify complex options in a workflow file without complicated escaping.

I added support for the kind property for pack registries, to allow downloading packs from GitHub repo-based registries, in addition to the existing Docker-based registries.

@dbartol dbartol requested a review from a team as a code owner November 1, 2024 14:10
henrymercer
henrymercer previously approved these changes Nov 1, 2024
Comment on lines 68 to 69
// Kind of registry, either "github" or "docker". Default is "docker".
kind?: "github" | "docker";
Copy link
Contributor

@henrymercer henrymercer Nov 1, 2024

Choose a reason for hiding this comment

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

I wonder if folks might confuse "docker" with docker hub when I think we're talking about GHCR (or I guess any OCI container registry?). We could say something like "repository" / "container"? I'm not sure what's best.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docker value is already baked into the CLI. We could consider changing it there (adding ghcr or whatever as an alias), but I don't think it's worth the effort. docker is the default, so nobody ever has to actually specify it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just adding a comment to say that the only docker registry we support is GHCR. This is public code and I wouldn't want to give any false impressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment

@@ -1,5 +1,6 @@
// All the languages supported by CodeQL
export enum Language {
actions = "actions",
Copy link
Contributor

Choose a reason for hiding this comment

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

Thought: is it worth adding a warning in the init Action when using this language that actions isn't officially supported yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They'll already have to enable the experimental language support in the CLI by setting CODEQL_ENABLE_EXPERIMENTAL_FEATURES=true, so I think they'll already know it's not officially supported yet.

Comment on lines 68 to 69
// Kind of registry, either "github" or "docker". Default is "docker".
kind?: "github" | "docker";
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just adding a comment to say that the only docker registry we support is GHCR. This is public code and I wouldn't want to give any false impressions.

@@ -64,6 +64,9 @@ export interface RegistryConfigNoCredentials {

// List of globs that determine which packs are associated with this registry.
packages: string[] | string;

// Kind of registry, either "github" or "docker". Default is "docker".
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Kind of registry, either "github" or "docker". Default is "docker".
// Kind of registry, either "github" or "docker". Default is "docker".
// The only docker registry that codeql supports is the GitHub Container Registry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, didn't see this suggestion until I'd already added a different comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think what you have now is fine. Thanks for updating.

@dbartol dbartol merged commit cbe1897 into main Nov 1, 2024
273 checks passed
@dbartol dbartol deleted the dbartol/actions-analysis branch November 1, 2024 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants