Merge pull request #501 from validatedpatterns/main

sync: main to rhdp-deploy

Author: mbaldessari

.github/workflows/ansible-lint.yml

@@ -15,4 +15,4 @@ jobs:
           persist-credentials: false
 
       - name: Lint Ansible Playbook
-        uses: ansible/ansible-lint@43e758bad47344f1ce7b699c0020299f486a8026
+        uses: ansible/ansible-lint@7f6abc5ef97d0fb043a0f3d416dfbc74399fbda0

ansible.cfg

@@ -1,6 +1,9 @@
 [defaults]
 localhost_warning=False
 retry_files_enabled=False
+# Retry files disabled to avoid cluttering CI/CD environments
+interpreter_python=auto_silent
+timeout=30
 library=~/.ansible/plugins/modules:./ansible/plugins/modules:./common/ansible/plugins/modules:/usr/share/ansible/plugins/modules
 roles_path=~/.ansible/roles:./ansible/roles:./common/ansible/roles:/usr/share/ansible/roles:/etc/ansible/roles
 filter_plugins=~/.ansible/plugins/filter:./ansible/plugins/filter:./common/ansible/plugins/filter:/usr/share/ansible/plugins/filter

ansible/site.yaml

@@ -3,15 +3,31 @@
   hosts: localhost
   connection: local
   tasks:
+    - name: Verify pattern.sh exists
+      ansible.builtin.stat:
+        path: "{{ playbook_dir }}/../pattern.sh"
+      register: pattern_script
+
+    - name: Fail if pattern.sh does not exist
+      ansible.builtin.fail:
+        msg: "pattern.sh not found at {{ playbook_dir }}/../pattern.sh"
+      when: not pattern_script.stat.exists
+
     # We cannot use .package or .dnf modules because python3 that is used comes
     # from a virtualenv
     - name: Launch the installation
       ansible.builtin.command: ./pattern.sh make install
       args:
-        chdir: "{{ lookup('env', 'PWD') }}"
+        chdir: "{{ playbook_dir }}/.."
       register: output
-      changed_when: false
+      changed_when: output.rc == 0
+      failed_when: output.rc != 0
 
     - name: Print output of installation
       ansible.builtin.debug:
-        msg: "{{ output }}"
+        msg: "{{ output.stdout_lines }}"
+
+    - name: Print errors if any
+      ansible.builtin.debug:
+        msg: "{{ output.stderr_lines }}"
+      when: output.stderr_lines | length > 0

charts/all/config-demo/templates/config-demo-deployment.yaml

@@ -12,7 +12,6 @@ spec:
       deploymentconfig: config-demo
   template:
     metadata:
-      creationTimestamp: null
       labels:
         app: config-demo
         deploymentconfig: config-demo
@@ -24,8 +23,8 @@ spec:
           type: RuntimeDefault
       containers:
       - name: apache
-        image: registry.access.redhat.com/ubi10/httpd-24:10.0-1755779646
-        #imagePullPolicy: Always
+        image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
+        imagePullPolicy: {{ .Values.image.pullPolicy }}
         ports:
         - containerPort: 8080
           name: http

charts/all/config-demo/values.yaml

@@ -13,3 +13,8 @@ global:
 
 clusterGroup:
   isHubCluster: true
+
+image:
+  repository: registry.access.redhat.com/ubi10/httpd-24
+  tag: "10.0-1755779646"
+  pullPolicy: IfNotPresent

charts/all/hello-world/templates/hello-world-deployment.yaml

@@ -23,8 +23,8 @@ spec:
           type: RuntimeDefault
       containers:
       - name: apache
-        image: registry.access.redhat.com/ubi10/httpd-24:10.0-1755779646
-        #imagePullPolicy: Always
+        image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
+        imagePullPolicy: {{ .Values.image.pullPolicy }}
         ports:
         - containerPort: 8080
           name: http

charts/all/hello-world/values.yaml

@@ -1,4 +1,9 @@
 ---
 global:
   hubClusterDomain: hub.example.com
-  localCluster: local.example.com
+  localClusterDomain: local.example.com
+
+image:
+  repository: registry.access.redhat.com/ubi10/httpd-24
+  tag: "10.0-1755779646"
+  pullPolicy: IfNotPresent

overrides/values-AWS.yaml

@@ -2,7 +2,7 @@
 # to enable letsencrypt certificates on API endpoint and default
 # ingress of the cluster
 # It is currently very experimental and unsupported.
-# PLEASE read https://github.com/hybrid-cloud-patterns/common/tree/main/letsencrypt#readme
+# PLEASE read https://github.com/validatedpatterns/common/tree/main/letsencrypt#readme
 # for all the limitations around it
 
 

pattern.sh

@@ -1,19 +1,20 @@
 #!/bin/bash
+set -euo pipefail
 
 function is_available {
-  command -v $1 >/dev/null 2>&1 || { echo >&2 "$1 is required but it's not installed. Aborting."; exit 1; }
+  command -v "$1" >/dev/null 2>&1 || { echo >&2 "$1 is required but it's not installed. Aborting."; exit 1; }
 }
 
 function version {
-    echo "$@" | awk -F. '{ printf("%d%03d%03d%03d\n", $1,$2,$3,$4); }'
+    echo "$1" | awk -F. '{ printf("%d%03d%03d%03d\n", $1,$2,$3,$4); }'
 }
 
-if [ -z "$PATTERN_UTILITY_CONTAINER" ]; then
+if [ -z "${PATTERN_UTILITY_CONTAINER:-}" ]; then
 	PATTERN_UTILITY_CONTAINER="quay.io/validatedpatterns/utility-container"
 fi
 # If PATTERN_DISCONNECTED_HOME is set it will be used to populate both PATTERN_UTILITY_CONTAINER
 # and PATTERN_INSTALL_CHART automatically
-if [ -n "${PATTERN_DISCONNECTED_HOME}" ]; then
+if [ -n "${PATTERN_DISCONNECTED_HOME:-}" ]; then
     PATTERN_UTILITY_CONTAINER="${PATTERN_DISCONNECTED_HOME}/utility-container"
     PATTERN_INSTALL_CHART="oci://${PATTERN_DISCONNECTED_HOME}/pattern-install"
     echo "PATTERN_DISCONNECTED_HOME is set to ${PATTERN_DISCONNECTED_HOME}"
@@ -23,10 +24,10 @@ if [ -n "${PATTERN_DISCONNECTED_HOME}" ]; then
 fi
 
 readonly commands=(podman)
-for cmd in ${commands[@]}; do is_available "$cmd"; done
+for cmd in "${commands[@]}"; do is_available "$cmd"; done
 
 UNSUPPORTED_PODMAN_VERSIONS="1.6 1.5"
-PODMAN_VERSION_STR=$(podman --version)
+PODMAN_VERSION_STR=$(podman --version) || { echo "Failed to get podman version"; exit 1; }
 for i in ${UNSUPPORTED_PODMAN_VERSIONS}; do
 	# We add a space
 	if echo "${PODMAN_VERSION_STR}" | grep -q -E "\b${i}"; then
@@ -41,19 +42,20 @@ done
 PODMAN_VERSION=$(echo "${PODMAN_VERSION_STR}" | awk '{ print $NF }')
 
 # podman < 4.3.0 do not support keep-id:uid=...
-if [ $(version "${PODMAN_VERSION}") -lt $(version "4.3.0") ]; then
-    PODMAN_ARGS="-v ${HOME}:/root"
+PODMAN_ARGS=()
+if [ "$(version "${PODMAN_VERSION}")" -lt "$(version "4.3.0")" ]; then
+    PODMAN_ARGS=(-v "${HOME}:/root")
 else
     # We do not rely on bash's $UID and $GID because on MacOSX $GID is not set
     MYNAME=$(id -n -u)
     MYUID=$(id -u)
     MYGID=$(id -g)
