From 865f2b7e0d09f9a18d3c34080768df1ff68c2d83 Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Thu, 25 Jul 2024 20:09:38 +0400 Subject: [PATCH] Validate Templates with external source Also: 1. Consider template type from spec as well 2. Adapt tests HMC-108 --- Makefile | 27 ++++++- internal/controller/suite_test.go | 3 +- internal/controller/template_controller.go | 76 +++++++++++++++---- .../controller/template_controller_test.go | 71 +++++++++++++++-- 4 files changed, 154 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index 3fc3e733a..5453d9a5c 100644 --- a/Makefile +++ b/Makefile @@ -106,7 +106,7 @@ tidy: go mod tidy .PHONY: test -test: generate-all fmt vet envtest tidy ## Run tests. +test: generate-all fmt vet envtest tidy external-crd ## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out # Utilize Kind or modify the e2e tests to load the image locally, enabling compatibility with other vendors. @@ -318,6 +318,16 @@ LOCALBIN ?= $(shell pwd)/bin $(LOCALBIN): mkdir -p $(LOCALBIN) +EXTERNAL_CRD_DIR ?= $(LOCALBIN)/crd +$(EXTERNAL_CRD_DIR): $(LOCALBIN) + mkdir -p $(EXTERNAL_CRD_DIR) + +FLUX_SOURCE_VERSION ?= $(shell go mod edit -json | jq -r '.Require[] | select(.Path == "github.com/fluxcd/source-controller/api") | .Version') +FLUX_SOURCE_REPO_CRD ?= $(EXTERNAL_CRD_DIR)/source-helmrepositories-$(FLUX_SOURCE_VERSION).yaml +FLUX_SOURCE_CHART_CRD ?= $(EXTERNAL_CRD_DIR)/source-helmchart-$(FLUX_SOURCE_VERSION).yaml +FLUX_HELM_VERSION ?= $(shell go mod edit -json | jq -r '.Require[] | select(.Path == "github.com/fluxcd/helm-controller/api") | .Version') +FLUX_HELM_CRD ?= $(EXTERNAL_CRD_DIR)/helm-$(FLUX_HELM_VERSION).yaml + ## Tool Binaries KUBECTL ?= kubectl KUSTOMIZE ?= $(LOCALBIN)/kustomize-$(KUSTOMIZE_VERSION) @@ -377,6 +387,21 @@ helmify: $(HELMIFY) ## Download helmify locally if necessary. $(HELMIFY): | $(LOCALBIN) $(call go-install-tool,$(HELMIFY),github.com/arttor/helmify/cmd/helmify,${HELMIFY_VERSION}) +$(FLUX_HELM_CRD): $(EXTERNAL_CRD_DIR) + rm -f $(FLUX_HELM_CRD) + curl -s https://raw.githubusercontent.com/fluxcd/helm-controller/$(FLUX_HELM_VERSION)/config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml > $(FLUX_HELM_CRD) + +$(FLUX_SOURCE_CHART_CRD): $(EXTERNAL_CRD_DIR) + rm -f $(FLUX_SOURCE_CHART_CRD) + curl -s https://raw.githubusercontent.com/fluxcd/source-controller/$(FLUX_SOURCE_VERSION)/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml > $(FLUX_SOURCE_CHART_CRD) + +$(FLUX_SOURCE_REPO_CRD): $(EXTERNAL_CRD_DIR) + rm -f $(FLUX_SOURCE_REPO_CRD) + curl -s https://raw.githubusercontent.com/fluxcd/source-controller/$(FLUX_SOURCE_VERSION)/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml > $(FLUX_SOURCE_REPO_CRD) + +.PHONY: external-crd +external-crd: $(FLUX_HELM_CRD) $(FLUX_SOURCE_CHART_CRD) $(FLUX_SOURCE_REPO_CRD) + .PHONY: kind kind: $(KIND) ## Download kind locally if necessary. $(KIND): | $(LOCALBIN) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 5d2c8e90b..d572140e5 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -54,7 +54,8 @@ var _ = BeforeSuite(func() { By("bootstrapping test environment") testEnv = &envtest.Environment{ - CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, + CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases"), + filepath.Join("..", "..", "bin", "crd")}, ErrorIfCRDPathMissing: true, // The BinaryAssetsDirectory is only required if you want to run the tests directly diff --git a/internal/controller/template_controller.go b/internal/controller/template_controller.go index af558632b..c44b775b3 100644 --- a/internal/controller/template_controller.go +++ b/internal/controller/template_controller.go @@ -21,6 +21,7 @@ import ( "strings" "time" + helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2" v2 "github.com/fluxcd/helm-controller/api/v2" sourcev1 "github.com/fluxcd/source-controller/api/v1" "helm.sh/helm/v3/pkg/chart" @@ -28,6 +29,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -51,7 +53,8 @@ var ( // TemplateReconciler reconciles a Template object type TemplateReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + downloadHelmChartFunc func(context.Context, *sourcev1.Artifact) (*chart.Chart, error) } // +kubebuilder:rbac:groups=hmc.mirantis.com,resources=templates,verbs=get;list;watch;create;update;patch;delete @@ -72,19 +75,35 @@ func (r *TemplateReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c l.Error(err, "Failed to get Template") return ctrl.Result{}, err } - l.Info("Reconciling helm-controller objects ") - hcChart, err := r.reconcileHelmChart(ctx, template) - if err != nil { - l.Error(err, "Failed to reconcile HelmChart") - return ctrl.Result{}, err + + var hcChart *sourcev1.HelmChart + var err error + if template.Spec.Helm.ChartRef != nil { + hcChart, err = r.getHelmChartFromChartRef(ctx, template.Spec.Helm.ChartRef) + if err != nil { + l.Error(err, "failed to get artifact from chartRef", "kind", template.Spec.Helm.ChartRef.Kind, "namespace", template.Spec.Helm.ChartRef.Namespace, "name", template.Spec.Helm.ChartRef.Name) + return ctrl.Result{}, err + } + } else { + if template.Spec.Helm.ChartName == "" { + err = fmt.Errorf("neither chartName nor chartRef is set") + l.Error(err, "invalid helm chart reference") + return ctrl.Result{}, err + } + l.Info("Reconciling helm-controller objects ") + hcChart, err = r.reconcileHelmChart(ctx, template) + if err != nil { + l.Error(err, "Failed to reconcile HelmChart") + return ctrl.Result{}, err + } } if hcChart == nil { - // TODO: add externally referenced source verification + err := fmt.Errorf("HelmChart is nil") + l.Error(err, "could not get the helm chart") return ctrl.Result{}, err } - template.Status.ChartRef = &v2.CrossNamespaceSourceReference{ - Kind: hcChart.Kind, + Kind: sourcev1.HelmChartKind, Name: hcChart.Name, Namespace: hcChart.Namespace, } @@ -96,8 +115,14 @@ func (r *TemplateReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{}, err } + artifact := hcChart.Status.Artifact + + if r.downloadHelmChartFunc == nil { + r.downloadHelmChartFunc = helm.DownloadChartFromArtifact + } + l.Info("Downloading Helm chart") - helmChart, err := helm.DownloadChartFromArtifact(ctx, hcChart.Status.Artifact) + helmChart, err := r.downloadHelmChartFunc(ctx, artifact) if err != nil { l.Error(err, "Failed to download Helm chart") err = fmt.Errorf("failed to download chart: %s", err) @@ -134,13 +159,17 @@ func (r *TemplateReconciler) parseChartMetadata(template *hmc.Template, chart *c if chart.Metadata == nil { return fmt.Errorf("chart metadata is empty") } - templateType := chart.Metadata.Annotations[hmc.ChartAnnotationType] - switch hmc.TemplateType(templateType) { - case hmc.TemplateTypeDeployment, hmc.TemplateTypeProvider, hmc.TemplateTypeCore: - default: - return errNoProviderType + // the value in spec has higher priority + templateType := template.Spec.Type + if templateType == "" { + templateType = hmc.TemplateType(chart.Metadata.Annotations[hmc.ChartAnnotationType]) + switch templateType { + case hmc.TemplateTypeDeployment, hmc.TemplateTypeProvider, hmc.TemplateTypeCore: + default: + return errNoProviderType + } } - template.Status.Type = hmc.TemplateType(templateType) + template.Status.Type = templateType // the value in spec has higher priority if len(template.Spec.Providers.InfrastructureProviders) > 0 { @@ -222,6 +251,21 @@ func (r *TemplateReconciler) reconcileHelmChart(ctx context.Context, template *h return helmChart, nil } +func (r *TemplateReconciler) getHelmChartFromChartRef(ctx context.Context, chartRef *helmcontrollerv2.CrossNamespaceSourceReference) (*sourcev1.HelmChart, error) { + if chartRef.Kind != sourcev1.HelmChartKind { + return nil, fmt.Errorf("invalid chartRef.Kind: %s. Only HelmChart kind is supported", chartRef.Kind) + } + helmChart := &sourcev1.HelmChart{} + err := r.Get(ctx, types.NamespacedName{ + Namespace: chartRef.Namespace, + Name: chartRef.Name, + }, helmChart) + if err != nil { + return nil, err + } + return helmChart, nil +} + // SetupWithManager sets up the controller with the Manager. func (r *TemplateReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). diff --git a/internal/controller/template_controller_test.go b/internal/controller/template_controller_test.go index c4f0f50f2..ac86ed55d 100644 --- a/internal/controller/template_controller_test.go +++ b/internal/controller/template_controller_test.go @@ -18,8 +18,10 @@ import ( "context" v2 "github.com/fluxcd/helm-controller/api/v2" + sourcev1 "github.com/fluxcd/source-controller/api/v1" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "helm.sh/helm/v3/pkg/chart" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -31,6 +33,20 @@ import ( var _ = Describe("Template Controller", func() { Context("When reconciling a resource", func() { const resourceName = "test-resource" + const helmRepoNamespace = "default" + const helmRepoName = "test-helmrepo" + const helmChartName = "test-helmchart" + const helmChartURL = "http://source-controller.hmc-system.svc.cluster.local./helmchart/hmc-system/test-chart/0.1.0.tar.gz" + + var fakeDownloadHelmChartFunc = func(context.Context, *sourcev1.Artifact) (*chart.Chart, error) { + return &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: "v2", + Version: "0.1.0", + Name: "test-chart", + }, + }, nil + } ctx := context.Background() @@ -39,10 +55,53 @@ var _ = Describe("Template Controller", func() { Namespace: "default", // TODO(user):Modify as needed } template := &hmcmirantiscomv1alpha1.Template{} + helmRepo := &sourcev1.HelmRepository{} + helmChart := &sourcev1.HelmChart{} BeforeEach(func() { + By("creating helm repository") + err := k8sClient.Get(ctx, types.NamespacedName{Name: helmRepoName, Namespace: helmRepoNamespace}, helmRepo) + if err != nil && errors.IsNotFound(err) { + helmRepo = &sourcev1.HelmRepository{ + ObjectMeta: metav1.ObjectMeta{ + Name: helmRepoName, + Namespace: helmRepoNamespace, + }, + Spec: sourcev1.HelmRepositorySpec{ + URL: "oci://test/helmrepo", + }, + } + Expect(k8sClient.Create(ctx, helmRepo)).To(Succeed()) + } + + By("creating helm chart") + err = k8sClient.Get(ctx, types.NamespacedName{Name: helmChartName, Namespace: helmRepoNamespace}, helmChart) + if err != nil && errors.IsNotFound(err) { + helmChart = &sourcev1.HelmChart{ + ObjectMeta: metav1.ObjectMeta{ + Name: helmChartName, + Namespace: helmRepoNamespace, + }, + Spec: sourcev1.HelmChartSpec{ + SourceRef: sourcev1.LocalHelmChartSourceReference{ + Kind: sourcev1.HelmRepositoryKind, + Name: helmRepoName, + }, + }, + } + Expect(k8sClient.Create(ctx, helmChart)).To(Succeed()) + } + + By("updating HelmChart status with artifact URL") + helmChart.Status.URL = helmChartURL + helmChart.Status.Artifact = &sourcev1.Artifact{ + URL: helmChartURL, + LastUpdateTime: metav1.Now(), + } + Expect(k8sClient.Status().Update(ctx, helmChart)).Should(Succeed()) + By("creating the custom resource for the Kind Template") - err := k8sClient.Get(ctx, typeNamespacedName, template) + err = k8sClient.Get(ctx, typeNamespacedName, template) if err != nil && errors.IsNotFound(err) { resource := &hmcmirantiscomv1alpha1.Template{ ObjectMeta: metav1.ObjectMeta{ @@ -53,10 +112,11 @@ var _ = Describe("Template Controller", func() { Helm: hmcmirantiscomv1alpha1.HelmSpec{ ChartRef: &v2.CrossNamespaceSourceReference{ Kind: "HelmChart", - Name: "ref-test", - Namespace: "default", + Name: helmChartName, + Namespace: helmRepoNamespace, }, }, + Type: hmcmirantiscomv1alpha1.TemplateTypeDeployment, }, // TODO(user): Specify other spec details if needed. } @@ -76,8 +136,9 @@ var _ = Describe("Template Controller", func() { It("should successfully reconcile the resource", func() { By("Reconciling the created resource") controllerReconciler := &TemplateReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), + Client: k8sClient, + Scheme: k8sClient.Scheme(), + downloadHelmChartFunc: fakeDownloadHelmChartFunc, } _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{