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

Enrich GCP asset types with multiple values #3028

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

orouz
Copy link
Collaborator

@orouz orouz commented Feb 17, 2025

Summary of your changes

  1. modifies ECS types that accept a keyword from string to []string
  2. adds cloud run enricher
  3. adds utility function to traverse maps/arrays and collect leaf string values
  4. improves testing to account for all enrichers

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

Screenshot 2025-02-18 at 13 16 37

Related Issues

@mergify mergify bot assigned orouz Feb 17, 2025
Copy link

mergify bot commented Feb 17, 2025

This pull request does not have a backport label. Could you fix it @orouz? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.
    NOTE: backport-v8.x has been added to help with the transition to the new branch 8.x.

Comment on lines 161 to 165
Related *Related
}

type Related struct {
Id []string `json:"id,omitempty"`
Copy link
Collaborator Author

@orouz orouz Feb 18, 2025

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 {
Copy link
Collaborator Author

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:
Copy link
Collaborator Author

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 {
Copy link
Collaborator Author

@orouz orouz Feb 18, 2025

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

@orouz orouz force-pushed the gcp_asset_path_values branch from c5e0432 to edcb6aa Compare February 18, 2025 09:32
@orouz orouz marked this pull request as ready for review February 18, 2025 11:25
@orouz orouz requested a review from a team as a code owner February 18, 2025 11:25
@orouz orouz requested review from kubasobon and romulets February 18, 2025 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enrich asset types with multiple values
1 participant