Open
Conversation
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); |
There was a problem hiding this comment.
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`); |
There was a problem hiding this comment.
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); | |
| }); | |
| ); |
dmichon-msft
reviewed
Mar 4, 2024
dmichon-msft
left a comment
dmichon-msft
left a comment
There was a problem hiding this comment.
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', |
There was a problem hiding this comment.
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" | ||
| } |
There was a problem hiding this comment.
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" | ||
| } | ||
| ] | ||
| }, |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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:*", |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
Better to just replicate these two steps for each extension folder in the repository, and keep it all in one pipeline.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Details
How it was tested
Impacted documentation