Install python-icm when building alert-manager in CICD while keeping it private#98
Install python-icm when building alert-manager in CICD while keeping it private#98
Conversation
2a62d8c to
22abada
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the CI/CD workflow to properly handle the alert-manager service by including it in the build process and installing its special dependency (python-icm) from environment secrets. The workflow previously excluded alert-manager from processing, but now conditionally installs the python-icm package when alert-manager is among the changed folders.
- Removed the exclusion of alert-manager from the changed folders processing
- Added conditional installation of python-icm dependency from base64-encoded environment secrets
- Updated gitignore to exclude kusto-sdk directories from version control
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/build-deploy-changes.yaml | Modified workflow to include alert-manager in processing and added conditional python-icm installation step |
| src/alert-manager/.gitignore | Added kusto-sdk directory exclusion pattern |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # check whether steps.changes.outputs.folders contains 'alert-manager' | ||
| if echo "${{ steps.changes.outputs.folders }}" | grep -q "alert-manager"; then | ||
| echo "Installing python-icm alertmanager dependencies..." | ||
| echo "${{ secrets.ICM_PACKAGE_B64 }}" | base64 -d > python-icm.zip |
There was a problem hiding this comment.
Secrets should not be decoded directly in shell commands as they may be exposed in logs. Consider using a temporary file with restricted permissions or a more secure method to handle the secret.
| echo "${{ secrets.ICM_PACKAGE_B64 }}" | base64 -d > python-icm.zip | |
| tmpfile=$(mktemp) | |
| echo "${{ secrets.ICM_PACKAGE_B64 }}" | base64 -d > "$tmpfile" | |
| chmod 600 "$tmpfile" | |
| mv "$tmpfile" python-icm.zip |
| # check whether steps.changes.outputs.folders contains 'alert-manager' | ||
| if echo "${{ steps.changes.outputs.folders }}" | grep -q "alert-manager"; then | ||
| echo "Installing python-icm alertmanager dependencies..." | ||
| echo "${{ secrets.ICM_PACKAGE_B64 }}" | base64 -d > python-icm.zip |
There was a problem hiding this comment.
Using unzip with the -o flag to overwrite files without validation could be a security risk. Consider validating the zip contents or using safer extraction methods to prevent potential zip bomb attacks.
| echo "${{ secrets.ICM_PACKAGE_B64 }}" | base64 -d > python-icm.zip | |
| echo "${{ secrets.ICM_PACKAGE_B64 }}" | base64 -d > python-icm.zip | |
| # Validate zip contents before extraction | |
| if unzip -l python-icm.zip | awk '{print $4}' | grep -E '(^/|(\.\./))'; then | |
| echo "Error: Zip file contains unsafe paths. Aborting extraction." >&2 | |
| exit 1 | |
| fi |
|
@zhogu will you keep this pr or close this pr? |
This pull request updates the build and deployment workflow for the repository, with a focus on improving the handling of the
alert-managerservice and its dependencies. The main changes involve modifying the workflow to conditionally install special dependencies foralert-managerand updating the.gitignoreto exclude additional generated files.Workflow improvements for alert-manager:
alert-managerfrom the list of changed service folders, ensuring it is included in subsequent processing steps..github/workflows/build-deploy-changes.yaml) now conditionally installs special dependencies (python-icm) foralert-managerif it is among the changed folders. This includes decoding and unzipping a base64-encoded package into the appropriate directory.Repository housekeeping:
.gitignoreforsrc/alert-manageris updated to exclude anykusto-sdkdirectories, preventing them from being tracked by git.Minor cleanup: