Skip to content

Commit 39d0a51

Browse files
committed
Adding logic to allow namespace metadata to be honoured on destination environment
Signed-off-by: Brian Philip <brian.b.philip@shell.com>
1 parent 1052e57 commit 39d0a51

File tree

1 file changed

+69
-26
lines changed

1 file changed

+69
-26
lines changed

internal/sync/object_syncer.go

Lines changed: 69 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ import (
3030

3131
corev1 "k8s.io/api/core/v1"
3232
apierrors "k8s.io/apimachinery/pkg/api/errors"
33+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3334
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3435
"k8s.io/apimachinery/pkg/runtime"
3536
"k8s.io/apimachinery/pkg/types"
37+
"k8s.io/apimachinery/pkg/api/meta"
3638
ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
3739
)
3840

@@ -325,11 +327,23 @@ func (s *objectSyncer) ensureDestinationObject(ctx context.Context, log *zap.Sug
325327
return fmt.Errorf("failed to create destination object: %w", err)
326328
}
327329

330+
log.Debugw("Starting ensureDestinationObject",
331+
"sourceNS", source.object.GetNamespace(),
332+
"destNS", destObj.GetNamespace(),
333+
"destGVK", destObj.GroupVersionKind(),
334+
"destName", destObj.GetName(),
335+
)
328336
// make sure the target namespace on the destination cluster exists
329-
if err := s.ensureNamespace(ctx, log, dest.client, destObj.GetNamespace()); err != nil {
337+
if err := s.ensureNamespace(ctx, log, dest.client, source, destObj.GetNamespace()); err != nil {
338+
log.Errorw("ensureNamespace failed",
339+
"namespace", destObj.GetNamespace(),
340+
"error", err,
341+
)
330342
return fmt.Errorf("failed to ensure destination namespace: %w", err)
331343
}
332344

345+
log.Debugw("Namespace ensured", "namespace", destObj.GetNamespace())
346+
333347
// remove source metadata (like UID and generation, but also labels and annotations belonging to
334348
// the sync-agent) to allow destination object creation to succeed
335349
if err := stripMetadata(destObj); err != nil {
@@ -402,31 +416,60 @@ func (s *objectSyncer) adoptExistingDestinationObject(ctx context.Context, log *
402416
return nil
403417
}
404418

405-
func (s *objectSyncer) ensureNamespace(ctx context.Context, log *zap.SugaredLogger, client ctrlruntimeclient.Client, namespace string) error {
406-
// cluster-scoped objects do not need namespaces
407-
if namespace == "" {
408-
return nil
409-
}
410-
411-
// Use a get-then-create approach to benefit from having a cache; otherwise if we always
412-
// send a create request, we're needlessly spamming the kube apiserver. Yes, this approach
413-
// is a race condition and we have to check for AlreadyExists later down the line, but that
414-
// only occurs on cold caches. During normal operations this should be more efficient.
415-
ns := &corev1.Namespace{}
416-
if err := client.Get(ctx, types.NamespacedName{Name: namespace}, ns); ctrlruntimeclient.IgnoreNotFound(err) != nil {
417-
return fmt.Errorf("failed to check: %w", err)
418-
}
419-
420-
if ns.Name == "" {
421-
ns.Name = namespace
422-
423-
log.Debugw("Creating namespace…", "namespace", namespace)
424-
if err := client.Create(ctx, ns); err != nil && !apierrors.IsAlreadyExists(err) {
425-
return fmt.Errorf("failed to create: %w", err)
426-
}
427-
}
428-
429-
return nil
419+
func (s *objectSyncer) ensureNamespace(ctx context.Context, log *zap.SugaredLogger, client ctrlruntimeclient.Client, source syncSide, namespace string) error {
420+
// cluster-scoped objects do not need namespaces
421+
if namespace == "" {
422+
return nil
423+
}
424+
425+
// Check if the destination namespace already exists
426+
destNs := &corev1.Namespace{}
427+
if err := client.Get(ctx, types.NamespacedName{Name: namespace}, destNs); ctrlruntimeclient.IgnoreNotFound(err) != nil {
428+
log.Errorw("Failed to fetch destination namespace", "namespace", namespace, "error", err)
429+
return fmt.Errorf("failed to check destination namespace %q: %w", namespace, err)
430+
}
431+
432+
// Only build a new namespace if one doesn’t exist
433+
if destNs.Name == "" {
434+
// Check if permissions claims on source namespace allow us to access metadata
435+
sourceNs := &corev1.Namespace{}
436+
labels, annotations := map[string]string{}, map[string]string{}
437+
438+
if err := source.client.Get(ctx, types.NamespacedName{Name: source.object.GetNamespace()}, sourceNs); err != nil {
439+
switch {
440+
case apierrors.IsForbidden(err):
441+
log.Warnw("Skipping namespace metadata: missing permission to read source namespace", "namespace", namespace)
442+
case apierrors.IsNotFound(err):
443+
log.Warnw("Source namespace not found, creating destination without metadata", "namespace", namespace)
444+
case meta.IsNoMatchError(err):
445+
log.Warnw("Skipping namespace metadata: APIExport does not expose Namespace", "namespace", namespace)
446+
default:
447+
log.Errorw("failed to fetch source namespace", "sourceNS", source.object.GetNamespace(), "error", err)
448+
return fmt.Errorf("failed to fetch source namespace %q: %w", source.object.GetNamespace(), err)
449+
}
450+
} else {
451+
labels = sourceNs.Labels
452+
annotations = sourceNs.Annotations
453+
log.Debugw("Fetched source namespace metadata", "labels", labels, "annotations", annotations)
454+
}
455+
456+
ns := &corev1.Namespace{
457+
ObjectMeta: metav1.ObjectMeta{
458+
Name: namespace,
459+
Labels: labels,
460+
Annotations: annotations,
461+
},
462+
}
463+
464+
log.Debugw("Creating namespace", "namespace", namespace)
465+
if err := client.Create(ctx, ns); err != nil && !apierrors.IsAlreadyExists(err) {
466+
log.Errorw("Failed to create destination namespace", "namespace", namespace, "error", err)
467+
return fmt.Errorf("failed to create namespace %q: %w", namespace, err)
468+
}
469+
log.Debugw("Namespace created or already existed", "namespace", namespace)
470+
}
471+
472+
return nil
430473
}
431474

432475
func (s *objectSyncer) handleDeletion(ctx context.Context, log *zap.SugaredLogger, source, dest syncSide) (requeue bool, err error) {

0 commit comments

Comments
 (0)