diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 31ef955f0..c6b68a954 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -86,5 +86,5 @@ A member of the core team will [triage](../docs/triage.md) the design proposal. [How to Contribute to Open Source]: https://opensource.guide/how-to-contribute/ [Using Pull Requests]: https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-requests [GitHub Help]: https://docs.github.com/ -[CLI Design System]: https://primer.style/cli/ +[CLI Design System]: /docs/primer/ [Google Docs Template]: https://docs.google.com/document/d/1JIRErIUuJ6fTgabiFYfCH3x91pyHuytbfa0QLnTfXKM/edit#heading=h.or54sa47ylpg diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 1a850c9b3..4c08abeab 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -7,7 +7,6 @@ updates: ignore: - dependency-name: "*" update-types: - - version-update:semver-minor - version-update:semver-major - package-ecosystem: "github-actions" directory: "/" diff --git a/.github/workflows/bump-go.yml b/.github/workflows/bump-go.yml index 627578853..3358b81aa 100644 --- a/.github/workflows/bump-go.yml +++ b/.github/workflows/bump-go.yml @@ -12,6 +12,17 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: 'go.mod' + - name: Bump Go version + env: + GIT_COMMITTER_NAME: cli automation + GIT_AUTHOR_NAME: cli automation + GIT_COMMITTER_EMAIL: noreply@github.com + GIT_AUTHOR_EMAIL: noreply@github.com + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | bash .github/workflows/scripts/bump-go.sh --apply go.mod diff --git a/.github/workflows/scripts/bump-go.sh b/.github/workflows/scripts/bump-go.sh index 74f5b3cbe..812b90417 100755 --- a/.github/workflows/scripts/bump-go.sh +++ b/.github/workflows/scripts/bump-go.sh @@ -76,6 +76,9 @@ if [[ "$CURRENT_TOOLCHAIN_DIRECTIVE" != "go$TOOLCHAIN_VERSION" ]]; then echo " • toolchain $CURRENT_TOOLCHAIN_DIRECTIVE → go$TOOLCHAIN_VERSION" fi +# ---- Run go mod tidy to ensure .0 minor version is added to go directive ---- +go mod tidy + rm -f "$GO_MOD.bak" git add "$GO_MOD" diff --git a/acceptance/testdata/secret/secret-org-with-selected-visibility.txtar b/acceptance/testdata/secret/secret-org-with-selected-visibility.txtar new file mode 100644 index 000000000..9d6bfed84 --- /dev/null +++ b/acceptance/testdata/secret/secret-org-with-selected-visibility.txtar @@ -0,0 +1,44 @@ +# Setup environment variables used for testscript +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} +env2upper SECRET_NAME=${SCRIPT_NAME}_${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create ${ORG}/${REPO} --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Confirm organization secret does not exist, will fail admin:org scope missing +exec gh secret list --org ${ORG} +! stdout ${SECRET_NAME} + +# Set an organization secret with no shared visibility, but no repos +exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --no-repos-selected + +# Defer organization secret cleanup +defer gh secret delete ${SECRET_NAME} --org ${ORG} + +# Verify new organization secret exists with shared visibility +exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME} --jq '.visibility' +stdout selected + +# Verify the secret is not shared with any repositories +exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories | length' +stdout 0 + +# Set the same organization secret with shared visibility to the previously created repository +exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --repos ${REPO} + +# Verify the secret is now shared with the repository +exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories[0].name' +stdout ${REPO} + +# Set the same organization secret with shared visibility back to no repositories selected +exec gh secret set ${SECRET_NAME} --org ${ORG} --body 'just an organization secret' --no-repos-selected + +# Verify the secret is not shared with any repositories +exec gh api -X GET /orgs/${ORG}/actions/secrets/${SECRET_NAME}/repositories --jq '.repositories | length' +stdout 0 diff --git a/acceptance/testdata/secret/secret-org.txtar b/acceptance/testdata/secret/secret-org.txtar index 7d383009c..3465628b7 100644 --- a/acceptance/testdata/secret/secret-org.txtar +++ b/acceptance/testdata/secret/secret-org.txtar @@ -1,4 +1,6 @@ # Setup environment variables used for testscript +# This script will most likely fail because you are most likely targeting a repo that is not public and an org +# that is not on the right plan: https://docs.github.com/en/actions/how-tos/security-for-github-actions/security-guides/using-secrets-in-github-actions#creating-secrets-for-an-organization env REPO=${SCRIPT_NAME}-${RANDOM_STRING} env2upper SECRET_NAME=${SCRIPT_NAME}_${RANDOM_STRING} diff --git a/api/queries_pr.go b/api/queries_pr.go index 1d394e864..525418a11 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -85,25 +85,15 @@ type PullRequest struct { Assignees Assignees AssignedActors AssignedActors - // AssignedActorsUsed is a GIGANTIC hack to carry around whether we expected AssignedActors to be requested - // on this PR. This is required because the Feature Detection of support for AssignedActors occurs inside the - // PR Finder, but knowledge of support is required at the command level. However, we can't easily construct - // the feature detector at the command level because it needs knowledge of the BaseRepo, which is only available - // inside the PR Finder. This is bad and we should feel bad. - // - // The right solution is to extract argument parsing from the PR Finder into each command, so that we have access - // to the BaseRepo and can construct the feature detector there. This is what happens in the issue commands with - // `shared.ParseIssueFromArg`. - AssignedActorsUsed bool - Labels Labels - ProjectCards ProjectCards - ProjectItems ProjectItems - Milestone *Milestone - Comments Comments - ReactionGroups ReactionGroups - Reviews PullRequestReviews - LatestReviews PullRequestReviews - ReviewRequests ReviewRequests + Labels Labels + ProjectCards ProjectCards + ProjectItems ProjectItems + Milestone *Milestone + Comments Comments + ReactionGroups ReactionGroups + Reviews PullRequestReviews + LatestReviews PullRequestReviews + ReviewRequests ReviewRequests ClosingIssuesReferences ClosingIssuesReferences } diff --git a/docs/install_linux.md b/docs/install_linux.md index b150c6205..15e843f4e 100644 --- a/docs/install_linux.md +++ b/docs/install_linux.md @@ -14,7 +14,7 @@ our release schedule. Install: ```bash -(type -p wget >/dev/null || (sudo apt update && sudo apt-get install wget -y)) \ +(type -p wget >/dev/null || (sudo apt update && sudo apt install wget -y)) \ && sudo mkdir -p -m 755 /etc/apt/keyrings \ && out=$(mktemp) && wget -nv -O$out https://cli.github.com/packages/githubcli-archive-keyring.gpg \ && cat $out | sudo tee /etc/apt/keyrings/githubcli-archive-keyring.gpg > /dev/null \ diff --git a/docs/primer/README.md b/docs/primer/README.md new file mode 100644 index 000000000..98625bbce --- /dev/null +++ b/docs/primer/README.md @@ -0,0 +1,15 @@ +# GitHub CLI Primer Design + +These guidelines are a collection of principles, foundations and usage guidelines for designing GitHub command line products. + +## [Components](components) + +Design guidance on how we format content in in the Terminal through text formatting, color and font weights. + +## [Foundations](foundations) + +Design concepts and constraints that can help create a better Terminal like experience for GitHub. + +## [Getting started](getting-started) + +Primer is also a design system for Terminal like implementations of GitHub. If you’re just starting out with creating those kind of experiences, here’s a list of principles and design foundations to get you started. diff --git a/docs/primer/components/README.md b/docs/primer/components/README.md new file mode 100644 index 000000000..1e002fab2 --- /dev/null +++ b/docs/primer/components/README.md @@ -0,0 +1,234 @@ +# Components + +Components are consistent, reusable patterns that we use throughout the command line tool. + +## Syntax + +We show meaning or objects through syntax such as angled brackets, square brackets, curly brackets, parenthesis, and color. + +### Branches + +Display branch names in brackets and/or cyan + +![A branch name in brackets and cyan](images/Syntax-Branch.png) + +### Labels + +Display labels in parenthesis and/or gray + +![A label name in parenthesis and gray](images/Syntax-Label.png) + +### Repository + +Display repository names in bold where appropriate + +![A repository name in bold](images/Syntax-Repo.png) + +### Help + +Use consistent syntax in [help pages](/docs/command-line-syntax.md) to explain command usage. + +#### Literal text + +Use plain text for parts of the command that cannot be changed + +```shell +gh help +``` + +The argument help is required in this command. + +#### Placeholder values + +Use angled brackets to represent a value the user must replace. No other expressions can be contained within the angled brackets. + +```shell +gh pr view +``` + +Replace "issue-number" with an issue number. + +#### Optional arguments + +Place optional arguments in square brackets. Mutually exclusive arguments can be included inside square brackets if they are separated with vertical bars. + + +```shell +gh pr checkout [--web] +``` + +The argument `--web` is optional. + +```shell +gh pr view [ | ] +``` + +The "number" and "url" arguments are optional. + +#### Required mutually exclusive arguments + +Place required mutually exclusive arguments inside braces, separate arguments with vertical bars. + +```shell +gh pr {view | create} +``` + +#### Repeatable arguments + +Ellipsis represent arguments that can appear multiple times + +```shell +gh pr close ... +``` + +#### Variable naming + +For multi-word variables use dash-case (all lower case with words separated by dashes) + + +```shell +gh pr checkout +``` + +#### Additional examples + +Optional argument with placeholder: + +```shell + [] +``` + +Required argument with mutually exclusive options: + +```shell + { | | literal} +``` + +Optional argument with mutually exclusive options: + +```shell + [ | ] +``` + +## Prompts + +Generally speaking, prompts are the CLI’s version of forms. + +- Use prompts for entering information +- Use a prompt when user intent is unclear +- Make sure to provide flags for all prompts + +### Yes/No + +Use for yes/no questions, usually a confirmation. The default (what will happen if you enter nothing and hit enter) is in caps. + +![An example of a yes/no prompt](images/Prompt-YesNo.png) + +### Short text + +Use to enter short strings of text. Enter will accept the auto fill if available + +![An example of a short text prompt](images/Prompt-ShortText.png) + +### Long text + +Use to enter large bodies of text. E key will open the user’s preferred editor, and Enter will skip. + +![An example of a long text prompt](images/Prompt-LongText.png) + +### Radio select + +Use to select one option + +![An example of a radio select prompt](images/Prompt-RadioSelect.png) + +### Multi select + +Use to select multiple options + +![An example of a multi select prompt](images/Prompt-MultiSelect.png) + +## State + +The CLI reflects how GitHub.com displays state through [color](/docs/primer/foundations#color) and [iconography](/docs/primer/foundations#iconography). + +![A collection of examples of state from various command outputs](images/States.png) + +## Progress indicators + +For processes that might take a while, include a progress indicator with context on what’s happening. + +![An example of a loading spinner when forking a repository](images/Progress-Spinner.png) + +## Headers + +When viewing output that could be unclear, headers can quickly set context for what the user is seeing and where they are. + +### Examples + +![An example of the header of the `gh pr create` command](images/Headers-Examples.png) + +The header of the `gh pr create` command reassures the user that they're creating the correct pull request. + +![An example of the header of the `gh pr list` command](images/Headers-gh-pr-list.png) + +The header of the `gh pr list` command sets context for what list the user is seeing. + +## Lists + +Lists use tables to show information. + +- State is shown in color. +- A header is used for context. +- Information shown may be branch names, dates, or what is most relevant in context. + +![An example of gh pr list](images/Lists-gh-pr-list.png) + +## Detail views + +Single item views show more detail than list views. The body of the item is rendered indented. The item’s URL is shown at the bottom. + +![An example of gh issue view](images/Detail-gh-issue-view.png) + +## Empty states + +Make sure to include empty messages in command outputs when appropriate. + +![The empty state of the gh pr status command](images/Empty-states-1.png) + +The empty state of `gh pr status` + +![The empty state of the gh issue list command](images/Empty-states-2.png) + +The empty state of `gh issue list` + +## Help pages + +Help commands can exist at any level: + +- Top level (`gh`) +- Second level (`gh [command]`) +- Third level (`gh [command] [subcommand]`) + +Each can be accessed using the `--help` flag, or using `gh help [command]`. + +Each help page includes a combination of different sections. + +### Required sections + +- Usage +- Core commands +- Flags +- Learn more +- Inherited flags + +### Other available sections + +- Additional commands +- Examples +- Arguments +- Feedback + +### Example + +![The output of gh help](images/Help.png) diff --git a/docs/primer/components/images/Detail-gh-issue-view.png b/docs/primer/components/images/Detail-gh-issue-view.png new file mode 100644 index 000000000..351859b0f Binary files /dev/null and b/docs/primer/components/images/Detail-gh-issue-view.png differ diff --git a/docs/primer/components/images/Empty-states-1.png b/docs/primer/components/images/Empty-states-1.png new file mode 100644 index 000000000..05e6b1cc9 Binary files /dev/null and b/docs/primer/components/images/Empty-states-1.png differ diff --git a/docs/primer/components/images/Empty-states-2.png b/docs/primer/components/images/Empty-states-2.png new file mode 100644 index 000000000..4e99ad37e Binary files /dev/null and b/docs/primer/components/images/Empty-states-2.png differ diff --git a/docs/primer/components/images/Headers-Examples.png b/docs/primer/components/images/Headers-Examples.png new file mode 100644 index 000000000..007f21973 Binary files /dev/null and b/docs/primer/components/images/Headers-Examples.png differ diff --git a/docs/primer/components/images/Headers-gh-pr-list.png b/docs/primer/components/images/Headers-gh-pr-list.png new file mode 100644 index 000000000..1ff4fb7d5 Binary files /dev/null and b/docs/primer/components/images/Headers-gh-pr-list.png differ diff --git a/docs/primer/components/images/Help.png b/docs/primer/components/images/Help.png new file mode 100644 index 000000000..935ab5478 Binary files /dev/null and b/docs/primer/components/images/Help.png differ diff --git a/docs/primer/components/images/Lists-gh-pr-list.png b/docs/primer/components/images/Lists-gh-pr-list.png new file mode 100644 index 000000000..1ff4fb7d5 Binary files /dev/null and b/docs/primer/components/images/Lists-gh-pr-list.png differ diff --git a/docs/primer/components/images/Progress-Spinner.png b/docs/primer/components/images/Progress-Spinner.png new file mode 100644 index 000000000..527017e36 Binary files /dev/null and b/docs/primer/components/images/Progress-Spinner.png differ diff --git a/docs/primer/components/images/Prompt-LongText.png b/docs/primer/components/images/Prompt-LongText.png new file mode 100644 index 000000000..6c6b08927 Binary files /dev/null and b/docs/primer/components/images/Prompt-LongText.png differ diff --git a/docs/primer/components/images/Prompt-MultiSelect.png b/docs/primer/components/images/Prompt-MultiSelect.png new file mode 100644 index 000000000..59769aabf Binary files /dev/null and b/docs/primer/components/images/Prompt-MultiSelect.png differ diff --git a/docs/primer/components/images/Prompt-RadioSelect.png b/docs/primer/components/images/Prompt-RadioSelect.png new file mode 100644 index 000000000..320f79957 Binary files /dev/null and b/docs/primer/components/images/Prompt-RadioSelect.png differ diff --git a/docs/primer/components/images/Prompt-ShortText.png b/docs/primer/components/images/Prompt-ShortText.png new file mode 100644 index 000000000..34219613f Binary files /dev/null and b/docs/primer/components/images/Prompt-ShortText.png differ diff --git a/docs/primer/components/images/Prompt-YesNo.png b/docs/primer/components/images/Prompt-YesNo.png new file mode 100644 index 000000000..aed39b2b9 Binary files /dev/null and b/docs/primer/components/images/Prompt-YesNo.png differ diff --git a/docs/primer/components/images/Spinner.png b/docs/primer/components/images/Spinner.png new file mode 100644 index 000000000..527017e36 Binary files /dev/null and b/docs/primer/components/images/Spinner.png differ diff --git a/docs/primer/components/images/States.png b/docs/primer/components/images/States.png new file mode 100644 index 000000000..c1baea326 Binary files /dev/null and b/docs/primer/components/images/States.png differ diff --git a/docs/primer/components/images/Syntax-Branch.png b/docs/primer/components/images/Syntax-Branch.png new file mode 100644 index 000000000..8dcbcbd15 Binary files /dev/null and b/docs/primer/components/images/Syntax-Branch.png differ diff --git a/docs/primer/components/images/Syntax-Label.png b/docs/primer/components/images/Syntax-Label.png new file mode 100644 index 000000000..630f6ee87 Binary files /dev/null and b/docs/primer/components/images/Syntax-Label.png differ diff --git a/docs/primer/components/images/Syntax-Repo.png b/docs/primer/components/images/Syntax-Repo.png new file mode 100644 index 000000000..0a9229131 Binary files /dev/null and b/docs/primer/components/images/Syntax-Repo.png differ diff --git a/docs/primer/foundations/README.md b/docs/primer/foundations/README.md new file mode 100644 index 000000000..e743bac02 --- /dev/null +++ b/docs/primer/foundations/README.md @@ -0,0 +1,214 @@ +# Foundations + +Design concepts and constraints that can help create a better Terminal like experience for GitHub. + +## Language + +Language is the most important tool at our disposal for creating a clear, understandable product. Having clear language helps us create memorable commands that are clear in what they will do. + +We generally follow this structure: + +| **gh** | **``** | **``** | **[value]** | **[flags]** | **[value]** | +| --- | ----------- | -------------- | ------- | --------- | ------- | +| gh | issue | view | 234 | --web | - | +| gh | pr | create | - | --title | “Title” | +| gh | repo | fork | cli/cli | --clone | false | +| gh | pr | status | - | - | - | +| gh | issue | list | - | --state | closed | +| gh | pr | review | 234 | --approve | - | + +**Command:** The object you want to interact with + +**Subcommand:** The action you want to take on that object. Most `gh` commands contain a command and subcommand. These may take arguments, such as issue/PR numbers, URLs, file names, OWNER/REPO, etc. + +**Flag:** A way to modify the command, also may be called “options”. You can use multiple flags. Flags can take values, but don’t always. Flags always have a long version with two dashes `(--state)` but often also have a shortcut with one dash and one letter `(-s)`. It’s possible to chain shorthand flags: `-sfv` is the same as `-s -f -v` + +**Values:** Are passed to the commands or flags + +- The most common command values are: + - Issue or PR number + - The “owner/repo” pair + - URLs + - Branch names + - File names +- The possible flag values depend on the flag: + - `--state` takes `{closed | open | merged}` + - `--clone` is a boolean flag + - `--title` takes a string + - `--limit` takes an integer + +_Tip: To get a better sense of what feels right, try writing out the commands in the CLI a few different ways._ + + + + + + +
+ Do: Use a flag for modifiers of actions. + `gh pr review --approve` command + + Don't: Avoid making modifiers their own commands. + `gh pr approve` command +
+ +**When designing your command’s language system:** + +- Use [GitHub language](/getting-started/principles#make-it-feel-like-github) +- Use unambiguous language that can’t be confused for something else +- Use shorter phrases if possible and appropriate + + + + + + +
+ Do: Use language that can't be misconstrued. + `gh pr create` command + + Don't: Avoid language that can be interpreted in multiple ways ("open in browser" or "open a pull request" here). + `gh pr open` command +
+ + + + + + +
+ Do: Use understood shorthands to save characters to type. + `gh repo view` command + + Don't: Avoid long words in commands if there's a reasonable alternative. + `gh repository view` command +
+ +## Typography + +Everything in a command line interface is text, so type hierarchy is important. All type is the same size and font, but you can still create type hierarchy using font weight and space. + +![An example of normal weight, and bold weight. Italics is striked through since it's not used.](images/Typography.png) + +- People customize their fonts, but you can assume it will be a monospace +- Monospace fonts inherently create visual order +- Fonts may have variable unicode support + +### Accessibility + +If you want to ensure that a screen reader will read a pause, you can use a: +- period (`.`) +- comma (`,`) +- colon (`:`) + +## Spacing + +You can use the following to create hierarchy and visual rhythm: + +- Line breaks +- Tables +- Indentation + +Do: Use space to create more legible output. + +`gh pr status` command indenting content under sections + +Don't: Not using space makes output difficult to parse. + +`gh pr status` command where content is not indented, making it harder to read + +## Color + +Terminals reliably recognize the 8 basic ANSI colors. There are also bright versions of each of these colors that you can use, but less reliably. + +A table describing the usage of the 8 basic colors. + +### Things to note +- Background color is available but we haven’t taken advantage of it yet. +- Some terminals do not reliably support 256-color escape sequences. +- Users can customize how their terminal displays the 8 basic colors, but that’s opt-in (for example, the user knows they’re making their greens not green). +- Only use color to [enhance meaning](https://primer.style/design/accessibility/guidelines#use-of-color), not to communicate meaning. + +## Iconography + +Since graphical image support in terminal emulators is unreliable, we rely on Unicode for iconography. When applying iconography consider: + +- People use different fonts that will have varying Unicode support +- Only use iconography to [enhance meaning](https://primer.style/design/global/accessibility#visual-accessibility), not to communicate meaning + +_Note: In Windows, Powershell’s default font (Lucida Console) has poor Unicode support. Microsoft suggests changing it for more Unicode support._ + +**Symbols currently used:** + +``` +✓ Success +- Neutral +✗ Failure ++ Changes requested +! Alert +``` + + + + + + +
+ Do: Use checks for success messages. + ✓ Checks passing + + Don't: Don't use checks for failure messages. + ✓ Checks failing +
+ + + + + + +
+ Do: Use checks for success of closing or deleting. + ✓ Issue closed + + Do: Don't use alerts when closing or deleting. + ! Issue closed +
+ +## Scriptability + +Make choices that ensure that creating automations or scripts with GitHub commands is obvious and frictionless. Practically, this means: + +- Create flags for anything interactive +- Ensure flags have clear language and defaults +- Consider what should be different for terminal vs machine output + +### In terminal + +![An example of gh pr list](images/Scriptability-gh-pr-list.png) + +### Through pipe + +![An example of gh pr list piped through the cat command](images/Scriptability-gh-pr-list-machine.png) + +### Differences to note in machine output + +- No color or styling +- State is explicitly written, not implied from color +- Tabs between columns instead of table layout, since `cut` uses tabs as a delimiter +- No truncation +- Exact date format +- No header + +## Customizability + +Be aware that people exist in different environments and may customize their setups. Customizations include: + +- **Shell:** shell prompt, shell aliases, PATH and other environment variables, tab-completion behavior +- **Terminal:** font, color scheme, and keyboard shortcuts +- **Operating system**: language input options, accessibility settings + +The CLI tool itself is also customizable. These are all tools at your disposal when designing new commands. + +- Aliasing: [`gh alias set`](https://cli.github.com/manual/gh_alias_set) +- Preferences: [`gh config set`](https://cli.github.com/manual/gh_config_set) +- Environment variables: `NO_COLOR`, `EDITOR`, etc diff --git a/docs/primer/foundations/images/Colors.png b/docs/primer/foundations/images/Colors.png new file mode 100644 index 000000000..ab25b1687 Binary files /dev/null and b/docs/primer/foundations/images/Colors.png differ diff --git a/docs/primer/foundations/images/Iconography-1.png b/docs/primer/foundations/images/Iconography-1.png new file mode 100644 index 000000000..012feba8b Binary files /dev/null and b/docs/primer/foundations/images/Iconography-1.png differ diff --git a/docs/primer/foundations/images/Iconography-2.png b/docs/primer/foundations/images/Iconography-2.png new file mode 100644 index 000000000..613e02377 Binary files /dev/null and b/docs/primer/foundations/images/Iconography-2.png differ diff --git a/docs/primer/foundations/images/Iconography-3.png b/docs/primer/foundations/images/Iconography-3.png new file mode 100644 index 000000000..4638e9fa8 Binary files /dev/null and b/docs/primer/foundations/images/Iconography-3.png differ diff --git a/docs/primer/foundations/images/Iconography-4.png b/docs/primer/foundations/images/Iconography-4.png new file mode 100644 index 000000000..b26ece5a8 Binary files /dev/null and b/docs/primer/foundations/images/Iconography-4.png differ diff --git a/docs/primer/foundations/images/Language-01.png b/docs/primer/foundations/images/Language-01.png new file mode 100644 index 000000000..d2a43ca5c Binary files /dev/null and b/docs/primer/foundations/images/Language-01.png differ diff --git a/docs/primer/foundations/images/Language-02.png b/docs/primer/foundations/images/Language-02.png new file mode 100644 index 000000000..0ec1c4bab Binary files /dev/null and b/docs/primer/foundations/images/Language-02.png differ diff --git a/docs/primer/foundations/images/Language-03.png b/docs/primer/foundations/images/Language-03.png new file mode 100644 index 000000000..30e75ec40 Binary files /dev/null and b/docs/primer/foundations/images/Language-03.png differ diff --git a/docs/primer/foundations/images/Language-04.png b/docs/primer/foundations/images/Language-04.png new file mode 100644 index 000000000..9d4276442 Binary files /dev/null and b/docs/primer/foundations/images/Language-04.png differ diff --git a/docs/primer/foundations/images/Language-05.png b/docs/primer/foundations/images/Language-05.png new file mode 100644 index 000000000..ab59700d4 Binary files /dev/null and b/docs/primer/foundations/images/Language-05.png differ diff --git a/docs/primer/foundations/images/Language-06.png b/docs/primer/foundations/images/Language-06.png new file mode 100644 index 000000000..9691d30cc Binary files /dev/null and b/docs/primer/foundations/images/Language-06.png differ diff --git a/docs/primer/foundations/images/Scriptability-gh-pr-list-machine.png b/docs/primer/foundations/images/Scriptability-gh-pr-list-machine.png new file mode 100644 index 000000000..872af4a24 Binary files /dev/null and b/docs/primer/foundations/images/Scriptability-gh-pr-list-machine.png differ diff --git a/docs/primer/foundations/images/Scriptability-gh-pr-list.png b/docs/primer/foundations/images/Scriptability-gh-pr-list.png new file mode 100644 index 000000000..1ff4fb7d5 Binary files /dev/null and b/docs/primer/foundations/images/Scriptability-gh-pr-list.png differ diff --git a/docs/primer/foundations/images/Spacing-gh-pr-status-compressed.png b/docs/primer/foundations/images/Spacing-gh-pr-status-compressed.png new file mode 100644 index 000000000..733cdf237 Binary files /dev/null and b/docs/primer/foundations/images/Spacing-gh-pr-status-compressed.png differ diff --git a/docs/primer/foundations/images/Spacing-gh-pr-status.png b/docs/primer/foundations/images/Spacing-gh-pr-status.png new file mode 100644 index 000000000..793e1451c Binary files /dev/null and b/docs/primer/foundations/images/Spacing-gh-pr-status.png differ diff --git a/docs/primer/foundations/images/Typography.png b/docs/primer/foundations/images/Typography.png new file mode 100644 index 000000000..5ed8b1172 Binary files /dev/null and b/docs/primer/foundations/images/Typography.png differ diff --git a/docs/primer/getting-started/README.md b/docs/primer/getting-started/README.md new file mode 100644 index 000000000..f402cbd3e --- /dev/null +++ b/docs/primer/getting-started/README.md @@ -0,0 +1,131 @@ +# Getting Started + +## Principles + +### Reasonable defaults, easy overrides + +Optimize for what most people will need to do most of the time, but make it easy for people to adjust it to their needs. Often this means considering the default behavior of each command, and how it might need to be adjusted with flags. + +### Make it feel like GitHub + +Using this tool, it should be obvious that it’s GitHub and not anything else. Use details that are specific to GitHub, such as language or color. When designing output, reflect the GitHub.com interface as much as possible and appropriate. + + + + + + +
+ Do: Use language accurate to GitHub.com. + `gh pr close` command + + Don't: Don't use language that GitHub.com doesn't use. + `gh pr delete` command +
+ + + + + + +
+ Do: Use sentence case. + Pull request with request being a lowercase r + + Don't: Don't use title case. + Pull Request with Request being an uppercase R +
+ +**Resources** + +- [GitHub Brand Content Guide](https://brand.github.com) + +### Reduce cognitive load + +Command line interfaces are not as visually intuitive as graphical interfaces. They have very few affordances (indicators of use), rely on memory, and are often unforgiving of mistakes. We do our best to design our commands to mitigate this. + +Reducing cognitive load is necessary for [making an accessible product](https://www.w3.org/TR/coga-usable/#summary) . + +**Ways to reduce cognitive load** + +- Include confirm steps, especially for riskier commands +- Include headers to help set context for output +- Ensure consistent command language to make memorizing easier +- Ensure similar commands are visually and behaviorally parallel. \* For example, any create command should behave the same +- Anticipate what people might want to do next. \* For example, we ask if you want to delete your branch after you merge. +- Anticipate what mistakes people might make + +### Bias towards terminal, but make it easy to get to the browser + +We want to help people stay in the terminal wherever they might want to maintain focus and reduce context switching, but when it’s necessary to jump to GitHub.com make it obvious, fast, and easy. Certain actions are probably better to do in a visual interface. + +![A prompt asking 'What's next?' with the choice 'Preview in browser' selected.](images/Principle4-01.png) + +A preview in browser step helps users create issues and pull requests more smoothly. + +![The `gh pr create command` with `--title` and `--body` flags outputting a pull request URL.](images/Principle4-02.png) + +Many commands output the relevant URL at the end. + +![The `gh issue view` command with the `--web` flag. The output is opening a URL in the browser.](images/Principle4-03.png) + +Web flags help users jump to the browser quickly + +## Process + +When designing for the command line, consider: + +### 1. What the command does + +- What makes sense to do from a terminal? What doesn’t? +- What might people want to automate? +- What is the default behavior? What flags might you need to change that behavior? +- What might people try and fail to do and how can you anticipate that? + +### 2. What the command is called + +- What should the [command language system](/docs/primer/foundations#language) be? +- What should be a command vs a flag? +- How can you align the language of the new command with the existing commands? + +### 3. What the command outputs + +- What can you do to make the CLI version [feel like the GitHub.com version](#make-it-feel-like-github), using [color](/docs/primer/foundations#color), [language](/docs/primer/foundations#language), [spacing](/docs/primer/foundations#spacing), info shown, etc? +- How should the [machine output](/docs/primer/foundations#scriptability) differ from the interactive behavior? + +### 4. How you explain your command + +- You will need to provide a short and long description of the command for the [help pages](/docs/primer/components#help). + +### 5. How people discover your command + +- Are there ways to integrate CLI into the feature where it exists on other platforms? + +## Prototyping + +When designing for GitHub CLI, there are several ways you can go about prototyping your ideas. + +### Google Docs + +![A screenshot of the Google Docs template](images/Prototyping-GoogleDocs.png) + +Best for simple quick illustrations of most ideas + +Use [this template](https://docs.google.com/document/d/1JIRErIUuJ6fTgabiFYfCH3x91pyHuytbfa0QLnTfXKM/edit?usp=sharing), or format your document with these steps: + +1. Choose a dark background (File > Page Setup > Page Color) +1. Choose a light text color +1. Choose a monospace font + +**Tips** + +- Mix it up since people’s setups change so much. Not everyone uses dark background! +- Make use of the document outline and headers to help communicate your ideas + +### Figma + +![A screenshot of the Figma library](images/Prototyping-Figma.png) + +If you need to show a process unfolding over time, or need to show a prototype that feels more real to users, Figma or code prototypes are best. + +[**Figma library**](https://www.figma.com/file/zYsBk5KFoMlovE4g2f4Wkg/Primer-Command-Line) (accessible to GitHub staff only) diff --git a/docs/primer/getting-started/images/Principle2-01.png b/docs/primer/getting-started/images/Principle2-01.png new file mode 100644 index 000000000..89f0942ed Binary files /dev/null and b/docs/primer/getting-started/images/Principle2-01.png differ diff --git a/docs/primer/getting-started/images/Principle2-02.png b/docs/primer/getting-started/images/Principle2-02.png new file mode 100644 index 000000000..171d5aa22 Binary files /dev/null and b/docs/primer/getting-started/images/Principle2-02.png differ diff --git a/docs/primer/getting-started/images/Principle2-03.png b/docs/primer/getting-started/images/Principle2-03.png new file mode 100644 index 000000000..118c8f82b Binary files /dev/null and b/docs/primer/getting-started/images/Principle2-03.png differ diff --git a/docs/primer/getting-started/images/Principle2-04.png b/docs/primer/getting-started/images/Principle2-04.png new file mode 100644 index 000000000..01192608b Binary files /dev/null and b/docs/primer/getting-started/images/Principle2-04.png differ diff --git a/docs/primer/getting-started/images/Principle2-05.png b/docs/primer/getting-started/images/Principle2-05.png new file mode 100644 index 000000000..18d57f391 Binary files /dev/null and b/docs/primer/getting-started/images/Principle2-05.png differ diff --git a/docs/primer/getting-started/images/Principle4-01.png b/docs/primer/getting-started/images/Principle4-01.png new file mode 100644 index 000000000..29bd0a2f9 Binary files /dev/null and b/docs/primer/getting-started/images/Principle4-01.png differ diff --git a/docs/primer/getting-started/images/Principle4-02.png b/docs/primer/getting-started/images/Principle4-02.png new file mode 100644 index 000000000..658cad333 Binary files /dev/null and b/docs/primer/getting-started/images/Principle4-02.png differ diff --git a/docs/primer/getting-started/images/Principle4-03.png b/docs/primer/getting-started/images/Principle4-03.png new file mode 100644 index 000000000..4864b8a98 Binary files /dev/null and b/docs/primer/getting-started/images/Principle4-03.png differ diff --git a/docs/primer/getting-started/images/Prototyping-Figma.png b/docs/primer/getting-started/images/Prototyping-Figma.png new file mode 100644 index 000000000..e4898a982 Binary files /dev/null and b/docs/primer/getting-started/images/Prototyping-Figma.png differ diff --git a/docs/primer/getting-started/images/Prototyping-GoogleDocs.png b/docs/primer/getting-started/images/Prototyping-GoogleDocs.png new file mode 100644 index 000000000..76c6d6c83 Binary files /dev/null and b/docs/primer/getting-started/images/Prototyping-GoogleDocs.png differ diff --git a/go.mod b/go.mod index e8be99e08..0291023a7 100644 --- a/go.mod +++ b/go.mod @@ -39,7 +39,7 @@ require ( github.com/mattn/go-colorable v0.1.14 github.com/mattn/go-isatty v0.0.20 github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d - github.com/microsoft/dev-tunnels v0.0.25 + github.com/microsoft/dev-tunnels v0.1.13 github.com/muhammadmuzzammil1998/jsonc v1.0.0 github.com/opentracing/opentracing-go v1.2.0 github.com/rivo/tview v0.0.0-20250625164341-a4a78f1e05cb diff --git a/go.sum b/go.sum index fd2a9c32b..c64e0b244 100644 --- a/go.sum +++ b/go.sum @@ -370,8 +370,8 @@ github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d h1:5PJl274Y63IEHC+7izoQ github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= github.com/microcosm-cc/bluemonday v1.0.27 h1:MpEUotklkwCSLeH+Qdx1VJgNqLlpY2KXwXFM08ygZfk= github.com/microcosm-cc/bluemonday v1.0.27/go.mod h1:jFi9vgW+H7c3V0lb6nR74Ib/DIB5OBs92Dimizgw2cA= -github.com/microsoft/dev-tunnels v0.0.25 h1:UlMKUI+2O8cSu4RlB52ioSyn1LthYSVkJA+CSTsdKoA= -github.com/microsoft/dev-tunnels v0.0.25/go.mod h1:frU++12T/oqxckXkDpTuYa427ncguEOodSPZcGCCrzQ= +github.com/microsoft/dev-tunnels v0.1.13 h1:bp1qqCvP/5iLol1Vz0c/lM2sexG7Gd8fRGcGv58vZdE= +github.com/microsoft/dev-tunnels v0.1.13/go.mod h1:Jvr6RlyjUXomM6KsDmIQbq+hhKd5mWrBcv3MEsa78dc= github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw= github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= diff --git a/internal/codespaces/connection/connection.go b/internal/codespaces/connection/connection.go index 36fef3e17..155e349bb 100644 --- a/internal/codespaces/connection/connection.go +++ b/internal/codespaces/connection/connection.go @@ -132,7 +132,9 @@ func getTunnelManager(tunnelProperties api.TunnelProperties, httpClient *http.Cl } // Create the tunnel manager - tunnelManager, err = tunnels.NewManager(userAgent, nil, url, httpClient) + // This api version seems to be the only acceptable api version: https://github.com/microsoft/dev-tunnels/blob/bf96ae5a128041d1a23f81d53a47e9e6c26fdc8d/go/tunnels/manager.go#L66 + apiVersion := "2023-09-27-preview" + tunnelManager, err = tunnels.NewManager(userAgent, nil, url, httpClient, apiVersion) if err != nil { return nil, fmt.Errorf("error creating tunnel manager: %w", err) } diff --git a/internal/codespaces/connection/tunnels_api_server_mock.go b/internal/codespaces/connection/tunnels_api_server_mock.go index cf8f05cfa..8f040886c 100644 --- a/internal/codespaces/connection/tunnels_api_server_mock.go +++ b/internal/codespaces/connection/tunnels_api_server_mock.go @@ -14,6 +14,7 @@ import ( "net/http/httptest" "net/url" "regexp" + "strconv" "strings" "sync" "time" @@ -25,7 +26,28 @@ import ( "golang.org/x/crypto/ssh" ) -func NewMockHttpClient() (*http.Client, error) { +type mockClientOpts struct { + ports map[int]tunnels.TunnelPort // Port number to protocol +} + +type mockClientOpt func(*mockClientOpts) + +// WithSpecificPorts allows you to specify a map of ports to TunnelPorts that will be returned by the mock HTTP client. +// Note that this does not take a copy of the map, so you should not modify the map after passing it to this function. +func WithSpecificPorts(ports map[int]tunnels.TunnelPort) mockClientOpt { + return func(opts *mockClientOpts) { + opts.ports = ports + } +} + +func NewMockHttpClient(opts ...mockClientOpt) (*http.Client, error) { + mockClientOpts := &mockClientOpts{} + for _, opt := range opts { + opt(mockClientOpts) + } + + specifiedPorts := mockClientOpts.ports + accessToken := "tunnel access-token" relayServer, err := newMockrelayServer(withAccessToken(accessToken)) if err != nil { @@ -35,7 +57,7 @@ func NewMockHttpClient() (*http.Client, error) { hostURL := strings.Replace(relayServer.URL(), "http://", "ws://", 1) mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var response []byte - if r.URL.Path == "/api/v1/tunnels/tunnel-id" { + if r.URL.Path == "/tunnels/tunnel-id" { tunnel := &tunnels.Tunnel{ AccessTokens: map[tunnels.TunnelAccessScope]string{ tunnels.TunnelAccessScopeConnect: accessToken, @@ -54,54 +76,141 @@ func NewMockHttpClient() (*http.Client, error) { if err != nil { log.Fatalf("json.Marshal returned an error: %v", err) } - } else if strings.HasPrefix(r.URL.Path, "/api/v1/tunnels/tunnel-id/ports") { - // Use regex to check if the path ends with a number - match, err := regexp.MatchString(`\/\d+$`, r.URL.Path) - if err != nil { - log.Fatalf("regexp.MatchString returned an error: %v", err) - } - // If the path ends with a number, it's a request for a specific port - if match || r.Method == http.MethodPost { + _, _ = w.Write(response) + return + } else if strings.HasPrefix(r.URL.Path, "/tunnels/tunnel-id/ports") { + // Use regex to capture the port number from the end of the path + re := regexp.MustCompile(`\/(\d+)$`) + matches := re.FindStringSubmatch(r.URL.Path) + targetingSpecificPort := len(matches) > 0 + + if targetingSpecificPort { if r.Method == http.MethodDelete { w.WriteHeader(http.StatusOK) return } - tunnelPort := &tunnels.TunnelPort{ + if r.Method == http.MethodGet { + // If no ports were configured, then we assume that every request for a port is valid. + if specifiedPorts == nil { + response, err := json.Marshal(tunnels.TunnelPort{ + AccessControl: &tunnels.TunnelAccessControl{ + Entries: []tunnels.TunnelAccessControlEntry{}, + }, + }) + + if err != nil { + log.Fatalf("json.Marshal returned an error: %v", err) + } + + _, _ = w.Write(response) + return + } else { + // Otherwise we'll fetch the port from our configured ports and include the protocol in the response. + port, err := strconv.Atoi(matches[1]) + if err != nil { + log.Fatalf("strconv.Atoi returned an error: %v", err) + } + + tunnelPort, ok := specifiedPorts[port] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + + response, err := json.Marshal(tunnelPort) + + if err != nil { + log.Fatalf("json.Marshal returned an error: %v", err) + } + + _, _ = w.Write(response) + return + } + } + + // Else this is an unexpected request, fall through to 404 at the bottom + } + + // If it's a PUT request, we assume it's for creating a new port so we'll do some validation + // and then return a stub. + if r.Method == http.MethodPut { + // If a port was already configured with this number, and the protocol has changed, return a 400 Bad Request. + if specifiedPorts != nil { + port, err := strconv.Atoi(matches[1]) + if err != nil { + log.Fatalf("strconv.Atoi returned an error: %v", err) + } + + var portRequest tunnels.TunnelPort + if err := json.NewDecoder(r.Body).Decode(&portRequest); err != nil { + log.Fatalf("json.NewDecoder returned an error: %v", err) + } + + tunnelPort, ok := specifiedPorts[port] + if ok { + if tunnelPort.Protocol != portRequest.Protocol { + w.WriteHeader(http.StatusBadRequest) + return + } + } + + // Create or update the new port entry. + specifiedPorts[port] = portRequest + } + + response, err := json.Marshal(tunnels.TunnelPort{ AccessControl: &tunnels.TunnelAccessControl{ Entries: []tunnels.TunnelAccessControlEntry{}, }, - } + }) - // Convert the tunnel to JSON and write it to the response - response, err = json.Marshal(*tunnelPort) if err != nil { log.Fatalf("json.Marshal returned an error: %v", err) } - } else { - // If the path doesn't end with a number and we aren't making a POST request, return an array of ports - tunnelPorts := []tunnels.TunnelPort{ - { - AccessControl: &tunnels.TunnelAccessControl{ - Entries: []tunnels.TunnelAccessControlEntry{}, - }, - }, - } - response, err = json.Marshal(tunnelPorts) - if err != nil { - log.Fatalf("json.Marshal returned an error: %v", err) - } + _, _ = w.Write(response) + return } + // Finally, if it's not targeting a specific port or a POST request, we return a list of ports, either + // totally stubbed, or whatever was configured in the mock client options. + if specifiedPorts == nil { + response, err := json.Marshal(tunnels.TunnelPortListResponse{ + Value: []tunnels.TunnelPort{ + { + AccessControl: &tunnels.TunnelAccessControl{ + Entries: []tunnels.TunnelAccessControlEntry{}, + }, + }, + }, + }) + if err != nil { + log.Fatalf("json.Marshal returned an error: %v", err) + } + + _, _ = w.Write(response) + return + } else { + var ports []tunnels.TunnelPort + for _, tunnelPort := range specifiedPorts { + ports = append(ports, tunnelPort) + } + response, err := json.Marshal(tunnels.TunnelPortListResponse{ + Value: ports, + }) + if err != nil { + log.Fatalf("json.Marshal returned an error: %v", err) + } + + _, _ = w.Write(response) + return + } } else { w.WriteHeader(http.StatusNotFound) return } - - // Write the response - _, _ = w.Write(response) })) url, err := url.Parse(mockServer.URL) diff --git a/internal/codespaces/portforwarder/port_forwarder.go b/internal/codespaces/portforwarder/port_forwarder.go index b62d13715..7f696c1a4 100644 --- a/internal/codespaces/portforwarder/port_forwarder.go +++ b/internal/codespaces/portforwarder/port_forwarder.go @@ -12,9 +12,9 @@ import ( ) const ( - githubSubjectId = "1" - InternalPortTag = "InternalPort" - UserForwardedPortTag = "UserForwardedPort" + githubSubjectId = "1" + InternalPortLabel = "InternalPort" + UserForwardedPortLabel = "UserForwardedPort" ) const ( @@ -108,7 +108,26 @@ func (fwd *CodespacesPortForwarder) ForwardPort(ctx context.Context, opts Forwar return fmt.Errorf("error converting port: %w", err) } - tunnelPort := tunnels.NewTunnelPort(port, "", "", tunnels.TunnelProtocolHttp) + // In v0.0.25 of dev-tunnels, the dev-tunnel manager `CreateTunnelPort` would "accept" requests that + // change the port protocol but they would not result in any actual change. This has changed, resulting in + // an error `Invalid arguments. The tunnel port protocol cannot be changed.`. It's not clear why the previous + // behaviour existed, whether it was truly the API version, or whether the `If-Not-Match` header being set inside + // `CreateTunnelPort` avoided the server accepting the request to change the protocol and that has since regressed. + // + // In any case, now we check whether a port exists with the given port number, if it does, we use the existing protocol. + // If it doesn't exist, we default to HTTP, which was the previous behaviour for all ports. + protocol := tunnels.TunnelProtocolHttp + + existingPort, err := fwd.connection.TunnelManager.GetTunnelPort(ctx, fwd.connection.Tunnel, opts.Port, fwd.connection.Options) + if err != nil && !strings.Contains(err.Error(), "404") { + return fmt.Errorf("error checking whether tunnel port already exists: %v", err) + } + + if existingPort != nil { + protocol = tunnels.TunnelProtocol(existingPort.Protocol) + } + + tunnelPort := tunnels.NewTunnelPort(port, "", "", protocol) // If no visibility is provided, Dev Tunnels will use the default (private) if opts.Visibility != "" { @@ -136,9 +155,9 @@ func (fwd *CodespacesPortForwarder) ForwardPort(ctx context.Context, opts Forwar // Tag the port as internal or user forwarded so we know if it needs to be shown in the UI if opts.Internal { - tunnelPort.Tags = []string{InternalPortTag} + tunnelPort.Labels = []string{InternalPortLabel} } else { - tunnelPort.Tags = []string{UserForwardedPortTag} + tunnelPort.Labels = []string{UserForwardedPortLabel} } // Create the tunnel port @@ -362,8 +381,8 @@ func visibilityToAccessControlEntries(visibility string) []tunnels.TunnelAccessC // IsInternalPort returns true if the port is internal. func IsInternalPort(port *tunnels.TunnelPort) bool { - for _, tag := range port.Tags { - if strings.EqualFold(tag, InternalPortTag) { + for _, label := range port.Labels { + if strings.EqualFold(label, InternalPortLabel) { return true } } diff --git a/internal/codespaces/portforwarder/port_forwarder_test.go b/internal/codespaces/portforwarder/port_forwarder_test.go index d107afec4..a951ed2b1 100644 --- a/internal/codespaces/portforwarder/port_forwarder_test.go +++ b/internal/codespaces/portforwarder/port_forwarder_test.go @@ -105,10 +105,10 @@ func TestAccessControlEntriesToVisibility(t *testing.T) { func TestIsInternalPort(t *testing.T) { internalPort := &tunnels.TunnelPort{ - Tags: []string{"InternalPort"}, + Labels: []string{"InternalPort"}, } userForwardedPort := &tunnels.TunnelPort{ - Tags: []string{"UserForwardedPort"}, + Labels: []string{"UserForwardedPort"}, } tests := []struct { @@ -137,3 +137,131 @@ func TestIsInternalPort(t *testing.T) { }) } } + +func TestForwardPortDefaultsToHTTPProtocol(t *testing.T) { + codespace := &api.Codespace{ + Name: "codespace-name", + State: api.CodespaceStateAvailable, + Connection: api.CodespaceConnection{ + TunnelProperties: api.TunnelProperties{ + ConnectAccessToken: "tunnel access-token", + ManagePortsAccessToken: "manage-ports-token", + ServiceUri: "http://global.rel.tunnels.api.visualstudio.com/", + TunnelId: "tunnel-id", + ClusterId: "usw2", + Domain: "domain.com", + }, + }, + RuntimeConstraints: api.RuntimeConstraints{ + AllowedPortPrivacySettings: []string{"public", "private"}, + }, + } + + // Given there are no forwarded ports. + tunnelPorts := map[int]tunnels.TunnelPort{} + + httpClient, err := connection.NewMockHttpClient( + connection.WithSpecificPorts(tunnelPorts), + ) + if err != nil { + t.Fatalf("NewMockHttpClient returned an error: %v", err) + } + + connection, err := connection.NewCodespaceConnection(t.Context(), codespace, httpClient) + if err != nil { + t.Fatalf("NewCodespaceConnection returned an error: %v", err) + } + + fwd, err := NewPortForwarder(t.Context(), connection) + if err != nil { + t.Fatalf("NewPortForwarder returned an error: %v", err) + } + + // When we forward a port without an existing one to use for a protocol, it should default to HTTP. + if err := fwd.ForwardPort(t.Context(), ForwardPortOpts{ + Port: 1337, + }); err != nil { + t.Fatalf("ForwardPort returned an error: %v", err) + } + + ports, err := fwd.ListPorts(t.Context()) + if err != nil { + t.Fatalf("ListPorts returned an error: %v", err) + } + + if len(ports) != 1 { + t.Fatalf("expected 1 port, got %d", len(ports)) + } + + if ports[0].Protocol != string(tunnels.TunnelProtocolHttp) { + t.Fatalf("expected port protocol to be http, got %s", ports[0].Protocol) + } +} + +func TestForwardPortRespectsProtocolOfExistingTunneledPorts(t *testing.T) { + codespace := &api.Codespace{ + Name: "codespace-name", + State: api.CodespaceStateAvailable, + Connection: api.CodespaceConnection{ + TunnelProperties: api.TunnelProperties{ + ConnectAccessToken: "tunnel access-token", + ManagePortsAccessToken: "manage-ports-token", + ServiceUri: "http://global.rel.tunnels.api.visualstudio.com/", + TunnelId: "tunnel-id", + ClusterId: "usw2", + Domain: "domain.com", + }, + }, + RuntimeConstraints: api.RuntimeConstraints{ + AllowedPortPrivacySettings: []string{"public", "private"}, + }, + } + + // Given we already have a port forwarded with an HTTPS protocol. + tunnelPorts := map[int]tunnels.TunnelPort{ + 1337: { + Protocol: string(tunnels.TunnelProtocolHttps), + AccessControl: &tunnels.TunnelAccessControl{ + Entries: []tunnels.TunnelAccessControlEntry{}, + }, + }, + } + + httpClient, err := connection.NewMockHttpClient( + connection.WithSpecificPorts(tunnelPorts), + ) + if err != nil { + t.Fatalf("NewMockHttpClient returned an error: %v", err) + } + + connection, err := connection.NewCodespaceConnection(t.Context(), codespace, httpClient) + if err != nil { + t.Fatalf("NewCodespaceConnection returned an error: %v", err) + } + + fwd, err := NewPortForwarder(t.Context(), connection) + if err != nil { + t.Fatalf("NewPortForwarder returned an error: %v", err) + } + + // When we forward a port, it would typically default to HTTP, to which the mock server would respond with a 400, + // but it should respect the existing port's protocol and forward it as HTTPS. + if err := fwd.ForwardPort(t.Context(), ForwardPortOpts{ + Port: 1337, + }); err != nil { + t.Fatalf("ForwardPort returned an error: %v", err) + } + + ports, err := fwd.ListPorts(t.Context()) + if err != nil { + t.Fatalf("ListPorts returned an error: %v", err) + } + + if len(ports) != 1 { + t.Fatalf("expected 1 port, got %d", len(ports)) + } + + if ports[0].Protocol != string(tunnels.TunnelProtocolHttps) { + t.Fatalf("expected port protocol to be https, got %s", ports[0].Protocol) + } +} diff --git a/pkg/cmd/issue/edit/edit_test.go b/pkg/cmd/issue/edit/edit_test.go index d14b2f462..caf99df58 100644 --- a/pkg/cmd/issue/edit/edit_test.go +++ b/pkg/cmd/issue/edit/edit_test.go @@ -263,12 +263,12 @@ func TestNewCmdEdit(t *testing.T) { }, { name: "argument is a URL", - input: "https://github.com/cli/cli/issues/23", + input: "https://example.com/cli/cli/issues/23", output: EditOptions{ IssueNumbers: []int{23}, Interactive: true, }, - expectedBaseRepo: ghrepo.New("cli", "cli"), + expectedBaseRepo: ghrepo.NewWithHost("cli", "cli", "example.com"), wantsErr: false, }, { diff --git a/pkg/cmd/pr/edit/edit.go b/pkg/cmd/pr/edit/edit.go index becbfce47..c01364d24 100644 --- a/pkg/cmd/pr/edit/edit.go +++ b/pkg/cmd/pr/edit/edit.go @@ -3,6 +3,7 @@ package edit import ( "fmt" "net/http" + "time" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" @@ -81,12 +82,28 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.Finder = shared.NewFinder(f) + + // support `-R, --repo` override opts.BaseRepo = f.BaseRepo if len(args) > 0 { opts.SelectorArg = args[0] } + if opts.SelectorArg != "" { + // If a URL is provided, we need to parse it to override the + // base repository, especially the hostname part. That's because + // we need a feature detector down in this command, and that + // needs to know the API host. If the command is run outside of + // a git repo, we cannot instantiate the detector unless we have + // already parsed the URL. + if baseRepo, _, err := shared.ParseURL(opts.SelectorArg); err == nil { + opts.BaseRepo = func() (ghrepo.Interface, error) { + return baseRepo, nil + } + } + } + flags := cmd.Flags() bodyProvided := flags.Changed("body") @@ -203,17 +220,38 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman } func editRun(opts *EditOptions) error { - findOptions := shared.FindOptions{ - Selector: opts.SelectorArg, - Fields: []string{"id", "url", "title", "body", "baseRefName", "reviewRequests", "labels", "projectCards", "projectItems", "milestone", "assignees"}, - Detector: opts.Detector, - } - httpClient, err := opts.HttpClient() if err != nil { return err } + if opts.Detector == nil { + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + + cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) + opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) + } + + findOptions := shared.FindOptions{ + Selector: opts.SelectorArg, + Fields: []string{"id", "url", "title", "body", "baseRefName", "reviewRequests", "labels", "projectCards", "projectItems", "milestone"}, + Detector: opts.Detector, + } + + issueFeatures, err := opts.Detector.IssueFeatures() + if err != nil { + return err + } + + if issueFeatures.ActorIsAssignable { + findOptions.Fields = append(findOptions.Fields, "assignedActors") + } else { + findOptions.Fields = append(findOptions.Fields, "assignees") + } + pr, repo, err := opts.Finder.Find(findOptions) if err != nil { return err @@ -225,7 +263,7 @@ func editRun(opts *EditOptions) error { editable.Body.Default = pr.Body editable.Base.Default = pr.BaseRefName editable.Reviewers.Default = pr.ReviewRequests.Logins() - if pr.AssignedActorsUsed { + if issueFeatures.ActorIsAssignable { editable.Assignees.ActorAssignees = true editable.Assignees.Default = pr.AssignedActors.DisplayNames() editable.Assignees.DefaultLogins = pr.AssignedActors.Logins() diff --git a/pkg/cmd/pr/edit/edit_test.go b/pkg/cmd/pr/edit/edit_test.go index 374625912..bb468a307 100644 --- a/pkg/cmd/pr/edit/edit_test.go +++ b/pkg/cmd/pr/edit/edit_test.go @@ -26,11 +26,12 @@ func TestNewCmdEdit(t *testing.T) { require.NoError(t, err) tests := []struct { - name string - input string - stdin string - output EditOptions - wantsErr bool + name string + input string + stdin string + output EditOptions + expectedBaseRepo ghrepo.Interface + wantsErr bool }{ { name: "no argument", @@ -47,6 +48,16 @@ func TestNewCmdEdit(t *testing.T) { output: EditOptions{}, wantsErr: true, }, + { + name: "URL argument", + input: "https://example.com/cli/cli/pull/23", + output: EditOptions{ + SelectorArg: "https://example.com/cli/cli/pull/23", + Interactive: true, + }, + expectedBaseRepo: ghrepo.NewWithHost("cli", "cli", "example.com"), + wantsErr: false, + }, { name: "pull request number argument", input: "23", @@ -326,6 +337,15 @@ func TestNewCmdEdit(t *testing.T) { assert.Equal(t, tt.output.SelectorArg, gotOpts.SelectorArg) assert.Equal(t, tt.output.Interactive, gotOpts.Interactive) assert.Equal(t, tt.output.Editable, gotOpts.Editable) + if tt.expectedBaseRepo != nil { + baseRepo, err := gotOpts.BaseRepo() + require.NoError(t, err) + require.True( + t, + ghrepo.IsSame(tt.expectedBaseRepo, baseRepo), + "expected base repo %+v, got %+v", tt.expectedBaseRepo, baseRepo, + ) + } }) } } @@ -344,8 +364,7 @@ func Test_editRun(t *testing.T) { Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", - AssignedActorsUsed: true, + URL: "https://github.com/OWNER/REPO/pull/123", }, ghrepo.New("OWNER", "REPO")), Interactive: false, Editable: shared.Editable{ @@ -408,8 +427,7 @@ func Test_editRun(t *testing.T) { Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", - AssignedActorsUsed: true, + URL: "https://github.com/OWNER/REPO/pull/123", }, ghrepo.New("OWNER", "REPO")), Interactive: false, Editable: shared.Editable{ @@ -466,8 +484,7 @@ func Test_editRun(t *testing.T) { Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", - AssignedActorsUsed: true, + URL: "https://github.com/OWNER/REPO/pull/123", }, ghrepo.New("OWNER", "REPO")), Interactive: false, Editable: shared.Editable{ @@ -529,8 +546,7 @@ func Test_editRun(t *testing.T) { Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", - AssignedActorsUsed: true, + URL: "https://github.com/OWNER/REPO/pull/123", }, ghrepo.New("OWNER", "REPO")), Interactive: true, Surveyor: testSurveyor{ @@ -576,8 +592,7 @@ func Test_editRun(t *testing.T) { Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", - AssignedActorsUsed: true, + URL: "https://github.com/OWNER/REPO/pull/123", }, ghrepo.New("OWNER", "REPO")), Interactive: true, Surveyor: testSurveyor{ @@ -620,8 +635,7 @@ func Test_editRun(t *testing.T) { Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", - AssignedActorsUsed: true, + URL: "https://github.com/OWNER/REPO/pull/123", }, ghrepo.New("OWNER", "REPO")), Interactive: true, Surveyor: testSurveyor{ @@ -667,8 +681,7 @@ func Test_editRun(t *testing.T) { Detector: &fd.EnabledDetectorMock{}, SelectorArg: "123", Finder: shared.NewMockFinder("123", &api.PullRequest{ - URL: "https://github.com/OWNER/REPO/pull/123", - AssignedActorsUsed: true, + URL: "https://github.com/OWNER/REPO/pull/123", AssignedActors: api.AssignedActors{ Nodes: []api.Actor{ { diff --git a/pkg/cmd/pr/shared/finder.go b/pkg/cmd/pr/shared/finder.go index a19c69669..df8cc0fd4 100644 --- a/pkg/cmd/pr/shared/finder.go +++ b/pkg/cmd/pr/shared/finder.go @@ -112,7 +112,7 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err return nil, nil, errors.New("Find error: no fields specified") } - if repo, prNumber, err := f.parseURL(opts.Selector); err == nil { + if repo, prNumber, err := ParseURL(opts.Selector); err == nil { f.prNumber = prNumber f.baseRefRepo = repo } @@ -242,33 +242,6 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err } } - // Ok this is super, super horrible so bear with me. - // The `assignees` field on a Pull Request exposes users that are assigned. It is also possible for bots to be - // assigned, but they only appear under the `assignedActors` field. Ideally, the caller of `Find` would determine - // the correct field to use based on the `fd.Detector` that is passed in, but they can't construct a detector - // because the BaseRepo is only determined within this function. The more correct solution is to do what I did with - // the issue commands and decouple argument parsing from API lookup. See PR #10811 for example. - var actorAssigneesUsed bool - if fields.Contains("assignees") { - if opts.Detector == nil { - cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) - opts.Detector = fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost()) - } - - issueFeatures, err := opts.Detector.IssueFeatures() - if err != nil { - return nil, nil, fmt.Errorf("error detecting issue features: %v", err) - } - - // If actors are assignable on this host then we additionally request the `assignedActors` field. - // Note that we don't remove the `assignees` field because some commands (`pr view`) do not display actor - // assignees yet, so we have to have both sets of data. - if issueFeatures.ActorIsAssignable { - fields.Add("assignedActors") - actorAssigneesUsed = true - } - } - var pr *api.PullRequest if f.prNumber > 0 { // If we have a PR number, let's look it up @@ -324,16 +297,14 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err }) } - if actorAssigneesUsed { - pr.AssignedActorsUsed = true - } - return pr, f.baseRefRepo, g.Wait() } var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`) -func (f *finder) parseURL(prURL string) (ghrepo.Interface, int, error) { +// ParseURL parses a pull request URL and returns the repository and pull +// request number. +func ParseURL(prURL string) (ghrepo.Interface, int, error) { if prURL == "" { return nil, 0, fmt.Errorf("invalid URL: %q", prURL) } diff --git a/pkg/cmd/pr/shared/finder_test.go b/pkg/cmd/pr/shared/finder_test.go index 0fd96e09b..0f6da5a6e 100644 --- a/pkg/cmd/pr/shared/finder_test.go +++ b/pkg/cmd/pr/shared/finder_test.go @@ -9,12 +9,75 @@ import ( ghContext "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" - fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/httpmock" "github.com/stretchr/testify/require" ) +func TestParseURL(t *testing.T) { + tests := []struct { + name string + arg string + wantRepo ghrepo.Interface + wantNum int + wantErr string + }{ + { + name: "valid HTTPS URL", + arg: "https://example.com/owner/repo/pull/123", + wantRepo: ghrepo.NewWithHost("owner", "repo", "example.com"), + wantNum: 123, + }, + { + name: "valid HTTP URL", + arg: "http://example.com/owner/repo/pull/123", + wantRepo: ghrepo.NewWithHost("owner", "repo", "example.com"), + wantNum: 123, + }, + { + name: "empty URL", + wantErr: "invalid URL: \"\"", + }, + { + name: "invalid scheme", + arg: "ftp://github.com/owner/repo/pull/123", + wantErr: "invalid scheme: ftp", + }, + { + name: "incorrect path", + arg: "https://github.com/owner/repo/issues/123", + wantErr: "not a pull request URL: https://github.com/owner/repo/issues/123", + }, + { + name: "no PR number", + arg: "https://github.com/owner/repo/pull/", + wantErr: "not a pull request URL: https://github.com/owner/repo/pull/", + }, + { + name: "invalid PR number", + arg: "https://github.com/owner/repo/pull/foo", + wantErr: "not a pull request URL: https://github.com/owner/repo/pull/foo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo, num, err := ParseURL(tt.arg) + + if tt.wantErr != "" { + require.Error(t, err) + require.Equal(t, tt.wantErr, err.Error()) + return + } + + require.NoError(t, err) + require.Equal(t, tt.wantNum, num) + require.NotNil(t, repo) + require.True(t, ghrepo.IsSame(tt.wantRepo, repo)) + }) + } +} + type args struct { baseRepoFn func() (ghrepo.Interface, error) branchFn func() (string, error) @@ -706,81 +769,6 @@ func TestFind(t *testing.T) { } } -func TestFindAssignableActors(t *testing.T) { - t.Run("given actors are not assignable, do nothing special", func(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - // Ensure we never request assignedActors - reg.Exclude(t, httpmock.GraphQL(`assignedActors`)) - reg.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.StringResponse(`{"data":{"repository":{ - "pullRequest":{"number":13} - }}}`)) - - f := finder{ - httpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - }, - } - - pr, _, err := f.Find(FindOptions{ - Detector: &fd.DisabledDetectorMock{}, - Fields: []string{"assignees"}, - Selector: "https://github.com/cli/cli/pull/13", - }) - require.NoError(t, err) - - require.False(t, pr.AssignedActorsUsed, "expected PR not to have assigned actors used") - }) - - t.Run("given actors are assignable, request assignedActors and indicate that on the returned PR", func(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - // Ensure that we only respond if assignedActors is requested - reg.Register( - httpmock.GraphQL(`assignedActors`), - httpmock.StringResponse(`{"data":{"repository":{ - "pullRequest":{ - "number":13, - "assignedActors": { - "nodes": [ - { - "id": "HUBOTID", - "login": "hubot", - "__typename": "Bot" - }, - { - "id": "MONAID", - "login": "MonaLisa", - "name": "Mona Display Name", - "__typename": "User" - } - ], - "totalCount": 2 - }} - }}}`)) - - f := finder{ - httpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - }, - } - - pr, _, err := f.Find(FindOptions{ - Detector: &fd.EnabledDetectorMock{}, - Fields: []string{"assignees"}, - Selector: "https://github.com/cli/cli/pull/13", - }) - require.NoError(t, err) - - require.Equal(t, []string{"hubot", "MonaLisa"}, pr.AssignedActors.Logins()) - require.True(t, pr.AssignedActorsUsed, "expected PR to have assigned actors used") - }) -} - func stubBranchConfig(branchConfig git.BranchConfig, err error) func(context.Context, string) (git.BranchConfig, error) { return func(_ context.Context, branch string) (git.BranchConfig, error) { return branchConfig, err diff --git a/pkg/cmd/run/shared/shared.go b/pkg/cmd/run/shared/shared.go index b58f6b0f7..f085add49 100644 --- a/pkg/cmd/run/shared/shared.go +++ b/pkg/cmd/run/shared/shared.go @@ -1,7 +1,6 @@ package shared import ( - "archive/zip" "errors" "fmt" "net/http" @@ -230,8 +229,6 @@ type Job struct { CompletedAt time.Time `json:"completed_at"` URL string `json:"html_url"` RunID int64 `json:"run_id"` - - Log *zip.File } type Step struct { @@ -241,8 +238,6 @@ type Step struct { Number int StartedAt time.Time `json:"started_at"` CompletedAt time.Time `json:"completed_at"` - - Log *zip.File } type Steps []Step diff --git a/pkg/cmd/run/view/fixtures/run_log.zip b/pkg/cmd/run/view/fixtures/run_log.zip deleted file mode 100644 index 425ba09dd..000000000 Binary files a/pkg/cmd/run/view/fixtures/run_log.zip and /dev/null differ diff --git a/pkg/cmd/run/view/logs.go b/pkg/cmd/run/view/logs.go new file mode 100644 index 000000000..bd99830bb --- /dev/null +++ b/pkg/cmd/run/view/logs.go @@ -0,0 +1,349 @@ +package view + +import ( + "archive/zip" + "errors" + "fmt" + "io" + "net/http" + "regexp" + "slices" + "sort" + "strings" + "unicode/utf16" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/run/shared" +) + +type logFetcher interface { + GetLog() (io.ReadCloser, error) +} + +type zipLogFetcher struct { + File *zip.File +} + +func (f *zipLogFetcher) GetLog() (io.ReadCloser, error) { + return f.File.Open() +} + +type apiLogFetcher struct { + httpClient *http.Client + + repo ghrepo.Interface + jobID int64 +} + +func (f *apiLogFetcher) GetLog() (io.ReadCloser, error) { + logURL := fmt.Sprintf("%srepos/%s/actions/jobs/%d/logs", + ghinstance.RESTPrefix(f.repo.RepoHost()), ghrepo.FullName(f.repo), f.jobID) + + req, err := http.NewRequest("GET", logURL, nil) + if err != nil { + return nil, err + } + + resp, err := f.httpClient.Do(req) + if err != nil { + return nil, err + } + + if resp.StatusCode == 404 { + return nil, fmt.Errorf("log not found: %v", f.jobID) + } else if resp.StatusCode != 200 { + return nil, api.HandleHTTPError(resp) + } + + return resp.Body, nil +} + +// logSegment represents a segment of a log trail, which can be either an entire +// job log or an individual step log. +type logSegment struct { + job *shared.Job + step *shared.Step + fetcher logFetcher +} + +// maxAPILogFetchers is the maximum allowed number of API log fetchers that can +// be assigned to log segments. This is a heuristic limit to avoid overwhelming +// the API with too many requests when fetching logs for a run with many jobs or +// steps. +const maxAPILogFetchers = 25 + +var errTooManyAPILogFetchers = errors.New("too many missing logs") + +// populateLogSegments populates log segments from the provided jobs and data +// available in the given ZIP archive map. Any missing logs will be assigned a +// log fetcher that retrieves logs from the API. +// +// For example, if there's no step log available in the ZIP archive, the entire +// job log will be selected as a log segment. +// +// Note that, as heuristic approach, we only allow a limited number of API log +// fetchers to be assigned. This is to avoid overwhelming the API with too many +// requests. +func populateLogSegments(httpClient *http.Client, repo ghrepo.Interface, jobs []shared.Job, zlm *zipLogMap, onlyFailed bool) ([]logSegment, error) { + segments := make([]logSegment, 0, len(jobs)) + + apiLogFetcherCount := 0 + for _, job := range jobs { + if onlyFailed && !shared.IsFailureState(job.Conclusion) { + continue + } + + stepLogAvailable := slices.ContainsFunc(job.Steps, func(step shared.Step) bool { + _, ok := zlm.forStep(job.ID, step.Number) + return ok + }) + + // If at least one step log is available, we populate the segments with + // them and don't use the entire job log. + if stepLogAvailable { + steps := slices.Clone(job.Steps) + sort.Sort(steps) + for _, step := range steps { + if onlyFailed && !shared.IsFailureState(step.Conclusion) { + continue + } + + zf, ok := zlm.forStep(job.ID, step.Number) + if !ok { + // We have no step log in the zip archive, but there's nothing we can do + // about that because there is no API endpoint to fetch step logs. + continue + } + + segments = append(segments, logSegment{ + job: &job, + step: &step, + fetcher: &zipLogFetcher{File: zf}, + }) + } + continue + } + + segment := logSegment{job: &job} + if zf, ok := zlm.forJob(job.ID); ok { + segment.fetcher = &zipLogFetcher{File: zf} + } else { + segment.fetcher = &apiLogFetcher{ + httpClient: httpClient, + repo: repo, + jobID: job.ID, + } + apiLogFetcherCount++ + } + segments = append(segments, segment) + + if apiLogFetcherCount > maxAPILogFetchers { + return nil, errTooManyAPILogFetchers + } + } + + return segments, nil +} + +// zipLogMap is a map of job and step logs available in a ZIP archive. +type zipLogMap struct { + jobs map[int64]*zip.File + steps map[string]*zip.File +} + +func newZipLogMap() *zipLogMap { + return &zipLogMap{ + jobs: make(map[int64]*zip.File), + steps: make(map[string]*zip.File), + } +} + +func (l *zipLogMap) forJob(jobID int64) (*zip.File, bool) { + f, ok := l.jobs[jobID] + return f, ok +} + +func (l *zipLogMap) forStep(jobID int64, stepNumber int) (*zip.File, bool) { + logFetcherKey := fmt.Sprintf("%d/%d", jobID, stepNumber) + f, ok := l.steps[logFetcherKey] + return f, ok +} + +func (l *zipLogMap) addStep(jobID int64, stepNumber int, zf *zip.File) { + logFetcherKey := fmt.Sprintf("%d/%d", jobID, stepNumber) + l.steps[logFetcherKey] = zf +} + +func (l *zipLogMap) addJob(jobID int64, zf *zip.File) { + l.jobs[jobID] = zf +} + +// getZipLogMap populates a logs struct with appropriate log fetchers based on +// the provided zip file and list of jobs. +// +// The structure of zip file is expected to be as: +// +// zip/ +// ├── jobname1/ +// │ ├── 1_stepname.txt +// │ ├── 2_anotherstepname.txt +// │ ├── 3_stepstepname.txt +// │ └── 4_laststepname.txt +// ├── jobname2/ +// | ├── 1_stepname.txt +// | └── 2_somestepname.txt +// ├── 0_jobname1.txt +// ├── 1_jobname2.txt +// └── -9999999999_jobname3.txt +// +// The function iterates through the list of jobs and tries to find the matching +// log file in the ZIP archive. +// +// The top-level .txt files include the logs for an entire job run. Note that +// the prefixed number is either: +// - An ordinal and cannot be mapped to the corresponding job's ID. +// - A negative integer which is the ID of the job in the old Actions service. +// The service right now tries to get logs and use an ordinal in a loop. +// However, if it doesn't get the logs, it falls back to an old service +// where the ID can apparently be negative. +func getZipLogMap(rlz *zip.Reader, jobs []shared.Job) *zipLogMap { + zlm := newZipLogMap() + + for _, job := range jobs { + // So far we haven't yet encountered a ZIP containing both top-level job + // logs (i.e. the normal and the legacy .txt files). However, it's still + // possible. Therefore, we prioritise the normal log over the legacy one. + if zf := matchFileInZIPArchive(rlz, jobLogFilenameRegexp(job)); zf != nil { + zlm.addJob(job.ID, zf) + } else if zf := matchFileInZIPArchive(rlz, legacyJobLogFilenameRegexp(job)); zf != nil { + zlm.addJob(job.ID, zf) + } + + for _, step := range job.Steps { + if zf := matchFileInZIPArchive(rlz, stepLogFilenameRegexp(job, step)); zf != nil { + zlm.addStep(job.ID, step.Number, zf) + } + } + } + + return zlm +} + +const JOB_NAME_MAX_LENGTH = 90 + +func getJobNameForLogFilename(name string) string { + // As described in https://github.com/cli/cli/issues/5011#issuecomment-1570713070, there are a number of steps + // the server can take when producing the downloaded zip file that can result in a mismatch between the job name + // and the filename in the zip including: + // * Removing characters in the job name that aren't allowed in file paths + // * Truncating names that are too long for zip files + // * Adding collision deduplicating numbers for jobs with the same name + // + // We are hesitant to duplicate all the server logic due to the fragility but it may be unavoidable. Currently, we: + // * Strip `/` which occur when composite action job names are constructed of the form ` / ` + // * Truncate long job names + // + sanitizedJobName := strings.ReplaceAll(name, "/", "") + sanitizedJobName = strings.ReplaceAll(sanitizedJobName, ":", "") + sanitizedJobName = truncateAsUTF16(sanitizedJobName, JOB_NAME_MAX_LENGTH) + return sanitizedJobName +} + +// A job run log file is a top-level .txt file whose name starts with an ordinal +// number; e.g., "0_jobname.txt". +func jobLogFilenameRegexp(job shared.Job) *regexp.Regexp { + sanitizedJobName := getJobNameForLogFilename(job.Name) + re := fmt.Sprintf(`^\d+_%s\.txt$`, regexp.QuoteMeta(sanitizedJobName)) + return regexp.MustCompile(re) +} + +// A legacy job run log file is a top-level .txt file whose name starts with a +// negative number which is the ID of the run; e.g., "-2147483648_jobname.txt". +func legacyJobLogFilenameRegexp(job shared.Job) *regexp.Regexp { + sanitizedJobName := getJobNameForLogFilename(job.Name) + re := fmt.Sprintf(`^-\d+_%s\.txt$`, regexp.QuoteMeta(sanitizedJobName)) + return regexp.MustCompile(re) +} + +func stepLogFilenameRegexp(job shared.Job, step shared.Step) *regexp.Regexp { + sanitizedJobName := getJobNameForLogFilename(job.Name) + re := fmt.Sprintf(`^%s\/%d_.*\.txt$`, regexp.QuoteMeta(sanitizedJobName), step.Number) + return regexp.MustCompile(re) +} + +/* +If you're reading this comment by necessity, I'm sorry and if you're reading it for fun, you're welcome, you weirdo. + +What is the length of this string "a😅😅"? If you said 9 you'd be right. If you said 3 or 5 you might also be right! + +Here's a summary: + + "a" takes 1 byte (`\x61`) + "😅" takes 4 `bytes` (`\xF0\x9F\x98\x85`) + "a😅😅" therefore takes 9 `bytes` + In Go `len("a😅😅")` is 9 because the `len` builtin counts `bytes` + In Go `len([]rune("a😅😅"))` is 3 because each `rune` is 4 `bytes` so each character fits within a `rune` + In C# `"a😅😅".Length` is 5 because `.Length` counts `Char` objects, `Chars` hold 2 bytes, and "😅" takes 2 Chars. + +But wait, what does C# have to do with anything? Well the server is running C#. Which server? The one that serves log +files to us in `.zip` format of course! When the server is constructing the zip file to avoid running afoul of a 260 +byte zip file path length limitation, it applies transformations to various strings in order to limit their length. +In C#, the server truncates strings with this function: + + public static string TruncateAfter(string str, int max) + { + string result = str.Length > max ? str.Substring(0, max) : str; + result = result.Trim(); + return result; + } + +This seems like it would be easy enough to replicate in Go but as we already discovered, the length of a string isn't +as obvious as it might seem. Since C# uses UTF-16 encoding for strings, and Go uses UTF-8 encoding and represents +characters by runes (which are an alias of int32) we cannot simply slice the string without any further consideration. +Instead, we need to encode the string as UTF-16 bytes, slice it and then decode it back to UTF-8. + +Interestingly, in C# length and substring both act on the Char type so it's possible to slice into the middle of +a visual, "representable" character. For example we know `"a😅😅".Length` = 5 (1+2+2) and therefore Substring(0,4) +results in the final character being cleaved in two, resulting in "a😅�". Since our int32 runes are being encoded as +2 uint16 elements, we also mimic this behaviour by slicing into the UTF-16 encoded string. + +Here's a program you can put into a dotnet playground to see how C# works: + + using System; + public class Program { + public static void Main() { + string s = "a😅😅"; + Console.WriteLine("{0} {1}", s.Length, s); + string t = TruncateAfter(s, 4); + Console.WriteLine("{0} {1}", t.Length, t); + } + public static string TruncateAfter(string str, int max) { + string result = str.Length > max ? str.Substring(0, max) : str; + return result.Trim(); + } + } + +This will output: +5 a😅😅 +4 a😅� +*/ +func truncateAsUTF16(str string, max int) string { + // Encode the string to UTF-16 to count code units + utf16Encoded := utf16.Encode([]rune(str)) + if len(utf16Encoded) > max { + // Decode back to UTF-8 up to the max length + str = string(utf16.Decode(utf16Encoded[:max])) + } + return strings.TrimSpace(str) +} + +func matchFileInZIPArchive(zr *zip.Reader, re *regexp.Regexp) *zip.File { + for _, file := range zr.File { + if re.MatchString(file.Name) { + return file + } + } + return nil +} diff --git a/pkg/cmd/run/view/logs_test.go b/pkg/cmd/run/view/logs_test.go new file mode 100644 index 000000000..b49a605ab --- /dev/null +++ b/pkg/cmd/run/view/logs_test.go @@ -0,0 +1,542 @@ +package view + +import ( + "archive/zip" + "bytes" + "io" + "net/http" + "testing" + + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmd/run/shared" + "github.com/cli/cli/v2/pkg/httpmock" + ghAPI "github.com/cli/go-gh/v2/pkg/api" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestZipLogFetcher(t *testing.T) { + zr := createZipReader(t, map[string]string{ + "foo.txt": "blah blah", + }) + + fetcher := &zipLogFetcher{ + File: zr.File[0], + } + + rc, err := fetcher.GetLog() + assert.NoError(t, err) + + defer rc.Close() + + content, err := io.ReadAll(rc) + assert.NoError(t, err) + assert.Equal(t, "blah blah", string(content)) +} + +func TestApiLogFetcher(t *testing.T) { + tests := []struct { + name string + httpStubs func(reg *httpmock.Registry) + wantErr string + wantContent string + }{ + { + // This is the real flow as of now. When we call the `/logs` + // endpoint, the server will respond with a 302 redirect, pointing + // to the actual log file URL. + name: "successful with redirect (HTTP 302, then HTTP 200)", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/123/logs"), + httpmock.WithHeader( + httpmock.StatusStringResponse(http.StatusFound, ""), + "Location", + "https://some.domain/the-actual-log", + ), + ) + reg.Register( + httpmock.REST("GET", "the-actual-log"), + httpmock.StringResponse("blah blah"), + ) + }, + wantContent: "blah blah", + }, + { + name: "successful without redirect (HTTP 200)", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/123/logs"), + httpmock.StatusStringResponse(http.StatusOK, "blah blah"), + ) + }, + wantContent: "blah blah", + }, + { + name: "failed with not found error (HTTP 404)", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/123/logs"), + httpmock.StatusStringResponse(http.StatusNotFound, ""), + ) + }, + wantErr: "log not found: 123", + }, + { + name: "failed with server error (HTTP 500)", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/123/logs"), + httpmock.JSONErrorResponse(http.StatusInternalServerError, ghAPI.HTTPError{ + Message: "blah blah", + StatusCode: http.StatusInternalServerError, + }), + ) + }, + wantErr: "HTTP 500: blah blah (https://api.github.com/repos/OWNER/REPO/actions/jobs/123/logs)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + tt.httpStubs(reg) + + httpClient := &http.Client{Transport: reg} + + fetcher := &apiLogFetcher{ + httpClient: httpClient, + repo: ghrepo.New("OWNER", "REPO"), + jobID: 123, + } + + rc, err := fetcher.GetLog() + + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + assert.Nil(t, rc) + return + } + + assert.NoError(t, err) + assert.NotNil(t, rc) + + content, err := io.ReadAll(rc) + assert.NoError(t, err) + + assert.NoError(t, rc.Close()) + assert.Equal(t, tt.wantContent, string(content)) + }) + } +} + +func TestGetZipLogMap(t *testing.T) { + tests := []struct { + name string + job shared.Job + zipReader *zip.Reader + // wantJobLog can be nil (i.e. not found) or string + wantJobLog any + // wantStepLogs elements can be nil (i.e. not found) or string + wantStepLogs []any + }{ + { + name: "job log missing from zip, but step log present", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "job foo/1_step one.txt": "step one log", + }), + wantJobLog: nil, + wantStepLogs: []any{ + "step one log", + }, + }, + { + name: "matching job name and step number 1", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo.txt": "job log", + "job foo/1_step one.txt": "step one log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + "step one log", + }, + }, + { + name: "matching job name and step number 2", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "step two", + Number: 2, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo.txt": "job log", + "job foo/2_step two.txt": "step two log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + nil, // no log for step 1 + "step two log", + }, + }, + { + // We should just look for the step number and not the step name. + name: "matching job name and step number and mismatch step name", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "mismatch", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo.txt": "job log", + "job foo/1_step one.txt": "step one log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + "step one log", + }, + }, + { + name: "matching job name and mismatch step number", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "step two", + Number: 2, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo.txt": "job log", + "job foo/1_step one.txt": "step one log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + nil, // no log for step 1 + nil, // no log for step 2 + }, + }, + { + name: "matching job name with no step logs in zip", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo.txt": "job log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + nil, // no log for step 1 + }, + }, + { + name: "matching job name with no step data", + job: shared.Job{ + ID: 123, + Name: "job foo", + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo.txt": "job log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + nil, // no log for step 1 + }, + }, + { + name: "matching job name with random prefix and no step logs in zip", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "999999999_job foo.txt": "job log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + nil, // no log for step 1 + }, + }, + { + name: "matching job name with legacy filename and no step logs in zip", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "-9999999999_job foo.txt": "job log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + nil, // no log for step 1 + }, + }, + { + name: "matching job name with legacy filename and no step data", + job: shared.Job{ + ID: 123, + Name: "job foo", + }, + zipReader: createZipReader(t, map[string]string{ + "-9999999999_job foo.txt": "job log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + nil, // no log for step 1 + }, + }, + { + name: "matching job name with both normal and legacy filename", + job: shared.Job{ + ID: 123, + Name: "job foo", + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo.txt": "job log", + "-9999999999_job foo.txt": "legacy job log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + nil, // no log for step 1 + }, + }, + { + name: "one job name is a suffix of another", + job: shared.Job{ + ID: 123, + Name: "job foo", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "0_jjob foo.txt": "the other job log", + "jjob foo/1_step one.txt": "the other step one log", + "1_job foo.txt": "job log", + "job foo/1_step one.txt": "step one log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + "step one log", + }, + }, + { + name: "escape metacharacters in job name", + job: shared.Job{ + ID: 123, + Name: "metacharacters .+*?()|[]{}^$ job", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, nil), + wantJobLog: nil, + wantStepLogs: []any{ + nil, // no log for step 1 + }, + }, + { + name: "mismatching job name", + job: shared.Job{ + ID: 123, + Name: "mismatch", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, nil), + wantJobLog: nil, + wantStepLogs: []any{ + nil, // no log for step 1 + }, + }, + { + name: "job name with forward slash matches dir with slash removed", + job: shared.Job{ + ID: 123, + Name: "job foo / with slash", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo with slash.txt": "job log", + "job foo with slash/1_step one.txt": "step one log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + "step one log", + }, + }, + { + name: "job name with colon matches dir with colon removed", + job: shared.Job{ + ID: 123, + Name: "job foo : with colon", + Steps: []shared.Step{{ + Name: "step one", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "0_job foo with colon.txt": "job log", + "job foo with colon/1_step one.txt": "step one log", + }), + wantJobLog: "job log", + wantStepLogs: []any{ + "step one log", + }, + }, + { + name: "job name with really long name (over the ZIP limit)", + job: shared.Job{ + ID: 123, + Name: "thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_", + Steps: []shared.Step{{ + Name: "long name job", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnine/1_long name job.txt": "step one log", + }), + wantJobLog: nil, + wantStepLogs: []any{ + "step one log", + }, + }, + { + name: "job name that would be truncated by the C# server to split a grapheme", + job: shared.Job{ + ID: 123, + Name: "emoji test 😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅", + Steps: []shared.Step{{ + Name: "emoji job", + Number: 1, + }}, + }, + zipReader: createZipReader(t, map[string]string{ + "emoji test 😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅�/1_emoji job.txt": "step one log", + }), + wantJobLog: nil, + wantStepLogs: []any{ + "step one log", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logMap := getZipLogMap(tt.zipReader, []shared.Job{tt.job}) + + jobLogFile, ok := logMap.forJob(tt.job.ID) + + switch want := tt.wantJobLog.(type) { + case nil: + require.False(t, ok) + require.Nil(t, jobLogFile) + case string: + require.True(t, ok) + require.NotNil(t, jobLogFile) + require.Equal(t, want, string(readZipFile(t, jobLogFile))) + default: + t.Fatal("wantJobLog must be nil or string") + } + + for i, wantStepLog := range tt.wantStepLogs { + stepLogFile, ok := logMap.forStep(tt.job.ID, 1+i) // Step numbers start from 1 + + switch want := wantStepLog.(type) { + case nil: + require.False(t, ok) + require.Nil(t, stepLogFile) + case string: + require.True(t, ok) + require.NotNil(t, stepLogFile) + + gotStepLog := readZipFile(t, stepLogFile) + require.Equal(t, want, string(gotStepLog)) + default: + t.Fatal("wantStepLog must be nil or string") + } + } + }) + } +} + +func readZipFile(t *testing.T, zf *zip.File) []byte { + rc, err := zf.Open() + assert.NoError(t, err) + defer rc.Close() + + content, err := io.ReadAll(rc) + assert.NoError(t, err) + return content +} + +func createZipReader(t *testing.T, files map[string]string) *zip.Reader { + raw := createZipArchive(t, files) + + zr, err := zip.NewReader(bytes.NewReader(raw), int64(len(raw))) + assert.NoError(t, err) + + return zr +} + +func createZipArchive(t *testing.T, files map[string]string) []byte { + buf := bytes.NewBuffer(nil) + zw := zip.NewWriter(buf) + + for name, content := range files { + fileWriter, err := zw.Create(name) + assert.NoError(t, err) + + _, err = fileWriter.Write([]byte(content)) + assert.NoError(t, err) + } + + err := zw.Close() + assert.NoError(t, err) + + return buf.Bytes() +} diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 0dafbcc09..d50b14fdf 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -10,12 +10,8 @@ import ( "net/http" "os" "path/filepath" - "regexp" - "sort" "strconv" - "strings" "time" - "unicode/utf16" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" @@ -116,13 +112,17 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman Long: heredoc.Docf(` View a summary of a workflow run. - This command does not support authenticating via fine grained PATs - as it is not currently possible to create a PAT with the %[1]schecks:read%[1]s permission. + Due to platform limitations, %[1]sgh%[1]s may not always be able to associate jobs with their + corresponding logs when using the primary method of fetching logs in zip format. - Due to platform limitations, %[1]sgh%[1]s may not always be able to associate log lines with a - particular step in a job. In this case, the step name in the log output will be replaced with - %[1]sUNKNOWN STEP%[1]s. - `, "`"), + In such cases, %[1]sgh%[1]s will attempt to fetch logs for each job individually via the API. + This fallback is slower and more resource-intensive. If more than 25 job logs are missing, + the operation will fail with an error. + + Additionally, due to similar platform constraints, some log lines may not be + associated with a specific step within a job. In these cases, the step name will + appear as %[1]sUNKNOWN STEP%[1]s in the log output. + `, "`", maxAPILogFetchers), Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` # Interactively select a run to view, optionally selecting a single job @@ -322,9 +322,16 @@ func runView(opts *ViewOptions) error { } defer runLogZip.Close() - attachRunLog(&runLogZip.Reader, jobs) + zlm := getZipLogMap(&runLogZip.Reader, jobs) + segments, err := populateLogSegments(httpClient, repo, jobs, zlm, opts.LogFailed) + if err != nil { + if errors.Is(err, errTooManyAPILogFetchers) { + return fmt.Errorf("too many API requests needed to fetch logs; try narrowing down to a specific job with the `--job` option") + } + return err + } - return displayRunLog(opts.IO.Out, jobs, opts.LogFailed) + return displayLogSegments(opts.IO.Out, segments) } prNumber := "" @@ -535,212 +542,28 @@ func promptForJob(prompter shared.Prompter, cs *iostreams.ColorScheme, jobs []sh return nil, nil } -const JOB_NAME_MAX_LENGTH = 90 - -func getJobNameForLogFilename(name string) string { - // As described in https://github.com/cli/cli/issues/5011#issuecomment-1570713070, there are a number of steps - // the server can take when producing the downloaded zip file that can result in a mismatch between the job name - // and the filename in the zip including: - // * Removing characters in the job name that aren't allowed in file paths - // * Truncating names that are too long for zip files - // * Adding collision deduplicating numbers for jobs with the same name - // - // We are hesitant to duplicate all the server logic due to the fragility but it may be unavoidable. Currently, we: - // * Strip `/` which occur when composite action job names are constructed of the form ` / ` - // * Truncate long job names - // - sanitizedJobName := strings.ReplaceAll(name, "/", "") - sanitizedJobName = strings.ReplaceAll(sanitizedJobName, ":", "") - sanitizedJobName = truncateAsUTF16(sanitizedJobName, JOB_NAME_MAX_LENGTH) - return sanitizedJobName -} - -// A job run log file is a top-level .txt file whose name starts with an ordinal -// number; e.g., "0_jobname.txt". -func jobLogFilenameRegexp(job shared.Job) *regexp.Regexp { - sanitizedJobName := getJobNameForLogFilename(job.Name) - re := fmt.Sprintf(`^\d+_%s\.txt$`, regexp.QuoteMeta(sanitizedJobName)) - return regexp.MustCompile(re) -} - -// A legacy job run log file is a top-level .txt file whose name starts with a -// negative number which is the ID of the run; e.g., "-2147483648_jobname.txt". -func legacyJobLogFilenameRegexp(job shared.Job) *regexp.Regexp { - sanitizedJobName := getJobNameForLogFilename(job.Name) - re := fmt.Sprintf(`^-\d+_%s\.txt$`, regexp.QuoteMeta(sanitizedJobName)) - return regexp.MustCompile(re) -} - -func stepLogFilenameRegexp(job shared.Job, step shared.Step) *regexp.Regexp { - sanitizedJobName := getJobNameForLogFilename(job.Name) - re := fmt.Sprintf(`^%s\/%d_.*\.txt$`, regexp.QuoteMeta(sanitizedJobName), step.Number) - return regexp.MustCompile(re) -} - -/* -If you're reading this comment by necessity, I'm sorry and if you're reading it for fun, you're welcome, you weirdo. - -What is the length of this string "a😅😅"? If you said 9 you'd be right. If you said 3 or 5 you might also be right! - -Here's a summary: - - "a" takes 1 byte (`\x61`) - "😅" takes 4 `bytes` (`\xF0\x9F\x98\x85`) - "a😅😅" therefore takes 9 `bytes` - In Go `len("a😅😅")` is 9 because the `len` builtin counts `bytes` - In Go `len([]rune("a😅😅"))` is 3 because each `rune` is 4 `bytes` so each character fits within a `rune` - In C# `"a😅😅".Length` is 5 because `.Length` counts `Char` objects, `Chars` hold 2 bytes, and "😅" takes 2 Chars. - -But wait, what does C# have to do with anything? Well the server is running C#. Which server? The one that serves log -files to us in `.zip` format of course! When the server is constructing the zip file to avoid running afoul of a 260 -byte zip file path length limitation, it applies transformations to various strings in order to limit their length. -In C#, the server truncates strings with this function: - - public static string TruncateAfter(string str, int max) - { - string result = str.Length > max ? str.Substring(0, max) : str; - result = result.Trim(); - return result; - } - -This seems like it would be easy enough to replicate in Go but as we already discovered, the length of a string isn't -as obvious as it might seem. Since C# uses UTF-16 encoding for strings, and Go uses UTF-8 encoding and represents -characters by runes (which are an alias of int32) we cannot simply slice the string without any further consideration. -Instead, we need to encode the string as UTF-16 bytes, slice it and then decode it back to UTF-8. - -Interestingly, in C# length and substring both act on the Char type so it's possible to slice into the middle of -a visual, "representable" character. For example we know `"a😅😅".Length` = 5 (1+2+2) and therefore Substring(0,4) -results in the final character being cleaved in two, resulting in "a😅�". Since our int32 runes are being encoded as -2 uint16 elements, we also mimic this behaviour by slicing into the UTF-16 encoded string. - -Here's a program you can put into a dotnet playground to see how C# works: - - using System; - public class Program { - public static void Main() { - string s = "a😅😅"; - Console.WriteLine("{0} {1}", s.Length, s); - string t = TruncateAfter(s, 4); - Console.WriteLine("{0} {1}", t.Length, t); - } - public static string TruncateAfter(string str, int max) { - string result = str.Length > max ? str.Substring(0, max) : str; - return result.Trim(); - } - } - -This will output: -5 a😅😅 -4 a😅� -*/ -func truncateAsUTF16(str string, max int) string { - // Encode the string to UTF-16 to count code units - utf16Encoded := utf16.Encode([]rune(str)) - if len(utf16Encoded) > max { - // Decode back to UTF-8 up to the max length - str = string(utf16.Decode(utf16Encoded[:max])) - } - return strings.TrimSpace(str) -} - -// This function takes a zip file of logs and a list of jobs. -// Structure of zip file -// -// zip/ -// ├── jobname1/ -// │ ├── 1_stepname.txt -// │ ├── 2_anotherstepname.txt -// │ ├── 3_stepstepname.txt -// │ └── 4_laststepname.txt -// ├── jobname2/ -// | ├── 1_stepname.txt -// | └── 2_somestepname.txt -// ├── 0_jobname1.txt -// ├── 1_jobname2.txt -// └── -9999999999_jobname3.txt -// -// It iterates through the list of jobs and tries to find the matching -// log in the zip file. If the matching log is found it is attached -// to the job. -// -// The top-level .txt files include the logs for an entire job run. Note that -// the prefixed number is either: -// - An ordinal and cannot be mapped to the corresponding job's ID. -// - A negative integer which is the ID of the job in the old Actions service. -// The service right now tries to get logs and use an ordinal in a loop. -// However, if it doesn't get the logs, it falls back to an old service -// where the ID can apparently be negative. -func attachRunLog(rlz *zip.Reader, jobs []shared.Job) { - for i, job := range jobs { - // As a highest priority, we try to use the step logs first. We have seen zips that surprisingly contain - // step logs, normal job logs and legacy job logs. In this case, both job logs would be ignored. We have - // never seen a zip containing both job logs and no step logs, however, it may be possible. In that case - // let's prioritise the normal log over the legacy one. - jobLog := matchFileInZIPArchive(rlz, jobLogFilenameRegexp(job)) - if jobLog == nil { - jobLog = matchFileInZIPArchive(rlz, legacyJobLogFilenameRegexp(job)) +func displayLogSegments(w io.Writer, segments []logSegment) error { + for _, segment := range segments { + stepName := "UNKNOWN STEP" + if segment.step != nil { + stepName = segment.step.Name } - jobs[i].Log = jobLog - for j, step := range job.Steps { - jobs[i].Steps[j].Log = matchFileInZIPArchive(rlz, stepLogFilenameRegexp(job, step)) + rc, err := segment.fetcher.GetLog() + if err != nil { + return err } - } -} -func matchFileInZIPArchive(zr *zip.Reader, re *regexp.Regexp) *zip.File { - for _, file := range zr.File { - if re.MatchString(file.Name) { - return file - } - } - return nil -} - -func displayRunLog(w io.Writer, jobs []shared.Job, failed bool) error { - for _, job := range jobs { - // To display a run log, we first try to compile it from individual step - // logs, because this way we can prepend lines with the corresponding - // step name. However, at the time of writing, logs are sometimes being - // served by a service that doesn’t include the step logs (none of them), - // in which case we fall back to print the entire job run log. - var hasStepLogs bool - - steps := job.Steps - sort.Sort(steps) - for _, step := range steps { - if failed && !shared.IsFailureState(step.Conclusion) { - continue - } - if step.Log == nil { - continue - } - hasStepLogs = true - prefix := fmt.Sprintf("%s\t%s\t", job.Name, step.Name) - if err := printZIPFile(w, step.Log, prefix); err != nil { + err = func() error { + defer rc.Close() + prefix := fmt.Sprintf("%s\t%s\t", segment.job.Name, stepName) + if err := copyLogWithLinePrefix(w, rc, prefix); err != nil { return err } - } + return nil + }() - if hasStepLogs { - continue - } - - if failed && !shared.IsFailureState(job.Conclusion) { - continue - } - - if job.Log == nil { - continue - } - - // Here, we fall back to the job run log, which means we do not know - // the step name of lines. However, we want to keep the same line - // formatting to avoid breaking any code or script that rely on the - // tab-delimited formatting. So, an unknown-step placeholder is used - // instead of the actual step name. - prefix := fmt.Sprintf("%s\tUNKNOWN STEP\t", job.Name) - if err := printZIPFile(w, job.Log, prefix); err != nil { + if err != nil { return err } } @@ -748,14 +571,8 @@ func displayRunLog(w io.Writer, jobs []shared.Job, failed bool) error { return nil } -func printZIPFile(w io.Writer, file *zip.File, prefix string) error { - f, err := file.Open() - if err != nil { - return err - } - defer f.Close() - - scanner := bufio.NewScanner(f) +func copyLogWithLinePrefix(w io.Writer, r io.Reader, prefix string) error { + scanner := bufio.NewScanner(r) for scanner.Scan() { fmt.Fprintf(w, "%s%s\n", prefix, scanner.Text()) } diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 2d150934f..4c3a6a471 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -1,13 +1,12 @@ package view import ( - "archive/zip" "bytes" "fmt" "io" "net/http" "net/url" - "os" + "slices" "strings" "testing" "time" @@ -177,6 +176,65 @@ func TestNewCmdView(t *testing.T) { } func TestViewRun(t *testing.T) { + emptyZipArchive := createZipArchive(t, map[string]string{}) + zipArchive := createZipArchive(t, map[string]string{ + "0_cool job.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + log line 1 + log line 2 + log line 3`), + "cool job/1_fob the barz.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + `), + "cool job/2_barz the fob.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + `), + "1_sad job.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + log line 1 + log line 2 + log line 3 + `), + "sad job/1_barf the quux.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + `), + "sad job/2_quuz the barf.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + `), + "2_cool job with no step logs.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + `), + "3_sad job with no step logs.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + `), + "-9999999999_legacy cool job with no step logs.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + `), + "-9999999999_legacy sad job with no step logs.txt": heredoc.Doc(` + log line 1 + log line 2 + log line 3 + `), + }) + tests := []struct { name string httpStubs func(*httpmock.Registry) @@ -579,7 +637,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -632,7 +690,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/attempts/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -673,7 +731,7 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.SuccessfulRun)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -696,7 +754,7 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.SuccessfulRun)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/attempts/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -729,7 +787,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -776,7 +834,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -809,7 +867,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -862,7 +920,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/attempts/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -903,7 +961,7 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.FailedRun)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -936,7 +994,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), httpmock.JSONResponse(workflowShared.WorkflowsPayload{ @@ -983,7 +1041,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1016,7 +1074,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1057,7 +1115,7 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.SuccessfulRun)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1090,7 +1148,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1131,7 +1189,7 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.FailedRun)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1164,7 +1222,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1210,7 +1268,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1243,7 +1301,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1289,7 +1347,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1322,7 +1380,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1363,7 +1421,7 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.SuccessfulRun)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1397,7 +1455,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1438,7 +1496,7 @@ func TestViewRun(t *testing.T) { httpmock.JSONResponse(shared.FailedRun)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1471,7 +1529,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1517,7 +1575,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1550,7 +1608,7 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) @@ -1596,13 +1654,558 @@ func TestViewRun(t *testing.T) { })) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), - httpmock.FileResponse("./fixtures/run_log.zip")) + httpmock.BinaryResponse(zipArchive)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), httpmock.JSONResponse(shared.TestWorkflow)) }, wantOut: legacySadJobRunWithNoStepLogsLogOutput, }, + { + name: "interactive with log, fallback to retrieve job logs from API (#11169)", + tty: true, + opts: &ViewOptions{ + Prompt: true, + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"), + httpmock.JSONResponse(shared.RunsPayload{ + WorkflowRuns: shared.TestRuns, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/10/logs"), + httpmock.StringResponse("blah blah")) + }, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "✓ cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI [trunk] Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool job") + }) + }, + wantOut: "cool job\tUNKNOWN STEP\tblah blah\n", + }, + { + name: "noninteractive with log, fallback to retrieve job logs from API (#11169)", + opts: &ViewOptions{ + JobID: "10", + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/10"), + httpmock.JSONResponse(shared.SuccessfulJob)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/10/logs"), + httpmock.StringResponse("blah blah")) + }, + wantOut: "cool job\tUNKNOWN STEP\tblah blah\n", + }, + { + name: "interactive with run log, fallback to retrieve job logs from API (#11169)", + tty: true, + opts: &ViewOptions{ + Prompt: true, + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"), + httpmock.JSONResponse(shared.RunsPayload{ + WorkflowRuns: shared.TestRuns, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/10/logs"), + httpmock.StringResponse("blah blah")) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20/logs"), + httpmock.StringResponse("yo yo")) + }, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "✓ cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI [trunk] Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "View all jobs in this run") + }) + }, + wantOut: "cool job\tUNKNOWN STEP\tblah blah\nsad job\tUNKNOWN STEP\tyo yo\n", + }, + { + name: "noninteractive with run log, fallback to retrieve job logs from API (#11169)", + opts: &ViewOptions{ + RunID: "3", + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/10/logs"), + httpmock.StringResponse("blah blah")) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20/logs"), + httpmock.StringResponse("yo yo")) + }, + wantOut: "cool job\tUNKNOWN STEP\tblah blah\nsad job\tUNKNOWN STEP\tyo yo\n", + }, + { + name: "interactive with log-failed, fallback to retrieve job logs from API (#11169)", + tty: true, + opts: &ViewOptions{ + Prompt: true, + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"), + httpmock.JSONResponse(shared.RunsPayload{ + WorkflowRuns: shared.TestRuns, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20/logs"), + httpmock.StringResponse("yo yo")) + }, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "✓ cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI [trunk] Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "X sad job") + }) + }, + wantOut: "sad job\tUNKNOWN STEP\tyo yo\n", + }, + { + name: "noninteractive with log-failed, fallback to retrieve job logs from API (#11169)", + opts: &ViewOptions{ + JobID: "20", + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20"), + httpmock.JSONResponse(shared.FailedJob)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20/logs"), + httpmock.StringResponse("yo yo")) + }, + wantOut: "sad job\tUNKNOWN STEP\tyo yo\n", + }, + { + name: "interactive with run log-failed, fallback to retrieve job logs from API (#11169)", + tty: true, + opts: &ViewOptions{ + Prompt: true, + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"), + httpmock.JSONResponse(shared.RunsPayload{ + WorkflowRuns: shared.TestRuns, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + reg.Register( + httpmock.REST("GET", "runs/1234/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20/logs"), + httpmock.StringResponse("yo yo")) + }, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "✓ cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return 4, nil + }) + pm.RegisterSelect("View a specific job in this run?", + []string{"View all jobs in this run", "✓ cool job", "X sad job"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "View all jobs in this run") + }) + }, + wantOut: "sad job\tUNKNOWN STEP\tyo yo\n", + }, + { + name: "noninteractive with run log-failed, fallback to retrieve job logs from API (#11169)", + opts: &ViewOptions{ + RunID: "1234", + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + reg.Register( + httpmock.REST("GET", "runs/1234/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: []shared.Job{ + shared.SuccessfulJob, + shared.FailedJob, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/jobs/20/logs"), + httpmock.StringResponse("yo yo")) + }, + wantOut: "sad job\tUNKNOWN STEP\tyo yo\n", + }, + { + name: "interactive with run log, too many API calls required error (#11169)", + tty: true, + opts: &ViewOptions{ + Prompt: true, + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"), + httpmock.JSONResponse(shared.RunsPayload{ + WorkflowRuns: shared.TestRuns, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + + tooManyJobs := make([]shared.Job, 1+maxAPILogFetchers) + for i := range tooManyJobs { + tooManyJobs[i] = shared.SuccessfulJob + tooManyJobs[i].ID = int64(i + 100) + } + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: tooManyJobs, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + }, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "✓ cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "✓ cool commit, CI [trunk] Feb 23, 2021") + }) + pm.RegisterSelect("View a specific job in this run?", + slices.Concat([]string{"View all jobs in this run"}, slices.Repeat([]string{"✓ cool job"}, 26)), + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "View all jobs in this run") + }) + }, + wantErr: true, + errMsg: "too many API requests needed to fetch logs; try narrowing down to a specific job with the `--job` option", + }, + { + name: "noninteractive with run log, too many API calls required error (#11169)", + opts: &ViewOptions{ + RunID: "3", + Log: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3"), + httpmock.JSONResponse(shared.SuccessfulRun)) + + tooManyJobs := make([]shared.Job, 1+maxAPILogFetchers) + for i := range tooManyJobs { + tooManyJobs[i] = shared.SuccessfulJob + tooManyJobs[i].ID = int64(i + 100) + } + reg.Register( + httpmock.REST("GET", "runs/3/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: tooManyJobs, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/3/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + }, + wantErr: true, + errMsg: "too many API requests needed to fetch logs; try narrowing down to a specific job with the `--job` option", + }, + { + name: "interactive with run log-failed, too many API calls required error (#11169)", + tty: true, + opts: &ViewOptions{ + Prompt: true, + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"), + httpmock.JSONResponse(shared.RunsPayload{ + WorkflowRuns: shared.TestRuns, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + + tooManyJobs := make([]shared.Job, 1+maxAPILogFetchers) + for i := range tooManyJobs { + tooManyJobs[i] = shared.FailedJob + tooManyJobs[i].ID = int64(i + 100) + } + reg.Register( + httpmock.REST("GET", "runs/1234/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: tooManyJobs, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows"), + httpmock.JSONResponse(workflowShared.WorkflowsPayload{ + Workflows: []workflowShared.Workflow{ + shared.TestWorkflow, + }, + })) + }, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterSelect("Select a workflow run", + []string{"X cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "✓ cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "- cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "* cool commit, CI [trunk] Feb 23, 2021", "X cool commit, CI [trunk] Feb 23, 2021"}, + func(_, _ string, opts []string) (int, error) { + return 4, nil + }) + pm.RegisterSelect("View a specific job in this run?", + slices.Concat([]string{"View all jobs in this run"}, slices.Repeat([]string{"X sad job"}, 26)), + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "View all jobs in this run") + }) + }, + wantErr: true, + errMsg: "too many API requests needed to fetch logs; try narrowing down to a specific job with the `--job` option", + }, + { + name: "noninteractive with run log-failed, too many API calls required error (#11169)", + opts: &ViewOptions{ + RunID: "1234", + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + + tooManyJobs := make([]shared.Job, 1+maxAPILogFetchers) + for i := range tooManyJobs { + tooManyJobs[i] = shared.FailedJob + tooManyJobs[i].ID = int64(i + 100) + } + reg.Register( + httpmock.REST("GET", "runs/1234/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: tooManyJobs, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + }, + wantErr: true, + errMsg: "too many API requests needed to fetch logs; try narrowing down to a specific job with the `--job` option", + }, + { + name: "noninteractive with run log-failed, maximum API calls allowed (#11169)", + opts: &ViewOptions{ + RunID: "1234", + LogFailed: true, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.FailedRun)) + + tooManyJobs := make([]shared.Job, maxAPILogFetchers) + for i := range tooManyJobs { + tooManyJobs[i] = shared.FailedJob + tooManyJobs[i].ID = int64(i + 100) + } + reg.Register( + httpmock.REST("GET", "runs/1234/jobs"), + httpmock.JSONResponse(shared.JobsPayload{ + Jobs: tooManyJobs, + })) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234/logs"), + httpmock.BinaryResponse(emptyZipArchive)) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow)) + for i := range tooManyJobs { + reg.Register( + httpmock.REST("GET", fmt.Sprintf("repos/OWNER/REPO/actions/jobs/%d/logs", i+100)), + httpmock.StringResponse("yo yo")) + } + }, + wantOut: strings.Repeat("sad job\tUNKNOWN STEP\tyo yo\n", maxAPILogFetchers), + }, { name: "run log but run is not done", tty: true, @@ -2021,268 +2624,6 @@ func TestViewRun(t *testing.T) { } } -// Structure of fixture zip file -// To see the structure of fixture zip file, run: -// `❯ unzip -lv pkg/cmd/run/view/fixtures/run_log.zip` -// -// run log/ -// ├── cool job/ -// │ ├── 1_fob the barz.txt -// │ └── 2_barz the fob.txt -// ├── sad job/ -// │ ├── 1_barf the quux.txt -// │ └── 2_quux the barf.txt -// ├── ad job/ -// | └── 1_barf the quux.txt -// ├── 0_cool job.txt -// ├── 1_sad job.txt -// ├── 2_cool job with no step logs.txt -// ├── 3_sad job with no step logs.txt -// ├── -9999999999_legacy cool job with no step logs.txt -// ├── -9999999999_legacy sad job with no step logs.txt -// ├── 4_cool job with both legacy and new logs.txt -// └── -9999999999_cool job with both legacy and new logs.txt -func Test_attachRunLog(t *testing.T) { - tests := []struct { - name string - job shared.Job - wantJobMatch bool - wantJobFilename string - wantStepMatch bool - wantStepFilename string - }{ - { - name: "matching job name and step number 1", - job: shared.Job{ - Name: "cool job", - Steps: []shared.Step{{ - Name: "fob the barz", - Number: 1, - }}, - }, - wantJobMatch: true, - wantJobFilename: "0_cool job.txt", - wantStepMatch: true, - wantStepFilename: "cool job/1_fob the barz.txt", - }, - { - name: "matching job name and step number 2", - job: shared.Job{ - Name: "cool job", - Steps: []shared.Step{{ - Name: "barz the fob", - Number: 2, - }}, - }, - wantJobMatch: true, - wantJobFilename: "0_cool job.txt", - wantStepMatch: true, - wantStepFilename: "cool job/2_barz the fob.txt", - }, - { - name: "matching job name and step number and mismatch step name", - job: shared.Job{ - Name: "cool job", - Steps: []shared.Step{{ - Name: "mismatch", - Number: 1, - }}, - }, - wantJobMatch: true, - wantJobFilename: "0_cool job.txt", - wantStepMatch: true, - wantStepFilename: "cool job/1_fob the barz.txt", - }, - { - name: "matching job name and mismatch step number", - job: shared.Job{ - Name: "cool job", - Steps: []shared.Step{{ - Name: "fob the barz", - Number: 3, - }}, - }, - wantJobMatch: true, - wantJobFilename: "0_cool job.txt", - wantStepMatch: false, - }, - { - name: "matching job name with no step logs", - job: shared.Job{ - Name: "cool job with no step logs", - Steps: []shared.Step{{ - Name: "fob the barz", - Number: 1, - }}, - }, - wantJobMatch: true, - wantJobFilename: "2_cool job with no step logs.txt", - wantStepMatch: false, - }, - { - name: "matching job name with no step data", - job: shared.Job{ - Name: "cool job with no step logs", - }, - wantJobMatch: true, - wantJobFilename: "2_cool job with no step logs.txt", - wantStepMatch: false, - }, - { - name: "matching job name with legacy filename and no step logs", - job: shared.Job{ - Name: "legacy cool job with no step logs", - Steps: []shared.Step{{ - Name: "fob the barz", - Number: 1, - }}, - }, - wantJobMatch: true, - wantJobFilename: "-9999999999_legacy cool job with no step logs.txt", - wantStepMatch: false, - }, - { - name: "matching job name with legacy filename and no step data", - job: shared.Job{ - Name: "legacy cool job with no step logs", - }, - wantJobMatch: true, - wantJobFilename: "-9999999999_legacy cool job with no step logs.txt", - wantStepMatch: false, - }, - { - name: "matching job name with both normal and legacy filename", - job: shared.Job{ - Name: "cool job with both legacy and new logs", - }, - wantJobMatch: true, - wantJobFilename: "4_cool job with both legacy and new logs.txt", - wantStepMatch: false, - }, - { - name: "one job name is a suffix of another", - job: shared.Job{ - Name: "ad job", - Steps: []shared.Step{{ - Name: "barf the quux", - Number: 1, - }}, - }, - wantStepMatch: true, - wantStepFilename: "ad job/1_barf the quux.txt", - }, - { - name: "escape metacharacters in job name", - job: shared.Job{ - Name: "metacharacters .+*?()|[]{}^$ job", - Steps: []shared.Step{{ - Name: "fob the barz", - Number: 0, - }}, - }, - wantJobMatch: false, - wantStepMatch: false, - }, - { - name: "mismatching job name", - job: shared.Job{ - Name: "mismatch", - Steps: []shared.Step{{ - Name: "fob the barz", - Number: 1, - }}, - }, - wantJobMatch: false, - wantStepMatch: false, - }, - { - name: "job name with forward slash matches dir with slash removed", - job: shared.Job{ - Name: "cool job / with slash", - Steps: []shared.Step{{ - Name: "fob the barz", - Number: 1, - }}, - }, - wantJobMatch: false, - wantStepMatch: true, - // not the double space in the dir name, as the slash has been removed - wantStepFilename: "cool job with slash/1_fob the barz.txt", - }, - { - name: "job name with colon matches dir with colon removed", - job: shared.Job{ - Name: "cool job : with colon", - Steps: []shared.Step{{ - Name: "fob the barz", - Number: 1, - }}, - }, - wantJobMatch: false, - wantStepMatch: true, - wantStepFilename: "cool job with colon/1_fob the barz.txt", - }, - { - name: "Job name with really long name (over the ZIP limit)", - job: shared.Job{ - Name: "thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_", - Steps: []shared.Step{{ - Name: "Long Name Job", - Number: 1, - }}, - }, - wantJobMatch: false, - wantStepMatch: true, - wantStepFilename: "thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnineteenchars_thisisnine/1_Long Name Job.txt", - }, - { - name: "Job name that would be truncated by the C# server to split a grapheme", - job: shared.Job{ - Name: "Emoji Test 😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅", - Steps: []shared.Step{{ - Name: "Emoji Job", - Number: 1, - }}, - }, - wantJobMatch: false, - wantStepMatch: true, - wantStepFilename: "Emoji Test 😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅😅�/1_Emoji Job.txt", - }, - } - - run_log_zip_reader, err := zip.OpenReader("./fixtures/run_log.zip") - require.NoError(t, err) - defer run_log_zip_reader.Close() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - jobs := []shared.Job{tt.job} - - attachRunLog(&run_log_zip_reader.Reader, jobs) - - t.Logf("Job details: ") - - job := jobs[0] - - jobLog := job.Log - jobLogPresent := jobLog != nil - require.Equal(t, tt.wantJobMatch, jobLogPresent, "job log not present") - if jobLogPresent { - require.Equal(t, tt.wantJobFilename, jobLog.Name, "job log filename mismatch") - } - - for _, step := range job.Steps { - stepLog := step.Log - stepLogPresent := stepLog != nil - require.Equal(t, tt.wantStepMatch, stepLogPresent, "step log not present") - if stepLogPresent { - require.Equal(t, tt.wantStepFilename, stepLog.Name, "step log filename mismatch") - } - } - }) - } -} - var barfTheFobLogOutput = heredoc.Doc(` cool job barz the fob log line 1 cool job barz the fob log line 2 @@ -2382,9 +2723,8 @@ func TestRunLog(t *testing.T) { cacheDir := t.TempDir() rlc := RunLogCache{cacheDir: cacheDir} - f, err := os.Open("./fixtures/run_log.zip") - require.NoError(t, err) - defer f.Close() + raw := createZipArchive(t, map[string]string{"foo": "bar"}) + f := bytes.NewReader(raw) require.NoError(t, rlc.Create("key", f)) @@ -2392,5 +2732,6 @@ func TestRunLog(t *testing.T) { require.NoError(t, err) defer zipReader.Close() require.NotEmpty(t, zipReader.File) + require.Equal(t, "foo", zipReader.File[0].Name) }) } diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 0a6581559..7fbc77552 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -53,6 +53,11 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command Prompter: f.Prompter, } + // It is possible for a user to say `--no-repos-selected=false --repos cli/cli` and that would be equivalent to not + // specifying the flag at all. We could avoid this by checking whether the flag was set at all, but it seems like + // more trouble than it's worth since anyone who does `--no-repos-selected=false` is gonna get what's coming to them. + var noRepositoriesSelected bool + cmd := &cobra.Command{ Use: "set ", Short: "Create or update secrets", @@ -90,6 +95,9 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command # Set organization-level secret visible to specific repositories $ gh secret set MYSECRET --org myOrg --repos repo1,repo2,repo3 + # Set organization-level secret visible to no repositories + $ gh secret set MYSECRET --org myOrg --no-repos-selected + # Set user-level secret for Codespaces $ gh secret set MYSECRET --user @@ -131,6 +139,14 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command return err } + if err := cmdutil.MutuallyExclusive("specify only one of `--repos` or `--no-repos-selected`", len(opts.RepositoryNames) > 0, noRepositoriesSelected); err != nil { + return err + } + + if err := cmdutil.MutuallyExclusive("`--no-repos-selected` must be omitted when used with `--user`", opts.UserSecrets, noRepositoriesSelected); err != nil { + return err + } + if len(args) == 0 { if !opts.DoNotStore && opts.EnvFile == "" { return cmdutil.FlagErrorf("must pass name argument") @@ -148,11 +164,16 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command return cmdutil.FlagErrorf("`--repos` is only supported with `--visibility=selected`") } - if opts.Visibility == shared.Selected && len(opts.RepositoryNames) == 0 { - return cmdutil.FlagErrorf("`--repos` list required with `--visibility=selected`") + if opts.Visibility != shared.Selected && noRepositoriesSelected { + return cmdutil.FlagErrorf("`--no-repos-selected` is only supported with `--visibility=selected`") } + + if opts.Visibility == shared.Selected && (len(opts.RepositoryNames) == 0 && !noRepositoriesSelected) { + return cmdutil.FlagErrorf("`--repos` or `--no-repos-selected` required with `--visibility=selected`") + } + } else { - if len(opts.RepositoryNames) > 0 { + if len(opts.RepositoryNames) > 0 || noRepositoriesSelected { opts.Visibility = shared.Selected } } @@ -170,6 +191,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command cmd.Flags().BoolVarP(&opts.UserSecrets, "user", "u", false, "Set a secret for your user") cmdutil.StringEnumFlag(cmd, &opts.Visibility, "visibility", "v", shared.Private, []string{shared.All, shared.Private, shared.Selected}, "Set visibility for an organization secret") cmd.Flags().StringSliceVarP(&opts.RepositoryNames, "repos", "r", []string{}, "List of `repositories` that can access an organization or user secret") + cmd.Flags().BoolVar(&noRepositoriesSelected, "no-repos-selected", false, "No repositories can access the organization secret") cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "The value for the secret (reads from standard input if not specified)") cmd.Flags().BoolVar(&opts.DoNotStore, "no-store", false, "Print the encrypted, base64-encoded value instead of storing it on GitHub") cmd.Flags().StringVarP(&opts.EnvFile, "env-file", "f", "", "Load secret names and values from a dotenv-formatted `file`") diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 0b305eda6..38c0fb5a9 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -27,88 +27,120 @@ import ( func TestNewCmdSet(t *testing.T) { tests := []struct { - name string - cli string - wants SetOptions - stdinTTY bool - wantsErr bool + name string + args string + wants SetOptions + stdinTTY bool + wantsErr bool + wantsErrMessage string }{ { name: "invalid visibility", - cli: "cool_secret --org coolOrg -v'mistyVeil'", + args: "cool_secret --org coolOrg -v mistyVeil", wantsErr: true, }, { - name: "invalid visibility", - cli: "cool_secret --org coolOrg -v'selected'", + name: "when visibility is selected, requires indication of repos", + args: "cool_secret --org coolOrg -v selected", + wantsErr: true, + wantsErrMessage: "`--repos` or `--no-repos-selected` required with `--visibility=selected`", + }, + { + name: "visibilities other than selected do not accept --repos", + args: "cool_secret --org coolOrg -v private -r coolRepo", + wantsErr: true, + wantsErrMessage: "`--repos` is only supported with `--visibility=selected`", + }, + { + name: "visibilities other than selected do not accept --no-repos-selected", + args: "cool_secret --org coolOrg -v private --no-repos-selected", + wantsErr: true, + wantsErrMessage: "`--no-repos-selected` is only supported with `--visibility=selected`", + }, + { + name: "--repos and --no-repos-selected are mutually exclusive", + args: `--repos coolRepo --no-repos-selected cool_secret`, + wantsErr: true, + wantsErrMessage: "specify only one of `--repos` or `--no-repos-selected`", + }, + { + name: "secret name is required", + args: "", wantsErr: true, }, { - name: "repos with wrong vis", - cli: "cool_secret --org coolOrg -v'private' -rcoolRepo", + name: "multiple positional arguments are not allowed", + args: "cool_secret good_secret", wantsErr: true, }, { - name: "no name", - cli: "", + name: "visibility is only allowed with --org", + args: "cool_secret -v all", wantsErr: true, }, { - name: "multiple names", - cli: "cool_secret good_secret", - wantsErr: true, - }, - { - name: "visibility without org", - cli: "cool_secret -vall", - wantsErr: true, - }, - { - name: "repos without vis", - cli: "cool_secret -bs --org coolOrg -rcoolRepo", + name: "providing --repos without --visibility implies selected visibility", + args: "cool_secret --body secret-body --org coolOrg --repos coolRepo", wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"coolRepo"}, - Body: "s", + Body: "secret-body", + OrgName: "coolOrg", + }, + }, + { + name: "providing --no-repos-selected without --visibility implies selected visibility", + args: "cool_secret --body secret-body --org coolOrg --no-repos-selected", + wants: SetOptions{ + SecretName: "cool_secret", + Visibility: shared.Selected, + RepositoryNames: []string{}, + Body: "secret-body", OrgName: "coolOrg", }, }, { name: "org with selected repo", - cli: "-ocoolOrg -bs -vselected -rcoolRepo cool_secret", + args: "-o coolOrg --body secret-body -v selected -r coolRepo cool_secret", wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"coolRepo"}, - Body: "s", + Body: "secret-body", OrgName: "coolOrg", }, }, { name: "org with selected repos", - cli: `--org=coolOrg -bs -vselected -r="coolRepo,radRepo,goodRepo" cool_secret`, + args: `--org coolOrg --body secret-body -v selected --repos "coolRepo,radRepo,goodRepo" cool_secret`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"coolRepo", "goodRepo", "radRepo"}, - Body: "s", + Body: "secret-body", OrgName: "coolOrg", }, }, { name: "user with selected repos", - cli: `-u -bs -r"monalisa/coolRepo,cli/cli,github/hub" cool_secret`, + args: `-u --body secret-body -r "monalisa/coolRepo,cli/cli,github/hub" cool_secret`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"monalisa/coolRepo", "cli/cli", "github/hub"}, - Body: "s", + Body: "secret-body", }, }, + { + name: "--user is mutually exclusive with --no-repos-selected", + args: `-u --no-repos-selected cool_secret`, + wantsErr: true, + wantsErrMessage: "`--no-repos-selected` must be omitted when used with `--user`", + }, { name: "repo", - cli: `cool_secret -b"a secret"`, + args: `cool_secret --body "a secret"`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Private, @@ -118,7 +150,7 @@ func TestNewCmdSet(t *testing.T) { }, { name: "env", - cli: `cool_secret -b"a secret" -eRelease`, + args: `cool_secret --body "a secret" --env Release`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Private, @@ -129,7 +161,7 @@ func TestNewCmdSet(t *testing.T) { }, { name: "vis all", - cli: `cool_secret --org coolOrg -b"cool" -vall`, + args: `cool_secret --org coolOrg --body "cool" --visibility all`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.All, @@ -139,7 +171,7 @@ func TestNewCmdSet(t *testing.T) { }, { name: "no store", - cli: `cool_secret --no-store`, + args: `cool_secret --no-store`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Private, @@ -148,7 +180,7 @@ func TestNewCmdSet(t *testing.T) { }, { name: "Dependabot repo", - cli: `cool_secret -b"a secret" --app Dependabot`, + args: `cool_secret --body "a secret" --app Dependabot`, wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Private, @@ -159,19 +191,19 @@ func TestNewCmdSet(t *testing.T) { }, { name: "Dependabot org", - cli: "-ocoolOrg -bs -vselected -rcoolRepo cool_secret -aDependabot", + args: "--org coolOrg --body secret-body --visibility selected --repos coolRepo cool_secret --app Dependabot", wants: SetOptions{ SecretName: "cool_secret", Visibility: shared.Selected, RepositoryNames: []string{"coolRepo"}, - Body: "s", + Body: "secret-body", OrgName: "coolOrg", Application: "Dependabot", }, }, { name: "Codespaces org", - cli: `random_secret -ocoolOrg -b"random value" -vselected -r"coolRepo,cli/cli" -aCodespaces`, + args: `random_secret --org coolOrg --body "random value" --visibility selected --repos "coolRepo,cli/cli" --app Codespaces`, wants: SetOptions{ SecretName: "random_secret", Visibility: shared.Selected, @@ -192,7 +224,7 @@ func TestNewCmdSet(t *testing.T) { ios.SetStdinTTY(tt.stdinTTY) - argv, err := shlex.Split(tt.cli) + argv, err := shlex.Split(tt.args) assert.NoError(t, err) var gotOpts *SetOptions @@ -208,6 +240,9 @@ func TestNewCmdSet(t *testing.T) { _, err = cmd.ExecuteC() if tt.wantsErr { assert.Error(t, err) + if tt.wantsErrMessage != "" { + assert.EqualError(t, err, tt.wantsErrMessage) + } return } assert.NoError(t, err) @@ -497,6 +532,16 @@ func Test_setRun_org(t *testing.T) { wantRepositories: []int64{1, 2}, wantApp: "actions", }, + { + name: "no repos visibility", + opts: &SetOptions{ + OrgName: "UmbrellaCorporation", + Visibility: shared.Selected, + RepositoryNames: []string{}, + }, + wantRepositories: []int64{}, + wantApp: "actions", + }, { name: "Dependabot", opts: &SetOptions{ @@ -517,6 +562,17 @@ func Test_setRun_org(t *testing.T) { wantDependabotRepositories: []string{"1", "2"}, wantApp: "dependabot", }, + { + name: "Dependabot no repos visibility", + opts: &SetOptions{ + OrgName: "UmbrellaCorporation", + Visibility: shared.Selected, + Application: shared.Dependabot, + RepositoryNames: []string{}, + }, + wantRepositories: []int64{}, + wantApp: "dependabot", + }, } for _, tt := range tests { diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index 3b03ae718..f15423c84 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -127,6 +127,12 @@ func StringResponse(body string) Responder { } } +func BinaryResponse(body []byte) Responder { + return func(req *http.Request) (*http.Response, error) { + return httpResponse(200, req, bytes.NewBuffer(body)), nil + } +} + func WithHost(matcher Matcher, host string) Matcher { return func(req *http.Request) bool { if !strings.EqualFold(req.Host, host) { diff --git a/third-party-licenses.darwin.md b/third-party-licenses.darwin.md index 60498f974..53514f14d 100644 --- a/third-party-licenses.darwin.md +++ b/third-party-licenses.darwin.md @@ -106,7 +106,7 @@ Some packages may only be included on certain architectures or operating systems - [github.com/mattn/go-runewidth](https://pkg.go.dev/github.com/mattn/go-runewidth) ([MIT](https://github.com/mattn/go-runewidth/blob/v0.0.16/LICENSE)) - [github.com/mgutz/ansi](https://pkg.go.dev/github.com/mgutz/ansi) ([MIT](https://github.com/mgutz/ansi/blob/d51e80ef957d/LICENSE)) - [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md)) -- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.0.25/LICENSE)) +- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.1.13/LICENSE)) - [github.com/mitchellh/copystructure](https://pkg.go.dev/github.com/mitchellh/copystructure) ([MIT](https://github.com/mitchellh/copystructure/blob/v1.2.0/LICENSE)) - [github.com/mitchellh/go-homedir](https://pkg.go.dev/github.com/mitchellh/go-homedir) ([MIT](https://github.com/mitchellh/go-homedir/blob/v1.1.0/LICENSE)) - [github.com/mitchellh/hashstructure/v2](https://pkg.go.dev/github.com/mitchellh/hashstructure/v2) ([MIT](https://github.com/mitchellh/hashstructure/blob/v2.0.2/LICENSE)) diff --git a/third-party-licenses.linux.md b/third-party-licenses.linux.md index cb5d2db05..6ce47d8bc 100644 --- a/third-party-licenses.linux.md +++ b/third-party-licenses.linux.md @@ -106,7 +106,7 @@ Some packages may only be included on certain architectures or operating systems - [github.com/mattn/go-runewidth](https://pkg.go.dev/github.com/mattn/go-runewidth) ([MIT](https://github.com/mattn/go-runewidth/blob/v0.0.16/LICENSE)) - [github.com/mgutz/ansi](https://pkg.go.dev/github.com/mgutz/ansi) ([MIT](https://github.com/mgutz/ansi/blob/d51e80ef957d/LICENSE)) - [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md)) -- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.0.25/LICENSE)) +- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.1.13/LICENSE)) - [github.com/mitchellh/copystructure](https://pkg.go.dev/github.com/mitchellh/copystructure) ([MIT](https://github.com/mitchellh/copystructure/blob/v1.2.0/LICENSE)) - [github.com/mitchellh/go-homedir](https://pkg.go.dev/github.com/mitchellh/go-homedir) ([MIT](https://github.com/mitchellh/go-homedir/blob/v1.1.0/LICENSE)) - [github.com/mitchellh/hashstructure/v2](https://pkg.go.dev/github.com/mitchellh/hashstructure/v2) ([MIT](https://github.com/mitchellh/hashstructure/blob/v2.0.2/LICENSE)) diff --git a/third-party-licenses.windows.md b/third-party-licenses.windows.md index d276a5e44..c4ebb297e 100644 --- a/third-party-licenses.windows.md +++ b/third-party-licenses.windows.md @@ -109,7 +109,7 @@ Some packages may only be included on certain architectures or operating systems - [github.com/mattn/go-runewidth](https://pkg.go.dev/github.com/mattn/go-runewidth) ([MIT](https://github.com/mattn/go-runewidth/blob/v0.0.16/LICENSE)) - [github.com/mgutz/ansi](https://pkg.go.dev/github.com/mgutz/ansi) ([MIT](https://github.com/mgutz/ansi/blob/d51e80ef957d/LICENSE)) - [github.com/microcosm-cc/bluemonday](https://pkg.go.dev/github.com/microcosm-cc/bluemonday) ([BSD-3-Clause](https://github.com/microcosm-cc/bluemonday/blob/v1.0.27/LICENSE.md)) -- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.0.25/LICENSE)) +- [github.com/microsoft/dev-tunnels/go/tunnels](https://pkg.go.dev/github.com/microsoft/dev-tunnels/go/tunnels) ([MIT](https://github.com/microsoft/dev-tunnels/blob/v0.1.13/LICENSE)) - [github.com/mitchellh/copystructure](https://pkg.go.dev/github.com/mitchellh/copystructure) ([MIT](https://github.com/mitchellh/copystructure/blob/v1.2.0/LICENSE)) - [github.com/mitchellh/go-homedir](https://pkg.go.dev/github.com/mitchellh/go-homedir) ([MIT](https://github.com/mitchellh/go-homedir/blob/v1.1.0/LICENSE)) - [github.com/mitchellh/hashstructure/v2](https://pkg.go.dev/github.com/mitchellh/hashstructure/v2) ([MIT](https://github.com/mitchellh/hashstructure/blob/v2.0.2/LICENSE))