Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add e2e for apps #426

Closed
wants to merge 3 commits into from
Closed

add e2e for apps #426

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e274032
add e2e for apps
klinch0 Oct 15, 2024
03206eb
Refactor e2e tests for applications
kvaps Oct 18, 2024
fe80232
Merge pull request #1 from aenix-io/add-tests-refactor
klinch0 Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,19 @@ repos:
- id: mixed-line-ending
args: [--fix=lf]
- id: check-yaml
exclude: packages/apps/postgres/templates/init-script.yaml
exclude: .*/init-script\.yaml$
args: [--unsafe]
- repo: https://github.com/igorshubovych/markdownlint-cli
rev: v0.41.0
hooks:
- id: markdownlint
args: [--fix, --disable, MD013, MD041, --]
- id: markdownlint
args: [--fix, --disable, MD013, MD041, --]
- repo: local
hooks:
- id: gen-versions-map
name: Generate versions map and check for changes
entry: bash -c 'cd packages/apps && make check-version-map'
language: system
types: [file]
pass_filenames: false
description: Run the script and fail if it generates changes
16 changes: 16 additions & 0 deletions hack/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
.PHONY: test clean help

SCRIPT=./e2e.applications.sh
PRECHECKS=./pre-checks.sh

help:
@echo "Usage: make {test|clean}"
@echo " test - Run the end-to-end tests."
@echo " clean - Clean up resources."

test:
@bash $(PRECHECKS) test
@bash $(SCRIPT) test

