Merge pull request kruize#1296 from khansaad/DSMetadata-auth-issues

Fix issues while running kruize-demos
shreyabiradar07 · Sep 20, 2024 · 995cfc6 · 995cfc6
2 parents 37fd1df + fd15d34
commit 995cfc6
Show file tree

Hide file tree

Showing 13 changed files with 71 additions and 42 deletions.
diff --git a/manifests/crc/default-db-included-installation/openshift/kruize-crc-openshift.yaml b/manifests/crc/default-db-included-installation/openshift/kruize-crc-openshift.yaml
@@ -469,16 +469,3 @@ spec:
     - name: nginx-config-volume
       configMap:
         name: nginx-config
----
-apiVersion: rbac.authorization.k8s.io/v1
-kind: ClusterRoleBinding
-metadata:
-  name: kruize-monitoring-view
-subjects:
-  - kind: ServiceAccount
-    name: kruize-sa
-    namespace: openshift-tuning
-roleRef:
-  kind: ClusterRole
-  name: cluster-monitoring-view
-  apiGroup: rbac.authorization.k8s.io
diff --git a/src/main/java/com/autotune/analyzer/recommendations/engine/RecommendationEngine.java b/src/main/java/com/autotune/analyzer/recommendations/engine/RecommendationEngine.java
@@ -25,7 +25,6 @@
 import com.autotune.common.data.result.IntervalResults;
 import com.autotune.common.data.result.NamespaceData;
 import com.autotune.common.datasource.DataSourceInfo;
-import com.autotune.common.auth.AuthenticationConfig;
 import com.autotune.common.auth.AuthenticationStrategy;
 import com.autotune.common.auth.AuthenticationStrategyFactory;
 import com.autotune.common.exceptions.DataSourceNotExist;
@@ -45,9 +44,6 @@
 import org.slf4j.LoggerFactory;
 
 import javax.servlet.http.HttpServletResponse;
-import java.io.File;
-import java.io.FileWriter;
-import java.io.IOException;
 import java.lang.reflect.Method;
 import java.net.URLEncoder;
 import java.sql.Timestamp;
@@ -1771,9 +1767,11 @@ private String getResults(Map<String, KruizeObject> mainKruizeExperimentMAP, Kru
             }
         } else if (kruizeObject.getExperiment_usecase_type().isLocal_monitoring()) {
             // get data from the provided datasource in case of local monitoring
-            DataSourceInfo dataSourceInfo = new ExperimentDBService().loadDataSourceFromDBByName(dataSource);
-            if (dataSourceInfo == null) {
-                throw new DataSourceNotExist(KruizeConstants.DataSourceConstants.DataSourceErrorMsgs.MISSING_DATASOURCE_INFO);
+            DataSourceInfo dataSourceInfo;
+            try {
+                dataSourceInfo = CommonUtils.getDataSourceInfo(dataSource);
+            } catch (Exception e) {
+                throw new DataSourceNotExist(e.getMessage());
             }
             // Fetch metrics dynamically from Metric Profile based on the datasource
             fetchMetricsBasedOnProfileAndDatasource(kruizeObject, interval_end_time, intervalStartTime, dataSourceInfo);
@@ -1975,10 +1973,8 @@ public void fetchMetricsBasedOnProfileAndDatasource(KruizeObject kruizeObject, T
             long interval_end_time_epoc = 0;
             long interval_start_time_epoc = 0;
             SimpleDateFormat sdf = new SimpleDateFormat(KruizeConstants.DateFormats.STANDARD_JSON_DATE_FORMAT, Locale.ROOT);
-            AuthenticationStrategy authenticationStrategy = AuthenticationStrategyFactory.createAuthenticationStrategy(
-                    dataSourceInfo.getAuthenticationConfig());
             // Create the client
-            GenericRestApiClient client = new GenericRestApiClient(authenticationStrategy);
+            GenericRestApiClient client = new GenericRestApiClient(dataSourceInfo);
 
             String metricProfileName = kruizeObject.getPerformanceProfile();
             PerformanceProfile metricProfile = MetricProfileCollection.getInstance().getMetricProfileCollection().get(metricProfileName);

diff --git a/src/main/java/com/autotune/analyzer/services/CreateExperiment.java b/src/main/java/com/autotune/analyzer/services/CreateExperiment.java
@@ -180,7 +180,8 @@ protected void doDelete(HttpServletRequest request, HttpServletResponse response
                 sendSuccessResponse(response, "Experiment deleted successfully.");
             }
         } catch (Exception e) {
-            sendErrorResponse(inputData, response, e, HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
+            LOGGER.error("Exception occurred while deleting the experiment: {}", e.getMessage());
+            sendErrorResponse(inputData, response, null, HttpServletResponse.SC_BAD_REQUEST, e.getMessage());
         }
     }
 

diff --git a/src/main/java/com/autotune/analyzer/services/DSMetadataService.java b/src/main/java/com/autotune/analyzer/services/DSMetadataService.java
@@ -26,6 +26,7 @@
 import com.autotune.common.datasource.DataSourceInfo;
 import com.autotune.common.datasource.DataSourceManager;
 import com.autotune.common.datasource.DataSourceMetadataValidation;
+import com.autotune.common.utils.CommonUtils;
 import com.autotune.database.service.ExperimentDBService;
 import com.autotune.utils.KruizeConstants;
 import com.autotune.utils.KruizeSupportedTypes;
@@ -114,10 +115,11 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
             if (validationOutputData.isSuccess()) {
 
                 String dataSourceName = metadataAPIObject.getDataSourceName();
-
-                DataSourceInfo datasource = dataSourceManager.fetchDataSourceFromDBByName(dataSourceName);
-
-                if (datasource == null) {
+                // fetch the DatasourceInfo object based on datasource name
+                DataSourceInfo datasource;
+                try {
+                    datasource = CommonUtils.getDataSourceInfo(dataSourceName);
+                } catch (Exception e) {
                     sendErrorResponse(
                             inputData,
                             response,

diff --git a/src/main/java/com/autotune/common/auth/AuthenticationStrategyFactory.java b/src/main/java/com/autotune/common/auth/AuthenticationStrategyFactory.java
@@ -7,6 +7,8 @@ public class AuthenticationStrategyFactory {
     public static AuthenticationStrategy createAuthenticationStrategy(AuthenticationConfig authConfig) {
         String type = authConfig.getType();
         switch (type) {
+            case KruizeConstants.AuthenticationConstants.NONE:
+                return new NoAuthStrategy();
             case KruizeConstants.AuthenticationConstants.BASIC:
                 String username = authConfig.getCredentials().getUsername();
                 String password = authConfig.getCredentials().getPassword();

diff --git a/src/main/java/com/autotune/common/auth/NoAuthStrategy.java b/src/main/java/com/autotune/common/auth/NoAuthStrategy.java
@@ -0,0 +1,9 @@
+package com.autotune.common.auth;
+
+public class NoAuthStrategy implements AuthenticationStrategy {
+    @Override
+    public String applyAuthentication() {
+        // No authentication is applied
+        return null;
+    }
+}
diff --git a/src/main/java/com/autotune/common/datasource/DataSourceCollection.java b/src/main/java/com/autotune/common/datasource/DataSourceCollection.java
@@ -153,10 +153,17 @@ public void addDataSourcesFromConfigFile(String configFileName) {
                 String serviceName = dataSourceObject.getString(KruizeConstants.DataSourceConstants.DATASOURCE_SERVICE_NAME);
                 String namespace = dataSourceObject.getString(KruizeConstants.DataSourceConstants.DATASOURCE_SERVICE_NAMESPACE);
                 String dataSourceURL = dataSourceObject.getString(KruizeConstants.DataSourceConstants.DATASOURCE_URL);
-                JSONObject authenticationObj = dataSourceObject.optJSONObject(KruizeConstants.AuthenticationConstants.AUTHENTICATION);
-                // create the corresponding authentication object
-                AuthenticationConfig authConfig = AuthenticationConfig.createAuthenticationConfigObject(authenticationObj);
-                DataSourceInfo datasource = null;
+                AuthenticationConfig authConfig;
+                try {
+                    JSONObject authenticationObj = dataSourceObject.optJSONObject(KruizeConstants.AuthenticationConstants.AUTHENTICATION);
+                    // create the corresponding authentication object
+                    authConfig = AuthenticationConfig.createAuthenticationConfigObject(authenticationObj);
+                } catch (Exception e) {
+                    LOGGER.warn("Auth details are missing for datasource: {}", name);
+                    authConfig = AuthenticationConfig.noAuth();
+                }
+
+                DataSourceInfo datasource;
                 // Validate input
                 if (!validateInput(name, provider, serviceName, dataSourceURL, namespace)) {
                     continue;

diff --git a/src/main/java/com/autotune/common/datasource/DataSourceInfo.java b/src/main/java/com/autotune/common/datasource/DataSourceInfo.java
@@ -135,6 +135,7 @@ public String toString() {
                 ", serviceName='" + serviceName + '\'' +
                 ", namespace='" + namespace + '\'' +
                 ", url=" + url +
+                ", authenticationConfig=" + authenticationConfig +
                 '}';
     }
 }
diff --git a/src/main/java/com/autotune/common/datasource/DataSourceOperatorImpl.java b/src/main/java/com/autotune/common/datasource/DataSourceOperatorImpl.java
@@ -165,10 +165,8 @@ public ArrayList<String> getAppsForLayer(DataSourceInfo dataSource, String query
         String queryURL = dataSourceURL + queryEndpoint + query;
         LOGGER.debug("Query URL is: {}", queryURL);
         try {
-            AuthenticationStrategy authenticationStrategy = AuthenticationStrategyFactory.createAuthenticationStrategy(
-                    dataSource.getAuthenticationConfig());
             // Create the client
-            GenericRestApiClient genericRestApiClient = new GenericRestApiClient(authenticationStrategy);
+            GenericRestApiClient genericRestApiClient = new GenericRestApiClient(dataSource);
             genericRestApiClient.setBaseURL(dataSourceURL + queryEndpoint);
             JSONObject responseJson = genericRestApiClient.fetchMetricsJson("GET", query);
             int level = 0;

diff --git a/src/main/java/com/autotune/common/datasource/prometheus/PrometheusDataOperatorImpl.java b/src/main/java/com/autotune/common/datasource/prometheus/PrometheusDataOperatorImpl.java
@@ -143,10 +143,8 @@ public Object getValueForQuery(DataSourceInfo dataSource, String query) {
      */
     @Override
     public JSONObject getJsonObjectForQuery(DataSourceInfo dataSource, String query) {
-        AuthenticationStrategy authenticationStrategy = AuthenticationStrategyFactory.createAuthenticationStrategy(
-                dataSource.getAuthenticationConfig());
         // Create the client
-        GenericRestApiClient apiClient = new GenericRestApiClient(authenticationStrategy);
+        GenericRestApiClient apiClient = new GenericRestApiClient(dataSource);
         apiClient.setBaseURL(CommonUtils.getBaseDataSourceUrl(
                 dataSource,
                 KruizeConstants.SupportedDatasources.PROMETHEUS

diff --git a/src/main/java/com/autotune/common/utils/CommonUtils.java b/src/main/java/com/autotune/common/utils/CommonUtils.java
@@ -16,7 +16,9 @@
 
 package com.autotune.common.utils;
 
+import com.autotune.common.datasource.DataSourceCollection;
 import com.autotune.common.datasource.DataSourceInfo;
+import com.autotune.common.datasource.DataSourceManager;
 import com.autotune.utils.KruizeConstants;
 
 import java.sql.Timestamp;
@@ -293,4 +295,24 @@ public static double getPercentage(double newer, double older) {
 
         return ((newer - older)/older) * 100;
     }
+
+    public static DataSourceInfo getDataSourceInfo(String dataSourceName) throws Exception {
+        DataSourceManager dataSourceManager = new DataSourceManager();
+        // fetch the datasource from the config file first
+        DataSourceInfo datasource = DataSourceCollection.getInstance().getDataSourcesCollection().get(dataSourceName);
+        if (isInvalidDataSource(datasource)) {
+            // fetch the datasource from the DB
+            datasource = dataSourceManager.fetchDataSourceFromDBByName(dataSourceName);
+            if (isInvalidDataSource(datasource)) {
+                throw new Exception(KruizeConstants.DataSourceConstants.DataSourceErrorMsgs.INVALID_DATASOURCE_INFO);
+            }
+        }
+        return datasource;
+    }
+
+    // Helper method to validate the DataSourceInfo object
+    public static boolean isInvalidDataSource(DataSourceInfo datasource) {
+        return datasource == null || datasource.getAuthenticationConfig() == null ||
+                datasource.getAuthenticationConfig().toString().isEmpty();
+    }
 }
diff --git a/src/main/java/com/autotune/utils/GenericRestApiClient.java b/src/main/java/com/autotune/utils/GenericRestApiClient.java
@@ -16,6 +16,8 @@
 package com.autotune.utils;
 
 import com.autotune.common.auth.AuthenticationStrategy;
+import com.autotune.common.auth.AuthenticationStrategyFactory;
+import com.autotune.common.datasource.DataSourceInfo;
 import com.autotune.utils.authModels.APIKeysAuthentication;
 import com.autotune.utils.authModels.BasicAuthentication;
 import com.autotune.utils.authModels.BearerAccessToken;
@@ -57,11 +59,13 @@ public class GenericRestApiClient {
     private AuthenticationStrategy authenticationStrategy;
 
     /**
-     * Initializes a new instance just by passing baseURL which does not need any authentication.
-     * @param authenticationStrategy
+     * constructor to set the authentication based on the datasourceInfo object
+     * @param dataSourceInfo object containing the datasource details
      */
-    public GenericRestApiClient(AuthenticationStrategy authenticationStrategy) {
-        this.authenticationStrategy = authenticationStrategy;
+    public GenericRestApiClient(DataSourceInfo dataSourceInfo) {
+        // TODO: add partial URL as well as part of this constructor
+        this.authenticationStrategy = AuthenticationStrategyFactory.createAuthenticationStrategy(
+                dataSourceInfo.getAuthenticationConfig());
     }
 
     /**

diff --git a/src/main/java/com/autotune/utils/KruizeConstants.java b/src/main/java/com/autotune/utils/KruizeConstants.java
@@ -446,6 +446,7 @@ public static class DataSourceErrorMsgs {
             public static final String SERVICE_NOT_FOUND = "Can not find service with specified name.";
             public static final String ENDPOINT_NOT_FOUND = "Service endpoint not found.";
             public static final String MISSING_DATASOURCE_INFO = "Datasource is missing, add a valid Datasource";
+            public static final String INVALID_DATASOURCE_INFO = "Datasource is either missing or is invalid";
             private DataSourceErrorMsgs() {
             }
         }
@@ -737,6 +738,7 @@ public static final class AuthenticationConstants {
         public static final String AUTHENTICATION_CLIENT_ID = "clientId";
         public static final String AUTHENTICATION_CLIENT_SECRET = "clientSecret";
         public static final String AUTHENTICATION_GRANT_TYPE = "grantType";
+        public static final String NONE = "none";
         public static final String BASIC = "basic";
         public static final String BEARER = "bearer";
         public static final String API_KEY = "apikey";