Skip to content

Conversation

JeremyDahlgren
Copy link
Contributor

Currently the presence of credentials for a linked project alias determines if the RCS2.0 REMOTE_CLUSTER_PROFILE is used for the transport profile. For CPS we want to use the REMOTE_CLUSTER_PROFILE, but will not be using the credentials manager. This PR adds a check to see if CPS is enabled and if so the REMOTE_CLUSTER_PROFILE is used when constructing RemoteClusterConnection instances.

I wanted to keep the knowledge of CPS limited to the RemoteClusterService, so the transport profile name and a boolean for if the alias has credentials are now passed to the RemoteClusterConnection constructor. An additional unit test case has been added to verify that REMOTE_CLUSTER_PROFILE is used when CPS is enabled.

Resolves: ES-13109

… enabled

Currently the presence of credentials for a linked project alias determines
if the RCS2.0 REMOTE_CLUSTER_PROFILE is used for the transport profile.
For CPS we want to use the REMOTE_CLUSTER_PROFILE, but will not be using the
credentials manager.  This PR adds a check to see if CPS is enabled and if so
the REMOTE_CLUSTER_PROFILE is used when constructing RemoteClusterConnection
instances.

Resolves: ES-13109
@JeremyDahlgren JeremyDahlgren added >non-issue :Distributed Coordination/Network Http and internode communication implementations Team:Distributed Coordination Meta label for Distributed Coordination team v9.3.0 labels Oct 3, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Oct 6, 2025
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.

I have a different view on where we decide the transport profile.

Comment on lines 70 to 71
RemoteClusterCredentialsManager credentialsManager,
boolean hasCredentials,
Copy link
Member

Choose a reason for hiding this comment

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

It feels a bit awkward to me that hasCredentials is essentially redundant. I'd personally prefer passing crossProjectEnabled instead and still resolve the transport profile here. Both low and high level profiles are connection specific concepts. It feels natural to me that they are decided inside the connection? Also, I wonder whether we might need crossProjectEnabled in future for other things in this class, e.g. returning it somehow in getConnectionInfo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. I refactored to pass crossProjectEnabled.

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

Copy link
Contributor

@slobodanadamovic slobodanadamovic left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@slobodanadamovic
Copy link
Contributor

Note: The serverless checks are now failing which seem related. You may need to adjust some of the integration tests (probably mute for now).

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 >non-issue serverless-linked Added by automation, don't add manually Team:Distributed Coordination Meta label for Distributed Coordination team v9.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants