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

[Rules] Remove the UUID requirement for using a pre-defined rule.id #182594

Closed
simianhacker opened this issue May 3, 2024 · 9 comments · Fixed by #199119
Closed

[Rules] Remove the UUID requirement for using a pre-defined rule.id #182594

simianhacker opened this issue May 3, 2024 · 9 comments · Fixed by #199119
Assignees
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@simianhacker
Copy link
Member

Users need a way to create a consistent ID across resources that are human readable. Imagine a user is creating an SLO with a custom id admin-console-availability-sla and they want to check if a rule exists using the same ID for the rule; if it doesn't exists, create it.

In order to do that today, they would need to use the same UUID for both the SLO and the Rule. This is less than ideal because they end up with a bunch of UUIDs which requires a lot of cognitive overhead when looking at a logs or a large list of these resources; it requires an extra step of looking up which UUIDs belong to which SLOs.

The work around right now is customers will search all the rules to try and find the corresponding burn rate rule, the system doesn't limit the number of burn rate rules for an SLO and if that search returns multiple rules for the same SLO (which a user might have created by accident) they struggle to determine which is the "right one".

@simianhacker simianhacker added Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels May 3, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@pmuellr
Copy link
Member

pmuellr commented May 6, 2024

Update: woops! Missed that used rule and the other rules so they don't actually overap

I check the list a little closer - I don't think we have a problem with any of them.

There are unfortunately some overlaps in our URLs that we'll have to fix to support this. A list of all the rule URLs and methods is below.

For example, the following two are problematic:

  • GET /internal/alerting/rule/{id}
  • GET /internal/alerting/rules/_find

If a rule uses _find for the id, then I suspect the first URL will not be able to be used against it.

note: router has a super-handy getRoutes() method which returns all the routes registered, I just dumped via console.log() in x-pack/plugins/alerting/server/routes/index.ts.

alerting URLs and methods
/api/alerting/_health                                                 get    
/api/alerting/rule/{id?}                                              post   
/api/alerting/rule/{id}                                               delete 
/api/alerting/rule/{id}                                               get    
/api/alerting/rule/{id}                                               put    
/api/alerting/rule/{id}/_disable                                      post   
/api/alerting/rule/{id}/_enable                                       post   
/api/alerting/rule/{id}/_mute_all                                     post   
/api/alerting/rule/{id}/_unmute_all                                   post   
/api/alerting/rule/{id}/_update_api_key                               post   
/api/alerting/rule/{id}/alert/{alert_id}/_mute                        post   
/api/alerting/rule/{id}/alert/{alert_id}/_unmute                      post   
/api/alerting/rule_types                                              get    
/api/alerting/rules/_find                                             get    
/api/alerts/_find                                                     get    
/api/alerts/_health                                                   get    
/api/alerts/alert/{alert_id}/alert_instance/{alert_instance_id}/_mute post   
/api/alerts/alert/{alertId}/alert_instance/{alertInstanceId}/_unmute  post   
/api/alerts/alert/{id?}                                               post   
/api/alerts/alert/{id}                                                delete 
/api/alerts/alert/{id}                                                get    
/api/alerts/alert/{id}                                                put    
/api/alerts/alert/{id}/_disable                                       post   
/api/alerts/alert/{id}/_enable                                        post   
/api/alerts/alert/{id}/_instance_summary                              get    
/api/alerts/alert/{id}/_mute_all                                      post   
/api/alerts/alert/{id}/_unmute_all                                    post   
/api/alerts/alert/{id}/_update_api_key                                post   
/api/alerts/alert/{id}/state                                          get    
/api/alerts/list_alert_types                                          get    
/internal/alerting/_global_execution_kpi                              get    
/internal/alerting/_global_execution_logs                             get    
/internal/alerting/alerts/_bulk_untrack                               post   
/internal/alerting/alerts/_bulk_untrack_by_query                      post   
/internal/alerting/rule/{id}                                          get    
/internal/alerting/rule/{id}/_action_error_log                        get    
/internal/alerting/rule/{id}/_alert_summary                           get    
/internal/alerting/rule/{id}/_clone/{newId?}                          post   
/internal/alerting/rule/{id}/_execution_kpi                           get    
/internal/alerting/rule/{id}/_execution_log                           get    
/internal/alerting/rule/{id}/_resolve                                 get    
/internal/alerting/rule/{id}/_run_soon                                post   
/internal/alerting/rule/{id}/_snooze                                  post   
/internal/alerting/rule/{id}/_unsnooze                                post   
/internal/alerting/rule/{id}/state                                    get    
/internal/alerting/rules/_aggregate                                   post   
/internal/alerting/rules/_bulk_delete                                 patch  
/internal/alerting/rules/_bulk_disable                                patch  
/internal/alerting/rules/_bulk_edit                                   post   
/internal/alerting/rules/_bulk_enable                                 patch  
/internal/alerting/rules/_find                                        get    
/internal/alerting/rules/_find                                        post   
/internal/alerting/rules/_schedule_frequency                          get    
/internal/alerting/rules/_tags                                        get    
/internal/alerting/rules/backfill/_find                               post   
/internal/alerting/rules/backfill/_schedule                           post   
/internal/alerting/rules/backfill/{id}                                delete 
/internal/alerting/rules/backfill/{id}                                get    
/internal/alerting/rules/maintenance_window                           post   
/internal/alerting/rules/maintenance_window/_active                   get    
/internal/alerting/rules/maintenance_window/_bulk_get                 post   
/internal/alerting/rules/maintenance_window/_find                     get    
/internal/alerting/rules/maintenance_window/{id}                      delete 
/internal/alerting/rules/maintenance_window/{id}                      get    
/internal/alerting/rules/maintenance_window/{id}                      post   
/internal/alerting/rules/maintenance_window/{id}/_archive             post   
/internal/alerting/rules/maintenance_window/{id}/_finish              post   
/internal/alerting/rules/settings/_flapping                           get    
/internal/alerting/rules/settings/_flapping                           post   
/internal/alerting/rules/settings/_query_delay                        get    
/internal/alerting/rules/settings/_query_delay                        post   
/internal/alerts/suggestions/values                                   post   
/internal/rules/saved_objects/fields                                  post   
/internal/rules/suggestions/values                                    post   

