Skip to content

Conversation

eedugon
Copy link
Contributor

@eedugon eedugon commented Jun 9, 2025

Preview:

Summary of changes:

  • Procedure updated to configure trust on ECH and ECK sides.
  • Step to modify trust.yml added due to being invalid as it is (whenever this is solved in cloud UI we should change this doc).
  • Yaml example updated, it had multiple bugs (thanks @naemono for highlighting that out).

Fixes #1378
Relates to private cloud issue https://github.com/elastic/cloud/issues/141360
Closes #1378

@eedugon eedugon requested review from jeanfabrice, naemono and pebrc June 9, 2025 09:43
@eedugon eedugon marked this pull request as ready for review June 9, 2025 09:50
@eedugon eedugon requested a review from a team as a code owner June 9, 2025 09:50
@eedugon eedugon changed the title ECH/ECK remote clusters is incorrect for trust.yml ECH/ECK remote clusters doc is incorrect for trust.yml Jun 9, 2025
Copy link
Contributor

@jeanfabrice jeanfabrice left a comment

Choose a reason for hiding this comment

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

I think it is confusing to let the customer to enter a value, <kubernetes-namespace>.es.local, and have it corrected afterwards (because of the issue with the wizard) in the trust.yml file with another value *.node.<cluster-name>.<kubernetes-namespace>.es.local, rather than:

  • not selecting trust clusters whose CN follow the Elastic pattern so that no misleading value is added to the trust.yml file,
  • later on, after downloading the trust.yml file, add the definitive correct value (*.node.<cluster-name>.<kubernetes-namespace>.es.local).

Both solutions are ok that said. The proposed solution has the advantage to be easier to amend once the wizard has been fix

@eedugon
Copy link
Contributor Author

eedugon commented Jun 10, 2025

@jeanfabrice , thanks! I think your proposal would simplify the process also, so let me apply a few changes and share them with you.

Update: I have a concern of your proposal:

  • From the ECK deployment point of view all is perfect, and your suggestion of not selecting anything in the Select Trusted clusters section of the UI should be OK as long as we add the ECK details to the generated trust.yml later on.
image

But what about from ECH deployment point of view? would ECH trust the external / self-managed (ECK) cluster providing the CA only and leaving that section empty?

@naemono , would you please help us to determine if that part of the section can be left empty or if it's better to provide a Scope ID and then fix the generated trust.yml?

cc: @pebrc

@eedugon
Copy link
Contributor Author

eedugon commented Jun 17, 2025

@jeanfabrice : As mentioned in private, your proposal won't work, as ECH needs the scope ID to be defined, as it was mentioned in the original doc. The why I don't know, but definitely ECH configures something that properly trusts the certificate coming from ECK when following the procedure.

So I'm leaving the procedure as it is, as I have fully tested it a few times and remote cluster functionality works fine in both directions.

In summary: The select trusted clusters section cannot be left empty

Copy link

github-actions bot commented Jun 17, 2025

🔍 Preview links for changed docs:

🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes.

@eedugon eedugon requested a review from shainaraskas June 17, 2025 07:07
@eedugon eedugon added bug Something isn't working Team:Admin Issues owned by the Admin Docs Team labels Jun 17, 2025
@eedugon
Copy link
Contributor Author

eedugon commented Jun 17, 2025

@shainaraskas : I'm pinging you for reviewing. Nothing urgent.

This PR fixes a bug that we had in the original procedure.

Anyway we should soon add to this document the procedure using API Keys for this use case (as that's feasible now but still not documented). But that should be done at a later stage.

@pebrc : Feel free to give your thoughts, the procedure has been tested using the exact yamls provided in the doc and all works fine.

@eedugon eedugon requested a review from jeanfabrice June 17, 2025 07:14
Copy link
Collaborator

@shainaraskas shainaraskas left a comment

Choose a reason for hiding this comment

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

really nice. nitpicks only

This guide uses TLS certificates to secure remote cluster connections and follows a similar approach to [Access clusters of a self-managed environment](ec-remote-cluster-self-managed.md).

### Establish trust in the {{ech}} cluster [ec_establish_trust_in_the_elasticsearch_service_cluster]
### Establish trust in the ECH cluster [ec_establish_trust_in_the_elasticsearch_service_cluster]
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: an intro sentence before a procedure is nice

eedugon and others added 2 commits June 25, 2025 10:19
Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com>
@eedugon eedugon requested a review from shainaraskas June 25, 2025 11:01
@eedugon
Copy link
Contributor Author

eedugon commented Jun 25, 2025

@shainaraskas : would you do a final review here? I've applied the suggestions you mentioned:

  • Moving edit trust.yml to its own heading (I added a short introductory text).
  • Added short introductions in the other headings.

Copy link
Collaborator

@shainaraskas shainaraskas left a comment

Choose a reason for hiding this comment

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

looks good to me

@eedugon eedugon merged commit bf331dc into main Jun 26, 2025
7 checks passed
@eedugon eedugon deleted the eck_trust_ech_fix branch June 26, 2025 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Team:Admin Issues owned by the Admin Docs Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Issue]: Doc about ECH/ECK CCS/CCR is misleading

3 participants