Skip to content

Commit 1b9724b

Browse files
wtrockiwtrocki
authored
Merge pull request #146 from secondsun/http_error_unification
2 parents 9e0ea19 + a361852 commit 1b9724b

File tree

3 files changed

+94
-33
lines changed

3 files changed

+94
-33
lines changed

‎source/rhoas/src/main/java/com/openshift/cloud/beans/AccessTokenSecretTool.java

+44-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.openshift.cloud.beans;
22

33
import com.openshift.cloud.controllers.ConditionAwareException;
4+
import com.openshift.cloud.controllers.ConditionUtil;
45
import com.openshift.cloud.v1alpha.models.KafkaCondition;
56
import io.fabric8.kubernetes.client.KubernetesClient;
67
import io.vertx.core.json.JsonObject;
@@ -52,9 +53,12 @@ public String getAccessToken(String accessTokenSecretName, String namespace)
5253
var offlineToken = getOfflineTokenFromSecret(accessTokenSecretName, namespace);
5354
var accessToken = exchangeToken(offlineToken);
5455
return accessToken;
56+
} catch (ConditionAwareException ex) {
57+
// Log and rethrow exception
58+
LOG.log(Level.SEVERE, ex.getMessage());
59+
throw ex;
5560
} catch (Throwable ex) {
56-
// I do not like ^^, but it seems like one of the APIs we call throws a type "error" when it
57-
// should throw "exception"
61+
// Unexpected exception or error (NPE, IOException, out of memory, etc)
5862
LOG.log(Level.SEVERE, ex.getMessage());
5963
throw new ConditionAwareException(
6064
ex.getMessage(),
@@ -66,24 +70,42 @@ public String getAccessToken(String accessTokenSecretName, String namespace)
6670
}
6771
}
6872

