-
-
Notifications
You must be signed in to change notification settings - Fork 812
Support staged rollout of migration to Rust Crypto #12184
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| /* | ||
| Copyright 2024 The Matrix.org Foundation C.I.C. | ||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| import { test, expect } from "../../element-web-test"; | ||
| import { logIntoElement } from "./utils"; | ||
|
|
||
| test.describe("Migration of existing logins", () => { | ||
| test("Test migration of existing logins when rollout is 100%", async ({ | ||
| page, | ||
| context, | ||
| app, | ||
| credentials, | ||
| homeserver, | ||
| }, workerInfo) => { | ||
| test.skip(workerInfo.project.name === "Rust Crypto", "This test only works with Rust crypto."); | ||
| await page.goto("/#/login"); | ||
|
|
||
| let featureRustCrypto = false; | ||
| let stagedRolloutPercent = 0; | ||
|
|
||
| await context.route(`http://localhost:8080/config.json*`, async (route) => { | ||
| const json = {}; | ||
| json["features"] = { | ||
| feature_rust_crypto: featureRustCrypto, | ||
| }; | ||
| json["setting_defaults"] = { | ||
| "RustCrypto.staged_rollout_percent": stagedRolloutPercent, | ||
| }; | ||
| await route.fulfill({ json }); | ||
| }); | ||
|
|
||
| await logIntoElement(page, homeserver, credentials); | ||
|
|
||
| await app.settings.openUserSettings("Help & About"); | ||
| await expect(page.getByText("Crypto version: Olm")).toBeVisible(); | ||
|
|
||
| featureRustCrypto = true; | ||
|
|
||
| await page.reload(); | ||
|
|
||
| await app.settings.openUserSettings("Help & About"); | ||
| await expect(page.getByText("Crypto version: Olm")).toBeVisible(); | ||
|
|
||
| stagedRolloutPercent = 100; | ||
|
|
||
| await page.reload(); | ||
|
|
||
| await app.settings.openUserSettings("Help & About"); | ||
| await expect(page.getByText("Crypto version: Rust SDK")).toBeVisible(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,15 +18,15 @@ limitations under the License. | |
| */ | ||
|
|
||
| import { | ||
| ICreateClientOpts, | ||
| PendingEventOrdering, | ||
| RoomNameState, | ||
| RoomNameType, | ||
| EventTimeline, | ||
| EventTimelineSet, | ||
| ICreateClientOpts, | ||
| IStartClientOpts, | ||
| MatrixClient, | ||
| MemoryStore, | ||
| PendingEventOrdering, | ||
| RoomNameState, | ||
| RoomNameType, | ||
| TokenRefreshFunction, | ||
| } from "matrix-js-sdk/src/matrix"; | ||
| import * as utils from "matrix-js-sdk/src/utils"; | ||
|
|
@@ -53,6 +53,7 @@ import PlatformPeg from "./PlatformPeg"; | |
| import { formatList } from "./utils/FormattingUtils"; | ||
| import SdkConfig from "./SdkConfig"; | ||
| import { Features } from "./settings/Settings"; | ||
| import { PhasedRolloutFeature } from "./utils/PhasedRolloutFeature"; | ||
|
|
||
| export interface IMatrixClientCreds { | ||
| homeserverUrl: string; | ||
|
|
@@ -302,13 +303,34 @@ class MatrixClientPegClass implements IMatrixClientPeg { | |
| throw new Error("createClient must be called first"); | ||
| } | ||
|
|
||
| const useRustCrypto = SettingsStore.getValue(Features.RustCrypto); | ||
| let useRustCrypto = SettingsStore.getValue(Features.RustCrypto); | ||
|
|
||
| // We want the value that is set in the config.json for that web instance | ||
| const defaultUseRustCrypto = SettingsStore.getValueAt(SettingLevel.CONFIG, Features.RustCrypto); | ||
| const migrationPercent = SettingsStore.getValueAt(SettingLevel.CONFIG, "RustCrypto.staged_rollout_percent"); | ||
|
|
||
| // If the default config is to use rust crypto, and the user is on legacy crypto, | ||
| // we want to check if we should migrate the current user. | ||
| if (!useRustCrypto && defaultUseRustCrypto && Number.isInteger(migrationPercent)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't bother with the check on defaultUseRustCrypto. It's fiddly to implement and I don't think it's hugely helpful.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it be strange that if
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Counterpoint: it's strange that setting staged_rollout_percent does nothing unless you also remember to set feature_rust_crypto. But you make a good point. I have no strong feelings. |
||
| // The user is not on rust crypto, but the default stack is now rust; Let's check if we should migrate | ||
| // the current user to rust crypto. | ||
| try { | ||
| const stagedRollout = new PhasedRolloutFeature("RustCrypto.staged_rollout_percent", migrationPercent); | ||
| // Device id should not be null at that point, or init crypto will fail anyhow | ||
| const deviceId = this.matrixClient.getDeviceId()!; | ||
| // we use deviceId rather than userId because we don't particularly want all devices | ||
| // of a user to be migrated at the same time. | ||
| useRustCrypto = stagedRollout.isFeatureEnabled(deviceId); | ||
| } catch (e) { | ||
| logger.warn("Failed to create staged rollout feature for rust crypto migration", e); | ||
| } | ||
| } | ||
|
|
||
| // we want to make sure that the same crypto implementation is used throughout the lifetime of a device, | ||
| // so persist the setting at the device layer | ||
| // (At some point, we'll allow the user to *enable* the setting via labs, which will migrate their existing | ||
| // device to the rust-sdk implementation, but that won't change anything here). | ||
| await SettingsStore.setValue("feature_rust_crypto", null, SettingLevel.DEVICE, useRustCrypto); | ||
| await SettingsStore.setValue(Features.RustCrypto, null, SettingLevel.DEVICE, useRustCrypto); | ||
|
|
||
| // Now we can initialise the right crypto impl. | ||
| if (useRustCrypto) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| /* | ||
| Copyright 2024 New Vector Ltd | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| import { xxHash32 } from "js-xxhash"; | ||
|
|
||
| /** | ||
| * The PhasedRolloutFeature class is used to manage the phased rollout of a new feature. | ||
| * | ||
| * It uses a hash of the user's identifier and the feature name to determine if a feature is enabled for a specific user. | ||
| * The rollout percentage determines the probability that a user will be enabled for the feature. | ||
| * The feature will be enabled for all users if the rollout percentage is 100, and for no users if the percentage is 0. | ||
| * If a user is enabled for a feature at x% rollout, it will also be for any greater than x percent. | ||
| * | ||
| * The process ensures a uniform distribution of enabled features across users. | ||
| * | ||
| * @property featureName - The name of the feature to be rolled out. | ||
| * @property rolloutPercentage - The int percentage (0..100) of users for whom the feature should be enabled. | ||
| */ | ||
| export class PhasedRolloutFeature { | ||
| public readonly featureName: string; | ||
| private readonly rolloutPercentage: number; | ||
| private readonly seed: number; | ||
|
|
||
| public constructor(featureName: string, rolloutPercentage: number) { | ||
| this.featureName = featureName; | ||
| if (!Number.isInteger(rolloutPercentage) || rolloutPercentage < 0 || rolloutPercentage > 100) { | ||
| throw new Error("Rollout percentage must be an integer between 0 and 100"); | ||
| } | ||
| this.rolloutPercentage = rolloutPercentage; | ||
| // We add the feature name for the seed to ensure that the hash is different for each feature | ||
| this.seed = Array.from(featureName).reduce((sum, char) => sum + char.charCodeAt(0), 0); | ||
| } | ||
|
|
||
| /** | ||
| * Returns true if the feature should be enabled for the given user. | ||
| * @param userIdentifier - Some unique identifier for the user, e.g. their user ID or device ID. | ||
| */ | ||
| public isFeatureEnabled(userIdentifier: string): boolean { | ||
| /* | ||
| * We use a hash function to convert the unique user ID string into an integer. | ||
| * This integer can then be used as a basis for deciding whether the user should have access to the new feature. | ||
| * We need some hash with good uniform distribution properties, security is not a concern here. | ||
| * We use xxHash32, which is fast and has good distribution properties. | ||
| */ | ||
| const hash = xxHash32(userIdentifier, this.seed); | ||
| // We use the hash modulo 100 to get a number between 0 and 99. | ||
| // Modulo is simple and effective and the distribution should be uniform enough for our purposes. | ||
| return hash % 100 < this.rolloutPercentage; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be
!==?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to test legacy to rust migration (feature_rust_crypto from
falsetotrue).In the project
Rust Crypto, the app is started withfeature_rust_crypto: trueBut anyhow it doesn't matter much as we anyhow override the
cryptoBackendby interceptinghttp://localhost:8080/config.json*, so it would test twice the same thing.