Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix absent namespaces for cluster-scoped resources in hierarchies #726

Merged
merged 1 commit into from
Mar 29, 2021

Conversation

nolar
Copy link
Owner

@nolar nolar commented Mar 29, 2021

In #671, a case with cluster-scoped resources (which have no namespaces) was forgotten.

This PR fixes it: absent namespace in the owner object is not an error now, None is assigned as a namespace in children (as it was before the fix).


Besides, a little optimization for clarity: the current contextual owner is looked up ONLY if namespace= is really unset, not when it is passed as None — for the case when adjust_namespace(…, namespace=None) is called from adopt() where the owner lookup is already performed.

The same distinguishing logic of None-vs-unset is applied for "name" harmonization — also a string that can be sometimes absent in the owner — at least, in theory.

The same for labels: passing None is a sign of an error (e.g., improper .get('labels')?): labels cannot be None, but they can be {} (as a default value in adopt()).


There are almost no changes in the tests because the contracts/signatures/behaviour do not change (except for namespaces of cluster-scoped resources), only the implementation is changed.

fixes #725

Signed-off-by: Sergey Vasilyev <sergey.vasilyev@zalando.de>
@nolar nolar added the bug Something isn't working label Mar 29, 2021
@nolar nolar merged commit 4325ec3 into release/1.30 Mar 29, 2021
@nolar nolar deleted the fix-cluster-scoped-hierarchies branch March 29, 2021 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant