Added helm chart renderer #17
Conversation
kyma-bot
added
the
size/XXL
| sampleObj := &unstructured.Unstructured{ | ||
| Object: map[string]interface{}{ | ||
| "kind": "Deployment", | ||
| }, | ||
| } |
There was a problem hiding this comment.
Why do we need this in the test? Isn't the StatefulSet construct enough?
There was a problem hiding this comment.
GetStatefulSets() internally filters the k8s objects and only returns statefulsets, to test this, I added a non-statefulset set objects.
pkg/k8s/chart/types.go
Outdated
| // RenderManifestAsUnStructured of the given chart as unstructured objects. | ||
| RenderManifestAsUnStructured(*ReleaseInstance) (*ManifestResources, error) |
There was a problem hiding this comment.
UnStructured -> Unstructured
| clientGetter := genericclioptions.NewConfigFlags(false) | ||
| clientGetter.Namespace = &namespace | ||
| cfg := new(action.Configuration) | ||
| if err := cfg.Init(clientGetter, namespace, "secrets", c.logger.Debugf); err != nil { |
There was a problem hiding this comment.
What does secrets mean in this context?
There was a problem hiding this comment.
Here is the definition of this method, it seems that its uses secret based client for some internal store.
There was a problem hiding this comment.
I saw different options like (secrets, configmaps, memory, ...etc), do you know what does it mean to use one over the other?
There was a problem hiding this comment.
Not exactly, I re-used the configurations as used by the kyma-reconciler.
There was a problem hiding this comment.
Nothing much mentioned in docs https://pkg.go.dev/helm.sh/helm/v3@v3.11.2/pkg/action#Configuration.Init
| tplAction := action.NewInstall(cfg) | ||
| tplAction.ReleaseName = releaseInstance.Name | ||
| tplAction.Namespace = releaseInstance.Namespace | ||
| tplAction.Atomic = true | ||
| tplAction.Wait = true | ||
| tplAction.CreateNamespace = true | ||
| tplAction.DryRun = true | ||
| tplAction.Replace = true // Skip the name check | ||
| tplAction.IncludeCRDs = true // include CRDs in the templated output | ||
| tplAction.ClientOnly = true // if false, it will validate the manifests against the Kubernetes cluster |
There was a problem hiding this comment.
Maybe in a follow-up ticket, we should think about making some parts of the installation behaviour to be configurable.
There was a problem hiding this comment.
added a task here for it.
pkg/file/exists.go
Outdated
|
|
||
| import "os" | ||
|
|
||
| func DirExists(file string) bool { |
There was a problem hiding this comment.
file string -> path string
| Name string | ||
| Namespace string | ||
| Configuration map[string]interface{} | ||
| RenderedManifests ManifestResources |
There was a problem hiding this comment.
Why is this information exported?
IMO, it is dangerous to allow those fields to be modifiable by any entity that has access to the release instance.
There was a problem hiding this comment.
I only used the struct to pass data among methods. But yes, valid point. I will do it in later PR, as it may fail some things in the parent PR. I have added a task here for it.
| Items []*unstructured.Unstructured | ||
| Blobs [][]byte |
There was a problem hiding this comment.
There was a problem hiding this comment.
I only used the struct to pass data among methods. But yes, valid point. I will do it in later PR, as it may fail some things in the parent PR. I have added a task here for it.
Description
Changes proposed in this pull request:
go fmtandgo imports.Related issue(s)
#11