-
Notifications
You must be signed in to change notification settings - Fork 1
Add proxy configuration support #33
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,8 +12,14 @@ | |||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Configuration class for setting up OpenAI-related beans. | ||||||||||||||||||||||||
* <p> | ||||||||||||||||||||||||
* This class is responsible for creating and configuring the necessary beans for interacting with the OpenAI API. It includes beans for the OpenAI | ||||||||||||||||||||||||
* service and the REST client used to communicate with the OpenAI API. | ||||||||||||||||||||||||
* This class is responsible for creating and configuring the necessary beans for interacting | ||||||||||||||||||||||||
* with the OpenAI API. It includes beans for the OpenAI service and the REST client used | ||||||||||||||||||||||||
* to communicate with the OpenAI API. | ||||||||||||||||||||||||
* </p> | ||||||||||||||||||||||||
* <p> | ||||||||||||||||||||||||
* The configuration supports proxy settings for environments that require connecting | ||||||||||||||||||||||||
* through a corporate proxy server. Proxy settings can be enabled via configuration | ||||||||||||||||||||||||
* properties. | ||||||||||||||||||||||||
* </p> | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
@Slf4j | ||||||||||||||||||||||||
|
@@ -47,16 +53,89 @@ public OpenAIService openAIService() { | |||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Creates an instance of the OpenAI REST client. | ||||||||||||||||||||||||
* <p> | ||||||||||||||||||||||||
* The client is configured with the API endpoint, content type, and authorization header. | ||||||||||||||||||||||||
* The client is configured with the API endpoint, content type, authorization header, | ||||||||||||||||||||||||
* and proxy settings if enabled. | ||||||||||||||||||||||||
* </p> | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
* @return an instance of {@link RestClient} | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
@Bean(name = "openAIRestClient") | ||||||||||||||||||||||||
public RestClient openAIRestClient() { | ||||||||||||||||||||||||
log.info("Creating OpenAI REST client with endpoint: {}", properties.getApiEndpoint()); | ||||||||||||||||||||||||
return RestClient.builder().baseUrl(properties.getApiEndpoint()).defaultHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||||||||||||||||||||||||
.defaultHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE) | ||||||||||||||||||||||||
.defaultHeader(HttpHeaders.AUTHORIZATION, BEARER_TOKEN_PREFIX + properties.getApiKey()).build(); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
RestClient.Builder builder = RestClient.builder() | ||||||||||||||||||||||||
.baseUrl(properties.getApiEndpoint()) | ||||||||||||||||||||||||
.defaultHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE) | ||||||||||||||||||||||||
.defaultHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE) | ||||||||||||||||||||||||
.defaultHeader(HttpHeaders.AUTHORIZATION, BEARER_TOKEN_PREFIX + properties.getApiKey()); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Apply proxy configuration if enabled | ||||||||||||||||||||||||
if (properties.getProxy().isEnabled()) { | ||||||||||||||||||||||||
// Validate proxy configuration | ||||||||||||||||||||||||
validateProxyConfiguration(properties.getProxy()); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
log.info("Configuring proxy for OpenAI client: {}:{}", properties.getProxy().getHost(), properties.getProxy().getPort()); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Configure proxy settings using system properties | ||||||||||||||||||||||||
System.setProperty("http.proxyHost", properties.getProxy().getHost()); | ||||||||||||||||||||||||
System.setProperty("http.proxyPort", String.valueOf(properties.getProxy().getPort())); | ||||||||||||||||||||||||
System.setProperty("https.proxyHost", properties.getProxy().getHost()); | ||||||||||||||||||||||||
System.setProperty("https.proxyPort", String.valueOf(properties.getProxy().getPort())); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Add proxy authentication if credentials are provided | ||||||||||||||||||||||||
if (properties.getProxy().getUsername() != null && !properties.getProxy().getUsername().isEmpty()) { | ||||||||||||||||||||||||
log.debug("Adding proxy authentication for user: {}", properties.getProxy().getUsername()); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Set system properties for proxy authentication | ||||||||||||||||||||||||
System.setProperty("http.proxyUser", properties.getProxy().getUsername()); | ||||||||||||||||||||||||
System.setProperty("http.proxyPassword", properties.getProxy().getPassword()); | ||||||||||||||||||||||||
System.setProperty("https.proxyUser", properties.getProxy().getUsername()); | ||||||||||||||||||||||||
System.setProperty("https.proxyPassword", properties.getProxy().getPassword()); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Configure proxy authentication | ||||||||||||||||||||||||
java.net.Authenticator authenticator = new java.net.Authenticator() { | ||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||
protected java.net.PasswordAuthentication getPasswordAuthentication() { | ||||||||||||||||||||||||
if (getRequestorType() == java.net.Authenticator.RequestorType.PROXY) { | ||||||||||||||||||||||||
return new java.net.PasswordAuthentication( | ||||||||||||||||||||||||
properties.getProxy().getUsername(), | ||||||||||||||||||||||||
properties.getProxy().getPassword().toCharArray() | ||||||||||||||||||||||||
Comment on lines
+100
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The password could be null even when username is not null and not empty, which would cause a NullPointerException. The validation only checks if password is null or empty when username is provided, but this check happens after the validation.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||
); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
}; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Set the authenticator | ||||||||||||||||||||||||
java.net.Authenticator.setDefault(authenticator); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return builder.build(); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Validates that the proxy configuration is complete and valid. | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
* @param proxyConfig the proxy configuration to validate | ||||||||||||||||||||||||
* @throws IllegalArgumentException if the proxy configuration is invalid | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
private void validateProxyConfiguration(OpenAIConfigProperties.ProxyConfig proxyConfig) { | ||||||||||||||||||||||||
if (proxyConfig.getHost() == null || proxyConfig.getHost().trim().isEmpty()) { | ||||||||||||||||||||||||
throw new IllegalArgumentException("Proxy host cannot be null or empty when proxy is enabled"); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (proxyConfig.getPort() <= 0 || proxyConfig.getPort() > 65535) { | ||||||||||||||||||||||||
throw new IllegalArgumentException("Proxy port must be between 1 and 65535, got: " + proxyConfig.getPort()); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// If username is provided, password must also be provided | ||||||||||||||||||||||||
if (proxyConfig.getUsername() != null && !proxyConfig.getUsername().isEmpty()) { | ||||||||||||||||||||||||
if (proxyConfig.getPassword() == null || proxyConfig.getPassword().isEmpty()) { | ||||||||||||||||||||||||
throw new IllegalArgumentException("Proxy password cannot be null or empty when username is provided"); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
log.debug("Proxy configuration validated successfully"); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
package com.digitalsanctuary.springaiclient.adapters.openai.config; | ||
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertFalse; | ||
import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
||
import org.junit.jupiter.api.Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.test.context.SpringBootTest; | ||
import org.springframework.test.context.ActiveProfiles; | ||
|
||
import com.digitalsanctuary.springaiclient.TestApplication; | ||
|
||
import lombok.extern.slf4j.Slf4j; | ||
|
||
@Slf4j | ||
@SpringBootTest(classes = TestApplication.class) | ||
@ActiveProfiles("test") | ||
class OpenAIConfigProxyTest { | ||
|
||
@Autowired | ||
private OpenAIConfigProperties properties; | ||
|
||
@Autowired | ||
private OpenAIConfig openAIConfig; | ||
|
||
@Test | ||
void testProxyConfigurationLoaded() { | ||
log.info("Testing proxy configuration loading"); | ||
|
||
// Verify the basic properties are loaded | ||
assertNotNull(properties); | ||
assertNotNull(properties.getApiKey()); | ||
assertEquals("gpt-4o", properties.getModel()); | ||
|
||
// Verify the proxy configuration is loaded | ||
assertNotNull(properties.getProxy()); | ||
assertFalse(properties.getProxy().isEnabled()); // Disabled by default in test config | ||
assertEquals("test-proxy.example.com", properties.getProxy().getHost()); | ||
assertEquals(8080, properties.getProxy().getPort()); | ||
assertEquals("testuser", properties.getProxy().getUsername()); | ||
assertEquals("testpass", properties.getProxy().getPassword()); | ||
} | ||
|
||
@Test | ||
void testCustomProxyConfiguration() { | ||
log.info("Testing custom proxy configuration"); | ||
|
||
// Create a custom proxy configuration | ||
OpenAIConfigProperties.ProxyConfig proxyConfig = new OpenAIConfigProperties.ProxyConfig(); | ||
proxyConfig.setEnabled(true); | ||
proxyConfig.setHost("custom-proxy.example.org"); | ||
proxyConfig.setPort(3128); | ||
proxyConfig.setUsername("customuser"); | ||
proxyConfig.setPassword("custompass"); | ||
|
||
// Apply the custom configuration | ||
properties.setProxy(proxyConfig); | ||
|
||
// Verify the custom configuration | ||
assertEquals(true, properties.getProxy().isEnabled()); | ||
assertEquals("custom-proxy.example.org", properties.getProxy().getHost()); | ||
assertEquals(3128, properties.getProxy().getPort()); | ||
assertEquals("customuser", properties.getProxy().getUsername()); | ||
assertEquals("custompass", properties.getProxy().getPassword()); | ||
} | ||
|
||
// We'll just directly test the proxy configuration values | ||
@Test | ||
void testExtraProxyConfigurationValues() { | ||
log.info("Testing additional proxy configuration values"); | ||
|
||
// Create a custom proxy configuration with various values | ||
OpenAIConfigProperties.ProxyConfig proxyConfig = new OpenAIConfigProperties.ProxyConfig(); | ||
|
||
// Test default values | ||
assertFalse(proxyConfig.isEnabled()); | ||
|
||
// Test setting values | ||
proxyConfig.setEnabled(true); | ||
proxyConfig.setHost("custom-proxy.domain.com"); | ||
proxyConfig.setPort(3128); | ||
proxyConfig.setUsername("proxyuser123"); | ||
proxyConfig.setPassword("securepass456"); | ||
|
||
assertEquals(true, proxyConfig.isEnabled()); | ||
assertEquals("custom-proxy.domain.com", proxyConfig.getHost()); | ||
assertEquals(3128, proxyConfig.getPort()); | ||
assertEquals("proxyuser123", proxyConfig.getUsername()); | ||
assertEquals("securepass456", proxyConfig.getPassword()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,3 +6,11 @@ ds: | |||||
output-tokens: 4096 | ||||||
api-endpoint: https://api.openai.com/v1/chat/completions | ||||||
system-prompt: "You are a helpful assistant." | ||||||
|
||||||
# Test proxy configuration (disabled for normal tests) | ||||||
proxy: | ||||||
enabled: false | ||||||
host: test-proxy.example.com | ||||||
port: 8080 | ||||||
username: testuser | ||||||
password: testpass | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] Hardcoded passwords in test configuration files could be a security concern if they resemble real credentials. Consider using obviously fake values like 'test-password-123' to make it clear these are test-only values.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
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.
Setting global system properties for proxy configuration affects the entire JVM and could interfere with other components. Consider using a more scoped approach like configuring the RestClient with a custom ClientHttpRequestFactory that handles proxy settings.
Copilot uses AI. Check for mistakes.