Add ollama auth and custom client kwargs - #2025
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new configuration knob to Ollama generators so users can pass additional keyword arguments through to the underlying ollama.Client constructor, enabling custom HTTP/TLS/auth configuration for self-hosted or secured Ollama endpoints.
Changes:
- Add
client_kwargstoOllamaGenerator.DEFAULT_PARAMS. - Forward
client_kwargsinto theollama.Client(...)instantiation.
Suppressed comments (2)
garak/generators/ollama.py:44
- Passing
client_kwargsvia**(...)aftertimeout=self.timeoutwill raiseTypeError: got multiple values for keyword argument 'timeout'if the user also suppliestimeoutinclient_kwargs. Build a single kwargs dict and pass it once soclient_kwargscan safely override the default timeout.
self.client = self.ollama.Client(
self.host, timeout=self.timeout, **(self.client_kwargs or {})
) # Instantiates the client with the timeout
garak/generators/ollama.py:43
- New behaviour is introduced by forwarding
client_kwargsinto the underlying client, but the existing mocked Ollama generator tests don't exercise this. Add a unit test (e.g. withrespx) that setsclient_kwargs(headers/verify/timeout override) and asserts the resulting request uses the expected settings.
self.client = self.ollama.Client(
self.host, timeout=self.timeout, **(self.client_kwargs or {})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Timo <43245438+TheRisenPhoenix@users.noreply.github.com>
35b6d13 to
6b930e6
Compare
jmartin-tech
left a comment
There was a problem hiding this comment.
This revision does provide what is describe, however I suspect the goal it seems to be targeting, that of enabling authorization tokens and ssl validation suppression in the example, are things that would benefit from first class support.
{
"ollama": {
"OllamaGeneratorChat": {
"host": "https://my-self-hosted-server.com/ollama",
"api_key": "sk-123456789abc"
"verify_ssl": false
}
}
}Note as with other generators the api_key would be possible to directly supply or have supplied via the default ENV_VAR or a custom key_env_var specified in the generators configuration. Allowing for more portable and more secure configuration patterns.
The suggestions guide towards how I think this can be accomplished and aligned to the patterns in other generators.
If extended arguments need to be possible to configure on the client, I think we can that could be added via a similar pattern to the original offer here though with some extended precedence controls similar to how OpenAICompatible enables extra_params.
Note this PR triggered team discussion that should also lead to better documentation on what should be considered for inclusion as first class supported attributes for generators and how configuration precedence should be handled when resolving competing first class parameter attributes and free form parameters like the one suggested in this PR.
Signed-off-by: Timo <43245438+TheRisenPhoenix@users.noreply.github.com>
|
Thank you for your review @jmartin-tech ! A new exemplary config file could look like this: |
jmartin-tech
left a comment
There was a problem hiding this comment.
This rework looks great, and the support for extra_params is a great add.
I think a unit test or so would help document this and also increase confidence this is working as desired.
|
One more note looking at the latest config example. |
Signed-off-by: Timo <43245438+TheRisenPhoenix@users.noreply.github.com>
That's totally possible, I was just looking for a quick and easy example for a header to demonstrate the header union in practice :D |
Signed-off-by: Timo <43245438+TheRisenPhoenix@users.noreply.github.com>
jmartin-tech
left a comment
There was a problem hiding this comment.
Minor testing feedback and this will be ready to land.
Signed-off-by: Timo <43245438+TheRisenPhoenix@users.noreply.github.com>
This PR adds the possibility to pass kwargs to the Ollama client. This enables support for an enhanced ollama configuration, for example for instances that require an authorization header or that are self hosted (which is likely with ollama), thus also likely having a self-signed certificate that httpx will reject (and which can be circumvented by passing
verify=False).Personally, I'd benefit hugely from this rather small changes, finally being able to correctly configure the client for my instance without the need of some hacky circumventions. :)
Verification
This is an example for connecting to a fictional (!) ollama server that is self-hosted with self-signed certificates and requires authentication.
{ "ollama": { "OllamaGeneratorChat": { "host": "https://my-self-hosted-server.com/ollama", "client_kwargs": { "verify": false, "headers": { "Authorization": "Bearer sk-123456789abc" } } } } }garak -t ollama -n gemma3:latest --probes test.Blankpython -m pytest tests/(5673 passed, 105 skipped, 2 warnings)