Added test for copy-ns-labels policy

[siddhikhapare]

Related Issue(s)

Partially addresses #950

Description

added chainsaw test for copy-namespace-labels policy to test working of policy for outside and within ns with owner and multiple additional labels.

Checklist

• I have read the policy contribution guidelines.
• I have added test manifests and resources covering both positive and negative tests that prove this policy works as intended.
• [] I have added the artifacthub-pkg.yml file and have verified it is complete and correct.

[siddhikhapare]

@chipzoller Could you help me in resolving this issue of test failures?

[chipzoller]

Sure. Forget the failures for right now. I see you are missing some fundamental points of what this policy does.
Hint: Look at what the policy matches on versus what you're testing. Do they all make sense?

[siddhikhapare]

Sure. Forget the failures for right now. I see you are missing some fundamental points of what this policy does. Hint: Look at what the policy matches on versus what you're testing. Do they all make sense?

Thank you for your helpful response. As per my understanding ,in order to validate both conditions of policy simultaneously. I have to define both rules in single resource but I have created resources with separated methods of policy. I realize now that I was mistaken here. Could you please share your thoughts on this?

[chipzoller]

Not sure what you're saying. Look at my hint, then look at the policy match block, then look at all the resources you're trying to create as part of the test. Something is off there. What is it?

[siddhikhapare]

Not sure what you're saying. Look at my hint, then look at the policy match block, then look at all the resources you're trying to create as part of the test. Something is off there. What is it?

pod resource kind is not included in the match block which I have provided and rules will only apply to deployment resources which will create and manage the associated pods.

[siddhikhapare]

@chipzoller I will update it.

[chipzoller]

Yes, correct. Fix that first then let's look.

[siddhikhapare]

Should use a ClusterRole to grant permissions here for this policy?

[chipzoller]

There should be no need to grant permissions here.

[siddhikhapare]

@chipzoller I've noticed that policy is taking more time to initialize. I tried increasing the sleep duration in chainsaw-test.yaml file but still providing me same error regarding policy status is null which I mentioned here. I guess my system resource is responsible for taking time run this policy. I found this error on kyverno playground while running this policy is ServerError: Variable owner is not defined in the contexteven if I have provided the namespace resource definition with the deployment Kubernetes resource.
Could you please help me in understanding what's causing issue here?

[siddhikhapare]

I assert that owner label is not a static value, so we do not have to specify in values.yaml.
owner label value is fetched dynamically from the namespace's metadata labels using the apiCall and jmesPath in the policy's context.

[chipzoller]

Please look at the failed test log fro your error. You can and should also run this locally on your machine and you will see the same thing. You should always be running tests locally before pushing changes in a PR.

[siddhikhapare]

Please look at the failed test log fro your error. You can and should also run this locally on your machine and you will see the same thing. You should always be running tests locally before pushing changes in a PR.

I have attached passed test output here using chainsaw.

[siddhikhapare]

@chipzoller I tested files locally which have got output as passed tests as expected but here test are failed.

[chipzoller]

Look at the logged output in CI tests here and see why they fail.

[siddhikhapare]

for bad-deployment-with-ouside-ns resource have the kubernetes.io/metadata.name label. According to the policy, this label will not be copied to the deploy. However, other labels from the ns should be copied to the deployments. Therefore, this resource should fail because I didn't copy the owner label from the ns to the deploy. It failed locally once because the API call is taking time to run, but it passed the test once so I removed that resource.

[siddhikhapare]

I think the tests were failing because of the creationTimestamp field, so I removed it . But when I ran the tests on locally, I found that they all passed, whether I included that field or not. You can see the results here output

[chipzoller]

You still don't seem to understand what this policy is about and how to test it. This is a mutation policy with two rules. There is no "bad" resource and no failure should occur. The test is about asserting on what the resulting mutation should look like, for both rules, in different scenarios with applicable resources.

[siddhikhapare]

You still don't seem to understand what this policy is about and how to test it. This is a mutation policy with two rules. There is no "bad" resource and no failure should occur. The test is about asserting on what the resulting mutation should look like, for both rules, in different scenarios with applicable resources.

I found internal error today that failed when calling the webhook "mutate-policy.kyverno.svc". However, according to this note mentioned in Kyverno's documentation https://kyverno.io/docs/writing-policies/mutate/ , Kubernetes disallows changes to certain fields in resources including name, namespace, uid, kind, and apiVersion, therefore you cannot use Kyverno policy to effect any of these fields either during admission or once a resource has been persisted. I thought I needed to change the creationTimestamp field, which indicates when an object was created. and this also mentioned in k8s doc here https://kubernetes.io/docs/concepts/workloads/pods/#pod-update-and-replacement so - Most of the metadata about a Pod is immutable that cannot change the namespace, name, uid, or creationTimestamp fields; the generation field is unique. It only accepts updates that increment the field's current value. This means that these fields can't be modified once they're set.

I did not understand that error correctly so thanks for providing clarification.

[chipzoller]

None of what you're saying matters in this case. A mutation during admission is not subject to these limitations since there is no "existing" resource. New resources being created are mutated before being persisted. All that's needed is to check the resource after it is created.

[siddhikhapare]

All test passed as you have explained @chipzoller. Thanks for your guidance.

[siddhikhapare]

I think this test might be showing there's a issue with resources timeout. Could you please provide your insights regarding this issue it would be helpful for me. should I use replicas : 0 for few resource for not to create pods if cluster have limit to create pod on server?

[chipzoller]

Please also use an error step to fail if the kubernetes.io/metadata.name is found on at least one Deployment.

[siddhikhapare]

@chipzoller Sorry for delay in resolving issue. I was out of station. I have checked test here that kubernetes.io/metadata.name label for only one resource. I have checked for all resource definition also to matched that label and they should provide error. for this I found this output -

- error:
       resource:
         apiVersion: apps/v1
         kind: Deployment
         metadata:
           labels:
             kubernetes.io/metadata.name:  within-ns
          namespace: within-ns

[chipzoller]

So are you saying the previous comment is resolved? Ready for review again?

Also, in the future, there's no need to take a screenshot of terminal output. Please just paste the text into a code block here so it's easy to follow the path of communication.

[siddhikhapare]

kubernetes.io/metadata.name

yes I believe that all test passed for that change what you have suggested to add.

[siddhikhapare]

I have added whole resource definition in test file if that kubernetes.io/metadata.name label found with value of the label is the namespace name then that resource should provide error before applying policy because after applying policy that label should exclude from that resource.

[JimBugwadia]

@siddhikhapare - has this requested change been addressed?

#967 (review)

[siddhikhapare]

@chipzoller I got policy lint error but my all test passed locally.

[siddhikhapare]

@chipzoller Should I update the digest hash string in the artifacthub-pkg.yml file? I believe it should be updated when the policy definition changes. I'm not sure what the exact lint error is. Could you help me understand it better?

[siddhikhapare]

I found this error Use with "kasten-validate-ns-by-preset-label" policy to require "dataprotection" labeling on new namespaces. while testing diff --git a/kasten/kasten-generate-policy-by-preset-label/artifacthub-pkg.yml b/kasten/kasten-generate-policy-by-preset-label/artifacthub-pkg.yml

[chipzoller]

Sent #1036 to fix.

[chipzoller]

Nice work, thanks.