Skip to content

[Backport] Mute tests in SSLErrorMessageFileTests that rely on Se… #131342

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.

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

Open
wants to merge 1 commit into
base: 9.0
Choose a base branch
from

Conversation

gmjehovich
Copy link
Contributor

Backport of PR #130474

References #127192 #127193 #127190

@gmjehovich gmjehovich self-assigned this Jul 15, 2025
@gmjehovich gmjehovich added >non-issue :Security/TLS SSL/TLS, Certificates Team:Core/Infra Meta label for core/infra team Team:Security Meta label for security team auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged v8.19.0 v9.1.0 :Core/Infra/Entitlements Entitlements infrastructure v8.17.8 v8.18.3 labels Jul 15, 2025
@gmjehovich gmjehovich marked this pull request as ready for review July 16, 2025 00:57
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@gmjehovich gmjehovich requested a review from n1v0lg July 16, 2025 01:06
Copy link
Contributor

@ldematte ldematte left a comment

Choose a reason for hiding this comment

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

These tests are never going to be fixed or work in 9.0/8.18.
It would be better to remove them from those branches.

@n1v0lg
Copy link
Contributor

n1v0lg commented Jul 16, 2025

@gmjehovich I'm ++ on @ldematte's suggestion

a few other questions:

  • are the tests failing on 8.17 and 8.18? I don't recall if we disabled the security manager there too, or not.
  • this PR is open against 9.0 so I don't think the backport would work against 9.1

@ldematte
Copy link
Contributor

are the tests failing on 8.17 and 8.18? I don't recall if we disabled the security manager there too, or not.

8.17 is OK, no change.
8.18 already has entitlements (no security manager).

this PR is open against 9.0 so I don't think the backport would work against 9.1

9.1 should be OK -- correct me if I'm wrong, but in 8.19/9.1/main we now have unit tests for entitlements and these test should have been updated to use it?
I think it's OK to mute in 8.19/9.1/main until we get around to use entitlements in these tests (if we havent' done that already!).
But we are never going to backport unit tests for entitlements to 9.0/8.18, so for these branches we should delete instead of mute (IMO).

@n1v0lg
Copy link
Contributor

n1v0lg commented Jul 17, 2025

9.1 should be OK -- correct me if I'm wrong, but in 8.19/9.1/main we now have unit tests for entitlements and these test should have been updated to use it?

@ldematte I just meant that I'm not sure our backport tool works if you open a PR against 9.0 with a 9.1 label -- I'm just assuming that the backport tool would only open backport PRs against versions lower than the branch the PR is open against.

@slobodanadamovic
Copy link
Contributor

slobodanadamovic commented Jul 17, 2025

@ldematte I just meant that I'm not sure our backport tool works if you open a PR against 9.0 with a 9.1 label -- I'm just assuming that the backport tool would only open backport PRs against versions lower than the branch the PR is open against.

I can confirm that this works :)
See this example of backport from 8.x to main

Edit: Sorry, just double checked it and it wont work. The 9.1 is not listed here:

"targetBranchChoices" : [ "main", "9.0", "8.18", "8.17", "8.16", "8.15", "8.14", "8.13", "8.12", "8.11", "8.10", "8.9", "8.8", "8.7", "8.6", "8.5", "8.4", "8.3", "8.2", "8.1", "8.0", "7.17", "6.8" ],

It would work from 9.0 to main, but not 9.0 to 9.1

@ldematte
Copy link
Contributor

@mosche confirmed that he fixed the tests for 8.19/9.1/main porting them to use entitlements. These tests should be now unmuted on those branches, and should be not muted again.

We are not backporting unit tests support for entitlement to 8.18/9.0, so these test there will never work. I suggest we remove them.

8.17 is unchanged and uses the SM if available, which means Java < 24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Core/Infra/Entitlements Entitlements infrastructure >non-issue :Security/TLS SSL/TLS, Certificates Team:Core/Infra Meta label for core/infra team Team:Security Meta label for security team v8.17.8 v8.18.3 v8.19.0 v9.0.5 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants