From e05016b6f6c322f7cac7f35328ccb21f2a4f55eb Mon Sep 17 00:00:00 2001 From: Mateus Oliveira Date: Thu, 7 Mar 2024 12:05:53 -0300 Subject: [PATCH] fixup! fix: Enable more linters Signed-off-by: Mateus Oliveira --- .github/workflows/ci.yml | 3 +- .golangci.yml | 12 +- Makefile | 2 +- cmd/main.go | 1 + internal/common/constant/constant.go | 31 ++ internal/common/function/function.go | 224 +++++++++++++ internal/common/function/function_test.go | 308 ++++++++++++++++++ internal/controller/common_k8s.go | 93 ------ internal/controller/common_k8s_test.go | 149 --------- internal/controller/common_nab.go | 92 ------ internal/controller/common_nab_test.go | 104 ------ internal/controller/common_velero.go | 83 ----- internal/controller/common_velero_test.go | 101 ------ .../controller/nonadminbackup_controller.go | 44 +-- .../velerobackup_handler.go | 15 +- .../composite_predicate.go | 8 +- .../nonadminbackup_predicate.go | 7 +- .../velerobackup_predicate.go | 13 +- 18 files changed, 635 insertions(+), 655 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 rename internal/{controller => handler}/velerobackup_handler.go (84%) rename internal/{controller => predicate}/composite_predicate.go (89%) rename internal/{controller => predicate}/nonadminbackup_predicate.go (93%) rename internal/{controller => predicate}/velerobackup_predicate.go (86%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bea4022..ba4176f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,9 +36,10 @@ jobs: # TODO coverage report - name: Go linters + if: ${{ matrix.go-version == '1.21' }} uses: golangci/golangci-lint-action@v4 with: - version: v1.55.2 + version: v1.56.2 skip-pkg-cache: true container-check: diff --git a/.golangci.yml b/.golangci.yml index 1bf0edd..a786073 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,4 +1,4 @@ -# Documentation reference https://github.com/golangci/golangci-lint/blob/v1.55.2/.golangci.reference.yml +# Documentation reference https://github.com/golangci/golangci-lint/blob/v1.56.2/.golangci.reference.yml run: skip-dirs-use-default: false modules-download-mode: readonly @@ -61,6 +61,13 @@ linters-settings: 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 @@ -114,9 +121,6 @@ issues: - linters: - revive text: "^add-constant: avoid magic numbers like '1', create a named constant for it$" - # - linters: - # - stylecheck - # text: "ST1000:|ST1020:|ST1021:|ST1022:" max-issues-per-linter: 0 max-same-issues: 0 diff --git a/Makefile b/Makefile index a76479a..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.55.2 +GOLANGCI_LINT_VERSION ?= v1.56.2 .PHONY: kustomize kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. diff --git a/cmd/main.go b/cmd/main.go index 5da2f20..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 ( 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..219adf7 --- /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 := mergeUniqueKeyTOfTMaps(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 := mergeUniqueKeyTOfTMaps(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 +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/common/function/function_test.go b/internal/common/function/function_test.go new file mode 100644 index 0000000..4091a22 --- /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 TestMergeUniqueKeyTOfTMaps(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 := mergeUniqueKeyTOfTMaps(tt.args...) + 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 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 361c157..0000000 --- a/internal/controller/common_k8s.go +++ /dev/null @@ -1,93 +0,0 @@ -/* -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 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 { - // TODO logger - _, _ = 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 { - // TODO logger - _, _ = 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 efabe76..0000000 --- a/internal/controller/common_k8s_test.go +++ /dev/null @@ -1,149 +0,0 @@ -/* -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 controller - -import ( - "reflect" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestMergeUniqueKeyTOfTMaps(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 := mergeUniqueKeyTOfTMaps(tt.args...) - 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 a3ec45f..0000000 --- a/internal/controller/common_nab.go +++ /dev/null @@ -1,92 +0,0 @@ -/* -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 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 1f98eef..0000000 --- a/internal/controller/common_nab_test.go +++ /dev/null @@ -1,104 +0,0 @@ -/* -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 controller - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - - nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" -) - -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 e455481..0000000 --- a/internal/controller/common_velero.go +++ /dev/null @@ -1,83 +0,0 @@ -/* -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 controller - -import ( - "context" - "fmt" - - 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" -) - -const requiredAnnotationError = "backup does not have the required annotation '%s'" - -func HasRequiredLabel(backup *velerov1api.Backup) bool { - labels := backup.GetLabels() - value, exists := labels[ManagedByLabel] - return exists && value == ManagedByLabelValue -} - -func GetNonAdminFromBackup(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[NabOriginNamespaceAnnotation] - if !ok { - return nil, fmt.Errorf(requiredAnnotationError, NabOriginNamespaceAnnotation) - } - - nabOriginName, ok := annotations[NabOriginNameAnnotation] - if !ok { - return nil, fmt.Errorf(requiredAnnotationError, 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[NabOriginUUIDAnnotation] - if !ok { - return nil, fmt.Errorf(requiredAnnotationError, 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 d8570d9..0000000 --- a/internal/controller/common_velero_test.go +++ /dev/null @@ -1,101 +0,0 @@ -/* -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 controller - -import ( - "context" - "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" -) - -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 235cbb0..5f4281d 100644 --- a/internal/controller/nonadminbackup_controller.go +++ b/internal/controller/nonadminbackup_controller.go @@ -14,6 +14,7 @@ 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 ( @@ -31,6 +32,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" + "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 @@ -42,7 +46,10 @@ type NonAdminBackupReconciler struct { NamespacedName types.NamespacedName } -const nameField = "Name" +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 @@ -79,10 +86,10 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque return ctrl.Result{}, err } - veleroBackupSpec, err := GetVeleroBackupSpecFromNonAdminBackup(&nab) + backupSpec, err := function.GetBackupSpecFromNonAdminBackup(&nab) - if veleroBackupSpec == nil { - logger.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 } @@ -93,10 +100,10 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque 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 @@ -104,33 +111,32 @@ func (r *NonAdminBackupReconciler) Reconcile(ctx context.Context, req ctrl.Reque veleroBackup = velerov1api.Backup{ ObjectMeta: metav1.ObjectMeta{ Name: veleroBackupName, - Namespace: OadpNamespace, + Namespace: oadpNamespace, }, - Spec: *veleroBackupSpec, + Spec: *backupSpec, } } else if err != nil && !errors.IsNotFound(err) { logger.Error(err, "Unable to fetch VeleroBackup") return ctrl.Result{}, err } else { logger.Info("Backup already exists, updating NonAdminBackup status", nameField, veleroBackupName) - err = UpdateNonAdminBackupFromVeleroBackup(ctx, r.Client, logger, &nab, &veleroBackup) + 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 { @@ -149,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/velerobackup_handler.go b/internal/handler/velerobackup_handler.go similarity index 84% rename from internal/controller/velerobackup_handler.go rename to internal/handler/velerobackup_handler.go index c270070..d1fe4c1 100644 --- a/internal/controller/velerobackup_handler.go +++ b/internal/handler/velerobackup_handler.go @@ -14,7 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controller +// Package handler contains all event handlers of the project +package handler import ( "context" @@ -25,9 +26,11 @@ import ( "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 Handles VeleroBackup events +// VeleroBackupHandler contains event handlers for Velero Backup objects type VeleroBackupHandler struct { Logger logr.Logger } @@ -36,6 +39,7 @@ func getVeleroBackupHandlerLogger(ctx context.Context, name, namespace string) l 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() @@ -43,6 +47,7 @@ func (*VeleroBackupHandler) Create(ctx context.Context, evt event.CreateEvent, _ 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() @@ -56,13 +61,13 @@ func (*VeleroBackupHandler) Update(ctx context.Context, evt event.UpdateEvent, q return } - nabOriginNamespace, ok := annotations[NabOriginNamespaceAnnotation] + nabOriginNamespace, ok := annotations[constant.NabOriginNamespaceAnnotation] if !ok { logger.V(1).Info("Backup NonAdminBackup origin namespace annotation not found") return } - nabOriginName, ok := annotations[NabOriginNameAnnotation] + nabOriginName, ok := annotations[constant.NabOriginNameAnnotation] if !ok { logger.V(1).Info("Backup NonAdminBackup origin name annotation not found") return @@ -74,10 +79,12 @@ func (*VeleroBackupHandler) Update(ctx context.Context, evt event.UpdateEvent, q }}) } +// 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 89% rename from internal/controller/composite_predicate.go rename to internal/predicate/composite_predicate.go index ae83961..751bebc 100644 --- a/internal/controller/composite_predicate.go +++ b/internal/predicate/composite_predicate.go @@ -14,7 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controller +// Package predicate contains all event filters of the project +package predicate import ( "context" @@ -22,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 } +// Create event filter func (p CompositePredicate) Create(evt event.CreateEvent) bool { // If NonAdminBackupPredicate returns true, ignore VeleroBackupPredicate if p.NonAdminBackupPredicate.Create(p.Context, evt) { @@ -37,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) { @@ -46,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) { @@ -55,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/controller/nonadminbackup_predicate.go b/internal/predicate/nonadminbackup_predicate.go similarity index 93% rename from internal/controller/nonadminbackup_predicate.go rename to internal/predicate/nonadminbackup_predicate.go index 35fcfdb..a117061 100644 --- a/internal/controller/nonadminbackup_predicate.go +++ b/internal/predicate/nonadminbackup_predicate.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controller +package predicate import ( "context" @@ -27,6 +27,7 @@ import ( nacv1alpha1 "github.com/migtools/oadp-non-admin/api/v1alpha1" ) +// NonAdminBackupPredicate contains event filters for Non Admin Backup objects type NonAdminBackupPredicate struct { Logger logr.Logger } @@ -35,6 +36,7 @@ func getNonAdminBackupPredicateLogger(ctx context.Context, name, namespace strin 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() @@ -47,6 +49,7 @@ func (NonAdminBackupPredicate) Create(ctx context.Context, evt event.CreateEvent 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() @@ -58,6 +61,7 @@ func (NonAdminBackupPredicate) Update(ctx context.Context, evt event.UpdateEvent 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() @@ -69,6 +73,7 @@ func (NonAdminBackupPredicate) Delete(ctx context.Context, evt event.DeleteEvent 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() diff --git a/internal/controller/velerobackup_predicate.go b/internal/predicate/velerobackup_predicate.go similarity index 86% rename from internal/controller/velerobackup_predicate.go rename to internal/predicate/velerobackup_predicate.go index ea2c39c..08c9fe5 100644 --- a/internal/controller/velerobackup_predicate.go +++ b/internal/predicate/velerobackup_predicate.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controller +package predicate import ( "context" @@ -24,8 +24,11 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/log" + + "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. @@ -33,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 { @@ -52,12 +58,13 @@ func (veleroBackupPredicate VeleroBackupPredicate) Create(ctx context.Context, e // 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() @@ -67,10 +74,12 @@ func (veleroBackupPredicate VeleroBackupPredicate) Update(ctx context.Context, e return nameSpace == veleroBackupPredicate.OadpVeleroNamespace } +// Delete event filter func (VeleroBackupPredicate) Delete(_ context.Context, _ event.DeleteEvent) bool { return false } +// Generic event filter func (VeleroBackupPredicate) Generic(_ context.Context, _ event.GenericEvent) bool { return false }