fluxcd
/
helm-controller
mirror of https://github.com/fluxcd/helm-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix install and upgrade applying subchart CRDs when condition is false 

Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>
This commit is contained in:
Matheus Pimenta 2024-12-10 12:25:32 +00:00
parent 9b78c2e670
commit dd3b66a3c5
No known key found for this signature in database
GPG Key ID: DE594AAD698A94DE
6 changed files with 433 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
internal/action/crds.go
View File

@ -24,6 +24,7 @@ import (
helmaction "helm.sh/helm/v3/pkg/action"
helmchart "helm.sh/helm/v3/pkg/chart"
helmchartutil "helm.sh/helm/v3/pkg/chartutil"
helmkube "helm.sh/helm/v3/pkg/kube"
apiextension "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
apierrors "k8s.io/apimachinery/pkg/api/errors"
@ -64,7 +65,9 @@ func (*rootScoped) Name() apimeta.RESTScopeName {
return apimeta.RESTScopeNameRoot
}
func applyCRDs(cfg *helmaction.Configuration, policy v2.CRDsPolicy, chrt *helmchart.Chart, visitorFunc ...resource.VisitorFunc) error {
func applyCRDs(cfg *helmaction.Configuration, policy v2.CRDsPolicy, chrt *helmchart.Chart,
vals helmchartutil.Values, visitorFunc ...resource.VisitorFunc) error {
if len(chrt.CRDObjects()) == 0 {
return nil
}
@ -74,6 +77,10 @@ func applyCRDs(cfg *helmaction.Configuration, policy v2.CRDsPolicy, chrt *helmch
return nil
}
if err := helmchartutil.ProcessDependenciesWithMerge(chrt, vals); err != nil {
return fmt.Errorf("failed to process chart dependencies: %w", err)
}
// Collect all CRDs from all files in `crds` directory.
allCRDs := make(helmkube.ResourceList, 0)
for _, obj := range chrt.CRDObjects() {

2
internal/action/install.go
View File

@ -55,7 +55,7 @@ func Install(ctx context.Context, config *helmaction.Configuration, obj *v2.Helm
if err != nil {
return nil, err
}
if err := applyCRDs(config, policy, chrt, setOriginVisitor(v2.GroupVersion.Group, obj.Namespace, obj.Name)); err != nil {
if err := applyCRDs(config, policy, chrt, vals, setOriginVisitor(v2.GroupVersion.Group, obj.Namespace, obj.Name)); err != nil {
return nil, fmt.Errorf("failed to apply CustomResourceDefinitions: %w", err)
}

2
internal/action/upgrade.go
View File

@ -55,7 +55,7 @@ func Upgrade(ctx context.Context, config *helmaction.Configuration, obj *v2.Helm
if err != nil {
return nil, err
}
if err := applyCRDs(config, policy, chrt, setOriginVisitor(v2.GroupVersion.Group, obj.Namespace, obj.Name)); err != nil {
if err := applyCRDs(config, policy, chrt, vals, setOriginVisitor(v2.GroupVersion.Group, obj.Namespace, obj.Name)); err != nil {
return nil, fmt.Errorf("failed to apply CustomResourceDefinitions: %w", err)
}

140
internal/reconcile/install_test.go
View File

@ -31,8 +31,10 @@ import (
helmstorage "helm.sh/helm/v3/pkg/storage"
helmdriver "helm.sh/helm/v3/pkg/storage/driver"
corev1 "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client"
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
"github.com/fluxcd/pkg/apis/meta"
@ -300,6 +302,144 @@ func TestInstall_Reconcile(t *testing.T) {
}
}
func TestInstall_Reconcile_withSubchartWithCRDs(t *testing.T) {
getValues := func(subchartValues map[string]any) helmchartutil.Values {
return helmchartutil.Values{"subchart": subchartValues}
}
expectConditions := []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, v2.InstallSucceededReason,
"Helm install succeeded"),
*conditions.TrueCondition(v2.ReleasedCondition, v2.InstallSucceededReason,
"Helm install succeeded"),
}
expectHistory := func(releases []*helmrelease.Release) v2.Snapshots {
return v2.Snapshots{
release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
}
}
for _, tt := range []struct {
name string
subchartValues map[string]any
subchartResourcesPresent bool
expectedMainChartValues map[string]any
}{
{
name: "subchart disabled should not deploy resources, including CRDs",
subchartValues: map[string]any{"enabled": false},
subchartResourcesPresent: false,
expectedMainChartValues: map[string]any{
"foo": "baz",
"myimports": map[string]any{"myint": 0},
},
},
{
name: "subchart enabled should deploy resources, including CRDs",
subchartValues: map[string]any{"enabled": true},
subchartResourcesPresent: true,
expectedMainChartValues: map[string]any{
"foo": "baz",
"myint": 123,
"myimports": map[string]any{"myint": 0}, // should be 456: https://github.com/helm/helm/issues/13223
"subchart": map[string]any{
"foo": "bar",
"global": map[string]any{},
"exports": map[string]any{"data": map[string]any{"myint": 123}},
"default": map[string]any{"data": map[string]any{"myint": 456}},
},
},
},
} {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
namedNS, err := testEnv.CreateNamespace(context.TODO(), mockReleaseNamespace)
g.Expect(err).NotTo(HaveOccurred())
t.Cleanup(func() {
_ = testEnv.Delete(context.TODO(), namedNS)
})
releaseNamespace := namedNS.Name
obj := &v2.HelmRelease{
Spec: v2.HelmReleaseSpec{
ReleaseName: mockReleaseName,
TargetNamespace: releaseNamespace,
StorageNamespace: releaseNamespace,
Timeout: &metav1.Duration{Duration: 100 * time.Millisecond},
},
}
getter, err := RESTClientGetterFromManager(testEnv.Manager, obj.GetReleaseNamespace())
g.Expect(err).ToNot(HaveOccurred())
cfg, err := action.NewConfigFactory(getter,
action.WithStorage(action.DefaultStorageDriver, obj.GetStorageNamespace()),
)
g.Expect(err).ToNot(HaveOccurred())
store := helmstorage.Init(cfg.Driver)
chart := testutil.BuildChartWithSubchartWithCRD()
recorder := new(record.FakeRecorder)
got := (NewInstall(cfg, recorder)).Reconcile(context.TODO(), &Request{
Object: obj,
Chart: chart,
Values: getValues(tt.subchartValues),
})
g.Expect(got).ToNot(HaveOccurred())
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(expectConditions))
releases, _ := store.History(mockReleaseName)
releaseutil.SortByRevision(releases)
g.Expect(obj.Status.History).To(testutil.Equal(expectHistory(releases)))
// Assert main chart configmap is present.
mainChartCM := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cm-main-chart",
Namespace: releaseNamespace,
},
}
err = testEnv.Get(context.TODO(), client.ObjectKeyFromObject(mainChartCM), mainChartCM)
g.Expect(err).NotTo(HaveOccurred())
// Assert subchart configmap is absent or present.
subChartCM := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cm-sub-chart",
Namespace: releaseNamespace,
},
}
err = testEnv.Get(context.TODO(), client.ObjectKeyFromObject(subChartCM), subChartCM)
if tt.subchartResourcesPresent {
g.Expect(err).NotTo(HaveOccurred())
} else {
g.Expect(err).To(HaveOccurred())
}
// Assert subchart CRD is absent or present.
subChartCRD := &apiextensionsv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "crontabs.stable.example.com",
},
}
err = testEnv.Get(context.TODO(), client.ObjectKeyFromObject(subChartCRD), subChartCRD)
if tt.subchartResourcesPresent {
g.Expect(err).NotTo(HaveOccurred())
} else {
g.Expect(err).To(HaveOccurred())
}
// Assert main chart values.
g.Expect(chart.Values).To(testutil.Equal(tt.expectedMainChartValues))
})
}
}
func TestInstall_failure(t *testing.T) {
var (
obj = &v2.HelmRelease{

169
internal/reconcile/upgrade_test.go
View File

@ -31,8 +31,10 @@ import (
helmstorage "helm.sh/helm/v3/pkg/storage"
helmdriver "helm.sh/helm/v3/pkg/storage/driver"
corev1 "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client"
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
"github.com/fluxcd/pkg/apis/meta"
@ -431,6 +433,173 @@ func TestUpgrade_Reconcile(t *testing.T) {
}
}
func TestUpgrade_Reconcile_withSubchartWithCRDs(t *testing.T) {
getValues := func(subchartValues map[string]any) helmchartutil.Values {
return helmchartutil.Values{"subchart": subchartValues}
}
releases := func(namespace string) []*helmrelease.Release {
return []*helmrelease.Release{
testutil.BuildRelease(&helmrelease.MockReleaseOptions{
Name: mockReleaseName,
Namespace: namespace,
Chart: testutil.BuildChart(testutil.ChartWithTestHook()),
Version: 1,
Status: helmrelease.StatusDeployed,
}),
}
}
status := func(releases []*helmrelease.Release) v2.HelmReleaseStatus {
return v2.HelmReleaseStatus{
History: v2.Snapshots{
release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
},
}
}
expectConditions := []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded"),
*conditions.TrueCondition(v2.ReleasedCondition, v2.UpgradeSucceededReason, "Helm upgrade succeeded"),
}
expectHistory := func(releases []*helmrelease.Release) v2.Snapshots {
return v2.Snapshots{
release.ObservedToSnapshot(release.ObserveRelease(releases[1])),
release.ObservedToSnapshot(release.ObserveRelease(releases[0])),
}
}
for _, tt := range []struct {
name string
subchartValues map[string]any
subchartResourcesPresent bool
expectedMainChartValues map[string]any
}{
{
name: "subchart disabled should not deploy resources, including CRDs",
subchartValues: map[string]any{"enabled": false},
subchartResourcesPresent: false,
expectedMainChartValues: map[string]any{
"foo": "baz",
"myimports": map[string]any{"myint": 0},
},
},
{
name: "subchart enabled should deploy resources, including CRDs",
subchartValues: map[string]any{"enabled": true},
subchartResourcesPresent: true,
expectedMainChartValues: map[string]any{
"foo": "baz",
"myint": 123,
"myimports": map[string]any{"myint": 0}, // should be 456: https://github.com/helm/helm/issues/13223
"subchart": map[string]any{
"foo": "bar",
"global": map[string]any{},
"exports": map[string]any{"data": map[string]any{"myint": 123}},
"default": map[string]any{"data": map[string]any{"myint": 456}},
},
},
},
} {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
namedNS, err := testEnv.CreateNamespace(context.TODO(), mockReleaseNamespace)
g.Expect(err).NotTo(HaveOccurred())
t.Cleanup(func() {
_ = testEnv.Delete(context.TODO(), namedNS)
})
releaseNamespace := namedNS.Name
releases := releases(releaseNamespace)
helmreleaseutil.SortByRevision(releases)
obj := &v2.HelmRelease{
Spec: v2.HelmReleaseSpec{
ReleaseName: mockReleaseName,
TargetNamespace: releaseNamespace,
StorageNamespace: releaseNamespace,
Timeout: &metav1.Duration{Duration: 100 * time.Millisecond},
},
}
obj.Status = status(releases)
getter, err := RESTClientGetterFromManager(testEnv.Manager, obj.GetReleaseNamespace())
g.Expect(err).ToNot(HaveOccurred())
cfg, err := action.NewConfigFactory(getter,
action.WithStorage(action.DefaultStorageDriver, obj.GetStorageNamespace()),
)
g.Expect(err).ToNot(HaveOccurred())
store := helmstorage.Init(cfg.Driver)
for _, r := range releases {
g.Expect(store.Create(r)).To(Succeed())
}
// Delete any prior CRD.
subChartCRD := &apiextensionsv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "crontabs.stable.example.com",
},
}
_ = testEnv.Delete(context.TODO(), subChartCRD)
chart := testutil.BuildChartWithSubchartWithCRD()
recorder := new(record.FakeRecorder)
got := NewUpgrade(cfg, recorder).Reconcile(context.TODO(), &Request{
Object: obj,
Chart: chart,
Values: getValues(tt.subchartValues),
})
g.Expect(got).ToNot(HaveOccurred())
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(expectConditions))
releases, _ = store.History(mockReleaseName)
helmreleaseutil.SortByRevision(releases)
g.Expect(obj.Status.History).To(testutil.Equal(expectHistory(releases)))
// Assert main chart configmap is present.
mainChartCM := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cm-main-chart",
Namespace: releaseNamespace,
},
}
err = testEnv.Get(context.TODO(), client.ObjectKeyFromObject(mainChartCM), mainChartCM)
g.Expect(err).NotTo(HaveOccurred())
// Assert subchart configmap is absent or present.
subChartCM := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cm-sub-chart",
Namespace: releaseNamespace,
},
}
err = testEnv.Get(context.TODO(), client.ObjectKeyFromObject(subChartCM), subChartCM)
if tt.subchartResourcesPresent {
g.Expect(err).NotTo(HaveOccurred())
} else {
g.Expect(err).To(HaveOccurred())
}
// Assert subchart CRD is absent or present.
err = testEnv.Get(context.TODO(), client.ObjectKeyFromObject(subChartCRD), subChartCRD)
if tt.subchartResourcesPresent {
g.Expect(err).NotTo(HaveOccurred())
} else {
g.Expect(err).To(HaveOccurred())
}
// Assert main chart values.
g.Expect(chart.Values).To(testutil.Equal(tt.expectedMainChartValues))
})
}
}
func TestUpgrade_failure(t *testing.T) {
var (
obj = &v2.HelmRelease{

114
internal/testutil/mock_chart.go
View File

@ -20,6 +20,7 @@ import (
"fmt"
helmchart "helm.sh/helm/v3/pkg/chart"
helmchartutil "helm.sh/helm/v3/pkg/chartutil"
)
var manifestTmpl = `apiVersion: v1
@ -31,6 +32,15 @@ data:
foo: bar
`
var manifestWithCustomNameTmpl = `apiVersion: v1
kind: ConfigMap
metadata:
name: cm-%[1]s
namespace: %[2]s
data:
foo: bar
`
var manifestWithHookTmpl = `apiVersion: v1
kind: ConfigMap
metadata:
@ -82,6 +92,38 @@ spec:
restartPolicy: Never
`
var crdManifest = `apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: crontabs.stable.example.com
spec:
group: stable.example.com
versions:
- name: v1
served: true
storage: true
schema:
openAPIV3Schema:
type: object
properties:
spec:
type: object
properties:
cronSpec:
type: string
image:
type: string
replicas:
type: integer
scope: Namespaced
names:
plural: crontabs
singular: crontab
kind: CronTab
shortNames:
- ct
`
// ChartOptions is a helper to build a Helm chart object.
type ChartOptions struct {
*helmchart.Chart
@ -166,3 +208,75 @@ func ChartWithFailingTestHook() ChartOption {
})
}
}
// ChartWithManifestWithCustomName sets the name of the manifest.
func ChartWithManifestWithCustomName(name string) ChartOption {
return func(opts *ChartOptions) {
opts.Templates = []*helmchart.File{
{
Name: "templates/manifest",
Data: []byte(fmt.Sprintf(manifestWithCustomNameTmpl, name, "{{ default .Release.Namespace }}")),
},
}
}
}
// ChartWithCRD appends a CRD to the chart.
func ChartWithCRD() ChartOption {
return func(opts *ChartOptions) {
opts.Files = []*helmchart.File{
{
Name: "crds/crd.yaml",
Data: []byte(crdManifest),
},
}
}
}
// ChartWithDependency appends a dependency to the chart.
func ChartWithDependency(md *helmchart.Dependency, chrt *helmchart.Chart) ChartOption {
return func(opts *ChartOptions) {
opts.Metadata.Dependencies = append(opts.Metadata.Dependencies, md)
opts.AddDependency(chrt)
}
}
// ChartWithValues sets the values.yaml file of the chart.
func ChartWithValues(values map[string]any) ChartOption {
return func(opts *ChartOptions) {
opts.Values = values
}
}
// BuildChartWithSubchartWithCRD returns a Helm chart object with a subchart
// that contains a CRD. Useful for testing helm-controller's staged CRDs-first
// deployment logic.
func BuildChartWithSubchartWithCRD() *helmchart.Chart {
subChart := BuildChart(
ChartWithName("subchart"),
ChartWithManifestWithCustomName("sub-chart"),
ChartWithCRD(),
ChartWithValues(helmchartutil.Values{
"foo": "bar",
"exports": map[string]any{"data": map[string]any{"myint": 123}},
"default": map[string]any{"data": map[string]any{"myint": 456}},
}))
mainChart := BuildChart(
ChartWithManifestWithCustomName("main-chart"),
ChartWithValues(helmchartutil.Values{
"foo": "baz",
"myimports": map[string]any{"myint": 0},
}),
ChartWithDependency(&helmchart.Dependency{
Name: "subchart",
Condition: "subchart.enabled",
ImportValues: []any{
"data",
map[string]any{
"child": "default.data",
"parent": "myimports",
},
},
}, subChart))
return mainChart
}