Skip to content

Bootstrap: fix ptl/ptlsbox bootstrap kustomize controller FIC creation#818

Open
danielwilsonkainos wants to merge 2 commits intomainfrom
bootstrap-kustomize-controller-ptlptlsbox-fix
Open

Bootstrap: fix ptl/ptlsbox bootstrap kustomize controller FIC creation#818
danielwilsonkainos wants to merge 2 commits intomainfrom
bootstrap-kustomize-controller-ptlptlsbox-fix

Conversation

@danielwilsonkainos
Copy link
Copy Markdown
Contributor

@danielwilsonkainos danielwilsonkainos commented Mar 10, 2026

Change description

  • Currently PTL/PTLSBOX envs for CFT have a clash of federated identity credentials for the kustomize controller - this exists since last AKS upgrades where clusters were re-built and re-bootstrapped
  • This is due to bootstrap loading Workload Identity manifests from apps/flux-system/workload-identity/, whereas PTL & PTLSBOX have their own workload-identity manifests in their sub-directories due to deviance in naming structure for postBuild substitutions
  • Implement sub-dir check for envs so if sub-dir for WI exists then sub-dir is used, if not then the base apps/flux-system/workload-identity/ is used
  • Once bootstrap and flux are both aligned in which WI manifests are used this will fix the duplicate federated identity credential we're seeing currently
Screenshot 2026-03-10 at 07 35 20

Testing done

  • This needs testing but requires rebuild of ptlsbox or ptl

Checklist

  • commit messages are meaningful and follow good commit message guidelines
  • README and other documentation has been updated / added (if needed)
  • tests have been updated / new tests has been added (if needed)
  • Does this PR introduce a breaking change

Link to Terraform Plan

https://tfplan-viewer.hmcts.net/aks-cft-deploy/818

🤖AEP PR SUMMARY🤖

  • bootstrap/scripts/install-flux.sh 🚀
    • Enhanced install_flux function to conditionally use an environment-specific workload-identity folder if it exists (apps/flux-system/${CLUSTER_ENV}/workload-identity), otherwise falls back to the shared folder (apps/flux-system/workload-identity).
    • Added a curl check to verify the presence of the environment-specific folder before downloading.
    • Updated download logic to dynamically reference the appropriate folder based on the check.
    • Set new variable WI_FOLDER_CHECK for existence check and WI_FOLDER for the download path.
    • Introduced ASO_PATCH_URL variable to point to a patch YAML file within the environment-specific base folder.

@danielwilsonkainos danielwilsonkainos requested a review from a team as a code owner March 10, 2026 14:33
@danielwilsonkainos danielwilsonkainos requested review from Babunawa, niallk and reespozzi and removed request for a team March 10, 2026 14:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 10, 2026

Recommendations for Improvement

Code Quality

  1. Avoid Hardcoding URLs:
    • The hardcoded URLs (https://api.github.com/repos/hmcts/cnp-flux-config/...) are prone to future changes and maintenance overhead. Consider pulling these from a configuration file or environment variables to improve maintainability.
    • Example:
      bash
      BASE_URL=${BASE_FLUX_CONFIG_URL:-"https://api.github.com/repos/hmcts/cnp-flux-config\"}
      download_files "${BASE_URL}/contents/${WI_FOLDER}" ...
      
      
  2. Remove Redundant Environment Variable Substitutions:
    • Both if and else blocks have duplication for the download_files invocation, with mostly identical logic. Refactor this into a separate reusable function to prevent code repetition.
    • Example:
      function download_workload_identity {
        local folder_url=$1
        download_files \"${folder_url}\" \"${TMP_DIR}/gotk\" \
          \"s|\${WI_CLUSTER}|$ENVIRONMENT-$CLUSTER_NAME|g\" \
          \"s|\${ENVIRONMENT}|$ENVIRONMENT|g\" \
          \"s|\${ISSUER_URL}|$ISSUER_URL|g\"
      }
      
      if curl --output /dev/null --silent --head --fail \"${WI_FOLDER_CHECK}\"; then
        echo \"Using env-specific workload-identity folder for ${CLUSTER_ENV}\"
        download_workload_identity \"https://api.github.com/repos/hmcts/cnp-flux-config/contents/${WI_FOLDER}\"
      else
        echo \"Using shared workload-identity folder\"
        download_workload_identity \"https://api.github.com/repos/hmcts/cnp-flux-config/contents/apps/flux-system/workload-identity\"
      fi

Security

  1. Use Secure Curl Parameters:

    • The curl command lacks security-related flags to prevent potential man-in-the-middle attacks. Add --fail --show-error --connect-timeout 10 --retry 3 --retry-connrefused for better error handling and to enforce secure connections.
    • Example:
      if curl --output /dev/null --silent --head --fail --show-error --connect-timeout 10 --retry 3 --retry-connrefused \"${WI_FOLDER_CHECK}\"; then
  2. Sanitize Environment Variables:

    • Ensure ENVIRONMENT, CLUSTER_NAME, and ISSUER_URL are sanitized to prevent potential command injection attacks or unintended behavior, as they are directly interpolated into strings.

Best Practices

  1. Fail Fast with Error Messages:

    • Add error handling to gracefully inform the user if curl or download_files fails.
    • Example:
      if ! curl ...; then
        echo \"Failed to check WI folder: ${WI_FOLDER_CHECK}. Exiting.\" >&2
        exit 1
      fi
  2. Enable Shell Options:

    • Use set -euo pipefail at the top of the script to catch common errors, such as undefined variables or failed commands.

Cost

  • Using curl to dynamically fetch manifests increases build-time dependency on external services. Consider caching these files in a local mirror or CI/CD pipeline artifact storage to reduce recurring API requests. Hosting costs for such files on GitHub will increase slightly (~£0.01 per 1000-requests) depending on usage.

Carbon Usage

  • Each API call to GitHub consumes network energy. Consider batch downloading a repository archive rather than making multiple curl calls if this script is run frequently.

Summary

  • Refactor duplicated logic into functions.
  • Externalize URLs into configuration or environment variables.
  • Add secure curl flags and sanitize inputs.
  • Implement robust error handling and fail fast on critical failures.
  • Batch file downloads where possible to reduce cost and carbon impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant