Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(util): allow tryRun to take a regex. #6640

Merged
merged 3 commits into from
Feb 20, 2025

Conversation

Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Feb 20, 2025

Problem

This is aimed at fixing the following ec2 bug.

Attempting to generate the ssh keys via tryRun can log a failure, when there is no failure. Looking at

public static async tryKeyGen(keyPath: string, keyType: sshKeyType): Promise<boolean> {
const overrideKeys = async (_t: string, proc: RunParameterContext) => {
await proc.send('yes')
}
return !(await tryRun('ssh-keygen', ['-t', keyType, '-N', '', '-q', '-f', keyPath], 'yes', 'unknown key type', {
onStdout: overrideKeys,
timeout: new Timeout(5000),
}))
}

We use tryRun to detect if the output contains "unknown key type". This allows us to detect when a certain key type such as RSA or ed25519 is not supported on the local machine. Detecting if the key generated successfully is a harder problem, as the output did not contain a consistent message.

However, it results in tryRun logging a failure to find "unknown key type" in the output, which means failed to find the expected text in the output, but still generated the key. This is then logged as a failure, but is functionally a success. This is very confusing behavior.

The root of this problem is we are trying to use tryRun in a way it doesn't naturally support. Rather than use it awkwardly, we can extend it.

Solution

  • add support for regex in tryRun

New logs:

2025-02-20 12:07:01.624 [info] tryRun: ok: PID 22605: [ssh-keygen -t ed25519 -N  -q -f /Users/hkobew/Library/Application Support/Code/User/globalStorage/amazonwebservices.aws-toolkit-vscode/aws-ec2-key] {
  exitCode: 0,
  stdout: '/Users/hkobew/Library/Application Support/Code/User/globalStorage/amazonwebservices.aws-toolkit-vscode/aws-ec2-key already exists.\n' +
    'Overwrite (y/n)?',
  stderr: '',
  error: undefined,
  signal: undefined
}

  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Hweinstock Hweinstock changed the title refactor(util): allow tryRun to take a regEx. refactor(util): allow tryRun to take a regex. Feb 20, 2025
@Hweinstock Hweinstock marked this pull request as ready for review February 20, 2025 17:36
@Hweinstock Hweinstock requested a review from a team as a code owner February 20, 2025 17:36
Copy link
Contributor

@justinmk3 justinmk3 left a comment

Choose a reason for hiding this comment

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

Nice!

@justinmk3 justinmk3 merged commit 82b016f into aws:master Feb 20, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants