Skip to content

Conversation

inlined
Copy link
Member

@inlined inlined commented Mar 24, 2025

apphosting:secrets:set now asks if the secret is test-only and can update apphosting.emulator.yaml instead for these cases (as well as granting individuals or groups access).

  1. When adding a new secret, prompts if this secret is for local development only.
  2. If only local development, asks for a list of users or groups to grant access.
  3. If local only, tries to add the secret to apphosting.emulator.yaml instead of apphosting.yaml
  4. If local only, tries stripping any "test-" prefix off of suggested key names to help with user flows where people will want something like stripe-key and test-stripe-key to both point to STRIPE_KEY in apphosting.yaml and apphosting.emulator.yaml respectively
  5. Fixes a slight bug where we found a project root with X file and try to insert the variable in Y file (e.g. apphosting.local.yaml exists, but we're adding to apphosting.yaml)
  6. Prints guidance on how to grant access to a secret in further commands

Base automatically changed from inlined.grantaccess-user to master March 24, 2025 18:53
Copy link
Contributor

@taeold taeold left a comment

Choose a reason for hiding this comment

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

Great devx

export async function maybeAddSecretToYaml(secretName: string): Promise<void> {
export async function maybeAddSecretToYaml(
secretName: string,
fileName: string = APPHOSTING_BASE_YAML_FILE,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: update docstring above
Given a secret name, guides the user whether they want to add that secret to the specified apphosting yaml file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed other bullets too. Good catch.

Comment on lines 219 to 220
} else if (upper.startsWith("TEST")) {
upper = upper.substring("TEST".length);
Copy link
Contributor

Choose a reason for hiding this comment

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

why not stick with "TEST_" only?

Unless there's a common pattern we want to support, "TEST" could always lead to surprises for secrets coincidentally named with a "TEST" prefix (TESTER, TESTING, , and instead we could just say we only support stripping "TEST_".
especially since we're forcing this upon the user as long as they tell us some secret is only for the local emulator

Copy link
Member Author

Choose a reason for hiding this comment

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

Reasonable point

@inlined inlined enabled auto-merge (squash) March 25, 2025 15:27
@inlined inlined merged commit 13c1962 into master Mar 25, 2025
49 checks passed
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.

3 participants