Support protected files and some input cleanup #39

kuthiala · 2025-07-18T21:13:51Z

This change adds support for the protected files
feature supported by NGINXaaS. Users can give a new optional
input called protected-files that contains a comma
separated list of all the files that need to be marked
as protected. For more information, visit:
https://docs.nginx.com/nginxaas/azure/getting-started/nginx-configuration/nginx-configuration-portal/#add-an-nginx-configuration

Testing done:

Used a service principal with Contributor role only over the NGINXaaS deployment.
Updated an NGINXaaS deployment config with and without protected files.
Updated an NGINXaaS deployment config with and without certificates.
Tested this version of the action with the above combinations from an independent repo to test real world invocations.

amudukutore

@kuthiala - given that these are both substantial changes (splitting up Actions and Azure pipeline, replace ARM templates with Azure CLI), I'd recommend creating separate MRs for these so we can discuss each one on it's merits (unless you feel there is a strong coupling in which case, I'd still prefer them to be separate but maybe sequenced in the right order). Also, when removing support for Azure pipelines, are you considering moving that to a separate repository or just removing it entirely? I'm not sure if we have customers actively using this (can we tell?) but I'd prefer we move this rather than removing it.

puneetsarna · 2025-07-22T22:44:43Z

README.md

@@ -1,32 +1,200 @@
-# NGINXaaS for Azure Configuration Automation
+# NGINXaaS for Azure Deployment Action


This is a move in the right direction to come inline with Github Actions documentation 👍

Can you please file work to track Azure Pipelines separately? We want to move it to separate repo and follow Azure guidelines around Pipelines tasks.

I'd also suggest looking into usage for this Pipeline task. Are there any customers using this? If so, removing it completely is not a good idea. You may want to follow deprecation guidelines to point users to where the task is actually hosted from here on.

Repo moved via separate merge requests:
https://github.com/nginxinc/nginx-for-azure-pipeline-task/pull/2
#45

src/deploy-config.sh

puneetsarna · 2025-07-22T22:52:41Z

src/deploy-config.sh

 az_cmd=(
    "az"
+    "nginx"


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

action.yml

puneetsarna · 2025-07-22T23:09:47Z

action.yml

@@ -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 }}


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 👍

Input handling fixed.

puneetsarna · 2025-07-22T23:13:03Z

action.yml

-      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.


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.

action.yml

puneetsarna · 2025-07-22T23:22:42Z

src/deploy-config.sh

+    local relative_path="$1"
+    for protected_file in "${protected_files_list[@]}"; do
+        if [ "$relative_path" = "$protected_file" ]; then
+            return 0  # File is protected


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 👍

puneetsarna · 2025-07-22T23:33:48Z

src/deploy-config.sh

-    "rootFile=$transformed_root_config_file_path"
-    "tarball=$encoded_config_tarball"
+    "--root-file" "$transformed_root_config_file_path"
+    "--files" "$files_json"


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/

Either we can support protected files or we can support the tarball. We can solve that problem in a follow up PR. If a user doesn't provide protected files, we can switch to using the tarball approach.

https://github.com/Azure/azure-rest-api-specs/blob/5470a13587ba49e4fb0980073ca6ba468ef837c3/specification/nginx/resource-manager/NGINX.NGINXPLUS/preview/2025-03-01-preview/openapi.json#L2459C6-L2475 this shows that our API supports protected files within the tarball.

I have updated the PR to use tarballs with protected files.

amudukutore

@kuthiala - these changes look good to me. I'll defer to @puneetsarna for final approval. What testing did you run on these changes? Can you please describe that in the PR description?

puneetsarna · 2025-08-13T19:58:21Z

src/deploy-config.sh

+    fi
+}
+
+# Process protected files first to build exclusion list


I think you should call out that you are gathering the list of protected files from user input.

The exclusion list is an internal implementation detail.

This logic has been replaced.

This change also adds support for the protected files feature supported by NGINXaaS. Users can give a new optional input called protected-files that contains a comma separated list of all the files that need to be marked as protected. For more information, visit: https://docs.nginx.com/nginxaas/azure/getting-started/nginx-configuration/nginx-configuration-portal/#add-an-nginx-configuration

This brings deploy-certificate.sh up to parity with the input validation changes made in deploy-config.sh. Adds some more input validation for certificate parameters.

kuthiala force-pushed the main branch 2 times, most recently from 7abd52e to f9dbec3 Compare July 18, 2025 21:26

kuthiala changed the title ~~Draft: Support protected files and remove need for rg permissions for service principal~~ Draft: Remove Azure pipeline task. Support protected files and remove need for rg permissions for service principal Jul 18, 2025

amudukutore reviewed Jul 22, 2025

View reviewed changes

puneetsarna reviewed Jul 22, 2025

View reviewed changes

src/deploy-config.sh Show resolved Hide resolved

puneetsarna reviewed Jul 22, 2025

View reviewed changes

action.yml Show resolved Hide resolved

puneetsarna reviewed Jul 22, 2025

View reviewed changes

action.yml Show resolved Hide resolved

puneetsarna reviewed Jul 22, 2025

View reviewed changes

kuthiala force-pushed the main branch 3 times, most recently from 09c8a09 to ac4ab39 Compare August 8, 2025 00:50

kuthiala changed the title ~~Draft: Remove Azure pipeline task. Support protected files and remove need for rg permissions for service principal~~ Support protected files and some input cleanup Aug 8, 2025

kuthiala force-pushed the main branch 8 times, most recently from 76a3797 to 5b90df0 Compare August 13, 2025 19:02

amudukutore reviewed Aug 13, 2025

View reviewed changes

puneetsarna reviewed Aug 13, 2025

View reviewed changes

kuthiala force-pushed the main branch 2 times, most recently from 97296e1 to a402007 Compare August 15, 2025 20:39

amudukutore approved these changes Aug 15, 2025

View reviewed changes

kuthiala force-pushed the main branch from a402007 to 96afaa3 Compare August 15, 2025 20:54

kuthiala added 4 commits August 15, 2025 15:15

Input cleanup in deploy-certificate.sh

bcc5ca9

This brings deploy-certificate.sh up to parity with the input validation changes made in deploy-config.sh. Adds some more input validation for certificate parameters.

restore previous action name

c08eec9

update README.md to reflect new version of the action

3fd6364

kuthiala force-pushed the main branch from 96afaa3 to 3fd6364 Compare August 15, 2025 21:15

puneetsarna approved these changes Aug 16, 2025

View reviewed changes

		@@ -1,32 +1,200 @@
		# NGINXaaS for Azure Configuration Automation
		# NGINXaaS for Azure Deployment Action

Support protected files and some input cleanup #39

Are you sure you want to change the base?

Support protected files and some input cleanup #39

Uh oh!

Conversation

kuthiala commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amudukutore left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amudukutore left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kuthiala commented Jul 18, 2025 •

edited

Loading