@cnasikas
Copy link
Member

cnasikas commented Jun 19, 2024

@simianhacker As @pmuellr said, the create rule API supports creating rules with a specified ID. Unfortunately the ID has to be a UUID. The reason is that the rule SO has encrypted values (the API key of the user that created or updated the rule) and SOs with encrypted values must have IDs as UUID because the IDs are part of the AAD used during encryption and they should be hard to guess. For example, making the following API call will result to a 400 error.

API call

POST https://localhost:5601/api/alerting/rule/my-rule-id <--- my rule ID
{
  "params": {
    "searchType": "esQuery",
    "timeWindowSize": 5,
    "timeWindowUnit": "m",
    "threshold": [
      0
    ],
    "thresholdComparator": ">",
    "size": 100,
    "esQuery": "{\n    \"query\":{\n      \"match_all\" : {}\n    }\n  }",
    "aggType": "count",
    "groupBy": "all",
    "termSize": 5,
    "excludeHitsFromPreviousRun": false,
    "sourceFields": [],
    "index": [
      ".kibana"
    ],
    "timeField": "created_at"
  },
  "consumer": "stackAlerts",
  "schedule": {
    "interval": "1m"
  },
  "tags": [],
  "name": "test",
  "rule_type_id": ".es-query",
  "actions": []
}

Response

{
    "statusCode": 400,
    "error": "Bad Request",
    "message": "Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID.: Bad Request"
}

@elastic/kibana-core @elastic/kibana-security Any ideas or alternatives?

@pgayvallet
Copy link
Contributor

Any ideas or alternatives?

I don't think I really do. As you already mentionned, it's possible to use custom ids for SOs, but ESOs require their IDs to be valid uuids, so custom ids for ESOs needs to be UUIDs.

But it's the 30 years old "technical ID vs functional ID" discussion if you ask me. If you think a feature would benefit from more "user friendly functional IDs", then such field can just be added to any SO type's attributes, and the type owner can then change their APIs to create/search/retrieve/update/anything based on a functional ID rather than the technical one (and for ESO, the type owner could decide to have their functional ID attribute unencrypted to even make this easier?)

@cnasikas
Copy link
Member

Thank you @pgayvallet for the input! The functional ID is a great idea and seems the only way to move forward with this request.

@azasypkin
Copy link
Member

Sorry for the late reply, but I also agree that the "function ID" is something worth exploring despite the limitations imposed by the ESO plugin and might provide more flexibility in the long run. That being said, if it turns out that this approach isn't viable or significantly worsens DX/UX in your specific case, we can consider relaxing the ESO UUID-ID requirement on a type-by-type basis.

@kobelb
Copy link
Contributor

kobelb commented Jul 1, 2024

But it's the 30 years old "technical ID vs functional ID" discussion if you ask me. If you think a feature would benefit from more "user friendly functional IDs", then such field can just be added to any SO type's attributes, and the type owner can then change their APIs to create/search/retrieve/update/anything based on a functional ID rather than the technical one (and for ESO, the type owner could decide to have their functional ID attribute unencrypted to even make this easier?)

If there was a way to guarantee uniqueness of both a functional ID and a technical ID this would be a viable option. However, it sadly is not. We've primarily received this feedback from customers who are using tools like terraform to provision their rules as code, and as such, they need the user friendly ID to be unique. Therefore, I think the only reasonable way forward is to relax the ESO UUID-ID requirement on a type-by-type basis.

@pgayvallet
Copy link
Contributor

they need the user friendly ID to be unique

Got it. Then yeah, I agree, relaxing the constraint on ESO UUID seems like the only viable option then.

(and it's very hard for me, but I think I will manage not trolling with things like transaction support for ES, unique constraints support for ES, proper service layer enforcing unique functional IDs without those technical features and similar concepts. Yeah, I will totally finish my comment without mentioning any of those things. Promise. Here, I did it)

@cnasikas
Copy link
Member

cnasikas commented Oct 2, 2024

Blocked by #194692.

@cnasikas cnasikas self-assigned this Nov 6, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Nov 7, 2024
…random IDs (elastic#199119)

## Summary

This PR allows users to create rules with predefined nonrandom IDs. To
test it, try to create a rule with a predefined ID like `POST
/api/alerting/rule/<my_rule_id>`

Fixes: elastic#182594

### Checklist

Delete any items that are not applicable to this PR.

- [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

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)

(cherry picked from commit 669761b)
mgadewoll pushed a commit to mgadewoll/kibana that referenced this issue Nov 7, 2024
…random IDs (elastic#199119)

## Summary

This PR allows users to create rules with predefined nonrandom IDs. To
test it, try to create a rule with a predefined ID like `POST
/api/alerting/rule/<my_rule_id>`

Fixes: elastic#182594

### Checklist

Delete any items that are not applicable to this PR.

- [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

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants