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(helm-chart): make charts Openshift compliant #3415

Merged
merged 9 commits into from
Apr 23, 2024

Conversation

smuda
Copy link
Contributor

@smuda smuda commented Apr 13, 2024

Description

Allow removal of runAsUser and runAsGroup when running on openshift.

Fixes #3414

How to test

I've added a test under .github/scripts/.helm-tests and I hope that will run automatically together with the other tests, even though the github actions didn't run in my fork.

At least make helm-test works nicely :-).

Checklist

  • My PR fulfills the Definition of Done of the corresponding issue and not more (or parts if the issue is separated
    into multiple PRs)
  • I used descriptive commit messages to help reviewers understand my thought process
  • I signed off all my commits according to the Developer Certificate of Origin (DCO)
    see Contribution Guide
  • My PR title is formatted according to the semantic PR conventions described in
    the Contribution Guide
  • My code follows the style guidelines of this project (golangci-lint passes, YAMLLint passes)
  • I regenerated the auto-generated docs for Helm and the CRD documentation (if applicable)
  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation (if needed)
  • My changes result in all-green PR checks (first-time contributors need to ask a maintainer to approve their test runs)
  • New and existing unit and integration tests pass locally with my changes

@smuda smuda requested review from a team as code owners April 13, 2024 04:04
@smuda smuda force-pushed the feature/make-chart-openshift-compliant branch from a9ecacc to 76ac9c0 Compare April 13, 2024 06:16
@smuda smuda changed the title Make charts Openshift compliant fix: Make charts Openshift compliant Apr 13, 2024
@smuda smuda force-pushed the feature/make-chart-openshift-compliant branch 4 times, most recently from f7ee0cb to 72142ea Compare April 13, 2024 06:35
@smuda smuda changed the title fix: Make charts Openshift compliant fix: make charts Openshift compliant Apr 13, 2024
Copy link
Contributor

@StackScribe StackScribe left a comment

Choose a reason for hiding this comment

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

Very nice start! Thanks for doing this. Just a few little comments on the documentation portions. The only big issue is that I think this belongs on its own page in the "Configuration" section.

To create a new page, move the contents to the contributing sub-section and follow the instructions in https://keptn.sh/stable/docs/contribute/docs/source-file-structure/#specifying-the-doc-structure to add this file to the mkdocs.yml file.

docs/docs/installation/index.md Outdated Show resolved Hide resolved
docs/docs/installation/index.md Outdated Show resolved Hide resolved
docs/docs/installation/index.md Outdated Show resolved Hide resolved
docs/docs/installation/index.md Outdated Show resolved Hide resolved
docs/docs/installation/index.md Outdated Show resolved Hide resolved
docs/docs/installation/index.md Outdated Show resolved Hide resolved
lifecycle-operator/chart/README.md Outdated Show resolved Hide resolved
metrics-operator/chart/README.md Outdated Show resolved Hide resolved
Copy link

codecov bot commented Apr 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.33%. Comparing base (ed152f0) to head (87cae22).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3415      +/-   ##
==========================================
+ Coverage   85.29%   85.33%   +0.04%     
==========================================
  Files         167      167              
  Lines        7412     7412              
==========================================
+ Hits         6322     6325       +3     
+ Misses        801      799       -2     
+ Partials      289      288       -1     

see 1 file with indirect coverage changes

Flag Coverage Δ
certificate-operator 69.23% <ø> (ø)
component-tests 58.04% <ø> (+0.24%) ⬆️
lifecycle-operator 83.46% <ø> (ø)
metrics-operator 88.32% <ø> (ø)
scheduler 34.74% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

.github/scripts/.helm-tests/Openshift/values.yaml Outdated Show resolved Hide resolved
chart/README.md Outdated Show resolved Hide resolved
@mowies mowies changed the title fix: make charts Openshift compliant feat(helm-charts): make charts Openshift compliant Apr 15, 2024
@mowies mowies changed the title feat(helm-charts): make charts Openshift compliant feat(helm-chart): make charts Openshift compliant Apr 15, 2024
@odubajDT
Copy link
Contributor

Hi @smuda thank you for your contribution! Do you please have the capacity to make the required changes in the following days? We are planning to do a release in the next days and we would like to have this fix merged until then. If you do not have the capacity, we will take over the PR.

Thank you in advance!

@smuda
Copy link
Contributor Author

smuda commented Apr 22, 2024

Thank you! I'll fix this tonight.

@odubajDT
Copy link
Contributor

Thank you! I'll fix this tonight.

@smuda thank you!

@smuda
Copy link
Contributor Author

smuda commented Apr 22, 2024

@StackScribe Thank you for the docs changes!

@smuda smuda force-pushed the feature/make-chart-openshift-compliant branch from 368a008 to e36fd1b Compare April 22, 2024 18:26
@smuda smuda force-pushed the feature/make-chart-openshift-compliant branch from e36fd1b to e1c87c0 Compare April 22, 2024 18:33
@smuda
Copy link
Contributor Author

smuda commented Apr 22, 2024

I rebased on top of main and handled most of your suggestions/comments. Could you please allow the rest of the tests to run?

@smuda
Copy link
Contributor Author

smuda commented Apr 22, 2024

Not many OSS have great documentation and you have even documented how to document. Impressive and awesome!

Copy link
Contributor

@odubajDT odubajDT left a comment

Choose a reason for hiding this comment

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

Awesome work! Some minor comments for the documentation :)

docs/docs/installation/configuration/Openshift.md Outdated Show resolved Hide resolved
docs/docs/installation/configuration/Openshift.md Outdated Show resolved Hide resolved
docs/docs/installation/configuration/Openshift.md Outdated Show resolved Hide resolved
chart/README.md Show resolved Hide resolved
chart/values.yaml Show resolved Hide resolved
keptn-cert-manager/chart/README.md Show resolved Hide resolved
keptn-cert-manager/chart/values.yaml Show resolved Hide resolved
lifecycle-operator/chart/README.md Show resolved Hide resolved
lifecycle-operator/chart/values.yaml Show resolved Hide resolved
metrics-operator/chart/README.md Show resolved Hide resolved
metrics-operator/chart/values.yaml Show resolved Hide resolved
Copy link

sonarcloud bot commented Apr 23, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@odubajDT odubajDT merged commit 32f077a into keptn:main Apr 23, 2024
42 checks passed
@smuda smuda deleted the feature/make-chart-openshift-compliant branch April 23, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove runAsUser & runAsGroup on Openshift
5 participants