diff --git a/pkg/task/inspection/googlecloudcommon/contract/legacy_query_task.go b/pkg/task/inspection/googlecloudcommon/contract/legacy_query_task.go index 31f5b65c2..2a52ade53 100644 --- a/pkg/task/inspection/googlecloudcommon/contract/legacy_query_task.go +++ b/pkg/task/inspection/googlecloudcommon/contract/legacy_query_task.go @@ -230,5 +230,5 @@ func NewLegacyCloudLoggingListLogTask(taskId taskid.TaskImplementationID[[]*log. } return []*log.Log{}, err - }, inspectioncore_contract.NewQueryTaskLabelOpt(logType, sampleQuery)) + }, inspectioncore_contract.NewQueryTaskLabelOpt(logType, sampleQuery), coretask.WithLabelValue(RequestOptionalInputResourceNameTaskLabel, taskId.ReferenceIDString())) } diff --git a/pkg/task/inspection/googlecloudcommon/contract/listlogentries_task.go b/pkg/task/inspection/googlecloudcommon/contract/listlogentries_task.go index 47e4321f5..f989fc96c 100644 --- a/pkg/task/inspection/googlecloudcommon/contract/listlogentries_task.go +++ b/pkg/task/inspection/googlecloudcommon/contract/listlogentries_task.go @@ -167,6 +167,7 @@ func NewListLogEntriesTask(taskSetting ListLogEntriesTaskSetting) coretask.Task[ return allLogs, nil }, inspectioncore_contract.NewQueryTaskLabelOpt(description.DefaultLogType, description.ExampleQuery), + coretask.WithLabelValue(RequestOptionalInputResourceNameTaskLabel, taskID.ReferenceIDString()), ) } @@ -200,7 +201,7 @@ func setQueryInfo(ctx context.Context, taskID, baseLogFilter string, logFilterIn } finalFilter := fmt.Sprintf("%s\n%s", baseLogFilter, gcpqueryutil.TimeRangeQuerySection(startTime, endTime, true)) if len(finalFilter) > 20000 { - slog.WarnContext(ctx, fmt.Sprintf("Logging filter is exceeding Cloud Logging limitation 20000 charactors\n%s", finalFilter)) + slog.WarnContext(ctx, fmt.Sprintf("Logging filter is exceeding Cloud Logging limitation 20000 characters\n%s", finalFilter)) } queryInfo.SetQuery(taskID, logFilterName, finalFilter) return nil diff --git a/pkg/task/inspection/googlecloudcommon/contract/resourcename.go b/pkg/task/inspection/googlecloudcommon/contract/resourcename.go index 2294832e0..9eb174731 100644 --- a/pkg/task/inspection/googlecloudcommon/contract/resourcename.go +++ b/pkg/task/inspection/googlecloudcommon/contract/resourcename.go @@ -73,19 +73,6 @@ func (r *ResourceNamesInput) GetResourceNamesForQuery(ctx context.Context, query return queryNames } -// GetQueryResourceNamePairs returns all query ID and resource name pairs. -func (r *ResourceNamesInput) GetQueryResourceNamePairs() []*QueryResourceNames { - queries := []*QueryResourceNames{} - for _, queryID := range r.resourceNames.Keys() { - resourceNames, found := typeddict.Get(r.resourceNames, queryID) - if !found { - continue - } - queries = append(queries, resourceNames) - } - return queries -} - func (r *ResourceNamesInput) ensureQueryID(queryID string) { _, found := typeddict.Get(r.resourceNames, queryID) if !found { diff --git a/pkg/task/inspection/googlecloudcommon/contract/tasklabel.go b/pkg/task/inspection/googlecloudcommon/contract/tasklabel.go new file mode 100644 index 000000000..350192b70 --- /dev/null +++ b/pkg/task/inspection/googlecloudcommon/contract/tasklabel.go @@ -0,0 +1,21 @@ +// 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. + +package googlecloudcommon_contract + +import "github.com/GoogleCloudPlatform/khi/pkg/common/typedmap" + +// RequestOptionalInputResourceNameTaskLabel is a label assigned to a task that requests the Cloud Logging resource name optionally. +// The value is the query ID. +var RequestOptionalInputResourceNameTaskLabel = typedmap.NewTypedKey[string]("request-optional-input-resource-name") diff --git a/pkg/task/inspection/googlecloudcommon/impl/inputloggingfilterresourcename_task.go b/pkg/task/inspection/googlecloudcommon/impl/inputloggingfilterresourcename_task.go index 3869de87d..f6ceefdd2 100644 --- a/pkg/task/inspection/googlecloudcommon/impl/inputloggingfilterresourcename_task.go +++ b/pkg/task/inspection/googlecloudcommon/impl/inputloggingfilterresourcename_task.go @@ -24,6 +24,7 @@ import ( "github.com/GoogleCloudPlatform/khi/pkg/common/typedmap" inspectionmetadata "github.com/GoogleCloudPlatform/khi/pkg/core/inspection/metadata" inspectiontaskbase "github.com/GoogleCloudPlatform/khi/pkg/core/inspection/taskbase" + coretask "github.com/GoogleCloudPlatform/khi/pkg/core/task" "github.com/GoogleCloudPlatform/khi/pkg/core/task/taskid" googlecloudcommon_contract "github.com/GoogleCloudPlatform/khi/pkg/task/inspection/googlecloudcommon/contract" inspectioncore_contract "github.com/GoogleCloudPlatform/khi/pkg/task/inspection/inspectioncore/contract" @@ -34,6 +35,10 @@ var resourceNamesInputKey = typedmap.NewTypedKey[*googlecloudcommon_contract.Res // InputLoggingFilterResourceNameTask defines an inspection task that creates a form group // for overriding log filter resource names for advanced users. var InputLoggingFilterResourceNameTask = inspectiontaskbase.NewInspectionTask(googlecloudcommon_contract.InputLoggingFilterResourceNameTaskID, []taskid.UntypedTaskReference{}, func(ctx context.Context, taskMode inspectioncore_contract.InspectionTaskModeType) (*googlecloudcommon_contract.ResourceNamesInput, error) { + // Tasks requiring active resource names can change, so we always retrieve current tasks that need resource names from the task graph. + taskRunner := khictx.MustGetValue(ctx, inspectioncore_contract.TaskRunner) + currentActiveResourceNameInputRequests := getCurrentActiveQueryIDsForResourceName(taskRunner) + // Since the default resource names registered by the tasks actually used are not known until those tasks are executed, we store them in sharedMap and have the actual tasks update them. sharedMap := khictx.MustGetValue(ctx, inspectioncore_contract.InspectionSharedMap) resourceNamesInput := typedmap.GetOrSetFunc(sharedMap, resourceNamesInputKey, googlecloudcommon_contract.NewResourceNamesInput) @@ -46,19 +51,20 @@ var InputLoggingFilterResourceNameTask = inspectiontaskbase.NewInspectionTask(go requestInput := khictx.MustGetValue(ctx, inspectioncore_contract.InspectionTaskInput) queryForms := []inspectionmetadata.ParameterFormField{} - for _, form := range resourceNamesInput.GetQueryResourceNamePairs() { - defaultValue := strings.Join(form.DefaultResourceNames, " ") + for _, request := range currentActiveResourceNameInputRequests { + queryInfo := resourceNamesInput.GetResourceNamesForQuery(ctx, request) + defaultValue := strings.Join(queryInfo.DefaultResourceNames, " ") formFieldBase := inspectionmetadata.ParameterFormFieldBase{ Priority: 0, - ID: form.GetInputID(), + ID: queryInfo.GetInputID(), Type: inspectionmetadata.Text, - Label: form.QueryID, + Label: queryInfo.QueryID, Description: "", HintType: inspectionmetadata.None, Hint: "", } // This task validates the inputs only. - formInput, found := requestInput[form.GetInputID()] + formInput, found := requestInput[queryInfo.GetInputID()] if found { resourceNamesFromInput := strings.Split(formInput.(string), " ") for i, resourceNameFromInput := range resourceNamesFromInput { @@ -74,7 +80,7 @@ var InputLoggingFilterResourceNameTask = inspectiontaskbase.NewInspectionTask(go queryForms = append(queryForms, &inspectionmetadata.TextParameterFormField{ ParameterFormFieldBase: formFieldBase, Default: defaultValue, - Suggestions: form.DefaultResourceNames, + Suggestions: queryInfo.DefaultResourceNames, ValidationTiming: inspectionmetadata.Change, }) } @@ -100,3 +106,17 @@ var InputLoggingFilterResourceNameTask = inspectiontaskbase.NewInspectionTask(go return resourceNamesInput, nil }) + +// getCurrentActiveQueryIDsForResourceName returns the query IDs that are currently active with retrieving them from the current task graph. +func getCurrentActiveQueryIDsForResourceName(runner coretask.TaskRunner) []string { + tasks := runner.Tasks() + result := []string{} + for _, t := range tasks { + requestInput, found := typedmap.Get(t.Labels(), googlecloudcommon_contract.RequestOptionalInputResourceNameTaskLabel) + if !found { + continue + } + result = append(result, requestInput) + } + return result +} diff --git a/pkg/task/inspection/googlecloudcommon/impl/inputloggingfilterresourcename_task_test.go b/pkg/task/inspection/googlecloudcommon/impl/inputloggingfilterresourcename_task_test.go index 0ac2c8e00..55e9408c2 100644 --- a/pkg/task/inspection/googlecloudcommon/impl/inputloggingfilterresourcename_task_test.go +++ b/pkg/task/inspection/googlecloudcommon/impl/inputloggingfilterresourcename_task_test.go @@ -15,28 +15,68 @@ package googlecloudcommon_impl import ( + "context" "testing" + "github.com/GoogleCloudPlatform/khi/pkg/common/khictx" "github.com/GoogleCloudPlatform/khi/pkg/common/typedmap" inspectionmetadata "github.com/GoogleCloudPlatform/khi/pkg/core/inspection/metadata" inspectiontest "github.com/GoogleCloudPlatform/khi/pkg/core/inspection/test" + coretask "github.com/GoogleCloudPlatform/khi/pkg/core/task" + "github.com/GoogleCloudPlatform/khi/pkg/core/task/taskid" googlecloudcommon_contract "github.com/GoogleCloudPlatform/khi/pkg/task/inspection/googlecloudcommon/contract" inspectioncore_contract "github.com/GoogleCloudPlatform/khi/pkg/task/inspection/inspectioncore/contract" "github.com/google/go-cmp/cmp" ) +type mockTaskRunner struct { + tasks []coretask.UntypedTask +} + +// AddInterceptor implements coretask.TaskRunner. +func (m *mockTaskRunner) AddInterceptor(interceptor coretask.Interceptor) { + panic("unimplemented") +} + +// Result implements coretask.TaskRunner. +func (m *mockTaskRunner) Result() (*typedmap.ReadonlyTypedMap, error) { + panic("unimplemented") +} + +// Run implements coretask.TaskRunner. +func (m *mockTaskRunner) Run(ctx context.Context) error { + panic("unimplemented") +} + +// Wait implements coretask.TaskRunner. +func (m *mockTaskRunner) Wait() <-chan interface{} { + panic("unimplemented") +} + +func (m *mockTaskRunner) Tasks() []coretask.UntypedTask { + return m.tasks +} + +var _ coretask.TaskRunner = (*mockTaskRunner)(nil) + func TestInputLoggingFilterResourceNameTask(t *testing.T) { defaultNames := []string{"projects/foo"} + t1 := coretask.NewTask(taskid.NewDefaultImplementationID[struct{}]("t1"), nil, nil, coretask.WithLabelValue( + googlecloudcommon_contract.RequestOptionalInputResourceNameTaskLabel, "test", + )) + nonRelatedTask := coretask.NewTask(taskid.NewDefaultImplementationID[struct{}]("not-related"), nil, nil) testCases := []struct { desc string taskMode inspectioncore_contract.InspectionTaskModeType inputValue string + tasks []coretask.UntypedTask wantForm inspectionmetadata.GroupParameterFormField }{ { desc: "basic input", taskMode: inspectioncore_contract.TaskModeDryRun, inputValue: "projects/foo", + tasks: []coretask.UntypedTask{t1, nonRelatedTask}, wantForm: inspectionmetadata.GroupParameterFormField{ ParameterFormFieldBase: inspectionmetadata.ParameterFormFieldBase{ Priority: -1000000, @@ -68,6 +108,7 @@ func TestInputLoggingFilterResourceNameTask(t *testing.T) { desc: "invalid input", taskMode: inspectioncore_contract.TaskModeDryRun, inputValue: "invalid-resource-name", + tasks: []coretask.UntypedTask{t1, nonRelatedTask}, wantForm: inspectionmetadata.GroupParameterFormField{ ParameterFormFieldBase: inspectionmetadata.ParameterFormFieldBase{ Priority: -1000000, @@ -100,6 +141,7 @@ func TestInputLoggingFilterResourceNameTask(t *testing.T) { desc: "basic input for run mode", taskMode: inspectioncore_contract.TaskModeRun, inputValue: "projects/foo", + tasks: []coretask.UntypedTask{t1, nonRelatedTask}, wantForm: inspectionmetadata.GroupParameterFormField{ ParameterFormFieldBase: inspectionmetadata.ParameterFormFieldBase{ Priority: -1000000, @@ -127,10 +169,31 @@ func TestInputLoggingFilterResourceNameTask(t *testing.T) { CollapsedByDefault: true, }, }, + { + desc: "shouldn't populate inputs when the task requesting the resource name wasn't included in the graph even it already has the default value updated", + taskMode: inspectioncore_contract.TaskModeRun, + inputValue: "projects/foo", + tasks: []coretask.UntypedTask{nonRelatedTask}, + wantForm: inspectionmetadata.GroupParameterFormField{ + ParameterFormFieldBase: inspectionmetadata.ParameterFormFieldBase{ + Priority: -1000000, + ID: googlecloudcommon_contract.InputLoggingFilterResourceNameTaskID.ReferenceIDString(), + Type: inspectionmetadata.Group, + Label: "Logging filter resource names (advanced)", + Description: "Override these parameters when your logs are not on the same project of the cluster, or customize the log filter target resources.", + HintType: inspectionmetadata.None, + Hint: "", + }, + Children: []inspectionmetadata.ParameterFormField{}, + Collapsible: true, + CollapsedByDefault: true, + }, + }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { ctx := inspectiontest.WithDefaultTestInspectionTaskContext(t.Context()) + ctx = khictx.WithValue[coretask.TaskRunner](ctx, inspectioncore_contract.TaskRunner, &mockTaskRunner{tasks: tc.tasks}) resourceNames, _, err := inspectiontest.RunInspectionTask(ctx, InputLoggingFilterResourceNameTask, inspectioncore_contract.TaskModeDryRun, map[string]any{}) if err != nil { t.Fatalf("Failed to call InputLoggingFilterResourceNameTask at 1st time:%v", err) @@ -139,6 +202,7 @@ func TestInputLoggingFilterResourceNameTask(t *testing.T) { QueryID: "test", } newCtx := inspectiontest.NextRunTaskContext(t.Context(), ctx) + newCtx = khictx.WithValue[coretask.TaskRunner](newCtx, inspectioncore_contract.TaskRunner, &mockTaskRunner{tasks: tc.tasks}) resourceNames.UpdateDefaultResourceNamesForQuery("test", defaultNames) _, metadata, err := inspectiontest.RunInspectionTask(newCtx, InputLoggingFilterResourceNameTask, tc.taskMode, map[string]any{ resourceName.GetInputID(): tc.inputValue, @@ -157,3 +221,40 @@ func TestInputLoggingFilterResourceNameTask(t *testing.T) { }) } } + +func TestGetCurrentActiveQueryIDsForResourceName(t *testing.T) { + t1 := coretask.NewTask(taskid.NewDefaultImplementationID[struct{}]("t1"), nil, nil, coretask.WithLabelValue( + googlecloudcommon_contract.RequestOptionalInputResourceNameTaskLabel, "test1", + )) + t2 := coretask.NewTask(taskid.NewDefaultImplementationID[struct{}]("t2"), nil, nil, coretask.WithLabelValue( + googlecloudcommon_contract.RequestOptionalInputResourceNameTaskLabel, "test2", + )) + nonRelatedTask := coretask.NewTask(taskid.NewDefaultImplementationID[struct{}]("not-related"), nil, nil) + testCases := []struct { + desc string + tasks []coretask.UntypedTask + want []string + }{ + { + desc: "with mixed tasks", + tasks: []coretask.UntypedTask{t1, t2, nonRelatedTask}, + want: []string{ + "test1", + "test2", + }, + }, + { + desc: "with no tasks", + tasks: []coretask.UntypedTask{}, + want: []string{}, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + got := getCurrentActiveQueryIDsForResourceName(&mockTaskRunner{tasks: tc.tasks}) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("getResourceNameInputRequests mismatch (-want,+got):\n%s", diff) + } + }) + } +}