Skip to content

Commit

Permalink
fix: check for double definition (#15670)
Browse files Browse the repository at this point in the history
* fix: check for double definition

As found in #13965 (and as a follow-up to #13999), we also need to
define what happens if _both_ managedNamespaceMetadata _and_ an
Application manifest are both defined for the same namespace.

The idea here is that if that happens, we emit an
`ApplicationConditionRepeatedResourceWarning`, and set the sync status
to `Unknown`, since it's unclear what is supposed to happen.

The user will then have the option of removing one of the two
definitions.

Signed-off-by: Blake Pettersson <[email protected]>

* fix: check for double definition

A simpler fix - don't add a managed namespace to the targetObjs list if
a namespace already exists in the application source.

Signed-off-by: Blake Pettersson <[email protected]>

* test: add test cases

This adds a test case showing that an ns manifest will override
`managedNamespaceMetadata` without failing horribly (as it currently
does on `HEAD`), as well as a "standard" mNMd diff vs live.

Signed-off-by: Blake Pettersson <[email protected]>

---------

Signed-off-by: Blake Pettersson <[email protected]>
  • Loading branch information
blakepettersson authored Nov 8, 2023
1 parent 35ab139 commit 21fc0d0
Showing 1 changed file with 116 additions and 0 deletions.
116 changes: 116 additions & 0 deletions controller/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,122 @@ func TestCompareAppStateNamespaceMetadataDiffers(t *testing.T) {
assert.Len(t, app.Status.Conditions, 0)
}

// TestCompareAppStateNamespaceMetadataDiffers tests comparison when managed namespace metadata differs to live and manifest ns
func TestCompareAppStateNamespaceMetadataDiffersToManifest(t *testing.T) {
ns := NewNamespace()
ns.SetName(test.FakeDestNamespace)
ns.SetNamespace(test.FakeDestNamespace)
ns.SetAnnotations(map[string]string{"bar": "bat"})

app := newFakeApp()
app.Spec.SyncPolicy.ManagedNamespaceMetadata = &argoappv1.ManagedNamespaceMetadata{
Labels: map[string]string{
"foo": "bar",
},
Annotations: map[string]string{
"foo": "bar",
},
}
app.Status.OperationState = &argoappv1.OperationState{
SyncResult: &argoappv1.SyncOperationResult{},
}

liveNs := ns.DeepCopy()
liveNs.SetAnnotations(nil)

data := fakeData{
manifestResponse: &apiclient.ManifestResponse{
Manifests: []string{toJSON(t, liveNs)},
Namespace: test.FakeDestNamespace,
Server: test.FakeClusterURL,
Revision: "abc123",
},
managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
kube.GetResourceKey(ns): ns,
},
}
ctrl := newFakeController(&data)
sources := make([]argoappv1.ApplicationSource, 0)
sources = append(sources, app.Spec.GetSource())
revisions := make([]string, 0)
revisions = append(revisions, "")
compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false)
assert.NotNil(t, compRes)
assert.NotNil(t, compRes.syncStatus)
assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status)
assert.Len(t, compRes.resources, 1)
assert.Len(t, compRes.managedResources, 1)
assert.NotNil(t, compRes.diffResultList)
assert.Len(t, compRes.diffResultList.Diffs, 1)

result := NewNamespace()
assert.NoError(t, json.Unmarshal(compRes.diffResultList.Diffs[0].PredictedLive, result))

labels := result.GetLabels()
delete(labels, "kubernetes.io/metadata.name")

assert.Equal(t, map[string]string{}, labels)
// Manifests override definitions in managedNamespaceMetadata
assert.Equal(t, map[string]string{"bar": "bat"}, result.GetAnnotations())
assert.Len(t, app.Status.Conditions, 0)
}

// TestCompareAppStateNamespaceMetadata tests comparison when managed namespace metadata differs to live
func TestCompareAppStateNamespaceMetadata(t *testing.T) {
ns := NewNamespace()
ns.SetName(test.FakeDestNamespace)
ns.SetNamespace(test.FakeDestNamespace)
ns.SetAnnotations(map[string]string{"bar": "bat"})

app := newFakeApp()
app.Spec.SyncPolicy.ManagedNamespaceMetadata = &argoappv1.ManagedNamespaceMetadata{
Labels: map[string]string{
"foo": "bar",
},
Annotations: map[string]string{
"foo": "bar",
},
}
app.Status.OperationState = &argoappv1.OperationState{
SyncResult: &argoappv1.SyncOperationResult{},
}

data := fakeData{
manifestResponse: &apiclient.ManifestResponse{
Manifests: []string{},
Namespace: test.FakeDestNamespace,
Server: test.FakeClusterURL,
Revision: "abc123",
},
managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
kube.GetResourceKey(ns): ns,
},
}
ctrl := newFakeController(&data)
sources := make([]argoappv1.ApplicationSource, 0)
sources = append(sources, app.Spec.GetSource())
revisions := make([]string, 0)
revisions = append(revisions, "")
compRes := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false)
assert.NotNil(t, compRes)
assert.NotNil(t, compRes.syncStatus)
assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status)
assert.Len(t, compRes.resources, 1)
assert.Len(t, compRes.managedResources, 1)
assert.NotNil(t, compRes.diffResultList)
assert.Len(t, compRes.diffResultList.Diffs, 1)

result := NewNamespace()
assert.NoError(t, json.Unmarshal(compRes.diffResultList.Diffs[0].PredictedLive, result))

labels := result.GetLabels()
delete(labels, "kubernetes.io/metadata.name")

assert.Equal(t, map[string]string{"foo": "bar"}, labels)
assert.Equal(t, map[string]string{"argocd.argoproj.io/sync-options": "ServerSideApply=true", "bar": "bat", "foo": "bar"}, result.GetAnnotations())
assert.Len(t, app.Status.Conditions, 0)
}

// TestCompareAppStateNamespaceMetadataIsTheSame tests comparison when managed namespace metadata is the same
func TestCompareAppStateNamespaceMetadataIsTheSame(t *testing.T) {
app := newFakeApp()
Expand Down

0 comments on commit 21fc0d0

Please sign in to comment.