turbot / steampipe-plugin-turbot

Use SQL to instantly query the Turbot CMDB. Open source CLI. No DB required.
https://hub.steampipe.io/plugins/turbot/turbot
Apache License 2.0
8 stars 1 forks source link

Use of `data` for resource notifications should be `object` #48

Open Joeturbot opened 2 years ago

Joeturbot commented 2 years ago

Describe the bug When querying {resource{data}}, Turbot will always return the most recent resource data state. When making queries in turbot_resource, this is appropriate behavior. However, for notifications, we want the previous resource state. Using {resource{object}} is the correct query to make.

Steampipe version (steampipe -v) ❯ steampipe -v steampipe version 0.14.6

Plugin version (steampipe plugin list) | hub.steampipe.io/plugins/turbot/turbot@latest | 0.5.0 |

To reproduce Steps to reproduce the behavior (please include relevant code and/or commands).

  1. Pick any cloud resource with resource_created and resource_updated notifications
  2. Replace 259791392905645 in the below query with the resource ID from step 1.
    select id, process_id, notification_type, create_timestamp, resource_title, resource_new_version_id, resource_old_version_id, resource_type_id, resource_type_uri, resource_data 
    from turbot_notification 
    where filter = 'notificationType:resource resourceId:"259791392905645"'
  3. Compare the resource_data column as the resource changes.

Expected behavior The results from the query in step 2, should reflect the resource's change over time.

Additional context The query needs to change on line 164, line 309 and line 66

cbruno10 commented 2 years ago

Hey @Joeturbot , thanks for raising this request! Does the resource_data column in the turbot_notification table return no data, or does it return data, but incorrect data?

Since we already have the resource data as a column, assuming it returns data correctly from the GraphQL query (even if it's not optimal), it seems like adding a new column (resource_object?) would be a way to add the functionality you're looking for. If this approach would meet your requirements, as you've already pointed out some of the places we'd need to modify, would you mind raising a PR with the GraphQL query changes and column addition?

If you need any help or have questions, please let us know. Thanks!

Joeturbot commented 2 years ago

@cbruno10 the resource_data column does return data, it's just wrong; it always returns the current state of the resource.

I'm working on a PR now. I just swapped out all instances of $.*.resource.data to be $.*.resource.object while keeping the column name as resource_data. Implicitly, if someone is interested in the current state of the resource, they will use the turbot_resource table. Use turbot_notification when you want history, which resource.object gets.

If I have questions, I'll raise them in Steampipe Slack.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] commented 2 years ago

'This issue was closed because it has been stalled for 60 days with no activity.'

Joeturbot commented 1 year ago

Closed by: https://github.com/turbot/steampipe-plugin-turbot/pull/65