Remove redundant token BWC tests #89417

n1v0lg · 2022-08-17T10:11:32Z

This PR removes YAML cluster upgrade tests for token backwards
compatibility. The Java REST test suite TokenBackwardsCompatibilityIT
fully covers all scenarios included in the YAML tests already. Due to
the complex test setup, and since this particular YAML test suite is
not meant to be tested with different clients, Java REST tests are more
appropriate.

The test suite we are removing is resource intensive, both in terms of
build times and maintenance, and as such not justified as a redundant
suite.

Relates: #77350

n1v0lg · 2022-08-17T10:13:48Z

x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/50_token_auth.yml

@@ -1,130 +0,0 @@
---
-"Create a token and reuse it across the upgrade":


Covered by: https://github.com/elastic/elasticsearch/blob/main/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java#L57

n1v0lg · 2022-08-17T10:14:37Z

x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/50_token_auth.yml

-  - match: { expires_in: 3600 }
-  - is_false: scope
-
-  # test refresh token (use it)


Covered by: https://github.com/elastic/elasticsearch/blob/main/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java#L80

n1v0lg · 2022-08-17T10:15:51Z

x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/50_token_auth.yml

@@ -1,180 +0,0 @@
---
-"Get the indexed token and use if to authenticate":


Covered by: https://github.com/elastic/elasticsearch/blob/main/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java#L126

n1v0lg · 2022-08-17T10:16:22Z

x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/50_token_auth.yml

-  - match: { hits.total: 8 }
-
---
-"Get the indexed refreshed access token and use if to authenticate":


Covered by: https://github.com/elastic/elasticsearch/blob/main/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java#L126

(Token at idx 4)

n1v0lg · 2022-08-17T10:18:15Z

x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/50_token_auth.yml

-  - match: { hits.total: 8 }
-
---
-"Get the indexed refresh token and use it to get another access token and authenticate":


Covered by: https://github.com/elastic/elasticsearch/blob/main/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java#L169

In the above Java REST test, we "verify new nodes can refresh tokens created by old nodes and vice versa" so the scenario of using an old token on a new node is covered

n1v0lg · 2022-08-17T10:24:29Z

.../qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml

@@ -1,92 +0,0 @@
---
-"Get the indexed token and use if to authenticate":


Covered by: https://github.com/elastic/elasticsearch/blob/main/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java#L214

n1v0lg · 2022-08-17T10:24:53Z

.../qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml

-        index: token_index
-
---
-"Get the indexed refresh token and use if to get another access token and authenticate":


Covered by: https://github.com/elastic/elasticsearch/blob/main/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java#L232

n1v0lg · 2022-08-17T10:49:10Z

I will follow this PR up with an actual fix addressing #77350.

For the current PR, I just wanted another pair of eyes to confirm we are not losing coverage by dropping these.

elasticsearchmachine · 2022-08-17T11:37:31Z

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

jakelandis

LGTM, thanks for the links to the tests with duplication

albertzaharovits

LGTM
I appreciate the effort to track down all these!

* upstream/main: (265 commits) Disable openid connect tests due to missing fixture (elastic#89478) Add periodic job for single processor node testing Updates to changelog processing after docs redesign (elastic#89463) Better support for multi cluster for run task (elastic#89442) Mute failing tests (elastic#89465) [ML] Performance improvements related to ECS Grok pattern usage (elastic#89424) Add source fallback support for date and date_nanos mapped types (elastic#89440) Reuse Info in lifecycle step (elastic#89419) feature: support metrics for multi value fields (elastic#88818) Upgrade OpenTelemetry API and remove workaround (elastic#89438) Remove LegacyClusterTaskResultActionListener (elastic#89459) Add YAML spec docs about matching errors (elastic#89370) Remove redundant cluster upgrade tests for auth tokens (elastic#89417) Return 400 error for GetUserPrivileges call with API keys (elastic#89333) User Profile - Detailed errors in hasPrivileges response (elastic#89224) Rollover min_* conditions docs and highlight (elastic#89434) REST tests for percentiles_bucket agg (elastic#88029) REST tests for cumulative pipeline aggs (elastic#88966) Clean-up file watcher keys. (elastic#89429) fix a typo in Security.java (elastic#89248) ... # Conflicts: # server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java

Remove redundant cluster upgrade tests for auth tokens

bad969f

n1v0lg added >test Issues or PRs that are addressing/adding tests :Security/Security Security issues without another label labels Aug 17, 2022

n1v0lg self-assigned this Aug 17, 2022

elasticsearchmachine added the v8.5.0 label Aug 17, 2022

n1v0lg commented Aug 17, 2022

View reviewed changes

n1v0lg marked this pull request as ready for review August 17, 2022 11:37

n1v0lg requested review from albertzaharovits and jakelandis August 17, 2022 11:37

elasticsearchmachine added the Team:Security Meta label for security team label Aug 17, 2022

jakelandis approved these changes Aug 17, 2022

View reviewed changes

albertzaharovits approved these changes Aug 18, 2022

View reviewed changes

n1v0lg merged commit 1403ab3 into elastic:main Aug 18, 2022

n1v0lg deleted the remove-redundant-upgrade-tests-for-tokens branch August 18, 2022 12:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove redundant token BWC tests #89417

Remove redundant token BWC tests #89417

n1v0lg commented Aug 17, 2022 •

edited

Loading

n1v0lg Aug 17, 2022 •

edited

Loading

n1v0lg Aug 17, 2022

n1v0lg Aug 17, 2022

n1v0lg Aug 17, 2022

n1v0lg Aug 17, 2022

n1v0lg Aug 17, 2022

n1v0lg Aug 17, 2022

n1v0lg commented Aug 17, 2022 •

edited

Loading

elasticsearchmachine commented Aug 17, 2022

jakelandis left a comment

albertzaharovits left a comment

		@@ -1,130 +0,0 @@
		---
		"Create a token and reuse it across the upgrade":

		@@ -1,180 +0,0 @@
		---
		"Get the indexed token and use if to authenticate":

		@@ -1,92 +0,0 @@
		---
		"Get the indexed token and use if to authenticate":

Remove redundant token BWC tests #89417

Remove redundant token BWC tests #89417

Conversation

n1v0lg commented Aug 17, 2022 • edited Loading

n1v0lg Aug 17, 2022 • edited Loading

Choose a reason for hiding this comment

n1v0lg Aug 17, 2022

Choose a reason for hiding this comment

n1v0lg Aug 17, 2022

Choose a reason for hiding this comment

n1v0lg Aug 17, 2022

Choose a reason for hiding this comment

n1v0lg Aug 17, 2022

Choose a reason for hiding this comment

n1v0lg Aug 17, 2022

Choose a reason for hiding this comment

n1v0lg Aug 17, 2022

Choose a reason for hiding this comment

n1v0lg commented Aug 17, 2022 • edited Loading

elasticsearchmachine commented Aug 17, 2022

jakelandis left a comment

Choose a reason for hiding this comment

albertzaharovits left a comment

Choose a reason for hiding this comment

n1v0lg commented Aug 17, 2022 •

edited

Loading

n1v0lg Aug 17, 2022 •

edited

Loading

n1v0lg commented Aug 17, 2022 •

edited

Loading