Skip to content

Create an InternalExpression type which allows precanned access to env.FIREBASE_CONFIG #1231

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

Merged
merged 7 commits into from
Oct 5, 2022

Conversation

Berlioz
Copy link
Contributor

@Berlioz Berlioz commented Sep 12, 2022

Defines special Expressions that users can import from firebase-functions/params that pull a value out of process.env.FIREBASE_CONFIG, but are never prompted for. Pairs with firebase/firebase-tools#4970.

Comment on lines 45 to 50
* A CEL expression which represents an internal Firebase variable. This class
* cannot be instantiated by developers, but we provide several canned instances
* of it to make available params that will never have to be defined at
* deployment time, and can always be read from process.env.
*/
export class InternalExpression extends Expression<string> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be marked @hidden or @beta or @Alpha or whatever so we don't expose it to our end user?

I wish we can use @internal, but I'm afraid that might break user code that tries to reference variables with that type 🤔

Copy link
Member

Choose a reason for hiding this comment

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

No, it should be @internal and the variable should be declared as an Expression. Type erasure is great for hiding implementation details and not tying our hands to them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. Done.

@Berlioz Berlioz changed the title WIP: Create an InternalExpression type which allows precanned access to env.FIREBASE_CONFIG Create an InternalExpression type which allows precanned access to env.FIREBASE_CONFIG Sep 14, 2022
@inlined
Copy link
Member

inlined commented Sep 29, 2022

Unfortunately the part you're trying not to do is the part I specifically want to do. I want people to be able to say:

const bucket = defineResource("BUCKET", {
  input: {
    resourceSelector: { type: "storage.googleapis.com/Bucket" },
   },
   default: params.storageBucket,
});

That means you're going to need to support both a toCEL and a CEL parser for something like {{ params.FIREBASE_CONFIG.bucket }} in the CLI

@inlined
Copy link
Member

inlined commented Sep 29, 2022

Hmmm... on second thought I can see how (for now) you can turn that into literal values in the CEL expression?... there may be less work than anticipated (e.g. not extending the parser) but the user journey is what I'm hoping for.

@Berlioz
Copy link
Contributor Author

Berlioz commented Sep 29, 2022

It turns out that feature isn't difficult to support. Done.

@inlined
Copy link
Member

inlined commented Oct 3, 2022

I think your magic transformation of FIREBASE_CONFIG to params.DEFAULT_BUCKET et. al. might be on the right track. The extensions team is looking at adding a bunch of new expressions, and I think I've negotiated that they will not be published as environment variables (since we'll use up the user's list). So I think we should strongly consider having a list of magic envs in the CLI that we have values for but don't write to .env files. Everything you've done here is a good candidate.

@Berlioz Berlioz merged commit 0cd8b9d into launch.next Oct 5, 2022
@taeold taeold added this to the v4 milestone Oct 6, 2022
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.

4 participants