feat: proxy component #1444
feat: proxy component #1444
Conversation
|
@hetangmodi-crest please fix |
| "username": true, | ||
| "password": true, |
There was a problem hiding this comment.
Does it make sense to move proxy_type, username and password to a default configuration? I feel that more TAs would benefit from having it already there.
There was a problem hiding this comment.
With respect to proxy server setup, username, password and proxy_type are not required configurations. I am inclined not keeping by default so that developers have awareness of the new features UCC rolls out. Yet, giving these fields by default would easy for the developers.
There was a problem hiding this comment.
Thanks for remembering about this file as well!
Do we know how much is it used by folks in their TAs?
There was a problem hiding this comment.
I am unsure about how much oauth.template file is used overall, but add-ons like Box and ServiceNow use it.
There was a problem hiding this comment.
There is some groundbreaking work that was done to bring support of proxy components into UCC in this PR! Great work @hetangmodi-crest!
| {%- if need_proxy %} | ||
| { | ||
| "type": "proxyTab" | ||
| }, | ||
| {%- endif %} |
There was a problem hiding this comment.
Let's move it after loggingTab definition.
There was a problem hiding this comment.
Updated the template file.
| @@ -12,6 +12,7 @@ | |||
| import splunk.admin as admin | |||
| from solnlib import log | |||
| from solnlib import conf_manager | |||
| from solnlib.conf_manager import InvalidHostnameError,InvalidPortError | |||
There was a problem hiding this comment.
| from solnlib.conf_manager import InvalidHostnameError,InvalidPortError | |
| from solnlib.conf_manager import InvalidHostnameError, InvalidPortError |
There was a problem hiding this comment.
Made the changes, also updated oauth.template
|
|
||
| def updating_dictionaries(key_name: str, const: Any) -> None: | ||
| for key, value in const.items(): | ||
| nonlocal definition |
There was a problem hiding this comment.
This looks hacky to me, how we can avoid having this?
There was a problem hiding this comment.
This was a result of another complex logic there due to one older scenario (which actually is invalid), I have simplified the logic.
| { | ||
| "type": "loggingTab" | ||
| } | ||
| {%- endif %} |
There was a problem hiding this comment.
Can we move {%- if need_proxy %} after loggingTab? We have loggingTab by default and it is being referenced in 2 places with this approach.
There was a problem hiding this comment.
Made the changes.
| @@ -29,6 +29,7 @@ | |||
|
|
|||
|
|
|||
| LIBS_REQUIRED_FOR_UI = {"splunktaucclib": "6.4.0"} | |||
| LIBS_REQUIRED_FOR_OAUTH = {"solnlib": "5.5.0"} | |||
There was a problem hiding this comment.
Let's leave the OAuth code updates for the next PRs, it's quite hard to review this one and I am not totally sure we need to require a specific version of solnlib for updating OAuth-related handlers.
There was a problem hiding this comment.
For the OAuth helper that we are generating, the solnlib.conf_manager's get_proxy_dict is being used, hence I've fixed the solnlib version and OAuth template file with this Proxy related PR.
| > **_NOTE:_** | ||
| There are 2 ways to exclude optional entities in your add-on, either omit them from the proxy tab, or set the entities to false. | ||
|
|
||
| ### Keys |
There was a problem hiding this comment.
Now I think: wouldn't it be good to put this table above ### Available configurations? Plus add "name" and "title" to the table and indicate required and optional fields.
There was a problem hiding this comment.
I think moving this table above Available configuration will break the flow of the document, as first we should mention what is being generated by default and then steps on how to customize it, which we are following here.
I have added name and title keys in the Keys table.
There was a problem hiding this comment.
Ok. So maybe would it be good to indicate what the ### Available configurations is about? To mention something about those values.
E.g.:
- Enable proxy :
```json
{
"type": "checkbox",
"label": "Enable",
"field": "proxy_enabled"
}
```
Because when I had a first look at that, I wasn't sure whether this is a parameter to set, or does it say that it is the default value for the minimal definition.
There was a problem hiding this comment.
I had made the changes in documentation, can you please look into it?
Issue number: ADDON-72372
Summary
Similar to logging component, we now have a proxy component, also introduced a new argument
--need-proxywhich includes default proxy configuration duringinitmethodChanges
Just like logging component we now have a new type
proxyTabwhich gives default proxy configuration, so that user doesn't have to mention default configuration themselves they just have to mention their custom configurations.Introduced a new argument in
initmethod that is--need-proxy, which includes default proxy configuration inglobalConfig.jsonduring creation of new add-on.Changed the approach of fetching proxy_details in
oauth templatefile. For that created a function in solnlib which fetches proxy_details and validatesproxy_hostandproxy_port.User experience
type=proxyTabin their globalConfig.for ex. If a user wants to customize the label of
portthey would just have to write{ "type": "proxyTab", "port": { "label": "Proxy port", } }get_proxy_dictfunction of solnlib.--need-proxywhich includes default proxy configuration inglobalConfig.json.Checklist
If your change doesn't seem to apply, please leave them unchecked.