-    PODMAN_ARGS="--passwd-entry ${MYNAME}:x:${MYUID}:${MYGID}::/pattern-home:/bin/bash --user ${MYUID}:${MYGID} --userns keep-id:uid=${MYUID},gid=${MYGID}"
-
+    PODMAN_ARGS=(--passwd-entry "${MYNAME}:x:${MYUID}:${MYGID}::/pattern-home:/bin/bash" --user "${MYUID}:${MYGID}" --userns "keep-id:uid=${MYUID},gid=${MYGID}")
 fi
 
-if [ -n "$KUBECONFIG" ]; then
-    if [[ ! "${KUBECONFIG}" =~ ^$HOME* ]]; then
+if [ -n "${KUBECONFIG:-}" ]; then
+    # Check if KUBECONFIG path starts with HOME directory
+    if [[ ! "${KUBECONFIG}" =~ ^"${HOME}" ]]; then
         echo "${KUBECONFIG} is pointing outside of the HOME folder, this will make it unavailable from the container."
         echo "Please move it somewhere inside your $HOME folder, as that is what gets bind-mounted inside the container"
         exit 1
@@ -62,20 +64,26 @@ fi
 
 # Detect if we use podman machine. If we do not then we bind mount local host ssl folders
 # if we are using podman machine then we do not bind mount anything (for now!)
-REMOTE_PODMAN=$(podman system connection list | tail -n +2 | wc -l)
-if [ $REMOTE_PODMAN -eq 0 ]; then # If we are not using podman machine we check the hosts folders
+REMOTE_PODMAN=$(podman system connection list | tail -n +2 | wc -l) || REMOTE_PODMAN=0
+PKI_HOST_MOUNT_ARGS=()
+if [ "${REMOTE_PODMAN}" -eq 0 ]; then # If we are not using podman machine we check the hosts folders
     # We check /etc/pki/tls because on ubuntu /etc/pki/fwupd sometimes
     # exists but not /etc/pki/tls and we do not want to bind mount in such a case
     # as it would find no certificates at all.
     if [ -d /etc/pki/tls ]; then
-        PKI_HOST_MOUNT_ARGS="-v /etc/pki:/etc/pki:ro"
+        PKI_HOST_MOUNT_ARGS=(-v /etc/pki:/etc/pki:ro)
     elif [ -d /etc/ssl ]; then
-        PKI_HOST_MOUNT_ARGS="-v /etc/ssl:/etc/ssl:ro"
+        PKI_HOST_MOUNT_ARGS=(-v /etc/ssl:/etc/ssl:ro)
     else
-        PKI_HOST_MOUNT_ARGS="-v /usr/share/ca-certificates:/usr/share/ca-certificates:ro"
+        PKI_HOST_MOUNT_ARGS=(-v /usr/share/ca-certificates:/usr/share/ca-certificates:ro)
     fi
-else
-    PKI_HOST_MOUNT_ARGS=""
+fi
+
+# Parse EXTRA_ARGS into an array if set
+EXTRA_ARGS_ARRAY=()
+if [ -n "${EXTRA_ARGS:-}" ]; then
+    # shellcheck disable=SC2206
+    EXTRA_ARGS_ARRAY=(${EXTRA_ARGS})
 fi
 
 # Copy Kubeconfig from current environment. The utilities will pick up ~/.kube/config if set so it's not mandatory
@@ -106,12 +114,12 @@ podman run -it --rm --pull=newer \
     -e TOKEN_SECRET \
     -e UUID_FILE \
     -e VALUES_SECRET \
-    ${PKI_HOST_MOUNT_ARGS} \
+    "${PKI_HOST_MOUNT_ARGS[@]}" \
     -v "$(pwd -P)":"$(pwd -P)" \
     -v "${HOME}":"${HOME}" \
     -v "${HOME}":/pattern-home \
-    ${PODMAN_ARGS} \
-    ${EXTRA_ARGS} \
+    "${PODMAN_ARGS[@]}" \
+    "${EXTRA_ARGS_ARRAY[@]}" \
     -w "$(pwd -P)" \
     "$PATTERN_UTILITY_CONTAINER" \
-    $@
+    "$@"

tests/interop/create_ci_badge.py

