Fix hierarchy conflict validation excluding non-propagated objects

[joe2far]

• Resolves issue with hierarchy conflict validation
• Excludes objects/resources not being propagated

for example, (behaviour with current logic):
hnc-parent namespace has kube-root-ca.crt configmap with applied propagate.hnc.x-k8s.io/none=true annotation
when creating a child namespace this will result in the following validation error, even though the parent configmap is not going to be propagated to the child namespace

kubectl apply -f child-ns.yaml
kubectl apply -n hnc-child  -f child-ns-hierarchy.yaml
Error from server (Conflict): error when creating "child-ns-hierarchy.yaml": admission webhook "hierarchyconfigurations.hnc.x-k8s.io" denied the request: Cannot update hierarchy because it would overwrite the following object(s):
  * Namespace "hnc-child": kube-root-ca.crt (/v1, Kind=ConfigMap)

with this PR, the parent annotated configmap resource would be excluded from this check and there is no validation error

[k8s-ci-robot]

Welcome @joe2far!

It looks like this is your first PR to kubernetes-sigs/hierarchical-namespaces 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/hierarchical-namespaces has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @joe2far. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[joe2far]

Adding info on tested scenarios ->

TESTED:

### with latest hnc version, errors seen for resources with propagate none annotation ->
$ kubectl apply -n hnc-child  -f child-ns-hierarchy.yaml
Error from server (Conflict): error when creating "child-ns-hierarchy.yaml": admission webhook "hierarchyconfigurations.hnc.x-k8s.io" denied the request: Cannot update hierarchy because it would overwrite the following object(s):
  * Namespace "hnc-child": kube-root-ca.crt (/v1, Kind=ConfigMap)
  * Namespace "hnc-child": istio-ca-root-cert (/v1, Kind=ConfigMap)

------
### now after updating hnc image from this PR
### creation works without issue (and no validation errors)
kubectl apply -n hnc-child  -f child-ns-hierarchy.yaml
hierarchyconfiguration.hnc.x-k8s.io/hierarchy created

### after removing the annotation the conflict validation errors return as expected
kubectl -n hnc-parent annotate cm istio-ca-root-cert propagate.hnc.x-k8s.io/none- # remove annotation
kubectl apply -n hnc-child  -f child-ns-hierarchy.yaml
Error from server (Conflict): error when creating "child-ns-hierarchy.yaml": admission webhook "hierarchyconfigurations.hnc.x-k8s.io" denied the request: Cannot update hierarchy because it would overwrite the following object(s):
  * Namespace "hnc-child": istio-ca-root-cert (/v1, Kind=ConfigMap)

[adrianludwin]

Nice change!

[adrianludwin]

/ok-to-test

[adrianludwin]

Can you please also add a short note about testing to the commit message? E.g.:

Tested: got a spurious error on an unpropagated object with this change, which is fixed with this change. Also added a unit test that fails without this change but passes with it.

For that second part, that's always a good sanity check if you didn't already try it - just make sure the test fails if you break the feature.

[joe2far]

Can you please also add a short note about testing to the commit message? E.g.:

Tested: got a spurious error on an unpropagated object with this change, which is fixed with this change. Also added a unit test that fails without this change but passes with it.

For that second part, that's always a good sanity check if you didn't already try it - just make sure the test fails if you break the feature.

• I have updated the commit message with details of testing and unit test
• I did validate that the unit test also fails, when the added getConflictingObjectsOfType logic is removed

[adrianludwin]

/lgtm
/approve
/hold
/assign @rjbez17

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianludwin, joe2far

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [adrianludwin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[adrianludwin]

@rjbez17 lmk if you have any concerns about this

/hold cancel

[rjbez17]

Sorry I meant to respond earlier. Lgtm as well.