Skip to content

fix: print-basic-auth now supports secrets with namespace declared in annotation #42

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.

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

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

Monska85
Copy link
Collaborator

@Monska85 Monska85 commented Sep 10, 2024

PR Type

Bug fix


Description

  • Fixed issue where print-basic-auth function couldn't handle secrets with namespace declared in annotation
  • Added logic to remove prefix from secret name using cut command
  • Improves compatibility and flexibility of the authentication process
  • Ensures correct retrieval of username and password from Kubernetes secrets

Changes walkthrough 📝

Relevant files
Bug fix
bash_functions.sh
Support for secrets with namespace in annotation                 

scripts/bash_functions.sh

  • Added functionality to remove prefix from secret name using cut
    command
  • Ensures compatibility with secrets that have namespace declared in
    annotation
  • +2/-0     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @sparkfabrik-ai-bot
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No key issues to review

    @sparkfabrik-ai-bot
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Error handling
    Add error handling for kubectl commands to improve robustness

    Add error handling to check if the kubectl commands succeed. This will help identify
    issues if the secret retrieval fails.

    scripts/bash_functions.sh [28-29]

    -USERNAME=$(kubectl --namespace "${CURRENT_NAMESPACE}" get secret "${SECRET}" -o jsonpath="{.data.username}" | base64 -d)
    -PASSWORD=$(kubectl --namespace "${CURRENT_NAMESPACE}" get secret "${SECRET}" -o jsonpath="{.data.password}" | base64 -d)
    +if ! USERNAME=$(kubectl --namespace "${CURRENT_NAMESPACE}" get secret "${SECRET}" -o jsonpath="{.data.username}" | base64 -d); then
    +  echo "Failed to retrieve username from secret ${SECRET}"
    +  continue
    +fi
    +if ! PASSWORD=$(kubectl --namespace "${CURRENT_NAMESPACE}" get secret "${SECRET}" -o jsonpath="{.data.password}" | base64 -d); then
    +  echo "Failed to retrieve password from secret ${SECRET}"
    +  continue
    +fi
     
    Suggestion importance[1-10]: 9

    Why: This suggestion significantly improves the script's robustness by adding error handling for critical kubectl commands, which is crucial for identifying and handling potential issues.

    9
    Performance
    Use parameter expansion instead of external commands for string manipulation

    Consider using parameter expansion instead of echo and cut for better performance
    and readability. The ${SECRET#*/} syntax will remove everything up to and including
    the first forward slash, if present.

    scripts/bash_functions.sh [26-27]

     # Remove the prefix from the secret name, if it is present
    -SECRET="$(echo "${SECRET}" | cut -d"/" -f1)"
    +SECRET="${SECRET#*/}"
     
    Suggestion importance[1-10]: 8

    Why: The suggestion improves both performance and readability by using native Bash parameter expansion instead of external commands like 'echo' and 'cut'.

    8
    Optimize kubectl usage by retrieving multiple fields in a single command

    Consider using a single kubectl command to retrieve both username and password,
    reducing the number of API calls and improving performance.

    scripts/bash_functions.sh [28-29]

    -USERNAME=$(kubectl --namespace "${CURRENT_NAMESPACE}" get secret "${SECRET}" -o jsonpath="{.data.username}" | base64 -d)
    -PASSWORD=$(kubectl --namespace "${CURRENT_NAMESPACE}" get secret "${SECRET}" -o jsonpath="{.data.password}" | base64 -d)
    +if ! SECRET_DATA=$(kubectl --namespace "${CURRENT_NAMESPACE}" get secret "${SECRET}" -o jsonpath="{.data.username} {.data.password}"); then
    +  echo "Failed to retrieve secret data for ${SECRET}"
    +  continue
    +fi
    +read -r USERNAME_B64 PASSWORD_B64 <<< "$SECRET_DATA"
    +USERNAME=$(echo -n "$USERNAME_B64" | base64 -d)
    +PASSWORD=$(echo -n "$PASSWORD_B64" | base64 -d)
     
    Suggestion importance[1-10]: 7

    Why: This suggestion offers a performance improvement by reducing the number of API calls to kubectl, but it also increases code complexity slightly.

    7

    @Monska85 Monska85 merged commit 414daea into main Sep 10, 2024
    2 checks passed
    @Monska85 Monska85 deleted the fix/print_basic_auth branch September 10, 2024 14:23
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant