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 Injecting Secrets into Apps Running in the Cloud #14612

Merged
merged 6 commits into from
Sep 15, 2022
Merged

Conversation

alecmerdler
Copy link
Contributor

@alecmerdler alecmerdler commented Sep 8, 2022

What does this PR do?

Adds a new '--secret' flag to 'lightning run app':

lightning run app --cloud --secret MY_SECRET=my-secret-name app.py

When the Lightning App runs in the cloud, the 'MY_SECRET'
environment variable will be populated with the value of the
referenced Secret. The value of the Secret is encrypted in the
database, and will only be decrypted and accessible to the
Flow/Work processes in the cloud.

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

@alecmerdler alecmerdler self-assigned this Sep 8, 2022
@github-actions github-actions bot added the app (removed) Generic label for Lightning App package label Sep 8, 2022
@Lightning-AI Lightning-AI deleted a comment from mergify bot Sep 8, 2022
Copy link
Contributor

@awaelchli awaelchli left a comment

Choose a reason for hiding this comment

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

It's finally coming <3

src/lightning_app/runners/cloud.py Outdated Show resolved Hide resolved
src/lightning_app/utilities/secrets.py Show resolved Hide resolved
src/lightning_app/cli/lightning_cli.py Outdated Show resolved Hide resolved
@Borda Borda added this to the app:0.7 milestone Sep 8, 2022
@alecmerdler alecmerdler marked this pull request as ready for review September 8, 2022 17:41
@mergify mergify bot added the ready PRs ready to be merged label Sep 8, 2022
@edenlightning
Copy link
Contributor

This needs documentation @Felonious-Spellfire

@alecmerdler alecmerdler force-pushed the LAI2-10393 branch 3 times, most recently from 8751291 to f9529e5 Compare September 12, 2022 21:07
@alecmerdler alecmerdler force-pushed the LAI2-10393 branch 6 times, most recently from 4a6c0c2 to abfd373 Compare September 14, 2022 15:56
@alecmerdler alecmerdler force-pushed the LAI2-10393 branch 2 times, most recently from e1acedd to e807d20 Compare September 14, 2022 17:24
@alecmerdler
Copy link
Contributor Author

@hhsecond The App.cloud-e2e tests are failing again. Could you help out when you get a chance?

docs/source-app/glossary/secrets.rst Outdated Show resolved Hide resolved
@mergify mergify bot added the ready PRs ready to be merged label Sep 14, 2022
Copy link
Contributor

@Felonious-Spellfire Felonious-Spellfire left a comment

Choose a reason for hiding this comment

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

LGTM

Adds a new '--secret' flag to 'lightning run app':

lightning run app --cloud --secret MY_SECRET=my-secret-name app.py

When the Lightning App runs in the cloud, the 'MY_SECRET'
environment variable will be populated with the value of the
referenced Secret. The value of the Secret is encrypted in the
database, and will only be decrypted and accessible to the
Flow/Work processes in the cloud.
@lexierule lexierule merged commit 71719b9 into master Sep 15, 2022
@lexierule lexierule deleted the LAI2-10393 branch September 15, 2022 13:16
Copy link
Contributor

@edenlightning edenlightning left a comment

Choose a reason for hiding this comment

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

Encrypted Secrets
#################

We understand that many Apps require access to private data like API keys, access tokens, database passwords, or other credentials. And that you need to protect this data.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove "We understand", just say "Many apps require access..."

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Please review my PR. The structure was completely changed


We understand that many Apps require access to private data like API keys, access tokens, database passwords, or other credentials. And that you need to protect this data.

Secrets provie a secure way to make private data like API keys or passwords accessible to your app, without hardcoding. You can use secrets to authenticate third-party services/solutions.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Please review my PR. The structure was completely changed and this typo came across when it was.

Comment on lines +14 to +23
*******************
Overview of Secrets
*******************

The ``--secret`` option has been added to the **lightning run app** command. ``--secret`` can be used by itself or alongside ``--env``.

When a Lightning App (App) **runs in the cloud**, the Secret can be exposed to the App using environment variables.
The value of the Secret is encrypted in the Lightning.ai database, and is only decrypted and accessible to
LightningFlow (Flow) or LightningWork (Work) processes in the cloud (when you use the ``--cloud`` option running your App).

Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Please review my PR. The structure was completely changed

----

*********************
Use Encrypted Secrets
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is specific to lightning cloud, we should say "Add secrets to Lightning Cloud"

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm gonna disagree on this. It's called out that this is only available in the cloud in several places. What we want to point out is how they should be using the feature correctly.

Use Encrypted Secrets
*********************

First, a Secret must be created using the admin web UI. Once you create a Secret, you can bind it to any of your Apps. You do not need to create a new Secret for each App if the Secret value is the same.
Copy link
Contributor

Choose a reason for hiding this comment

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

Step 1- Log into lighning.ai to add your secrets.
Avatar > Profile > secrets > add a secret

Add a screenshot/gif of how to do that

Copy link
Contributor

Choose a reason for hiding this comment

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


First, a Secret must be created using the admin web UI. Once you create a Secret, you can bind it to any of your Apps. You do not need to create a new Secret for each App if the Secret value is the same.

.. note::
Copy link
Contributor

Choose a reason for hiding this comment

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

Step 2- Add a secret

Copy link
Contributor

Choose a reason for hiding this comment

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


In the example below, we already used the admin UI to create a Secret named ``my-secret`` with the value ``some-value``` and will bind it to the environment variable ``MY_APP_SECRET`` within our App. The binding is accomplished by using the ``--secret`` option when running the App from the Lightning CLI.

The ``--secret``` option works similar to ``--env``, but instead of providing a value, you provide the name of the Secret which will be replaced with with the value that you want to bind to the environment variable:
Copy link
Contributor

Choose a reason for hiding this comment

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

Step 3- Add env variable to your app

Copy link
Contributor

Choose a reason for hiding this comment

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


The ``--secret``` option works similar to ``--env``, but instead of providing a value, you provide the name of the Secret which will be replaced with with the value that you want to bind to the environment variable:

.. code:: bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Step 4- add the secret to the lighnging app

Copy link
Contributor

Choose a reason for hiding this comment

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

@Felonious-Spellfire
Copy link
Contributor

also @alecmerdler @Felonious-Spellfire I didn't see anything in the doc about the secret name formatting constraints i.e. that you can't use underscores in the secret name

@nohalon
It's here: #14734
My changes kept getting overwritten so I made my own PR so that wouldn't happen.

Borda pushed a commit that referenced this pull request Oct 19, 2022
Adds a new '--secret' flag to 'lightning run app':

lightning run app --cloud --secret MY_SECRET=my-secret-name app.py

When the Lightning App runs in the cloud, the 'MY_SECRET'
environment variable will be populated with the value of the
referenced Secret. The value of the Secret is encrypted in the
database, and will only be decrypted and accessible to the
Flow/Work processes in the cloud.

Co-authored-by: Sherin Thomas <[email protected]>
Co-authored-by: Noha Alon <[email protected]>
Co-authored-by: thomas chaton <[email protected]>

(cherry picked from commit 71719b9)
Borda pushed a commit that referenced this pull request Oct 19, 2022
Adds a new '--secret' flag to 'lightning run app':

lightning run app --cloud --secret MY_SECRET=my-secret-name app.py

When the Lightning App runs in the cloud, the 'MY_SECRET'
environment variable will be populated with the value of the
referenced Secret. The value of the Secret is encrypted in the
database, and will only be decrypted and accessible to the
Flow/Work processes in the cloud.

Co-authored-by: Sherin Thomas <[email protected]>
Co-authored-by: Noha Alon <[email protected]>
Co-authored-by: thomas chaton <[email protected]>

(cherry picked from commit 71719b9)
lantiga pushed a commit that referenced this pull request Oct 20, 2022
* Support Injecting Secrets into Apps Running in the Cloud (#14612)

Adds a new '--secret' flag to 'lightning run app':

lightning run app --cloud --secret MY_SECRET=my-secret-name app.py

When the Lightning App runs in the cloud, the 'MY_SECRET'
environment variable will be populated with the value of the
referenced Secret. The value of the Secret is encrypted in the
database, and will only be decrypted and accessible to the
Flow/Work processes in the cloud.

Co-authored-by: Sherin Thomas <[email protected]>
Co-authored-by: Noha Alon <[email protected]>
Co-authored-by: thomas chaton <[email protected]>

(cherry picked from commit 71719b9)

* secrets docs (#14951)

* secrets docs

* Update docs/source-app/glossary/secrets.rst

Co-authored-by: Yurij Mikhalevich <[email protected]>

* Apply suggestions from code review

Co-authored-by: Adrian Wälchli <[email protected]>

* Update secrets.rst

* links

Co-authored-by: Yurij Mikhalevich <[email protected]>
Co-authored-by: Jirka Borovec <[email protected]>
Co-authored-by: Adrian Wälchli <[email protected]>
Co-authored-by: Jirka <[email protected]>
(cherry picked from commit 8715cd0)

# Conflicts:
#	docs/source-app/glossary/secrets.rst

* Add support for command descriptions (#15193)

(cherry picked from commit 4acb10f)

* docs: temp drop S3 from index (#15099)

Co-authored-by: awaelchli <[email protected]>
(cherry picked from commit 05d91c8)

* version 0.7.0

* chlog join 0.6.3 & 0.7

Co-authored-by: Alec Merdler <[email protected]>
Co-authored-by: edenlightning <[email protected]>
Co-authored-by: Ethan Harris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app (removed) Generic label for Lightning App package ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.