69-
private String getOfflineTokenFromSecret(String secretName, String namespace) throws Exception {
73+
/**
74+
* Given a secret and a namespace load the secret and decode the value with the key "value"
75+
*
76+
* @param secretName name of the secret
77+
* @param namespace namespace of the secret
78+
* @return the 64 decoded value of namespace/secretName
79+
* @throws ConditionAwareException if the secret does not exist
80+
* @throws IllegalArgumentException if secret value is not in valid Base64
81+
*/
82+
private String getOfflineTokenFromSecret(String secretName, String namespace)
83+
throws ConditionAwareException {
7084
var token = k8sClient.secrets().inNamespace(namespace).withName(secretName).get();
7185
if (token != null) {
7286
var offlineToken = token.getData().get(ACCESS_TOKEN_SECRET_KEY);
87+
// decode may throw IllegalArgumentException
7388
offlineToken = new String(Base64.getDecoder().decode(offlineToken));
74-
7589
return offlineToken;
7690
}
77-
throw new Exception("Missing Offline Token Secret " + secretName);
91+
// We expect the token to exist, and if it doesn't raise an exception.
92+
throw new ConditionAwareException(
93+
String.format("Missing Offline Token Secret %s", secretName),
94+
null,
95+
KafkaCondition.Type.AcccesTokenSecretValid,
96+
KafkaCondition.Status.False,
97+
"ConditionAwareException",
98+
String.format("Missing Offline Token Secret %s", secretName));
7899
}
79100

80101
/**
81102
* This method exchanges an offline token for a new refresh token
82103
*
83104
* @param offlineToken the token from ss.redhat.com
84105
* @return a token to be used as a bearer token to authorize the user
106+
* @throws ConditionAwareException
85107
*/
86-
private String exchangeToken(String offlineToken) {
108+
private String exchangeToken(String offlineToken) throws ConditionAwareException {
87109
try {
88110
HttpRequest request =
89111
HttpRequest.newBuilder()
@@ -107,10 +129,24 @@ private String exchangeToken(String offlineToken) {
107129
var json = new JsonObject(tokens);
108130
return json.getString("access_token");
109131
} else {
110-
throw new RuntimeException(response.body());
132+
LOG.log(
133+
Level.SEVERE, String.format("Exchange token failed with error %s", response.body()));
134+
throw new ConditionAwareException(
135+
response.body(),
136+
null,
137+
KafkaCondition.Type.AcccesTokenSecretValid,
138+
KafkaCondition.Status.False,
139+
String.format("Http Error Code %d", response.statusCode()),
140+
ConditionUtil.getStandarizedErrorMessage(response.statusCode()));
111141
}
112142
} catch (IOException | InterruptedException e) {
113-
throw new RuntimeException(e);
143+
throw new ConditionAwareException(
144+
e.getMessage(),
145+
e,
146+
KafkaCondition.Type.AcccesTokenSecretValid,
147+
KafkaCondition.Status.False,
148+
e.getClass().getName(),
149+
e.getMessage());
114150
}
115151
}
116152

‎source/rhoas/src/main/java/com/openshift/cloud/beans/KafkaApiClient.java

+4-25
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.openshift.cloud.api.models.ServiceAccountRequest;
1111
import com.openshift.cloud.auth.HttpBearerAuth;
1212
import com.openshift.cloud.controllers.ConditionAwareException;
13+
import com.openshift.cloud.controllers.ConditionUtil;
1314
import com.openshift.cloud.v1alpha.models.CloudServiceAccountRequest;
1415
import com.openshift.cloud.v1alpha.models.CloudServiceAccountRequestSpec;
1516
import com.openshift.cloud.v1alpha.models.KafkaCondition;
@@ -49,7 +50,7 @@ public KafkaRequest getKafkaById(String kafkaId, String accessToken)
4950
try {
5051
return createClient(accessToken).getKafkaById(kafkaId);
5152
} catch (ApiException e) {
52-
String message = getStandarizedErrorMessage(e);
53+
String message = ConditionUtil.getStandarizedErrorMessage(e);
5354
throw new ConditionAwareException(
5455
message,
5556
e,
@@ -64,7 +65,7 @@ public KafkaRequestList listKafkas(String accessToken) throws ConditionAwareExce
6465
try {
6566
return createClient(accessToken).listKafkas(null, null, null, null);
6667
} catch (ApiException e) {
67-
String message = getStandarizedErrorMessage(e);
68+
String message = ConditionUtil.getStandarizedErrorMessage(e);
6869
throw new ConditionAwareException(
6970
message,
7071
e,
@@ -83,7 +84,7 @@ public ServiceAccount createServiceAccount(
8384
serviceAccountRequest.setName(spec.getServiceAccountName());
8485
return createClient(accessToken).createServiceAccount(serviceAccountRequest);
8586
} catch (ApiException e) {
86-
String message = getStandarizedErrorMessage(e);
87+
String message = ConditionUtil.getStandarizedErrorMessage(e);
8788
throw new ConditionAwareException(
8889
message,
8990
e,
@@ -136,26 +137,4 @@ public void createSecretForServiceAccount(
136137
e.getMessage());
137138
}
138139
}
139-
140-
private String getStandarizedErrorMessage(ApiException e) {
141-
if (e.getCode() == 504) {
142-
return "Server timeout. Server is not responding";
143-
}
144-
if (e.getCode() == 500) {
145-
return "Unknown server error.";
146-
}
147-
if (e.getCode() == 500) {
148-
return "Unknown server error.";
149-
}
150-
if (e.getCode() == 400) {
151-
return "Invalid request " + e.getMessage();
152-
}
153-
if (e.getCode() == 401) {
154-
return "Auth Token is invalid.";
155-
}
156-
if (e.getCode() == 403) {
157-
return "User not authorized to access the service";
158-
}
159-
return e.getMessage();
160-
}
161140
}

‎source/rhoas/src/main/java/com/openshift/cloud/controllers/ConditionUtil.java

+46
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static com.openshift.cloud.v1alpha.models.KafkaCondition.Status.False;
44
import static com.openshift.cloud.v1alpha.models.KafkaCondition.Status.True;
55

6+
import com.openshift.cloud.ApiException;
67
import com.openshift.cloud.v1alpha.models.*;
78
import com.openshift.cloud.v1alpha.models.KafkaCondition.Status;
89
import java.time.ZoneOffset;
@@ -222,4 +223,49 @@ public static boolean allTrue(List<KafkaCondition> conditions) {
222223
;
223224
return allTrue.get();
224225
}
226+
227+
/**
228+
* Given an api exception, map the http error code to a known string.
229+
*
230+
* @param e an exception thrown by a call to the MAS API
231+
* @return a human readable String to be set as the message property of a failed condition
232+
*/
233+
public static String getStandarizedErrorMessage(ApiException e) {
234+
235+
switch (e.getCode()) {
236+
case 504: // SC_GATEWAY_TIMEOUT:
237+
case 500: // SC_INTERNAL_SERVER_ERROR:
238+
case 401: // HttpStatus.SC_UNAUTHORIZED:
239+
case 403: // HttpStatus.SC_FORBIDDEN:
240+
return getStandarizedErrorMessage(e.getCode());
241+
case 400: // HttpStatus.SC_BAD_REQUEST:
242+
return "Invalid request " + e.getMessage();
243+
default:
244+
return e.getMessage();
245+
}
246+
}
247+
/**
248+
* Map the http error code to a known string.
249+
*
250+
* @param statusCode a non 200 HTTP code returned by the system.
251+
* @return a human readable String to be set as the message property of a failed condition
252+
*/
253+
public static String getStandarizedErrorMessage(int statusCode) {
254+
switch (statusCode) {
255+
case 504: // SC_GATEWAY_TIMEOUT:
256+
return "Server timeout. Server is not responding";
257+
case 503: // SC_UNAVAILABILE
258+
return "Service unavailable at the moment";
259+
case 500: // SC_INTERNAL_SERVER_ERROR:
260+
return "Unknown server error.";
261+
case 400: // HttpStatus.SC_BAD_REQUEST:
262+
return "Invalid request";
263+
case 401: // HttpStatus.SC_UNAUTHORIZED:
264+
return "Auth Token is invalid.";
265+
case 403: // HttpStatus.SC_FORBIDDEN:
266+
return "User not authorized to access the service";
267+
default:
268+
return String.format("Http Error Code %d", statusCode);
269+
}
270+
}
225271
}

0 commit comments

Comments
 (0)