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

Environment Variables import .env files #2501

Merged
merged 7 commits into from
Jul 24, 2023
Merged

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented Jul 18, 2023

Description

As per #2372 (comment):

  • Adds search bar (as per Environment Variable Template Import #2372 (comment))
  • Adds Import .env button
  • Moves the "+" add button to the top of the table
    • NOTE: This is all achieved by utilising the ff component ff-table
  • Adds new template utility function parseDotEnv()
  • Implements the importing of env var file
  • Removes eslint exception for frontend/src/pages/admin/Template/sections/Environment.vue
  • Updates docs and images
  • Adds unit test for new utility function

Additional:

Following discussion with @joepavitt

  • Table elements sized up for better display of long vars
  • Table padding reduced to minimise scrolling
  • Fixes issues

New tests (frontend unit tests to cover new utility function dotenv

✓ test/unit/frontend/utils/admin/Template/utils.spec.js (5)
   ✓ dotenv (5)
     ✓ skips comments
     ✓ skips empty lines
     ✓ skips lines without =
     ✓ drops surrounding quotes
     ✓ keeps embedded quotes

Demo

env-var-demo.mp4

Related Issue(s)

#2372

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on flowforge/helm to update ConfigMap Template
    • Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

  • Backport needed? -> add the backport label
  • Includes a DB migration? -> add the area:migration label

@Steve-Mcl
Copy link
Contributor Author

For reference:

The dotenv package was NOT added since it requires node packages os and process internally. Without fallbacks in webpack (v5+) this causes an error. In the end, it made more sense to add a simple utility function for extracting env vars from .env file.

There is a follow up issue to raise around better sanitisation of env vars in the backend. For example, it is possible via the API to add blank name vars, duplicate vars, invalid named vars.

@Steve-Mcl Steve-Mcl linked an issue Jul 18, 2023 that may be closed by this pull request
4 tasks
@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #2501 (744a6c7) into main (5beb535) will decrease coverage by 0.07%.
The diff coverage is 8.88%.

@@            Coverage Diff             @@
##             main    #2501      +/-   ##
==========================================
- Coverage   39.94%   39.88%   -0.07%     
==========================================
  Files         489      489              
  Lines       17133    17198      +65     
  Branches     3976     3991      +15     
==========================================
+ Hits         6844     6859      +15     
- Misses      10289    10339      +50     
Flag Coverage Δ
backend 74.37% <ø> (ø)
frontend 1.63% <8.88%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../src/pages/admin/Template/sections/Environment.vue 0.00% <0.00%> (ø)
...ontend/src/pages/instance/Settings/Environment.vue 0.00% <0.00%> (ø)
frontend/src/pages/admin/Template/utils.js 7.28% <85.71%> (+7.28%) ⬆️

* Parse .env file data into an object
* NOTES:
* * Variable expansion is not supported
* * Comments are not supported
Copy link
Member

@knolleary knolleary Jul 24, 2023

Choose a reason for hiding this comment

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

Comments are not supported

Is that strictly true? I think the regex on line 308 means it will ignore lines that start with a comment character.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fairness, it should say comments are not imported / maintained like a regular .env file would be (they are discarded)

@knolleary
Copy link
Member

Verified I could upload a text file in env format (on Mac).

@knolleary knolleary merged commit 7a0ef5c into main Jul 24, 2023
4 of 5 checks passed
@knolleary knolleary deleted the 2372-env-template-support branch July 24, 2023 14:47
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.

Environment Variable Template Import
2 participants