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.

Sign up for GitHub

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

Jump to bottom

[Fleet] Rename force param to revoke for agent unenroll APIs #96873

Closed
jfsiii opened this issue Apr 12, 2021 · 4 comments
Closed

[Fleet] Rename force param to revoke for agent unenroll APIs #96873

jfsiii opened this issue Apr 12, 2021 · 4 comments
Assignees
@jfsiii
Labels
Team:Fleet Team label for Observability Data Collection Fleet team v7.13.0 v8.0.0

Comments

@jfsiii
Copy link
Contributor

jfsiii commented Apr 12, 2021 •
edited
Loading

Current behavior

The /agents/{agent_id}/unenroll & /agents/bulk_unenroll Fleet endpoints support a force parameter to immediately revoke the API keys for an agent.

Problem

The unenroll endpoints are the only ones to use force this way. All the other Fleet endpoints which have a force param use it as a signal to "attempt or execute regardless of permissions". Much like the --force flag in many CLI programs.

Proposed solution

Breaking API change or backwards-incompatible, at least

  1. Rename force to revoke (suggestions welcome. some others considered were: invalidate, revoke_tokens, invaldate_keys)
  2. Change force param to follow the convention of ignoring certain restrictions

In matrix form:

Unenroll AgentRevoke API Keys
RegularHosted
Rename force to revoke
Current force=false|undefined✅❌❌
Proposed revoke=false|undefined✅❌❌
Current force=true✅❌✅
Proposed revoke=true✅❌✅
Change force param
Proposed force=false|undefined✅❌❌
Proposed force=true✅✅❌
Proposed force=true & revoke=true✅✅✅

Changes required for consumers

Any call to /agents/{agent_id}/unenroll & /agents/bulk_unenroll which passes the force param should change to revoke to maintain the current behavior.

I'd prefer not to make a breaking change, but I think it's unavoidable if we want consistency with the rest of the Fleet API.

Impact on consumers who continue using force to mean "revoke tokens"

The agent will still be logged out, but it will not revoke the APIs.

Unenroll AgentRevoke API Keys
RegularHosted
Current force=true✅❌✅
Proposed force=true✅✅❌
@jfsiii jfsiii added v8.0.0 Team:Fleet Team label for Observability Data Collection Fleet team v7.13.0 labels Apr 12, 2021
@jfsiii jfsiii self-assigned this Apr 12, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jfsiii
Copy link
Contributor Author

jfsiii commented Apr 12, 2021

Pinging @mdelapenya since this would affect e2e tests. I'm happy to submit a PR. I found https://github.com/elastic/e2e-testing/blob/150652154d11f916976c969a0935917a741f207a/e2e/_suites/fleet/fleet.go and will look for any other instances

@jfsiii jfsiii mentioned this issue Apr 13, 2021
Merged
1 task
@jfsiii
Copy link
Contributor Author

jfsiii commented Apr 13, 2021

#97041 is a (draft) PR towards this.

fcbc9d9 has the changes to rename force to revoke

