Add support for GCP scopes #1296
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,5 @@ | ||
| package config | ||
|
|
||
| type GCPTerraformConfig struct { | ||
| Scopes []string `cty:"scopes"` | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,6 @@ | ||
| package google | ||
|
|
||
| import ( | ||
| "github.com/snyk/driftctl/pkg/output" | ||
| "github.com/snyk/driftctl/pkg/remote/google/config" | ||
| "github.com/snyk/driftctl/pkg/remote/terraform" | ||
|
|
@@ -34,7 +32,7 @@ func NewGCPTerraformProvider(version string, progress output.Progress, configDir | |
| tfProvider, err := terraform.NewTerraformProvider(installer, terraform.TerraformProviderConfig{ | ||
| Name: p.name, | ||
| GetProviderConfig: func(alias string) interface{} { | ||
| return p.SetConfig(nil) | ||
| }, | ||
| }, progress) | ||
|
|
||
|
|
@@ -55,10 +53,8 @@ func (p *GCPTerraformProvider) Version() string { | |
| return p.version | ||
| } | ||
|
|
||
| func (p *GCPTerraformProvider) SetConfig(scopes []string) config.GCPTerraformConfig { | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment. Well there's also nothing to get as during automated tests we have to set the config (scope) |
||
| return config.GCPTerraformConfig{ | ||
|
Comment on lines
-61
to
-62
There was a problem hiding this comment. Why do you removed Region and Zone parameters ? There was a problem hiding this comment. Because they are not needed since we use scopes. |
||
| Scopes: scopes, | ||
|
There was a problem hiding this comment. I think you missed the point, scopes are not what we are expecting here in this struct https://registry.terraform.io/providers/hashicorp/google/latest/docs/guides/provider_reference#scopes I think this PR is more complicated than you think, it seems that it is not possible to initialize a We probably need to create another kind of detail fetcher that will lazy instantiate one terraform providers per project. We are doing that for S3 buckets, but it's a really tricky thing. Did you try to run this PR in deep mode ? I think this is broken There was a problem hiding this comment. The entire point I had in mind was to support multiple or boarder scopes for scanning. I did not run it in deep mode since it's not recommended, but for a simple mode targeting various projects or an entire GCP org it works really good for me: "summary": {
"total_resources": 400,
"total_changed": 0,
"total_unmanaged": 398,
"total_missing": 1,
"total_managed": 1
} |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,13 +2,16 @@ package repository | |
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
|
|
||
| asset "cloud.google.com/go/asset/apiv1" | ||
| "github.com/snyk/driftctl/pkg/remote/cache" | ||
| "github.com/snyk/driftctl/pkg/remote/google/config" | ||
| "google.golang.org/api/iterator" | ||
| assetpb "google.golang.org/genproto/googleapis/cloud/asset/v1" | ||
| "google.golang.org/grpc/codes" | ||
| "google.golang.org/grpc/status" | ||
| ) | ||
|
|
||
| // https://cloud.google.com/asset-inventory/docs/supported-asset-types#supported_resource_types | ||
|
|
@@ -75,102 +78,145 @@ func NewAssetRepository(client *asset.Client, config config.GCPTerraformConfig, | |
| } | ||
|
|
||
| func (s assetRepository) listAllResources(ty string) ([]*assetpb.Asset, error) { | ||
|
|
||
| filteredResults := []*assetpb.Asset{} | ||
| var errorString string | ||
| var errCode codes.Code | ||
|
|
||
| for _, scope := range s.config.Scopes { | ||
| cacheKey := fmt.Sprintf("listAllResources_%s", scope) | ||
| cachedResults := s.cache.GetAndLock(cacheKey) | ||
| defer s.cache.Unlock(cacheKey) | ||
|
|
||
| req := &assetpb.ListAssetsRequest{ | ||
| Parent: scope, | ||
|
There was a problem hiding this comment. I'm not sure that a folder is supported for Can you double check that ? There was a problem hiding this comment. Indeed. Ideally we should not use listAssets but only searchAssets as the main difference is that list takes a snapshot of how the assets were at a particular time, which could result in a diff from what it's actually present. There was a problem hiding this comment. Where did you get this info ? I asked this question here a couple of month ago and I didn't get any answer. There was a problem hiding this comment. From here there are a couple of hints about it: Also in the API it's mentioned the purpose is to list resources from a specific timeframe While the search is more appropriate for our intent: |
||
| ContentType: assetpb.ContentType_RESOURCE, | ||
| AssetTypes: []string{ | ||
| cloudFunctionsFunction, | ||
| bigtableInstanceAssetType, | ||
| bigtableTableAssetType, | ||
| sqlDatabaseInstanceAssetType, | ||
| computeGlobalAddressAssetType, | ||
| nodeGroupAssetType, | ||
| }, | ||
| } | ||
|
|
||
| var results []*assetpb.Asset | ||
|
|
||
| if cachedResults != nil { | ||
| results = cachedResults.([]*assetpb.Asset) | ||
| } | ||
|
|
||
| if results == nil { | ||
| it := s.client.ListAssets(context.Background(), req) | ||
| for { | ||
| resource, err := it.Next() | ||
| if err == iterator.Done { | ||
| break | ||
| } | ||
| if err != nil && resource != nil { | ||
| errorString = errorString + fmt.Sprintf("For scope %s on resource %s got error: %s; ", scope, resource.AssetType, err.Error()) | ||
| continue | ||
| } | ||
| if err != nil && resource == nil { | ||
| errorString = errorString + fmt.Sprintf("For scope %s got error: %s; ", scope, err.Error()) | ||
| errCode = status.Code(err) | ||
| break | ||
| } | ||
| results = append(results, resource) | ||
| } | ||
| s.cache.Put(cacheKey, results) | ||
| } | ||
|
|
||
| for _, result := range results { | ||
| if result.AssetType == ty { | ||
| filteredResults = append(filteredResults, result) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if len(errorString) > 0 && len(filteredResults) > 0 { | ||
| return filteredResults, errors.New(errorString) | ||
| } | ||
|
|
||
| if len(errorString) > 0 && len(filteredResults) == 0 { | ||
| return nil, status.Error(errCode, errorString) | ||
| } | ||
|
|
||
| return filteredResults, nil | ||
| } | ||
|
|
||
| func (s assetRepository) searchAllResources(ty string) ([]*assetpb.ResourceSearchResult, error) { | ||
|
|
||
| filteredResults := []*assetpb.ResourceSearchResult{} | ||
| var errorString string | ||
| var errCode codes.Code | ||
|
|
||
| for _, scope := range s.config.Scopes { | ||
| cacheKey := fmt.Sprintf("SearchAllResources_%s", scope) | ||
| cachedResults := s.cache.GetAndLock(cacheKey) | ||
| defer s.cache.Unlock(cacheKey) | ||
|
|
||
| req := &assetpb.SearchAllResourcesRequest{ | ||
| Scope: scope, | ||
| AssetTypes: []string{ | ||
| storageBucketAssetType, | ||
| computeFirewallAssetType, | ||
| computeRouterAssetType, | ||
| computeInstanceAssetType, | ||
| computeNetworkAssetType, | ||
| computeSubnetworkAssetType, | ||
| dnsManagedZoneAssetType, | ||
| computeInstanceGroupAssetType, | ||
| bigqueryDatasetAssetType, | ||
| bigqueryTableAssetType, | ||
| computeAddressAssetType, | ||
| computeDiskAssetType, | ||
| computeImageAssetType, | ||
| healthCheckAssetType, | ||
| cloudRunServiceAssetType, | ||
| }, | ||
| } | ||
| var results []*assetpb.ResourceSearchResult | ||
|
|
||
| if cachedResults != nil { | ||
| results = cachedResults.([]*assetpb.ResourceSearchResult) | ||
| } | ||
|
|
||
| if results == nil { | ||
| it := s.client.SearchAllResources(context.Background(), req) | ||
| for { | ||
| resource, err := it.Next() | ||
| if err == iterator.Done { | ||
| break | ||
| } | ||
| if err != nil && resource != nil { | ||
| errorString = errorString + fmt.Sprintf("For scope %s on resource %s got error: %s; ", scope, resource.AssetType, err.Error()) | ||
| continue | ||
| } | ||
| if err != nil && resource == nil { | ||
| errorString = errorString + fmt.Sprintf("For scope %s got error: %s; ", scope, err.Error()) | ||
| errCode = status.Code(err) | ||
| break | ||
| } | ||
| results = append(results, resource) | ||
| } | ||
| s.cache.Put(cacheKey, results) | ||
| } | ||
|
|
||
| for _, result := range results { | ||
| if result.AssetType == ty { | ||
| filteredResults = append(filteredResults, result) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if len(errorString) > 0 && len(filteredResults) > 0 { | ||
| return filteredResults, errors.New(errorString) | ||
| } | ||
|
|
||
| if len(errorString) > 0 && len(filteredResults) == 0 { | ||
| return nil, status.Error(errCode, errorString) | ||
| } | ||
|
|
||
| return filteredResults, nil | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plea try to avoid else-if statements, there is not reason do use them here