Stop subscribing users to security updates on setup - #10484
Merged
Conversation
The setup flow collected a "security updates" preference and forwarded it, along with the user's email, to an external comms endpoint to subscribe them to security update communications. We no longer want to send that subscription. The securityUpdates field stays in the API for backward compatibility but is marked deprecated and ignored by the server. > on setup we communicate subscribe to security update. remove the code from > the client and the server Explored the "comm prefs" feature end to end (webui setup form, /setup_lakefs and /setup_comm_prefs handlers, auth metadata, stats collector/sender). Removed the securityUpdates handling from the server (controller, metadata KV, stats) and from the webui, and dropped "security" from the checkbox label. > at this point keep the api/swagger.yml and just comment the field as > deprecated Kept securityUpdates in the Setup and CommPrefsInput schemas, marked deprecated: true with a note that the server ignores it, and regenerated the Go code and the Python/Java/Rust SDKs. > removing a required field is an api break, so lets keep it where it is a > required and just pass false. the server can keep ignore it as it is marked > as deprecated Restored securityUpdates to the CommPrefsInput required list to avoid the contract break; the webui now always sends securityUpdates: false, which the server accepts and ignores.
The E2E setup-validation and quickstart checks failed: SetupPage.fillForm
still located the opt-in checkbox by its old label ("...security, product and
feature updates"), which no longer exists after dropping "security" from the
label, so locator.check timed out on every setup test.
> review the github pull request checks that failed and see if we need to
> update the tests
The two failing E2E jobs both timed out at setupPage.ts:37 waiting for the old
label. Updated the POM locator to the new "I'd like to receive product and
feature updates" text.
N-o-Z
approved these changes
Jul 22, 2026
| @@ -148,12 +138,11 @@ func (m *KVMetadataManager) GetCommPrefs(ctx context.Context) (CommPrefs, error) | |||
| } | |||
|
|
|||
| return CommPrefs{ | |||
Contributor
There was a problem hiding this comment.
Not a blocker - but this method is no longer called anywhere. Might as well remove it altogether
Contributor
Author
There was a problem hiding this comment.
Good catch — removed GetCommPrefs and its now-orphaned helper getOptionalMetadataValue in a04b5bf.
After dropping the securityUpdates handling, KVMetadataManager.GetCommPrefs had no callers and is not part of the MetadataManager interface. Its only helper, getOptionalMetadataValue, was used solely by it. Both are dead code now. > review the pull-request comments - any change is needed? Reviewer flagged GetCommPrefs as no longer called anywhere. Confirmed it (and getOptionalMetadataValue, which would otherwise become an unused function) have no remaining references, and removed them.
kingster
added a commit
to kingster/lakeFS
that referenced
this pull request
Jul 23, 2026
* upstream/master: Refresh webui lock file with transitive dependency updates (treeverse#10494) Report data read/write size through lakeFS API and S3 gateway (treeverse#10487) Stop subscribing users to security updates on setup (treeverse#10484) # Conflicts: # pkg/api/apigen/lakefs.gen.go
nopcoder
added a commit
that referenced
this pull request
Jul 23, 2026
- SetCommPrefsOnce: once-only via SetIf; write data first, flip comm_prefs_set last as the commit point so a failed write stays retryable and concurrent writers can't double-commit - skip the not-initialized gate for external RBAC to match GetSetupState - keep the in-memory cache in sync with KV; a nil update un-sticks a previously cached true (Setup retry divergence) - validate email before any KV access - treat 409 as success in the webui setup client (double-submit/retry) - drop security_updates handling, removed in #10484
nopcoder
added a commit
that referenced
this pull request
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The setup flow collected a "security updates" preference and forwarded it, along with the user's email, to an external comms endpoint to subscribe them to security-update communications. We no longer want to send that subscription. The
securityUpdatesfield must remain in the API for backward compatibility, so it stays in the schema but is marked deprecated and ignored by the server.Prompt log
Explored the "comm prefs" feature end to end (webui setup form,
/setup_lakefsand/setup_comm_prefshandlers, auth metadata KV, stats collector/sender). RemovedsecurityUpdateshandling from the server and the webui, and dropped "security" from the setup checkbox label.Kept
securityUpdatesin theSetupandCommPrefsInputschemas, markeddeprecated: truewith a note that the server ignores it, and regenerated the Go code and the Python/Java/Rust SDKs.Restored
securityUpdatesto theCommPrefsInputrequired list to avoid the contract break; the webui now always sendssecurityUpdates: false, which the server accepts and ignores.The E2E setup-validation and Quickstart jobs timed out at
setupPage.ts:37because the Playwright POM still located the opt-in checkbox by its old label. Updated the POM locator to the new "I'd like to receive product and feature updates" text; both jobs pass now.Related Issue
Closes #10485
Test plan
go build ./...go veton pkg/api, pkg/auth, pkg/statsgolangci-linton affected packages — 0 issuesgo teston pkg/api (setup/comm-prefs), pkg/auth, pkg/stats — passmake gen-api+ SDK regen stable and idempotentnpm run lint— passes clean