Avoid conflicts when deployed by ArgoCD #147
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mihaigalos
reviewed
Oct 11, 2024
mihaigalos
reviewed
Oct 11, 2024
| return V1ObjectMeta( | ||
| name=name, | ||
| namespace=namespace, | ||
| annotations=_annotations, | ||
| labels=_labels, | ||
| annotations=dict(_annotations), |
There was a problem hiding this comment.
filter_dict already returns a dict, no need for a typecast. Or?
Suggested change
| annotations=dict(_annotations), | |
| annotations=_annotations, |
There was a problem hiding this comment.
No, actually it returns a generator object, which has to be unfolded into a dict before passing to V1ObjectMeta constructor. The type annotation of filter_dict is wrong. Nice catch! I'll fix that!
There was a problem hiding this comment.
Yes, of course. Makes sense.
I see you corrected the logic.
Any chance you can add some u-tests as well?
There was a problem hiding this comment.
@mihaigalos working on it
harmathy
force-pushed
the
do-not-copy-app.kubernetes.io-labels
branch
from
d3831a6
to
1c5ce10
Compare
This change introduces a black list for labels like the one already existing for metadata. The list contains one entry for the prefix "app.kubernetes.io". The label "app.kubernetes.io/instance" is per default used by ArgoCD to track resources, which causes copied Secrets to be potentially deleted again by ArgoCD. Also labels with prefix "app.kubernetes.io" are in general very specific to the resources in their respective namespace and therefore shouldn't probably be automatically copied to resources in other namespaces anyway. In order to avoid code duplication the filtering is delegated to an embedded function filter_dict. Signed-off-by: Max Harmathy <[email protected]>
harmathy
force-pushed
the
do-not-copy-app.kubernetes.io-labels
branch
from
1c5ce10
to
60d5abc
Compare
This tests the create_secret_metadata function against some combinations of annotations and labels. Signed-off-by: Max Harmathy <[email protected]>
harmathy
force-pushed
the
do-not-copy-app.kubernetes.io-labels
branch
from
a22358e
to
2330b7d
Compare
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change introduces a black list for labels like the one already existing for metadata.
The list contains one entry for the prefix "app.kubernetes.io". The label "app.kubernetes.io/instance" is per default used by ArgoCD to track resources, which causes copied Secrets to be potentially deleted again by ArgoCD. Also labels with prefix "app.kubernetes.io" are in general very specific to the resources in their respective namespace and therefore shouldn't probably be automatically copied to resources in other namespaces anyway.
In order to avoid code duplication the filtering is delegated to an embedded function filter_dict.
closes #146