Add more helm custom fields #5622

antgamdia · 2022-11-08T17:27:59Z

Description of the change

This PR brings more custom fields to the Helm plugin (repos API). Specifically, I've mimicked the currently customizable values through the values.yaml file. That is: tolerations, nodeSelector, securityContext and proxyOptions.
Notwithstanding, users were able to set their custom PodTemplateSpec for the sync job in the old fashion way, but there's too much information to be included as a custom value in the repos API. But why?

Well, since we are generating code from the grpc messages, if we define a pod_template_spec field of type PodTemplateSpec in the grpc proto file.... we need to have the corresponding type defined.
In doing so, there's two alternatives: 1) using an existing definition and importing it using Buf (via BSR) or 2) maintaining ourselves the grpc messages definition.

Option 1: almost not viable. There is no official definition in BSR, just one coming from a private company with no guarantees whatsoever. I've explored that way, but I don't feel we should go in this direction. Too risky.
Option 2: if we are to maintain the messages, we have two options: 2.1) using the one from Kubernetes or 2.2) defining our own model.
- Option 2.1: the autogenerated file is huge (=every k8s API object model...) and we would have to ensure we're in sync when a new k8s version is out.
- Option 2.2: it would require copying a subset of messages from the k8s API, but keeping just those ones we want.

I've gone with option 2.2, but simplifying the fields we support. Just 1 or 2 objects having simple types like string or bool.
I'm adding some PR review messages to highlight what I mean.

That said, I'm open to any other better ideas you may have.

On the other hand, there's a separate issue: what should we display in the Repos UI?
Well, in this case, I've opted for just showing the proxy options. The rest of the fields are just related to the PodTemplateSpec and I feel they are too complex for even advanced users. If someone really wants to tune them up, they can just go an invoke the API.

Benefits

Users will be able to customize the proxy options (along with a few other options) via the repos API instead of just using the chart initialRepos field.

Possible drawbacks

Not every custom field is available through the UI, but I don't really think we want to.

Applicable issues

fixes Repos API/Helm - Analyze and migrate the former SyncJobTemplate options to the API #5128

Additional information

N/A

Signed-off-by: Antonio Gamez Diaz <[email protected]>

netlify · 2022-11-08T17:28:04Z

✅ Deploy Preview for kubeapps-dev canceled.

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`3f62412`
🔍 Latest deploy log	https://app.netlify.com/sites/kubeapps-dev/deploys/63a1332056a5920009bfb49b

Signed-off-by: Antonio Gamez Diaz <[email protected]>

antgamdia · 2022-11-09T17:11:27Z

cmd/kubeapps-apis/proto/kubeappsapis/plugins/helm/packages/v1alpha1/helm.proto

+
+  // TODO
+  ProxyOptions proxy_options = 5;
+
+  // TODO
+  map<string, string> node_selector = 6;
+
+  // TODO
+  repeated Toleration tolerations = 7;
+
+  // TODO
+  PodSecurityContext security_context = 8;


New options for the Helm plugin (repos API)
[will add the description soon, I just wanted to share my approach with you ASAP]

antgamdia · 2022-11-09T17:13:21Z

cmd/kubeapps-apis/proto/kubeappsapis/plugins/helm/packages/v1alpha1/helm.proto

+// Extracted from the K8s API to avoid a dependency on the K8s API
+// https://github.com/kubernetes/api/blob/master/core/v1/generated.proto
+message PodSecurityContext {
+  optional int64 run_as_user = 1;
+  optional int64 run_as_group = 6;
+  optional bool run_as_non_root = 3;
+  repeated int64 supplemental_groups = 4;
+  optional int64 f_s_group = 5;
+
+  // TODO(agamez): more complex fields are not supported yet
+  // optional SELinuxOptions seLinuxOptions = 1;
+  // optional WindowsSecurityContextOptions windowsOptions = 8;
+  // repeated Sysctl sysctls = 7;
+  // optional string fsGroupChangePolicy = 9;
+  // optional SeccompProfile seccompProfile = 10;
+}


As I said in the PR description, I only kept the most simple fields. This is preventing us from importing the whole proto document from k8s.

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: cmd/kubeapps-apis/docs/kubeapps-apis.swagger.json cmd/kubeapps-apis/gen/plugins/helm/packages/v1alpha1/helm.pb.gw.go

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: cmd/kubeapps-apis/docs/kubeapps-apis.swagger.json cmd/kubeapps-apis/gen/plugins/helm/packages/v1alpha1/helm.pb.go cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories.go

Signed-off-by: Antonio Gamez Diaz <[email protected]>

…8-addMoreHelmFields

Signed-off-by: Antonio Gamez Diaz <[email protected]>

absoludity

LGTM, after a rebase/resolve. Thanks Antonio!

cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories.go

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: chart/kubeapps/README.md cmd/kubeapps-apis/docs/kubeapps-apis.swagger.json

Co-authored-by: Michael Nelson <[email protected]> Signed-off-by: Antonio Gámez, PhD <[email protected]>

absoludity · 2022-12-20T03:51:27Z

I've just merged main again and fixed the conflict. Should be good to land once tests pass.

Signed-off-by: Michael Nelson <[email protected]>

antgamdia added 24 commits October 28, 2022 17:38

Change "Replace" str with the exported str from batchv1

99e1552

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add Interval to the AppRepo spec

0f082cc

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'main' into 3661-appRepoCustomInterval

8ae0e0b

Add test case

37cf53d

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Fix typo

f84221b

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add explicit error handling in intervalToCron

9c0b85f

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add interval to the crd openapi model

a1147e3

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Fix interval generation syntax

b23e055

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Use the apprepo.Interval in the plugin

72a3259

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Use the interval in the repo form even if using helm plugin

030be87

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Fix linter issue

b9a5265

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Change apprepository.crontab msg

ac5a047

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'main' into 3661-appRepoCustomInterval

010f1d8

Use the field in the get repo details. Disable field for helm

2960a2b

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Fix disable condition

a6eae43

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'main' into 3661-appRepoCustomInterval

95c1775

prevent invalid cron lines

6042c2d

Signed-off-by: Antonio Gamez Diaz <[email protected]>

fix wrong interval examples

04b02f5

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add more replacements with their respective constants

f3344a4

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Trigger AppRepo controller sync on every change

1b65469

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add more fields to helm custom details

cc38302

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add autogen files

fed55d1

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Handle new custom fields when adding helm repo

4e30429

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add proxy fields in the repo UI

2d40330

Signed-off-by: Antonio Gamez Diaz <[email protected]>

vmwclabot added the cla-not-required label Nov 8, 2022

antgamdia added 2 commits November 9, 2022 13:08

Fix typos

43efb9d

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add more interval formats in the api doc

00f9dc0

Signed-off-by: Antonio Gamez Diaz <[email protected]>

antgamdia commented Nov 9, 2022

View reviewed changes

Merge branch '3661-appRepoCustomInterval'

2fa8d0e

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: cmd/kubeapps-apis/docs/kubeapps-apis.swagger.json cmd/kubeapps-apis/gen/plugins/helm/packages/v1alpha1/helm.pb.gw.go

antgamdia added 4 commits November 9, 2022 18:42

Fix test case

dd2858b

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge branch 'main'

f31aeb7

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: cmd/kubeapps-apis/docs/kubeapps-apis.swagger.json cmd/kubeapps-apis/gen/plugins/helm/packages/v1alpha1/helm.pb.go cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories.go

Run buf generate

136aeaa

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Remove todo

18c15e3

Signed-off-by: Antonio Gamez Diaz <[email protected]>

antgamdia marked this pull request as ready for review November 21, 2022 17:38

antgamdia added 8 commits November 21, 2022 18:39

Merge branch 'main' into 5128-addMoreHelmFields

cda2999

Fix wrong initialization

b8af022

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Fix var names

d040417

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Use custom details when updating repo

347c9ca

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Add test cases

04a60b4

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Merge remote-tracking branch 'origin/5128-addMoreHelmFields' into 512…

f829334

…8-addMoreHelmFields

Fix netlify issue with chart readme

981a296

Signed-off-by: Antonio Gamez Diaz <[email protected]>

Fix unhandled type conversion

271ffc9

Signed-off-by: Antonio Gamez Diaz <[email protected]>

absoludity approved these changes Dec 14, 2022

View reviewed changes

cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories.go Outdated Show resolved Hide resolved

antgamdia and others added 3 commits December 14, 2022 18:06

Merge branch 'main'

91fa839

Signed-off-by: Antonio Gamez Diaz <[email protected]> Conflicts: chart/kubeapps/README.md cmd/kubeapps-apis/docs/kubeapps-apis.swagger.json

Update cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories.go

457ccb1

Co-authored-by: Michael Nelson <[email protected]> Signed-off-by: Antonio Gámez, PhD <[email protected]>

Merge branch 'main' into 5128-addMoreHelmFields

dafe67e

Prettier

3f62412

Signed-off-by: Michael Nelson <[email protected]>

absoludity merged commit 5202eb6 into vmware-tanzu:main Dec 20, 2022

antgamdia deleted the 5128-addMoreHelmFields branch December 18, 2023 10:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more helm custom fields #5622

Add more helm custom fields #5622

antgamdia commented Nov 8, 2022 •

edited

Loading

netlify bot commented Nov 8, 2022 •

edited

Loading

antgamdia Nov 9, 2022

antgamdia Nov 9, 2022

absoludity left a comment

absoludity commented Dec 20, 2022

Add more helm custom fields #5622

Add more helm custom fields #5622

Conversation

antgamdia commented Nov 8, 2022 • edited Loading

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

netlify bot commented Nov 8, 2022 • edited Loading

✅ Deploy Preview for kubeapps-dev canceled.

antgamdia Nov 9, 2022

Choose a reason for hiding this comment

antgamdia Nov 9, 2022

Choose a reason for hiding this comment

absoludity left a comment

Choose a reason for hiding this comment

absoludity commented Dec 20, 2022

antgamdia commented Nov 8, 2022 •

edited

Loading

netlify bot commented Nov 8, 2022 •

edited

Loading