jfsiii pushed a commit that referenced this issue Apr 14, 2021
[Fleet] Rename force to revoke agent unenroll APIs (#97041)
de4bcdb 
## Summary

 - fcbc9d9 Rename `force` param to `revoke` for `/agents/{agent_id}/unenroll` & `/agents/bulk_unenroll`
 - 03b9b90 Add new `force` param

See #96873 for background

<table>
  <thead>
    <tr>
      <td rowspan="2"></td><td colspan="2">Unenroll Agent</td><td rowspan="2">Revoke API Keys</td>
    </tr>
    <tr>
      <td>Regular</td><td>Hosted</td></td>
    </tr>
  </thead>
  <tr><td colspan="4"><strong>Rename <code>force</code> to <code>revoke</code></strong></td></tr>
  <tr><td>Current <code>force=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Proposed <code>revoke=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Current <code>force=true</code></td><td>✅</td><td>❌</td><td>✅</td></tr>
  <tr><td>Proposed <code>revoke=true</code></td><td>✅</td><td>❌</td><td>✅</td></tr>
  <tr><td colspan="4"><strong>Change <code>force</code> param </strong></td></tr>
  <tr><td>Proposed <code>force=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Proposed <code>force=true</code></td><td>✅</td><td>✅</td><td>❌</td></tr>
  <tr><td>Proposed <code>force=true</code> & <code>revoke=true</code></td><td>✅</td><td>✅</td><td>✅</td></tr>
</table>

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

### Changes required for consumers
Any call to `/agents/{agent_id}/unenroll` & `/agents/bulk_unenroll` which passes the `force` param should change to `revoke` to maintain the current behavior.
@jfsiii
Copy link
Contributor Author

jfsiii commented Apr 14, 2021

Closed by #97041

@jfsiii jfsiii closed this as completed Apr 14, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Apr 14, 2021
@kibanamachine
[Fleet] Rename force to revoke agent unenroll APIs (elastic#97041)
7028d7e 
## Summary

 - fcbc9d9 Rename `force` param to `revoke` for `/agents/{agent_id}/unenroll` & `/agents/bulk_unenroll`
 - 03b9b90 Add new `force` param

See elastic#96873 for background

<table>
  <thead>
    <tr>
      <td rowspan="2"></td><td colspan="2">Unenroll Agent</td><td rowspan="2">Revoke API Keys</td>
    </tr>
    <tr>
      <td>Regular</td><td>Hosted</td></td>
    </tr>
  </thead>
  <tr><td colspan="4"><strong>Rename <code>force</code> to <code>revoke</code></strong></td></tr>
  <tr><td>Current <code>force=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Proposed <code>revoke=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Current <code>force=true</code></td><td>✅</td><td>❌</td><td>✅</td></tr>
  <tr><td>Proposed <code>revoke=true</code></td><td>✅</td><td>❌</td><td>✅</td></tr>
  <tr><td colspan="4"><strong>Change <code>force</code> param </strong></td></tr>
  <tr><td>Proposed <code>force=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Proposed <code>force=true</code></td><td>✅</td><td>✅</td><td>❌</td></tr>
  <tr><td>Proposed <code>force=true</code> & <code>revoke=true</code></td><td>✅</td><td>✅</td><td>✅</td></tr>
</table>

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

### Changes required for consumers
Any call to `/agents/{agent_id}/unenroll` & `/agents/bulk_unenroll` which passes the `force` param should change to `revoke` to maintain the current behavior.
kibanamachine added a commit that referenced this issue Apr 15, 2021
@kibanamachine
[Fleet] Rename force to revoke agent unenroll APIs (#97041) (#97196)
b338c69 
## Summary

 - fcbc9d9 Rename `force` param to `revoke` for `/agents/{agent_id}/unenroll` & `/agents/bulk_unenroll`
 - 03b9b90 Add new `force` param

See #96873 for background

<table>
  <thead>
    <tr>
      <td rowspan="2"></td><td colspan="2">Unenroll Agent</td><td rowspan="2">Revoke API Keys</td>
    </tr>
    <tr>
      <td>Regular</td><td>Hosted</td></td>
    </tr>
  </thead>
  <tr><td colspan="4"><strong>Rename <code>force</code> to <code>revoke</code></strong></td></tr>
  <tr><td>Current <code>force=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Proposed <code>revoke=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Current <code>force=true</code></td><td>✅</td><td>❌</td><td>✅</td></tr>
  <tr><td>Proposed <code>revoke=true</code></td><td>✅</td><td>❌</td><td>✅</td></tr>
  <tr><td colspan="4"><strong>Change <code>force</code> param </strong></td></tr>
  <tr><td>Proposed <code>force=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Proposed <code>force=true</code></td><td>✅</td><td>✅</td><td>❌</td></tr>
  <tr><td>Proposed <code>force=true</code> & <code>revoke=true</code></td><td>✅</td><td>✅</td><td>✅</td></tr>
</table>

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

### Changes required for consumers
Any call to `/agents/{agent_id}/unenroll` & `/agents/bulk_unenroll` which passes the `force` param should change to `revoke` to maintain the current behavior.

Co-authored-by: John Schulz <john.schulz@elastic.co>
madirey pushed a commit to madirey/kibana that referenced this issue May 11, 2021
@madirey
[Fleet] Rename force to revoke agent unenroll APIs (elastic#97041)
73071a0 
## Summary

 - fcbc9d9 Rename `force` param to `revoke` for `/agents/{agent_id}/unenroll` & `/agents/bulk_unenroll`
 - 03b9b90 Add new `force` param

See elastic#96873 for background

<table>
  <thead>
    <tr>
      <td rowspan="2"></td><td colspan="2">Unenroll Agent</td><td rowspan="2">Revoke API Keys</td>
    </tr>
    <tr>
      <td>Regular</td><td>Hosted</td></td>
    </tr>
  </thead>
  <tr><td colspan="4"><strong>Rename <code>force</code> to <code>revoke</code></strong></td></tr>
  <tr><td>Current <code>force=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Proposed <code>revoke=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Current <code>force=true</code></td><td>✅</td><td>❌</td><td>✅</td></tr>
  <tr><td>Proposed <code>revoke=true</code></td><td>✅</td><td>❌</td><td>✅</td></tr>
  <tr><td colspan="4"><strong>Change <code>force</code> param </strong></td></tr>
  <tr><td>Proposed <code>force=false|undefined</code></td><td>✅</td><td>❌</td><td>❌</td></tr>
  <tr><td>Proposed <code>force=true</code></td><td>✅</td><td>✅</td><td>❌</td></tr>
  <tr><td>Proposed <code>force=true</code> & <code>revoke=true</code></td><td>✅</td><td>✅</td><td>✅</td></tr>
</table>

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

### Changes required for consumers
Any call to `/agents/{agent_id}/unenroll` & `/agents/bulk_unenroll` which passes the `force` param should change to `revoke` to maintain the current behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jfsiii jfsiii

Labels
Team:Fleet Team label for Observability Data Collection Fleet team v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jfsiii @elasticmachine