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

[Snapshot Restore] Fix error when deleting snapshots behind reverse proxy #66147

Merged

Conversation

alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented May 12, 2020

Fixes #51166

It appears there was an encoding/decoding issue with the delete snapshots request when running through a reverse proxy (more details around nginx behavior here). I changed the request method from DELETE to POST to work around this.

How to test

To reproduce the bug, first checkout master and follow the reverse proxy setup instructions in this gist.

How to create a snapshot:

  1. [Prereq] Requires defining the path.repo ES setting, e.g., yarn es snapshot -E path.repo=/tmp/es_backups
  2. Set up a repository in Snapshot and Restore and then create a policy.
  3. Click the "play" icon next to the policy in the "Policies" table to execute the policy and create a new snapshot.

Navigate to the "Snapshots" tab and try to delete a snapshot. You should receive a 404 error.

With this change, the snapshot should now be deleted successfully.

Release note

This change fixes a bug in Snapshot and Restore where snapshots could not be deleted if a user was running Kibana through a reverse proxy.

@alisonelizabeth alisonelizabeth added release_note:fix v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI v7.8.0 v7.9.0 labels May 12, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@alisonelizabeth alisonelizabeth marked this pull request as ready for review May 12, 2020 12:55
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner May 12, 2020 12:55
@alisonelizabeth alisonelizabeth requested a review from cjcenizal May 12, 2020 12:55
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Great job @alisonelizabeth! I repro'ed and verified the fix. Code LGTM. I like how it became simpler as a result of this fix. Thanks for the detailed setup instructions in the gist. Can you walk me through how nginx's proxy pass rules were causing the bug in the first place?

@alisonelizabeth
Copy link
Contributor Author

alisonelizabeth commented May 13, 2020

Thanks @cjcenizal for the review!

Can you walk me through how nginx's proxy pass rules were causing the bug in the first place?

I’m by no means an nginx expert 😄… but from what I could understand decoding occurs before the route hits the kibana server if the proxy_pass in the nginx config is specified with a path or just has a trailing slash. So if you actually update the nginx.conf in my gist without the trailing slash, the problem doesn’t occur anymore.

From the nginx docs:

A request URI is passed to the server as follows:

If the proxy_pass directive is specified with a URI, then when a request is passed to the server, the part of a normalized request URI matching the location is replaced by a URI specified in the directive:
location /name/ {
proxy_pass http://127.0.0.1/remote/;
}
If proxy_pass is specified without a URI, the request URI is passed to the server in the same form as sent by a client when the original request is processed, or the full normalized request URI is passed when processing the changed URI:
location /some/path/ {
proxy_pass http://127.0.0.1;
}

@alisonelizabeth alisonelizabeth merged commit 4202850 into elastic:master May 13, 2020
@alisonelizabeth alisonelizabeth deleted the bugfix/sr/deleting_snapshot branch May 13, 2020 13:00
alisonelizabeth added a commit to alisonelizabeth/kibana that referenced this pull request May 13, 2020
v1v added a commit to v1v/kibana that referenced this pull request May 13, 2020
* upstream/master: (223 commits)
  [Ingest] Support root level yaml variables in agent stream template (elastic#66120)
  [Snapshot Restore] Fix error when deleting snapshots behind reverse proxy (elastic#66147)
  [Lens] fix empty state for pie (elastic#66206)
  [APM] Improve e2e tests (elastic#66373)
  [ML] Data Frame Analytics: Fix steps to be named phases. (elastic#65855)
  [Discover] Encode context link filter part (elastic#63831)
  [APM] Scope APM alert creation to environment (elastic#65681)
  Move Kibana Usage collectors outside the telemetry plugin (elastic#65663)
  [ML] Data Frame Analytics: Fix confusion matrix data grid width. (elastic#65818)
  Switch to core application service (elastic#63443)
  Removes use of prefer_v2_templates (elastic#66316)
  [Maps] handle case where fit to bounds does not match any documents (elastic#66307)
  log error instead of throw (elastic#66254)
  [plugin-helpers] remove outdated postinstall task (elastic#66324)
  Spaces - migrate default space and enter space view to KP (elastic#66098)
  [APM] Change plugin id for `apm_oss` to `apmOss` (elastic#66164)
  [Maps] convert map_selectors to TS (elastic#65905)
  [uptime/usage-collector] add missing await (elastic#66079)
  [Ingest] Add additional attributes to the Datasources Saved Object (elastic#66127)
  [Endpoint]EMT-339: add new policy response schema (elastic#66264)
  ...
@TinaHeiligers
Copy link
Contributor

@alisonelizabeth I'm working on another sdh that's reporting not being able to delete a snapshot from Kibana Management > Elastic > Snapshot and Restore but for v7.4.
Is there a chance that you could backport this fix to earlier versions of Kibana? v7.4 would be ideal but from the docs page, it looks like we're no longer releasing bug fixes for versions < 7.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.8.0 v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snapshots not getting deleted through kibana UI (7.4.2)
5 participants