Skip to content

Conversation

@JeremyDahlgren
Copy link
Contributor

Adds a new unit test class for TransportReloadRemoteClusterCredentialsAction. This is a follow up from #135491, where a significant chunk of code was moved from RemoteClusterService into TransportReloadRemoteClusterCredentialsAction. This also verifies code touched recently in #139012.

Resolves: ES-13161

Adds a new unit test class for TransportReloadRemoteClusterCredentialsAction.
This is a follow up from elastic#135491, where a significant chunk of code was
moved from RemoteClusterService into TransportReloadRemoteClusterCredentialsAction.
This also verifies code touched recently in elastic#139012.

Resolves: ES-13161
@JeremyDahlgren JeremyDahlgren added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Network Http and internode communication implementations Team:Distributed Coordination Meta label for Distributed Coordination team v9.3.0 labels Dec 12, 2025
@JeremyDahlgren JeremyDahlgren marked this pull request as ready for review December 12, 2025 01:54
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

clusterService.getClusterApplierService()
.setInitialState(new ClusterState.Builder(clusterService.getClusterName()).blocks(blocks).build());
Consumer<ActionListener<ActionResponse.Empty>> consumer = listener -> action.execute(task, request, listener);
expectThrows(ClusterBlockException.class, () -> { throw safeAwaitFailure(consumer); });
Copy link
Member

Choose a reason for hiding this comment

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

Seems unnecessary to throw it again?

Suggested change
expectThrows(ClusterBlockException.class, () -> { throw safeAwaitFailure(consumer); });
assertThat(safeAwaitFailure(consumer), instanceOf(ClusterBlockException.class));

or we can inline the consumer with

safeAwaitFailure(ClusterBlockException.class, ActionResponse.Empty.class, l -> action.execute(task, request, l));

Comment on lines 185 to 186
.put("cluster.remote.foo.proxy_address", remoteNode.getAddress().toString())
.setSecureSettings(toSecureSettings("foo", randomAlphaOfLength(10)))
Copy link
Member

Choose a reason for hiding this comment

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

It's better if the test does not change the proxy_address so that we can verify the behaviour with just the credentials update.

@JeremyDahlgren JeremyDahlgren merged commit c5c2773 into elastic:main Dec 13, 2025
35 checks passed
parkertimmins pushed a commit to parkertimmins/elasticsearch that referenced this pull request Dec 17, 2025
…astic#139414)

Adds a new unit test class for TransportReloadRemoteClusterCredentialsAction.
This is a follow up from elastic#135491, where a significant chunk of code was
moved from RemoteClusterService into TransportReloadRemoteClusterCredentialsAction.
This also verifies code touched recently in elastic#139012.

Resolves: ES-13161
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Network Http and internode communication implementations Team:Distributed Coordination Meta label for Distributed Coordination team >test Issues or PRs that are addressing/adding tests v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants