Skip to content

Draft: Remove Azure pipeline task. Support protected files and remove need for rg permissions for service principal #39

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Draft: Remove Azure pipeline task. Support protected files and remove need for rg permissions for service principal #39

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14cc47c
remove azure pipeline task from the actions repo
kuthiala Jul 16, 2025
f9dbec3
make use of nginx cli extension for config updates
kuthiala Jul 16, 2025
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: 10 additions & 5 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,19 @@ inputs:
default: "nginx.conf"
transformed-nginx-config-directory-path:
description: >
'The transformed absolute path of the NGINX configuration directory in NGINXaaS for Azure deployment, example: "/etc/nginx/".
If the "include" directive in the NGINX configuration files uses absolute paths, the path transformation
can be used to overwrite the file paths when the action synchronizes the files to the NGINXaaS for Azure deployment.'
'The absolute directory path in the NGINXaaS for Azure deployment where your configuration files will be placed.
All files found in the nginx-config-directory-path will be copied to this location in the deployment.
For example, use "/etc/nginx/" to match the standard NGINX directory structure on Azure.
Copy link
Contributor

Choose a reason for hiding this comment

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

For example, use "/etc/nginx/" to match the standard NGINX directory structure on Azure. -> For example, use "/etc/nginx/" to match the standard NGINX directory structure on your deployment. << something along these lines.

If your NGINX configuration files use absolute paths in "include" directives, this setting ensures those paths are correctly mapped in the deployment by prepending the specified directory.'
required: false
default: ""
nginx-certificates:
description: 'An array of JSON objects each with keys nginx_cert_name, keyvault_secret, certificate_virtual_path and key_virtual_path. Example: [{"certificateName": "server1", "keyvaultSecret": "https://...", "certificateVirtualPath": "/etc/ssl/certs/server1.crt", "keyVirtualPath": "/etc/ssl/certs/server1.key" }, {"name": "server2", "keyvaultSecret": "https://...", "certificateVirtualPath": "/etc/ssl/certs/server2.crt", "keyVirtualPath": "/etc/ssl/certs/server2.key" }] '
required: false
protected-files:
Copy link
Contributor

Choose a reason for hiding this comment

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

From an Actions perspective, this is a new feature in that it was not there earlier on and now we have added support for handling protected files via the action:

  • This should be done in a separate MR. You have the changes here which is fine but try splitting up the work so you are not refactoring the action and adding support for new (missing) things at the same time.
  • Separately, do we want to offer protected files support on git? Seems risky and something a customer may not want to use since they would not want to store sensitive stuff on git, right?

description: "Comma-separated list of file paths relative to nginx-config-directory-path that should be marked as protected. Example: 'ssl/private.key,conf.d/secrets.conf'"
required: false
default: ""
debug:
description: "Enable/Disable debug output."
required: false
Expand All @@ -40,10 +45,10 @@ runs:
using: "composite"
steps:
- name: "Synchronize NGINX certificate(s) from the Git repository to an NGINXaaS for Azure deployment"
run: ${{github.action_path}}/src/deploy-certificate.sh --subscription_id=${{ inputs.subscription-id }} --resource_group_name=${{ inputs.resource-group-name }} --nginx_deployment_name=${{ inputs.nginx-deployment-name }} --nginx_resource_location=${{ inputs.nginx-deployment-location }} --certificates=${{ toJSON(inputs.nginx-certificates) }} --debug=${{ inputs.debug }}
run: ${{github.action_path}}/src/deploy-certificate.sh --subscription-id=${{ inputs.subscription-id }} --resource-group-name=${{ inputs.resource-group-name }} --nginx-deployment-name=${{ inputs.nginx-deployment-name }} --nginx-resource-location=${{ inputs.nginx-deployment-location }} --certificates=${{ toJSON(inputs.nginx-certificates) }} --debug=${{ inputs.debug }}
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/kuthiala/nginx-for-azure-deploy-action/blob/f9dbec3c93f58bd3634762d4b2de1f2f0193a4d7/src/deploy-certificate.sh The input handling here needs to be fixed. I am okay if you want to preserve both but in that separate MR, I think you can safely consider refactoring configs and certs both as separate commits. Folks may have a preference in splitting that up further as the changeset will be a bit large but 👍

if: ${{ inputs.nginx-deployment-location != '' && inputs.nginx-certificates != '' }}
shell: bash
- name: "Synchronize NGINX configuration from the Git repository to an NGINXaaS for Azure deployment"
run: ${{github.action_path}}/src/deploy-config.sh --subscription_id=${{ inputs.subscription-id }} --resource_group_name=${{ inputs.resource-group-name }} --nginx_deployment_name=${{ inputs.nginx-deployment-name }} --config_dir_path=${{ inputs.nginx-config-directory-path }} --root_config_file=${{ inputs.nginx-root-config-file }} --transformed_config_dir_path=${{ inputs.transformed-nginx-config-directory-path }} --debug=${{ inputs.debug }}
run: ${{github.action_path}}/src/deploy-config.sh --subscription-id=${{ inputs.subscription-id }} --resource-group-name=${{ inputs.resource-group-name }} --nginx-deployment-name=${{ inputs.nginx-deployment-name }} --nginx-config-directory-path=${{ inputs.nginx-config-directory-path }} --nginx-root-config-file=${{ inputs.nginx-root-config-file }} --transformed-nginx-config-directory-path=${{ inputs.transformed-nginx-config-directory-path }} --protected-files=${{ inputs.protected-files }} --debug=${{ inputs.debug }}
Copy link
Contributor

Choose a reason for hiding this comment

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

here's a quick suggestion:

I agree with what you are doing here but:

  • This MR is too big in scope as you are already removing the Azure Task.
  • And I see that we have only made changes to deploy-config.sh and not deploy-certificate.sh? 🤔

I'd recommend the following:

  • Keep this MR to retiring the Azure Pipeline Task.
  • Split up the rest of the work into its own where you are replacing what the Action itself is doing underneath:
    • You should also think about expanding the action use-cases in mind.
      • This repository currently has configs and certs. Do we want to preserve the flows as they stand with two separate scripts wrapped in a single action or do we want separate actions for managing certs vs configs.
      • Do we want to expand this action to create and update deployments?

if: ${{ inputs.nginx-config-directory-path != '' }}
shell: bash
259 changes: 204 additions & 55 deletions src/deploy-config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,69 +1,72 @@
#!/bin/bash
set -euo pipefail
set -eo pipefail
Copy link
Contributor

Choose a reason for hiding this comment

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

was there a reason to drop -u? It's usually good to have it.

IFS=$'\n\t'

transformed_config_dir_path=''
for i in "$@"
do
case $i in
--subscription_id=*)
--subscription-id=*)
subscription_id="${i#*=}"
shift
;;
--resource_group_name=*)
--resource-group-name=*)
resource_group_name="${i#*=}"
shift
;;
--nginx_deployment_name=*)
--nginx-deployment-name=*)
nginx_deployment_name="${i#*=}"
shift
;;
--config_dir_path=*)
--nginx-config-directory-path=*)
config_dir_path="${i#*=}"
shift
;;
--root_config_file=*)
--nginx-root-config-file=*)
root_config_file="${i#*=}"
shift
;;
--transformed_config_dir_path=*)
--transformed-nginx-config-directory-path=*)
transformed_config_dir_path="${i#*=}"
shift
;;
--debug=*)
debug="${i#*=}"
shift
;;
--protected-files=*)
protected_files="${i#*=}"
shift
;;
*)
echo "Not matched option '${i#*=}' passed in."
echo "Unknown option '${i}' passed in."
exit 1
;;
esac
done

if [[ ! -v subscription_id ]];
then
echo "Please set 'subscription-id' ..."
exit 1
# Validate Required Parameters
missing_params=()
if [ -z "$subscription_id" ]; then
missing_params+=("subscription-id")
fi
if [[ ! -v resource_group_name ]];
then
echo "Please set 'resource-group-name' ..."
exit 1
if [ -z "$resource_group_name" ]; then
missing_params+=("resource-group-name")
fi
if [[ ! -v nginx_deployment_name ]];
then
echo "Please set 'nginx-deployment-name' ..."
exit 1
if [ -z "$nginx_deployment_name" ]; then
missing_params+=("nginx-deployment-name")
fi
if [[ ! -v config_dir_path ]];
then
echo "Please set 'nginx-config-directory-path' ..."
exit 1
if [ -z "$config_dir_path" ]; then
missing_params+=("nginx-config-directory-path")
fi
if [[ ! -v root_config_file ]];
then
echo "Please set 'nginx-root-config-file' ..."
if [ -z "$root_config_file" ]; then
missing_params+=("nginx-root-config-file")
fi

