Remove @UpdateForV9 annotations from Security code #123176

mattc58 · 2025-02-21T18:09:25Z

This removes instances of the @UpdateForV9 annotation since the code it was referencing has been changed for version 9.0.0.

… removed

…ateforv9

…o remove-security-updateforv9

elasticsearchmachine · 2025-02-24T23:57:11Z

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

jakelandis

Due the partial removal of some of the related functionality, I think this needs a slightly deeper dive before we kick the can down the road to v10.

jakelandis · 2025-02-25T16:25:20Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/License.java

@@ -142,7 +142,7 @@ static boolean isEnterprise(String typeName) {
     * XContent param name to map the "enterprise" license type to "platinum"
     * for backwards compatibility with older clients
     */
-    @UpdateForV9(owner = UpdateForV9.Owner.SECURITY) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7
+    @UpdateForV10(owner = UpdateForV10.Owner.SECURITY) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7


I'm not sure that is as simple as it appears since this a marker for some partially removed functionality:

This is related to use and deprecation of "accept_enterprise" for license. This specific string isn't the primary reason for the annotation, rather it a marker for the related code which has been partially removed.

Specifically it is part of the change code via 8.x branch: https://github.com/elastic/elasticsearch/blob/8.x/x-pack/plugin/core/src/main/java/org/elasticsearch/license/RestGetLicenseAction.java#L82

It also says that if you send "accept_enterprise=false" on or after REST API v8, then raise an exception. To put another way it says it if you send "accept_enterprise=false" before REST API v8 by sending the v7 compatibility header, then don't error.

Since we removed support for v7 compatibility that logic was changed via https://github.com/elastic/elasticsearch/pull/114881/files#diff-f0af8e6057207cbe3628cfbc9bcab24d2d38ad1c39bdb34efb92262dbd3eb8e4L75.

It also appears there was additional related stuff to the v7 REST API compatibility that was removed s
https://github.com/elastic/elasticsearch/blob/8.x/x-pack/plugin/core/src/main/java/org/elasticsearch/license/RestGetLicenseAction.java#L66-L72 (this part is relevant to this exact string)

So it appears that some of this functionality is already partially removed with the expectation it will be fully removed. Any gates that changed behavior based on the v7 REST API have been removed. I am not familiar enough with the functionality or original change to comment on exactly what functionality is gated by v7 REST API compatibility vs. breaking change vs. the expectations in the current state.

It now says if you send "accept_enterprise=false" for ANY REST API version, then raise an exception. I think this is OK and desired since it still consumes the parameter (avoiding an exception) and will continue emit a warning as it did in v8. However, since there are other parts that have been partially removed I am not sure what happens if you send "accept_enterprise=true". If that doesn't work or it errors, then maybe we are better off just removing fully removing support for accept_enterprise (which is a bigger change). If we leave it as-is (this PR), then I think should at least ensure no error and correct results for "accept_enterprise=true". Else, we should restore the relevant code from 8.x and bump REST API versions V_7 to V_8 ; and V_8 to V_9 so it behave identically to 8.x.

I added back all the original code that we removed via different PRs here: 32682ba , then adjusted the functionality per REST API version needs : a3336f2, simplified the code 2012ab2 ... and ended up right back where @mattc58 left off and I started 🪃 . The only real change since @mattc58's version was some minor clean up of dead code paths: dc697d4

jakelandis · 2025-03-12T18:19:34Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/License.java

@@ -497,8 +489,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
    public XContentBuilder toInnerXContent(XContentBuilder builder, Params params) throws IOException {
        boolean licenseSpecMode = params.paramAsBoolean(LICENSE_SPEC_VIEW_MODE, false);
        boolean restViewMode = params.paramAsBoolean(REST_VIEW_MODE, false);
-        boolean hideEnterprise = params.paramAsBoolean(XCONTENT_HIDE_ENTERPRISE, false);
-
        boolean previouslyHumanReadable = builder.humanReadable();


The only way this could have ever been true is via REST API compatibility with v7: https://github.com/elastic/elasticsearch/blob/8.x/x-pack/plugin/core/src/main/java/org/elasticsearch/license/RestGetLicenseAction.java#L66-L72 . 9.x does not support REST API compatibility with v7, so this value will always be false. (If you used v7 compatibility, you could set the accept_enterprise parameter to false which resulted in XCONTENT_HIDE_ENTERPRISE being true, now that v7 compatibility is gone there is no way to set accept_enterprise to false (via other validation) , so XCONTENT_HIDE_ENTERPRISE is always false) This change is adjusting the code for hideEnterprise is always false.

jakelandis · 2025-03-12T18:32:15Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/RestGetLicenseAction.java

        if (request.hasParam("accept_enterprise")) {
            deprecationLogger.warn(
                DeprecationCategory.API,
                "get_license_accept_enterprise",
                "Including [accept_enterprise] in get license requests is deprecated."
                    + " The parameter will be removed in the next major version"
            );
-            if (request.paramAsBoolean("accept_enterprise", true) == false) {
+            if (request.paramAsBoolean("accept_enterprise", true) == false) { // consumes the parameter to avoid error


no actual change here (just updated comment). However, it is worth noting that this is BWC to leave the code as-is.

8.x has this same behavior, and the gating by REST API version (removed in a different PR) was not necessary:
https://github.com/elastic/elasticsearch/blob/8.x/x-pack/plugin/core/src/main/java/org/elasticsearch/license/RestGetLicenseAction.java#L82-L84

Removing support for accept_enterprise = true is technically a breaking change, so it is not fully removed here. (accept_enterprise = false was only supported by v7 compatibility) )The behavior behind this only worked for v7 compatibility, so this flag does nothing. We read it here to keep from throwning exception for unkonwn parameter. Arguably we should have removed this v9, but given timing, other priorities, and low impact, this didn't happen. We should finally remove this for v10 and doesn't actually do anything in v9 (nor did it v8 unless you sent v7 compatibility).

jakelandis · 2025-03-12T18:38:45Z

x-pack/plugin/core/src/main/java/org/elasticsearch/license/License.java

@@ -142,7 +142,7 @@ static boolean isEnterprise(String typeName) {
     * XContent param name to map the "enterprise" license type to "platinum"
     * for backwards compatibility with older clients
     */
-    @UpdateForV9(owner = UpdateForV9.Owner.SECURITY) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7
+    @UpdateForV10(owner = UpdateForV10.Owner.SECURITY) // v7 REST API no longer exists: eliminate ref to RestApiVersion.V_7


I added back all the original code that we removed via different PRs here: 32682ba , then adjusted the functionality per REST API version needs : a3336f2, simplified the code 2012ab2 ... and ended up right back where @mattc58 left off and I started 🪃 . The only real change since @mattc58's version was some minor clean up of dead code paths: dc697d4

jakelandis

Paired with Matt on this... LGTM

elasticsearchmachine · 2025-03-12T22:33:53Z

💚 Backport successful

Status	Branch	Result
✅	9.0

Remove @UpdateForV9 annotation on code that has been deprecated and…

c18b3ab

… removed

mattc58 added >non-issue :Security/License License functionality for commercial features Team:Security Meta label for security team v9.0.0 labels Feb 21, 2025

mattc58 changed the title ~~Remove @UpdateForV9 annotations from Data Management code~~ Remove @UpdateForV9 annotations from Security code Feb 21, 2025

elasticsearchmachine added the v9.1.0 label Feb 21, 2025

elasticsearchmachine and others added 4 commits February 21, 2025 18:15

[CI] Auto commit changes from spotless

94d6062

Merge remote-tracking branch 'upstream/main' into remove-security-upd…

1f236f1

…ateforv9

Change UpdateForV9 to UpdateForV10 for License hide_enterprise

a402f2b

Merge remote-tracking branch 'origin/remove-security-updateforv9' int…

7edaedc

…o remove-security-updateforv9

mattc58 added the auto-backport Automatically create backport pull requests when merged label Feb 24, 2025

mattc58 marked this pull request as ready for review February 24, 2025 23:56

mattc58 requested a review from jakelandis February 24, 2025 23:56

mattc58 added the :Security/Security Security issues without another label label Feb 24, 2025

jakelandis requested changes Feb 25, 2025

View reviewed changes

jakelandis added 2 commits March 11, 2025 16:37

Merge branch 'main' into remove-security-updateforv9

e206c2a

kick can down the road for accept_enterprise

32682ba

jakelandis requested a review from a team as a code owner March 11, 2025 22:37

jakelandis added 4 commits March 11, 2025 18:06

adjust rest api compat for accept_enterprise

f3b7c6d

adjust to no longer rely on rest api compatiblity

a3336f2

full circle back to where I started

2012ab2

remove unecessary code

dc697d4

jakelandis reviewed Mar 12, 2025

View reviewed changes

Merge branch 'main' into remove-security-updateforv9

e756b91

jakelandis approved these changes Mar 12, 2025

View reviewed changes

mattc58 merged commit b9ec8fd into elastic:main Mar 12, 2025
22 checks passed

mattc58 deleted the remove-security-updateforv9 branch March 12, 2025 22:32

mattc58 mentioned this pull request Mar 12, 2025

[9.0] Remove @UpdateForV9 annotations from Security code (#123176) #124686

Merged

mattc58 added a commit to mattc58/elasticsearch that referenced this pull request Mar 12, 2025

Remove @UpdateForV9 annotations from Security code (elastic#123176)

3487ad0

elasticsearchmachine pushed a commit that referenced this pull request Mar 12, 2025

Remove @UpdateForV9 annotations from Security code (#123176) (#124686)

1a746ce

albertzaharovits pushed a commit to albertzaharovits/elasticsearch that referenced this pull request Mar 13, 2025

Remove @UpdateForV9 annotations from Security code (elastic#123176)

df1b190

jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Mar 13, 2025

Remove @UpdateForV9 annotations from Security code (elastic#123176)

3bc7c8e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove @UpdateForV9 annotations from Security code #123176

Remove @UpdateForV9 annotations from Security code #123176

Uh oh!

mattc58 commented Feb 21, 2025

Uh oh!

elasticsearchmachine commented Feb 24, 2025

Uh oh!

jakelandis left a comment

Uh oh!

jakelandis Feb 25, 2025

Uh oh!

jakelandis Mar 12, 2025

Uh oh!

jakelandis Mar 12, 2025

Uh oh!

jakelandis Mar 12, 2025

Uh oh!

jakelandis Mar 12, 2025

Uh oh!

jakelandis left a comment

Uh oh!

Uh oh!

elasticsearchmachine commented Mar 12, 2025

Uh oh!

Uh oh!

Remove @UpdateForV9 annotations from Security code #123176

Remove @UpdateForV9 annotations from Security code #123176

Uh oh!

Conversation

mattc58 commented Feb 21, 2025

Uh oh!

elasticsearchmachine commented Feb 24, 2025

Uh oh!

jakelandis left a comment

Choose a reason for hiding this comment

Uh oh!

jakelandis Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

jakelandis Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

jakelandis Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

jakelandis Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

jakelandis Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

jakelandis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Mar 12, 2025

💚 Backport successful

Uh oh!

Uh oh!