map[] in error output exposes secret data in last-applied-annotation
& patch error
Invalid secrets with stringData exposes the secret values in diff. Attempt a
normalization to prevent it.
Refactor stringData to data conversion to eliminate code duplication
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
* fix: Server side diff now works correctly with some fields removal
Helps with https://github.com/argoproj/argo-cd/issues/20792
Removed and modified sets may only contain the fields that changed, not including key fields like "name". This can cause merge to fail, since it expects those fields to be present if they are present in the predicted live.
Fortunately, we can inspect the set and derive the key fields necessary. Then they can be added to the set and used during a merge.
Also, have a new test which fails before the fix, but passes now.
Failure of the new test before the fix
```
Error: Received unexpected error:
error removing non config mutations for resource Deployment/nginx-deployment: error reverting webhook removed fields in predicted live resource: .spec.template.spec.containers: element 0: associative list with keys has an element that omits key field "name" (and doesn't have default value)
Test: TestServerSideDiff/will_test_removing_some_field_with_undoing_changes_done_by_webhook
```
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
* Use new version of structured merge diff with a new option
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
* Add DCO
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
* Try to fix sonar exclusions config
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
---------
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
* fix: Ability to disable Server Side Apply on individual resource level
Signed-off-by: pashakostohrys <pavel@codefresh.io>
* fix: Ability to disable Server Side Apply on individual resource level
Signed-off-by: pashakostohrys <pavel@codefresh.io>
---------
Signed-off-by: pashakostohrys <pavel@codefresh.io>
* chore: avoid unnecessary json marshal
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* more tests
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* refactor test to satisfy sonarcloud
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---------
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* chore: More optimal IterateHierarchyV2 and iterateChildrenV2 [#600]
Closes#600
The existing (effectively v1) implementations are suboptimal since they don't construct a graph before the iteration. They search for children by looking at all namespace resources and checking `isParentOf`, which can give `O(tree_size * namespace_resources_count)` time complexity. The v2 algorithms construct the graph and have `O(namespace_resources_count)` time complexity. See more details in the linked issues.
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
* improvements to graph building
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* use old name
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* chore: More optimal IterateHierarchyV2 and iterateChildrenV2 [#600]
Closes#600
The existing (effectively v1) implementations are suboptimal since they don't construct a graph before the iteration. They search for children by looking at all namespace resources and checking `isParentOf`, which can give `O(tree_size * namespace_resources_count)` time complexity. The v2 algorithms construct the graph and have `O(namespace_resources_count)` time complexity. See more details in the linked issues.
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
* finish merge
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* chore: More optimal IterateHierarchyV2 and iterateChildrenV2 [#600]
Closes#600
The existing (effectively v1) implementations are suboptimal since they don't construct a graph before the iteration. They search for children by looking at all namespace resources and checking `isParentOf`, which can give `O(tree_size * namespace_resources_count)` time complexity. The v2 algorithms construct the graph and have `O(namespace_resources_count)` time complexity. See more details in the linked issues.
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
* discard unneeded copies of child resources as we go
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* remove unnecessary comment
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* make childrenByUID sparse
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* eliminate duplicate map
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* fix comment
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* add useful comment back
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* use nsNodes instead of dupe map
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* remove unused struct
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* skip invalid APIVersion
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---------
Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* sync.Reconcile: guard against incomplete discovery
When Reconcile performs its logic to compare the desired state (target
objects) against the actual state (live objects), it looks up each live
object based on a key comprised of data from the target object: API
group, API kind, namespace, and name. While group, kind, and name will
always be accurate, there is a chance that the value for namespace is
not. If a cluster-scoped target object has a namespace (because it
incorrectly has a namespace from its source) or the namespace parameter
passed into the Reconcile method has a non-empty value (indicating a
default value to use on namespace-scoped objects that don't have it set
in the source), AND the resInfo ResourceInfoProvider has incomplete or
missing API discovery data, the call to IsNamespacedOrUnknown will
return true when the information is unknown. This leads to the key being
incorrect - it will have a value for namespace when it shouldn't. As a
result, indexing into liveObjByKey will fail. This failure results in
the reconciliation containing incorrect data: there will be a nil entry
appended to targetObjs when there shouldn't be.
Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>
* Address code review comments
Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>
---------
Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>
* fix: deduplicate OpenAPI definitions for GVKParser
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* do the thing that was the whole point
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* more logs
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* don't uniquify models
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* schema for both
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* more logs
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* fix logic
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* better tainted gvk handling, better docs, update mocks
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* add a test
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* improvements from comments
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
---------
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
* Prune resources in reverse of sync wave order
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
* Use waveOverride var instead of directly patching live obj
Directly patching live objs results into incorrect wave ordering
as the new wave value from live obj is used to perform reordering during next sync
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
---------
Signed-off-by: Siddhesh Ghadi <sghadi1203@gmail.com>
* Revert "feat: retry with client side dry run if server one was failed (#548)"
This reverts commit c0c2dd1f6f.
Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
* Revert "fix(server): use server side dry run in case if it is server side apply (#546)"
This reverts commit 4a5648ee41.
Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
* Fixed the logic to disable server side apply if it is a dry run
Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
* Added more values in the log message for better debugging
Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
* Fixed compilation error
Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
* Written an inline fn to get string value of dry-run strategy
Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
* Added comment as requested with reference to the issue number
Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
---------
Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
Co-authored-by: Leonardo Luz Almeida <leoluz@users.noreply.github.com>
* feat: retry with client side dry run if server one was failed
Signed-off-by: pashakostohrys <pavel@codefresh.io>
* feat: retry with client side dry run if server one was failed
Signed-off-by: pashakostohrys <pavel@codefresh.io>
* feat: retry with client side dry run if server one was failed
Signed-off-by: pashakostohrys <pavel@codefresh.io>
---------
Signed-off-by: pashakostohrys <pavel@codefresh.io>
* fix: use server side dry run in case if it is server side apply
Signed-off-by: pashakostohrys <pavel@codefresh.io>
* fix: use server side dry run in case if it is server side apply
Signed-off-by: pashakostohrys <pavel@codefresh.io>
---------
Signed-off-by: pashakostohrys <pavel@codefresh.io>
When doing `kubectl replace`, namespaces should not be affected. Fixes
argoproj/argo-cd#12810 and argoproj/argo-cd#12539.
Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
* fix: avoid acquiring lock on mutex and semaphore at the same time to prevent deadlock
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
* apply reviewer notes
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
---------
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
