From c36300071225665e3a78f06c6047aa8f6be69fbe Mon Sep 17 00:00:00 2001 From: John Breault Date: Mon, 21 Sep 2026 20:33:13 -0400 Subject: [PATCH] Improve UX around --os-include/--os-exclude and --destination-url - Log a warning when an --os-include/--os-exclude value does not match any release asset OS identifier actually encountered, listing the valid identifiers found (e.g. linux64, linux-arm64, osx64, win64) so typos like "linux" or "win" are caught instead of silently filtering out every OS-specific asset. - Automatically assume https:// for --destination-url when no protocol scheme is provided, instead of failing later with an opaque "unsupported protocol scheme" error. - Clarify README docs for --os-include, --os-exclude and --destination-url accordingly. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 12 ++++++------ internal/pull/pull.go | 34 ++++++++++++++++++++++++++++++++++ internal/pull/pull_test.go | 32 ++++++++++++++++++++++++++++++++ internal/push/push.go | 4 ++++ 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index e7389ad..cfc2582 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ If your GitHub Enterprise Server instance is on a completely isolated network wh From a machine with access to both GitHub.com and GitHub Enterprise Server use the `./codeql-action-sync sync` command to copy the CodeQL Action and bundles. **Required Arguments:** -* `--destination-url` - The URL of the GitHub Enterprise Server instance to push the Action to. +* `--destination-url` - The URL of the GitHub Enterprise Server instance to push the Action to. If no protocol scheme is included (e.g. you pass `github.example.com` instead of `https://github.example.com`), `https://` is assumed. * `--destination-token` - A [Personal Access Token](https://docs.github.com/en/enterprise/user/github/authenticating-to-github/creating-a-personal-access-token) for the destination GitHub Enterprise Server instance. If the destination repository is in an organization that does not yet exist or that you are not an owner of, your token will need to have the `site_admin` scope in order to create the organization or update the repository in it. The organization can also be created manually or an existing organization that you own can be used, in which case the `repo` and `workflow` scopes are sufficient. The token can also be provided by setting the `CODEQL_ACTION_SYNC_TOOL_DESTINATION_TOKEN` environment variable. **Optional Arguments:** @@ -29,8 +29,8 @@ From a machine with access to both GitHub.com and GitHub Enterprise Server use t * `--actions-admin-user` - The name of the Actions admin user, which will be used if you are updating the bundled CodeQL Action. If not specified `actions-admin` will be used. * `--force` - By default the tool will not overwrite existing repositories. Providing this flag will allow it to. * `--push-ssh` - Push Git contents over SSH rather than HTTPS. To use this option you must have SSH access to your GitHub Enterprise instance configured. -* `--os-include` - A comma-separated list of operating systems (e.g. `linux64,win64`) to include CodeQL bundle release assets for. Cannot be used together with `--os-exclude`. If neither is specified, assets for all operating systems are synced. -* `--os-exclude` - A comma-separated list of operating systems (e.g. `win64,osx64`) to exclude CodeQL bundle release assets for. Cannot be used together with `--os-include`. +* `--os-include` - A comma-separated list of operating systems to include CodeQL bundle release assets for. Cannot be used together with `--os-exclude`. If neither is specified, assets for all operating systems are synced. Values must exactly match the OS identifier embedded in the asset name (for example `linux64`, `linux-arm64`, `osx64`, `win64` — not shorthand like `linux` or `win`). If a value does not match any asset in the releases being pulled, a warning listing the OS identifiers that were actually found is logged. +* `--os-exclude` - A comma-separated list of operating systems to exclude CodeQL bundle release assets for. Cannot be used together with `--os-include`. Values must exactly match the OS identifier embedded in the asset name (for example `linux64`, `linux-arm64`, `osx64`, `win64`). * `--compression-format` - The compression format of CodeQL bundle release assets to sync, either `gz` or `zst`. If not specified, both compression formats are synced. ### I don't have a machine that can access both GitHub.com and GitHub Enterprise Server. @@ -39,8 +39,8 @@ From a machine with access to GitHub.com use the `./codeql-action-sync pull` com **Optional Arguments:** * `--cache-dir` - The directory in which to store data downloaded from GitHub.com. If not specified a directory next to the sync tool will be used. * `--source-token` - A token to access the API of GitHub.com. This is normally not required, but can be provided if you have issues with API rate limiting. The token does not need to have any scopes. -* `--os-include` - A comma-separated list of operating systems (e.g. `linux64,win64`) to include CodeQL bundle release assets for. Cannot be used together with `--os-exclude`. If neither is specified, assets for all operating systems are synced. -* `--os-exclude` - A comma-separated list of operating systems (e.g. `win64,osx64`) to exclude CodeQL bundle release assets for. Cannot be used together with `--os-include`. +* `--os-include` - A comma-separated list of operating systems to include CodeQL bundle release assets for. Cannot be used together with `--os-exclude`. If neither is specified, assets for all operating systems are synced. Values must exactly match the OS identifier embedded in the asset name (for example `linux64`, `linux-arm64`, `osx64`, `win64` — not shorthand like `linux` or `win`). If a value does not match any asset in the releases being pulled, a warning listing the OS identifiers that were actually found is logged. +* `--os-exclude` - A comma-separated list of operating systems to exclude CodeQL bundle release assets for. Cannot be used together with `--os-include`. Values must exactly match the OS identifier embedded in the asset name (for example `linux64`, `linux-arm64`, `osx64`, `win64`). * `--compression-format` - The compression format of CodeQL bundle release assets to sync, either `gz` or `zst`. If not specified, both compression formats are synced. Next copy the sync tool and cache directory to another machine which has access to GitHub Enterprise Server. @@ -48,7 +48,7 @@ Next copy the sync tool and cache directory to another machine which has access Now use the `./codeql-action-sync push` command to upload the CodeQL Action and bundles to GitHub Enterprise Server. **Required Arguments:** -* `--destination-url` - The URL of the GitHub Enterprise Server instance to push the Action to. +* `--destination-url` - The URL of the GitHub Enterprise Server instance to push the Action to. If no protocol scheme is included (e.g. you pass `github.example.com` instead of `https://github.example.com`), `https://` is assumed. * `--destination-token` - A [Personal Access Token](https://docs.github.com/en/enterprise/user/github/authenticating-to-github/creating-a-personal-access-token) for the destination GitHub Enterprise Server instance. If the destination repository is in an organization that does not yet exist or that you are not an owner of, your token will need to have the `site_admin` scope in order to create the organization or update the repository in it. The organization can also be created manually or an existing organization that you own can be used, in which case the `repo` and `workflow` scopes are sufficient. The token can also be provided by setting the `CODEQL_ACTION_SYNC_TOOL_DESTINATION_TOKEN` environment variable. **Optional Arguments:** diff --git a/internal/pull/pull.go b/internal/pull/pull.go index 8c3bb27..ea1f415 100644 --- a/internal/pull/pull.go +++ b/internal/pull/pull.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "regexp" + "sort" "strings" log "github.com/sirupsen/logrus" @@ -50,6 +51,7 @@ type pullService struct { assetOSIncludes []string assetOSExcludes []string assetCompressionFormat string + seenAssetOSs map[string]bool } // shouldDownloadAsset determines whether a release asset should be downloaded, based on the @@ -64,6 +66,11 @@ func (pullService *pullService) shouldDownloadAsset(assetName string) bool { assetOS := matches[1] assetCompressionFormat := matches[2] + if pullService.seenAssetOSs == nil { + pullService.seenAssetOSs = map[string]bool{} + } + pullService.seenAssetOSs[assetOS] = true + if len(pullService.assetOSIncludes) > 0 { included := false for _, os := range pullService.assetOSIncludes { @@ -309,9 +316,36 @@ func (pullService *pullService) pullReleases() error { } } } + pullService.warnOnUnmatchedOSFilters() return nil } +// warnOnUnmatchedOSFilters logs a warning if none of the configured --os-include or --os-exclude +// values matched the OS identifier of any release asset that was actually encountered. This +// helps surface typos or incorrect assumptions about asset OS identifiers (for example passing +// "linux" when the real identifiers are "linux64"/"linux-arm64"), which would otherwise silently +// filter out every OS-specific asset without any indication of why. +func (pullService *pullService) warnOnUnmatchedOSFilters() { + seenList := make([]string, 0, len(pullService.seenAssetOSs)) + for os := range pullService.seenAssetOSs { + seenList = append(seenList, os) + } + sort.Strings(seenList) + + checkUnmatched := func(flagName string, values []string) { + for _, value := range values { + if !pullService.seenAssetOSs[value] { + log.Warnf( + "The %s value %q did not match any release asset. The OS identifiers found in the releases pulled were: %s.", + flagName, value, strings.Join(seenList, ", "), + ) + } + } + } + checkUnmatched("--os-include", pullService.assetOSIncludes) + checkUnmatched("--os-exclude", pullService.assetOSExcludes) +} + func Pull(ctx context.Context, cacheDirectory cachedirectory.CacheDirectory, sourceToken string, sourceURL string, assetOSIncludes []string, assetOSExcludes []string, assetCompressionFormat string) error { err := cacheDirectory.CheckOrCreateVersionFile(true, version.Version()) if err != nil { diff --git a/internal/pull/pull_test.go b/internal/pull/pull_test.go index d6648c9..c2c1340 100644 --- a/internal/pull/pull_test.go +++ b/internal/pull/pull_test.go @@ -9,6 +9,8 @@ import ( "github.com/github/codeql-action-sync/internal/cachedirectory" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" + log "github.com/sirupsen/logrus" + logtest "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/require" "github.com/github/codeql-action-sync/test" @@ -202,6 +204,36 @@ func TestShouldDownloadAsset(t *testing.T) { } } +func TestWarnOnUnmatchedOSFilters(t *testing.T) { + hook := logtest.NewGlobal() + defer hook.Reset() + + pullService := pullService{ + assetOSIncludes: []string{"linux", "win64"}, + seenAssetOSs: map[string]bool{"win64": true, "osx64": true}, + } + pullService.warnOnUnmatchedOSFilters() + + require.Len(t, hook.Entries, 1) + require.Equal(t, log.WarnLevel, hook.Entries[0].Level) + require.Contains(t, hook.Entries[0].Message, `The --os-include value "linux" did not match any release asset`) + require.Contains(t, hook.Entries[0].Message, "osx64, win64") +} + +func TestWarnOnUnmatchedOSFiltersNoWarningWhenAllMatch(t *testing.T) { + hook := logtest.NewGlobal() + defer hook.Reset() + + pullService := pullService{ + assetOSIncludes: []string{"linux64"}, + assetOSExcludes: []string{"win64"}, + seenAssetOSs: map[string]bool{"linux64": true, "win64": true, "osx64": true}, + } + pullService.warnOnUnmatchedOSFilters() + + require.Empty(t, hook.Entries) +} + func TestFindRelevantReleases(t *testing.T) { temporaryDirectory := test.CreateTemporaryDirectory(t) pullService := getTestPullService(t, temporaryDirectory, initialActionRepository, "") diff --git a/internal/push/push.go b/internal/push/push.go index 05439b7..3cd5f80 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -444,6 +444,10 @@ func Push(ctx context.Context, cacheDirectory cachedirectory.CacheDirectory, des } destinationURL = strings.TrimRight(destinationURL, "/") + if !strings.Contains(destinationURL, "://") { + log.Warnf("No protocol scheme specified in --destination-url %q, assuming https://.", destinationURL) + destinationURL = "https://" + destinationURL + } token := oauth2.Token{AccessToken: destinationToken} tokenSource := oauth2.StaticTokenSource( &token,