clean:
@bash $(SCRIPT) clean
54 changes: 54 additions & 0 deletions hack/e2e.applications.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
for file in ./modules/*.sh; do
source "$file"
done
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve script initialization and error handling

Consider the following improvements:

  1. Add a shebang line at the beginning of the script.
  2. Implement error handling for missing modules.
  3. Set shell options for better error detection.

Here's a suggested improvement:

+#!/bin/bash
+set -euo pipefail
+
+# Source all modules required for this script
 for file in ./modules/*.sh; do
+    if [[ ! -f "$file" ]]; then
+        echo "Error: Required module $file not found" >&2
+        exit 1
+    fi
     source "$file"
 done

This change addresses the static analysis warning about the missing shebang and improves overall script robustness.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for file in ./modules/*.sh; do
source "$file"
done
#!/bin/bash
set -euo pipefail
# Source all modules required for this script
for file in ./modules/*.sh; do
if [[ ! -f "$file" ]]; then
echo "Error: Required module $file not found" >&2
exit 1
fi
source "$file"
done
🧰 Tools
🪛 Shellcheck

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

[warning] 2-2: ShellCheck can't follow non-constant source. Use a directive to specify location.

(SC1090)


ROOT_NS="tenant-root"
TEST_TENANT="tenant-e2e"

FLUX_NS="cozy-fluxcd"
GITREPO_NAME="e2e-repo"
BRANCH="main"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Undefined clean function detected

The clean function is called in hack/e2e.applications.sh but its definition was not found in the codebase. Please ensure that the clean function is properly defined or included from the sourced modules to avoid runtime errors.

🔗 Analysis chain

Add shebang and improve script initialization

  1. Add a shebang at the beginning of the file to specify the interpreter.
  2. Consider adding a comment explaining the purpose of sourcing multiple modules.
  3. Add error handling for the case when modules are missing.

Here's a suggested improvement:

+#!/bin/bash
+set -euo pipefail
+
+# Source all modules required for this script
for file in ./modules/*.sh; do
+    if [[ ! -f "$file" ]]; then
+        echo "Error: Required module $file not found" >&2
+        exit 1
+    fi
    source "$file"
done

ROOT_NS="tenant-root"
TEST_TENANT="tenant-e2e"

FLUX_NS="cozy-fluxcd"
GITREPO_NAME="e2e-repo"
BRANCH="main"

To ensure all required modules are present, run:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if all required modules are present
if ! ls ./modules/*.sh >/dev/null 2>&1; then
    echo "Error: No modules found in ./modules/ directory"
    exit 1
fi
echo "All required modules are present"

Length of output: 261

Script:

#!/bin/bash
# Search for the definition of the 'clean' function in the codebase
ast-grep --lang bash --pattern $'function clean() { $$$ }'

Length of output: 404

Script:

#!/bin/bash
# Search for the definition of the 'clean' function in the codebase with flexible patterns
ast-grep --lang bash --pattern $'function clean { $$$ }' || \
ast-grep --lang bash --pattern $'clean() { $$$ }' || \
ast-grep --lang bash --pattern $'clean () { $$$ }'

Length of output: 402

🧰 Tools
🪛 Shellcheck

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

[warning] 2-2: ShellCheck can't follow non-constant source. Use a directive to specify location.

(SC1090)


function test() {
create_git_repo $GITREPO_NAME $FLUX_NS $BRANCH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to omit this logic, sicne Cozystack approach different, we need to test charts which are packaged to cozystak image only.

We don't expect that user will install them from external repo.


install_tenant $TEST_TENANT $ROOT_NS $GITREPO_NAME $FLUX_NS
check_helmrelease_status $TEST_TENANT $ROOT_NS

install_all_apps "../packages/apps" "$TEST_TENANT" $GITREPO_NAME $FLUX_NS

if true; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always be true, isn't that?

echo -e "${GREEN}All tests passed!${RESET}"
return 0
else
echo -e "${RED}Some tests failed!${RESET}"
return 1
fi
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve test function logic and error handling

The current implementation of the test function has several issues:

  1. It always returns success (0) regardless of the actual test results.
  2. The error case (lines 24-25) is unreachable due to the if true condition.
  3. There are no actual test assertions or checks for the installed applications.

Consider refactoring the function to include proper error handling and test assertions. Here's a suggested improvement:

 function test() {
-    create_git_repo $GITREPO_NAME $FLUX_NS $BRANCH
+    if ! create_git_repo $GITREPO_NAME $FLUX_NS $BRANCH; then
+        echo -e "${RED}Failed to create Git repository${RESET}"
+        return 1
+    fi

-    install_tenant $TEST_TENANT $ROOT_NS $GITREPO_NAME $FLUX_NS
-    check_helmrelease_status $TEST_TENANT $ROOT_NS
+    if ! install_tenant $TEST_TENANT $ROOT_NS $GITREPO_NAME $FLUX_NS; then
+        echo -e "${RED}Failed to install tenant${RESET}"
+        return 1
+    fi
+
+    if ! check_helmrelease_status $TEST_TENANT $ROOT_NS; then
+        echo -e "${RED}Helm release status check failed${RESET}"
+        return 1
+    fi

-    install_all_apps "../packages/apps" "$TEST_TENANT" $GITREPO_NAME $FLUX_NS
+    if ! install_all_apps "../packages/apps" "$TEST_TENANT" $GITREPO_NAME $FLUX_NS; then
+        echo -e "${RED}Failed to install all apps${RESET}"
+        return 1
+    fi

-    if true; then
-        echo -e "${GREEN}All tests passed!${RESET}"
-        return 0
-    else
-        echo -e "${RED}Some tests failed!${RESET}"
-        return 1
-    fi
+    # Add actual test assertions here
+    # For example:
+    # if ! check_app_status app1 $TEST_TENANT; then
+    #     echo -e "${RED}App1 status check failed${RESET}"
+    #     return 1
+    # fi
+
+    echo -e "${GREEN}All tests passed!${RESET}"
+    return 0
 }

This refactoring adds proper error handling and allows for the addition of specific test assertions for the installed applications.

Would you like assistance in implementing specific test assertions for the installed applications?

Committable suggestion was skipped due to low confidence.


function clean() {
kubectl delete gitrepository.source.toolkit.fluxcd.io $GITREPO_NAME -n $FLUX_NS
kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS
if true; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always be true, isn't that?

echo -e "${GREEN}Cleanup successful!${RESET}"
return 0
else
echo -e "${RED}Cleanup failed!${RESET}"
return 1
fi
}

case "$1" in
test)
echo -e "${YELLOW}Running tests...${RESET}"
test
;;
clean)
echo -e "${YELLOW}Cleaning up...${RESET}"
clean
;;
*)
echo -e "${RED}Usage: $0 {test|clean}${RESET}"
exit 1
;;
esac
Comment on lines +34 to +47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve main script logic to handle function return values

The main script logic correctly uses a case statement to handle command-line arguments. However, it doesn't capture or handle the return values from the test and clean functions.

Consider modifying the script to capture and handle the return values:

 case "$1" in
     test)
         echo -e "${YELLOW}Running tests...${RESET}"
-        test
+        if test; then
+            echo -e "${GREEN}Tests completed successfully${RESET}"
+            exit 0
+        else
+            echo -e "${RED}Tests failed${RESET}"
+            exit 1
+        fi
         ;;
     clean)
         echo -e "${YELLOW}Cleaning up...${RESET}"
-        clean
+        if clean; then
+            echo -e "${GREEN}Cleanup completed successfully${RESET}"
+            exit 0
+        else
+            echo -e "${RED}Cleanup failed${RESET}"
+            exit 1
+        fi
         ;;
     *)
         echo -e "${RED}Usage: $0 {test|clean}${RESET}"
         exit 1
         ;;
 esac

This modification ensures that the script's exit status reflects the success or failure of the test and clean operations, which is crucial for integration with CI/CD pipelines or other automated processes.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case "$1" in
test)
echo -e "${YELLOW}Running tests...${RESET}"
test
;;
clean)
echo -e "${YELLOW}Cleaning up...${RESET}"
clean
;;
*)
echo -e "${RED}Usage: $0 {test|clean}${RESET}"
exit 1
;;
esac
case "$1" in
test)
echo -e "${YELLOW}Running tests...${RESET}"
if test; then
echo -e "${GREEN}Tests completed successfully${RESET}"
exit 0
else
echo -e "${RED}Tests failed${RESET}"
exit 1
fi
;;
clean)
echo -e "${YELLOW}Cleaning up...${RESET}"
if clean; then
echo -e "${GREEN}Cleanup completed successfully${RESET}"
exit 0
else
echo -e "${RED}Cleanup failed${RESET}"
exit 1
fi
;;
*)
echo -e "${RED}Usage: $0 {test|clean}${RESET}"
exit 1
;;
esac

32 changes: 32 additions & 0 deletions hack/modules/check_helmrelease_status.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/bin/bash

source ./modules/colors.sh
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using a more robust path for sourcing colors.sh

The current method of sourcing colors.sh assumes a specific directory structure, which might not be portable across different environments or when the script is called from different locations.

Consider using a more robust method to source the colors.sh file. Here's an example:

-source ./modules/colors.sh
+SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+source "${SCRIPT_DIR}/colors.sh"

This change ensures that the colors.sh file is sourced from the same directory as the current script, regardless of where it's called from.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/bin/bash
source ./modules/colors.sh
#!/bin/bash
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
source "${SCRIPT_DIR}/colors.sh"


function check_helmrelease_status() {
local release_name="$1"
local namespace="$2"
local timeout=300 # Timeout in seconds
local interval=5 # Interval between checks in seconds
local elapsed=0
Comment on lines +5 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider making timeout and interval configurable

The timeout and interval values are currently hardcoded, which might limit the script's flexibility in different environments or for different releases.

Consider making these values configurable by accepting them as optional parameters with default values. Here's an example implementation:

 function check_helmrelease_status() {
     local release_name="$1"
     local namespace="$2"
-    local timeout=300  # Timeout in seconds
-    local interval=5   # Interval between checks in seconds
+    local timeout="${3:-300}"  # Timeout in seconds, default 300
+    local interval="${4:-5}"   # Interval between checks in seconds, default 5
     local elapsed=0

This change allows users to optionally specify custom timeout and interval values when calling the function, while maintaining the current defaults if not specified.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function check_helmrelease_status() {
local release_name="$1"
local namespace="$2"
local timeout=300 # Timeout in seconds
local interval=5 # Interval between checks in seconds
local elapsed=0
function check_helmrelease_status() {
local release_name="$1"
local namespace="$2"
local timeout="${3:-300}" # Timeout in seconds, default 300
local interval="${4:-5}" # Interval between checks in seconds, default 5
local elapsed=0


while [[ $elapsed -lt $timeout ]]; do
Copy link
Member

@kvaps kvaps Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be simpler to use kubectl wait --for=condition=ready?

local status_output
status_output=$(kubectl get helmrelease "$release_name" -n "$namespace" -o json | jq -r '.status.conditions[-1].reason')

if [[ "$status_output" == "InstallSucceeded" ]]; then
echo -e "${GREEN}Helm release '$release_name' is ready.${RESET}"
return 0
elif [[ "$status_output" == "InstallFailed" ]]; then
echo -e "${RED}Helm release '$release_name': InstallFailed${RESET}"
exit 1
else
echo -e "${YELLOW}Helm release '$release_name' is not ready. Current status: $status_output${RESET}"
fi

sleep "$interval"
elapsed=$((elapsed + interval))
done
Comment on lines +12 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve error handling and provide more detailed failure information

The current implementation doesn't handle potential errors from the kubectl command, and it provides limited information in case of failure.

Consider adding error handling for the kubectl command and providing more detailed information in case of failure. Here's an example implementation:

     while [[ $elapsed -lt $timeout ]]; do
         local status_output
-        status_output=$(kubectl get helmrelease "$release_name" -n "$namespace" -o json | jq -r '.status.conditions[-1].reason')
+        if ! status_output=$(kubectl get helmrelease "$release_name" -n "$namespace" -o json 2>&1); then
+            echo -e "${RED}Error retrieving status for '$release_name': $status_output${RESET}"
+            exit 1
+        fi
+        
+        status_reason=$(echo "$status_output" | jq -r '.status.conditions[-1].reason')
+        status_message=$(echo "$status_output" | jq -r '.status.conditions[-1].message')
 
-        if [[ "$status_output" == "InstallSucceeded" ]]; then
+        if [[ "$status_reason" == "InstallSucceeded" ]]; then
             echo -e "${GREEN}Helm release '$release_name' is ready.${RESET}"
             return 0
-        elif [[ "$status_output" == "InstallFailed" ]]; then
-          echo -e "${RED}Helm release '$release_name': InstallFailed${RESET}"
+        elif [[ "$status_reason" == "InstallFailed" ]]; then
+          echo -e "${RED}Helm release '$release_name': InstallFailed${RESET}"
+          echo -e "${RED}Failure reason: $status_message${RESET}"
           exit 1
         else
-            echo -e "${YELLOW}Helm release '$release_name' is not ready. Current status: $status_output${RESET}"
+            echo -e "${YELLOW}Helm release '$release_name' is not ready. Current status: $status_reason${RESET}"
+            echo -e "${YELLOW}Status message: $status_message${RESET}"
         fi

These changes add error handling for the kubectl command and provide more detailed information about the status, including the status message, which can be helpful for debugging.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while [[ $elapsed -lt $timeout ]]; do
local status_output
status_output=$(kubectl get helmrelease "$release_name" -n "$namespace" -o json | jq -r '.status.conditions[-1].reason')
if [[ "$status_output" == "InstallSucceeded" ]]; then
echo -e "${GREEN}Helm release '$release_name' is ready.${RESET}"
return 0
elif [[ "$status_output" == "InstallFailed" ]]; then
echo -e "${RED}Helm release '$release_name': InstallFailed${RESET}"
exit 1
else
echo -e "${YELLOW}Helm release '$release_name' is not ready. Current status: $status_output${RESET}"
fi
sleep "$interval"
elapsed=$((elapsed + interval))
done
while [[ $elapsed -lt $timeout ]]; do
local status_output
if ! status_output=$(kubectl get helmrelease "$release_name" -n "$namespace" -o json 2>&1); then
echo -e "${RED}Error retrieving status for '$release_name': $status_output${RESET}"
exit 1
fi
status_reason=$(echo "$status_output" | jq -r '.status.conditions[-1].reason')
status_message=$(echo "$status_output" | jq -r '.status.conditions[-1].message')
if [[ "$status_reason" == "InstallSucceeded" ]]; then
echo -e "${GREEN}Helm release '$release_name' is ready.${RESET}"
return 0
elif [[ "$status_reason" == "InstallFailed" ]]; then
echo -e "${RED}Helm release '$release_name': InstallFailed${RESET}"
echo -e "${RED}Failure reason: $status_message${RESET}"
exit 1
else
echo -e "${YELLOW}Helm release '$release_name' is not ready. Current status: $status_reason${RESET}"
echo -e "${YELLOW}Status message: $status_message${RESET}"
fi
sleep "$interval"
elapsed=$((elapsed + interval))
done


echo -e "${RED}Timeout reached. Helm release '$release_name' is still not ready after $timeout seconds.${RESET}"
exit 1
}
Comment on lines +30 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance timeout error message with more information

While the current timeout error message is clear, it could be more informative by including the last known status of the release.

Consider modifying the timeout error message to include the last known status of the release. Here's an example implementation:

-    echo -e "${RED}Timeout reached. Helm release '$release_name' is still not ready after $timeout seconds.${RESET}"
+    echo -e "${RED}Timeout reached. Helm release '$release_name' is still not ready after $timeout seconds.${RESET}"
+    echo -e "${RED}Last known status: $status_reason${RESET}"
+    echo -e "${RED}Last status message: $status_message${RESET}"
     exit 1

This change provides more context about the state of the release when the timeout occurs, which can be helpful for debugging and understanding why the release didn't become ready in time.

Committable suggestion was skipped due to low confidence.

6 changes: 6 additions & 0 deletions hack/modules/colors.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/bash

RED='\033[0;31m'
GREEN='\033[0;32m'
RESET='\033[0m'
YELLOW='\033[0;33m'
33 changes: 33 additions & 0 deletions hack/modules/create_git_repo.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/bin/bash

function create_git_repo() {
local repo_name="$1"
local namespace="$2"
local branch="$3"

if [[ -z "$repo_name" || -z "$namespace" || -z "$branch" ]]; then
echo "Usage: create_git_repo <repo_name> <namespace> <branch>"
return 1
fi

local gitrepo_file=$(mktemp /tmp/GitRepository.XXXXXX.yaml)
{
echo "apiVersion: source.toolkit.fluxcd.io/v1"
echo "kind: GitRepository"
echo "metadata:"
echo " name: \"$repo_name\""
echo " namespace: \"$namespace\""
echo "spec:"
echo " interval: 1m"
echo " url: https://github.com/aenix-io/cozystack"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider parameterizing the GitHub URL.

The GitHub URL is currently hardcoded to "https://github.com/aenix-io/cozystack". If this script is intended for general use, it might be beneficial to make this URL a parameter of the function.

Here's a suggested improvement:

  1. Add a new parameter to the function:
function create_git_repo() {
    local repo_name="$1"
    local namespace="$2"
    local branch="$3"
    local repo_url="$4"
  1. Update the parameter check and usage message:
if [[ -z "$repo_name" || -z "$namespace" || -z "$branch" || -z "$repo_url" ]]; then
    echo "Usage: create_git_repo <repo_name> <namespace> <branch> <repo_url>"
    return 1
fi
  1. Use the new parameter in the YAML generation:
echo "  url: $repo_url"

This change would make the script more flexible and reusable across different projects.

echo " ref:"
echo " branch: \"$branch\""
echo " ignore: |"
echo " !/packages/apps/ "

} > "$gitrepo_file"

kubectl apply -f "$gitrepo_file"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error handling for the kubectl command.

The kubectl apply command is used correctly, but there's no error handling in case the command fails. Adding error handling would make the script more robust.

Here's a suggested improvement:

-    kubectl apply -f "$gitrepo_file"
+    if ! kubectl apply -f "$gitrepo_file"; then
+        echo "Error: Failed to apply GitRepository resource"
+        rm -f "$gitrepo_file"
+        return 1
+    fi

This change will catch any errors from the kubectl apply command, clean up the temporary file, and return an error code if the application fails.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
kubectl apply -f "$gitrepo_file"
if ! kubectl apply -f "$gitrepo_file"; then
echo "Error: Failed to apply GitRepository resource"
rm -f "$gitrepo_file"
return 1
fi


rm -f "$gitrepo_file"
}
6 changes: 6 additions & 0 deletions hack/modules/ignored_charts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
tenant
http-cache
mysql
rabbitmq
virtual-machine
vpn
66 changes: 66 additions & 0 deletions hack/modules/install_all_apps.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#!/bin/bash

source ./modules/colors.sh
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add sourcing for required scripts

The script is missing the sourcing of required scripts for install_helmrelease and check_helmrelease_status functions. Add the following lines after sourcing the colors module:

source ./hack/modules/install_chart.sh
source ./hack/modules/check_helmrelease_status.sh

This ensures that all required functions are available before they are called in the script.


# Function to load ignored charts from a file
function load_ignored_charts() {
local ignore_file="$1"
local ignored_charts=()

if [[ -f "$ignore_file" ]]; then
while IFS= read -r chart; do
ignored_charts+=("$chart")
done < "$ignore_file"
else
echo "Ignore file not found: $ignore_file"
fi

# Return the array of ignored charts
echo "${ignored_charts[@]}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use 'printf' instead of 'echo' to safely handle array output

Using echo to output array elements can cause word splitting and globbing issues, especially if any chart names contain spaces or special characters. It's safer to use printf with a null character as a delimiter.

Modify the function to use printf:

-    echo "${ignored_charts[@]}"
+    printf '%s\0' "${ignored_charts[@]}"

This change ensures that the array elements are outputted correctly without unintended splitting.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
echo "${ignored_charts[@]}"
printf '%s\0' "${ignored_charts[@]}"

}
Comment on lines +6 to +20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use 'printf' for safer array output

To safely handle array output, especially when chart names might contain spaces or special characters, replace the echo command with printf:

-    echo "${ignored_charts[@]}"
+    printf '%s\0' "${ignored_charts[@]}"

This change ensures that array elements are outputted correctly without unintended splitting.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function load_ignored_charts() {
local ignore_file="$1"
local ignored_charts=()
if [[ -f "$ignore_file" ]]; then
while IFS= read -r chart; do
ignored_charts+=("$chart")
done < "$ignore_file"
else
echo "Ignore file not found: $ignore_file"
fi
# Return the array of ignored charts
echo "${ignored_charts[@]}"
}
function load_ignored_charts() {
local ignore_file="$1"
local ignored_charts=()
if [[ -f "$ignore_file" ]]; then
while IFS= read -r chart; do
ignored_charts+=("$chart")
done < "$ignore_file"
else
echo "Ignore file not found: $ignore_file"
fi
# Return the array of ignored charts
printf '%s\0' "${ignored_charts[@]}"
}


# Function to check if a chart is in the ignored list
function is_chart_ignored() {
local chart_name="$1"
shift
local ignored_charts=("$@")

for ignored_chart in "${ignored_charts[@]}"; do
if [[ "$ignored_chart" == "$chart_name" ]]; then
return 0
fi
done
return 1
}

function install_all_apps() {
local charts_dir="$1"
local namespace="$2"
local gitrepo_name="$3"
local flux_ns="$4"

local ignore_file="./modules/ignored_charts"
local ignored_charts
ignored_charts=($(load_ignored_charts "$ignore_file"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use 'mapfile' for safer array assignment

To safely read the array elements and avoid potential word splitting issues, use mapfile with process substitution:

-    ignored_charts=($(load_ignored_charts "$ignore_file"))
+    mapfile -d '' -t ignored_charts < <(load_ignored_charts "$ignore_file")

This change reads the null-delimited output from load_ignored_charts into the ignored_charts array safely.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ignored_charts=($(load_ignored_charts "$ignore_file"))
mapfile -d '' -t ignored_charts < <(load_ignored_charts "$ignore_file")
🧰 Tools
🪛 Shellcheck

[warning] 44-44: Prefer mapfile or read -a to split command output (or quote to avoid splitting).

(SC2207)


for chart_path in "$charts_dir"/*; do
if [[ -d "$chart_path" ]]; then
local chart_name
chart_name=$(basename "$chart_path")
# Check if the chart is in the ignored list
if is_chart_ignored "$chart_name" "${ignored_charts[@]}"; then
echo "Skipping chart: $chart_name (listed in ignored charts)"
continue
fi

chart_name="$chart_name-e2e"
echo "Installing chart: $chart_name"
install_helmrelease "$chart_name" "$namespace" "$chart_path" "$gitrepo_name" "$flux_ns"

echo "Checking status for HelmRelease: $chart_name"
check_helmrelease_status "$chart_name" "$namespace"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Missing sourcing of required scripts for functions

The install_all_apps.sh script does not source install_chart.sh and check_helmrelease_status.sh, which define the install_helmrelease and check_helmrelease_status functions. Ensure that these scripts are sourced before invoking the functions.

  • Add the following lines to install_all_apps.sh: 
    source ./hack/modules/install_chart.sh
source ./hack/modules/check_helmrelease_status.sh
🔗 Analysis chain

Check for the existence of required functions

The script calls install_helmrelease and check_helmrelease_status, but these functions are not defined within the provided code segment. Ensure that these functions are sourced or defined before they are called.

Run the following script to search for the definitions of these functions:

This script searches recursively for the definitions of install_helmrelease and check_helmrelease_status in the current directory.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that required functions are defined in the codebase.

# Test: Search for the function definitions. Expect: Functions are defined.
grep -r -E 'function (install_helmrelease|check_helmrelease_status)\s*\(' .

Length of output: 223

Script:

#!/bin/bash
# Description: Verify that required scripts are sourced in install_all_apps.sh.

# Check if install_chart.sh is sourced
grep -E '^\s*(source|\.)\s+./hack/modules/install_chart\.sh' ./hack/modules/install_all_apps.sh

# Check if check_helmrelease_status.sh is sourced
grep -E '^\s*(source|\.)\s+./hack/modules/check_helmrelease_status\.sh' ./hack/modules/install_all_apps.sh

Length of output: 206

else
echo "$chart_path is not a directory. Skipping."
fi
done
}
60 changes: 60 additions & 0 deletions hack/modules/install_chart.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/bin/bash

source ./modules/colors.sh

function install_helmrelease() {
local release_name="$1"
local namespace="$2"
local chart_path="$3"
local gitrepo_name="$4"
local flux_ns="$5"
local values_file="$6"

if [[ -z "$release_name" ]]; then
echo -e "${RED}Error: Release name is required.${RESET}"
exit 1
fi

if [[ -z "$namespace" ]]; then
echo -e "${RED}Error: Namespace name is required.${RESET}"
exit 1
fi

if [[ -z "$chart_path" ]]; then
echo -e "${RED}Error: Chart path name is required.${RESET}"
exit 1
fi

local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)

Comment on lines +28 to +29
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Separate variable declaration and assignment for error handling

To improve error handling and follow best practices, it's recommended to separate the declaration and assignment of the helmrelease_file variable. This allows for checking if mktemp succeeded and handling any potential errors.

Apply this change to address the issue:

-local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
+local helmrelease_file
+helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
+if [[ $? -ne 0 ]]; then
+    echo -e "${RED}Error: Failed to create temporary file.${RESET}"
+    exit 1
+fi

This change allows you to check if mktemp succeeded and handle the error appropriately.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
local helmrelease_file
helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
if [[ $? -ne 0 ]]; then
echo -e "${RED}Error: Failed to create temporary file.${RESET}"
exit 1
fi
🧰 Tools
🪛 Shellcheck

[warning] 28-28: Declare and assign separately to avoid masking return values.

(SC2155)

{
echo "apiVersion: helm.toolkit.fluxcd.io/v2"
echo "kind: HelmRelease"
echo "metadata:"
echo " labels:"
echo " cozystack.io/ui: \"true\""
echo " name: \"$release_name\""
echo " namespace: \"$namespace\""
echo "spec:"
echo " chart:"
echo " spec:"
echo " chart: \"$chart_path\""
echo " reconcileStrategy: Revision"
echo " sourceRef:"
echo " kind: GitRepository"
echo " name: \"$gitrepo_name\""
echo " namespace: \"$flux_ns\""
echo " version: '*'"
echo " interval: 1m0s"
echo " timeout: 5m0s"

if [[ -n "$values_file" && -f "$values_file" ]]; then
echo " values:"
cat "$values_file" | sed 's/^/ /'
fi
} > "$helmrelease_file"

kubectl apply -f "$helmrelease_file"

rm -f "$helmrelease_file"
}
Comment on lines +57 to +60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling for kubectl command

While the YAML application and cleanup are correctly implemented, it's important to handle potential errors when applying the YAML with kubectl.

Implement error handling for the kubectl command as follows:

-kubectl apply -f "$helmrelease_file"
+if ! kubectl apply -f "$helmrelease_file"; then
+    echo -e "${RED}Error: Failed to apply HelmRelease.${RESET}"
+    rm -f "$helmrelease_file"
+    exit 1
+fi

rm -f "$helmrelease_file"

This change ensures that any errors during the kubectl apply operation are caught and reported, providing better feedback to the user and maintaining the script's robustness.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
kubectl apply -f "$helmrelease_file"
rm -f "$helmrelease_file"
}
if ! kubectl apply -f "$helmrelease_file"; then
echo -e "${RED}Error: Failed to apply HelmRelease.${RESET}"
rm -f "$helmrelease_file"
exit 1
fi
rm -f "$helmrelease_file"
}

11 changes: 11 additions & 0 deletions hack/modules/install_tenant.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash

function install_tenant (){
local release_name="$1"
local namespace="$2"
local gitrepo_name="$3"
local flux_ns="$4"
local values_file="${5:-tenant.yaml}"

install_helmrelease "$release_name" "$namespace" "../../packages/apps/tenant" "$gitrepo_name" "$flux_ns" "$values_file"
}
6 changes: 6 additions & 0 deletions hack/values/tenant.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
host: ""
etcd: false
monitoring: true
ingress: false
seaweedfs: true
isolated: true
2 changes: 1 addition & 1 deletion packages/apps/ferretdb/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.4.0
version: 0.4.1

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
7 changes: 5 additions & 2 deletions packages/apps/ferretdb/templates/init-script.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ stringData:
init.sh: |
#!/bin/bash
set -e

until pg_isready ; do sleep 5; done

echo "== create users"
{{- if .Values.users }}
psql -v ON_ERROR_STOP=1 <<\EOT
Expand All @@ -60,7 +63,7 @@ stringData:
DROP USER $user;
EOT
done

echo "== create roles"
psql -v ON_ERROR_STOP=1 --echo-all <<\EOT
SELECT 'CREATE ROLE app_admin NOINHERIT;'
Expand All @@ -80,7 +83,7 @@ stringData:
FOR schema_record IN SELECT schema_name FROM information_schema.schemata WHERE schema_name NOT IN ('pg_catalog', 'information_schema') LOOP
-- Changing Schema Ownership
EXECUTE format('ALTER SCHEMA %I OWNER TO %I', schema_record.schema_name, 'app_admin');

-- Add rights for the admin role
EXECUTE format('GRANT ALL ON SCHEMA %I TO %I', schema_record.schema_name, 'app_admin');
EXECUTE format('GRANT ALL ON ALL TABLES IN SCHEMA %I TO %I', schema_record.schema_name, 'app_admin');
Expand Down
3 changes: 2 additions & 1 deletion packages/apps/versions_map
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ ferretdb 0.1.0 4ffa8615
ferretdb 0.1.1 5ca8823
ferretdb 0.2.0 adaf603
ferretdb 0.3.0 aa2f553
ferretdb 0.4.0 HEAD
ferretdb 0.4.0 def2eb0f
ferretdb 0.4.1 HEAD
Comment on lines +12 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Commit hash def2eb0f for ferretdb version 0.4.0 not found.

The specified commit hash def2eb0f for ferretdb version 0.4.0 does not exist in the repository. Please verify and update the commit hash accordingly.

🔗 Analysis chain

LGTM! Verify commit hashes for ferretdb versions.

The changes to the ferretdb package versions look good. Version 0.4.0 has been updated with a specific commit hash, and a new version 0.4.1 has been added pointing to HEAD.

To ensure the accuracy of these changes, please run the following script to verify the commit hashes:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify commit hashes for ferretdb versions

# Test: Check if the commit hash for version 0.4.0 exists
git rev-parse --quiet --verify def2eb0f

# Test: Check if HEAD is different from the 0.4.0 commit
if [ "$(git rev-parse HEAD)" != "$(git rev-parse def2eb0f)" ]; then
    echo "HEAD is different from 0.4.0 commit, which is correct."
else
    echo "Warning: HEAD is the same as 0.4.0 commit. Please verify if this is intended."
fi

Length of output: 462

http-cache 0.1.0 a956713
http-cache 0.2.0 5ca8823
http-cache 0.3.0 fab5940
Expand Down
Loading