Skip to content
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

feat: add render and diff helm action #25

Merged
merged 4 commits into from
Mar 6, 2024
Merged

Conversation

dlactin
Copy link
Contributor

@dlactin dlactin commented Mar 6, 2024

Added a new reusable action to render helm charts on both base and head refs, then post the diff as a comment or comments on the associated pull request.

There are a few changes here from what I demoed in the sandbox:

  1. Using a matrix to split chart rendering into jobs
    This slows down smaller changes but significantly improves performance when multiple charts need to be rendered
  2. Splitting large diffs into multiple comments
    This is using a function adapted from the Atlantis SplitComment code to ensure we are sending comments within the limits defined by GitHub, I have also reused the summary and sep additions to string comments together.
  3. I am excluding files that do not exist in either head or base ref, this was done intentionally to avoid having 100s of comments on a pull request when a new chart is created or removed

@dlactin dlactin requested a review from a team March 6, 2024 18:00
@dlactin dlactin changed the title Add render and diff helm action feat: add render and diff helm action Mar 6, 2024
for chart in ${{ needs.get_changed_helm_charts.outputs.charts }}; do
chart_diff_output=$(diff -r "shared/base-charts/${chart}" "shared/head-charts/${chart}" || true)
if [ -n "$chart_diff_output" ]; then
echo -e "Changes found in chart: ${chart}\n$(diff -ru shared/base-charts/${chart} shared/head-charts/${chart})\n" >> diff.log
Copy link
Contributor

Choose a reason for hiding this comment

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

The unified context for diff shows the new changes with a time of the beginning of epoch. Although I am not sure how to make this any better, just a note since it could be confusing. I looked through diff man page for other alternates, but nothing comes to mind. I think for now it's fine the way it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could potentially use git diff -u --no-index instead which will not have timestamps

@dlactin dlactin merged commit 82f39e5 into main Mar 6, 2024
Copy link

github-actions bot commented Mar 6, 2024

🎉 This PR is included in version 3.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@dlactin dlactin deleted the add-render-and-diff-helm branch March 6, 2024 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants