-
Notifications
You must be signed in to change notification settings - Fork 10
Support sync multiple files NGINX configuration to an NGINX for Azure deployment #6
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
Conversation
While it's not the primary use case, it would be nice to document that this now requires |
I see. Let me address OS compatibility in a follow up PR - I think multiple places need to be addressed to ensure such compatibility, for example: tar, uuid/random string generator. I don't have a mac and mostly tested the shell script from Ubuntu, Windows subsystem for Linux or Git bash for Windows (bash emulation). Will do some investigation to figure out a proper approach. |
Are we completely removing single file support? I know it will not be the majority usecase and it is easy to understand that nginx configs should all be under one directory, but is there any desire to just support a single file config anywhere in the repo? |
As long as the single file is in its own dedicated directory in the repo, the action will work properly. Multiple files support is a super set of single file support. I don't seem to see a strong reason why customer cannot put nginx config, even a single one, in its own directory, and instead must require the action to pick config from individual file paths. Thought? |
My only concern is if people are stuffing more than just the nginx configs in that directory. The config update will ignore the files in that case though so we should be good. |
Right. So the behavior shouldn't be broken. Besides that, consider the CICD scenario, this action should only be triggered if there is any NGINX config file change committed to the repo. All other changes are unrelated and should not trigger the action. To properly configure such trigger for a GitHub workflow, it is also a better practice to have a dedicated folder with only NGINX configuration files inside. Trigger based on exact file name is not reliable upon file renaming. Then the workflow can be triggered upon any change to the files inside the folder using the "path" filter: https://github.com/bangbingsyb/nginx-config-github-composite-action/blob/main/.github/workflows/nginx-multi-files-config-absolute-paths.yml#L9 |
The change enables the action to support sync multiple files NGINX config to NGINX for Azure by:
Challenges
Solution
To overcome the above challenges, file path transformation is required when pack the config directory. The implementation leverages two key features from tar: https://man7.org/linux/man-pages/man1/tar.1.html
Action metadata changes
Introduce a new optional input parameter 'transformed-nginx-config-directory-path' to allow users to overwrite the absolute path of the config directory to the NGINX for Azure deployment. If not specified, relative paths to the config directory will be used for the files.
Testing
Testing has been done for a config directory containing child config and njs files in nested subdirectories:
-- Config: https://github.com/bangbingsyb/nginx-config-github-composite-action/tree/main/config-relative-paths
-- Action result: https://github.com/bangbingsyb/nginx-config-github-composite-action/actions/runs/2311611077
-- Config: https://github.com/bangbingsyb/nginx-config-github-composite-action/tree/main/config-absolute-paths
-- Action result: https://github.com/bangbingsyb/nginx-config-github-composite-action/actions/runs/2311600337