Skip to content

Commit

Permalink
Use get operation on HelmChart and fix RBAC bug
Browse files Browse the repository at this point in the history
Change Named design

Signed-off-by: chengleqi <leqicheng@stu.xidian.edu.cn>
  • Loading branch information
chengleqi committed Aug 10, 2022
1 parent 1f7aac1 commit d8a7a2d
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 58 deletions.
3 changes: 2 additions & 1 deletion config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,10 @@ rules:
- list
- update
- apiGroups:
- source.toolkit.fluxcd.io/v1beta2
- source.toolkit.fluxcd.io
resources:
- helmcharts
verbs:
- create
- get
- list
67 changes: 27 additions & 40 deletions controllers/fluxcd/application-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch
//+kubebuilder:rbac:groups="kustomize.toolkit.fluxcd.io",resources=kustomizations,verbs=get;list;create;update;delete
//+kubebuilder:rbac:groups="helm.toolkit.fluxcd.io",resources=helmreleases,verbs=get;list;create;update;delete
//+kubebuilder:rbac:groups="source.toolkit.fluxcd.io/v1beta2",resources=helmcharts,verbs=get;list
//+kubebuilder:rbac:groups="source.toolkit.fluxcd.io",resources=helmcharts,verbs=get;list;create

// ApplicationReconciler is the reconciler of the FluxCD HelmRelease and FluxCD Kustomization
type ApplicationReconciler struct {
Expand Down Expand Up @@ -95,15 +95,11 @@ func (r *ApplicationReconciler) reconcileHelm(app *v1alpha1.Application) (err er
if helmTemplateName := app.Spec.FluxApp.Spec.Config.HelmRelease.Template; helmTemplateName != "" {
// use template
helmTemplateNS := app.GetNamespace()
helmTemplateList := createBareFluxHelmTemplateObjectList()

if err = r.List(ctx, helmTemplateList, client.InNamespace(helmTemplateNS), client.MatchingLabels{
v1alpha1.HelmTemplateName: helmTemplateName,
}); err != nil {
helmChart = createBareFluxHelmTemplateObject()
if err = r.Get(ctx, types.NamespacedName{Namespace: helmTemplateNS, Name: helmTemplateName}, helmChart); err != nil {
return
}
// there is a helmtemplate that user specified
helmChart = &helmTemplateList.Items[0]
} else {
helmChart = createUnstructuredFluxHelmTemplate(app)
}
Expand All @@ -123,7 +119,7 @@ func (r *ApplicationReconciler) reconcileHelmReleaseList(app *v1alpha1.Applicati

fluxHelmReleaseList := createBareFluxHelmReleaseListObject()
if err = r.List(ctx, fluxHelmReleaseList, client.InNamespace(appNS), client.MatchingLabels{
"app.kubernetes.io/managed-by": getHelmTemplateName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
}); err != nil {
return
}
Expand All @@ -136,7 +132,7 @@ func (r *ApplicationReconciler) reconcileHelmReleaseList(app *v1alpha1.Applicati
helmReleaseNum := len(fluxApp.Spec.Config.HelmRelease.Deploy)
for i := 0; i < helmReleaseNum; i++ {
deploy := fluxApp.Spec.Config.HelmRelease.Deploy[i]
helmReleaseName := getHelmReleaseName(deploy.Destination.TargetNamespace)
helmReleaseName := getHelmReleaseName(deploy)
if hr, ok := hrMap[helmReleaseName]; !ok {
// there is no matching helmRelease
// create
Expand All @@ -156,14 +152,14 @@ func (r *ApplicationReconciler) reconcileHelmReleaseList(app *v1alpha1.Applicati

func (r *ApplicationReconciler) createHelmRelease(ctx context.Context, app *v1alpha1.Application, helmChart *unstructured.Unstructured, deploy *v1alpha1.Deploy) (err error) {
appNS, appName := app.GetNamespace(), app.GetName()
hrNS, hrName := appNS, getHelmReleaseName(deploy.Destination.TargetNamespace)
hrNS, hrName := appNS, getHelmReleaseName(deploy)

newFluxHelmRelease := createBareFluxHelmReleaseObject()
setFluxHelmReleaseFields(newFluxHelmRelease, helmChart, deploy)
newFluxHelmRelease.SetNamespace(hrNS)
newFluxHelmRelease.SetName(hrName)
newFluxHelmRelease.SetLabels(map[string]string{
"app.kubernetes.io/managed-by": getHelmTemplateName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
})
newFluxHelmRelease.SetOwnerReferences([]metav1.OwnerReference{
{
Expand Down Expand Up @@ -212,7 +208,7 @@ func (r *ApplicationReconciler) reconcileKustomization(app *v1alpha1.Application

kusList := createBareFluxKustomizationListObject()
if err = r.List(ctx, kusList, client.InNamespace(appNS), client.MatchingLabels{
"app.kubernetes.io/managed-by": getKusGroupName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
}); err != nil {
return
}
Expand All @@ -225,7 +221,7 @@ func (r *ApplicationReconciler) reconcileKustomization(app *v1alpha1.Application
kusNum := len(app.Spec.FluxApp.Spec.Config.Kustomization)
for i := 0; i < kusNum; i++ {
kusDeploy := fluxApp.Spec.Config.Kustomization[i]
kusName := getKustomizationName(kusDeploy.Destination.TargetNamespace)
kusName := getKustomizationName(kusDeploy)

if kus, ok := kusMap[kusName]; !ok {
// not found
Expand All @@ -242,7 +238,7 @@ func (r *ApplicationReconciler) reconcileKustomization(app *v1alpha1.Application

func (r *ApplicationReconciler) createKustomization(ctx context.Context, app *v1alpha1.Application, deploy *v1alpha1.KustomizationSpec) (err error) {
appNS, appName := app.GetNamespace(), app.GetName()
kusNS, kusName := appNS, getKustomizationName(deploy.Destination.TargetNamespace)
kusNS, kusName := appNS, getKustomizationName(deploy)

kus := createBareFluxKustomizationObject()
setFluxKustomizationFields(kus, app, deploy)
Expand All @@ -257,7 +253,7 @@ func (r *ApplicationReconciler) createKustomization(ctx context.Context, app *v1
},
})
kus.SetLabels(map[string]string{
"app.kubernetes.io/managed-by": getKusGroupName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
})
err = r.Create(ctx, kus)
return
Expand Down Expand Up @@ -304,28 +300,29 @@ func (r *ApplicationReconciler) GetName() string {
return "FluxApplicationReconciler"
}

// TODO: it will be better to use hash name for naming conflict reason
func getHelmReleaseName(targetNamespace string) string {
return targetNamespace
}

func getKustomizationName(targetNamespace string) string {
return targetNamespace
}

func getHelmTemplateName(ns, name string) string {
return ns + "-" + name
func getHelmReleaseName(deploy *v1alpha1.Deploy) string {
// host cluster
if deploy.Destination.KubeConfig == nil {
return deploy.Destination.TargetNamespace
}
// member cluster
return deploy.Destination.KubeConfig.SecretRef.Name + "-" + deploy.Destination.TargetNamespace
}

func getKusGroupName(ns, name string) string {
return ns + "-" + name
func getKustomizationName(deploy *v1alpha1.KustomizationSpec) string {
// host cluster
if deploy.Destination.KubeConfig == nil {
return deploy.Destination.TargetNamespace
}
// member cluster
return deploy.Destination.KubeConfig.SecretRef.Name + "-" + deploy.Destination.TargetNamespace
}

func (r *ApplicationReconciler) saveTemplate(ctx context.Context, app *v1alpha1.Application) (err error) {
helmTemplate := createBareFluxHelmTemplateObject()

appNS, appName := app.GetNamespace(), app.GetName()
helmTemplateNS, helmTemplateName := appNS, getHelmTemplateName(appNS, appName)
helmTemplateNS, helmTemplateName := appNS, appName

if err = r.Get(ctx, types.NamespacedName{Namespace: helmTemplateNS, Name: helmTemplateName}, helmTemplate); err != nil {
if !apierrors.IsNotFound(err) {
Expand All @@ -336,7 +333,7 @@ func (r *ApplicationReconciler) saveTemplate(ctx context.Context, app *v1alpha1.
newFluxHelmTemplate := createUnstructuredFluxHelmTemplate(app)
newFluxHelmTemplate.SetName(helmTemplateName)
newFluxHelmTemplate.SetNamespace(helmTemplateNS)
newFluxHelmTemplate.SetLabels(map[string]string{
newFluxHelmTemplate.SetAnnotations(map[string]string{
v1alpha1.HelmTemplateName: helmTemplateName,
})
if err = r.Create(ctx, newFluxHelmTemplate); err != nil {
Expand Down Expand Up @@ -376,16 +373,6 @@ func createBareFluxHelmTemplateObject() *unstructured.Unstructured {
return fluxHelmChart
}

func createBareFluxHelmTemplateObjectList() *unstructured.UnstructuredList {
fluxHelmTemplateList := &unstructured.UnstructuredList{}
fluxHelmTemplateList.SetGroupVersionKind(schema.GroupVersionKind{
Group: "source.toolkit.fluxcd.io",
Version: "v1beta2",
Kind: "HelmChartList",
})
return fluxHelmTemplateList
}

func createBareFluxHelmReleaseObject() *unstructured.Unstructured {
fluxHelmRelease := &unstructured.Unstructured{}
fluxHelmRelease.SetGroupVersionKind(schema.GroupVersionKind{
Expand Down
46 changes: 29 additions & 17 deletions controllers/fluxcd/application-controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,24 +301,24 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {

fluxHelmChart := createUnstructuredFluxHelmTemplate(helmApp)
fluxHelmChart.SetNamespace(helmApp.GetNamespace())
fluxHelmChart.SetName(getHelmTemplateName(helmApp.GetNamespace(), helmApp.GetName()))
fluxHelmChart.SetLabels(map[string]string{
v1alpha1.HelmTemplateName: getHelmTemplateName(helmApp.GetNamespace(), helmApp.GetName()),
fluxHelmChart.SetName(helmApp.GetName())
fluxHelmChart.SetAnnotations(map[string]string{
v1alpha1.HelmTemplateName: helmApp.GetName(),
})

fluxHR := createBareFluxHelmReleaseObject()
helmDeploy := helmApp.Spec.FluxApp.Spec.Config.HelmRelease.Deploy[0]
setFluxHelmReleaseFields(fluxHR, fluxHelmChart, helmDeploy)
fluxHR.SetNamespace(helmApp.GetNamespace())
fluxHR.SetName(getHelmReleaseName(helmDeploy.Destination.TargetNamespace))
fluxHR.SetName(getHelmReleaseName(helmDeploy))
fluxHR.SetLabels(map[string]string{
"app.kubernetes.io/managed-by": getHelmTemplateName(helmApp.GetNamespace(), helmApp.GetName()),
"app.kubernetes.io/managed-by": helmApp.GetName(),
})

helmAppWithTemplate := helmApp.DeepCopy()
helmAppWithTemplate.Spec.FluxApp.Spec.Source = nil
helmAppWithTemplate.Spec.FluxApp.Spec.Config.HelmRelease.Chart = nil
helmAppWithTemplate.Spec.FluxApp.Spec.Config.HelmRelease.Template = "fake-ns-fake-app"
helmAppWithTemplate.Spec.FluxApp.Spec.Config.HelmRelease.Template = "fake-app"

kusApp := &v1alpha1.Application{
Spec: v1alpha1.ApplicationSpec{
Expand Down Expand Up @@ -372,9 +372,9 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
KusDeploy := kusApp.Spec.FluxApp.Spec.Config.Kustomization[0]
setFluxKustomizationFields(fluxKus, kusApp, KusDeploy)
fluxKus.SetNamespace(kusApp.GetNamespace())
fluxKus.SetName(getKustomizationName(KusDeploy.Destination.TargetNamespace))
fluxKus.SetName(getKustomizationName(KusDeploy))
fluxKus.SetLabels(map[string]string{
"app.kubernetes.io/managed-by": getKusGroupName(kusApp.GetNamespace(), kusApp.GetName()),
"app.kubernetes.io/managed-by": kusApp.GetName(),
})

kusAppWithUpdate := kusApp.DeepCopy()
Expand Down Expand Up @@ -443,7 +443,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
appNS, appName := helmApp.GetNamespace(), helmApp.GetName()

err = Client.List(ctx, fluxHRList, client.InNamespace(appNS), client.MatchingLabels{
"app.kubernetes.io/managed-by": getHelmTemplateName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
})
assert.Nil(t, err)
assert.Equal(t, 2, len(fluxHRList.Items))
Expand Down Expand Up @@ -476,7 +476,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
}

fluxChart := createBareFluxHelmTemplateObject()
err = Client.Get(ctx, types.NamespacedName{Namespace: appNS, Name: getHelmTemplateName(appNS, appName)}, fluxChart)
err = Client.Get(ctx, types.NamespacedName{Namespace: appNS, Name: appName}, fluxChart)
assert.True(t, apierrors.IsNotFound(err))
},
},
Expand All @@ -496,7 +496,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
appNS, appName := helmApp.GetNamespace(), helmApp.GetName()

err = Client.List(ctx, fluxHRList, client.InNamespace(appNS), client.MatchingLabels{
"app.kubernetes.io/managed-by": getHelmTemplateName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
})
assert.Nil(t, err)
assert.Equal(t, 2, len(fluxHRList.Items))
Expand Down Expand Up @@ -537,7 +537,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
}

fluxChart := createBareFluxHelmTemplateObject()
err = Client.Get(ctx, types.NamespacedName{Namespace: appNS, Name: getHelmTemplateName(appNS, appName)}, fluxChart)
err = Client.Get(ctx, types.NamespacedName{Namespace: appNS, Name: appName}, fluxChart)
assert.Nil(t, err)

sourceAPIVersion, _, _ := unstructured.NestedString(fluxChart.Object, "spec", "sourceRef", "apiVersion")
Expand Down Expand Up @@ -577,7 +577,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
appNS, appName := helmApp.GetNamespace(), helmApp.GetName()

err = Client.List(ctx, fluxHRList, client.InNamespace(appNS), client.MatchingLabels{
"app.kubernetes.io/managed-by": getHelmTemplateName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
})
assert.Nil(t, err)
assert.Equal(t, 2, len(fluxHRList.Items))
Expand Down Expand Up @@ -613,7 +613,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
appNS, appName := helmApp.GetNamespace(), helmApp.GetName()

err = Client.List(ctx, fluxHRList, client.InNamespace(appNS), client.MatchingLabels{
"app.kubernetes.io/managed-by": getHelmTemplateName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
})
assert.Nil(t, err)
assert.Equal(t, 2, len(fluxHRList.Items))
Expand Down Expand Up @@ -654,7 +654,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
}

fluxChart := createBareFluxHelmTemplateObject()
err = Client.Get(ctx, types.NamespacedName{Namespace: appNS, Name: getHelmTemplateName(appNS, appName)}, fluxChart)
err = Client.Get(ctx, types.NamespacedName{Namespace: appNS, Name: appName}, fluxChart)
assert.Nil(t, err)

sourceAPIVersion, _, _ := unstructured.NestedString(fluxChart.Object, "spec", "sourceRef", "apiVersion")
Expand All @@ -678,6 +678,18 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
assert.Equal(t, "./helm-chart/values.yaml", chartValueFiles[0])
},
},
{
name: "create a Multi-Clusters FluxApp(HelmRelease) by using a Template",
fields: fields{
Client: fake.NewFakeClientWithScheme(schema),
},
args: args{
app: helmAppWithTemplate.DeepCopy(),
},
verify: func(t *testing.T, Client client.Client, err error) {
assert.NotNil(t, err)
},
},
{
name: "create a Multi-Clusters FluxApp(Kustomization)",
fields: fields{
Expand All @@ -693,7 +705,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
kusList := createBareFluxKustomizationListObject()
appNS, appName := kusApp.GetNamespace(), kusApp.GetName()
err = Client.List(ctx, kusList, client.InNamespace(appNS), client.MatchingLabels{
"app.kubernetes.io/managed-by": getKusGroupName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
})
assert.Nil(t, err)
assert.Equal(t, 2, len(kusList.Items))
Expand Down Expand Up @@ -743,7 +755,7 @@ func TestApplicationReconciler_reconcileApp(t *testing.T) {
kusList := createBareFluxKustomizationListObject()
appNS, appName := kusApp.GetNamespace(), kusApp.GetName()
err = Client.List(ctx, kusList, client.InNamespace(appNS), client.MatchingLabels{
"app.kubernetes.io/managed-by": getKusGroupName(appNS, appName),
"app.kubernetes.io/managed-by": appName,
})
assert.Nil(t, err)
assert.Equal(t, 2, len(kusList.Items))
Expand Down

0 comments on commit d8a7a2d

Please sign in to comment.