Add default snapshot repo cluster setting #139155

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.

Sign up for GitHub

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

Jump to bottom

Open

seanzatzdev wants to merge 12 commits into elastic:main from seanzatzdev:default-repo

Contributor

seanzatzdev commented Dec 5, 2025 •

edited

Loading

This PR adds a cluster setting for the default repository. The setting defaults to "".

The default repo cannot be deleted, you must change the setting before being able to delete a repo. We check that the repo is registered upon updating the setting. The setting can be cleared by setting it to ""

closes #66040
closes https://github.com/elastic/elasticsearch-team/issues/2103


          add default snapshot repo cluster setting

8024dea

elasticsearchmachine added the v9.3.0 label


          Move to RepositoriesService. Add yaml rest tests.

8d6dfaa

seanzatzdev requested a review from Copilot

December 5, 2025 23:41

Copilot AI reviewed

View reviewed changes

Contributor

Copilot AI left a comment

Pull request overview

This PR introduces a cluster setting to specify a default snapshot repository for snapshot and restore operations. When configured, this eliminates the need to explicitly specify a repository name in these operations.

Key changes:

Added repositories.default_repository dynamic cluster setting
Implemented validation to ensure only registered repositories can be set as default
Added protection to prevent deletion of the default repository

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
`RepositoriesService.java`	Adds the default repository cluster setting, validation logic, getter/setter methods, and prevention of default repository deletion
`ClusterSettings.java`	Registers the new `DEFAULT_REPOSITORY_SETTING` in the cluster settings registry
`10_default_repository.yml`	Adds comprehensive YAML REST tests covering setting, validation, and edge cases for the default repository feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java Show resolved Hide resolved

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java Show resolved Hide resolved

seanzatzdev and others added 2 commits

December 5, 2025 18:48


          Update server/src/main/java/org/elasticsearch/repositories/Repositori…

35467c5

…esService.java

Co-authored-by: Copilot <[email protected]>


          Update server/src/main/java/org/elasticsearch/repositories/Repositori…

004eadb

…esService.java

Co-authored-by: Copilot <[email protected]>

seanzatzdev requested a review from dakrone

December 5, 2025 23:53


          revert copilot change

67011c1

seanzatzdev removed the request for review from dakrone

December 6, 2025 02:22

seanzatzdev added 4 commits

December 5, 2025 21:50


          add node feature for default repo

d4fa7a0


          add support for multi-project

e117940


          remove mocking from test suite for cluster settings

d8acbba


          fix npe in failing tests

c12a1fa

seanzatzdev marked this pull request as ready for review

December 8, 2025 00:43

seanzatzdev requested a review from a team as a code owner

December 8, 2025 00:43

elasticsearchmachine added the needs:triage label

seanzatzdev requested review from dakrone and masseyke and removed request for dakrone and masseyke

December 8, 2025 00:43


          Update comments

fd0c58d

seanzatzdev requested a review from Copilot

December 8, 2025 16:16

Copilot AI reviewed

View reviewed changes

Contributor

Copilot AI left a comment

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java Outdated Show resolved Hide resolved


          Refactor validate function.

967739d

seanzatzdev added :Data Management/Data streams Team:Data Management labels

elasticsearchmachine removed the needs:triage label

Collaborator

elasticsearchmachine commented Dec 8, 2025

Pinging @elastic/es-data-management (Team:Data Management)

seanzatzdev added the >non-issue label

nielsbauman reviewed

View reviewed changes

Contributor

nielsbauman left a comment

Thanks for working on this, Sean! I added a few minor comments, but we should get someone from the Distributed team to look at this as well, as they own the snapshot/restore feature.

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java Outdated Show resolved Hide resolved

...pi-spec/src/yamlRestTest/resources/rest-api-spec/test/repositories/10_default_repository.yml Outdated Show resolved Hide resolved

...pi-spec/src/yamlRestTest/resources/rest-api-spec/test/repositories/10_default_repository.yml Outdated Show resolved Hide resolved

...pi-spec/src/yamlRestTest/resources/rest-api-spec/test/repositories/10_default_repository.yml Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java Show resolved Hide resolved

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

Comment on lines +170 to +171

    
                      clusterService.getClusterSettings()

                          .addSettingsUpdateConsumer(DEFAULT_REPOSITORY_SETTING, this::setDefaultRepository, this::validateDefaultRepository);

Contributor

nielsbauman Dec 9, 2025

We should get someone from the Distributed team to have a look at this validation, to verify if there's any reason this validation could cause issues. I'll add the area label to the PR, could you reach out to #es-destrib and ask for a review?

nielsbauman added the :Distributed Coordination/Snapshot/Restore label

elasticsearchmachine added the Team:Distributed Coordination label

Collaborator

elasticsearchmachine commented Dec 9, 2025

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)


          address PR comments

29d8ac6

seanzatzdev requested a review from nielsbauman

December 10, 2025 15:07

ywangd reviewed

View reviewed changes

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

Comment on lines +225 to +235

    
                  private boolean isRepositoryRegistered(String repositoryName) {

                      if (repositoryOrNull(ProjectId.DEFAULT, repositoryName) != null) {

                          return true;

                      }

                      boolean found = repositories.values().stream().anyMatch(projectRepos -> projectRepos.containsKey(repositoryName));

                      if (found) {

                          return true;

                      }

                      found = internalRepositories.values().stream().anyMatch(projectRepos -> projectRepos.containsKey(repositoryName));

                      return found;

                  }

Member

ywangd Dec 15, 2025

Two comments:

We should not check internal repositories since those are used for CCR.
We should be explicit about it being the default project and not check with any other project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/Data streams :Distributed Coordination/Snapshot/Restore >non-issue Team:Data Management Team:Distributed Coordination v9.3.0