Skip to content

Commit

Permalink
Address code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ncdc committed Jul 12, 2024
1 parent 1c6dbfe commit 649b4f2
Showing 1 changed file with 11 additions and 14 deletions.
25 changes: 11 additions & 14 deletions pkg/sync/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,25 +78,22 @@ func Reconcile(targetObjs []*unstructured.Unstructured, liveObjByKey map[kube.Re
gvk := obj.GroupVersionKind()

ns := text.FirstNonEmpty(obj.GetNamespace(), namespace)
if namespaced := kubeutil.IsNamespacedOrUnknown(resInfo, obj.GroupVersionKind().GroupKind()); !namespaced {
ns = ""
}

keys := []kubeutil.ResourceKey{
kubeutil.NewResourceKey(gvk.Group, gvk.Kind, ns, obj.GetName()),
namespaced, err := resInfo.IsNamespaced(gvk.GroupKind())
unknownScope := err != nil

var keysToCheck []kubeutil.ResourceKey
// If we get an error, we don't know whether the resource is namespaced. So we need to check for both in the
// live objects. If we don't check for both, then we risk missing the object and deleting it.
if namespaced || unknownScope {
keysToCheck = append(keysToCheck, kubeutil.NewResourceKey(gvk.Group, gvk.Kind, ns, obj.GetName()))
}
if ns != "" {
// If IsNamespacedOrUnknown returned namespaced=true because it's actually unknown, there is a chance
// that we incorrectly use a non-empty value for ns for a cluster-scoped resource for the resource
// key, when it should actually be empty. This happens if the namespace parameter passed in to this
// method has a value. Therefore, we check a key both with and without the namespace set. Otherwise,
// we risk indicating that the live object does not exist when it in fact does, which then
// incorrectly appends a nil to targetObjs and potentially causes data loss.
keys = append(keys, kubeutil.NewResourceKey(gvk.Group, gvk.Kind, "", obj.GetName()))
if !namespaced || unknownScope {
keysToCheck = append(keysToCheck, kubeutil.NewResourceKey(gvk.Group, gvk.Kind, "", obj.GetName()))
}

found := false
for _, key := range keys {
for _, key := range keysToCheck {
if liveObj, ok := liveObjByKey[key]; ok {
managedLiveObj[i] = liveObj
delete(liveObjByKey, key)
Expand Down

0 comments on commit 649b4f2

Please sign in to comment.