Skip to content

prevent environment key from leaking into all builds#9752

Merged
brad-decker merged 1 commit intodevelopfrom
patch-env-setup
Oct 29, 2020
Merged

prevent environment key from leaking into all builds#9752
brad-decker merged 1 commit intodevelopfrom
patch-env-setup

Conversation

@brad-decker
Copy link
Copy Markdown
Contributor

Fixes: all builds in circleci would include the segment write key, resulting in test data in our segment instance.

Explanation: The logic for setting the real key based on the environment was flawed, and the safest way of dealing with this is having a separate key/env name for production environments, even if it's the same underlying value.

Manual testing steps:

  • Unzip a build without this change and a build that is on this pr
  • Examine the UI build output for our segment legacy write key (see brad for key, or login to segment)
  • It should exist in any previous build, and not exist in builds on this PR

NOTE
@Gudahtt, we should probably make this change immediately after the final release of 8.1.3, or include this in the release. Either way I will not update the environment variables in CircleCI until this PR is included in a release to avoid missing the key assignment.

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

This makes sense to me!

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [0144f28]
Page Load Metrics (430 ± 65 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint297940147
domContentLoaded24265342913665
load24365443013665
domInteractive24265242813665

@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Oct 29, 2020

@brad-decker is this ready to bring out of draft?

@brad-decker brad-decker marked this pull request as ready for review October 29, 2020 14:06
@brad-decker brad-decker requested review from a team and kumavis as code owners October 29, 2020 14:06
@brad-decker brad-decker requested a review from Gudahtt October 29, 2020 14:06
@brad-decker
Copy link
Copy Markdown
Contributor Author

@Gudahtt sorry, ready now -- should I mark this as do not merge until 8.1.3 is published or will we just note not to include it in that build?

@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Oct 29, 2020

v8.1.3 is out! So that's no longer a concern.

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@brad-decker brad-decker merged commit c130a7a into develop Oct 29, 2020
@brad-decker brad-decker deleted the patch-env-setup branch October 29, 2020 14:31
@github-actions github-actions bot locked and limited conversation to collaborators Oct 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants