turbot / steampipe-plugin-gcp

Use SQL to instantly query GCP resources across regions, projects and organizations. Open source CLI. No DB required.
https://hub.steampipe.io/plugins/turbot/gcp
Apache License 2.0
39 stars 24 forks source link

fixes GEN 2 cloudfunctions function not getting listed #568

Closed ashutoshmore658 closed 2 months ago

ashutoshmore658 commented 2 months ago

This PR fixes following bug issue: https://github.com/turbot/steampipe-plugin-gcp/issues/565

misraved commented 2 months ago

@ashutoshmore658 welcome to Steampipe and thank you so much for raising the PR 👍.

Could you please add the example queries in the PR removing the sensitive data?

ashutoshmore658 commented 2 months ago

Sure @misraved , thank you for replying. could you please elaborate ? exactly what are the example queries ?, so that I can get it in to the action.

misraved commented 2 months ago

@ashutoshmore658 please take a look at the following suggestions for the PR:

ashutoshmore658 commented 2 months ago

Thank you for the detailed explanation @misraved , I will act accordingly.

ashutoshmore658 commented 2 months ago

Hi @misraved ,

with building GCP Steampipe plugin from Gcp-CloudFunctions-Gen2-Flooding branch from this branches code and querying the vpc for cloud function names, in below image you can see all the cloud functions of both GEN 1 origin and GEN 2 origin are also being listed. following images are proofs for the same.

Branch: branchgen2

Results received: resultgen2

After removing the branch plugin from the Steampipes plugin directory and switching the branch of the GCP Steampipe back to main branch, rebuilding the plugin from scratch, and reconfiguiring the project, and after that querying the VPC listis only GEN 1 functions .

Results received: mainresult

This PR is adding the GEN 2 functions which were missing in previous version of gcp steampipe plugin and it is not breaking anything.

ashutoshmore658 commented 2 months ago

@misraved I am adding some more commits in the current PR, but how to show the results, cause security reasons are involved ?

What else is needed to merge this PR ?

misraved commented 2 months ago

@ashutoshmore658 what commits are you trying to add? I feel the results are sufficient for the PR to be merged 👍.

ashutoshmore658 commented 2 months ago

@misraved other columns values such as Https Trigress, Ingress Count etc which were missing, or for them should I raise another PR ? Another confusion is if the results are okay, when we are going to merge this PR

misraved commented 2 months ago

@ashutoshmore658 we can go ahead and raise a new PR for those changes. Meanwhile, let me merge this PR 👍.

Great work 🎉 !!

ashutoshmore658 commented 2 months ago

@misraved Thank you, It's been a pleasure collaborating with you.