diff --git a/source/rhoas/src/main/java/com/openshift/cloud/beans/AccessTokenSecretTool.java b/source/rhoas/src/main/java/com/openshift/cloud/beans/AccessTokenSecretTool.java index c842c2c3..bda41604 100644 --- a/source/rhoas/src/main/java/com/openshift/cloud/beans/AccessTokenSecretTool.java +++ b/source/rhoas/src/main/java/com/openshift/cloud/beans/AccessTokenSecretTool.java @@ -1,6 +1,7 @@ package com.openshift.cloud.beans; import com.openshift.cloud.controllers.ConditionAwareException; +import com.openshift.cloud.controllers.ConditionUtil; import com.openshift.cloud.v1alpha.models.KafkaCondition; import io.fabric8.kubernetes.client.KubernetesClient; import io.vertx.core.json.JsonObject; @@ -52,9 +53,12 @@ public String getAccessToken(String accessTokenSecretName, String namespace) var offlineToken = getOfflineTokenFromSecret(accessTokenSecretName, namespace); var accessToken = exchangeToken(offlineToken); return accessToken; + } catch (ConditionAwareException ex) { + // Log and rethrow exception + LOG.log(Level.SEVERE, ex.getMessage()); + throw ex; } catch (Throwable ex) { - // I do not like ^^, but it seems like one of the APIs we call throws a type "error" when it - // should throw "exception" + // Unexpected exception or error (NPE, IOException, out of memory, etc) LOG.log(Level.SEVERE, ex.getMessage()); throw new ConditionAwareException( ex.getMessage(), @@ -66,15 +70,32 @@ public String getAccessToken(String accessTokenSecretName, String namespace) } } - private String getOfflineTokenFromSecret(String secretName, String namespace) throws Exception { + /** + * Given a secret and a namespace load the secret and decode the value with the key "value" + * + * @param secretName name of the secret + * @param namespace namespace of the secret + * @return the 64 decoded value of namespace/secretName + * @throws ConditionAwareException if the secret does not exist + * @throws IllegalArgumentException if secret value is not in valid Base64 + */ + private String getOfflineTokenFromSecret(String secretName, String namespace) + throws ConditionAwareException { var token = k8sClient.secrets().inNamespace(namespace).withName(secretName).get(); if (token != null) { var offlineToken = token.getData().get(ACCESS_TOKEN_SECRET_KEY); + // decode may throw IllegalArgumentException offlineToken = new String(Base64.getDecoder().decode(offlineToken)); - return offlineToken; } - throw new Exception("Missing Offline Token Secret " + secretName); + // We expect the token to exist, and if it doesn't raise an exception. + throw new ConditionAwareException( + String.format("Missing Offline Token Secret %s", secretName), + null, + KafkaCondition.Type.AcccesTokenSecretValid, + KafkaCondition.Status.False, + "ConditionAwareException", + String.format("Missing Offline Token Secret %s", secretName)); } /** @@ -82,8 +103,9 @@ private String getOfflineTokenFromSecret(String secretName, String namespace) th * * @param offlineToken the token from ss.redhat.com * @return a token to be used as a bearer token to authorize the user + * @throws ConditionAwareException */ - private String exchangeToken(String offlineToken) { + private String exchangeToken(String offlineToken) throws ConditionAwareException { try { HttpRequest request = HttpRequest.newBuilder() @@ -107,10 +129,24 @@ private String exchangeToken(String offlineToken) { var json = new JsonObject(tokens); return json.getString("access_token"); } else { - throw new RuntimeException(response.body()); + LOG.log( + Level.SEVERE, String.format("Exchange token failed with error %s", response.body())); + throw new ConditionAwareException( + response.body(), + null, + KafkaCondition.Type.AcccesTokenSecretValid, + KafkaCondition.Status.False, + String.format("Http Error Code %d", response.statusCode()), + ConditionUtil.getStandarizedErrorMessage(response.statusCode())); } } catch (IOException | InterruptedException e) { - throw new RuntimeException(e); + throw new ConditionAwareException( + e.getMessage(), + e, + KafkaCondition.Type.AcccesTokenSecretValid, + KafkaCondition.Status.False, + e.getClass().getName(), + e.getMessage()); } } diff --git a/source/rhoas/src/main/java/com/openshift/cloud/beans/KafkaApiClient.java b/source/rhoas/src/main/java/com/openshift/cloud/beans/KafkaApiClient.java index c8411cfc..ca62a22b 100644 --- a/source/rhoas/src/main/java/com/openshift/cloud/beans/KafkaApiClient.java +++ b/source/rhoas/src/main/java/com/openshift/cloud/beans/KafkaApiClient.java @@ -10,6 +10,7 @@ import com.openshift.cloud.api.models.ServiceAccountRequest; import com.openshift.cloud.auth.HttpBearerAuth; import com.openshift.cloud.controllers.ConditionAwareException; +import com.openshift.cloud.controllers.ConditionUtil; import com.openshift.cloud.v1alpha.models.CloudServiceAccountRequest; import com.openshift.cloud.v1alpha.models.CloudServiceAccountRequestSpec; import com.openshift.cloud.v1alpha.models.KafkaCondition; @@ -49,7 +50,7 @@ public KafkaRequest getKafkaById(String kafkaId, String accessToken) try { return createClient(accessToken).getKafkaById(kafkaId); } catch (ApiException e) { - String message = getStandarizedErrorMessage(e); + String message = ConditionUtil.getStandarizedErrorMessage(e); throw new ConditionAwareException( message, e, @@ -64,7 +65,7 @@ public KafkaRequestList listKafkas(String accessToken) throws ConditionAwareExce try { return createClient(accessToken).listKafkas(null, null, null, null); } catch (ApiException e) { - String message = getStandarizedErrorMessage(e); + String message = ConditionUtil.getStandarizedErrorMessage(e); throw new ConditionAwareException( message, e, @@ -83,7 +84,7 @@ public ServiceAccount createServiceAccount( serviceAccountRequest.setName(spec.getServiceAccountName()); return createClient(accessToken).createServiceAccount(serviceAccountRequest); } catch (ApiException e) { - String message = getStandarizedErrorMessage(e); + String message = ConditionUtil.getStandarizedErrorMessage(e); throw new ConditionAwareException( message, e, @@ -136,26 +137,4 @@ public void createSecretForServiceAccount( e.getMessage()); } } - - private String getStandarizedErrorMessage(ApiException e) { - if (e.getCode() == 504) { - return "Server timeout. Server is not responding"; - } - if (e.getCode() == 500) { - return "Unknown server error."; - } - if (e.getCode() == 500) { - return "Unknown server error."; - } - if (e.getCode() == 400) { - return "Invalid request " + e.getMessage(); - } - if (e.getCode() == 401) { - return "Auth Token is invalid."; - } - if (e.getCode() == 403) { - return "User not authorized to access the service"; - } - return e.getMessage(); - } } diff --git a/source/rhoas/src/main/java/com/openshift/cloud/controllers/ConditionUtil.java b/source/rhoas/src/main/java/com/openshift/cloud/controllers/ConditionUtil.java index e31c6126..12b74a47 100644 --- a/source/rhoas/src/main/java/com/openshift/cloud/controllers/ConditionUtil.java +++ b/source/rhoas/src/main/java/com/openshift/cloud/controllers/ConditionUtil.java @@ -3,6 +3,7 @@ import static com.openshift.cloud.v1alpha.models.KafkaCondition.Status.False; import static com.openshift.cloud.v1alpha.models.KafkaCondition.Status.True; +import com.openshift.cloud.ApiException; import com.openshift.cloud.v1alpha.models.*; import com.openshift.cloud.v1alpha.models.KafkaCondition.Status; import java.time.ZoneOffset; @@ -222,4 +223,49 @@ public static boolean allTrue(List conditions) { ; return allTrue.get(); } + + /** + * Given an api exception, map the http error code to a known string. + * + * @param e an exception thrown by a call to the MAS API + * @return a human readable String to be set as the message property of a failed condition + */ + public static String getStandarizedErrorMessage(ApiException e) { + + switch (e.getCode()) { + case 504: // SC_GATEWAY_TIMEOUT: + case 500: // SC_INTERNAL_SERVER_ERROR: + case 401: // HttpStatus.SC_UNAUTHORIZED: + case 403: // HttpStatus.SC_FORBIDDEN: + return getStandarizedErrorMessage(e.getCode()); + case 400: // HttpStatus.SC_BAD_REQUEST: + return "Invalid request " + e.getMessage(); + default: + return e.getMessage(); + } + } + /** + * Map the http error code to a known string. + * + * @param statusCode a non 200 HTTP code returned by the system. + * @return a human readable String to be set as the message property of a failed condition + */ + public static String getStandarizedErrorMessage(int statusCode) { + switch (statusCode) { + case 504: // SC_GATEWAY_TIMEOUT: + return "Server timeout. Server is not responding"; + case 503: // SC_UNAVAILABILE + return "Service unavailable at the moment"; + case 500: // SC_INTERNAL_SERVER_ERROR: + return "Unknown server error."; + case 400: // HttpStatus.SC_BAD_REQUEST: + return "Invalid request"; + case 401: // HttpStatus.SC_UNAUTHORIZED: + return "Auth Token is invalid."; + case 403: // HttpStatus.SC_FORBIDDEN: + return "User not authorized to access the service"; + default: + return String.format("Http Error Code %d", statusCode); + } + } }