STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge branch 'cmbrose/no-auto-key-if-uploaded' of https://github.com/cli/cli into cmbrose/no-auto-key-if-uploaded

Browse source
This commit is contained in:
Caleb Brose 2022-08-02 20:57:55 +00:00
commit d3c07706a4
13 changed files with 172 additions and 108 deletions
Download patch file Download diff file Expand all files Collapse all files

7
internal/codespaces/ssh.go
View file

@ -56,7 +56,11 @@ func NewRemoteCommand(ctx context.Context, tunnelPort int, destination string, s
// newSSHCommand populates an exec.Cmd to run a command (or if blank,
// an interactive shell) over ssh.
func newSSHCommand(ctx context.Context, port int, dst string, cmdArgs []string) (*exec.Cmd, []string, error) {
connArgs := []string{"-p", strconv.Itoa(port), "-o", "NoHostAuthenticationForLocalhost=yes"}
connArgs := []string{
"-p", strconv.Itoa(port),
"-o", "NoHostAuthenticationForLocalhost=yes",
"-o", "PasswordAuthentication=no",
}
// The ssh command syntax is: ssh [flags] user@host command [args...]
// There is no way to specify the user@host destination as a flag.
@ -101,6 +105,7 @@ func newSCPCommand(ctx context.Context, port int, dst string, cmdArgs []string)
connArgs := []string{
"-P", strconv.Itoa(port),
"-o", "NoHostAuthenticationForLocalhost=yes",
"-o", "PasswordAuthentication=no",
"-C", // compression
}

74
pkg/cmd/codespace/ssh.go
View file

@ -225,26 +225,24 @@ func shouldUseAutomaticSSHKeys(
args []string,
opts sshOptions,
) bool {
if opts.profile != "" {
// The profile may specify the identity file so cautiously don't override anything with that option
return false
}
customConfigPath := ""
for i := 0; i < len(args); i += 1 {
arg := args[i]
for _, arg := range args {
if arg == "-i" {
// User specified the identity file so it should exist
// User specified the identity file so just trust it is correct
return false
}
if arg == "-F" {
// User specified a config file so, similar to profile, cautiously don't override settings
return false
if arg == "-F" && i < len(args)-1 {
// ssh only pays attention to that last specified -F value, so it's correct to overwrite here
customConfigPath = args[i+1]
}
}
// If there is a public key uploaded which matches a configured
// private key, there is no need for automatic key generation
return hasUploadedPublicKeyForConfig(ctx, sshContext, apiClient)
return !hasUploadedPublicKeyForConfig(ctx, sshContext, apiClient, customConfigPath, opts.profile)
}
func setupAutomaticSSHKeys(sshContext ssh.Context) (*ssh.KeyPair, error) {
@ -314,26 +312,26 @@ func hasUploadedPublicKeyForConfig(
ctx context.Context,
sshContext ssh.Context,
apiClient apiClient,
customConfigFile string,
customHost string,
) bool {
configuredPrivateKeyPaths, err := getConfiguredPrivateKeys(ctx)
configuredPrivateKeyPaths, err := getConfiguredPrivateKeys(ctx, customConfigFile, customHost)
if err != nil {
return false
}
configuredPublicKeys := []string{}
for _, privateKeyPath := range configuredPrivateKeyPaths {
if _, err := os.Stat(privateKeyPath); err != nil {
// The default configuration includes standard keys like id_rsa,
// id_ed25519, etc, but these may not actually exist
continue
}
publicKeyPath := privateKeyPath + ".pub"
configuredPublicKey, err := sshContext.GetPublicKeyFromPrivateKey(privateKeyPath)
publicKeyContent, err := os.ReadFile(publicKeyPath)
if err != nil {
// The default configuration includes standard keys like id_rsa,
// id_ed25519, etc, but these may not actually exist so just skip
continue
}
configuredPublicKeys = append(configuredPublicKeys, configuredPublicKey)
configuredPublicKeys = append(configuredPublicKeys, string(publicKeyContent))
}
if len(configuredPublicKeys) == 0 {
@ -368,15 +366,30 @@ func hasUploadedPublicKeyForConfig(
// getConfiguredPrivateKeys reads the effective configuration for a localhost
// connection and returns all private keys which would be tried for authentication
func getConfiguredPrivateKeys(ctx context.Context) ([]string, error) {
func getConfiguredPrivateKeys(
ctx context.Context,
customConfigFile string,
customHost string,
) ([]string, error) {
sshExe, err := safeexec.LookPath("ssh")
if err != nil {
return nil, fmt.Errorf("could not find ssh executable: %w", err)
}
// The -G option tells ssh to output the effective config for the given host, but not connect
// The target host is always localhost because we skip this check if --profile was set.
sshGCmd := exec.CommandContext(ctx, sshExe, "-G", "localhost")
sshGArgs := []string{"-G"}
if customConfigFile != "" {
sshGArgs = append(sshGArgs, "-F", customConfigFile)
}
if customHost != "" {
sshGArgs = append(sshGArgs, customHost)
} else {
sshGArgs = append(sshGArgs, "localhost")
}
sshGCmd := exec.CommandContext(ctx, sshExe, sshGArgs...)
configBytes, err := sshGCmd.Output()
if err != nil {
return nil, fmt.Errorf("could not load ssh configuration: %w", err)
@ -482,6 +495,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro
StrictHostKeyChecking no
LogLevel quiet
ControlMaster auto
IdentityFile ~/.ssh/{{.AutomaticIdentityFile}}
`))
if err != nil {
@ -508,17 +522,19 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro
// flattened to '-' to prevent problems with tab completion or when the
// hostname appears in ControlMaster socket paths.
type codespaceSSHConfig struct {
Name string // the codespace name, passed to `ssh -c`
EscapedRef string // the currently checked-out branch
SSHUser string // the remote ssh username
GHExec string // path used for invoking the current `gh` binary
Name string // the codespace name, passed to `ssh -c`
EscapedRef string // the currently checked-out branch
SSHUser string // the remote ssh username
GHExec string // path used for invoking the current `gh` binary
AutomaticIdentityFile string
}
conf := codespaceSSHConfig{
Name: result.codespace.Name,
EscapedRef: strings.ReplaceAll(result.codespace.GitStatus.Ref, "/", "-"),
SSHUser: result.user,
GHExec: ghExec,
Name: result.codespace.Name,
EscapedRef: strings.ReplaceAll(result.codespace.GitStatus.Ref, "/", "-"),
SSHUser: result.user,
GHExec: ghExec,
AutomaticIdentityFile: automaticPrivateKeyName,
}
if err := t.Execute(a.io.Out, conf); err != nil {
return err

4
pkg/cmd/pr/checks/output.go
View file

@ -3,7 +3,6 @@ package checks
import (
"fmt"
"sort"
"time"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/cli/cli/v2/utils"
@ -12,9 +11,8 @@ import (
func addRow(tp utils.TablePrinter, io *iostreams.IOStreams, o check) {
cs := io.ColorScheme()
elapsed := ""
zeroTime := time.Time{}
if o.StartedAt != zeroTime && o.CompletedAt != zeroTime {
if !o.StartedAt.IsZero() && !o.CompletedAt.IsZero() {
e := o.CompletedAt.Sub(o.StartedAt)
if e > 0 {
elapsed = e.String()

27
pkg/cmd/run/list/list.go
View file

@ -24,19 +24,21 @@ type ListOptions struct {
HttpClient func() (*http.Client, error)
BaseRepo func() (ghrepo.Interface, error)
PlainOutput bool
Exporter cmdutil.Exporter
Exporter cmdutil.Exporter
Limit int
WorkflowSelector string
Branch string
Actor string
now time.Time
}
func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command {
opts := &ListOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
now: time.Now(),
}
cmd := &cobra.Command{
@ -48,9 +50,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
terminal := opts.IO.IsStdoutTTY() && opts.IO.IsStdinTTY()
opts.PlainOutput = !terminal
if opts.Limit < 1 {
return cmdutil.FlagErrorf("invalid limit: %v", opts.Limit)
}
@ -126,7 +125,7 @@ func listRun(opts *ListOptions) error {
cs := opts.IO.ColorScheme()
if !opts.PlainOutput {
if tp.IsTTY() {
tp.AddField("STATUS", nil, nil)
tp.AddField("NAME", nil, nil)
tp.AddField("WORKFLOW", nil, nil)
@ -139,12 +138,12 @@ func listRun(opts *ListOptions) error {
}
for _, run := range runs {
if opts.PlainOutput {
tp.AddField(string(run.Status), nil, nil)
tp.AddField(string(run.Conclusion), nil, nil)
} else {
if tp.IsTTY() {
symbol, symbolColor := shared.Symbol(cs, run.Status, run.Conclusion)
tp.AddField(symbol, nil, symbolColor)
} else {
tp.AddField(string(run.Status), nil, nil)
tp.AddField(string(run.Conclusion), nil, nil)
}
tp.AddField(run.CommitMsg(), nil, cs.Bold)
@ -154,12 +153,8 @@ func listRun(opts *ListOptions) error {
tp.AddField(string(run.Event), nil, nil)
tp.AddField(fmt.Sprintf("%d", run.ID), nil, cs.Cyan)
elapsed := run.UpdatedAt.Sub(run.CreatedAt)
if elapsed < 0 {
elapsed = 0
}
tp.AddField(elapsed.String(), nil, nil)
tp.AddField(utils.FuzzyAgoAbbr(time.Now(), run.CreatedAt), nil, nil)
tp.AddField(run.Duration(opts.now).String(), nil, nil)
tp.AddField(utils.FuzzyAgoAbbr(time.Now(), run.StartedTime()), nil, nil)
tp.EndRow()
}

24
pkg/cmd/run/list/list_test.go
View file

@ -7,6 +7,7 @@ import (
"net/http"
"net/url"
"testing"
"time"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/run/shared"
@ -115,13 +116,15 @@ func TestListRun(t *testing.T) {
wantOut string
wantErrOut string
stubs func(*httpmock.Registry)
nontty bool
isTTY bool
}{
{
name: "blank tty",
name: "default arguments",
opts: &ListOptions{
Limit: defaultLimit,
now: shared.TestRunStartTime.Add(time.Minute*4 + time.Second*34),
},
isTTY: true,
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"),
@ -132,12 +135,12 @@ func TestListRun(t *testing.T) {
wantOut: "STATUS NAME WORKFLOW BRANCH EVENT ID ELAPSED AGE\nX cool commit timed out trunk push 1 4m34s Feb 23, 2021\n* cool commit in progress trunk push 2 4m34s Feb 23, 2021\n✓ cool commit successful trunk push 3 4m34s Feb 23, 2021\nX cool commit cancelled trunk push 4 4m34s Feb 23, 2021\nX cool commit failed trunk push 1234 4m34s Feb 23, 2021\n- cool commit neutral trunk push 6 4m34s Feb 23, 2021\n- cool commit skipped trunk push 7 4m34s Feb 23, 2021\n* cool commit requested trunk push 8 4m34s Feb 23, 2021\n* cool commit queued trunk push 9 4m34s Feb 23, 2021\nX cool commit stale trunk push 10 4m34s Feb 23, 2021\n",
},
{
name: "blank nontty",
name: "default arguments nontty",
opts: &ListOptions{
Limit: defaultLimit,
PlainOutput: true,
Limit: defaultLimit,
now: shared.TestRunStartTime.Add(time.Minute*4 + time.Second*34),
},
nontty: true,
isTTY: false,
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs"),
@ -151,7 +154,9 @@ func TestListRun(t *testing.T) {
name: "pagination",
opts: &ListOptions{
Limit: 101,
now: shared.TestRunStartTime.Add(time.Minute*4 + time.Second*34),
},
isTTY: true,
stubs: func(reg *httpmock.Registry) {
var runID int64
runs := []shared.Run{}
@ -175,8 +180,7 @@ func TestListRun(t *testing.T) {
{
name: "no results",
opts: &ListOptions{
Limit: defaultLimit,
PlainOutput: true,
Limit: defaultLimit,
},
stubs: func(reg *httpmock.Registry) {
reg.Register(
@ -191,7 +195,9 @@ func TestListRun(t *testing.T) {
opts: &ListOptions{
Limit: defaultLimit,
WorkflowSelector: "flow.yml",
now: shared.TestRunStartTime.Add(time.Minute*4 + time.Second*34),
},
isTTY: true,
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/flow.yml"),
@ -249,7 +255,7 @@ func TestListRun(t *testing.T) {
}
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(!tt.nontty)
ios.SetStdoutTTY(tt.isTTY)
tt.opts.IO = ios
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")

24
pkg/cmd/run/shared/shared.go
View file

@ -49,6 +49,7 @@ var RunFields = []string{
"headSha",
"createdAt",
"updatedAt",
"startedAt",
"status",
"conclusion",
"event",
@ -61,11 +62,13 @@ type Run struct {
Name string
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
StartedAt time.Time `json:"run_started_at"`
Status Status
Conclusion Conclusion
Event string
ID int64
WorkflowID int64 `json:"workflow_id"`
Attempts uint8 `json:"run_attempt"`
HeadBranch string `json:"head_branch"`
JobsURL string `json:"jobs_url"`
HeadCommit Commit `json:"head_commit"`
@ -74,6 +77,25 @@ type Run struct {
HeadRepository Repo `json:"head_repository"`
}
func (r *Run) StartedTime() time.Time {
if r.StartedAt.IsZero() {
return r.CreatedAt
}
return r.StartedAt
}
func (r *Run) Duration(now time.Time) time.Duration {
endTime := r.UpdatedAt
if r.Status != Completed {
endTime = now
}
d := endTime.Sub(r.StartedTime())
if d < 0 {
return 0
}
return d.Round(time.Second)
}
type Repo struct {
Owner struct {
Login string
@ -329,7 +351,7 @@ func PromptForRun(cs *iostreams.ColorScheme, runs []Run) (string, error) {
symbol, _ := Symbol(cs, run.Status, run.Conclusion)
candidates = append(candidates,
// TODO truncate commit message, long ones look terrible
fmt.Sprintf("%s %s, %s (%s) %s", symbol, run.CommitMsg(), run.Name, run.HeadBranch, preciseAgo(now, run.CreatedAt)))
fmt.Sprintf("%s %s, %s (%s) %s", symbol, run.CommitMsg(), run.Name, run.HeadBranch, preciseAgo(now, run.StartedTime())))
}
// TODO consider custom filter so it's fuzzier. right now matches start anywhere in string but

52
pkg/cmd/run/shared/shared_test.go
View file

@ -1,10 +1,12 @@
package shared
import (
"encoding/json"
"net/http"
"testing"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/httpmock"
@ -52,3 +54,53 @@ func TestGetAnnotations404(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, result, []Annotation{})
}
func TestRun_Duration(t *testing.T) {
now, _ := time.Parse(time.RFC3339, "2022-07-20T11:22:58Z")
tests := []struct {
name string
json string
wants string
}{
{
name: "no run_started_at",
json: heredoc.Doc(`
{
"created_at": "2022-07-20T11:20:13Z",
"updated_at": "2022-07-20T11:21:16Z",
"status": "completed"
}`),
wants: "1m3s",
},
{
name: "with run_started_at",
json: heredoc.Doc(`
{
"created_at": "2022-07-20T11:20:13Z",
"run_started_at": "2022-07-20T11:20:55Z",
"updated_at": "2022-07-20T11:21:16Z",
"status": "completed"
}`),
wants: "21s",
},
{
name: "in_progress",
json: heredoc.Doc(`
{
"created_at": "2022-07-20T11:20:13Z",
"run_started_at": "2022-07-20T11:20:55Z",
"updated_at": "2022-07-20T11:21:16Z",
"status": "in_progress"
}`),
wants: "2m3s",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var r Run
assert.NoError(t, json.Unmarshal([]byte(tt.json), &r))
assert.Equal(t, tt.wants, r.Duration(now).String())
})
}
}

23
pkg/cmd/run/shared/test.go
View file

@ -5,23 +5,14 @@ import (
"time"
)
// Test data for use in the various run and job tests
func created() time.Time {
created, _ := time.Parse("2006-01-02 15:04:05", "2021-02-23 04:51:00")
return created
}
func updated() time.Time {
updated, _ := time.Parse("2006-01-02 15:04:05", "2021-02-23 04:55:34")
return updated
}
var TestRunStartTime, _ = time.Parse("2006-01-02 15:04:05", "2021-02-23 04:51:00")
func TestRun(name string, id int64, s Status, c Conclusion) Run {
return Run{
Name: name,
ID: id,
CreatedAt: created(),
UpdatedAt: updated(),
CreatedAt: TestRunStartTime,
UpdatedAt: TestRunStartTime.Add(time.Minute*4 + time.Second*34),
Status: s,
Conclusion: c,
Event: "push",
@ -66,8 +57,8 @@ var SuccessfulJob Job = Job{
Status: Completed,
Conclusion: Success,
Name: "cool job",
StartedAt: created(),
CompletedAt: updated(),
StartedAt: TestRunStartTime,
CompletedAt: TestRunStartTime.Add(time.Minute*4 + time.Second*34),
URL: "https://github.com/jobs/10",
RunID: 3,
Steps: []Step{
@ -91,8 +82,8 @@ var FailedJob Job = Job{
Status: Completed,
Conclusion: Failure,
Name: "sad job",
StartedAt: created(),
CompletedAt: updated(),
StartedAt: TestRunStartTime,
CompletedAt: TestRunStartTime.Add(time.Minute*4 + time.Second*34),
URL: "https://github.com/jobs/20",
RunID: 1234,
Steps: []Step{

4
pkg/cmd/run/view/view.go
View file

@ -319,7 +319,7 @@ func runView(opts *ViewOptions) error {
out := opts.IO.Out
ago := opts.Now().Sub(run.CreatedAt)
ago := opts.Now().Sub(run.StartedTime())
fmt.Fprintln(out)
fmt.Fprintln(out, shared.RenderRunHeader(cs, *run, utils.FuzzyAgo(ago), prNumber))
@ -416,7 +416,7 @@ func getLog(httpClient *http.Client, logURL string) (io.ReadCloser, error) {
}
func getRunLog(cache runLogCache, httpClient *http.Client, repo ghrepo.Interface, run *shared.Run) (*zip.ReadCloser, error) {
filename := fmt.Sprintf("run-log-%d-%d.zip", run.ID, run.CreatedAt.Unix())
filename := fmt.Sprintf("run-log-%d-%d.zip", run.ID, run.StartedTime().Unix())
filepath := filepath.Join(os.TempDir(), "gh-cli-cache", filename)
if !cache.Exists(filepath) {
// Run log does not exist in cache so retrieve and store it

2
pkg/cmd/run/watch/watch.go
View file

@ -206,7 +206,7 @@ func renderRun(out io.Writer, opts WatchOptions, client *api.Client, repo ghrepo
return nil, fmt.Errorf("failed to get run: %w", err)
}
ago := opts.Now().Sub(run.CreatedAt)
ago := opts.Now().Sub(run.StartedTime())
jobs, err := shared.GetJobs(client, repo, *run)
if err != nil {

5
pkg/cmd/run/watch/watch_test.go
View file

@ -288,7 +288,10 @@ func TestWatchRun(t *testing.T) {
} else {
assert.NoError(t, err)
}
assert.Equal(t, tt.wantOut, stdout.String())
// avoiding using `assert.Equal` here because it would print raw escape sequences to stdout
if got := stdout.String(); got != tt.wantOut {
t.Errorf("got stdout:\n%q\nwant:\n%q", got, tt.wantOut)
}
reg.Verify(t)
})
}

10
pkg/cmd/workflow/view/view.go
View file

@ -5,6 +5,7 @@ import (
"net/http"
"net/url"
"strings"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
@ -30,6 +31,8 @@ type ViewOptions struct {
Prompt bool
Raw bool
YAML bool
now time.Time
}
func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command {
@ -37,6 +40,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
IO: f.IOStreams,
HttpClient: f.HttpClient,
Browser: f.Browser,
now: time.Now(),
}
cmd := &cobra.Command{
@ -223,11 +227,7 @@ func viewWorkflowInfo(opts *ViewOptions, client *api.Client, repo ghrepo.Interfa
tp.AddField(string(run.Event), nil, nil)
if opts.Raw {
elapsed := run.UpdatedAt.Sub(run.CreatedAt)
if elapsed < 0 {
elapsed = 0
}
tp.AddField(elapsed.String(), nil, nil)
tp.AddField(run.Duration(opts.now).String(), nil, nil)
}
tp.AddField(fmt.Sprintf("%d", run.ID), nil, cs.Cyan)

24
pkg/ssh/ssh_keys.go
View file

@ -7,7 +7,6 @@ import (
"os/exec"
"path/filepath"
"runtime"
"strings"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/run"
@ -80,29 +79,6 @@ func (c *Context) GenerateSSHKey(keyName string, passphrase string) (*KeyPair, e
return &keyPair, nil
}
func (c *Context) GetPublicKeyFromPrivateKey(privateKeyPath string) (string, error) {
keygenExe, err := c.findKeygen()
if err != nil {
return "", err
}
keygenCmd := exec.Command(keygenExe, "-y", "-f", privateKeyPath)
keygenRunnable := run.PrepareCmd(keygenCmd)
fullPublicKeyBytes, err := keygenRunnable.Output()
if err != nil {
return "", fmt.Errorf("failed to generate public key: %w", err)
}
fullPublicKey := string(fullPublicKeyBytes)
// The public key format is "<type> <key>[ <comment>]" - remove the comment if it exists
publicKeyParts := strings.SplitN(fullPublicKey, " ", 3)
publicKeyWithoutComment := strings.Join(publicKeyParts[:2], " ")
return publicKeyWithoutComment, nil
}
func (c *Context) sshDir() (string, error) {
if c.ConfigDir != "" {
return c.ConfigDir, nil