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

Support envFile option in launch.json #2462

Merged
merged 2 commits into from
Aug 17, 2018

Conversation

gregg-miskelly
Copy link
Contributor

Adds a new option for launch.json - envFile: it will read environment variables from the given UTF8 file. This way environment variables can be passed to the debugger and launch.json stays clean of sensitive data (like passwords).

This resolves #1944

@gregg-miskelly
Copy link
Contributor Author

@WardenGnaw the first commit in this PR is from @SebastianPfliegel. You are certainly welcome to review that code if you want, but I reviewed it earlier.

The second commit is my work and should definitely be reviewed.

@codecov
Copy link

codecov bot commented Aug 16, 2018

Codecov Report

Merging #2462 into master will increase coverage by 0.33%.
The diff coverage is 59.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2462      +/-   ##
==========================================
+ Coverage   64.23%   64.57%   +0.33%     
==========================================
  Files          89       90       +1     
  Lines        4054     4104      +50     
  Branches      573      587      +14     
==========================================
+ Hits         2604     2650      +46     
+ Misses       1283     1282       -1     
- Partials      167      172       +5
Flag Coverage Δ
#integration 53.42% <6.12%> (-0.42%) ⬇️
#unit 84.52% <86.66%> (+0.04%) ⬆️
Impacted Files Coverage Δ
src/configurationProvider.ts 21.42% <5.26%> (-6.35%) ⬇️
src/coreclr-debug/ParsedEnvironmentFile.ts 93.33% <93.33%> (ø)
src/features/codeLensProvider.ts 46.6% <0%> (-1.95%) ⬇️
src/omnisharp/server.ts 72.08% <0%> (-0.36%) ⬇️
src/features/documentation.ts 38.09% <0%> (+2.38%) ⬆️
src/omnisharp/delayTracker.ts 73.68% <0%> (+5.26%) ⬆️
src/omnisharp/requestQueue.ts 92.42% <0%> (+7.57%) ⬆️
src/features/completionItemProvider.ts 77.77% <0%> (+17.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87b8709...f0d5384. Read the comment docs.

Copy link
Contributor

@WardenGnaw WardenGnaw left a comment

Choose a reason for hiding this comment

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

Mostly nitpicks and requests for comments.

* Parse envFile and add to config.env
*/
private parseEnvFile(envFile: string, config: vscode.DebugConfiguration): vscode.DebugConfiguration {
if(envFile) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space after if

}

// remove envFile from config after parsing
if(config.envFile) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space after if

public Env: { [key: string]: any };
public Warning: string|null;

private constructor(env: { [key: string]: any }, warning: string|null)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space around |

private parseEnvFile(envFile: string, config: vscode.DebugConfiguration): vscode.DebugConfiguration {
if(envFile) {
try {
const parsedFile = ParsedEnvironmentFile.CreateFromFile(envFile, config["env"]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we get a type here?

/**
* Try to add all missing attributes to the debug configuration being launched.
*/
resolveDebugConfiguration(folder: vscode.WorkspaceFolder | undefined, config: vscode.DebugConfiguration, token?: vscode.CancellationToken): vscode.ProviderResult<vscode.DebugConfiguration> {
// vsdbg does the error checking

// read from envFile and set config.env
if(config.envFile) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space after if


return config;
}

/**
* Try to add all missing attributes to the debug configuration being launched.
*/
resolveDebugConfiguration(folder: vscode.WorkspaceFolder | undefined, config: vscode.DebugConfiguration, token?: vscode.CancellationToken): vscode.ProviderResult<vscode.DebugConfiguration> {
// vsdbg does the error checking
Copy link
Contributor

Choose a reason for hiding this comment

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

move to above line 144 return config and change to something like // vsdbg will error check the debug configuration fields

const openItem: MessageItem = { title: 'Open envFile' };
let result: MessageItem = await vscode.window.showWarningMessage(message, openItem);
if (result && result.title === openItem.title) {
let doc = await vscode.workspace.openTextDocument(fileName);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: type

}

content.split("\n").forEach(line => {
const r: RegExpMatchArray = line.match(/^\s*([\w\.\-]+)\s*=\s*(.*)?\s*$/);
Copy link
Contributor

Choose a reason for hiding this comment

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

comment what regex is looking for?

@gregg-miskelly gregg-miskelly force-pushed the env_file branch 2 times, most recently from 95c3355 to 45c2f6d Compare August 16, 2018 23:27
@gregg-miskelly
Copy link
Contributor Author

@WardenGnaw I believe I have fixed all your feedback

SebastianPfliegel and others added 2 commits August 16, 2018 23:09
This commit builds on the work of @SebastianPfliegel to finish up support for
env files in launch.json.

Two changes:
1. Fixes a build break in the way warnings were hooked up. I also decided to
add a button to open up the env file that has the warning.
2. Moves the parser into a seperate class and adds a unit test.
@gregg-miskelly gregg-miskelly merged commit f4e8700 into dotnet:master Aug 17, 2018
@gregg-miskelly gregg-miskelly deleted the env_file branch August 17, 2018 17:54
@andreasblueher
Copy link

@gregg-miskelly Hey gregg, where can I find documentation on how to use this feature?

@SebastianPfliegel
Copy link
Contributor

@andreasblueher simply add the path to an existing UTF-8 formatted file. The default value is "envFile": "${workspaceFolder}/.env", but can be changed to any value. The file itself should be formatted like this:

myvar=myvalue
myvar2 = another value
myvar3="this is my string"

When starting the debugger new environment variables will be created with the given values. This way you can hide secrets in different files from your debugger launch config.

@andreasblueher
Copy link

Thank you @SebastianPfliegel it appears for local azure function debugging it behaves differently because I couldn't get it workin, but adding a "local.settings.json" to my repo allowed me to store the env variables and access them from my code.

@armchairdeity
Copy link

armchairdeity commented Apr 15, 2020

@gregg-miskelly - Question: Does the parser do command substitution or support multi-line quoted sections?

I am trying to use the feature for my secrets and neither the quoted multiline nor the ${< /path/to/secret.key} features seem to be working.

Just to make this clearer, this is what I'm trying to do:

NODE_ENV=development
LOG_LEVEL=debug

JWT_PRIVATE_KEY="$(< /path/to/private.key)"
JWT_PUBLIC_KEY="$(< /path/to/public.key)"

... or ...

NODE_ENV=development
LOG_LEVEL=debug

JWT_PRIVATE_KEY="-----BEGIN OPENSSH PRIVATE KEY-----
... key junk goes right here ...
-----END OPENSSH PRIVATE KEY-----"

JWT_PUBLIC_KEY="$(< /path/to/public.key)"

@armchairdeity
Copy link

Specifically mirroring the version used in the Ruby Gem...

https://github.com/bkeepers/dotenv#command-substitution

@SebastianPfliegel
Copy link
Contributor

@armchairdeity if someone didn't improve my and @gregg-miskelly's code then it's not possible to do so. Maybe I'll find the time to look into that.

@armchairdeity
Copy link

@armchairdeity if someone didn't improve my and @gregg-miskelly's code then it's not possible to do so. Maybe I'll find the time to look into that.

I did take a brief look at the code... it's not terribly complicated, I just don't have time this week. I will try to look at it soon and if I can, contribute. Meanwhile I just embedded /n throughout my token... seems to be working OK for now. :)

Thanks for responding! I don't mind helping achieve the features I ask for... I'll see what I can do to help out! :)

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.

argsFile and envFile in launch.json
5 participants