-
Notifications
You must be signed in to change notification settings - Fork 42
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
Enrich GCP asset types with multiple values #3028
base: main
Are you sure you want to change the base?
Conversation
This pull request does not have a backport label. Could you fix it @orouz? 🙏
|
internal/inventory/asset.go
Outdated
Related *Related | ||
} | ||
|
||
type Related struct { | ||
Id []string `json:"id,omitempty"` |
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.
in order to test WithRelatedAssetIds
i needed an exported field, so i replaced Entity.relatedEntityId
with Related.Entity
ids = lo.Compact(ids) | ||
ids = lo.Uniq(ids) | ||
return ids | ||
} | ||
|
||
func findRelatedAssetIdsForType(t inventory.AssetType, item *gcpinventory.ExtendedGcpAsset) []string { | ||
func findRelatedAssetIdsForType(item *gcpinventory.ExtendedGcpAsset) []string { |
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.
changes in this function aren't required for this PR, but since i introduced values
function to extract values from a protobuf map, using it here simplifies the code and makes it more readable and consistent with the rest of the file.
|
||
case gcpinventory.ComputeFirewallAssetType, gcpinventory.ComputeSubnetworkAssetType: | ||
ids = append(ids, values([]string{"network"}, pb)...) | ||
case gcpinventory.CrmProjectAssetType, gcpinventory.StorageBucketAssetType: |
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.
i'm not sure why we're adding binding.Role
and binding.Members
just for projects and buckets asset types.
unrelated to this PR tho
if value, ok := f[key]; ok { | ||
return value.GetStringValue() | ||
// returns string values of keys in a map/array | ||
func values(keys []string, current any) []string { |
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.
extracting values from a protobuf is kind of a pain. (GetStructValue().GetFields()
, GetListValue().GetValues()
)
this function simplifies extraction by enabling map/array traversal with provided keys and defaults to returning an empty slice.
for example, this is how we collect values for a container name:
containerNames: = values([]string{"spec", "template", "spec", "containers", "name"}, pb)
instead of:
var containerNames []string
specField, ok := s.Fields["spec"]
if !ok {
return nil
}
templateField, ok := specField.GetStructValue().Fields["template"]
if !ok {
return nil
}
specTemplateField, ok := templateField.GetStructValue().Fields["spec"]
if !ok {
return nil
}
containersField, ok := specTemplateField.GetStructValue().Fields["containers"]
if !ok {
return nil
}
containersList, ok := containersField.GetListValue().GetValues()
if !ok {
return nil
}
for _, container := range containersList {
if nameField, ok := container.GetStructValue().Fields["name"]; ok {
containerNames = append(containerNames, nameField.GetStringValue())
}
}
return containerNames
c5e0432
to
edcb6aa
Compare
Summary of your changes
keyword
fromstring
to[]string
with this PR, all asset types required for GA are fetched and enriched plus additional enrichments we may want to include next would be easy to collect.
Screenshot/Data
Related Issues