From 459407ad55dfa26d668bd0502808e48e8d68c201 Mon Sep 17 00:00:00 2001 From: Ben Oukhanov Date: Tue, 1 Aug 2023 12:13:54 +0300 Subject: [PATCH] refactor: cleanup tekton operands Cleanup Tekton operands to improve code quality and readability. Also set specific RBAC groups to core instead of the wildcard. Signed-off-by: Ben Oukhanov --- config/rbac/role.yaml | 44 ++-- controllers/setup.go | 6 +- .../ssp-operator.clusterserviceversion.yaml | 44 ++-- internal/common/resource.go | 29 +++ .../operands/tekton-pipelines/reconcile.go | 29 +-- internal/operands/tekton-tasks/reconcile.go | 112 +++++----- .../operands/tekton-tasks/reconcile_test.go | 80 +++---- internal/tekton-bundle/tekton-bundle.go | 195 ++++++++---------- internal/tekton-bundle/tekton-bundle_test.go | 63 +++--- 9 files changed, 290 insertions(+), 312 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index ba803b9a8..c2c3a6e0d 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -25,33 +25,6 @@ rules: - list - update - watch -- apiGroups: - - '*' - resources: - - configmaps - verbs: - - create - - delete - - list - - watch -- apiGroups: - - '*' - resources: - - persistentvolumeclaims - verbs: - - '*' -- apiGroups: - - '*' - resources: - - pods - verbs: - - create -- apiGroups: - - '*' - resources: - - secrets - verbs: - - '*' - apiGroups: - admissionregistration.k8s.io resources: @@ -153,6 +126,15 @@ rules: - infrastructures verbs: - get +- apiGroups: + - "" + resources: + - configmaps + verbs: + - create + - delete + - list + - watch - apiGroups: - "" resources: @@ -180,6 +162,7 @@ rules: resources: - persistentvolumeclaims verbs: + - '*' - create - delete - get @@ -208,9 +191,16 @@ rules: resources: - pods verbs: + - create - get - list - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - '*' - apiGroups: - "" resources: diff --git a/controllers/setup.go b/controllers/setup.go index 9fbbef429..4afa6e996 100644 --- a/controllers/setup.go +++ b/controllers/setup.go @@ -65,12 +65,14 @@ func setupManager(ctx context.Context, cancel context.CancelFunc, mgr controller return fmt.Errorf("failed to read vm-console-proxy bundle: %w", err) } - tektonPipelinesBundle, err := tekton_bundle.ReadPipelineBundle() + tektonPipelinesBundlePaths := tekton_bundle.GetTektonPipelineBundlePaths() + tektonPipelinesBundle, err := tekton_bundle.ReadBundle(tektonPipelinesBundlePaths) if err != nil { return err } - tektonTasksBundle, err := tekton_bundle.ReadTasksBundle(runningOnOpenShift) + tektonTasksBundlePath := tekton_bundle.GetTektonTasksBundlePath(runningOnOpenShift) + tektonTasksBundle, err := tekton_bundle.ReadBundle([]string{tektonTasksBundlePath}) if err != nil { return err } diff --git a/data/olm-catalog/ssp-operator.clusterserviceversion.yaml b/data/olm-catalog/ssp-operator.clusterserviceversion.yaml index 533b27720..76b192f4b 100644 --- a/data/olm-catalog/ssp-operator.clusterserviceversion.yaml +++ b/data/olm-catalog/ssp-operator.clusterserviceversion.yaml @@ -83,33 +83,6 @@ spec: - list - update - watch - - apiGroups: - - '*' - resources: - - configmaps - verbs: - - create - - delete - - list - - watch - - apiGroups: - - '*' - resources: - - persistentvolumeclaims - verbs: - - '*' - - apiGroups: - - '*' - resources: - - pods - verbs: - - create - - apiGroups: - - '*' - resources: - - secrets - verbs: - - '*' - apiGroups: - admissionregistration.k8s.io resources: @@ -211,6 +184,15 @@ spec: - infrastructures verbs: - get + - apiGroups: + - "" + resources: + - configmaps + verbs: + - create + - delete + - list + - watch - apiGroups: - "" resources: @@ -238,6 +220,7 @@ spec: resources: - persistentvolumeclaims verbs: + - '*' - create - delete - get @@ -266,9 +249,16 @@ spec: resources: - pods verbs: + - create - get - list - watch + - apiGroups: + - "" + resources: + - secrets + verbs: + - '*' - apiGroups: - "" resources: diff --git a/internal/common/resource.go b/internal/common/resource.go index c5ca94857..e8a336365 100644 --- a/internal/common/resource.go +++ b/internal/common/resource.go @@ -549,3 +549,32 @@ func defaultStatusFunc(obj client.Object) ResourceStatus { } return status } + +// AppendDeepCopies appends deep copies of objects from a source slice to a destination slice. +// This function is useful for creating a new slice containing deep copies of the provided objects. +// +// The PT interface {*T ; client.Object } is a trick. It means that type PT satisfies +// an anonymous typeset defined directly in the function signature. +// This typeset interface { *T; client.Object } means that it is a pointer to T +// and implements interface client.Object. +// +// Parameters: +// - destination: The destination slice where the deep copies of objects will be appended. +// - objects: A slice of objects (of type T) to be deep copied and appended to the destination slice. +// +// Returns: +// - The updated destination slice containing the appended deep copies of objects. +// +// Type Parameters: +// - PT: A type that is a pointer to the object type (should implement *T and client.Object interfaces). +// - T: The actual object type. +func AppendDeepCopies[PT interface { + *T + client.Object +}, T any](destination []client.Object, objects []T) []client.Object { + for i := range objects { + var object = PT(&objects[i]) + destination = append(destination, object.DeepCopyObject().(client.Object)) + } + return destination +} diff --git a/internal/operands/tekton-pipelines/reconcile.go b/internal/operands/tekton-pipelines/reconcile.go index 5ad875944..4b4dcf3ee 100644 --- a/internal/operands/tekton-pipelines/reconcile.go +++ b/internal/operands/tekton-pipelines/reconcile.go @@ -17,7 +17,7 @@ import ( ) // +kubebuilder:rbac:groups=tekton.dev,resources=pipelines,verbs=list;watch;create;update;delete -// +kubebuilder:rbac:groups=*,resources=configmaps,verbs=list;watch;create;delete +// +kubebuilder:rbac:groups=core,resources=configmaps,verbs=list;watch;create;delete // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=list;watch;create;update;delete const ( @@ -116,25 +116,15 @@ func (t *tektonPipelines) Reconcile(request *common.Request) ([]common.Reconcile func (t *tektonPipelines) Cleanup(request *common.Request) ([]common.CleanupResult, error) { var objects []client.Object + if request.CrdList.CrdExists(tektonCrd) { - for _, p := range t.pipelines { - o := p.DeepCopy() - objects = append(objects, o) - } - } - for _, cm := range t.configMaps { - o := cm.DeepCopy() - objects = append(objects, o) - } - for _, rb := range t.roleBindings { - o := rb.DeepCopy() - objects = append(objects, o) - } - for _, sa := range t.serviceAccounts { - o := sa.DeepCopy() - objects = append(objects, o) + objects = common.AppendDeepCopies(objects, t.pipelines) } + objects = common.AppendDeepCopies(objects, t.configMaps) + objects = common.AppendDeepCopies(objects, t.roleBindings) + objects = common.AppendDeepCopies(objects, t.serviceAccounts) + namespace, isUserDefinedNamespace := getTektonPipelinesNamespace(request) for i, o := range objects { objectNamespace := namespace @@ -144,10 +134,7 @@ func (t *tektonPipelines) Cleanup(request *common.Request) ([]common.CleanupResu objects[i].SetNamespace(objectNamespace) } - for _, cr := range t.clusterRoles { - o := cr.DeepCopy() - objects = append(objects, o) - } + objects = common.AppendDeepCopies(objects, t.clusterRoles) return common.DeleteAll(request, objects...) } diff --git a/internal/operands/tekton-tasks/reconcile.go b/internal/operands/tekton-tasks/reconcile.go index 1ac48444b..09c7300f3 100644 --- a/internal/operands/tekton-tasks/reconcile.go +++ b/internal/operands/tekton-tasks/reconcile.go @@ -7,6 +7,7 @@ import ( pipeline "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" v1 "k8s.io/api/core/v1" rbac "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "kubevirt.io/ssp-operator/internal/common" "kubevirt.io/ssp-operator/internal/operands" @@ -24,9 +25,9 @@ import ( // +kubebuilder:rbac:groups=cdi.kubevirt.io,resources=datavolumes,verbs=* // +kubebuilder:rbac:groups=cdi.kubevirt.io,resources=datasources,verbs=get;create;delete // +kubebuilder:rbac:groups=kubevirt.io,resources=virtualmachines/finalizers,verbs=* -// +kubebuilder:rbac:groups=*,resources=persistentvolumeclaims,verbs=* -// +kubebuilder:rbac:groups=*,resources=pods,verbs=create -// +kubebuilder:rbac:groups=*,resources=secrets,verbs=* +// +kubebuilder:rbac:groups=core,resources=persistentvolumeclaims,verbs=* +// +kubebuilder:rbac:groups=core,resources=pods,verbs=create +// +kubebuilder:rbac:groups=core,resources=secrets,verbs=* const ( operandName = "tekton-tasks" @@ -87,43 +88,11 @@ type tektonTasks struct { var _ operands.Operand = &tektonTasks{} func New(bundle *tektonbundle.Bundle) operands.Operand { - newTasks := []pipeline.Task{} - for _, task := range bundle.Tasks { - if _, ok := AllowedTasks[task.Name]; ok { - newTasks = append(newTasks, task) - } - } - bundle.Tasks = newTasks - - newServiceAccounts := []v1.ServiceAccount{} - for _, serviceAccount := range bundle.ServiceAccounts { - if _, ok := AllowedTasks[strings.TrimSuffix(serviceAccount.Name, "-task")]; ok { - newServiceAccounts = append(newServiceAccounts, serviceAccount) - } - } - bundle.ServiceAccounts = newServiceAccounts - - newRoleBinding := []rbac.RoleBinding{} - for _, roleBinding := range bundle.RoleBindings { - if _, ok := AllowedTasks[strings.TrimSuffix(roleBinding.Name, "-task")]; ok { - newRoleBinding = append(newRoleBinding, roleBinding) - } - } - bundle.RoleBindings = newRoleBinding - - newClusterRole := []rbac.ClusterRole{} - for _, clusterRole := range bundle.ClusterRoles { - if _, ok := AllowedTasks[strings.TrimSuffix(clusterRole.Name, "-task")]; ok { - newClusterRole = append(newClusterRole, clusterRole) - } - } - bundle.ClusterRoles = newClusterRole - return &tektonTasks{ - tasks: bundle.Tasks, - serviceAccounts: bundle.ServiceAccounts, - roleBindings: bundle.RoleBindings, - clusterRoles: bundle.ClusterRoles, + tasks: filterTektonResources(bundle.Tasks, false), + serviceAccounts: filterTektonResources(bundle.ServiceAccounts, true), + roleBindings: filterTektonResources(bundle.RoleBindings, true), + clusterRoles: filterTektonResources(bundle.ClusterRoles, true), } } @@ -174,39 +143,27 @@ func (t *tektonTasks) Reconcile(request *common.Request) ([]common.ReconcileResu func (t *tektonTasks) Cleanup(request *common.Request) ([]common.CleanupResult, error) { var objects []client.Object + if request.CrdList.CrdExists(tektonCrd) { - for _, t := range t.tasks { - o := t.DeepCopy() - objects = append(objects, o) - } - } - for _, rb := range t.roleBindings { - o := rb.DeepCopy() - objects = append(objects, o) - } - for _, sa := range t.serviceAccounts { - o := sa.DeepCopy() - objects = append(objects, o) + objects = common.AppendDeepCopies(objects, t.tasks) } + objects = common.AppendDeepCopies(objects, t.roleBindings) + objects = common.AppendDeepCopies(objects, t.serviceAccounts) + for i := range objects { objects[i].SetNamespace(getTektonTasksNamespace(request)) } - for _, cr := range t.clusterRoles { - o := cr.DeepCopy() - objects = append(objects, o) - } + objects = common.AppendDeepCopies(objects, t.clusterRoles) if request.CrdList.CrdExists(tektonCrd) { clusterTasks, err := listDeprecatedClusterTasks(request) if err != nil { return nil, err } - for _, ct := range clusterTasks { - o := ct.DeepCopy() - objects = append(objects, o) - } + + objects = common.AppendDeepCopies(objects, clusterTasks) } return common.DeleteAll(request, objects...) @@ -307,3 +264,40 @@ func getTektonTasksNamespace(request *common.Request) string { } return request.Instance.Namespace } + +// filterTektonResources filters a slice of Tekton resources based on the provided criteria. +// It allows filtering based on the name of the resources, optionally trimming a specified suffix. +// The function only includes resources with names present in the AllowedTasks map. +// +// The PT interface {*T ; meta1.Object } is a trick. It means that type PT satisfies +// an anonymous typeset defined directly in the function signature. +// This typeset interface { *T; metav1.Object } means that it is a pointer to T +// and implements interface metav1.Object. +// +// Parameters: +// - items: A slice of Tekton resource objects to be filtered. +// - trimSuffix: A boolean indicating whether to trim the "-task" suffix from the resource names. +// +// Returns: +// - A new slice containing the filtered Tekton resource objects. +// +// Type Parameters: +// - PT: A type that is a pointer to the resource type (should implement *T and metav1.Object interfaces). +// - T: The actual resource type. +func filterTektonResources[PT interface { + *T + metav1.Object +}, T any](items []T, trimSuffix bool) []T { + var results []T + for i := range items { + object := PT(&items[i]) + name := object.GetName() + if trimSuffix { + name = strings.TrimSuffix(name, "-task") + } + if _, ok := AllowedTasks[name]; ok { + results = append(results, *object) + } + } + return results +} diff --git a/internal/operands/tekton-tasks/reconcile_test.go b/internal/operands/tekton-tasks/reconcile_test.go index fd728f1ca..b711b6cea 100644 --- a/internal/operands/tekton-tasks/reconcile_test.go +++ b/internal/operands/tekton-tasks/reconcile_test.go @@ -32,9 +32,9 @@ const ( var _ = Describe("environments", func() { var ( - operand operands.Operand bundle *tektonbundle.Bundle - request *common.Request + operand operands.Operand + request common.Request ) BeforeEach(func() { @@ -54,69 +54,69 @@ var _ = Describe("environments", func() { }) It("Reconcile function should return correct functions", func() { - functions, err := operand.Reconcile(request) + functions, err := operand.Reconcile(&request) Expect(err).ToNot(HaveOccurred(), "should not throw err") Expect(functions).To(HaveLen(8), "should return correct number of reconcile functions") }) It("Should create tekton-tasks resources", func() { - _, err := operand.Reconcile(request) + _, err := operand.Reconcile(&request) Expect(err).ToNot(HaveOccurred()) for _, task := range bundle.Tasks { - ExpectResourceExists(&task, *request) + ExpectResourceExists(&task, request) } for _, clusterRole := range bundle.ClusterRoles { - ExpectResourceExists(&clusterRole, *request) + ExpectResourceExists(&clusterRole, request) } for _, serviceAccount := range bundle.ServiceAccounts { - ExpectResourceExists(&serviceAccount, *request) + ExpectResourceExists(&serviceAccount, request) } for _, roleBinding := range bundle.RoleBindings { - ExpectResourceExists(&roleBinding, *request) + ExpectResourceExists(&roleBinding, request) } }) - It("should remove tekton-tasks resources on cleanup", func() { - _, err := operand.Reconcile(request) + It("Should remove tekton-tasks resources on cleanup", func() { + _, err := operand.Reconcile(&request) Expect(err).ToNot(HaveOccurred()) for _, task := range bundle.Tasks { - ExpectResourceExists(&task, *request) + ExpectResourceExists(&task, request) } for _, clusterRole := range bundle.ClusterRoles { - ExpectResourceExists(&clusterRole, *request) + ExpectResourceExists(&clusterRole, request) } for _, serviceAccount := range bundle.ServiceAccounts { - ExpectResourceExists(&serviceAccount, *request) + ExpectResourceExists(&serviceAccount, request) } for _, roleBinding := range bundle.RoleBindings { - ExpectResourceExists(&roleBinding, *request) + ExpectResourceExists(&roleBinding, request) } - _, err = operand.Cleanup(request) + _, err = operand.Cleanup(&request) Expect(err).ToNot(HaveOccurred()) for _, task := range bundle.Tasks { - ExpectResourceNotExists(&task, *request) + ExpectResourceNotExists(&task, request) } for _, clusterRole := range bundle.ClusterRoles { - ExpectResourceNotExists(&clusterRole, *request) + ExpectResourceNotExists(&clusterRole, request) } for _, serviceAccount := range bundle.ServiceAccounts { - ExpectResourceNotExists(&serviceAccount, *request) + ExpectResourceNotExists(&serviceAccount, request) } for _, roleBinding := range bundle.RoleBindings { - ExpectResourceNotExists(&roleBinding, *request) + ExpectResourceNotExists(&roleBinding, request) } }) }) @@ -127,23 +127,23 @@ var _ = Describe("environments", func() { }) It("Should not create tekton-tasks resources", func() { - _, err := operand.Reconcile(request) + _, err := operand.Reconcile(&request) Expect(err).ToNot(HaveOccurred()) for _, task := range bundle.Tasks { - ExpectResourceNotExists(&task, *request) + ExpectResourceNotExists(&task, request) } for _, clusterRole := range bundle.ClusterRoles { - ExpectResourceNotExists(&clusterRole, *request) + ExpectResourceNotExists(&clusterRole, request) } for _, serviceAccount := range bundle.ServiceAccounts { - ExpectResourceNotExists(&serviceAccount, *request) + ExpectResourceNotExists(&serviceAccount, request) } for _, roleBinding := range bundle.RoleBindings { - ExpectResourceNotExists(&roleBinding, *request) + ExpectResourceNotExists(&roleBinding, request) } }) }) @@ -154,7 +154,7 @@ func TestTektonTasks(t *testing.T) { RunSpecs(t, "Tekton Tasks Suite") } -func getMockedRequest() *common.Request { +func getMockedRequest() common.Request { log := logf.Log.WithName("tekton-tasks-operand") Expect(internalmeta.AddToScheme(scheme.Scheme)).To(Succeed()) @@ -179,7 +179,7 @@ func getMockedRequest() *common.Request { crdWatch := crd_watch.New(tektonCrd) Expect(crdWatch.Init(context.Background(), client)).To(Succeed()) - return &common.Request{ + return common.Request{ Request: reconcile.Request{ NamespacedName: types.NamespacedName{ Namespace: namespace, @@ -217,8 +217,9 @@ func getMockedTestBundle() *tektonbundle.Bundle { Tasks: []pipeline.Task{ { ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{}, - Name: diskVirtSysprepTaskName, + Labels: map[string]string{}, + Name: diskVirtSysprepTaskName, + Namespace: namespace, }, Spec: pipeline.TaskSpec{ Steps: []pipeline.Step{ @@ -229,8 +230,9 @@ func getMockedTestBundle() *tektonbundle.Bundle { }, }, { ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{}, - Name: modifyTemplateTaskName, + Labels: map[string]string{}, + Name: modifyTemplateTaskName, + Namespace: namespace, }, Spec: pipeline.TaskSpec{ Steps: []pipeline.Step{ @@ -244,33 +246,39 @@ func getMockedTestBundle() *tektonbundle.Bundle { ServiceAccounts: []v1.ServiceAccount{ { ObjectMeta: metav1.ObjectMeta{ - Name: diskVirtSysprepTaskName + "-task", + Name: diskVirtSysprepTaskName + "-task", + Namespace: namespace, }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: modifyTemplateTaskName + "-task", + Name: modifyTemplateTaskName + "-task", + Namespace: namespace, }, }, }, RoleBindings: []rbac.RoleBinding{ { ObjectMeta: metav1.ObjectMeta{ - Name: diskVirtSysprepTaskName + "-task", + Name: diskVirtSysprepTaskName + "-task", + Namespace: namespace, }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: modifyTemplateTaskName + "-task", + Name: modifyTemplateTaskName + "-task", + Namespace: namespace, }, }, }, ClusterRoles: []rbac.ClusterRole{ { ObjectMeta: metav1.ObjectMeta{ - Name: diskVirtSysprepTaskName + "-task", + Name: diskVirtSysprepTaskName + "-task", + Namespace: namespace, }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: modifyTemplateTaskName + "-task", + Name: modifyTemplateTaskName + "-task", + Namespace: namespace, }, }, }, diff --git a/internal/tekton-bundle/tekton-bundle.go b/internal/tekton-bundle/tekton-bundle.go index b867b479e..596ca5bbb 100644 --- a/internal/tekton-bundle/tekton-bundle.go +++ b/internal/tekton-bundle/tekton-bundle.go @@ -1,11 +1,9 @@ package tekton_bundle import ( - "bytes" "encoding/json" "io" "os" - "path/filepath" pipeline "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -38,138 +36,109 @@ type Bundle struct { ConfigMaps []v1.ConfigMap } -func ReadTasksBundle(isOpenshift bool) (*Bundle, error) { - var files [][]byte - path := getTasksBundlePath(isOpenshift) - data, err := os.ReadFile(path) - if err != nil { - return nil, err - } - files = append(files, data) - - tektonObjs, err := decodeObjectsFromFiles(files) - if err != nil { - return nil, err - } - - return tektonObjs, nil +var tektonPipelineBundlePaths = []string{ + filepath.Join(tektonPipelinesBundleDir, "pipelines-rbac.yaml"), + filepath.Join(tektonPipelinesBundleDir, "windows-bios-installer-configmaps.yaml"), + filepath.Join(tektonPipelinesBundleDir, "windows-bios-installer-pipeline.yaml"), + filepath.Join(tektonPipelinesBundleDir, "windows-customize-configmaps.yaml"), + filepath.Join(tektonPipelinesBundleDir, "windows-customize-pipeline.yaml"), + filepath.Join(tektonPipelinesBundleDir, "windows-efi-installer-configmaps.yaml"), + filepath.Join(tektonPipelinesBundleDir, "windows-efi-installer-pipeline.yaml"), } -func ReadPipelineBundle() (*Bundle, error) { - path := getPipelineBundlePath() - files, err := readFolder(path) - if err != nil { - return nil, err - } - - tektonObjs, err := decodeObjectsFromFiles(files) - if err != nil { - return nil, err - } - - return tektonObjs, nil -} - -func getPipelineBundlePath() string { - return tektonPipelinesBundleDir +func GetTektonPipelineBundlePaths() []string { + return tektonPipelineBundlePaths } -func getTasksBundlePath(isOpenshift bool) string { - if isOpenshift { +func GetTektonTasksBundlePath(isOpenShift bool) string { + if isOpenShift { return filepath.Join(tektonTasksOKDBundleDir, "kubevirt-tekton-tasks-okd.yaml") } return filepath.Join(tektonTasksKubernetesBundleDir, "kubevirt-tekton-tasks-kubernetes.yaml") } -func readFolder(folderPath string) ([][]byte, error) { - files, err := os.ReadDir(folderPath) - if err != nil { - return nil, err - } - filesBytes := make([][]byte, 0, len(files)) - for _, file := range files { - if file.IsDir() { - continue - } - - f, err := os.ReadFile(filepath.Join(folderPath, file.Name())) +func ReadBundle(paths []string) (*Bundle, error) { + bundle := &Bundle{} + for _, path := range paths { + err := decodeObjects(path, bundle) if err != nil { return nil, err } - filesBytes = append(filesBytes, f) } - - return filesBytes, nil + return bundle, nil } -func decodeObjectsFromFiles(files [][]byte) (*Bundle, error) { - bundle := &Bundle{} - for _, file := range files { - decoder := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(file), 1024) - for { - var obj map[string]interface{} - err := decoder.Decode(&obj) - if err == io.EOF { - break - } - if err != nil { - return nil, err +func decodeObjects(path string, bundle *Bundle) error { + file, err := os.Open(path) + if err != nil { + return err + } + defer file.Close() + + decoder := yaml.NewYAMLOrJSONDecoder(file, 1024) + for { + var obj map[string]interface{} + err := decoder.Decode(&obj) + if err == io.EOF { + break + } + if err != nil { + return err + } + if kind, ok := obj["kind"].(string); ok { + if kind == "" { + continue } - if kind, ok := obj["kind"].(string); ok { - if kind == "" { - continue - } - switch kind { - case tasksString: - task := pipeline.Task{} - err = getObject(obj, &task) - if err != nil { - return nil, err - } - bundle.Tasks = append(bundle.Tasks, task) - case pipelineKindString: - p := pipeline.Pipeline{} - err = getObject(obj, &p) - if err != nil { - return nil, err - } - bundle.Pipelines = append(bundle.Pipelines, p) - case serviceAccountKind: - sa := v1.ServiceAccount{} - err = getObject(obj, &sa) - if err != nil { - return nil, err - } - bundle.ServiceAccounts = append(bundle.ServiceAccounts, sa) - case roleBindingKind: - rb := rbac.RoleBinding{} - err = getObject(obj, &rb) - if err != nil { - return nil, err - } - bundle.RoleBindings = append(bundle.RoleBindings, rb) - case clusterRoleKind: - cr := rbac.ClusterRole{} - err = getObject(obj, &cr) - if err != nil { - return nil, err - } - bundle.ClusterRoles = append(bundle.ClusterRoles, cr) - case configMapKind: - cm := v1.ConfigMap{} - err = getObject(obj, &cm) - if err != nil { - return nil, err - } - bundle.ConfigMaps = append(bundle.ConfigMaps, cm) - default: - continue + switch kind { + case tasksString: + task := pipeline.Task{} + err = getObject(obj, &task) + if err != nil { + return err + } + bundle.Tasks = append(bundle.Tasks, task) + case pipelineKindString: + p := pipeline.Pipeline{} + err = getObject(obj, &p) + if err != nil { + return err + } + bundle.Pipelines = append(bundle.Pipelines, p) + case serviceAccountKind: + sa := v1.ServiceAccount{} + err = getObject(obj, &sa) + if err != nil { + return err } + bundle.ServiceAccounts = append(bundle.ServiceAccounts, sa) + case roleBindingKind: + rb := rbac.RoleBinding{} + err = getObject(obj, &rb) + if err != nil { + return err + } + bundle.RoleBindings = append(bundle.RoleBindings, rb) + case clusterRoleKind: + cr := rbac.ClusterRole{} + err = getObject(obj, &cr) + if err != nil { + return err + } + bundle.ClusterRoles = append(bundle.ClusterRoles, cr) + case configMapKind: + cm := v1.ConfigMap{} + err = getObject(obj, &cm) + if err != nil { + return err + } + bundle.ConfigMaps = append(bundle.ConfigMaps, cm) + default: + continue } } } - return bundle, nil + return nil } func getObject(obj map[string]interface{}, newObj interface{}) error { diff --git a/internal/tekton-bundle/tekton-bundle_test.go b/internal/tekton-bundle/tekton-bundle_test.go index 74be6986f..26ff2affb 100644 --- a/internal/tekton-bundle/tekton-bundle_test.go +++ b/internal/tekton-bundle/tekton-bundle_test.go @@ -19,43 +19,52 @@ const ( ) var _ = Describe("Tekton bundle", func() { - It("should return correct pipeline folder path ", func() { - path := getPipelineBundlePath() - Expect(path).To(Equal("/data/tekton-pipelines/")) + It("should return tekton pipeline paths", func() { + paths := GetTektonPipelineBundlePaths() + Expect(paths).To(HaveLen(7), "number of paths should equal") }) - It("should return correct task path on okd", func() { - path := getTasksBundlePath(true) - Expect(path).To(Equal("/data/tekton-tasks/okd/kubevirt-tekton-tasks-okd.yaml")) - }) + It("should return tekton tasks path correctly", func() { + kubernetesPath := GetTektonTasksBundlePath(false) + okdPath := GetTektonTasksBundlePath(true) - It("should return correct task path on kubernetes", func() { - path := getTasksBundlePath(false) - Expect(path).To(Equal("/data/tekton-tasks/kubernetes/kubevirt-tekton-tasks-kubernetes.yaml")) + Expect(kubernetesPath).To(Equal("/data/tekton-tasks/kubernetes/kubevirt-tekton-tasks-kubernetes.yaml")) + Expect(okdPath).To(Equal("/data/tekton-tasks/okd/kubevirt-tekton-tasks-okd.yaml")) }) - It("should load correct files and convert them", func() { + It("should read tekton tasks bundle correctly", func() { path, _ := os.Getwd() - - taskPath := filepath.Join(path, "test-bundle-files/test-tasks/test-tasks.yaml") - pipelinePath := filepath.Join(path, "test-bundle-files/test-pipelines/") - - taskFile, err := os.ReadFile(taskPath) + bundlePath := filepath.Join(path, "test-bundle-files/test-tasks/test-tasks.yaml") + bundle, err := ReadBundle([]string{bundlePath}) Expect(err).ToNot(HaveOccurred()) - pipelineFiles, err := readFolder(pipelinePath) + Expect(bundle.Tasks).To(HaveLen(numberOfTasks), "number of tasks should equal") + }) + + It("should read tekton pipelines bundle correctly", func() { + path, _ := os.Getwd() + var bundlePaths []string + bundlePaths = append(bundlePaths, filepath.Join(path, "test-bundle-files/test-pipelines/windows-installer.yaml")) + bundlePaths = append(bundlePaths, filepath.Join(path, "test-bundle-files/test-pipelines/windows-installer2-test.yaml")) + bundle, err := ReadBundle(bundlePaths) Expect(err).ToNot(HaveOccurred()) - files := [][]byte{} - files = append(files, taskFile) - files = append(files, pipelineFiles...) + Expect(bundle.Pipelines).To(HaveLen(numberOfPipelines), "number of pipelines should equal") + }) + + It("should load tekton bundle correctly", func() { + path, _ := os.Getwd() + var paths []string + paths = append(paths, filepath.Join(path, "test-bundle-files/test-tasks/test-tasks.yaml")) + paths = append(paths, filepath.Join(path, "test-bundle-files/test-pipelines/windows-installer.yaml")) + paths = append(paths, filepath.Join(path, "test-bundle-files/test-pipelines/windows-installer2-test.yaml")) - tektonObjs, err := decodeObjectsFromFiles(files) + bundle, err := ReadBundle(paths) Expect(err).ToNot(HaveOccurred(), "it should not throw error") - Expect(tektonObjs.Tasks).To(HaveLen(numberOfTasks), "number of tasks should equal") - Expect(tektonObjs.ServiceAccounts).To(HaveLen(numberOfServiceAccounts), "number of service accounts should equal") - Expect(tektonObjs.RoleBindings).To(HaveLen(numberOfRoleBindings), "number of role bindings should equal") - Expect(tektonObjs.ClusterRoles).To(HaveLen(numberOfClusterRoles), "number of cluster roles should equal") - Expect(tektonObjs.Pipelines).To(HaveLen(numberOfPipelines), "number of pipelines should equal") - Expect(tektonObjs.ConfigMaps).To(HaveLen(numberOfConfigMaps), "number of config maps should equal") + Expect(bundle.Tasks).To(HaveLen(numberOfTasks), "number of tasks should equal") + Expect(bundle.ServiceAccounts).To(HaveLen(numberOfServiceAccounts), "number of service accounts should equal") + Expect(bundle.RoleBindings).To(HaveLen(numberOfRoleBindings), "number of role bindings should equal") + Expect(bundle.ClusterRoles).To(HaveLen(numberOfClusterRoles), "number of cluster roles should equal") + Expect(bundle.Pipelines).To(HaveLen(numberOfPipelines), "number of pipelines should equal") + Expect(bundle.ConfigMaps).To(HaveLen(numberOfConfigMaps), "number of config maps should equal") }) })