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

Users/lukillgo/installsshkey #4258

Merged
merged 8 commits into from
May 8, 2017
Merged

Conversation

lkillgore
Copy link
Contributor

New task: Install SSH Key

"id": "5c9af2eb-5fc5-42dc-9b91-dc234a8c4400",
"name": "InstallSSHKey",
"friendlyName": "Install SSH Key (Preview)",
"description": "Install an SSH key prior to a build",
Copy link
Contributor

Choose a reason for hiding this comment

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

Install an SSH key prior to a build or release

{
"name": "hostName",
"type": "string",
"label": "Known hosts entry",
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize all words in the label for consistency with other fields.

"label": "SSH Key",
"defaultValue": "",
"required": true,
"helpMarkDown": "Select the SSH key that was uploaded to `Secure Files` to install on the build agent."
Copy link
Contributor

Choose a reason for hiding this comment

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

"build agent" can just be "agent" so that it applies to releases too.

}
},
"messages": {
"SSHKeyAlreadyInstalled": "The SSH Key is already installed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

"Key" shouldn't be capitalized here.

let deleteKey: string = tl.getTaskVariable(postDeleteKeySetting);
if (deleteKey) {
tl.debug('Deleting Key: ' + deleteKey);
tl.execSync(path.join(external, 'ssh-add.exe'), '-d ' + deleteKey);
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a risk of this not working when there are spaces in the deleteKey path. If you use a ToolRunner from the task library and add each item as a formal parameter ('-d', deleteKey), I think it will take care of putting quotes around the path for you if it has spaces.

if (knownHostsContents && knownHostsLocation) {
fs.writeFileSync(knownHostsLocation, knownHostsContents);
} else if (knownHostsLocation || knownHostsContents) {
tl.warning('Inconsistency with known_hosts cannot reset (location=' + knownHostsLocation + ' content=' + knownHostsContents + ')');
Copy link
Contributor

Choose a reason for hiding this comment

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

This warning should be localized and is a little difficult to understand. Or did you mean for it to be debug output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I probably should let them know that the known_hosts file has been changed and cannot be changed back. I'll re-work this.

} catch(err) {
tl.setResult(tl.TaskResult.Failed, err);
} finally {
// delete provisioning profile from temp location after installing
Copy link
Contributor

Choose a reason for hiding this comment

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

P999: This left-over comment refers to "provisioning profiles"


import util = require('./installsshkey-util');

// var sshAgent = require('ssh-agent-js');
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this commented line be removed?

@lkillgore lkillgore merged commit 9ccf41f into master May 8, 2017
@lkillgore lkillgore deleted the users/lukillgo/installsshkey branch May 8, 2017 13:02
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