@@ -1,11 +1,14 @@
 import json
 import os
 import subprocess
+import sys
 from datetime import datetime
 
 from junitparser import JUnitXml
 
-oc = os.environ["HOME"] + "/oc_client/oc"
+# Use os.environ.get() with fallback to avoid KeyError
+home_dir = os.environ.get("HOME", "/tmp")
+oc = os.path.join(home_dir, "oc_client", "oc")
 
 ci_badge = {
     "schemaVersion": 1,
@@ -24,19 +27,34 @@
 
 
 def get_openshift_version():
+    """Get OpenShift version from cluster.
+
+    Returns:
+        tuple: (full_version, major_minor) on success
+        None: on any error
+    """
     try:
-        version_ret = subprocess.run([oc, "version", "-o", "json"], capture_output=True)
+        version_ret = subprocess.run(
+            [oc, "version", "-o", "json"], capture_output=True, check=False
+        )
+        if version_ret.returncode != 0:
+            print(f"Error running oc version: {version_ret.stderr.decode('utf-8')}")
+            return None
         version_out = version_ret.stdout.decode("utf-8")
         openshift_version = json.loads(version_out)["openshiftVersion"]
         major_minor = ".".join(openshift_version.split(".")[:-1])
         return openshift_version, major_minor
-    except KeyError as e:
-        print("KeyError:" + str(e))
+    except (KeyError, json.JSONDecodeError, UnicodeDecodeError, OSError) as e:
+        print(f"Error getting OpenShift version: {type(e).__name__}: {e}")
         return None
 
 
 if __name__ == "__main__":
     versions = get_openshift_version()
+    if versions is None:
+        print("Failed to get OpenShift version, exiting")
+        sys.exit(1)
+
     ci_badge["openshiftVersion"] = versions[0]
 
     pattern_repo = subprocess.run(
@@ -51,12 +69,20 @@ def get_openshift_version():
 
     # Check each xml file for failures
     results_dir = os.environ.get("WORKSPACE")
+    if results_dir is None:
+        print("WORKSPACE environment variable is not set, exiting")
+        sys.exit(1)
+
+    if not os.path.isdir(results_dir):
+        print(f"WORKSPACE directory does not exist: {results_dir}")
+        sys.exit(1)
+
     failures = 0
 
     for file in os.listdir(results_dir):
         if file.startswith("test_") and file.endswith(".xml"):
-            with open(os.path.join(results_dir, file), "r") as result_file:  # type: ignore
-                xml = JUnitXml.fromfile(result_file)  # type: ignore
+            with open(os.path.join(results_dir, file), "r") as result_file:
+                xml = JUnitXml.fromfile(result_file)
                 for suite in xml:
                     for case in suite:
                         if case.result:
@@ -69,15 +95,26 @@ def get_openshift_version():
     # For now we assume `message` is the same as patternBranch
     ci_badge["message"] = ci_badge["patternBranch"]
 
+    # Validate required environment variables for filename
+    pattern_shortname = os.environ.get("PATTERN_SHORTNAME")
+    infra_provider = os.environ.get("INFRA_PROVIDER")
+
+    if not pattern_shortname:
+        print("PATTERN_SHORTNAME environment variable is not set, exiting")
+        sys.exit(1)
+    if not infra_provider:
+        print("INFRA_PROVIDER environment variable is not set, exiting")
+        sys.exit(1)
+
     ci_badge_json_basename = (
-        os.environ.get("PATTERN_SHORTNAME")  # type: ignore
+        pattern_shortname
         + "-"
-        + os.environ.get("INFRA_PROVIDER")
+        + infra_provider
         + "-"
         + versions[1]
         + "-stable-badge.json"
     )
-    ci_badge_json_filename = os.path.join(results_dir, ci_badge_json_basename)  # type: ignore
+    ci_badge_json_filename = os.path.join(results_dir, ci_badge_json_basename)
     print(f"Creating CI badge file at: {ci_badge_json_filename}")
 
     with open(ci_badge_json_filename, "w") as ci_badge_file: