[WIP] fix: add changes to the Remediator to revert Config Sync metadata changes on drift #1486
Conversation
Camila-B
commented
Nov 9, 2024
Camila-B
commented
- Makes changes to the Remediator to revert Config Sync metadata changes. Config Sync metadata should not be modified by users
- Depends on fix: the ignore mutation annotation in the applier #1477
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Camila-B
changed the title
| core.Annotation(metadata.LifecycleMutationAnnotation, metadata.IgnoreMutation), | ||
| ) | ||
| nt.Must(rootSyncGitRepo.Add("ns.yaml", nsObj)) | ||
| nt.MustKubectl("apply", "-f", filepath.Join(rootSyncGitRepo.Root, "ns.yaml")) |
There was a problem hiding this comment.
Ditto. Suggesting using nt.KubeClient.Apply.
Also we need a cleanup block to delete the object in case the test fails halfway.
There was a problem hiding this comment.
I replaced MustKubectl with nt.KubeClient.Apply. However, the test now fails. I explained the issue here #1477 (comment)
| @@ -781,9 +781,9 @@ func TestDriftKubectlAnnotateManagedFieldWithIgnoreMutationAnnotation(t *testing | |||
| )) | |||
| }) | |||
| tg.Go(func() error { | |||
| // Note: this proves that the applier does not currently honor the ignore-mutation annotation. | |||
There was a problem hiding this comment.
There are a few ignore-mutation related test cases in this file. Can it be covered by the new ignore_mutation_test.go? It would be good to remove those duplicated tests.
|
|
||
| applier.Apply(context.Background(), eventHandler, resources) | ||
|
|
||
| testutil.AssertEqual(t, tc.expectedObjsToApply, fakeKptApplier.objsToApply) |
There was a problem hiding this comment.
Try to use fakeClient.Check(t, tc.expectedServerObjs...) so that we don't need to declare expectedObjsToApply as object.UnstructuredSet. Instead, it can be declared as []client.Object, which is simpler.
There was a problem hiding this comment.
Right now, the objects are not actually applied to the fake server when the fake KptApplier is called. Should the fake KptApplier's Run fn be modified to actually apply the objects? Or should a function be added to the fake client that converts a given object to the scheme-preferred version so that it can be used in the tests?
Camila-B
force-pushed
the
ignore-mutation-remediator
branch
from
a729a16
to
d8946d4
Compare
This reverts commit 713be94.
Camila-B
force-pushed
the
ignore-mutation-remediator
branch
from
5937514
to
2bb6284
Compare
