Skip to content

Commit

Permalink
fix: remove deprecated Config.errorMessages field
Browse files Browse the repository at this point in the history
This field was deprecated in #5199 , we should remove this in v7

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
  • Loading branch information
rohanKanojia committed Jun 25, 2024
1 parent 1ac6b9a commit be15898
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 89 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#### Improvements
* Fix #6008: removing the optional dependency on bouncy castle
* Fix #5264: Remove deprecated `Config.errorMessages` field

#### Dependency Upgrade
* Fix #6052: Removed dependency on no longer maintained com.github.mifmif:generex
Expand All @@ -16,6 +17,7 @@

#### _**Note**_: Breaking changes
* Check detailed migration documentation for breaking changes in [7.0.0](./doc/MIGRATION-v7.md)
* `Config.errorMessages` has been removed. Please use Kubernetes status messages directly.

### 6.13.0 (2024-05-29)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,6 @@ public class Config {

private boolean onlyHttpWatches;

/**
* @deprecated Use Kubernetes Status directly for extracting error messages.
*/
@Deprecated
private Map<Integer, String> errorMessages = new HashMap<>();

/**
* custom headers
*/
Expand Down Expand Up @@ -335,7 +329,7 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru
autoOAuthToken,
watchReconnectInterval, watchReconnectLimit, connectionTimeout, requestTimeout, scaleTimeout,
loggingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, false, httpProxy, httpsProxy, noProxy,
errorMessages, userAgent, tlsVersions, websocketPingInterval, proxyUsername, proxyPassword,
userAgent, tlsVersions, websocketPingInterval, proxyUsername, proxyPassword,
trustStoreFile, trustStorePassphrase, keyStoreFile, keyStorePassphrase, impersonateUsername, impersonateGroups,
impersonateExtras, null, null, DEFAULT_REQUEST_RETRY_BACKOFFLIMIT, DEFAULT_REQUEST_RETRY_BACKOFFINTERVAL,
DEFAULT_UPLOAD_REQUEST_TIMEOUT, false);
Expand All @@ -348,7 +342,7 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru
String oauthToken, String autoOAuthToken, int watchReconnectInterval, int watchReconnectLimit, int connectionTimeout,
int requestTimeout,
long scaleTimeout, int loggingInterval, int maxConcurrentRequests, int maxConcurrentRequestsPerHost,
boolean http2Disable, String httpProxy, String httpsProxy, String[] noProxy, Map<Integer, String> errorMessages,
boolean http2Disable, String httpProxy, String httpsProxy, String[] noProxy,
String userAgent, TlsVersion[] tlsVersions, long websocketPingInterval, String proxyUsername,
String proxyPassword, String trustStoreFile, String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase,
String impersonateUsername, String[] impersonateGroups, Map<String, List<String>> impersonateExtras,
Expand Down Expand Up @@ -385,7 +379,6 @@ public Config(String masterUrl, String apiVersion, String namespace, boolean tru
this.noProxy = noProxy;
this.proxyUsername = proxyUsername;
this.proxyPassword = proxyPassword;
this.errorMessages = errorMessages;
this.userAgent = userAgent;
this.tlsVersions = tlsVersions;
this.trustStoreFile = trustStoreFile;
Expand Down Expand Up @@ -1186,20 +1179,6 @@ public void setWatchReconnectLimit(int watchReconnectLimit) {
this.requestConfig.setWatchReconnectLimit(watchReconnectLimit);
}

/**
* @deprecated Use Kubernetes status messages directly
* @return map of error codes to message mappings
*/
@Deprecated
@JsonProperty("errorMessages")
public Map<Integer, String> getErrorMessages() {
return errorMessages;
}

public void setErrorMessages(Map<Integer, String> errorMessages) {
this.errorMessages = errorMessages;
}

public static ConfigBuilder builder() {
return new ConfigBuilder();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,6 @@ void testEmptyConfig() {
.hasFieldOrPropertyWithValue("impersonateExtras", Collections.emptyMap())
.hasFieldOrPropertyWithValue("http2Disable", false)
.hasFieldOrPropertyWithValue("tlsVersions", new TlsVersion[] { TlsVersion.TLS_1_3, TlsVersion.TLS_1_2 })
.hasFieldOrPropertyWithValue("errorMessages", Collections.emptyMap())
.satisfies(e -> assertThat(e.getCurrentContext()).isNull())
.satisfies(e -> assertThat(e.getImpersonateGroups()).isEmpty())
.satisfies(e -> assertThat(e.getUserAgent()).isNotNull());
Expand Down Expand Up @@ -693,7 +692,6 @@ private void assertConfig(Config config, boolean autoToken) {

assertEquals(120, config.getMaxConcurrentRequests());
assertEquals(20, config.getMaxConcurrentRequestsPerHost());
assertThat(config.getErrorMessages()).isEmpty();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,25 +579,7 @@ protected void assertResponseCode(HttpRequest request, HttpResponse<?> response)
return;
}

int statusCode = response.code();
String customMessage = config.getErrorMessages().get(statusCode);

if (customMessage != null) {
throw requestFailure(request,
createStatus(statusCode, combineMessages(customMessage, createStatus(response, getKubernetesSerialization()))));
} else {
throw requestFailure(request, createStatus(response, getKubernetesSerialization()));
}
}

private String combineMessages(String customMessage, Status defaultStatus) {
if (defaultStatus != null) {
String message = defaultStatus.getMessage();
if (message != null && message.length() > 0) {
return customMessage + " " + message;
}
}
return customMessage;
throw requestFailure(request, createStatus(response, getKubernetesSerialization()));
}

public static Status createStatus(HttpResponse<?> response, KubernetesSerialization kubernetesSerialization) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,22 +121,6 @@ void assertResponseCodeClientError() throws Exception {
assertThat(result).hasMessageContaining("Failure executing: GET at: https://example.com. Message: Bad Request.");
}

@Test
@DisplayName("assertResponse, with client error and custom message, should throw exception")
void assertResponseCodeClientErrorAndCustomMessage() throws Exception {
// Given
operationSupport.getConfig().getErrorMessages().put(400, "Custom message");
final HttpRequest request = new StandardHttpRequest.Builder().method("GET", null, null).uri(new URI("https://example.com"))
.build();
final HttpResponse<String> response = new TestHttpResponse<String>().withCode(400);
// When
final KubernetesClientException result = assertThrows(KubernetesClientException.class,
() -> operationSupport.assertResponseCode(request, response));
// Then
assertThat(result)
.hasMessageContaining("Failure executing: GET at: https://example.com. Message: Custom message Bad Request.");
}

@Test
@DisplayName("assertResponse, with client error, should throw exception with server response body")
void assertResponseCodeClientErrorAndStatus() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,16 @@

import io.fabric8.kubernetes.api.model.Event;
import io.fabric8.kubernetes.api.model.EventBuilder;
import io.fabric8.kubernetes.api.model.EventList;
import io.fabric8.kubernetes.api.model.Status;
import io.fabric8.kubernetes.api.model.StatusBuilder;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient;
import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer;
import org.junit.jupiter.api.Test;

import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;

@EnableKubernetesMockClient
Expand All @@ -38,27 +35,6 @@ class ErrorMessageTest {
KubernetesMockServer server;
KubernetesClient client;

@Test
void whenCustomErrorMessageInConfig_thenErrorMessageShouldNotContainCustomMessage() {
// Given
String customMessage = "INVALID";
client.getConfiguration().getErrorMessages().put(403, customMessage);
server.expect().delete()
.withPath("/api/v1/namespaces/test/events")
.andReturn(HTTP_FORBIDDEN, new StatusBuilder()
.withCode(HTTP_FORBIDDEN)
.withMessage("forbidden")
.build())
.once();
NonNamespaceOperation<Event, EventList, Resource<Event>> eventResource = client.v1().events().inNamespace("test");

// When + Then
assertThatExceptionOfType(KubernetesClientException.class)
.isThrownBy(eventResource::delete)
.withMessageContaining(customMessage)
.hasFieldOrPropertyWithValue("code", HTTP_FORBIDDEN);
}

@Test
void whenResponseBodyContainsKubernetesStatus_thenErrorMessageShouldContainStatusMessage() {
// Given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public OpenShiftConfig(String openShiftUrl, String oapiVersion, String masterUrl
int connectionTimeout, int requestTimeout, long scaleTimeout, int loggingInterval,
int maxConcurrentRequests, int maxConcurrentRequestsPerHost, boolean http2Disable, String httpProxy,
String httpsProxy,
String[] noProxy, Map<Integer, String> errorMessages, String userAgent, TlsVersion[] tlsVersions,
String[] noProxy, String userAgent, TlsVersion[] tlsVersions,
long websocketPingInterval, String proxyUsername, String proxyPassword, String trustStoreFile,
String trustStorePassphrase, String keyStoreFile, String keyStorePassphrase, String impersonateUsername,
String[] impersonateGroups, Map<String, List<String>> impersonateExtras, OAuthTokenProvider oauthTokenProvider,
Expand All @@ -94,8 +94,7 @@ public OpenShiftConfig(String openShiftUrl, String oapiVersion, String masterUrl
oauthToken, autoOAuthToken,
watchReconnectInterval, watchReconnectLimit, connectionTimeout, requestTimeout, scaleTimeout,
loggingInterval, maxConcurrentRequests, maxConcurrentRequestsPerHost, http2Disable, httpProxy, httpsProxy,
noProxy,
errorMessages, userAgent, tlsVersions, websocketPingInterval, proxyUsername, proxyPassword,
noProxy, userAgent, tlsVersions, websocketPingInterval, proxyUsername, proxyPassword,
trustStoreFile, trustStorePassphrase, keyStoreFile, keyStorePassphrase, impersonateUsername, impersonateGroups,
impersonateExtras, oauthTokenProvider, customHeaders,
requestRetryBackoffLimit,
Expand Down Expand Up @@ -131,8 +130,7 @@ public OpenShiftConfig(Config kubernetesConfig, String openShiftUrl, String oapi
kubernetesConfig.getLoggingInterval(), kubernetesConfig.getMaxConcurrentRequests(),
kubernetesConfig.getMaxConcurrentRequestsPerHost(), kubernetesConfig.isHttp2Disable(),
kubernetesConfig.getHttpProxy(), kubernetesConfig.getHttpsProxy(), kubernetesConfig.getNoProxy(),
kubernetesConfig.getErrorMessages(), kubernetesConfig.getUserAgent(),
kubernetesConfig.getTlsVersions(),
kubernetesConfig.getUserAgent(), kubernetesConfig.getTlsVersions(),
kubernetesConfig.getWebsocketPingInterval(), kubernetesConfig.getProxyUsername(),
kubernetesConfig.getProxyPassword(), kubernetesConfig.getTrustStoreFile(),
kubernetesConfig.getTrustStorePassphrase(), kubernetesConfig.getKeyStoreFile(),
Expand Down

0 comments on commit be15898

Please sign in to comment.