-
Notifications
You must be signed in to change notification settings - Fork 122
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
[WIP] IEP-1042: Run the installer action with the publishing of release on idf-eclipse-plugin github page #836
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe GitHub Actions workflow has been updated to automate the build process for the Espressif IoT Development Framework (IDF) installer. It is configured to activate on new releases and pull requests, managing tasks from retrieving IDE versions to fetching the latest esp-idf release, and initiating a subsequent action with defined inputs. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Files selected for processing (1)
- .github/workflows/run_idf_installer_build.yml (1 hunks)
Additional comments (Suppressed): 1
.github/workflows/run_idf_installer_build.yml (1)
- 1-21: The GitHub Actions workflow looks well-structured and follows the standard conventions. However, it's important to ensure that the
PERSONAL_ACCESS_TOKEN
secret is set up correctly in the repository settings. This token is used to authenticate thecurl
request that triggers the action in theespressif/idf-installer
repository. If the token is not set or is incorrect, the action will fail. Also, make sure that the token has the necessary permissions to trigger workflows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- .github/workflows/run_idf_installer_build.yml (1 hunks)
Additional comments (Suppressed): 1
.github/workflows/run_idf_installer_build.yml (1)
- 1-52: The new GitHub Actions workflow looks well-structured and follows the standard conventions. However, there are a few points to consider:
The use of a personal access token (
secrets.PERSONAL_ACCESS_TOKEN
) in lines 29, 42. This could potentially expose sensitive information if not handled properly. Make sure that the token has the minimum required permissions and is stored securely as a GitHub secret.The workflow is triggered on both release events and pull requests (lines 4, 7). Ensure that this is the intended behavior as it could lead to unnecessary runs of the workflow on every pull request.
The IDE version is extracted from the release tag (line 21). This assumes that the tag name always starts with 'v'. If this convention changes in the future, the extraction logic will fail.
The IDF version is fetched from the latest release of the
espressif/esp-idf
repository (lines 27-36). This assumes that the latest release always contains the desired IDF version. If a specific IDF version is required, this logic may not work as expected.The Python version is hardcoded as "3.11" in the payload (line 50). If the Python version changes in the future, this will need to be updated manually.
Please verify these points to ensure the workflow functions as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pull_request: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull_request
trigger does not specify event types. If the intention is to run the workflow for all pull request events, the colon after pull_request
should be removed. If specific event types are desired, they need to be listed.
- pull_request:
+ pull_request
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
pull_request: | |
pull_request |
TAG=${{ github.event.release.tag_name }} | ||
IDE_VERSION=${TAG#v} | ||
echo "IDE Version is $IDE_VERSION" | ||
echo "IDE_VERSION=$IDE_VERSION" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IDE version extraction assumes the tag starts with 'v'. If this is not always the case, the script may fail to extract the correct version. A check should be added to ensure the tag starts with 'v' before stripping it.
RESPONSE=$(curl -L \ | ||
-H "Accept: application/vnd.github+json" \ | ||
-H "Authorization: Bearer ${{ secrets.REPO_ACCESS_TOKEN }}" \ | ||
-H "X-GitHub-Api-Version: 2022-11-28" \ | ||
https://api.github.com/repos/espressif/esp-idf/releases/latest) | ||
|
||
TAG_NAME=$(echo "$RESPONSE" | jq -r '.tag_name') | ||
IDF_VERSION=${TAG_NAME#v} | ||
echo "Latest IDF Version is: $IDF_VERSION" | ||
echo "IDF_VERSION=$IDF_VERSION" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a personal access token with potentially broad permissions could be a security risk. Ensure that the token used has the minimum required permissions, or consider fetching the release without authentication if possible.
TAG_NAME=$(echo "$RESPONSE" | jq -r '.tag_name') | ||
IDF_VERSION=${TAG_NAME#v} | ||
echo "Latest IDF Version is: $IDF_VERSION" | ||
echo "IDF_VERSION=$IDF_VERSION" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script assumes that the IDF version tag will always start with 'v'. If this is not always the case, the script may fail to extract the correct version. A check should be added to ensure the tag starts with 'v' before stripping it.
curl -X POST \ | ||
-H "Authorization: Bearer ${{ secrets.REPO_ACCESS_TOKEN }}" \ | ||
-H "Accept: application/vnd.github.v3+json" \ | ||
https://api.github.com/repos/espressif/idf-installer/actions/workflows/build-espressif-ide-installer.yml/dispatches \ | ||
-d '{ | ||
"ref": "main", | ||
"inputs": { | ||
"espressif_ide_version": "'$IDE_VERSION'", | ||
"esp_idf_version": "'$IDF_VERSION'", | ||
"python_version": "3.11" | ||
} | ||
}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script uses a personal access token to trigger the Receiver Action. Ensure that the token used has the minimum required permissions to mitigate security risks.
Description
Run the installer action with the publishing of release on idf-eclipse-plugin github page
Fixes # (IEP-1042)
Type of change
Please delete options that are not relevant.
How has this been tested?
Running and testing the action first by PR and then after merge via a release
Summary by CodeRabbit