Skip to content

Conversation

@atingmicrosoft
Copy link
Owner

Summary

Details

How it was tested

Impacted documentation

Comment on lines 89 to 107
const unresolvedUserFolder: string = homedir();
const userProfilePath: string = path.resolve(unresolvedUserFolder);
if (!FileSystem.exists(userProfilePath)) {
throw new Error("Unable to determine the current user's home directory");
}

const serveDataPath: string = path.join(userProfilePath, '.rushstack');
FileSystem.ensureFolder(serveDataPath);

const caCertificatePath: string = path.join(serveDataPath, 'rushstack-ca.pem');
const certificatePath: string = path.join(serveDataPath, 'rushstack-serve.pem');
const keyPath: string = path.join(serveDataPath, 'rushstack-serve.key');

if (pemCaCertificate) {
await FileSystem.writeFileAsync(caCertificatePath, pemCaCertificate);
}

await FileSystem.writeFileAsync(certificatePath, pemCertificate);
await FileSystem.writeFileAsync(keyPath, pemKey);

Choose a reason for hiding this comment

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

This code should all be redundant; ensureCertificateAsync contains exactly this logic internally.

Comment on lines 69 to 87
const certificateManager: CertificateManager = new CertificateManager();

// logger.terminal.writeLine(`Untrusting existing CA certificate`);
// await certificateManager.untrustCertificateAsync(logger.terminal);

terminal.writeLine(`Obtaining a TLS certificate signed by a local self-signed Certificate Authority`);

const {
pemCaCertificate,
pemCertificate,
pemKey
}: ICertificate & {
pemCaCertificate?: string;
} = await certificateManager.ensureCertificateAsync(true, terminal);

if (!pemCertificate || !pemKey) {
throw new Error(`No certificate available, exiting.`);
}
terminal.writeLine(`Trusted TLS certificate successfully obtained`);

Choose a reason for hiding this comment

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

Suggested change
const certificateManager: CertificateManager = new CertificateManager();
// logger.terminal.writeLine(`Untrusting existing CA certificate`);
// await certificateManager.untrustCertificateAsync(logger.terminal);
terminal.writeLine(`Obtaining a TLS certificate signed by a local self-signed Certificate Authority`);
const {
pemCaCertificate,
pemCertificate,
pemKey
}: ICertificate & {
pemCaCertificate?: string;
} = await certificateManager.ensureCertificateAsync(true, terminal);
if (!pemCertificate || !pemKey) {
throw new Error(`No certificate available, exiting.`);
}
terminal.writeLine(`Trusted TLS certificate successfully obtained`);
context.subscriptions.push(
vscode.commands.registerCommand('rushstack.ensureDebugCertificate', async () => {
const certificateManager: CertificateManager = new CertificateManager();
// logger.terminal.writeLine(`Untrusting existing CA certificate`);
// await certificateManager.untrustCertificateAsync(logger.terminal);
terminal.writeLine(`Obtaining a TLS certificate signed by a local self-signed Certificate Authority`);
const {
pemCaCertificate,
pemCertificate,
pemKey
}: ICertificate & {
pemCaCertificate?: string;
} = await certificateManager.ensureCertificateAsync(true, terminal);
if (!pemCertificate || !pemKey) {
throw new Error(`No certificate available, exiting.`);
}
terminal.writeLine(`Trusted TLS certificate successfully obtained`);
// TODO: Save the certificates to the remote machine.
// If this ends up requiring a separate extension, this will look something like:
// await vscode.commands.executeCommand('rushstack.saveDebugCertificates', pemCaCertificate, pemCertificate, pemKey);
});
);

Copy link

@dmichon-msft dmichon-msft left a comment

Choose a reason for hiding this comment

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

Although there's a bunch of bogus stuff in package.json, I don't see anything that would explain why the code doesn't run

