-
Notifications
You must be signed in to change notification settings - Fork 62
Introduce enabled/disabled tool list in configuration file #155
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
Conversation
Currently implementing your review suggestions. Will ping you there when ready for a last review |
pkg/config/config.go
Outdated
SSEBaseURL string `toml:"sse_base_url,omitempty"` | ||
KubeConfig string `toml:"kubeconfig,omitempty"` | ||
ListOutput string `toml:"list_output,omitempty"` | ||
ReadOnly bool `toml:"read_only,omitempty"` |
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.
I think it would be good to move the description comment here since to clarify its purpose:
// When true, expose only tools annotated with readOnlyHint=true
Same for DisableDestructive
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.
Good point. I'll add these comments.
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.
Updated.
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.
LGTM, however, I think I'd rename allowed_tools
to enabled_tools
and denied_tools
to disabled_tools
.
enabled/disabled is more descriptive than allowed/denied. I'll update with that. |
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.
LGTM, thx!
Fixes #156
This PR introduces enabled/disabled tool list in the configuration file.
Users will be able to add enabled and disabled tools in the configuration file. e.g.
This list will be applied to the given profile (full is the default);
This PR partially fixes #151
In a follow up PR, I'll add validations to prevent duplicate entries in both enable and disabled list.