Skip to content

Commit

Permalink
fix: Update NAB status phase and condition together
Browse files Browse the repository at this point in the history
Signed-off-by: Mateus Oliveira <[email protected]>
  • Loading branch information
mateusoliveira43 committed Sep 25, 2024
1 parent 282620c commit f5e9c6b
Show file tree
Hide file tree
Showing 9 changed files with 252 additions and 276 deletions.
5 changes: 3 additions & 2 deletions internal/common/constant/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package constant
// of the specific Object, such as Backup/Restore.
const (
OadpLabel = "openshift.io/oadp" // TODO import?
OadpLabelValue = TrueString
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"
Expand All @@ -38,8 +39,8 @@ const (
// EmptyString defines a constant for the empty string
const EmptyString = ""

// MaxKubernetesNameLength represents maximum length of the name in k8s
const MaxKubernetesNameLength = 253
// TrueString defines a constant for the True string
const TrueString = "True"

// VeleroBackupNamePrefix represents the prefix for the object name generated
// by the NonAdminController
Expand Down
59 changes: 51 additions & 8 deletions internal/common/function/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import (
"encoding/hex"
"fmt"

"github.com/go-logr/logr"
velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

Expand All @@ -38,7 +40,7 @@ const requiredAnnotationError = "backup does not have the required annotation '%
// 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.OadpLabel: constant.OadpLabelValue,
constant.ManagedByLabel: constant.ManagedByLabelValue,
}

Expand Down Expand Up @@ -125,9 +127,9 @@ func GenerateVeleroBackupName(namespace, nabName string) string {
veleroBackupName := fmt.Sprintf("%s-%s-%s", constant.VeleroBackupNamePrefix, namespace, nameHash)

// Ensure the name is within the character limit
if len(veleroBackupName) > constant.MaxKubernetesNameLength {
if len(veleroBackupName) > validation.DNS1123SubdomainMaxLength {
// Truncate the namespace if necessary
maxNamespaceLength := constant.MaxKubernetesNameLength - len(nameHash) - prefixLength
maxNamespaceLength := validation.DNS1123SubdomainMaxLength - len(nameHash) - prefixLength
if len(namespace) > maxNamespaceLength {
namespace = namespace[:maxNamespaceLength]
}
Expand All @@ -137,11 +139,46 @@ func GenerateVeleroBackupName(namespace, nabName string) string {
return veleroBackupName
}

// CheckVeleroBackupLabels return true if Velero Backup object has required Non Admin labels, false otherwise
func CheckVeleroBackupLabels(labels map[string]string) bool {
// TODO also need to check for constant.OadpLabel label?
value, exists := labels[constant.ManagedByLabel]
return exists && value == constant.ManagedByLabelValue
// CheckVeleroBackupMetadata return true if Velero Backup object has required Non Admin labels and annotations, false otherwise
func CheckVeleroBackupMetadata(obj client.Object) bool {
labels := obj.GetLabels()
if !checkLabelValue(labels, constant.OadpLabel, constant.OadpLabelValue) {
return false
}
if !checkLabelValue(labels, constant.ManagedByLabel, constant.ManagedByLabelValue) {
return false
}

annotations := obj.GetAnnotations()
if !checkAnnotationValueIsValid(annotations, constant.NabOriginNamespaceAnnotation) {
return false
}
if !checkAnnotationValueIsValid(annotations, constant.NabOriginNameAnnotation) {
return false
}
// TODO what is a valid uuid?
if !checkAnnotationValueIsValid(annotations, constant.NabOriginUUIDAnnotation) {
return false
}

return true
}

func checkLabelValue(labels map[string]string, key string, value string) bool {
got, exists := labels[key]
if !exists {
return false
}
return got == value
}

func checkAnnotationValueIsValid(annotations map[string]string, key string) bool {
value, exists := annotations[key]
if !exists {
return false
}
length := len(value)
return length > 0 && length < validation.DNS1123SubdomainMaxLength
}

// TODO not used
Expand Down Expand Up @@ -217,3 +254,9 @@ func mergeMaps[T comparable](maps ...map[T]T) (map[T]T, error) {
}
return merge, nil
}

// GetLogger return a logger from input ctx, with additional key/value pairs being
// input key and input obj name and namespace
func GetLogger(ctx context.Context, obj client.Object, key string) logr.Logger {
return log.FromContext(ctx).WithValues(key, types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()})
}
142 changes: 114 additions & 28 deletions internal/common/function/function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ import (
"context"
"fmt"
"reflect"
"strings"
"testing"

"github.com/onsi/ginkgo/v2"
"github.com/stretchr/testify/assert"
velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
Expand All @@ -38,6 +40,12 @@ import (

var _ = ginkgo.Describe("PLACEHOLDER", func() {})

const (
testNonAdminBackupNamespace = "non-admin-backup-namespace"
testNonAdminBackupName = "non-admin-backup-name"
testNonAdminBackupUUID = "12345678-1234-1234-1234-123456789abc"
)

func TestMergeMaps(t *testing.T) {
const (
d = "d"
Expand Down Expand Up @@ -294,18 +302,18 @@ func TestGetNonAdminBackupFromVeleroBackup(t *testing.T) {
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",
constant.NabOriginNamespaceAnnotation: testNonAdminBackupNamespace,
constant.NabOriginNameAnnotation: testNonAdminBackupName,
constant.NabOriginUUIDAnnotation: testNonAdminBackupUUID,
},
},
}

nonAdminBackup := &nacv1alpha1.NonAdminBackup{
ObjectMeta: metav1.ObjectMeta{
Namespace: "non-admin-backup-namespace",
Name: "non-admin-backup-name",
UID: types.UID("12345678-1234-1234-1234-123456789abc"),
Namespace: testNonAdminBackupNamespace,
Name: testNonAdminBackupName,
UID: types.UID(testNonAdminBackupUUID),
},
}

Expand All @@ -317,32 +325,110 @@ func TestGetNonAdminBackupFromVeleroBackup(t *testing.T) {
assert.Equal(t, nonAdminBackup, result, "Returned NonAdminBackup should match expected NonAdminBackup")
}

func TestCheckVeleroBackupLabels(t *testing.T) {
// Backup has the required label
backupWithLabel := &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
constant.ManagedByLabel: constant.ManagedByLabelValue,
func TestCheckVeleroBackupMetadata(t *testing.T) {
tests := []struct {
backup *velerov1.Backup
name string
expected bool
}{
{
name: "Velero Backup without required non admin labels and annotations",
backup: &velerov1.Backup{},
expected: false,
},
{
name: "Velero Backup without required non admin annotations",
backup: &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
constant.OadpLabel: constant.OadpLabelValue,
constant.ManagedByLabel: constant.ManagedByLabelValue,
},
},
},
expected: false,
},
}
assert.True(t, CheckVeleroBackupLabels(backupWithLabel.GetLabels()), "Expected backup to have required label")

// Backup does not have the required label
backupWithoutLabel := &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{},
{
name: "Velero Backup with wrong required non admin label",
backup: &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
constant.OadpLabel: constant.OadpLabelValue,
constant.ManagedByLabel: "foo",
},
},
},
expected: false,
},
}
assert.False(t, CheckVeleroBackupLabels(backupWithoutLabel.GetLabels()), "Expected backup to not have required label")

// Backup has the required label with incorrect value
backupWithIncorrectValue := &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
constant.ManagedByLabel: "incorrect-value",
{
name: "Velero Backup without required non admin labels",
backup: &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
constant.NabOriginNamespaceAnnotation: testNonAdminBackupNamespace,
constant.NabOriginNameAnnotation: testNonAdminBackupName,
constant.NabOriginUUIDAnnotation: testNonAdminBackupUUID,
},
},
},
expected: false,
},
{
name: "Velero Backup with wrong required non admin annotation [empty]",
backup: &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
constant.OadpLabel: constant.OadpLabelValue,
constant.ManagedByLabel: constant.ManagedByLabelValue,
},
Annotations: map[string]string{
constant.NabOriginNamespaceAnnotation: constant.EmptyString,
constant.NabOriginNameAnnotation: testNonAdminBackupName,
constant.NabOriginUUIDAnnotation: testNonAdminBackupUUID,
},
},
},
expected: false,
},
{
name: "Velero Backup with wrong required non admin annotation [long]",
backup: &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
constant.OadpLabel: constant.OadpLabelValue,
constant.ManagedByLabel: constant.ManagedByLabelValue,
},
Annotations: map[string]string{
constant.NabOriginNamespaceAnnotation: testNonAdminBackupNamespace,
constant.NabOriginNameAnnotation: strings.Repeat("nn", validation.DNS1123SubdomainMaxLength),
constant.NabOriginUUIDAnnotation: testNonAdminBackupUUID,
},
},
},
expected: false,
},
{
name: "Velero Backup with required non admin labels and annotations",
backup: &velerov1.Backup{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
constant.OadpLabel: constant.OadpLabelValue,
constant.ManagedByLabel: constant.ManagedByLabelValue,
},
Annotations: map[string]string{
constant.NabOriginNamespaceAnnotation: testNonAdminBackupNamespace,
constant.NabOriginNameAnnotation: testNonAdminBackupName,
constant.NabOriginUUIDAnnotation: testNonAdminBackupUUID,
},
},
},
expected: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := CheckVeleroBackupMetadata(test.backup)
assert.Equal(t, test.expected, result)
})
}
assert.False(t, CheckVeleroBackupLabels(backupWithIncorrectValue.GetLabels()), "Expected backup to not have required label")
}
Loading

0 comments on commit f5e9c6b

Please sign in to comment.