From 954152c7928bf0da9be221e011e32f74bc4cebbc Mon Sep 17 00:00:00 2001 From: Twisha Bansal <58483338+twishabansal@users.noreply.github.com> Date: Wed, 5 Nov 2025 13:44:55 +0530 Subject: [PATCH] fix(cloudmonitoring): populate authRequired in tool manifest (#1800) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Corrects an issue where the `cloud-monitoring-query-prometheus` tool would fail to populate the `authRequired` field in its generated manifest. ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [x] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/genai-toolbox/blob/main/CONTRIBUTING.md) - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/genai-toolbox/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Make sure to add `!` if this involve a breaking change 🛠️ Fixes # --- .../tools/cloudmonitoring/cloudmonitoring.go | 2 +- .../cloudmonitoring/cloudmonitoring_test.go | 101 +++++++++++++++++- 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/internal/tools/cloudmonitoring/cloudmonitoring.go b/internal/tools/cloudmonitoring/cloudmonitoring.go index 9f1e8bbbaba..5e37059aabf 100644 --- a/internal/tools/cloudmonitoring/cloudmonitoring.go +++ b/internal/tools/cloudmonitoring/cloudmonitoring.go @@ -87,7 +87,7 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error) BaseURL: s.BaseURL, UserAgent: s.UserAgent, Client: s.Client, - manifest: tools.Manifest{Description: cfg.Description, Parameters: allParameters.Manifest()}, + manifest: tools.Manifest{Description: cfg.Description, Parameters: allParameters.Manifest(), AuthRequired: cfg.AuthRequired}, mcpManifest: mcpManifest, }, nil } diff --git a/internal/tools/cloudmonitoring/cloudmonitoring_test.go b/internal/tools/cloudmonitoring/cloudmonitoring_test.go index e5af7c9aaa9..cbf17697d1d 100644 --- a/internal/tools/cloudmonitoring/cloudmonitoring_test.go +++ b/internal/tools/cloudmonitoring/cloudmonitoring_test.go @@ -21,10 +21,109 @@ import ( yaml "github.com/goccy/go-yaml" "github.com/google/go-cmp/cmp" "github.com/googleapis/genai-toolbox/internal/server" + "github.com/googleapis/genai-toolbox/internal/sources" + cloudmonitoringsrc "github.com/googleapis/genai-toolbox/internal/sources/cloudmonitoring" "github.com/googleapis/genai-toolbox/internal/testutils" + "github.com/googleapis/genai-toolbox/internal/tools" "github.com/googleapis/genai-toolbox/internal/tools/cloudmonitoring" ) +// mockIncompatibleSource is a source of a different kind to test error paths. +type mockIncompatibleSource struct{ sources.Source } + +func TestInitialize(t *testing.T) { + t.Parallel() + testSource := &cloudmonitoringsrc.Source{Kind: "cloud-monitoring"} + srcs := map[string]sources.Source{ + "my-monitoring-source": testSource, + "incompatible-source": &mockIncompatibleSource{}, + } + + wantParams := tools.Parameters{ + tools.NewStringParameterWithRequired("projectId", "The Id of the Google Cloud project.", true), + tools.NewStringParameterWithRequired("query", "The promql query to execute.", true), + } + + testCases := []struct { + desc string + cfg cloudmonitoring.Config + want *tools.Manifest + wantErr string + }{ + { + desc: "Success case with nil authRequired", + cfg: cloudmonitoring.Config{ + Name: "test-tool", + Kind: "cloud-monitoring-query-prometheus", + Source: "my-monitoring-source", + Description: "A test description.", + AuthRequired: nil, + }, + want: &tools.Manifest{ + Description: "A test description.", + Parameters: wantParams.Manifest(), + AuthRequired: nil, + }, + }, + { + desc: "Success case with specified authRequired", + cfg: cloudmonitoring.Config{ + Name: "test-tool-with-auth", + Kind: "cloud-monitoring-query-prometheus", + Source: "my-monitoring-source", + Description: "Another test description.", + AuthRequired: []string{"google-auth-service"}, + }, + want: &tools.Manifest{ + Description: "Another test description.", + Parameters: wantParams.Manifest(), + AuthRequired: []string{"google-auth-service"}, + }, + }, + { + desc: "Error: source not found", + cfg: cloudmonitoring.Config{ + Name: "test-tool", + Source: "non-existent-source", + }, + wantErr: `no source named "non-existent-source" configured`, + }, + { + desc: "Error: incompatible source kind", + cfg: cloudmonitoring.Config{ + Name: "test-tool", + Source: "incompatible-source", + }, + wantErr: "invalid source for \"cloud-monitoring-query-prometheus\" tool", + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + tool, err := tc.cfg.Initialize(srcs) + + if tc.wantErr != "" { + if err == nil { + t.Fatalf("Initialize() succeeded, want error containing %q", tc.wantErr) + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("Initialize() error = %q, want error containing %q", err, tc.wantErr) + } + return + } + + if err != nil { + t.Fatalf("Initialize() failed: %v", err) + } + + got := tool.Manifest() + if diff := cmp.Diff(tc.want, &got); diff != "" { + t.Errorf("Initialize() manifest mismatch (-want +got):\n%s", diff) + } + }) + } +} + func TestParseFromYamlCloudMonitoring(t *testing.T) { ctx, err := testutils.ContextWithNewLogger() if err != nil { @@ -87,7 +186,7 @@ func TestParseFromYamlCloudMonitoring(t *testing.T) { if err != nil { t.Fatalf("unable to unmarshal: %s", err) } - if diff := cmp.Diff(tc.want, got.Tools); diff != "" { + if diff := cmp.Diff(tc.want, got.Tools, cmp.AllowUnexported(cloudmonitoring.Config{})); diff != "" { t.Fatalf("incorrect parse: diff %v", diff) } })