Comment on lines 27 to 28
// eslint-disable-next-line @typescript-eslint/naming-convention
'@microsoft/rush-lib': 'commonjs @microsoft/rush-lib',

Choose a reason for hiding this comment

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

Suggested change
// eslint-disable-next-line @typescript-eslint/naming-convention
'@microsoft/rush-lib': 'commonjs @microsoft/rush-lib',

We won't need these for this extension.

Comment on lines 93 to 97
{
"command": "rushstack.saveDebugCertificate",
"category": "RushStack",
"title": "Save Debug Certificate"
}

Choose a reason for hiding this comment

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

Suggested change
{
"command": "rushstack.saveDebugCertificate",
"category": "RushStack",
"title": "Save Debug Certificate"
}

This belongs to the other extension

Comment on lines 99 to 157
"taskDefinitions": [
{
"type": "rush",
"required": [
"cwd",
"displayName",
"command",
"args"
],
"properties": {
"cwd": {
"type": "string",
"description": "The working directory for the task"
},
"displayName": {
"type": "string",
"description": "The display name for the command"
},
"command": {
"type": "string",
"description": "The command to run"
},
"args": {
"type": "array",
"description": "The arguments to pass to the command"
}
}
},
{
"type": "rushx",
"required": [
"cwd",
"command"
],
"properties": {
"cwd": {
"type": "string",
"description": "The working directory for the command"
},
"displayName": {
"type": "string",
"description": "The display name for the command"
},
"command": {
"type": "string",
"description": "The command to run"
}
}
}
],
"viewsContainers": {
"activitybar": [
{
"id": "rushstack",
"title": "Rush Stack",
"icon": "resources/rushstack-icon.svg"
}
]
},

Choose a reason for hiding this comment

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

Suggested change
"taskDefinitions": [
{
"type": "rush",
"required": [
"cwd",
"displayName",
"command",
"args"
],
"properties": {
"cwd": {
"type": "string",
"description": "The working directory for the task"
},
"displayName": {
"type": "string",
"description": "The display name for the command"
},
"command": {
"type": "string",
"description": "The command to run"
},
"args": {
"type": "array",
"description": "The arguments to pass to the command"
}
}
},
{
"type": "rushx",
"required": [
"cwd",
"command"
],
"properties": {
"cwd": {
"type": "string",
"description": "The working directory for the command"
},
"displayName": {
"type": "string",
"description": "The display name for the command"
},
"command": {
"type": "string",
"description": "The command to run"
}
}
}
],
"viewsContainers": {
"activitybar": [
{
"id": "rushstack",
"title": "Rush Stack",
"icon": "resources/rushstack-icon.svg"
}
]
},

These aren't provided by this extension

}
},
"activationEvents": [
"onView:rushCommands"

Choose a reason for hiding this comment

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

Suggested change
"onView:rushCommands"

Not applicable

Comment on lines 178 to 180
"@rushstack/rush-sdk": "workspace:*",
"@rushstack/ts-command-line": "workspace:*",
"@rushstack/rush-vscode-command-webview": "workspace:*",

Choose a reason for hiding this comment

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

Suggested change
"@rushstack/rush-sdk": "workspace:*",
"@rushstack/ts-command-line": "workspace:*",
"@rushstack/rush-vscode-command-webview": "workspace:*",

Comment on lines +37 to +45
- script: node $(Build.SourcesDirectory)/common/scripts/install-run-rushx.js package
workingDirectory: $(Build.SourcesDirectory)/vscode-extensions/tls-certification-ui
displayName: 'Package tls certification ui extension'

- script: node $(Build.SourcesDirectory)/common/scripts/install-run-rushx.js deploy
workingDirectory: $(Build.SourcesDirectory)/vscode-extensions/tls-certification-ui
displayName: 'Publish tls certification ui extension'
env:
VSCE_PAT: $(vscePat)

Choose a reason for hiding this comment

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

Better to just replicate these two steps for each extension folder in the repository, and keep it all in one pipeline.

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