adds e2e tests for validating webhook #772
Conversation
Codecov Report
@@ Coverage Diff @@
## master #772 +/- ##
==========================================
+ Coverage 73.78% 74.87% +1.08%
==========================================
Files 110 110
Lines 3285 3315 +30
==========================================
+ Hits 2424 2482 +58
+ Misses 677 648 -29
- Partials 184 185 +1
|
patilpankaj212
requested review from
devang-gaur,
jlk and
kanchwala-yusuf
and removed request for
jlk
2. fix e2e tests after new policy addition
2. build file changes 3. add copyright
pkg/http-server/webhook-scan.go
Outdated
| @@ -31,6 +31,7 @@ import ( | |||
|
|
|||
| // validateK8SWebhook handles the incoming validating admission webhook from kubernetes API server | |||
| func (g *APIHandler) validateK8SWebhook(w http.ResponseWriter, r *http.Request) { | |||
| zap.S().Info("handle: validating webhook request") | |||
There was a problem hiding this comment.
I honestly feel this should be logged in debug mode level and not in info
There was a problem hiding this comment.
Also, for logging in the incoming requests, we should work on #784
There was a problem hiding this comment.
it is an informational log and hence to be logged in info.
Also, I had a discussion with @kanchwala-yusuf that we should handle it in separate PR. And, any other logging related changes can be taken up with #784.
|
|
||
| # install kind | ||
| install-kind: | ||
| ./scripts/install-kind.sh |
There was a problem hiding this comment.
can we make this script accomodating for multiple OS types? This script currently only cover linux amd64 as of now
There was a problem hiding this comment.
if not, then we should not include it in makefile atleast
There was a problem hiding this comment.
installing kind is a required step of the validate job in our github pipeline.
also, all are scripts are for linux.
| limitations under the License. | ||
| */ | ||
|
|
||
| package validatingwebhook |
There was a problem hiding this comment.
this package name is confusing. all of the functions inside are utilities that deal with kubernetes and kind clusters. so naming the package validatingwebhook is a bit misleading. maybe just call it util or kube_util
|
@patilpankaj212 I have sent a PR on your branch which changes the code to use a deterministic kubeconfig. |
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -134,6 +135,7 @@ func (g *APIHandler) getLogs(w http.ResponseWriter, r *http.Request) { | |||
| } | |||
|
|
|||
| func (g *APIHandler) getLogByUID(w http.ResponseWriter, r *http.Request) { | |||
| zap.S().Info("handle: validating webhook's get log by uid request") | |||
There was a problem hiding this comment.
Forgot to make this Debug?
| @@ -134,6 +135,7 @@ func (g *APIHandler) getLogs(w http.ResponseWriter, r *http.Request) { | |||
| } | |||
|
|
|||
| func (g *APIHandler) getLogByUID(w http.ResponseWriter, r *http.Request) { | |||
| zap.S().Info("handle: validating webhook's get log by uid request") | |||
There was a problem hiding this comment.
| zap.S().Info("handle: validating webhook's get log by uid request") | |
| zap.S().Debug("handle: validating webhook's get log by uid request") |
| } | ||
|
|
||
| // getK8sClient creates a kubernetes clientset with default config path | ||
| func (k *KubernetesClient) getK8sClient() (*kubernetes.Clientset, error) { |
There was a problem hiding this comment.
Do we need to associate this method with KubernetesClient object? I can't see this method using any context from the KubernetesClient object.
There was a problem hiding this comment.
Maybe shift it to a util file
| func NewKubernetesClient() (*KubernetesClient, error) { | ||
| kubernetesClient := new(KubernetesClient) | ||
| var err error | ||
| kubernetesClient.client, err = kubernetesClient.getK8sClient() |
There was a problem hiding this comment.
This line looks wierd to me. object.property = object.GetProperty( ).
rather can we just do object.InitiateProperty( ) ?
In this case, kubernetes.InitiateK8sClient( ) .
Or let's remove the association betweek KubernetesClient type and getK8sClient( ) method at all.
No description provided.