Access check & logging framework refactor & update code-gen version

[shivdudhani]

• use a logging interface for logs (fixes Update the logging framework #742)
• update code-generator version and instructions to run (fixes [BUG] Update instructions for code-generator #743)
• add selfReviewAccess check for generate rules. (fixes [BUG] Policy validation for custom Kyverno cluster / roles #714)

[shivdudhani]

Added instructions to run code-generator at https://github.com/nirmata/kyverno/wiki/Building

[shivdudhani]

Pending: Add instructions to change the verbosity of logs as a command-line argument.

[realshuting]

It's a huge PR, gonna need help from @shravanshetty1 to review :)

[realshuting]

Pending: Add instructions to change the verbosity of logs as a command-line argument.

Is this going to be added to this PR?

[shivdudhani]

The documentation for setting in a part of Wiki, so will not be in this PR.

• I just need to add some documentation and examples for logging in Wiki.

[realshuting]

@shivdudhani Can you please add the link to Wiki once you have it?

[shivdudhani]

Updated logging docs:
https://github.com/nirmata/kyverno/wiki/Logging-Conventions

[shravanshetty1]

fixes #649

[shravanshetty1]

@shivdudhani @realshuting
I feel like instead of injecting loggers into each and every function - we should have a logger global variable for each package. Loggers are generally used this way.

We are still able to say who generated the logs since each package will have a log with unique name.

[shivdudhani]

@shivdudhani @realshuting
I feel like instead of injecting loggers into each and every function - we should have a logger global variable for each package. Loggers are generally used this way.

We are still able to say who generated the logs since each package will have a log with unique name.

In this PR we are just moving to a new framework for logging. The previous implementation was not modified. Previously we didn't have options to provide context for debugging, but with this we do.

Log provides withName to give context for the logs, and the reason for passing them from root to bottom function is to provide a complete stack trace for the error log.
The logs are not defined per package but per component.

Instead of using global variables in a package, it would be better to have each struct manage its own to logs. If the logs are not to be passed, then the generic/base implementation of the log can be used.

The pkg sigs.k8s.io/controller-runtime/pkg/log exposes a variable Log which refers to the base logger implementation.

[shravanshetty1]

@shivdudhani @realshuting
I feel like instead of injecting loggers into each and every function - we should have a logger global variable for each package. Loggers are generally used this way.
We are still able to say who generated the logs since each package will have a log with unique name.

In this PR we are just moving to a new framework for logging. The previous implementation was not modified. Previously we didn't have options to provide context for debugging, but with this we do.

Log provides withName to give context for the logs, and the reason for passing them from root to bottom function is to provide a complete stack trace for the error log.
The logs are not defined per package but per component.

Instead of using global variables in a package, it would be better to have each struct manage its own to logs. If the logs are not to be passed, then the generic/base implementation of the log can be used.

The pkg sigs.k8s.io/controller-runtime/pkg/log exposes a variable Log which refers to the base logger implementation.

That makes sense.
If we are going with the current implementation, i feel we should use base log for all functions and we should only use injected logs for methods. If we NEED to inject logs to a function we should make it a method. Pointer receiver methods have no overhead over calling a function directly.

We could make this an ongoing change like godoc comments - this has to be documented in the contributor docs though.

[shivdudhani]

@shivdudhani @realshuting
I feel like instead of injecting loggers into each and every function - we should have a logger global variable for each package. Loggers are generally used this way.
We are still able to say who generated the logs since each package will have a log with unique name.

In this PR we are just moving to a new framework for logging. The previous implementation was not modified. Previously we didn't have options to provide context for debugging, but with this we do.
Log provides withName to give context for the logs, and the reason for passing them from root to bottom function is to provide a complete stack trace for the error log.
The logs are not defined per package but per component.
Instead of using global variables in a package, it would be better to have each struct manage its own to logs. If the logs are not to be passed, then the generic/base implementation of the log can be used.
The pkg sigs.k8s.io/controller-runtime/pkg/log exposes a variable Log which refers to the base logger implementation.

That makes sense.
If we are going with the current implementation, i feel we should use base log for all functions and we should only use injected logs for methods. If we NEED to inject logs to a function we should make it a method. Pointer receiver methods have no overhead over calling a function directly.

We could make this an ongoing change like godoc comments - this has to be documented in the contributor docs though.

I didn't understand the comment injected logs for methods, Can you explain with an example?

A developer can use the logging framework they want, as long as the logs provides details for debugging. Storing the logger interface as a variable in a struct or passed as a function parameter is up to the design.

The idea of building context is based on the kubebuilder controller-runtime patterns.

[shravanshetty1]

@shivdudhani @realshuting
I feel like instead of injecting loggers into each and every function - we should have a logger global variable for each package. Loggers are generally used this way.
We are still able to say who generated the logs since each package will have a log with unique name.

In this PR we are just moving to a new framework for logging. The previous implementation was not modified. Previously we didn't have options to provide context for debugging, but with this we do.
Log provides withName to give context for the logs, and the reason for passing them from root to bottom function is to provide a complete stack trace for the error log.
The logs are not defined per package but per component.
Instead of using global variables in a package, it would be better to have each struct manage its own to logs. If the logs are not to be passed, then the generic/base implementation of the log can be used.
The pkg sigs.k8s.io/controller-runtime/pkg/log exposes a variable Log which refers to the base logger implementation.

That makes sense.
If we are going with the current implementation, i feel we should use base log for all functions and we should only use injected logs for methods. If we NEED to inject logs to a function we should make it a method. Pointer receiver methods have no overhead over calling a function directly.
We could make this an ongoing change like godoc comments - this has to be documented in the contributor docs though.

I didn't understand the comment injected logs for methods, Can you explain with an example?

A developer can use the logging framework they want, as long as the logs provides details for debugging. Storing the logger interface as a variable in a struct or passed as a function parameter is up to the design.

The idea of building context is based on the kubebuilder controller-runtime patterns.

we can add logger in reciever of methods (functions with reciever arguments). However for normal function (no reciever arguments) we should use base logs.