# Check and print if any required params are missing
if [ ${#missing_params[@]} -gt 0 ]; then
echo "Error: Missing required variables in the workflow:"
echo "${missing_params[*]}"
exit 1
fi

Expand Down Expand Up @@ -121,60 +124,206 @@ fi
transformed_root_config_file_path="$transformed_config_dir_path$root_config_file"
echo "The transformed root NGINX configuration file path is '$transformed_root_config_file_path'."

# Create a NGINX configuration tarball.
# Common utility functions

# Function to trim whitespace from a string
trim_whitespace() {
local var="$1"
# Trim leading whitespace from the file path (var)
# ${var%%[![:space:]]*} starts at the file path's end
# and finds the longest match of non-whitespace
# characters leaving only leading whitespaces
# ${var#"..." } removes the leading whitespace found
var="${var#"${var%%[![:space:]]*}"}"
# Remove trailing whitespace
# See explanation above. The process is reversed here.
var="${var%"${var##*[![:space:]]}"}"
# Check if the file exists in the repository
echo "$var"
}

# Function to encode file content to base64
encode_file_base64() {
local file_path="$1"
# Use base64 to encode the file content
# -w 0 option is used to avoid line wrapping in the output
base64 -w 0 "$file_path"
}

config_tarball="nginx-config.tar.gz"
# Function to build virtual path from relative path
build_virtual_path() {
local relative_path="$1"
echo "${transformed_config_dir_path}${relative_path}"
}

echo "Creating a tarball from the NGINX configuration directory."
tar -cvzf "$config_tarball" -C "$config_dir_path" --xform s:'./':"$transformed_config_dir_path": .
echo "Successfully created the tarball from the NGINX configuration directory."
# Function to add a file entry to a JSON array
# The add_file_to_json_array function uses indirect variable references
# and global assignment to update JSON arrays and flags that track
# which files have been processed. The variable names for the JSON array
# and the "first file" flag are passed as arguments, allowing the
# function to generically update different arrays
# (for regular and protected files) without hardcoding their names.
# The syntax ${!var} retrieves the value of the variable whose
# name is stored in 'var', and declare -g ensures the updated
# values are set globally, so changes persist outside the function.
add_file_to_json_array() {
local file_path="$1"
local virtual_path="$2"
local file_type="$3" # "regular" or "protected"
local json_var_name="$4" # Variable name to modify
local first_file_var_name="$5" # Variable name for first_file flag

if [ -f "$file_path" ]; then
echo "Processing $file_type file: $file_path -> $virtual_path"

# Base64 encode the file content
local file_content_b64
file_content_b64=$(encode_file_base64 "$file_path")

# Get current values using indirect variable references
local current_json="${!json_var_name}"
local is_first_file="${!first_file_var_name}"

# Add comma separator if not the first file
if [ "$is_first_file" = false ]; then
current_json+=","
fi

# Add the file entry to JSON array
current_json+="{\"content\":\"$file_content_b64\",\"virtual-path\":\"$virtual_path\"}"

# Update the variables using indirect assignment
declare -g "$json_var_name=$current_json"
declare -g "$first_file_var_name=false"

if [[ "$debug" == true ]]; then
echo "$file_type file virtual path: $virtual_path"
echo "$file_type file content (base64): ${file_content_b64:0:50}..."
fi
else
echo "Warning: $file_type file '$file_path' not found"
fi
}

# Process protected files first to build exclusion list
protected_files_list=()
if [ -n "$protected_files" ]; then
IFS=',' read -ra files <<< "$protected_files"

for file in "${files[@]}"; do
file=$(trim_whitespace "$file")
if [ -n "$file" ]; then
protected_files_list+=("$file")
fi
done
fi

# Function to check if a file is in the protected files list
is_protected_file() {
local relative_path="$1"
for protected_file in "${protected_files_list[@]}"; do
if [ "$relative_path" = "$protected_file" ]; then
return 0 # File is protected
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Since you have the doc string on the function, I think it's okay to drop these inline comments. The logic makes it obvious 👍

fi
done
return 1 # File is not protected
}

echo "Listing the NGINX configuration file paths in the tarball."
tar -tf "$config_tarball"
# Process all configuration files individually (excluding protected files)

echo "Processing NGINX configuration files individually."

# Build the files JSON array
files_json="["
files_first_file=true

# Find all files in the config directory and process them (excluding protected files)
while IFS= read -r -d '' file; do
# Get relative path from config directory
relative_path="${file#$config_dir_path}"

# Skip if this file is in the protected files list
if is_protected_file "$relative_path"; then
echo "Skipping protected file from regular files: $relative_path"
continue
fi

# Apply transformation to get virtual path
virtual_path=$(build_virtual_path "$relative_path")

add_file_to_json_array "$file" "$virtual_path" "regular" "files_json" "files_first_file"
done < <(find "$config_dir_path" -type f -print0)

encoded_config_tarball=$(base64 "$config_tarball")
files_json+="]"

if [[ "$debug" == true ]]; then
echo "The base64 encoded NGINX configuration tarball"
echo "$encoded_config_tarball"
echo "Regular files JSON: $files_json"
fi
echo ""

# Synchronize the NGINX configuration tarball to the NGINXaaS for Azure deployment.
# Process protected files if specified
protected_files_arg=""
if [ -n "$protected_files" ]; then
echo "Processing protected files: $protected_files"

# Build the protected files JSON array
protected_files_json="["
protected_first_file=true
IFS=',' read -ra files <<< "$protected_files"

for file in "${files[@]}"; do
file=$(trim_whitespace "$file")
if [ -n "$file" ]; then
repo_file_path="${config_dir_path}${file}"
virtual_path=$(build_virtual_path "$file")

add_file_to_json_array "$repo_file_path" "$virtual_path" "protected" "protected_files_json" "protected_first_file"
fi
done

protected_files_json+="]"

if [ "$protected_first_file" = false ]; then
protected_files_arg="--protected-files"
if [[ "$debug" == true ]]; then
echo "Protected files JSON: $protected_files_json"
fi
fi
fi

uuid="$(cat /proc/sys/kernel/random/uuid)"
template_file="template-$uuid.json"
template_deployment_name="${nginx_deployment_name:0:20}-$uuid"

wget -O "$template_file" https://raw.githubusercontent.com/nginxinc/nginx-for-azure-deploy-action/487d1394d6115d4f42ece6200cbd20859595557d/src/nginx-for-azure-configuration-template.json
echo "Downloaded the ARM template for synchronizing NGINX configuration."
cat "$template_file"
echo ""
# Synchronize the NGINX configuration files to the NGINXaaS for Azure deployment.

echo "Synchronizing NGINX configuration"
echo "Subscription ID: $subscription_id"
echo "Resource group name: $resource_group_name"
echo "NGINXaaS for Azure deployment name: $nginx_deployment_name"
echo "ARM template deployment name: $template_deployment_name"
echo ""

az account set -s "$subscription_id" --verbose

echo "Installing the az nginx extension if not already installed."
az extension add --name nginx --allow-preview true

az_cmd=(
"az"
"nginx"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need /src/nginx-for-azure-configuration-template.json anymore? If not, can we not delete it as part of this commit?

"deployment"
"group"
"create"
"--name" "$template_deployment_name"
"configuration"
"update"
"--name" "default"
"--deployment-name" "$nginx_deployment_name"
"--resource-group" "$resource_group_name"
"--template-file" "$template_file"
"--parameters"
"nginxDeploymentName=$nginx_deployment_name"
"rootFile=$transformed_root_config_file_path"
"tarball=$encoded_config_tarball"
"--root-file" "$transformed_root_config_file_path"
"--files" "$files_json"
Copy link
Contributor

Choose a reason for hiding this comment

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

if a user has large number of static files to host static content via git, you will very likely hit the Azure API limit of 4M using this action. In those cases, a tarball is of huge help. You should consider the tradeoffs of dropping tarball support in favor using the config files directly.

It's common for users to host static content on Github. Example: https://github.com/nginx/documentation/

"--verbose"
)

# Add protected files argument if present
if [ -n "$protected_files_arg" ]; then
az_cmd+=("$protected_files_arg")
az_cmd+=("$protected_files_json")
fi

if [[ "$debug" == true ]]; then
az_cmd+=("--debug")
echo "${az_cmd[@]}"
Expand Down
Loading