diff --git a/api/v1alpha1/helmchartproxy_types.go b/api/v1alpha1/helmchartproxy_types.go index 5545871d..7982c988 100644 --- a/api/v1alpha1/helmchartproxy_types.go +++ b/api/v1alpha1/helmchartproxy_types.go @@ -67,7 +67,7 @@ type HelmChartProxySpec struct { // Options represents CLI flags passed to Helm operations (i.e. install, upgrade, delete) and // include options such as wait, skipCRDs, timeout, waitForJobs, etc. // +optional - Options *HelmOptions `json:"options,omitempty"` + Options HelmOptions `json:"options,omitempty"` // Credentials is a reference to an object containing the OCI credentials. If it is not specified, no credentials will be used. // +optional @@ -119,12 +119,12 @@ type HelmOptions struct { // Install represents CLI flags passed to Helm install operation which can be used to control // behaviour of helm Install operations via options like wait, skipCrds, timeout, waitForJobs, etc. // +optional - Install *HelmInstallOptions `json:"install,omitempty"` + Install HelmInstallOptions `json:"install,omitempty"` // Upgrade represents CLI flags passed to Helm upgrade operation which can be used to control // behaviour of helm Upgrade operations via options like wait, skipCrds, timeout, waitForJobs, etc. // +optional - Upgrade *HelmUpgradeOptions `json:"upgrade,omitempty"` + Upgrade HelmUpgradeOptions `json:"upgrade,omitempty"` // Uninstall represents CLI flags passed to Helm uninstall operation which can be used to control // behaviour of helm Uninstall operation via options like wait, timeout, etc. @@ -132,8 +132,9 @@ type HelmOptions struct { Uninstall *HelmUninstallOptions `json:"uninstall,omitempty"` // EnableClientCache is a flag to enable Helm client cache. If it is not specified, it will be set to true. + // +kubebuilder:default=false // +optional - EnableClientCache *bool `json:"enableClientCache,omitempty"` + EnableClientCache bool `json:"enableClientCache,omitempty"` } type HelmInstallOptions struct { @@ -141,8 +142,9 @@ type HelmInstallOptions struct { // HelmChartProxySpec.ReleaseNamespace if it does not exist yet. // On uninstall, the namespace will not be garbage collected. // If it is not specified by user, will be set to default 'true'. + // +kubebuilder:default=true // +optional - CreateNamespace *bool `json:"createNamespace,omitempty"` + CreateNamespace bool `json:"createNamespace,omitempty"` // IncludeCRDs determines whether CRDs stored as a part of helm templates directory should be installed. // +optional @@ -167,7 +169,8 @@ type HelmUpgradeOptions struct { // +optional Recreate bool `json:"recreate,omitempty"` - // MaxHistory limits the maximum number of revisions saved per release + // MaxHistory limits the maximum number of revisions saved per release (default is 10). + // +kubebuilder:default=10 // +optional MaxHistory int `json:"maxHistory,omitempty"` diff --git a/api/v1alpha1/helmchartproxy_webhook.go b/api/v1alpha1/helmchartproxy_webhook.go index 71dc5a44..4062e1ec 100644 --- a/api/v1alpha1/helmchartproxy_webhook.go +++ b/api/v1alpha1/helmchartproxy_webhook.go @@ -23,7 +23,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -45,7 +44,7 @@ func (r *HelmChartProxy) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Defaulter = &HelmChartProxy{} -const helmTimeout = time.Second * 600 +const helmTimeout = 10 * time.Minute // Default implements webhook.Defaulter so a webhook will be registered for the type. func (p *HelmChartProxy) Default() { @@ -55,18 +54,13 @@ func (p *HelmChartProxy) Default() { p.Spec.ReleaseNamespace = "default" } - // If 'Wait' is set, we need to set default 'Timeout' to make install successful. - if p.Spec.Options != nil && p.Spec.Options.Wait && p.Spec.Options.Timeout == nil { - p.Spec.Options.Timeout = &metav1.Duration{Duration: helmTimeout} + if p.Spec.Options.Atomic { + p.Spec.Options.Wait = true } - // If 'CreateNamespace' is not specified by user, set default value to 'true' - if p.Spec.Options != nil { - if p.Spec.Options.Install == nil { - p.Spec.Options.Install = &HelmInstallOptions{CreateNamespace: ptr.To(true)} - } else if p.Spec.Options.Install.CreateNamespace == nil { - p.Spec.Options.Install.CreateNamespace = ptr.To(true) - } + // Note: timeout is also needed to ensure that Spec.Options.Wait works. + if p.Spec.Options.Timeout == nil { + p.Spec.Options.Timeout = &metav1.Duration{Duration: helmTimeout} } } diff --git a/api/v1alpha1/helmreleaseproxy_types.go b/api/v1alpha1/helmreleaseproxy_types.go index 04af7827..f5a11d97 100644 --- a/api/v1alpha1/helmreleaseproxy_types.go +++ b/api/v1alpha1/helmreleaseproxy_types.go @@ -69,7 +69,7 @@ type HelmReleaseProxySpec struct { // Options represents the helm setting options which can be used to control behaviour of helm operations(Install, Upgrade, Delete, etc) // via options like wait, skipCrds, timeout, waitForJobs, etc. // +optional - Options *HelmOptions `json:"options,omitempty"` + Options HelmOptions `json:"options,omitempty"` // Credentials is a reference to an object containing the OCI credentials. If it is not specified, no credentials will be used. // +optional diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index dee84675..b6f01426 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -106,11 +106,7 @@ func (in *HelmChartProxyList) DeepCopyObject() runtime.Object { func (in *HelmChartProxySpec) DeepCopyInto(out *HelmChartProxySpec) { *out = *in in.ClusterSelector.DeepCopyInto(&out.ClusterSelector) - if in.Options != nil { - in, out := &in.Options, &out.Options - *out = new(HelmOptions) - (*in).DeepCopyInto(*out) - } + in.Options.DeepCopyInto(&out.Options) if in.Credentials != nil { in, out := &in.Credentials, &out.Credentials *out = new(Credentials) @@ -158,11 +154,6 @@ func (in *HelmChartProxyStatus) DeepCopy() *HelmChartProxyStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HelmInstallOptions) DeepCopyInto(out *HelmInstallOptions) { *out = *in - if in.CreateNamespace != nil { - in, out := &in.CreateNamespace, &out.CreateNamespace - *out = new(bool) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HelmInstallOptions. @@ -183,26 +174,13 @@ func (in *HelmOptions) DeepCopyInto(out *HelmOptions) { *out = new(v1.Duration) **out = **in } - if in.Install != nil { - in, out := &in.Install, &out.Install - *out = new(HelmInstallOptions) - (*in).DeepCopyInto(*out) - } - if in.Upgrade != nil { - in, out := &in.Upgrade, &out.Upgrade - *out = new(HelmUpgradeOptions) - **out = **in - } + out.Install = in.Install + out.Upgrade = in.Upgrade if in.Uninstall != nil { in, out := &in.Uninstall, &out.Uninstall *out = new(HelmUninstallOptions) **out = **in } - if in.EnableClientCache != nil { - in, out := &in.EnableClientCache, &out.EnableClientCache - *out = new(bool) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HelmOptions. @@ -278,11 +256,7 @@ func (in *HelmReleaseProxyList) DeepCopyObject() runtime.Object { func (in *HelmReleaseProxySpec) DeepCopyInto(out *HelmReleaseProxySpec) { *out = *in out.ClusterRef = in.ClusterRef - if in.Options != nil { - in, out := &in.Options, &out.Options - *out = new(HelmOptions) - (*in).DeepCopyInto(*out) - } + in.Options.DeepCopyInto(&out.Options) if in.Credentials != nil { in, out := &in.Credentials, &out.Credentials *out = new(Credentials) diff --git a/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml b/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml index 72966b9e..1d62b898 100644 --- a/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml +++ b/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml @@ -157,6 +157,7 @@ spec: validation is enforced. type: boolean enableClientCache: + default: false description: EnableClientCache is a flag to enable Helm client cache. If it is not specified, it will be set to true. type: boolean @@ -166,6 +167,7 @@ spec: behaviour of helm Install operations via options like wait, skipCrds, timeout, waitForJobs, etc. properties: createNamespace: + default: true description: |- CreateNamespace indicates the Helm install/upgrade action to create the HelmChartProxySpec.ReleaseNamespace if it does not exist yet. @@ -224,8 +226,9 @@ spec: This should be used with caution. type: boolean maxHistory: + default: 10 description: MaxHistory limits the maximum number of revisions - saved per release + saved per release (default is 10). type: integer recreate: description: Recreate will (if true) recreate pods after a diff --git a/config/crd/bases/addons.cluster.x-k8s.io_helmreleaseproxies.yaml b/config/crd/bases/addons.cluster.x-k8s.io_helmreleaseproxies.yaml index 68306f1b..9aeae0b2 100644 --- a/config/crd/bases/addons.cluster.x-k8s.io_helmreleaseproxies.yaml +++ b/config/crd/bases/addons.cluster.x-k8s.io_helmreleaseproxies.yaml @@ -166,6 +166,7 @@ spec: validation is enforced. type: boolean enableClientCache: + default: false description: EnableClientCache is a flag to enable Helm client cache. If it is not specified, it will be set to true. type: boolean @@ -175,6 +176,7 @@ spec: behaviour of helm Install operations via options like wait, skipCrds, timeout, waitForJobs, etc. properties: createNamespace: + default: true description: |- CreateNamespace indicates the Helm install/upgrade action to create the HelmChartProxySpec.ReleaseNamespace if it does not exist yet. @@ -233,8 +235,9 @@ spec: This should be used with caution. type: boolean maxHistory: + default: 10 description: MaxHistory limits the maximum number of revisions - saved per release + saved per release (default is 10). type: integer recreate: description: Recreate will (if true) recreate pods after a diff --git a/controllers/helmchartproxy/helmchartproxy_controller_phases.go b/controllers/helmchartproxy/helmchartproxy_controller_phases.go index a037dc01..75c7566c 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller_phases.go +++ b/controllers/helmchartproxy/helmchartproxy_controller_phases.go @@ -34,8 +34,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -var defaultEnableClientCache = true - // deleteOrphanedHelmReleaseProxies deletes any HelmReleaseProxy resources that belong to a Cluster that is not selected by its parent HelmChartProxy. func (r *HelmChartProxyReconciler) deleteOrphanedHelmReleaseProxies(ctx context.Context, helmChartProxy *addonsv1alpha1.HelmChartProxy, clusters []clusterv1.Cluster, helmReleaseProxies []addonsv1alpha1.HelmReleaseProxy) error { log := ctrl.LoggerFrom(ctx) @@ -229,15 +227,6 @@ func constructHelmReleaseProxy(existing *addonsv1alpha1.HelmReleaseProxy, helmCh } } - // Set the default value for EnableClientCache if it is not set - if helmReleaseProxy.Spec.Options == nil { - helmReleaseProxy.Spec.Options = &addonsv1alpha1.HelmOptions{} - } - - if helmReleaseProxy.Spec.Options.EnableClientCache == nil { - helmReleaseProxy.Spec.Options.EnableClientCache = &defaultEnableClientCache - } - return helmReleaseProxy } diff --git a/controllers/helmchartproxy/helmchartproxy_controller_phases_test.go b/controllers/helmchartproxy/helmchartproxy_controller_phases_test.go index fb870615..9c4516d7 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller_phases_test.go +++ b/controllers/helmchartproxy/helmchartproxy_controller_phases_test.go @@ -19,6 +19,7 @@ package helmchartproxy import ( "fmt" "testing" + "time" "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" @@ -52,7 +53,12 @@ var ( ReleaseNamespace: "test-release-namespace", Version: "test-version", ValuesTemplate: "apiServerPort: {{ .Cluster.spec.clusterNetwork.apiServerPort }}", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, } @@ -72,7 +78,12 @@ var ( ReleaseNamespace: "test-release-namespace", Version: "test-version", ValuesTemplate: "cidrBlockList: {{ .Cluster.spec.clusterNetwork.pods.cidrBlocks | join \",\" }}", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, } @@ -92,7 +103,12 @@ var ( ReleaseNamespace: "test-release-namespace", Version: "test-version", ValuesTemplate: "apiServerPort: {{ .Cluster.invalid-path }}", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, } @@ -112,7 +128,12 @@ var ( ReleaseNamespace: "test-release-namespace", Version: "test-version", ValuesTemplate: "apiServerPort: {{ .Cluster.spec.clusterNetwork.apiServerPort }}", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, } @@ -185,7 +206,12 @@ var ( ReleaseNamespace: "test-release-namespace", Version: "test-version", Values: "apiServerPort: 6443", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, } ) @@ -347,7 +373,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ReleaseNamespace: "test-release-namespace", Values: "test-parsed-values", Version: "test-version", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, }, helmChartProxy: &addonsv1alpha1.HelmChartProxy{ @@ -365,7 +396,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", Version: "test-version", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, }, parsedValues: "test-parsed-values", @@ -398,76 +434,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ChartName: "test-chart-name", RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", - Options: &addonsv1alpha1.HelmOptions{}, - }, - }, - parsedValues: "test-parsed-values", - cluster: &clusterv1.Cluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: clusterv1.GroupVersion.String(), - Kind: "Cluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "test-namespace", - }, - }, - expected: &addonsv1alpha1.HelmReleaseProxy{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-chart-name-test-cluster-", - Namespace: "test-namespace", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: addonsv1alpha1.GroupVersion.String(), - Kind: "HelmChartProxy", - Name: "test-hcp", - Controller: ptr.To(true), - BlockOwnerDeletion: ptr.To(true), + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, }, }, - Labels: map[string]string{ - clusterv1.ClusterNameLabel: "test-cluster", - addonsv1alpha1.HelmChartProxyLabelName: "test-hcp", - }, - }, - Spec: addonsv1alpha1.HelmReleaseProxySpec{ - ClusterRef: corev1.ObjectReference{ - APIVersion: clusterv1.GroupVersion.String(), - Kind: "Cluster", - Name: "test-cluster", - Namespace: "test-namespace", - }, - ReleaseName: "test-release-name", - ChartName: "test-chart-name", - RepoURL: "https://test-repo-url", - ReleaseNamespace: "test-release-namespace", - Values: "test-parsed-values", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(true), - }, - }, - }, - }, - { - name: "construct helm release proxy without existing disable client cache", - existing: nil, - helmChartProxy: &addonsv1alpha1.HelmChartProxy{ - TypeMeta: metav1.TypeMeta{ - APIVersion: addonsv1alpha1.GroupVersion.String(), - Kind: "HelmChartProxy", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-hcp", - Namespace: "test-namespace", - }, - Spec: addonsv1alpha1.HelmChartProxySpec{ - ReleaseName: "test-release-name", - ChartName: "test-chart-name", - RepoURL: "https://test-repo-url", - ReleaseNamespace: "test-release-namespace", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(false), - }, }, }, parsedValues: "test-parsed-values", @@ -511,78 +483,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", Values: "test-parsed-values", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(false), - }, - }, - }, - }, - { - name: "construct helm release proxy without existing enable client cache", - existing: nil, - helmChartProxy: &addonsv1alpha1.HelmChartProxy{ - TypeMeta: metav1.TypeMeta{ - APIVersion: addonsv1alpha1.GroupVersion.String(), - Kind: "HelmChartProxy", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-hcp", - Namespace: "test-namespace", - }, - Spec: addonsv1alpha1.HelmChartProxySpec{ - ReleaseName: "test-release-name", - ChartName: "test-chart-name", - RepoURL: "https://test-repo-url", - ReleaseNamespace: "test-release-namespace", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(true), - }, - }, - }, - parsedValues: "test-parsed-values", - cluster: &clusterv1.Cluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: clusterv1.GroupVersion.String(), - Kind: "Cluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "test-namespace", - }, - }, - expected: &addonsv1alpha1.HelmReleaseProxy{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-chart-name-test-cluster-", - Namespace: "test-namespace", - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: addonsv1alpha1.GroupVersion.String(), - Kind: "HelmChartProxy", - Name: "test-hcp", - Controller: ptr.To(true), - BlockOwnerDeletion: ptr.To(true), + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, }, }, - Labels: map[string]string{ - clusterv1.ClusterNameLabel: "test-cluster", - addonsv1alpha1.HelmChartProxyLabelName: "test-hcp", - }, - }, - Spec: addonsv1alpha1.HelmReleaseProxySpec{ - ClusterRef: corev1.ObjectReference{ - APIVersion: clusterv1.GroupVersion.String(), - Kind: "Cluster", - Name: "test-cluster", - Namespace: "test-namespace", - }, - ReleaseName: "test-release-name", - ChartName: "test-chart-name", - RepoURL: "https://test-repo-url", - ReleaseNamespace: "test-release-namespace", - Values: "test-parsed-values", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(true), - }, }, }, }, @@ -619,7 +525,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ReleaseNamespace: "test-release-namespace", Values: "test-parsed-values", Version: "test-version", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, }, helmChartProxy: &addonsv1alpha1.HelmChartProxy{ @@ -637,7 +548,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", Version: "another-version", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, }, parsedValues: "test-parsed-values", @@ -682,8 +598,11 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ReleaseNamespace: "test-release-namespace", Values: "test-parsed-values", Version: "another-version", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(true), + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, }, }, }, @@ -721,7 +640,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ReleaseNamespace: "test-release-namespace", Values: "test-parsed-values", Version: "test-version", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, }, helmChartProxy: &addonsv1alpha1.HelmChartProxy{ @@ -739,7 +663,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", Version: "test-version", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, }, }, parsedValues: "updated-parsed-values", @@ -784,8 +713,11 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ReleaseNamespace: "test-release-namespace", Values: "updated-parsed-values", Version: "test-version", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(true), + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, }, }, }, @@ -807,7 +739,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ChartName: "test-chart-name", RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, Credentials: &addonsv1alpha1.Credentials{ Secret: corev1.SecretReference{ Name: "test-secret", @@ -854,8 +791,11 @@ func TestConstructHelmReleaseProxy(t *testing.T) { RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", Values: "test-parsed-values", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(true), + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, }, Credentials: &addonsv1alpha1.Credentials{ Secret: corev1.SecretReference{ @@ -884,7 +824,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ChartName: "test-chart-name", RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, Credentials: &addonsv1alpha1.Credentials{ Secret: corev1.SecretReference{ Name: "test-secret", @@ -932,8 +877,11 @@ func TestConstructHelmReleaseProxy(t *testing.T) { RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", Values: "test-parsed-values", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(true), + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, }, Credentials: &addonsv1alpha1.Credentials{ Secret: corev1.SecretReference{ @@ -962,7 +910,12 @@ func TestConstructHelmReleaseProxy(t *testing.T) { ChartName: "test-chart-name", RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, + }, Credentials: &addonsv1alpha1.Credentials{ Secret: corev1.SecretReference{ Name: "test-secret", @@ -1010,8 +963,11 @@ func TestConstructHelmReleaseProxy(t *testing.T) { RepoURL: "https://test-repo-url", ReleaseNamespace: "test-release-namespace", Values: "test-parsed-values", - Options: &addonsv1alpha1.HelmOptions{ - EnableClientCache: ptr.To(true), + Options: addonsv1alpha1.HelmOptions{ + EnableClientCache: true, + Timeout: &metav1.Duration{ + Duration: 10 * time.Minute, + }, }, Credentials: &addonsv1alpha1.Credentials{ Secret: corev1.SecretReference{ diff --git a/controllers/helmchartproxy/helmchartproxy_controller_test.go b/controllers/helmchartproxy/helmchartproxy_controller_test.go index cad2086d..0c87474d 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller_test.go +++ b/controllers/helmchartproxy/helmchartproxy_controller_test.go @@ -56,7 +56,7 @@ var ( ReleaseNamespace: "test-release-namespace", Version: "test-version", ValuesTemplate: "apiServerPort: {{ .Cluster.spec.clusterNetwork.apiServerPort }}", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{}, }, } @@ -168,7 +168,7 @@ var ( ReleaseNamespace: "test-release-namespace", Version: "test-version", Values: "apiServerPort: 1234", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{}, }, Status: addonsv1alpha1.HelmReleaseProxyStatus{ Conditions: []clusterv1.Condition{ @@ -211,7 +211,7 @@ var ( ReleaseNamespace: "test-release-namespace", Version: "test-version", Values: "apiServerPort: 5678", - Options: &addonsv1alpha1.HelmOptions{}, + Options: addonsv1alpha1.HelmOptions{}, }, Status: addonsv1alpha1.HelmReleaseProxyStatus{ Conditions: []clusterv1.Condition{ diff --git a/internal/helm_client.go b/internal/helm_client.go index 63cc3e1b..c5e3f47c 100644 --- a/internal/helm_client.go +++ b/internal/helm_client.go @@ -155,14 +155,8 @@ func generateHelmInstallConfig(actionConfig *helmAction.Configuration, helmOptio installClient.SubNotes = helmOptions.SubNotes installClient.DisableOpenAPIValidation = helmOptions.DisableOpenAPIValidation installClient.Atomic = helmOptions.Atomic - - if helmOptions.Install != nil { - installClient.IncludeCRDs = helmOptions.Install.IncludeCRDs - // Safety check to avoid panic in case webhook is disabled. - if helmOptions.Install.CreateNamespace != nil { - installClient.CreateNamespace = *helmOptions.Install.CreateNamespace - } - } + installClient.IncludeCRDs = helmOptions.Install.IncludeCRDs + installClient.CreateNamespace = helmOptions.Install.CreateNamespace return installClient } @@ -187,14 +181,11 @@ func generateHelmUpgradeConfig(actionConfig *helmAction.Configuration, helmOptio upgradeClient.SubNotes = helmOptions.SubNotes upgradeClient.DisableOpenAPIValidation = helmOptions.DisableOpenAPIValidation upgradeClient.Atomic = helmOptions.Atomic - - if helmOptions.Upgrade != nil { - upgradeClient.Force = helmOptions.Upgrade.Force - upgradeClient.ResetValues = helmOptions.Upgrade.ResetValues - upgradeClient.ReuseValues = helmOptions.Upgrade.ReuseValues - upgradeClient.MaxHistory = helmOptions.Upgrade.MaxHistory - upgradeClient.CleanupOnFail = helmOptions.Upgrade.CleanupOnFail - } + upgradeClient.Force = helmOptions.Upgrade.Force + upgradeClient.ResetValues = helmOptions.Upgrade.ResetValues + upgradeClient.ReuseValues = helmOptions.Upgrade.ReuseValues + upgradeClient.MaxHistory = helmOptions.Upgrade.MaxHistory + upgradeClient.CleanupOnFail = helmOptions.Upgrade.CleanupOnFail return upgradeClient } @@ -209,10 +200,7 @@ func (c *HelmClient) InstallHelmRelease(ctx context.Context, kubeconfig string, } settings.RegistryConfig = credentialsPath - enableClientCache := true - if spec.Options != nil && spec.Options.EnableClientCache != nil { - enableClientCache = *spec.Options.EnableClientCache - } + enableClientCache := spec.Options.EnableClientCache log.V(2).Info("Creating install registry client", "enableClientCache", enableClientCache, "credentialsPath", credentialsPath) registryClient, err := newDefaultRegistryClient(credentialsPath, enableClientCache) if err != nil { @@ -226,7 +214,7 @@ func (c *HelmClient) InstallHelmRelease(ctx context.Context, kubeconfig string, return nil, err } - installClient := generateHelmInstallConfig(actionConfig, spec.Options) + installClient := generateHelmInstallConfig(actionConfig, &spec.Options) installClient.RepoURL = repoURL installClient.Version = spec.Version installClient.Namespace = spec.ReleaseNamespace @@ -342,10 +330,7 @@ func (c *HelmClient) UpgradeHelmReleaseIfChanged(ctx context.Context, kubeconfig } settings.RegistryConfig = credentialsPath - enableClientCache := true - if spec.Options != nil && spec.Options.EnableClientCache != nil { - enableClientCache = *spec.Options.EnableClientCache - } + enableClientCache := spec.Options.EnableClientCache log.V(2).Info("Creating upgrade registry client", "enableClientCache", enableClientCache, "credentialsPath", credentialsPath) registryClient, err := newDefaultRegistryClient(credentialsPath, enableClientCache) if err != nil { @@ -359,7 +344,7 @@ func (c *HelmClient) UpgradeHelmReleaseIfChanged(ctx context.Context, kubeconfig return nil, err } - upgradeClient := generateHelmUpgradeConfig(actionConfig, spec.Options) + upgradeClient := generateHelmUpgradeConfig(actionConfig, &spec.Options) upgradeClient.RepoURL = repoURL upgradeClient.Version = spec.Version upgradeClient.Namespace = spec.ReleaseNamespace @@ -535,7 +520,7 @@ func (c *HelmClient) UninstallHelmRelease(ctx context.Context, kubeconfig string return nil, err } - uninstallClient := generateHelmUninstallConfig(actionConfig, spec.Options) + uninstallClient := generateHelmUninstallConfig(actionConfig, &spec.Options) response, err := uninstallClient.Run(spec.ReleaseName) if err != nil {