configurable read timeout for image scanning (#452)

Author: janisz

functionaltest-jenkins-plugin/resources/template.xml

@@ -21,6 +21,7 @@
             <failOnPolicyEvalFailure></failOnPolicyEvalFailure>
             <failOnCriticalPluginError></failOnCriticalPluginError>
             <enableTLSVerification></enableTLSVerification>
+            <readTimeoutSeconds></readTimeoutSeconds>
         </com.stackrox.jenkins.plugins.StackroxBuilder>
     </builders>
     <publishers/>

functionaltest-jenkins-plugin/resources/templateNoFile.xml

@@ -18,6 +18,7 @@
             <failOnPolicyEvalFailure></failOnPolicyEvalFailure>
             <failOnCriticalPluginError></failOnCriticalPluginError>
             <enableTLSVerification></enableTLSVerification>
+            <readTimeoutSeconds></readTimeoutSeconds>
             <imageNames></imageNames>
         </com.stackrox.jenkins.plugins.StackroxBuilder>
     </builders>

functionaltest-jenkins-plugin/src/main/groovy/JenkinsClient.groovy

@@ -15,6 +15,63 @@ class JenkinsClient {
     public static final String TEMPLATE_WITHOUT_IMAGE_NAMES = "resources/template.xml"
     private final JenkinsServer jenkins
 
+    static class Config {
+        String imageName
+        String portalAddress
+        String token
+        Boolean policyEvalCheck
+        Boolean failOnCriticalPluginError
+        Integer readTimeoutSeconds = null
+
+        String createJobConfig() {
+            Map<String, Serializable> param = createConfigMap()
+            // parse the xml
+            String path = TEMPLATE_WITHOUT_IMAGE_NAMES
+            return createJobConfigFromPath(path, param)
+        }
+
+        String createJobConfigNoFile() {
+            Map<String, Serializable> param = createConfigMap()
+            // parse the xml
+            String path = JOB_TEMPLATE_WITH_IMAGE_NAMES
+            return createJobConfigFromPath(path, param)
+        }
+
+        //TODO(ROX-8458): add tests for pipeline
+        Map<String, Serializable>    createConfigMap() {
+            Map<String, Serializable> configMap = [  // codenarc-disable UnnecessaryCast
+                                                     command                  : """mkdir \$BUILD_TAG
+                               cd \$BUILD_TAG
+                               echo '${imageName}' >> rox_images_to_scan""",
+                                                     portalAddress            : portalAddress,
+                                                     apiToken                 : token,
+                                                     failOnPolicyEvalFailure  : policyEvalCheck,
+                                                     failOnCriticalPluginError: failOnCriticalPluginError,
+                                                     enableTLSVerification    : false,
+                                                     imageNames               : imageName,
+            ] as Map<String, Serializable>
+
+            if (readTimeoutSeconds != null) {
+                configMap.readTimeoutSeconds = readTimeoutSeconds
+            }
+
+            return configMap
+        }
+
+        @CompileStatic(TypeCheckingMode.SKIP)
+        private static String createJobConfigFromPath(String path, Map<String, Serializable> param) {
+            def parsexml = new XmlSlurper().parse(new File(path))
+            param.each { key, value ->
+                parsexml.breadthFirst().findAll { NodeChild it ->
+                    if (it.name() == key) {
+                        it.replaceBody value
+                    }
+                }
+            }
+            return XmlUtil.serialize(parsexml)
+        }
+    }
+
     JenkinsClient() {
         def env = System.getenv()
         String jenkinsAddress = env.getOrDefault('JENKINS_ADDRESS', "http://localhost:8080/jenkins/")
@@ -45,52 +102,4 @@ class JenkinsClient {
         println result.consoleOutputText
         return result.result
     }
-
-    static String createJobConfig(String imageName, String portalAddress, String token, Boolean policyEvalCheck,
-                                  Boolean failOnCriticalPluginError) {
-        Map<String, Serializable> param = createConfigMap(
-                imageName, portalAddress, token, policyEvalCheck, failOnCriticalPluginError)
-        // parse the xml
-        String path = TEMPLATE_WITHOUT_IMAGE_NAMES
-        return createJobConfigFromPath(path, param)
-    }
-
-    static String createJobConfigNoFile(String imageName, String portalAddress, String token, Boolean policyEvalCheck,
-                                        Boolean failOnCriticalPluginError) {
-        Map<String, Serializable> param = createConfigMap(
-                imageName, portalAddress, token, policyEvalCheck, failOnCriticalPluginError)
-        // parse the xml
-        String path = JOB_TEMPLATE_WITH_IMAGE_NAMES
-        return createJobConfigFromPath(path, param)
-    }
-
-    //TODO(ROX-8458): add tests for pipeline
-    private static Map<String, Serializable> createConfigMap(String imageName, String portalAddress, String token,
-                                                             boolean policyEvalCheck,
-                                                             boolean failOnCriticalPluginError) {
-        return [  // codenarc-disable UnnecessaryCast
-                command                  : """mkdir \$BUILD_TAG
-                               cd \$BUILD_TAG
-                               echo '${imageName}' >> rox_images_to_scan""",
-                portalAddress            : portalAddress,
-                apiToken                 : token,
-                failOnPolicyEvalFailure  : policyEvalCheck,
-                failOnCriticalPluginError: failOnCriticalPluginError,
-                enableTLSVerification    : false,
-                imageNames               : imageName,
-        ] as Map<String, Serializable>
-    }
-
-    @CompileStatic(TypeCheckingMode.SKIP)
-    private static String createJobConfigFromPath(String path, Map<String, Serializable> param) {
-        def parsexml = new XmlSlurper().parse(new File(path))
-        param.each { key, value ->
-            parsexml.breadthFirst().findAll { NodeChild it ->
-                if (it.name() == key) {
-                    it.replaceBody value
-                }
-            }
-        }
-        return XmlUtil.serialize(parsexml)
-    }
 }

functionaltest-jenkins-plugin/src/test/groovy/ImageScanningTest.groovy

@@ -1,4 +1,3 @@
-import static JenkinsClient.createJobConfig
 import static com.offbytwo.jenkins.model.BuildResult.FAILURE
 import static com.offbytwo.jenkins.model.BuildResult.SUCCESS
 import static com.stackrox.model.StorageEnforcementAction.FAIL_BUILD_ENFORCEMENT
@@ -22,6 +21,15 @@ class ImageScanningTest extends BaseSpecification {
     protected static final String CENTRAL_URI = Config.roxEndpoint
     protected static final String QUAY_REPO = "quay.io/openshifttest/"
 
+    def "Test read timeout with minimal timeout should fail"() {
+        when:
+        BuildResult status = jenkins.createAndRunJob(
+                getJobConfig("nginx-alpine:latest", false, true, 1))
+
+        then:
+        assert status == FAILURE
+    }
+
     @Unroll
     def "image scanning test with toggle enforcement(#imageName, #policyName,  #enforcements, #endStatus)"() {
         given:
@@ -90,8 +98,18 @@ class ImageScanningTest extends BaseSpecification {
         "mis-spelled:lts"     | false                     | SUCCESS
     }
 
-    String getJobConfig(String imageName, Boolean policyEvalCheck, Boolean failOnCriticalPluginError) {
-        return createJobConfig(QUAY_REPO + imageName, CENTRAL_URI, token, policyEvalCheck, failOnCriticalPluginError)
+    String getJobConfig(String imageName,
+                        Boolean policyEvalCheck,
+                        Boolean failOnCriticalPluginError,
+                        Integer readTimeoutSeconds = null) {
+        return new JenkinsClient.Config(
+                imageName: QUAY_REPO + imageName,
+                portalAddress: CENTRAL_URI,
+                token: token,
+                policyEvalCheck: policyEvalCheck,
+                failOnCriticalPluginError: failOnCriticalPluginError,
+                readTimeoutSeconds: readTimeoutSeconds)
+                .createJobConfig()
     }
 
     StoragePolicy updatePolicy(String policyName, String tag, List<StorageEnforcementAction> enforcements) {

functionaltest-jenkins-plugin/src/test/groovy/ImageScanningTestNoFileTest.groovy

@@ -1,9 +1,12 @@
-import static JenkinsClient.createJobConfigNoFile
-
 class ImageScanningTestNoFileTest extends ImageScanningTest {
     @Override
     String getJobConfig(String imageName, Boolean policyEvalCheck, Boolean failOnCriticalPluginError) {
-        String image = QUAY_REPO + imageName
-        return createJobConfigNoFile(image, CENTRAL_URI, token, policyEvalCheck, failOnCriticalPluginError)
+        return new JenkinsClient.Config(
+                imageName: QUAY_REPO + imageName,
+                portalAddress: CENTRAL_URI,
+                token: token,
+                policyEvalCheck: policyEvalCheck,
+                failOnCriticalPluginError: failOnCriticalPluginError,
+                ).createJobConfigNoFile()
     }
 }

stackrox-container-image-scanner/src/main/java/com/stackrox/jenkins/plugins/StackroxBuilder.java

@@ -75,6 +75,8 @@ public class StackroxBuilder extends Builder implements SimpleBuildStep {
     private String caCertPEM;
     @DataBoundSetter
     private String cluster;
+    @DataBoundSetter
+    private int readTimeoutSeconds = ApiClientFactory.DEFAULT_READ_TIMEOUT_SECONDS;
 
     private RunConfig runConfig;
 
@@ -150,7 +152,7 @@ private List<ImageCheckResults> checkImages() throws IOException {
         List<ImageCheckResults> results = Lists.newArrayList();
 
         ApiClient apiClient = ApiClientFactory.newApiClient(
-                getPortalAddress(), getApiToken().getPlainText(), getCaCertPEM(), getTLSValidationMode());
+                getPortalAddress(), getApiToken().getPlainText(), getCaCertPEM(), getTLSValidationMode(), getReadTimeoutSeconds());
         ImageService imageService = new ImageService(apiClient);
         DetectionService detectionService = new DetectionService(apiClient);
 
@@ -249,6 +251,16 @@ public FormValidation doCheckApiToken(@QueryParameter final String apiToken) {
             }
         }
 
+        @SuppressWarnings("unused")
+        public FormValidation doCheckReadTimeoutSeconds(@QueryParameter final int readTimeoutSeconds) {
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+            if (readTimeoutSeconds > 0 && readTimeoutSeconds <= 3600) {
+                return FormValidation.ok();
+            } else {
+                return FormValidation.error("Read timeout must be between 1 and 3600 seconds.");
+            }
+        }
+
         @SuppressWarnings("unused")
         @POST
         public FormValidation doTestConnection(@QueryParameter("portalAddress") final String portalAddress, @QueryParameter("apiToken") final String apiToken,
@@ -275,7 +287,7 @@ public FormValidation doTestConnection(@QueryParameter("portalAddress") final St
         }
 
         private boolean checkRoxAuthStatus(final String portalAddress, final String apiToken, final boolean tlsVerify, final String caCertPEM) throws IOException {
-            ApiClient apiClient = ApiClientFactory.newApiClient(portalAddress, apiToken, caCertPEM, validationMode(tlsVerify));
+            ApiClient apiClient = ApiClientFactory.newApiClient(portalAddress, apiToken, caCertPEM, validationMode(tlsVerify), 10);
             try {
                 V1AuthStatus status = new AuthServiceApi(apiClient).authServiceGetAuthStatus();
                 return !Strings.isNullOrEmpty(status.getUserId());

stackrox-container-image-scanner/src/main/java/com/stackrox/jenkins/plugins/services/ApiClientFactory.java

@@ -42,14 +42,15 @@ public enum StackRoxTlsValidationMode {
         INSECURE_ACCEPT_ANY
     }
 
+    public static final int DEFAULT_READ_TIMEOUT_SECONDS = 60;
     private static final Duration TIMEOUT = Duration.ofSeconds(30);
-    private static final Duration READ_TIMEOUT = Duration.ofMinutes(10);
     private static final int MAXIMUM_CACHE_SIZE = 5; // arbitrary chosen as there are no data to support this decision
 
     @Data
     private static class CacheKey {
         private final String caCert;
         private final StackRoxTlsValidationMode tlsValidationMode;
+        private final int readTimeoutSeconds;
     }
 
     // It is good practice to avoid creating OkHttpClient on each request.
@@ -61,30 +62,33 @@ private static class CacheKey {
                     new CacheLoader<CacheKey, OkHttpClient>() {
                         @Override
                         public OkHttpClient load(@Nonnull CacheKey key) throws IOException {
-                            return newHttpClient(key.caCert, key.tlsValidationMode);
+                            return newHttpClient(key.caCert, key.tlsValidationMode, key.readTimeoutSeconds);
                         }
                     });
 
 
-    public static ApiClient newApiClient(String basePath, String apiKey, @Nullable String caCert, StackRoxTlsValidationMode tlsValidationMode) throws IOException {
-        OkHttpClient client = getClient(tlsValidationMode, caCert);
+    public static ApiClient newApiClient(String basePath, String apiKey, @Nullable String caCert, StackRoxTlsValidationMode tlsValidationMode, int readTimeoutSeconds) throws IOException {
+        OkHttpClient client = getClient(tlsValidationMode, caCert, readTimeoutSeconds);
         ApiClient apiClient = new ApiClient(client);
         apiClient.setBearerToken(apiKey);
         apiClient.setBasePath(basePath);
         return apiClient;
     }
 
     @Nonnull
-    static OkHttpClient getClient(StackRoxTlsValidationMode tlsValidationMode, @Nullable String caCert) throws IOException {
+    static OkHttpClient getClient(StackRoxTlsValidationMode tlsValidationMode, @Nullable String caCert, int readTimeoutSeconds) throws IOException {
         try {
-            return CLIENT_CACHE.get(new CacheKey(caCert, tlsValidationMode));
+            return CLIENT_CACHE.get(new CacheKey(caCert, tlsValidationMode, readTimeoutSeconds));
         } catch (ExecutionException e) {
             throw new IOException("Could not get HTTP client from cache", e);
         }
     }
 
     @Nonnull
-    private static OkHttpClient newHttpClient(@Nullable String caCert, StackRoxTlsValidationMode tlsValidationMode) throws IOException {
+    private static OkHttpClient newHttpClient(@Nullable String caCert, StackRoxTlsValidationMode tlsValidationMode, int readTimeoutSeconds) throws IOException {
+        if (readTimeoutSeconds < 1) {
+            readTimeoutSeconds = DEFAULT_READ_TIMEOUT_SECONDS;
+        }
         OkHttpClient.Builder builder;
         try {
             if (tlsValidationMode == INSECURE_ACCEPT_ANY) {
@@ -101,7 +105,7 @@ private static OkHttpClient newHttpClient(@Nullable String caCert, StackRoxTlsVa
         }
         builder.retryOnConnectionFailure(true);
         builder.connectTimeout(TIMEOUT);
-        builder.readTimeout(READ_TIMEOUT);
+        builder.readTimeout(Duration.ofSeconds(readTimeoutSeconds));
         builder.writeTimeout(TIMEOUT);
         builder.addNetworkInterceptor(new UserAgentInterceptor());
         return builder.build();

stackrox-container-image-scanner/src/main/resources/com/stackrox/jenkins/plugins/StackroxBuilder/config.jelly

@@ -16,6 +16,10 @@
              help="/plugin/stackrox-container-image-scanner/help/help-cluster.html">
         <f:textbox field="cluster"/>
     </f:entry>
+    <f:entry title="${%ReadTimeoutTitle}"
+             help="/plugin/stackrox-container-image-scanner/help/help-readTimeoutSeconds.html">
+        <f:number field="readTimeoutSeconds" min="1" max="3600" step="1" default="60"/>
+    </f:entry>
     <f:entry>
         <j:if test="${instance != null}">
             <f:optionalBlock title="${%EnableTLSVerification}" name="enableTLSVerification"

stackrox-container-image-scanner/src/main/resources/com/stackrox/jenkins/plugins/StackroxBuilder/config.properties

@@ -7,3 +7,4 @@ EnableTLSVerification=Enable TLS verification
 CACertificate=CA Certificate
 ImageNames=Image Names
 ClusterTitle=Cluster
+ReadTimeoutTitle=Read Timeout (seconds)

stackrox-container-image-scanner/src/main/webapp/help/help-readTimeoutSeconds.html

@@ -0,0 +1,4 @@
+<div>
+    HTTP read timeout in seconds for API requests to StackRox portal.
+    Increase this value if you experience timeout errors during image scans.
+</div>