Skip to content
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

Deprecate /_xpack/security/* in favor of /_security/* #36293

Merged
merged 22 commits into from
Dec 11, 2018

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented Dec 6, 2018

This commit is part of our plan to deprecate and ultimately
remove the use of _xpack in the REST APIs. It includes:

  • Adjusted REST api docs
  • Adjusted HLRC docs and doc tests
  • Adjusted REST actions with deprecation warnings
  • Changed endpoints in rest-api-spec and relevant file names

Relates #35958

…e use of _xpack in the REST APIs.

- REST api docs
- HLRC docs and doc tests
- Handle REST actions with deprecation warnings
- Changed endpoints in rest-api-spec and relevant file names

Relates elastic#35958
@jkakavas jkakavas added :Core/Infra/REST API REST infrastructure and utilities >deprecation v7.0.0 labels Dec 6, 2018
@jkakavas jkakavas requested a review from jasontedor December 6, 2018 08:50
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@@ -1,10 +1,12 @@
{
Copy link
Member Author

Choose a reason for hiding this comment

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

I've seen the rest of the PRs are not touching the file names for these, but it made sense to me. No problem to revert if there are objections though.

@jkakavas
Copy link
Member Author

jkakavas commented Dec 6, 2018

elasticsearch-ci-1 failed with an error in testIndexFollowing that doesn't reproduce locally

./gradlew :client:rest-high-level:integTestRunner -Dtests.seed=C6BDD3AC7F18F9C1 -Dtests.class=org.elasticsearch.client.CCRIT -Dtests.method="testIndexFollowing" -Dtests.security.manager=true -Dtests.locale=de -Dtests.timezone=Africa/Tunis -Dcompiler.java=11 -Druntime.java=8

giving it another go:
run the gradle build tests 1

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM assuming that CI goes green.

Adds a number of api calls that use the deprecated endpoints so
that these can be used in the `old_cluster` tests
@jkakavas
Copy link
Member Author

jkakavas commented Dec 7, 2018

@jasontedor pinging to see if you want to take a look at a509387 which adds back a number of api definitions in the deprecated format in order to use these against the old cluster in rolling upgrade tests. I couldn't think of a more elegant way to do this

@jasontedor
Copy link
Member

@jkakavas I have a different idea, let's backport the new endpoints to 6.x (without all the other changes to the docs, specification, etc.). What do you think of this approach?

@droberts195
Copy link
Contributor

The idea of backporting the new endpoints to 6.x would be consistent with what watcher is doing in #36269 (although in that case it already did support the new endpoints albeit deprecated!) and what ML is doing in #36373. I think backporting the new endpoints is also nice because it means customers have the option to avoid deprecation warnings in the mixed version cluster during a rolling upgrade from 6.last to 7.x by switching clients to the new endpoints before starting the rolling upgrade.

@jkakavas
Copy link
Member Author

jkakavas commented Dec 7, 2018

Thanks folk, that definitely makes sense. I'll backport the endpoints and then revert the changes here to ensure a green CI

@jasontedor
Copy link
Member

Thanks @jkakavas.

This reverts commit a509387.
New endpoints can be used for tests and mixed clusters since
elastic#36379
jkakavas added a commit that referenced this pull request Dec 9, 2018
#36293 deprecates the /_xpack/security/* endpoints in favor of
the /_security/* ones. This commit adds support for the new
endpoints in 6.x to facilitate tests and normal operations in a
mixed 6.x/7.x cluster
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@jkakavas
Copy link
Member Author

@jasontedor any objections to me re-introducing ff242e8 so that we don't backport _security endpoints to 6.5 too for Full Cluster restart tests ?

This reverts commit c902e68 and
thus reintroduces ff242e8.
@jasontedor
Copy link
Member

Yeah, for full cluster restart that is fine because we test against very old clusters where we can't backport anything so that we have to take the if/else route.

@jkakavas
Copy link
Member Author

@elasticmachine test this please

@jkakavas
Copy link
Member Author

@elasticmachine test this please

@jasontedor
Copy link
Member

@elasticmachine run gradle build tests 1 and run gradle build tests 2

@jkakavas
Copy link
Member Author

elasticserach-ci-tests-2 was failing because of a typo in #36379 that I corrected in 9bf0999, and elasticserach-ci-tests-1 needed additional changes for put role that was merged while this PR was fighting with CI.

🤞

@jkakavas jkakavas merged commit d7c5d80 into elastic:master Dec 11, 2018
@jkakavas jkakavas deleted the deprecate-security-xpack branch December 11, 2018 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants