-
Notifications
You must be signed in to change notification settings - Fork 92
Refactor inventory task base types #394
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
Merged
kyasbal
merged 15 commits into
GoogleCloudPlatform:epic/issue-373
from
kyasbal:epic/issue-373-refactor-relationship-task
Dec 3, 2025
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
41af34d
Added new fieldset related tasks and history modifiers for error audi…
kyasbal ab9eff9
Adding k8s audit log parser tasks
kyasbal 7d457ce
fix issues pointed by gemini-code-assist
kyasbal 3cacc03
fix issues pointed by gemini-code-assist
kyasbal 67aa3e3
Adding grouping related tasks and tasks for gathering k8s audit logs …
kyasbal fe2d11b
Adding k8s audit log parser tasks
kyasbal 0e1be7c
fix issues pointed by gemini-code-assist
kyasbal ad006a7
Adding k8s audit log parser tasks
kyasbal e0f644a
fix issues pointed by gemini-code-assist
kyasbal 55f61e9
Added several test asserter for changeset testing
kyasbal 9fcb549
fix issues pointed by gemini-code-assist
kyasbal fdd8185
fix issues pointed by gemini-code-assist
kyasbal 5aa4148
Refactored relationship task and now it's named as InventoryTask
kyasbal c77ba6c
Update pkg/core/inspection/taskbase/inventory_task.go
kyasbal 29c13ce
fix issue pointed by gemini-code-assist
kyasbal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| // Copyright 2025 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| // Inventory related tasks defined in this file provides a framework for discovering and merging inventory data from various sources. | ||
| // | ||
| // In many inspection scenarios, it's necessary to associate information across different log sources. | ||
| // For example, a log might contain an IP address, while another log maps that IP to a specific VM or container name. | ||
| // However, the availability of these log sources is not always guaranteed, and consumers of this inventory | ||
| // data should not need to be aware of the specific tasks that provide it. | ||
| // | ||
| // This framework introduces two main components to address this: | ||
| // | ||
| // 1. DiscoveryTask: A task responsible for extracting a inventory map from a single data source. | ||
| // Providers of a discovery task must ensure it is added to the task graph when a task that may require its | ||
| // data is included. This is achieved by using the coretask.NewSubsequentTaskRefsTaskLabel, which links the | ||
| // discovery task to the merger task. | ||
| // | ||
| // 2. InventoryTask: A task that aggregates the results from all relevant DiscoveryTasks. | ||
| // Consumers can simply depend on this single merger task to access the complete, consolidated inventory map | ||
| // without needing to know about the individual discovery tasks. | ||
| // | ||
| // This approach decouples data consumers from data providers, allowing for a flexible and extensible inspection system. | ||
| package inspectiontaskbase | ||
|
|
||
| import ( | ||
| "context" | ||
| "log/slog" | ||
| "sync" | ||
|
|
||
| inspectionmetadata "github.com/GoogleCloudPlatform/khi/pkg/core/inspection/metadata" | ||
| coretask "github.com/GoogleCloudPlatform/khi/pkg/core/task" | ||
| "github.com/GoogleCloudPlatform/khi/pkg/core/task/taskid" | ||
| inspectioncore_contract "github.com/GoogleCloudPlatform/khi/pkg/task/inspection/inspectioncore/contract" | ||
| ) | ||
|
|
||
| // InventoryTaskBuilder builds a inventory task and discovery tasks. | ||
| // Inventory task merges information found in logs from multiple discovery tasks. | ||
| type InventoryTaskBuilder[T any] struct { | ||
| mu sync.Mutex | ||
| inventoryTaskID taskid.TaskImplementationID[T] | ||
| discoveryTaskRefs []taskid.TaskReference[T] | ||
| } | ||
|
|
||
| func NewInventoryTaskBuilder[T any](inventoryTaskID taskid.TaskImplementationID[T]) *InventoryTaskBuilder[T] { | ||
| return &InventoryTaskBuilder[T]{ | ||
| inventoryTaskID: inventoryTaskID, | ||
| } | ||
| } | ||
|
|
||
| func (s *InventoryTaskBuilder[T]) InventoryTask(strategy InventoryMergerStrategy[T]) coretask.Task[T] { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
| return NewInspectionTask(s.inventoryTaskID, []taskid.UntypedTaskReference{}, func(ctx context.Context, taskMode inspectioncore_contract.InspectionTaskModeType) (T, error) { | ||
| if taskMode == inspectioncore_contract.TaskModeDryRun { | ||
| return *new(T), nil | ||
| } | ||
| discoveryResults := make([]T, 0, len(s.discoveryTaskRefs)) | ||
| for _, ref := range s.discoveryTaskRefs { | ||
| r, found := coretask.GetTaskResultOptional(ctx, ref) | ||
| if found { | ||
| discoveryResults = append(discoveryResults, r) | ||
| } else { | ||
| slog.DebugContext(ctx, "discovery result not provided", "taskRef", ref.ReferenceIDString()) | ||
| } | ||
| } | ||
| return strategy.Merge(discoveryResults) | ||
| }) | ||
| } | ||
|
|
||
| func (s *InventoryTaskBuilder[T]) DiscoveryTask(taskID taskid.TaskImplementationID[T], dependencies []taskid.UntypedTaskReference, taskFunc ProgressReportableInspectionTaskFunc[T], labelOpts ...coretask.LabelOpt) coretask.Task[T] { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
| inventoryTaskID := s.inventoryTaskID.Ref() | ||
| labelOpts = append(labelOpts, coretask.NewSubsequentTaskRefsTaskLabel(inventoryTaskID)) | ||
|
|
||
| found := false | ||
| for _, ref := range s.discoveryTaskRefs { | ||
| if ref.ReferenceIDString() == taskID.ReferenceIDString() { | ||
| found = true | ||
| break | ||
| } | ||
| } | ||
| if !found { | ||
| s.discoveryTaskRefs = append(s.discoveryTaskRefs, taskID.Ref()) | ||
| } | ||
|
|
||
| return NewProgressReportableInspectionTask(taskID, dependencies, func(ctx context.Context, taskMode inspectioncore_contract.InspectionTaskModeType, progress *inspectionmetadata.TaskProgressMetadata) (T, error) { | ||
| if taskMode == inspectioncore_contract.TaskModeDryRun { | ||
| return *new(T), nil | ||
| } | ||
| return taskFunc(ctx, taskMode, progress) | ||
| }, labelOpts...) | ||
| } | ||
|
|
||
| // InventoryMergerStrategy defines the strategy how the generated InventoryMergerTasks merges results received from multiple discovery tasks. | ||
| type InventoryMergerStrategy[T any] interface { | ||
|
|
||
| // Merge defines the logic to combine multiple results from various discovery tasks | ||
| // into a single, consolidated result. | ||
| Merge(results []T) (T, error) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The deduplication logic for discovery tasks in the
DiscoveryTaskmethod can lead to incorrect behavior by silently ignoring some tasks. The checkref.ReferenceIDString() == taskID.ReferenceIDString()prevents registration of multiple discovery tasks that share the same base reference ID, even if they have different implementation hashes (e.g.,my-discovery#impl1andmy-discovery#impl2). As a result, theInventoryTaskwill only be aware of the first registered task and won't aggregate results from all intended discovery tasks.This contradicts the goal of aggregating results from all relevant discovery tasks. A possible fix would involve tracking each unique task implementation, but this may be constrained by the current implementation of
coretask.GetTaskResultOptional.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.
This is fine because a task graph won't include multiple implementations of a task after the resolution.