mirror of
https://github.com/googleapis/genai-toolbox.git
synced 2026-01-10 07:58:12 -05:00
fix(cloudmonitoring): populate authRequired in tool manifest (#1800)
## 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 #<issue_number_goes_here>
This commit is contained in:
@@ -87,7 +87,7 @@ func (cfg Config) Initialize(srcs map[string]sources.Source) (tools.Tool, error)
|
|||||||
BaseURL: s.BaseURL,
|
BaseURL: s.BaseURL,
|
||||||
UserAgent: s.UserAgent,
|
UserAgent: s.UserAgent,
|
||||||
Client: s.Client,
|
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,
|
mcpManifest: mcpManifest,
|
||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -21,10 +21,109 @@ import (
|
|||||||
yaml "github.com/goccy/go-yaml"
|
yaml "github.com/goccy/go-yaml"
|
||||||
"github.com/google/go-cmp/cmp"
|
"github.com/google/go-cmp/cmp"
|
||||||
"github.com/googleapis/genai-toolbox/internal/server"
|
"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/testutils"
|
||||||
|
"github.com/googleapis/genai-toolbox/internal/tools"
|
||||||
"github.com/googleapis/genai-toolbox/internal/tools/cloudmonitoring"
|
"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) {
|
func TestParseFromYamlCloudMonitoring(t *testing.T) {
|
||||||
ctx, err := testutils.ContextWithNewLogger()
|
ctx, err := testutils.ContextWithNewLogger()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -87,7 +186,7 @@ func TestParseFromYamlCloudMonitoring(t *testing.T) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("unable to unmarshal: %s", err)
|
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)
|
t.Fatalf("incorrect parse: diff %v", diff)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user