From 8d07f9f078ec64b1c4e0f8bf9f38bba88610bd00 Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Thu, 7 Mar 2024 14:29:28 -0300 Subject: [PATCH] fix: Enable more linters (#10) Signed-off-by: Mateus Oliveira --- .github/workflows/ci.yml | 6 +- .golangci.yml | 131 ++++++-- Dockerfile | 2 +- Makefile | 2 +- api/v1alpha1/nonadminbackup_types.go | 16 +- cmd/main.go | 14 +- internal/common/constant/constant.go | 31 ++ internal/common/function/function.go | 224 +++++++++++++ internal/common/function/function_test.go | 308 ++++++++++++++++++ internal/controller/common_k8s.go | 75 ----- internal/controller/common_k8s_test.go | 140 -------- internal/controller/common_nab.go | 77 ----- internal/controller/common_nab_test.go | 87 ----- internal/controller/common_velero.go | 65 ---- internal/controller/common_velero_test.go | 86 ----- .../controller/nonadminbackup_controller.go | 83 ++--- .../nonadminbackup_controller_test.go | 31 +- .../controller/nonadminbackup_predicate.go | 67 ---- internal/controller/suite_test.go | 35 +- internal/controller/velerobackup_handler.go | 70 ---- internal/handler/velerobackup_handler.go | 90 +++++ .../composite_predicate.go | 26 +- .../predicate/nonadminbackup_predicate.go | 86 +++++ .../velerobackup_predicate.go | 48 ++- test/utils/utils.go | 2 +- 25 files changed, 1002 insertions(+), 800 deletions(-) create mode 100644 internal/common/constant/constant.go create mode 100644 internal/common/function/function.go create mode 100644 internal/common/function/function_test.go delete mode 100644 internal/controller/common_k8s.go delete mode 100644 internal/controller/common_k8s_test.go delete mode 100644 internal/controller/common_nab.go delete mode 100644 internal/controller/common_nab_test.go delete mode 100644 internal/controller/common_velero.go delete mode 100644 internal/controller/common_velero_test.go delete mode 100644 internal/controller/nonadminbackup_predicate.go delete mode 100644 internal/controller/velerobackup_handler.go create mode 100644 internal/handler/velerobackup_handler.go rename internal/{controller => predicate}/composite_predicate.go (65%) create mode 100644 internal/predicate/nonadminbackup_predicate.go rename internal/{controller => predicate}/velerobackup_predicate.go (50%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c6955f..ba4176f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,11 @@ jobs: # TODO coverage report - name: Go linters - run: make lint + if: ${{ matrix.go-version == '1.21' }} + uses: golangci/golangci-lint-action@v4 + with: + version: v1.56.2 + skip-pkg-cache: true container-check: runs-on: ubuntu-latest diff --git a/.golangci.yml b/.golangci.yml index aed8644..a786073 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,40 +1,129 @@ +# Documentation reference https://github.com/golangci/golangci-lint/blob/v1.56.2/.golangci.reference.yml run: - deadline: 5m + skip-dirs-use-default: false + modules-download-mode: readonly allow-parallel-runners: true + skip-dirs: + - test/* + +output: + format: colored-line-number + print-issued-lines: true + print-linter-name: true + uniq-by-line: true + sort-results: true + +linters-settings: + dogsled: + max-blank-identifiers: 2 + errcheck: + check-type-assertions: true + check-blank: true + gci: + sections: + - standard + - default + - prefix(github.com/migtools/oadp-non-admin) + goconst: + min-len: 3 + min-occurrences: 5 + gofmt: + simplify: true + goheader: + # copy from ./hack/boilerplate.go.txt + template: |- + Copyright 2024. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + govet: + enable-all: true + misspell: + locale: US + nakedret: + max-func-lines: 30 + nolintlint: + allow-unused: false + allow-no-explanation: [] + require-explanation: true + require-specific: true + revive: + enable-all-rules: true + rules: + - name: line-length-limit + disabled: true + - name: function-length + disabled: true + # TODO remove + - name: cyclomatic + disabled: true + - name: cognitive-complexity + disabled: true + unparam: + check-exported: true -issues: - # don't skip warning about doc comments - # don't exclude the default set of lint - exclude-use-default: false - # restore some of the defaults - # (fill in the rest as needed) - exclude-rules: - - path: "api/*" - linters: - - lll - - path: "internal/*" - linters: - - dupl - - lll linters: disable-all: true enable: - - dupl + - asasalint + - asciicheck + - bidichk + - bodyclose + - dogsled + - dupword + - durationcheck - errcheck + - errchkjson - exportloopref + - gci + - ginkgolinter - goconst - - gocyclo - gofmt - - goimports + - goheader + - goprintffuncname + - gosec - gosimple - govet - ineffassign - - lll - misspell - nakedret - - prealloc + - nilerr + - noctx + - nolintlint + - nosprintfhostport + - revive - staticcheck - - typecheck + - stylecheck - unconvert - unparam - unused + - usestdlibvars + fast: false + +issues: + exclude-use-default: false + exclude-rules: + - linters: + - revive + text: "^struct-tag: unknown option 'inline' in JSON tag$" + - linters: + - revive + text: "^add-constant: avoid magic numbers like '0', create a named constant for it$" + - linters: + - revive + text: "^add-constant: avoid magic numbers like '1', create a named constant for it$" + max-issues-per-linter: 0 + max-same-issues: 0 + +severity: + default-severity: error + case-sensitive: false diff --git a/Dockerfile b/Dockerfile index aca26f9..fa0ed5d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,7 +14,7 @@ RUN go mod download # Copy the go source COPY cmd/main.go cmd/main.go COPY api/ api/ -COPY internal/controller/ internal/controller/ +COPY internal/ internal/ # Build # the GOARCH has not a default value to allow the binary be built according to the host where the command diff --git a/Makefile b/Makefile index 356b022..5195038 100644 --- a/Makefile +++ b/Makefile @@ -166,7 +166,7 @@ GOLANGCI_LINT = $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION) KUSTOMIZE_VERSION ?= v5.3.0 CONTROLLER_TOOLS_VERSION ?= v0.14.0 ENVTEST_VERSION ?= latest -GOLANGCI_LINT_VERSION ?= v1.54.2 +GOLANGCI_LINT_VERSION ?= v1.56.2 .PHONY: kustomize kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. diff --git a/api/v1alpha1/nonadminbackup_types.go b/api/v1alpha1/nonadminbackup_types.go index 56bb58e..10fb806 100644 --- a/api/v1alpha1/nonadminbackup_types.go +++ b/api/v1alpha1/nonadminbackup_types.go @@ -23,11 +23,6 @@ import ( // NonAdminBackupSpec defines the desired state of NonAdminBackup type NonAdminBackupSpec struct { - // NonAdminBackup log level (use debug for the most logging, leave unset for default) - // +optional - // +kubebuilder:validation:Enum=trace;debug;info;warning;error;fatal;panic - LogLevel string `json:"logLevel,omitempty"` - // https://github.com/vmware-tanzu/velero/blob/main/pkg/apis/velero/v1/backup_types.go // BackupSpec defines the specification for a Velero backup. @@ -35,6 +30,11 @@ type NonAdminBackupSpec struct { // BackupStatus captures the current status of a Velero backup. BackupStatus *velerov1api.BackupStatus `json:"backupStatus,omitempty"` + + // NonAdminBackup log level (use debug for the most logging, leave unset for default) + // +optional + // +kubebuilder:validation:Enum=trace;debug;info;warning;error;fatal;panic + LogLevel string `json:"logLevel,omitempty"` } // NonAdminBackupStatus defines the observed state of NonAdminBackup @@ -42,8 +42,8 @@ type NonAdminBackupStatus struct { Conditions []metav1.Condition `json:"conditions,omitempty"` } -//+kubebuilder:object:root=true -//+kubebuilder:subresource:status +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status // NonAdminBackup is the Schema for the nonadminbackups API type NonAdminBackup struct { @@ -54,7 +54,7 @@ type NonAdminBackup struct { Status NonAdminBackupStatus `json:"status,omitempty"` } -//+kubebuilder:object:root=true +// +kubebuilder:object:root=true // NonAdminBackupList contains a list of NonAdminBackup type NonAdminBackupList struct { diff --git a/cmd/main.go b/cmd/main.go index bd76f4e..218bb22 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Entrypoint of the project package main import ( @@ -22,14 +23,12 @@ import ( "os" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - - // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) - // to ensure that exec-entrypoint and run can make use of them. - _ "k8s.io/client-go/plugin/pkg/client/auth" - "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) + // to ensure that exec-entrypoint and run can make use of them. + _ "k8s.io/client-go/plugin/pkg/client/auth" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -38,7 +37,6 @@ import ( nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" "github.com/migtools/oadp-non-admin/internal/controller" - //+kubebuilder:scaffold:imports ) var ( @@ -52,7 +50,7 @@ func init() { utilruntime.Must(nacv1alpha1.AddToScheme(scheme)) utilruntime.Must(velerov1api.AddToScheme(scheme)) - //+kubebuilder:scaffold:scheme + // +kubebuilder:scaffold:scheme } func main() { @@ -133,7 +131,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "NonAdminBackup") os.Exit(1) } - //+kubebuilder:scaffold:builder + // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") diff --git a/internal/common/constant/constant.go b/internal/common/constant/constant.go new file mode 100644 index 0000000..6d44ee8 --- /dev/null +++ b/internal/common/constant/constant.go @@ -0,0 +1,31 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package constant contains all common constants used in the project +package constant + +// Common labels for objects manipulated by the Non Admin Controller +// Labels should be used to identify the NAC backup +// Annotations on the other hand should be used to define ownership +// of the specific Object, such as Backup. +const ( + OadpLabel = "openshift.io/oadp" // TODO import? + ManagedByLabel = "app.kubernetes.io/managed-by" + ManagedByLabelValue = "oadp-nac-controller" // TODO why not use same project name as in PROJECT file? + NabOriginNameAnnotation = "openshift.io/oadp-nab-origin-name" + NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace" + NabOriginUUIDAnnotation = "openshift.io/oadp-nab-origin-uuid" +) diff --git a/internal/common/function/function.go b/internal/common/function/function.go new file mode 100644 index 0000000..f6a9c0e --- /dev/null +++ b/internal/common/function/function.go @@ -0,0 +1,224 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package function contains all common functions used in the project +package function + +import ( + "context" + "crypto/sha1" //nolint:gosec // TODO remove + "encoding/hex" + "fmt" + "reflect" + + "github.com/go-logr/logr" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + "github.com/migtools/oadp-non-admin/internal/common/constant" +) + +const requiredAnnotationError = "backup does not have the required annotation '%s'" + +// AddNonAdminLabels return a map with both the object labels and with the default Non Admin labels. +// If error occurs, a map with only the default Non Admin labels is returned +func AddNonAdminLabels(labels map[string]string) map[string]string { + defaultLabels := map[string]string{ + constant.OadpLabel: "True", + constant.ManagedByLabel: constant.ManagedByLabelValue, + } + + mergedLabels, err := mergeMaps(defaultLabels, labels) + if err != nil { + // TODO logger + _, _ = fmt.Println("Error merging labels:", err) + // TODO break? + return defaultLabels + } + return mergedLabels +} + +// AddNonAdminBackupAnnotations return a map with both the object annotations and with the default NonAdminBackup annotations. +// If error occurs, a map with only the default NonAdminBackup annotations is returned +func AddNonAdminBackupAnnotations(ownerNamespace string, ownerName string, ownerUUID string, existingAnnotations map[string]string) map[string]string { + // TODO could not receive object meta and get info from there? + defaultAnnotations := map[string]string{ + constant.NabOriginNamespaceAnnotation: ownerNamespace, + constant.NabOriginNameAnnotation: ownerName, + constant.NabOriginUUIDAnnotation: ownerUUID, + } + + mergedAnnotations, err := mergeMaps(defaultAnnotations, existingAnnotations) + if err != nil { + // TODO logger + _, _ = fmt.Println("Error merging annotations:", err) + // TODO break? + return defaultAnnotations + } + return mergedAnnotations +} + +// GetBackupSpecFromNonAdminBackup return BackupSpec object from NonAdminBackup spec, if no error occurs +func GetBackupSpecFromNonAdminBackup(nonAdminBackup *nacv1alpha1.NonAdminBackup) (*velerov1api.BackupSpec, error) { + if nonAdminBackup == nil { + return nil, fmt.Errorf("nonAdminBackup is nil") + } + + // TODO check spec? + + if nonAdminBackup.Spec.BackupSpec == nil { + return nil, fmt.Errorf("BackupSpec is nil") + } + + // TODO: Additional validations, before continuing + + return nonAdminBackup.Spec.BackupSpec.DeepCopy(), nil +} + +// GenerateVeleroBackupName return generated name for Velero Backup object created from NonAdminBackup +func GenerateVeleroBackupName(namespace, nabName string) string { + // Calculate a hash of the name + hasher := sha1.New() //nolint:gosec // TODO use another tool + _, _ = hasher.Write([]byte(nabName)) + const nameLength = 14 + nameHash := hex.EncodeToString(hasher.Sum(nil))[:nameLength] // Take first 14 chars + + // Generate the Velero backup name created from NAB + veleroBackupName := fmt.Sprintf("nab-%s-%s", namespace, nameHash) + + const characterLimit = 253 + const occupiedSize = 4 + // Ensure the name is within the character limit + if len(veleroBackupName) > characterLimit { + // Truncate the namespace if necessary + maxNamespaceLength := characterLimit - len(nameHash) - occupiedSize // Account for "nab-" and "-" TODO should not be 5? + if len(namespace) > maxNamespaceLength { + namespace = namespace[:maxNamespaceLength] + } + veleroBackupName = fmt.Sprintf("nab-%s-%s", namespace, nameHash) + } + + return veleroBackupName +} + +// UpdateNonAdminBackupFromVeleroBackup update, if necessary, NonAdminBackup object fields related to referenced Velero Backup object, if no error occurs +func UpdateNonAdminBackupFromVeleroBackup(ctx context.Context, r client.Client, logger logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup *velerov1api.Backup) error { + // Make a copy of the current status for comparison + oldStatus := nab.Spec.BackupStatus.DeepCopy() + oldSpec := nab.Spec.BackupSpec.DeepCopy() + + // Update the status & spec + nab.Spec.BackupStatus = &veleroBackup.Status + nab.Spec.BackupSpec = &veleroBackup.Spec + + if reflect.DeepEqual(oldStatus, nab.Spec.BackupStatus) && reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) { + // No change, no need to update + logger.V(1).Info("NonAdminBackup status and spec is already up to date") + return nil + } + + if err := r.Update(ctx, nab); err != nil { + logger.Error(err, "Failed to update NonAdminBackup") + return err + } + + return nil +} + +// CheckVeleroBackupLabels return true if Velero Backup object has required Non Admin labels, false otherwise +func CheckVeleroBackupLabels(backup *velerov1api.Backup) bool { + // TODO also need to check for constant.OadpLabel label? + labels := backup.GetLabels() + value, exists := labels[constant.ManagedByLabel] + return exists && value == constant.ManagedByLabelValue +} + +// TODO not used + +// GetNonAdminBackupFromVeleroBackup return referenced NonAdminBackup object from Velero Backup object, if no error occurs +func GetNonAdminBackupFromVeleroBackup(ctx context.Context, clientInstance client.Client, backup *velerov1api.Backup) (*nacv1alpha1.NonAdminBackup, error) { + // Check if the backup has the required annotations to identify the associated NonAdminBackup object + logger := log.FromContext(ctx) + + annotations := backup.GetAnnotations() + + annotationsStr := fmt.Sprintf("%v", annotations) + logger.V(1).Info("Velero Backup Annotations", "annotations", annotationsStr) + + if annotations == nil { + return nil, fmt.Errorf("backup has no annotations") + } + + nabOriginNamespace, ok := annotations[constant.NabOriginNamespaceAnnotation] + if !ok { + return nil, fmt.Errorf(requiredAnnotationError, constant.NabOriginNamespaceAnnotation) + } + + nabOriginName, ok := annotations[constant.NabOriginNameAnnotation] + if !ok { + return nil, fmt.Errorf(requiredAnnotationError, constant.NabOriginNameAnnotation) + } + + nonAdminBackupKey := types.NamespacedName{ + Namespace: nabOriginNamespace, + Name: nabOriginName, + } + + nonAdminBackup := &nacv1alpha1.NonAdminBackup{} + err := clientInstance.Get(ctx, nonAdminBackupKey, nonAdminBackup) + if err != nil { + return nil, fmt.Errorf("failed to fetch NonAdminBackup object: %v", err) + } + + nabOriginUUID, ok := annotations[constant.NabOriginUUIDAnnotation] + if !ok { + return nil, fmt.Errorf(requiredAnnotationError, constant.NabOriginUUIDAnnotation) + } + // Ensure UID matches + if nonAdminBackup.ObjectMeta.UID != types.UID(nabOriginUUID) { + return nil, fmt.Errorf("UID from annotation does not match UID of fetched NonAdminBackup object") + } + + return nonAdminBackup, nil +} + +// TODO import? Similar to as pkg/common/common.go:AppendUniqueKeyTOfTMaps from github.com/openshift/oadp-operator + +// Return map, of the same type as the input maps, that contains all keys/values from all input maps. +// Key/value pairs that are identical in different input maps, are added only once to return map. +// If a key exists in more than one input map, with a different value, an error is returned +func mergeMaps[T comparable](maps ...map[T]T) (map[T]T, error) { + merge := make(map[T]T) + for _, m := range maps { + if m == nil { + continue + } + for k, v := range m { + existingValue, found := merge[k] + if found { + if existingValue != v { + return nil, fmt.Errorf("conflicting key %v: has both value %v and value %v in input maps", k, v, existingValue) + } + continue + } + merge[k] = v + } + } + return merge, nil +} diff --git a/internal/common/function/function_test.go b/internal/common/function/function_test.go new file mode 100644 index 0000000..281923f --- /dev/null +++ b/internal/common/function/function_test.go @@ -0,0 +1,308 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package function + +import ( + "context" + "fmt" + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + "github.com/migtools/oadp-non-admin/internal/common/constant" +) + +func TestMergeMaps(t *testing.T) { + const ( + d = "d" + delta = "delta" + ) + tests := []struct { + name string + want map[string]string + args []map[string]string + wantErr bool + }{ + { + name: "append unique labels together", + args: []map[string]string{ + {"a": "alpha"}, + {"b": "beta"}, + }, + want: map[string]string{ + "a": "alpha", + "b": "beta", + }, + }, + { + name: "append unique labels together, with valid duplicates", + args: []map[string]string{ + {"c": "gamma"}, + {d: delta}, + {d: delta}, + }, + want: map[string]string{ + "c": "gamma", + d: delta, + }, + }, + { + name: "append unique labels together - nil sandwich", + args: []map[string]string{ + {"x": "chi"}, + nil, + {"y": "psi"}, + }, + want: map[string]string{ + "x": "chi", + "y": "psi", + }, + }, + { + name: "should error when append duplicate label keys with different value together", + args: []map[string]string{ + {"key": "value-1"}, + {"key": "value-2"}, + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := mergeMaps(tt.args...) + if (err != nil) != tt.wantErr { + t.Errorf("mergeMaps() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("mergeMaps() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestAddNonAdminLabels(t *testing.T) { + // Additional labels provided + additionalLabels := map[string]string{ + "additionalLabel1": "value1", + "additionalLabel2": "value2", + } + + expectedLabels := map[string]string{ + constant.OadpLabel: "True", + constant.ManagedByLabel: constant.ManagedByLabelValue, + "additionalLabel1": "value1", + "additionalLabel2": "value2", + } + + mergedLabels := AddNonAdminLabels(additionalLabels) + assert.Equal(t, expectedLabels, mergedLabels, "Merged labels should match expected labels") +} + +func TestAddNonAdminBackupAnnotations(t *testing.T) { + // Merging annotations without conflicts + existingAnnotations := map[string]string{ + "existingKey1": "existingValue1", + "existingKey2": "existingValue2", + } + + ownerName := "testOwner" + ownerNamespace := "testNamespace" + ownerUUID := "f2c4d2c3-58d3-46ec-bf03-5940f567f7f8" + + expectedAnnotations := map[string]string{ + constant.NabOriginNamespaceAnnotation: ownerNamespace, + constant.NabOriginNameAnnotation: ownerName, + constant.NabOriginUUIDAnnotation: ownerUUID, + "existingKey1": "existingValue1", + "existingKey2": "existingValue2", + } + + mergedAnnotations := AddNonAdminBackupAnnotations(ownerNamespace, ownerName, ownerUUID, existingAnnotations) + assert.Equal(t, expectedAnnotations, mergedAnnotations, "Merged annotations should match expected annotations") + + // Merging annotations with conflicts + existingAnnotationsWithConflict := map[string]string{ + constant.NabOriginNameAnnotation: "conflictingValue", + } + + expectedAnnotationsWithConflict := map[string]string{ + constant.NabOriginNameAnnotation: ownerName, + constant.NabOriginNamespaceAnnotation: ownerNamespace, + constant.NabOriginUUIDAnnotation: ownerUUID, + } + + mergedAnnotationsWithConflict := AddNonAdminBackupAnnotations(ownerNamespace, ownerName, ownerUUID, existingAnnotationsWithConflict) + assert.Equal(t, expectedAnnotationsWithConflict, mergedAnnotationsWithConflict, "Merged annotations should match expected annotations with conflict") +} + +func TestGetBackupSpecFromNonAdminBackup(t *testing.T) { + // Test case: nonAdminBackup is nil + nonAdminBackup := (*nacv1alpha1.NonAdminBackup)(nil) + backupSpec, err := GetBackupSpecFromNonAdminBackup(nonAdminBackup) + assert.Error(t, err) + assert.Nil(t, backupSpec) + assert.Equal(t, "nonAdminBackup is nil", err.Error()) + + // Test case: BackupSpec is nil + nonAdminBackup = &nacv1alpha1.NonAdminBackup{ + Spec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: nil, + }, + } + backupSpec, err = GetBackupSpecFromNonAdminBackup(nonAdminBackup) + assert.Error(t, err) + assert.Nil(t, backupSpec) + assert.Equal(t, "BackupSpec is nil", err.Error()) + + // Test case: NonAdminBackup with valid BackupSpec + backupSpecInput := &velerov1api.BackupSpec{ + IncludedNamespaces: []string{"namespace1", "namespace2"}, + ExcludedNamespaces: []string{"namespace3"}, + StorageLocation: "s3://bucket-name/path/to/backup", + VolumeSnapshotLocations: []string{"volume-snapshot-location"}, + } + + nonAdminBackup = &nacv1alpha1.NonAdminBackup{ + Spec: nacv1alpha1.NonAdminBackupSpec{ + BackupSpec: backupSpecInput, + }, + } + backupSpec, err = GetBackupSpecFromNonAdminBackup(nonAdminBackup) + + assert.NoError(t, err) + assert.NotNil(t, backupSpec) + assert.Equal(t, backupSpecInput, backupSpec) +} + +func TestGenerateVeleroBackupName(t *testing.T) { + longString := "" + const longStringSize = 240 + for i := 0; i < longStringSize; i++ { + longString += "m" + } + + truncatedString := "" + // 253 - len(nameHash) - 4 + // 253 - 14 - 4 = 235 + const truncatedStringSize = 235 + for i := 0; i < truncatedStringSize; i++ { + truncatedString += "m" + } + + testCases := []struct { + namespace string + name string + expected string + }{ + { + namespace: "example-namespace", + name: "example-name", + expected: "nab-example-namespace-daf3757ac468f9", + }, + { + namespace: longString, + name: "example-name", + expected: fmt.Sprintf("nab-%s-daf3757ac468f9", truncatedString), + }, + } + + for _, tc := range testCases { + actual := GenerateVeleroBackupName(tc.namespace, tc.name) + if actual != tc.expected { + t.Errorf("Expected: %s, but got: %s", tc.expected, actual) + } + } +} + +func TestGetNonAdminBackupFromVeleroBackup(t *testing.T) { + log := zap.New(zap.UseDevMode(true)) + ctx := context.Background() + ctx = ctrl.LoggerInto(ctx, log) + + // Register NonAdminBackup type with the scheme + if err := nacv1alpha1.AddToScheme(clientgoscheme.Scheme); err != nil { + t.Fatalf("Failed to register NonAdminBackup type: %v", err) + } + + backup := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-namespace", + Name: "test-backup", + Annotations: map[string]string{ + constant.NabOriginNamespaceAnnotation: "non-admin-backup-namespace", + constant.NabOriginNameAnnotation: "non-admin-backup-name", + constant.NabOriginUUIDAnnotation: "12345678-1234-1234-1234-123456789abc", + }, + }, + } + + nonAdminBackup := &nacv1alpha1.NonAdminBackup{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "non-admin-backup-namespace", + Name: "non-admin-backup-name", + UID: types.UID("12345678-1234-1234-1234-123456789abc"), + }, + } + + client := fake.NewClientBuilder().WithObjects(nonAdminBackup).Build() + + result, err := GetNonAdminBackupFromVeleroBackup(ctx, client, backup) + assert.NoError(t, err, "GetNonAdminFromBackup should not return an error") + assert.NotNil(t, result, "Returned NonAdminBackup should not be nil") + assert.Equal(t, nonAdminBackup, result, "Returned NonAdminBackup should match expected NonAdminBackup") +} + +func TestCheckVeleroBackupLabels(t *testing.T) { + // Backup has the required label + backupWithLabel := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + constant.ManagedByLabel: constant.ManagedByLabelValue, + }, + }, + } + assert.True(t, CheckVeleroBackupLabels(backupWithLabel), "Expected backup to have required label") + + // Backup does not have the required label + backupWithoutLabel := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + } + assert.False(t, CheckVeleroBackupLabels(backupWithoutLabel), "Expected backup to not have required label") + + // Backup has the required label with incorrect value + backupWithIncorrectValue := &velerov1api.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + constant.ManagedByLabel: "incorrect-value", + }, + }, + } + assert.False(t, CheckVeleroBackupLabels(backupWithIncorrectValue), "Expected backup to not have required label") +} diff --git a/internal/controller/common_k8s.go b/internal/controller/common_k8s.go deleted file mode 100644 index 9d9ad40..0000000 --- a/internal/controller/common_k8s.go +++ /dev/null @@ -1,75 +0,0 @@ -package controller - -import ( - "fmt" -) - -// Common labels for objects manipulated by the Non Admin Controller -// Labels should be used to identify the NAC backup -// Annotations on the other hand should be used to define ownership -// of the specific Object, such as Backup. -const ( - OadpLabel = "openshift.io/oadp" - ManagedByLabel = "app.kubernetes.io/managed-by" - ManagedByLabelValue = "oadp-nac-controller" - NabOriginNameAnnotation = "openshift.io/oadp-nab-origin-name" - NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace" - NabOriginUuidAnnotation = "openshift.io/oadp-nab-origin-uuid" -) - -const ( - OadpNamespace = "openshift-adp" -) - -func CreateLabelsForNac(labels map[string]string) map[string]string { - defaultLabels := map[string]string{ - OadpLabel: "True", - ManagedByLabel: ManagedByLabelValue, - } - - mergedLabels, err := mergeUniqueKeyTOfTMaps(defaultLabels, labels) - if err != nil { - fmt.Println("Error merging labels:", err) - return defaultLabels - } - return mergedLabels -} - -func CreateAnnotationsForNac(ownerNamespace string, ownerName string, ownerUuid string, existingAnnotations map[string]string) map[string]string { - defaultAnnotations := map[string]string{ - NabOriginNamespaceAnnotation: ownerNamespace, - NabOriginNameAnnotation: ownerName, - NabOriginUuidAnnotation: ownerUuid, - } - - mergedAnnotations, err := mergeUniqueKeyTOfTMaps(defaultAnnotations, existingAnnotations) - if err != nil { - fmt.Println("Error merging annotations:", err) - return defaultAnnotations - } - return mergedAnnotations -} - -// Similar to as pkg/common/common.go:AppendUniqueKeyTOfTMaps from github.com/openshift/oadp-operator -func mergeUniqueKeyTOfTMaps[T comparable](userMap ...map[T]T) (map[T]T, error) { - var base map[T]T - for i, mapElements := range userMap { - if mapElements == nil { - continue - } - if base == nil { - base = make(map[T]T) - } - for k, v := range mapElements { - existingValue, found := base[k] - if found { - if existingValue != v { - return nil, fmt.Errorf("conflicting key %v with value %v in map %d may not override %v", k, v, i, existingValue) - } - } else { - base[k] = v - } - } - } - return base, nil -} diff --git a/internal/controller/common_k8s_test.go b/internal/controller/common_k8s_test.go deleted file mode 100644 index 9f98817..0000000 --- a/internal/controller/common_k8s_test.go +++ /dev/null @@ -1,140 +0,0 @@ -package controller - -import ( - "reflect" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestMergeUniqueKeyTOfTMaps(t *testing.T) { - type args struct { - userLabels []map[string]string - } - tests := []struct { - name string - args args - want map[string]string - wantErr bool - }{ - { - name: "append unique labels together", - args: args{ - userLabels: []map[string]string{ - {"a": "a"}, - {"b": "b"}, - }, - }, - want: map[string]string{ - "a": "a", - "b": "b", - }, - }, - { - name: "append unique labels together, with valid duplicates", - args: args{ - userLabels: []map[string]string{ - {"a": "a"}, - {"b": "b"}, - {"b": "b"}, - }, - }, - want: map[string]string{ - "a": "a", - "b": "b", - }, - }, - { - name: "append unique labels together - nil sandwich", - args: args{ - userLabels: []map[string]string{ - {"a": "a"}, - nil, - {"b": "b"}, - }, - }, - want: map[string]string{ - "a": "a", - "b": "b", - }, - }, - { - name: "should error when append duplicate label keys with different value together", - args: args{ - userLabels: []map[string]string{ - {"a": "a"}, - {"a": "b"}, - }, - }, - want: nil, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := mergeUniqueKeyTOfTMaps(tt.args.userLabels...) - if (err != nil) != tt.wantErr { - t.Errorf("mergeUniqueKeyTOfTMaps() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("mergeUniqueKeyTOfTMaps() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestCreateLabelsForNac(t *testing.T) { - // Additional labels provided - additionalLabels := map[string]string{ - "additionalLabel1": "value1", - "additionalLabel2": "value2", - } - - expectedLabels := map[string]string{ - OadpLabel: "True", - ManagedByLabel: "oadp-nac-controller", - "additionalLabel1": "value1", - "additionalLabel2": "value2", - } - - mergedLabels := CreateLabelsForNac(additionalLabels) - assert.Equal(t, expectedLabels, mergedLabels, "Merged labels should match expected labels") -} - -func TestCreateAnnotationsForNac(t *testing.T) { - // Merging annotations without conflicts - existingAnnotations := map[string]string{ - "existingKey1": "existingValue1", - "existingKey2": "existingValue2", - } - - ownerName := "testOwner" - ownerNamespace := "testNamespace" - ownerUuid := "f2c4d2c3-58d3-46ec-bf03-5940f567f7f8" - - expectedAnnotations := map[string]string{ - NabOriginNamespaceAnnotation: ownerNamespace, - NabOriginNameAnnotation: ownerName, - NabOriginUuidAnnotation: ownerUuid, - "existingKey1": "existingValue1", - "existingKey2": "existingValue2", - } - - mergedAnnotations := CreateAnnotationsForNac(ownerNamespace, ownerName, ownerUuid, existingAnnotations) - assert.Equal(t, expectedAnnotations, mergedAnnotations, "Merged annotations should match expected annotations") - - // Merging annotations with conflicts - existingAnnotationsWithConflict := map[string]string{ - NabOriginNameAnnotation: "conflictingValue", - } - - expectedAnnotationsWithConflict := map[string]string{ - NabOriginNameAnnotation: ownerName, - NabOriginNamespaceAnnotation: ownerNamespace, - NabOriginUuidAnnotation: ownerUuid, - } - - mergedAnnotationsWithConflict := CreateAnnotationsForNac(ownerNamespace, ownerName, ownerUuid, existingAnnotationsWithConflict) - assert.Equal(t, expectedAnnotationsWithConflict, mergedAnnotationsWithConflict, "Merged annotations should match expected annotations with conflict") -} diff --git a/internal/controller/common_nab.go b/internal/controller/common_nab.go deleted file mode 100644 index d28b23c..0000000 --- a/internal/controller/common_nab.go +++ /dev/null @@ -1,77 +0,0 @@ -package controller - -import ( - "context" - "crypto/sha1" - "encoding/hex" - "fmt" - "reflect" - - "github.com/go-logr/logr" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" -) - -func GetVeleroBackupSpecFromNonAdminBackup(nonAdminBackup *nacv1alpha1.NonAdminBackup) (*velerov1api.BackupSpec, error) { - - if nonAdminBackup == nil { - return nil, fmt.Errorf("nonAdminBackup is nil") - } - - if nonAdminBackup.Spec.BackupSpec == nil { - return nil, fmt.Errorf("BackupSpec is nil") - } - - // TODO: Additional validations, before continuing - - veleroBackup := nonAdminBackup.Spec.BackupSpec.DeepCopy() - - return veleroBackup, nil -} - -func GenerateVeleroBackupName(namespace, nabName string) string { - // Calculate a hash of the name - hasher := sha1.New() - hasher.Write([]byte(nabName)) - nameHash := hex.EncodeToString(hasher.Sum(nil))[:14] // Take first 14 chars - - // Generate the Velero backup name created from NAB - veleroBackupName := fmt.Sprintf("nab-%s-%s", namespace, nameHash) - - // Ensure the name is within the character limit - if len(veleroBackupName) > 253 { - // Truncate the namespace if necessary - maxNamespaceLength := 253 - len(nameHash) - 4 // Account for "-nab-" and "-" - if len(namespace) > maxNamespaceLength { - namespace = namespace[:maxNamespaceLength] - } - veleroBackupName = fmt.Sprintf("nab-%s-%s", namespace, nameHash) - } - - return veleroBackupName -} - -func UpdateNonAdminBackupFromVeleroBackup(ctx context.Context, r client.Client, log logr.Logger, nab *nacv1alpha1.NonAdminBackup, veleroBackup *velerov1api.Backup) error { - // Make a copy of the current status for comparison - oldStatus := nab.Spec.BackupStatus.DeepCopy() - oldSpec := nab.Spec.BackupSpec.DeepCopy() - - // Update the status & spec - nab.Spec.BackupStatus = &veleroBackup.Status - nab.Spec.BackupSpec = &veleroBackup.Spec - - if reflect.DeepEqual(oldStatus, nab.Spec.BackupStatus) && reflect.DeepEqual(oldSpec, nab.Spec.BackupSpec) { - // No change, no need to update - log.V(1).Info("NonAdminBackup status and spec is already up to date") - return nil - } - - if err := r.Update(ctx, nab); err != nil { - log.Error(err, "Failed to update NonAdminBackup") - return err - } - - return nil -} diff --git a/internal/controller/common_nab_test.go b/internal/controller/common_nab_test.go deleted file mode 100644 index ee1282d..0000000 --- a/internal/controller/common_nab_test.go +++ /dev/null @@ -1,87 +0,0 @@ -package controller - -import ( - "fmt" - "testing" - - nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" - "github.com/stretchr/testify/assert" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" -) - -func TestGetVeleroBackupSpecFromNonAdminBackup(t *testing.T) { - // Test case: nonAdminBackup is nil - nonAdminBackup := (*nacv1alpha1.NonAdminBackup)(nil) - backupSpec, err := GetVeleroBackupSpecFromNonAdminBackup(nonAdminBackup) - assert.Error(t, err) - assert.Nil(t, backupSpec) - assert.Equal(t, "nonAdminBackup is nil", err.Error()) - - // Test case: BackupSpec is nil - nonAdminBackup = &nacv1alpha1.NonAdminBackup{ - Spec: nacv1alpha1.NonAdminBackupSpec{ - BackupSpec: nil, - }, - } - backupSpec, err = GetVeleroBackupSpecFromNonAdminBackup(nonAdminBackup) - assert.Error(t, err) - assert.Nil(t, backupSpec) - assert.Equal(t, "BackupSpec is nil", err.Error()) - - // Test case: NonAdminBackup with valid BackupSpec - backupSpecInput := &velerov1api.BackupSpec{ - IncludedNamespaces: []string{"namespace1", "namespace2"}, - ExcludedNamespaces: []string{"namespace3"}, - StorageLocation: "s3://bucket-name/path/to/backup", - VolumeSnapshotLocations: []string{"volume-snapshot-location"}, - } - - nonAdminBackup = &nacv1alpha1.NonAdminBackup{ - Spec: nacv1alpha1.NonAdminBackupSpec{ - BackupSpec: backupSpecInput, - }, - } - backupSpec, err = GetVeleroBackupSpecFromNonAdminBackup(nonAdminBackup) - - assert.NoError(t, err) - assert.NotNil(t, backupSpec) - assert.Equal(t, backupSpecInput, backupSpec) -} - -func TestGenerateVeleroBackupName(t *testing.T) { - longString := "" - for i := 0; i < 240; i++ { - longString += "a" - } - - truncatedString := "" - // 253 - len(nameHash) - 4 - // 253 - 14 - 4 = 235 - for i := 0; i < 235; i++ { - truncatedString += "a" - } - - testCases := []struct { - namespace string - name string - expected string - }{ - { - namespace: "example-namespace", - name: "example-name", - expected: "nab-example-namespace-daf3757ac468f9", - }, - { - namespace: longString, - name: "example-name", - expected: fmt.Sprintf("nab-%s-daf3757ac468f9", truncatedString), - }, - } - - for _, tc := range testCases { - actual := GenerateVeleroBackupName(tc.namespace, tc.name) - if actual != tc.expected { - t.Errorf("Expected: %s, but got: %s", tc.expected, actual) - } - } -} diff --git a/internal/controller/common_velero.go b/internal/controller/common_velero.go deleted file mode 100644 index 38f1cce..0000000 --- a/internal/controller/common_velero.go +++ /dev/null @@ -1,65 +0,0 @@ -package controller - -import ( - "context" - "fmt" - - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - - nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" -) - -func HasRequiredLabel(backup *velerov1api.Backup) bool { - labels := backup.GetLabels() - value, exists := labels[ManagedByLabel] - return exists && value == ManagedByLabelValue -} - -func GetNonAdminFromBackup(ctx context.Context, client client.Client, backup *velerov1api.Backup) (*nacv1alpha1.NonAdminBackup, error) { - // Check if the backup has the required annotations to identify the associated NonAdminBackup object - log := log.FromContext(ctx) - - annotations := backup.GetAnnotations() - - annotationsStr := fmt.Sprintf("%v", annotations) - log.V(1).Info("Velero Backup Annotations", "annotations", annotationsStr) - - if annotations == nil { - return nil, fmt.Errorf("backup has no annotations") - } - - nabOriginNamespace, ok := annotations[NabOriginNamespaceAnnotation] - if !ok { - return nil, fmt.Errorf("backup does not have the required annotation '%s'", NabOriginNamespaceAnnotation) - } - - nabOriginName, ok := annotations[NabOriginNameAnnotation] - if !ok { - return nil, fmt.Errorf("backup does not have the required annotation '%s'", NabOriginNameAnnotation) - } - - nonAdminBackupKey := types.NamespacedName{ - Namespace: nabOriginNamespace, - Name: nabOriginName, - } - - nonAdminBackup := &nacv1alpha1.NonAdminBackup{} - err := client.Get(ctx, nonAdminBackupKey, nonAdminBackup) - if err != nil { - return nil, fmt.Errorf("failed to fetch NonAdminBackup object: %v", err) - } - - nabOriginUuid, ok := annotations[NabOriginUuidAnnotation] - if !ok { - return nil, fmt.Errorf("backup does not have the required annotation '%s'", NabOriginUuidAnnotation) - } - // Ensure UID matches - if nonAdminBackup.ObjectMeta.UID != types.UID(nabOriginUuid) { - return nil, fmt.Errorf("UID from annotation does not match UID of fetched NonAdminBackup object") - } - - return nonAdminBackup, nil -} diff --git a/internal/controller/common_velero_test.go b/internal/controller/common_velero_test.go deleted file mode 100644 index 9c63825..0000000 --- a/internal/controller/common_velero_test.go +++ /dev/null @@ -1,86 +0,0 @@ -package controller - -import ( - "context" - "testing" - - "github.com/stretchr/testify/assert" - "k8s.io/apimachinery/pkg/types" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/log/zap" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" -) - -func TestGetNonAdminFromBackup(t *testing.T) { - log := zap.New(zap.UseDevMode(true)) - ctx := context.Background() - ctx = ctrl.LoggerInto(ctx, log) - - // Register NonAdminBackup type with the scheme - if err := nacv1alpha1.AddToScheme(clientgoscheme.Scheme); err != nil { - t.Fatalf("Failed to register NonAdminBackup type: %v", err) - } - - backup := &velerov1api.Backup{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test-namespace", - Name: "test-backup", - Annotations: map[string]string{ - NabOriginNamespaceAnnotation: "non-admin-backup-namespace", - NabOriginNameAnnotation: "non-admin-backup-name", - NabOriginUuidAnnotation: "12345678-1234-1234-1234-123456789abc", - }, - }, - } - - nonAdminBackup := &nacv1alpha1.NonAdminBackup{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "non-admin-backup-namespace", - Name: "non-admin-backup-name", - UID: types.UID("12345678-1234-1234-1234-123456789abc"), - }, - } - - client := fake.NewClientBuilder().WithObjects(nonAdminBackup).Build() - - result, err := GetNonAdminFromBackup(ctx, client, backup) - assert.NoError(t, err, "GetNonAdminFromBackup should not return an error") - assert.NotNil(t, result, "Returned NonAdminBackup should not be nil") - assert.Equal(t, nonAdminBackup, result, "Returned NonAdminBackup should match expected NonAdminBackup") -} - -func TestHasRequiredLabel(t *testing.T) { - // Backup has the required label - backupWithLabel := &velerov1api.Backup{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - ManagedByLabel: ManagedByLabelValue, - }, - }, - } - assert.True(t, HasRequiredLabel(backupWithLabel), "Expected backup to have required label") - - // Backup does not have the required label - backupWithoutLabel := &velerov1api.Backup{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{}, - }, - } - assert.False(t, HasRequiredLabel(backupWithoutLabel), "Expected backup to not have required label") - - // Backup has the required label with incorrect value - backupWithIncorrectValue := &velerov1api.Backup{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - ManagedByLabel: "incorrect-value", - }, - }, - } - assert.False(t, HasRequiredLabel(backupWithIncorrectValue), "Expected backup to not have required label") -} diff --git a/internal/controller/nonadminbackup_controller.go b/internal/controller/nonadminbackup_controller.go index ca159b7..5f4281d 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -14,24 +14,27 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Package controller contains all controllers of the project package controller import ( "context" "github.com/go-logr/logr" + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "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/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/migtools/oadp-non-admin/internal/common/function" + "github.com/migtools/oadp-non-admin/internal/handler" + "github.com/migtools/oadp-non-admin/internal/predicate" ) // NonAdminBackupReconciler reconciles a NonAdminBackup object @@ -43,11 +46,16 @@ type NonAdminBackupReconciler struct { NamespacedName types.NamespacedName } -//+kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/finalizers,verbs=update +const ( + nameField = "Name" + oadpNamespace = "openshift-adp" // TODO user input +) + +// +kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=nac.oadp.openshift.io,resources=nonadminbackups/finalizers,verbs=update -//+kubebuilder:rbac:groups=velero.io,resources=backups,verbs=get;list;watch;create;update;patch +// +kubebuilder:rbac:groups=velero.io,resources=backups,verbs=get;list;watch;create;update;patch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -60,7 +68,7 @@ type NonAdminBackupReconciler struct { // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.17.0/pkg/reconcile func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { r.Log = log.FromContext(ctx) - log := r.Log.WithValues("NonAdminBackup", req.NamespacedName) + logger := r.Log.WithValues("NonAdminBackup", req.NamespacedName) r.Context = ctx r.NamespacedName = req.NamespacedName @@ -69,77 +77,76 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque err := r.Get(ctx, req.NamespacedName, &nab) if err != nil && errors.IsNotFound(err) { - log.V(1).Info("Deleted NonAdminBackup CR", "Name", req.Name, "Namespace", req.Namespace) + logger.V(1).Info("Deleted NonAdminBackup CR", nameField, req.Name, "Namespace", req.Namespace) return ctrl.Result{}, nil } if err != nil { - log.Error(err, "Unable to fetch NonAdminBackup CR", "Name", req.Name, "Namespace", req.Namespace) + logger.Error(err, "Unable to fetch NonAdminBackup CR", nameField, req.Name, "Namespace", req.Namespace) return ctrl.Result{}, err } - veleroBackupSpec, err := GetVeleroBackupSpecFromNonAdminBackup(&nab) + backupSpec, err := function.GetBackupSpecFromNonAdminBackup(&nab) - if veleroBackupSpec == nil { - log.Error(err, "NonAdminBackup CR does not contain valid VeleroBackupSpec") + if backupSpec == nil { + logger.Error(err, "NonAdminBackup CR does not contain valid BackupSpec") return ctrl.Result{}, nil } if err != nil { - log.Error(err, "Error while performing NonAdminBackup reconcile") + logger.Error(err, "Error while performing NonAdminBackup reconcile") return ctrl.Result{}, err } - log.Info("NonAdminBackup Reconcile loop") + logger.Info("NonAdminBackup Reconcile loop") - veleroBackupName := GenerateVeleroBackupName(nab.Namespace, nab.Name) + veleroBackupName := function.GenerateVeleroBackupName(nab.Namespace, nab.Name) veleroBackup := velerov1api.Backup{} - err = r.Get(ctx, client.ObjectKey{Namespace: OadpNamespace, Name: veleroBackupName}, &veleroBackup) + err = r.Get(ctx, client.ObjectKey{Namespace: oadpNamespace, Name: veleroBackupName}, &veleroBackup) if err != nil && errors.IsNotFound(err) { // Create backup - log.Info("No backup found", "Name", veleroBackupName) + logger.Info("No backup found", nameField, veleroBackupName) veleroBackup = velerov1api.Backup{ ObjectMeta: metav1.ObjectMeta{ Name: veleroBackupName, - Namespace: OadpNamespace, + Namespace: oadpNamespace, }, - Spec: *veleroBackupSpec, + Spec: *backupSpec, } } else if err != nil && !errors.IsNotFound(err) { - log.Error(err, "Unable to fetch VeleroBackup") + logger.Error(err, "Unable to fetch VeleroBackup") return ctrl.Result{}, err } else { - log.Info("Backup already exists, updating NonAdminBackup status", "Name", veleroBackupName) - err := UpdateNonAdminBackupFromVeleroBackup(ctx, r.Client, log, &nab, &veleroBackup) + logger.Info("Backup already exists, updating NonAdminBackup status", nameField, veleroBackupName) + err = function.UpdateNonAdminBackupFromVeleroBackup(ctx, r.Client, logger, &nab, &veleroBackup) if err != nil { return ctrl.Result{}, err } return ctrl.Result{}, nil } - // Ensure labels for the BackupSpec are merged - // with the existing NAB labels + // Ensure labels are set for the Backup object existingLabels := veleroBackup.Labels - nacManagedLabels := CreateLabelsForNac(existingLabels) - veleroBackup.Labels = nacManagedLabels + naManagedLabels := function.AddNonAdminLabels(existingLabels) + veleroBackup.Labels = naManagedLabels // Ensure annotations are set for the Backup object existingAnnotations := veleroBackup.Annotations ownerUUID := string(nab.ObjectMeta.UID) - nacManagedAnnotations := CreateAnnotationsForNac(nab.Namespace, nab.Name, ownerUUID, existingAnnotations) - veleroBackup.Annotations = nacManagedAnnotations + nabManagedAnnotations := function.AddNonAdminBackupAnnotations(nab.Namespace, nab.Name, ownerUUID, existingAnnotations) + veleroBackup.Annotations = nabManagedAnnotations _, err = controllerutil.CreateOrPatch(ctx, r.Client, &veleroBackup, nil) if err != nil { - log.Error(err, "Failed to create backup", "Name", veleroBackupName) + logger.Error(err, "Failed to create backup", nameField, veleroBackupName) return ctrl.Result{}, err - } else { - log.Info("Backup successfully created", "Name", veleroBackupName) } - log.Info("NonAdminBackup Reconcile loop end") + logger.Info("Backup successfully created", nameField, veleroBackupName) + + logger.Info("NonAdminBackup Reconcile loop end") return ctrl.Result{}, nil } @@ -148,10 +155,10 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque func (r *NonAdminBackupReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&nacv1alpha1.NonAdminBackup{}). - Watches(&velerov1api.Backup{}, &VeleroBackupHandler{}). - WithEventFilter(CompositePredicate{ - NonAdminBackupPredicate: NonAdminBackupPredicate{}, - VeleroBackupPredicate: VeleroBackupPredicate{ + Watches(&velerov1api.Backup{}, &handler.VeleroBackupHandler{}). + WithEventFilter(predicate.CompositePredicate{ + NonAdminBackupPredicate: predicate.NonAdminBackupPredicate{}, + VeleroBackupPredicate: predicate.VeleroBackupPredicate{ OadpVeleroNamespace: "openshift-adp", }, Context: r.Context, diff --git a/internal/controller/nonadminbackup_controller_test.go b/internal/controller/nonadminbackup_controller_test.go index be9434e..551e3e4 100644 --- a/internal/controller/nonadminbackup_controller_test.go +++ b/internal/controller/nonadminbackup_controller_test.go @@ -19,19 +19,18 @@ package controller import ( "context" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" + ginkgov2 "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/reconcile" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" ) -var _ = Describe("NonAdminBackup Controller", func() { - Context("When reconciling a resource", func() { +var _ = ginkgov2.Describe("NonAdminBackup Controller", func() { + ginkgov2.Context("When reconciling a resource", func() { const resourceName = "test-resource" ctx := context.Background() @@ -42,8 +41,8 @@ var _ = Describe("NonAdminBackup Controller", func() { } nonadminbackup := &nacv1alpha1.NonAdminBackup{} - BeforeEach(func() { - By("creating the custom resource for the Kind NonAdminBackup") + ginkgov2.BeforeEach(func() { + ginkgov2.By("creating the custom resource for the Kind NonAdminBackup") err := k8sClient.Get(ctx, typeNamespacedName, nonadminbackup) if err != nil && errors.IsNotFound(err) { resource := &nacv1alpha1.NonAdminBackup{ @@ -53,21 +52,21 @@ var _ = Describe("NonAdminBackup Controller", func() { }, // TODO(user): Specify other spec details if needed. } - Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + gomega.Expect(k8sClient.Create(ctx, resource)).To(gomega.Succeed()) } }) - AfterEach(func() { + ginkgov2.AfterEach(func() { // TODO(user): Cleanup logic after each test, like removing the resource instance. resource := &nacv1alpha1.NonAdminBackup{} err := k8sClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) - By("Cleanup the specific resource instance NonAdminBackup") - Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + ginkgov2.By("Cleanup the specific resource instance NonAdminBackup") + gomega.Expect(k8sClient.Delete(ctx, resource)).To(gomega.Succeed()) }) - It("should successfully reconcile the resource", func() { - By("Reconciling the created resource") + ginkgov2.It("should successfully reconcile the resource", func() { + ginkgov2.By("Reconciling the created resource") controllerReconciler := &NonAdminBackupReconciler{ Client: k8sClient, Scheme: k8sClient.Scheme(), @@ -76,7 +75,7 @@ var _ = Describe("NonAdminBackup Controller", func() { _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ NamespacedName: typeNamespacedName, }) - Expect(err).NotTo(HaveOccurred()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) // TODO(user): Add more specific assertions depending on your controller's reconciliation logic. // Example: If you expect a certain status condition after reconciliation, verify it here. }) diff --git a/internal/controller/nonadminbackup_predicate.go b/internal/controller/nonadminbackup_predicate.go deleted file mode 100644 index f9274f9..0000000 --- a/internal/controller/nonadminbackup_predicate.go +++ /dev/null @@ -1,67 +0,0 @@ -// velerobackup_predicate.go - -package controller - -import ( - "context" - - "github.com/go-logr/logr" - nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/log" -) - -type NonAdminBackupPredicate struct { - Logger logr.Logger -} - -func getNonAdminBackupPredicateLogger(ctx context.Context, name, namespace string) logr.Logger { - return log.FromContext(ctx).WithValues("NonAdminBackupPredicate", types.NamespacedName{Name: name, Namespace: namespace}) -} - -func (predicate NonAdminBackupPredicate) Create(ctx context.Context, evt event.CreateEvent) bool { - - if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { - nameSpace := evt.Object.GetNamespace() - name := evt.Object.GetName() - log := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) - log.V(1).Info("Received Create NonAdminBackupPredicate") - return true - } - - return false -} - -func (predicate NonAdminBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { - if _, ok := evt.ObjectNew.(*nacv1alpha1.NonAdminBackup); ok { - nameSpace := evt.ObjectNew.GetNamespace() - name := evt.ObjectNew.GetName() - log := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) - log.V(1).Info("Received Update NonAdminBackupPredicate") - return true - } - return false -} - -func (predicate NonAdminBackupPredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { - if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { - nameSpace := evt.Object.GetNamespace() - name := evt.Object.GetName() - log := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) - log.V(1).Info("Received Delete NonAdminBackupPredicate") - return true - } - return false -} - -func (predicate NonAdminBackupPredicate) Generic(ctx context.Context, evt event.GenericEvent) bool { - if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { - nameSpace := evt.Object.GetNamespace() - name := evt.Object.GetName() - log := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) - log.V(1).Info("Received Generic NonAdminBackupPredicate") - return true - } - return false -} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 6888143..e1510ba 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -22,9 +22,8 @@ import ( "runtime" "testing" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - + ginkgov2 "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -33,7 +32,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" - //+kubebuilder:scaffold:imports ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to @@ -44,15 +42,15 @@ var k8sClient client.Client var testEnv *envtest.Environment func TestControllers(t *testing.T) { - RegisterFailHandler(Fail) + gomega.RegisterFailHandler(ginkgov2.Fail) - RunSpecs(t, "Controller Suite") + ginkgov2.RunSpecs(t, "Controller Suite") } -var _ = BeforeSuite(func() { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) +var _ = ginkgov2.BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(ginkgov2.GinkgoWriter), zap.UseDevMode(true))) - By("bootstrapping test environment") + ginkgov2.By("bootstrapping test environment") testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, ErrorIfCRDPathMissing: true, @@ -69,22 +67,21 @@ var _ = BeforeSuite(func() { var err error // cfg is defined in this file globally. cfg, err = testEnv.Start() - Expect(err).NotTo(HaveOccurred()) - Expect(cfg).NotTo(BeNil()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(cfg).NotTo(gomega.BeNil()) err = nacv1alpha1.AddToScheme(scheme.Scheme) - Expect(err).NotTo(HaveOccurred()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) - //+kubebuilder:scaffold:scheme + // +kubebuilder:scaffold:scheme k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) - Expect(err).NotTo(HaveOccurred()) - Expect(k8sClient).NotTo(BeNil()) - + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.Expect(k8sClient).NotTo(gomega.BeNil()) }) -var _ = AfterSuite(func() { - By("tearing down the test environment") +var _ = ginkgov2.AfterSuite(func() { + ginkgov2.By("tearing down the test environment") err := testEnv.Stop() - Expect(err).NotTo(HaveOccurred()) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) }) diff --git a/internal/controller/velerobackup_handler.go b/internal/controller/velerobackup_handler.go deleted file mode 100644 index 3e64a40..0000000 --- a/internal/controller/velerobackup_handler.go +++ /dev/null @@ -1,70 +0,0 @@ -// velerobackup_handler.go - -package controller - -import ( - "context" - - "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "k8s.io/client-go/util/workqueue" - "sigs.k8s.io/controller-runtime/pkg/event" -) - -// Handler for VeleroBackup events -type VeleroBackupHandler struct { - Logger logr.Logger -} - -func getVeleroBackupHandlerLogger(ctx context.Context, name, namespace string) logr.Logger { - return log.FromContext(ctx).WithValues("VeleroBackupHandler", types.NamespacedName{Name: name, Namespace: namespace}) -} - -func (h *VeleroBackupHandler) Create(ctx context.Context, evt event.CreateEvent, q workqueue.RateLimitingInterface) { - nameSpace := evt.Object.GetNamespace() - name := evt.Object.GetName() - log := getVeleroBackupHandlerLogger(ctx, name, nameSpace) - log.V(1).Info("Received Create VeleroBackupHandler") -} - -func (h *VeleroBackupHandler) Update(ctx context.Context, evt event.UpdateEvent, q workqueue.RateLimitingInterface) { - nameSpace := evt.ObjectNew.GetNamespace() - name := evt.ObjectNew.GetName() - log := getVeleroBackupHandlerLogger(ctx, name, nameSpace) - log.V(1).Info("Received Update VeleroBackupHandler") - - annotations := evt.ObjectNew.GetAnnotations() - - if annotations == nil { - log.V(1).Info("Backup annotations not found") - return - } - - nabOriginNamespace, ok := annotations[NabOriginNamespaceAnnotation] - if !ok { - log.V(1).Info("Backup NonAdminBackup origin namespace annotation not found") - return - } - - nabOriginName, ok := annotations[NabOriginNameAnnotation] - if !ok { - log.V(1).Info("Backup NonAdminBackup origin name annotation not found") - return - } - - q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ - Name: nabOriginName, - Namespace: nabOriginNamespace, - }}) -} - -func (h *VeleroBackupHandler) Delete(ctx context.Context, evt event.DeleteEvent, q workqueue.RateLimitingInterface) { - // Delete event handler for the Backup object. We should ignore it. -} - -func (h *VeleroBackupHandler) Generic(ctx context.Context, evt event.GenericEvent, q workqueue.RateLimitingInterface) { - // Generic event handler for the Backup object. We should ignore it. -} diff --git a/internal/handler/velerobackup_handler.go b/internal/handler/velerobackup_handler.go new file mode 100644 index 0000000..d1fe4c1 --- /dev/null +++ b/internal/handler/velerobackup_handler.go @@ -0,0 +1,90 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package handler contains all event handlers of the project +package handler + +import ( + "context" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/migtools/oadp-non-admin/internal/common/constant" +) + +// VeleroBackupHandler contains event handlers for Velero Backup objects +type VeleroBackupHandler struct { + Logger logr.Logger +} + +func getVeleroBackupHandlerLogger(ctx context.Context, name, namespace string) logr.Logger { + return log.FromContext(ctx).WithValues("VeleroBackupHandler", types.NamespacedName{Name: name, Namespace: namespace}) +} + +// Create event handler +func (*VeleroBackupHandler) Create(ctx context.Context, evt event.CreateEvent, _ workqueue.RateLimitingInterface) { + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + logger := getVeleroBackupHandlerLogger(ctx, name, nameSpace) + logger.V(1).Info("Received Create VeleroBackupHandler") +} + +// Update event handler +func (*VeleroBackupHandler) Update(ctx context.Context, evt event.UpdateEvent, q workqueue.RateLimitingInterface) { + nameSpace := evt.ObjectNew.GetNamespace() + name := evt.ObjectNew.GetName() + logger := getVeleroBackupHandlerLogger(ctx, name, nameSpace) + logger.V(1).Info("Received Update VeleroBackupHandler") + + annotations := evt.ObjectNew.GetAnnotations() + + if annotations == nil { + logger.V(1).Info("Backup annotations not found") + return + } + + nabOriginNamespace, ok := annotations[constant.NabOriginNamespaceAnnotation] + if !ok { + logger.V(1).Info("Backup NonAdminBackup origin namespace annotation not found") + return + } + + nabOriginName, ok := annotations[constant.NabOriginNameAnnotation] + if !ok { + logger.V(1).Info("Backup NonAdminBackup origin name annotation not found") + return + } + + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{ + Name: nabOriginName, + Namespace: nabOriginNamespace, + }}) +} + +// Delete event handler +func (*VeleroBackupHandler) Delete(_ context.Context, _ event.DeleteEvent, _ workqueue.RateLimitingInterface) { + // Delete event handler for the Backup object +} + +// Generic event handler +func (*VeleroBackupHandler) Generic(_ context.Context, _ event.GenericEvent, _ workqueue.RateLimitingInterface) { + // Generic event handler for the Backup object +} diff --git a/internal/controller/composite_predicate.go b/internal/predicate/composite_predicate.go similarity index 65% rename from internal/controller/composite_predicate.go rename to internal/predicate/composite_predicate.go index 40000cc..751bebc 100644 --- a/internal/controller/composite_predicate.go +++ b/internal/predicate/composite_predicate.go @@ -1,6 +1,21 @@ -// composite_predicate.go +/* +Copyright 2024. -package controller +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package predicate contains all event filters of the project +package predicate import ( "context" @@ -8,12 +23,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" ) +// CompositePredicate is a combination of all project event filters type CompositePredicate struct { + Context context.Context NonAdminBackupPredicate NonAdminBackupPredicate VeleroBackupPredicate VeleroBackupPredicate - Context context.Context } +// Create event filter func (p CompositePredicate) Create(evt event.CreateEvent) bool { // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate if p.NonAdminBackupPredicate.Create(p.Context, evt) { @@ -23,6 +40,7 @@ func (p CompositePredicate) Create(evt event.CreateEvent) bool { return p.VeleroBackupPredicate.Create(p.Context, evt) } +// Update event filter func (p CompositePredicate) Update(evt event.UpdateEvent) bool { // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate if p.NonAdminBackupPredicate.Update(p.Context, evt) { @@ -32,6 +50,7 @@ func (p CompositePredicate) Update(evt event.UpdateEvent) bool { return p.VeleroBackupPredicate.Update(p.Context, evt) } +// Delete event filter func (p CompositePredicate) Delete(evt event.DeleteEvent) bool { // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate if p.NonAdminBackupPredicate.Delete(p.Context, evt) { @@ -41,6 +60,7 @@ func (p CompositePredicate) Delete(evt event.DeleteEvent) bool { return p.VeleroBackupPredicate.Delete(p.Context, evt) } +// Generic event filter func (p CompositePredicate) Generic(evt event.GenericEvent) bool { // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate if p.NonAdminBackupPredicate.Generic(p.Context, evt) { diff --git a/internal/predicate/nonadminbackup_predicate.go b/internal/predicate/nonadminbackup_predicate.go new file mode 100644 index 0000000..a117061 --- /dev/null +++ b/internal/predicate/nonadminbackup_predicate.go @@ -0,0 +1,86 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package predicate + +import ( + "context" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/log" + + nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" +) + +// NonAdminBackupPredicate contains event filters for Non Admin Backup objects +type NonAdminBackupPredicate struct { + Logger logr.Logger +} + +func getNonAdminBackupPredicateLogger(ctx context.Context, name, namespace string) logr.Logger { + return log.FromContext(ctx).WithValues("NonAdminBackupPredicate", types.NamespacedName{Name: name, Namespace: namespace}) +} + +// Create event filter +func (NonAdminBackupPredicate) Create(ctx context.Context, evt event.CreateEvent) bool { + if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("Received Create NonAdminBackupPredicate") + return true + } + + return false +} + +// Update event filter +func (NonAdminBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { + if _, ok := evt.ObjectNew.(*nacv1alpha1.NonAdminBackup); ok { + nameSpace := evt.ObjectNew.GetNamespace() + name := evt.ObjectNew.GetName() + logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("Received Update NonAdminBackupPredicate") + return true + } + return false +} + +// Delete event filter +func (NonAdminBackupPredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { + if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("Received Delete NonAdminBackupPredicate") + return true + } + return false +} + +// Generic event filter +func (NonAdminBackupPredicate) Generic(ctx context.Context, evt event.GenericEvent) bool { + if _, ok := evt.Object.(*nacv1alpha1.NonAdminBackup); ok { + nameSpace := evt.Object.GetNamespace() + name := evt.Object.GetName() + logger := getNonAdminBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("Received Generic NonAdminBackupPredicate") + return true + } + return false +} diff --git a/internal/controller/velerobackup_predicate.go b/internal/predicate/velerobackup_predicate.go similarity index 50% rename from internal/controller/velerobackup_predicate.go rename to internal/predicate/velerobackup_predicate.go index c24f135..08c9fe5 100644 --- a/internal/controller/velerobackup_predicate.go +++ b/internal/predicate/velerobackup_predicate.go @@ -1,23 +1,34 @@ -// velerobackup_predicate.go +/* +Copyright 2024. -package controller +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package predicate import ( "context" "github.com/go-logr/logr" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" -) -func VeleroPredicate(scheme *runtime.Scheme) predicate.Predicate { - return nil -} + "github.com/migtools/oadp-non-admin/internal/common/function" +) +// VeleroBackupPredicate contains event filters for Velero Backup objects type VeleroBackupPredicate struct { // We are watching only Velero Backup objects within // namespace where OADP is. @@ -25,10 +36,13 @@ type VeleroBackupPredicate struct { Logger logr.Logger } +// TODO try to remove calls to get logger functions, try to initialize it + func getBackupPredicateLogger(ctx context.Context, name, namespace string) logr.Logger { return log.FromContext(ctx).WithValues("VeleroBackupPredicate", types.NamespacedName{Name: name, Namespace: namespace}) } +// Create event filter func (veleroBackupPredicate VeleroBackupPredicate) Create(ctx context.Context, evt event.CreateEvent) bool { nameSpace := evt.Object.GetNamespace() if nameSpace != veleroBackupPredicate.OadpVeleroNamespace { @@ -36,34 +50,36 @@ func (veleroBackupPredicate VeleroBackupPredicate) Create(ctx context.Context, e } name := evt.Object.GetName() - log := getBackupPredicateLogger(ctx, name, nameSpace) - log.V(1).Info("Received Create VeleroBackupPredicate") + logger := getBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("Received Create VeleroBackupPredicate") backup, ok := evt.Object.(*velerov1api.Backup) if !ok { // The event object is not a Backup, ignore it return false } - if HasRequiredLabel(backup) { + if function.CheckVeleroBackupLabels(backup) { return true } return false } +// Update event filter func (veleroBackupPredicate VeleroBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent) bool { nameSpace := evt.ObjectNew.GetNamespace() name := evt.ObjectNew.GetName() - log := getBackupPredicateLogger(ctx, name, nameSpace) - log.V(1).Info("Received Update VeleroBackupPredicate") + logger := getBackupPredicateLogger(ctx, name, nameSpace) + logger.V(1).Info("Received Update VeleroBackupPredicate") return nameSpace == veleroBackupPredicate.OadpVeleroNamespace } -func (veleroBackupPredicate VeleroBackupPredicate) Delete(ctx context.Context, evt event.DeleteEvent) bool { - // NonAdminBackup should not care about VeleroBackup delete events +// Delete event filter +func (VeleroBackupPredicate) Delete(_ context.Context, _ event.DeleteEvent) bool { return false } -func (veleroBackupPredicate VeleroBackupPredicate) Generic(ctx context.Context, evt event.GenericEvent) bool { +// Generic event filter +func (VeleroBackupPredicate) Generic(_ context.Context, _ event.GenericEvent) bool { return false } diff --git a/test/utils/utils.go b/test/utils/utils.go index 7363aa5..01e2f51 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -22,7 +22,7 @@ import ( "os/exec" "strings" - . "github.com/onsi/ginkgo/v2" //nolint:golint,revive + . "github.com/onsi/ginkgo/v2" ) const (