feat(helm): #471 - Expose rules and aggregateRule in ClusterRole #478

mikesmithgh · 2022-03-21T14:59:26Z

#471 Is requesting to allow value overrides for ClusterRole rules. I also have a use case using aggregationRule so I have added them together in this PR.

mikesmithgh · 2022-03-21T15:02:08Z

Hi @grzesuav @AmitKumarDas , I am leaving this as a draft because I am thinking about adding tests for helm charts. I have used https://github.com/helm/chart-testing-action and think this would be a good addition to metacontroller repo. Thoughts?

codecov · 2022-03-21T15:05:43Z

Codecov Report

Merging #478 (711a7bb) into master (4d1e2de) will not change coverage.
The diff coverage is n/a.

❗ Current head 711a7bb differs from pull request most recent head 9016719. Consider uploading reports for the commit 9016719 to get more accurate results

@@           Coverage Diff           @@
##           master     #478   +/-   ##
=======================================
  Coverage   49.27%   49.27%           
=======================================
  Files          56       56           
  Lines        4980     4980           
=======================================
  Hits         2454     2454           
  Misses       2275     2275           
  Partials      251      251

Flag	Coverage Δ
integration	`43.12% <ø> (-0.11%)`	⬇️
unit	`28.31% <ø> (ø)`

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

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d1e2de...9016719. Read the comment docs.

grzesuav · 2022-03-21T20:25:42Z

@mjsmith1028 good idea. I thing what would be good is single "root" package (jsonnet/helm/anything else)which would be cusotmizable and allow creation two other kinds - helm charts and raw manifests. Need to do some research in that space.

mikesmithgh · 2022-03-24T20:15:32Z

.github/workflows/test_release.yaml

+    needs: ['setmatrix']
+    strategy:
+      fail-fast: false
+      # TODO: ask about skipping CRD versions v1beta because it is not in helm chart


Hi @grzesuav , I have added the github action helm chart testing for this PR. This will check for changes in the chart. If changes are found, deploy a kind cluster and run helm lint/install. It will test all the value files under the deploy/helm/ci directory. I created a symlink for default_values.yaml to values.yaml to make sure the default values are tested. One thing I noticed, is currently the helm chart only includes the v1 CRD and not the previous v1beta CRD. As a result, I removed the v1beta version for kind clusters from this test.

I am thinking either we deprecate the older version for k8s or update the charts in a manner to support both (outside scope of this ticket). Thoughts?

Please let me know if you have any feedback. I will remove from draft to ready for review once this TODO is addressed.

hi, actually we can always run this test, as always at least metacontroller version in helm will change. Removing v1beta1 is ok, I need to fix other PR and set minimal version to 1.16, but will do it as breaking change

Actually, referencing #485 I think I can conditionally add the CRD depending on the version of K8s. I'm going to give that a shot

I don't think you are able to template in the crds directory.
As far as I know this is only possible in the templates directory.

@roelvdberg Yep you are correct, I gave it a shot and no luck. Helm docs say the same. I started to look into using subcharts for the different CRD versions but it became unnecessarily complex. I will continue with ignoring k8s versions that do no support v1 in this PR because it does not break the existing setup. Thanks @grzesuav and @roelvdberg for feedback

.github/workflows/test_release.yaml

grzesuav · 2022-03-30T17:14:09Z

As I understand, currently the helmtest does linting and tries to install chart on kind cluster, right ? I think it is worth to always run this test, as i.e. metacontroller code can change with some runtime option, and we forget to update the chart. In this case I would like to catch this behaviour

mikesmithgh · 2022-03-31T17:27:18Z

As I understand, currently the helmtest does linting and tries to install chart on kind cluster, right ? I think it is worth to always run this test, as i.e. metacontroller code can change with some runtime option, and we forget to update the chart. In this case I would like to catch this behaviour

You are right, this makes sense. I can take off the conditional testing 👍

mikesmithgh · 2022-04-01T14:54:25Z

Hi @grzesuav , this PR is ready for review and merge if you are comfortable. 👍

grzesuav · 2022-04-04T17:57:10Z

@mjsmith1028 can you rebase on latest master ? Cannot automatically merge

…usterRole Signed-off-by: Mike Smith <10135646+mjsmith1028@users.noreply.github.com>

Signed-off-by: Mike Smith <10135646+mjsmith1028@users.noreply.github.com>

mikesmithgh · 2022-04-04T19:30:44Z

@mjsmith1028 can you rebase on latest master ? Cannot automatically merge

thanks @grzesuav , I just rebased :)

grzesuav · 2022-04-05T09:45:15Z

🎉 This PR is included in version 2.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

mikesmithgh force-pushed the helm-rbac-improvements branch 12 times, most recently from 4cb4c72 to a242bd8 Compare March 24, 2022 19:56

mikesmithgh commented Mar 24, 2022

View reviewed changes

mikesmithgh mentioned this pull request Mar 25, 2022

test(helm): Add test for leader election #483

Merged

grzesuav reviewed Mar 30, 2022

View reviewed changes

.github/workflows/test_release.yaml Show resolved Hide resolved

mikesmithgh force-pushed the helm-rbac-improvements branch from 74a0e5b to 4d02352 Compare April 1, 2022 14:28

mikesmithgh marked this pull request as ready for review April 1, 2022 14:53

grzesuav previously approved these changes Apr 4, 2022

View reviewed changes

mikesmithgh added 2 commits April 4, 2022 15:24

feat(helm): metacontroller#471 - Expose rules and aggregateRule in Cl…

37233e8

…usterRole Signed-off-by: Mike Smith <10135646+mjsmith1028@users.noreply.github.com>

ci(helm): Add helm chart testing

9016719

Signed-off-by: Mike Smith <10135646+mjsmith1028@users.noreply.github.com>

mikesmithgh dismissed grzesuav’s stale review via 9016719 April 4, 2022 19:29

mikesmithgh force-pushed the helm-rbac-improvements branch from 711a7bb to 9016719 Compare April 4, 2022 19:29

grzesuav approved these changes Apr 5, 2022

View reviewed changes

grzesuav merged commit 92bd0d7 into metacontroller:master Apr 5, 2022

grzesuav added the released label Apr 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(helm): #471 - Expose rules and aggregateRule in ClusterRole #478

feat(helm): #471 - Expose rules and aggregateRule in ClusterRole #478

mikesmithgh commented Mar 21, 2022 •

edited

Loading

mikesmithgh commented Mar 21, 2022

codecov bot commented Mar 21, 2022 •

edited

Loading

grzesuav commented Mar 21, 2022

mikesmithgh Mar 24, 2022

grzesuav Mar 30, 2022

mikesmithgh Mar 31, 2022

roelvdberg Mar 31, 2022

mikesmithgh Apr 1, 2022

grzesuav commented Mar 30, 2022

mikesmithgh commented Mar 31, 2022

mikesmithgh commented Apr 1, 2022

grzesuav commented Apr 4, 2022

mikesmithgh commented Apr 4, 2022

grzesuav commented Apr 5, 2022

feat(helm): #471 - Expose rules and aggregateRule in ClusterRole #478

feat(helm): #471 - Expose rules and aggregateRule in ClusterRole #478

Conversation

mikesmithgh commented Mar 21, 2022 • edited Loading

mikesmithgh commented Mar 21, 2022

codecov bot commented Mar 21, 2022 • edited Loading

Codecov Report

grzesuav commented Mar 21, 2022

mikesmithgh Mar 24, 2022

Choose a reason for hiding this comment

grzesuav Mar 30, 2022

Choose a reason for hiding this comment

mikesmithgh Mar 31, 2022

Choose a reason for hiding this comment

roelvdberg Mar 31, 2022

Choose a reason for hiding this comment

mikesmithgh Apr 1, 2022

Choose a reason for hiding this comment

grzesuav commented Mar 30, 2022

mikesmithgh commented Mar 31, 2022

mikesmithgh commented Apr 1, 2022

grzesuav commented Apr 4, 2022

mikesmithgh commented Apr 4, 2022

grzesuav commented Apr 5, 2022

mikesmithgh commented Mar 21, 2022 •

edited

Loading

codecov bot commented Mar 21, 2022 •

edited

Loading