chore: Improve conformance validation (#3023)

* feat(gateway): Improvement of existing conformance testing Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * refactor(gateway): Added missing comments Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Removed a print statement. Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * feat(gateway): Better response from the conformance endpoint Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * feat(gateway): Better response from the conformance endpoint Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * refactor(gateway): Got rid of duplicate discovery client calls Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * docs(gateway): Changed text in gateway-log-messages.yml based on comments on the PR Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * feat(gateway): Addressed the rest of comments on the PR Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * feat(gateway): Addressed the new comments on the PR Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Fixed a method not following a naming convention Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Fixed reason why build was failing Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Addressed Pablo's comments on PR Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Updated an integration test for conformance Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Updated integration tests Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * feat(gateway): Improvement of existing conformance testing Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * refactor(gateway): Added missing comments Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Removed a print statement. Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * feat(gateway): Better response from the conformance endpoint Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * refactor(gateway): Got rid of duplicate discovery client calls Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * docs(gateway): Changed text in gateway-log-messages.yml based on comments on the PR Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * feat(gateway): Addressed the rest of comments on the PR Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * feat(gateway): Addressed the new comments on the PR Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Fixed a method not following a naming convention Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Fixed reason why build was failing Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Addressed Pablo's comments on PR Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Updated an integration test for conformance Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Updated integration tests Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Fixed code smells Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Fixed 1 bug from sonar Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Disabled caching on a github action as a test Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Enabled cache again + removed some tests to see if they are the reason why the build fails Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Enabled cache again + removed some tests to see if they are the reason why the build fails Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Enabled cache again + removed some tests to see if they are the reason why the build fails Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Disabled cache again, changed unit tests :gateway-service:test shouldn't fail anymore Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Disabled cache again, changed unit tests :gateway-service:test shouldn't fail anymore Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Modified unit tests Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Modified unit tests Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Modified unit tests Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Fixed unit tests Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Enabled build cache Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Disabled build cache Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Enabled build cache + should create an artifact after failing Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> * fix(gateway): Disabled the creation of artifact from previous commit Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> --------- Signed-off-by: matejpopda <20053580+matejpopda@users.noreply.github.com> Co-authored-by: ShobhaJayanna <36433611+Shobhajayanna@users.noreply.github.com> Co-authored-by: Andrea Tabone <39694626+taban03@users.noreply.github.com>
zowe · Aug 11, 2023 · 4bb9df9 · 4bb9df9
1 parent 88942d4
commit 4bb9df9
Show file tree

Hide file tree

Showing 8 changed files with 739 additions and 228 deletions.
diff --git a/...ervice/src/main/java/org/zowe/apiml/gateway/conformance/ConformanceProblemsContainer.java b/...ervice/src/main/java/org/zowe/apiml/gateway/conformance/ConformanceProblemsContainer.java
@@ -0,0 +1,141 @@
+/*
+ * This program and the accompanying materials are made available under the terms of the
+ * Eclipse Public License v2.0 which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-v20.html
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Copyright Contributors to the Zowe Project.
+ */
+
+package org.zowe.apiml.gateway.conformance;
+
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.lang.text.StrSubstitutor;
+import org.zowe.apiml.message.api.ApiMessage;
+
+import java.util.*;
+
+
+/**
+ * Java class that is used to keep track of found conformance issues
+ */
+public class ConformanceProblemsContainer extends HashMap<String, ArrayList<String>> {
+
+
+    private final String serviceId;
+    private static final String RESPONSE_MESSAGE_TEMPLATE = "{\n" + "\"messageAction\": \"${messageAction}\",\n" + "\"messageContent\": {\n" + "    \"The service ${serviceId} is not conformant\": \n" + "        ${messageContent}\n" + "},\n" + "\"messageKey\": \"${messageKey}\",\n" + "\"messageNumber\": \"${messageNumber}\",\n" + "\"messageReason\": \"${messageReason}\",\n" + "\"messageType\": \"${messageType}\"\n" + "}";
+
+    ConformanceProblemsContainer(String serviceId) {
+        super();
+        this.serviceId = serviceId;
+    }
+
+    @Override
+    public ArrayList<String> put(String key, ArrayList<String> values) {
+        if (values == null) {
+            return new ArrayList<>();
+        }
+        if (this.get(key) == null || this.get(key).isEmpty()) {
+            return super.put(key, new ArrayList<>(values));
+        }
+        for (String value : values) {
+            if (this.get(key).contains(value)) {
+                this.get(key).add(value);
+            }
+        }
+        return new ArrayList<>();
+    }
+
+    public List<String> put(String key, String value) {
+        if (value == null || value.equals("")) {
+            return new ArrayList<>();
+        }
+        return put(key, new ArrayList<>(Collections.singleton(value)));
+    }
+
+    public List<String> put(String key, List<String> value) {
+        return put(key, new ArrayList<>(value));
+    }
+
+    @Override
+    public int size() {
+        int result = 0;
+        for (ArrayList<String> value : this.values()) {
+            if (value == null) {
+                continue;
+            }
+            result += value.size();
+        }
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        return super.equals(o);
+    }
+
+    @Override
+    public int hashCode() {
+        return super.hashCode();
+    }
+
+
+    @Override
+    public String toString() {
+        StringBuilder result = new StringBuilder();
+        result.append("{");
+        boolean firstLoop = true;
+
+        ArrayList<String> sortedKeySet = new ArrayList<>(this.keySet());
+        sortedKeySet.remove(null);  // since it used to be a set this removes all nulls
+        sortedKeySet.sort(null);
+        for (String key : sortedKeySet) {
+
+            if (this.get(key) == null || this.get(key).isEmpty()) {
+                continue;
+            }
+
+            if (!firstLoop) {
+                result.append(",");
+            }
+
+            result.append("\"").append(key).append("\"");
+            result.append(":[");
+
+            boolean firstInnerLoop = true;
+            for (String i : get(key)) {
+                if (!firstInnerLoop) {
+                    result.append(",");
+                }
+                try {
+                    result.append(new ObjectMapper().writeValueAsString(i));
+                } catch (JsonProcessingException e) {
+                    continue;
+                }
+                firstInnerLoop = false;
+            }
+            result.append("]");
+            firstLoop = false;
+        }
+        result.append("}");
+        return result.toString();
+    }
+
+
+    public String createBadRequestAPIResponseBody(String key, ApiMessage correspondingAPIMessage) {
+        Map<String, String> valuesMap = new HashMap<>();
+
+        valuesMap.put("messageKey", key);
+        valuesMap.put("messageContent", this.toString());
+        valuesMap.put("serviceId", serviceId);
+        valuesMap.put("messageReason", correspondingAPIMessage.getMessageReason());
+        valuesMap.put("messageNumber", correspondingAPIMessage.getMessageNumber());
+        valuesMap.put("messageType", correspondingAPIMessage.getMessageType().toString());
+        valuesMap.put("messageAction", correspondingAPIMessage.getMessageAction());
+
+        return new StrSubstitutor(valuesMap).replace(RESPONSE_MESSAGE_TEMPLATE);
+    }
+}
diff --git a/gateway-service/src/main/java/org/zowe/apiml/gateway/conformance/ValidateAPIController.java b/gateway-service/src/main/java/org/zowe/apiml/gateway/conformance/ValidateAPIController.java
@@ -10,96 +10,197 @@
 
 package org.zowe.apiml.gateway.conformance;
 
-import java.util.regex.Pattern;
-import java.util.function.Predicate;
-import java.util.regex.Matcher;
-
+import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand;
+import lombok.RequiredArgsConstructor;
+import org.springframework.cloud.client.ServiceInstance;
+import org.springframework.cloud.client.discovery.DiscoveryClient;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
-import org.springframework.stereotype.Controller;
-import org.springframework.http.HttpStatus;
-import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.*;
+import org.zowe.apiml.message.core.Message;
 import org.zowe.apiml.message.core.MessageService;
 
-import io.swagger.v3.oas.annotations.parameters.RequestBody;
-import lombok.RequiredArgsConstructor;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
- * Controller offered methods for validating serviceID under conformance criteria, it offer methods to
- * check the validation of the given serviceID
+ * Controller offers method to check the validation of the given serviceID under conformance criteria
  */
-@Controller
+@RestController
 @RequiredArgsConstructor
 public class ValidateAPIController {
 
-    private static final Pattern symbolPattern = Pattern.compile("[^a-z0-9]");
-    private static final Predicate<String> isTooLong = serviceId -> (serviceId).length() <= 64;
+    private static final int MAXIMUM_SERVICE_ID_LENGTH = 64;
+    private static final String INVALID_SERVICE_ID_REGEX_PATTERN = "[^a-z0-9]";
+
+
+    private static final String WRONG_SERVICE_ID_KEY = "org.zowe.apiml.gateway.verifier.wrongServiceId";
+    private static final String NO_METADATA_KEY = "org.zowe.apiml.gateway.verifier.noMetadata";
+    private static final String NON_CONFORMANT_KEY = "org.zowe.apiml.gateway.verifier.nonConformant";
+
+
+    private static final String REGISTRATION_PROBLEMS = "Registration problems";
+    private static final String METADATA_PROBLEMS = "Metadata problems";
+    private static final String CONFORMANCE_PROBLEMS = "Conformance problems";
+
+
     private final MessageService messageService;
 
     private final VerificationOnboardService verificationOnboardService;
-    private String invalidKey = "org.zowe.apiml.gateway.verifier.wrongServiceId";
+
+    private final DiscoveryClient discoveryClient;
+
 
     /**
-     * Accept serviceID and return the JSON file with appropirate message to show if it is valid
-     *
-     * @param serviceID accepted serviceID to check validation
-     * @return return the JSON file message of whether the serviceID is valid
+     * Accepts serviceID and checks conformance criteria
      *
+     * @param serviceId accepted serviceID to check for conformance
+     * @return 200 if service is conformant, 400 + JSON explanation if not
      */
-    @PostMapping(
-        value = "/validate",
+    @GetMapping(
+        value = "/gateway/conformance/{serviceId}",
         produces = MediaType.APPLICATION_JSON_VALUE
     )
-    public ResponseEntity<Object> checkValidate(@RequestBody String serviceID) {
+    @HystrixCommand
+    public ResponseEntity<String> checkConformance(@PathVariable String serviceId) {
+        ConformanceProblemsContainer foundNonConformanceIssues = new ConformanceProblemsContainer(serviceId);
+        foundNonConformanceIssues.put(CONFORMANCE_PROBLEMS, validateServiceIdFormat(serviceId));
+
+        if (foundNonConformanceIssues.size() != 0) {
+            return generateBadRequestResponseEntity(NON_CONFORMANT_KEY, foundNonConformanceIssues);
+        }
 
-        String message = validator(serviceID);
+        foundNonConformanceIssues.put(REGISTRATION_PROBLEMS, checkOnboarding(serviceId));
+        if (foundNonConformanceIssues.size() != 0) {     // cant continue if a service isn't registered
+            return generateBadRequestResponseEntity(WRONG_SERVICE_ID_KEY, foundNonConformanceIssues);
+        }
 
-        if (!message.isEmpty()) {
-            return new ResponseEntity<>(messageService.createMessage(invalidKey, message).mapToApiMessage(), HttpStatus.BAD_REQUEST);
+        List<ServiceInstance> serviceInstances = discoveryClient.getInstances(serviceId);
+        foundNonConformanceIssues.put(REGISTRATION_PROBLEMS, instanceCheck(serviceInstances));
+        if (foundNonConformanceIssues.size() != 0) {     // cant continue if we cant retrieve an instance
+            return generateBadRequestResponseEntity(WRONG_SERVICE_ID_KEY, foundNonConformanceIssues);
         }
 
-        return new ResponseEntity<>(HttpStatus.OK);
+        ServiceInstance serviceInstance = serviceInstances.get(0);
+        Map<String, String> metadata = getMetadata(serviceInstance);
+
+        foundNonConformanceIssues.put(METADATA_PROBLEMS, metaDataCheck(metadata));
+        if (foundNonConformanceIssues.size() != 0) {     // cant continue without metadata
+            return generateBadRequestResponseEntity(NO_METADATA_KEY, foundNonConformanceIssues);
+        }
 
+        return new ResponseEntity<>("{\"message\":\"Service " + serviceId + " fulfills all checked conformance criteria\"}", HttpStatus.OK);
+    }
+
+
+    /**
+     * Mapping so the old endpoint keeps working.
+     *
+     * @param serviceId serviceId to check for conformance
+     * @return 200 if service is conformant, 400 + JSON explanation if not
+     */
+    @PostMapping(value = "/validate", produces = MediaType.APPLICATION_JSON_VALUE)
+    @HystrixCommand
+    public ResponseEntity<String> checkValidateLegacy(@RequestBody String serviceId) {
+        if (serviceId.startsWith("serviceID")) {
+            serviceId = serviceId.replace("serviceID=", "");
+        }
+        return checkConformance(serviceId);
     }
 
+
     /**
-     * Accept serviceID and check if it contains only lower case characters without symbols, return true if it meets the criteria
-     * otherwise return false
+     * Creates a response when a conformance criteria is failed.
      *
-     * @param serviceID accept serviceID to check
-     * @return return boolean variable False if it only contains lower case characters without symbols
+     * @param foundNonConformanceIssues list of found issues
+     * @return Response that this controller returns
      */
-    private boolean checkValidPatternAPI(String serviceID) {
+    private ResponseEntity<String> generateBadRequestResponseEntity(String key, ConformanceProblemsContainer foundNonConformanceIssues) {
+        Message message = messageService.createMessage(key, "ThisWillBeRemoved");
+        return new ResponseEntity<>(foundNonConformanceIssues.createBadRequestAPIResponseBody(key, message.mapToApiMessage()), HttpStatus.BAD_REQUEST);
+    }
 
-        Matcher findSymbol = symbolPattern.matcher(serviceID);
 
-        return findSymbol.find();
+    /**
+     * Accepts serviceId and checks if the service is onboarded to the API Mediation Layer
+     * If it's not than it doesn't fulfill Item 1 of conformance criteria
+     *
+     * @param serviceId serviceId to check
+     * @return string describing the issue or an empty string
+     */
+    public String checkOnboarding(String serviceId) {
+        if (!verificationOnboardService.checkOnboarding(serviceId)) {
+            return "The service is not registered";
+        }
+        return "";
+
     }
 
 
     /**
-     * Accept serviceId and check conformant criteria and return invalid message if it does not meet
-     * the requirement
+     * Retrieves metadata
      *
-     * @param serviceId accept serviceID to check
-     * @return return invalid message if it is wrong otherwise an empty string
+     * @param serviceInstance serviceInstance from which to retrieve the metadata.
+     * @return Metadata of the instance
      */
-    private String validator(String serviceId) {
+    private Map<String, String> getMetadata(ServiceInstance serviceInstance) {
+        return serviceInstance.getMetadata();
+    }
 
-        if (!isTooLong.test(serviceId)) {
-            return "The serviceid is longer than 64 characters";
+
+    /**
+     * Checks if metadata was retrieved.
+     *
+     * @param metadata which to test
+     * @return string describing the issue or an empty string
+     */
+    public String metaDataCheck(Map<String, String> metadata) {
+        if (metadata != null && !metadata.isEmpty()) {
+            return "";
         }
-        else if (checkValidPatternAPI(serviceId)) {
-            return "The serviceid contains symbols or upper case letters";
+        return "Cannot Retrieve MetaData";
+    }
+
+    /**
+     * Checks if a single instance can be retrieved.
+     *
+     * @param serviceInstances to check
+     * @return string describing the issue or an empty string
+     */
+    public String instanceCheck(List<ServiceInstance> serviceInstances) {
+        if (serviceInstances.isEmpty()) {
+            return "Cannot retrieve metadata - no active instance of the service";
         }
-        else if (!verificationOnboardService.checkOnboarding(serviceId)) {
-            return "The service is not registered";
+        return "";
+    }
+
+
+    /**
+     * Accept serviceId and checks if it is Zowe conformant according to the specification,
+     * Item 5 from the conformance criteria list. That means that the serviceId contains only lower case
+     * characters without symbols and is shorter than 64 characters
+     *
+     * @param serviceId to check
+     * @return list of found issues, empty when conformant
+     */
+    public List<String> validateServiceIdFormat(String serviceId) {
+        ArrayList<String> result = new ArrayList<>();
+        if (serviceId.length() > MAXIMUM_SERVICE_ID_LENGTH) {
+            result.add("The serviceId is longer than 64 characters");
         }
-        else if (verificationOnboardService.retrieveMetaData(serviceId).isEmpty()) {
-            return "Cannot Retrieve MetaData";
+        // Check for invalid characters
+        final Pattern symbolPattern = Pattern.compile(INVALID_SERVICE_ID_REGEX_PATTERN);
+        Matcher findSymbol = symbolPattern.matcher(serviceId);
+        if (findSymbol.find()) {
+            result.add("The serviceId contains symbols or upper case letters");
         }
 
-        return "";
+        return result;
     }
-
+
+
 }