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.

Sign up for GitHub

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

Jump to bottom

Add Helm chart lint tool #1679

Merged
merged 26 commits into from May 26, 2023
Merged

Add Helm chart lint tool #1679

merged 26 commits into from May 26, 2023

Conversation

viniciusdc
Copy link
Contributor

@viniciusdc viniciusdc commented Mar 14, 2023 •
edited

Reference Issues or PRs

closes #1313

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

Any other comments?

iameskild and others added 12 commits March 14, 2023 00:42
@iameskild @aktech
@pavithraes @nebari-sensei @limacarvalho @trallard @alimanfoo @costrouc @pre-commit-ci @viniciusdc
Merge develop into main for release 2023.1.1 (#1637)
894ba2c 
Co-authored-by: Amit Kumar <dtu.amit@gmail.com>
Co-authored-by: Pavithra Eswaramoorthy <pavithraes@outlook.com>
Co-authored-by: Nebari-sensei <116370392+nebari-sensei@users.noreply.github.com>
Co-authored-by: João Carvalho <joaocarvalho12@gmail.com>
Co-authored-by: iameskild <eskild@doublee.io>
Co-authored-by: Tania Allard <taniar.allard@gmail.com>
Co-authored-by: Alistair Miles <alimanfoo@googlemail.com>
Co-authored-by: Christopher Ostrouchov <chris.ostrouchov@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@viniciusdc
add HCL helm provider parser
253c5cb
@viniciusdc
Initial version for helm validation/parser
de1a6ea
@viniciusdc
initial test workflow
27d8d07
@viniciusdc
undo incorrect rebase changes
bab85f9
@pre-commit-ci
[pre-commit.ci] Apply automatic pre-commit fixes
7e57f5b
@viniciusdc
Check current runner dir
6491fd6
@pre-commit-ci
[pre-commit.ci] Apply automatic pre-commit fixes
226f587
@viniciusdc
fix script name call
39f85c5
@pre-commit-ci
[pre-commit.ci] Apply automatic pre-commit fixes
7211335
@viniciusdc
Install nebari (optional)
cac62ff
@pre-commit-ci
[pre-commit.ci] Apply automatic pre-commit fixes
0b90e13
@viniciusdc
Copy link
Contributor Author

for a preview https://github.com/nebari-dev/nebari/actions/runs/4412080075/jobs/7731193311

@viniciusdc viniciusdc mentioned this pull request Mar 14, 2023
Closed
@iameskild
Copy link
Member

Hey @viniciusdc, is this ready for review yet? What else does this PR need before it can be merged?

@viniciusdc viniciusdc changed the title WIP - Helm chart lint Add Helm chart lint tool Apr 25, 2023
@viniciusdc
Copy link
Contributor Author

Hi @iameskild, it needed just some cleaning, I completed it and its ready for a review. Here's the look form the ouputed information into the action summary when the workflow susceeds

image

@viniciusdc viniciusdc added needs: review 👀 This PR is complete and ready for reviewing and removed status: in progress 🏗 This task is currently being worked on labels Apr 25, 2023
iameskild
iameskild reviewed May 2, 2023
View reviewed changes
Copy link
Member

@iameskild iameskild left a comment

Choose a reason for hiding this comment

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

This is looking great @viniciusdc! I'm glad we're starting to track our dependencies better and something like this is surely needed :)

I add some comments about being generalizing this script to make it more generalizable. Perhaps we can even consider adding this to the tests/test_dependencies.py, what are your thoughts?

.github/workflows/test_helm_charts.yaml
Comment on lines +31 to +32
pip install python-hcl2
pip install tqdm
Copy link
Member

Choose a reason for hiding this comment

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

Should we include these in our dev dependencies? Theoretically, we could run this script locally?

scripts/helm-validate.py Show resolved Hide resolved
@pavithraes pavithraes added needs: changes 🧱 Review completed - some changes are needed before merging and removed needs: review 👀 This PR is complete and ready for reviewing labels May 3, 2023
@viniciusdc
Copy link
Contributor Author

Found a problem with the validation, will have this fixed in a few minutes -- needs a double check

@viniciusdc
Copy link
Contributor Author

Fixed, all tests are green. I also made it so that this job runs on CRON not to block any user contribution in the future (in case we spot a problem with the helm source).
Sample summary : https://github.com/nebari-dev/nebari/actions/runs/4928157074#summary-13349356607

@viniciusdc viniciusdc added needs: review 👀 This PR is complete and ready for reviewing and removed needs: changes 🧱 Review completed - some changes are needed before merging labels May 10, 2023
iameskild
iameskild approved these changes May 18, 2023
View reviewed changes
Copy link
Member

@iameskild iameskild left a comment

Choose a reason for hiding this comment

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

I'm glad to see more of our dependencies being tested! Thanks @viniciusdc :)

@iameskild iameskild added status: approved 💪🏾 This PR has been reviewed and approved for merge area: dependencies 📦 All things dependencies and removed needs: review 👀 This PR is complete and ready for reviewing labels May 18, 2023
@pavithraes
Copy link
Member

@viniciusdc @iameskild Thanks for working on and reviewing this PR! Shall we merge?

@iameskild iameskild merged commit 467bcef into develop May 26, 2023
23 of 26 checks passed
@iameskild iameskild deleted the helm-chart-lint branch May 26, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iameskild iameskild iameskild approved these changes

Assignees
No one assigned
Labels
area: CI/CD 👷🏽‍♀️ area: dependencies 📦 All things dependencies area: testing ✅ Testing status: approved 💪🏾 This PR has been reviewed and approved for merge
Projects
🪴 Nebari Project Management
Status: Done 💪🏾
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ENH] Add helm chart validator
3 participants
@viniciusdc @iameskild @pavithraes