Skip to content

Conversation

MeNsaaH
Copy link
Collaborator

@MeNsaaH MeNsaaH commented Dec 18, 2024

Add support for basic kyverno policies. This uses the kyverno cli apply without any extra arguments

kyverno apply -r <ARGOCD_MANIFEST> /path/to/policies

Policies with variables will fail because they need an extra values.file config to be passed which is not currently supported.

Copy link

Temporary image available at ghcr.io/zapier/kubechecks:0.0.0-pr333.

Comment on lines 35 to 40
for _, manifest := range appManifests {
if _, err := tempFile.WriteString(manifest + "\n"); err != nil {
log.Error().Err(err).Msg("Failed to write manifest to temporary file")
return msg.Result{}, err
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need --- to seperate the manifests from each other here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's right! Nice catch.
FYI, this is still in progress. So, it's not fully ready

Comment on lines 61 to 66
var cr msg.Result
if output.Len() == 0 {
cr.State = pkg.StateWarning
} else {
cr.State = pkg.StateSuccess
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's no output, then it failed? This seems ... strange, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more like a dummy script to just see what happens.

We'll need to parse the output to know if it failed or not.

Comment on lines 71 to 75
// -- kyverno
EnableKyvernoChecks bool `mapstructure:"enable-kyverno-checks"`
KyvernoPoliciesLocation []string `mapstructure:"kyverno-policies-location"`
KyvernoPoliciesPaths []string `mapstructure:"kyverno-policies-paths"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add a WorstKyvernoState pkg.CommitState field, which would allow people to either warn or fail depending on configuration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! That'll be added

Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
@MeNsaaH MeNsaaH force-pushed the kyverno-policies-support branch from 6c8f707 to 0ac7b28 Compare January 2, 2025 13:49
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
@MeNsaaH MeNsaaH force-pushed the kyverno-policies-support branch from d522caf to 954a0b9 Compare January 8, 2025 16:38
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
@MeNsaaH MeNsaaH marked this pull request as ready for review January 10, 2025 12:42
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
Signed-off-by: Mmadu Manasseh <manasseh.mmadu@zapier.com>
@zapier-sre-bot
Copy link
Collaborator

Mergecat's Review

Click to read mergecats review!

😼 Mergecat review of pkg/git/repo.go

-	log.Info().Msg("repo has been cloned")
+	log.Info().Str("repo", r.CloneURL).Msg("repo has been cloned")

Feedback & Suggestions:

  1. Enhanced Logging Context: The addition of Str("repo", r.CloneURL) to the log message provides more context, which is beneficial for debugging and tracing. This is a good improvement. However, consider adding more context if available, such as the branch name or directory, to make the log entry even more informative.

  2. Security Consideration: Ensure that r.CloneURL does not contain sensitive information such as credentials. If it does, consider masking or omitting such details from the logs to prevent potential security issues.

  3. Consistency: If similar logging enhancements are made elsewhere in the code, ensure consistency in the logging format and information provided.


😼 Mergecat review of charts/kubechecks/Chart.yaml

@@ -1,7 +1,7 @@
 apiVersion: v2
 name: kubechecks
 description: A Helm chart for kubechecks
-version: 0.5.3
+version: 0.5.4
 type: application
 maintainers:
   - name: zapier

Feedback & Suggestions: The version bump from 0.5.3 to 0.5.4 is appropriate if there are backward-compatible bug fixes or minor improvements. Ensure that the changes in the codebase reflect this version update. Additionally, consider updating the description or adding a changelog entry to document what changes have been made in this new version. 📈


😼 Mergecat review of .github/workflows/on_pull-request_docs.yaml

@@ -24,4 +24,4 @@ jobs:
         run: ./earthly.sh +rebuild-docs
 
       - name: verify that the checked in file has not changed
-        run: ./hacks/exit-on-changed-files.sh "Please run './earthly +rebuild-docs' and commit the results to this PR"
+        run: ./hacks/exit-on-changed-files.sh "Please run './earthly.sh +rebuild-docs' and commit the results to this PR"

Feedback & Suggestions:

  • 🛡️ Security: Ensure that the script exit-on-changed-files.sh is secure and does not execute any untrusted code. This is especially important if the script processes any user input or external data.
  • 📈 Performance: Consider caching dependencies or outputs if the script earthly.sh +rebuild-docs is resource-intensive and run frequently. This can save time and resources in CI/CD pipelines.

😼 Mergecat review of docs/usage.md.tpl

@@ -39,3 +39,6 @@ The full list of supported environment variables is described below:
 {{- range .Options }}
 |`{{ .Env }}`|{{ .Usage }}|{{ if .Default }}`{{ .Default }}`{{ end }}|
 {{- end }}
+
+
+See [integrations](./integrations) for more information on the tools integrated into `kubechecks` .
\ No newline at end of file

Feedback & Suggestions:

  1. Remove Extra Blank Lines: The two additional blank lines before the new sentence are unnecessary and can be removed for cleaner formatting.

  2. Add Newline at End of File: It's a good practice to ensure that files end with a newline character. This can prevent potential issues with some text processing tools and version control systems.

  3. Clarify Link: Ensure that the link to integrations is correct and accessible. If ./integrations is a directory, consider specifying the exact file or document for clarity.


😼 Mergecat review of pkg/aisummary/openai_client.go

@@ -27,7 +27,7 @@ func GetOpenAiClient(apiToken string) *OpenAiClient {
 			client := openai.NewClient(apiToken)
 			openAiClient = &OpenAiClient{client: client, enabled: true}
 		} else {
-			log.Debug().Msg("OpenAI client not enabled")
+			log.Info().Msg("OpenAI client not enabled")
 			openAiClient = &OpenAiClient{enabled: false}
 		}
 	})

Feedback & Suggestions:

  1. Log Level Change: The change from log.Debug() to log.Info() increases the verbosity of the log message when the OpenAI client is not enabled. Consider whether this information is crucial for all users or if it should remain at the debug level to avoid cluttering the logs with less critical information. If the information is essential for operational awareness, then the change is justified. Otherwise, it might be better to keep it at the debug level to maintain a cleaner log output. 📝

😼 Mergecat review of .github/workflows/on_pull_request_go.yaml

@@ -12,6 +12,14 @@ jobs:
   ci-golang:
     runs-on: ubuntu-22.04
     steps:
+      - name: Free Disk Space (Ubuntu)
+        uses: jlumbroso/free-disk-space@main
+        with:
+          # this might remove tools that are actually needed,
+          # if set to "true" but frees about 6 GB
+          tool-cache: false
+          swap-storage: false
+
       - uses: actions/checkout@v3
 
       - uses: djeebus/parse-tool-versions@v2.1

Feedback & Suggestions:

  1. Disk Space Management: 🧹 The addition of the jlumbroso/free-disk-space action is a good step towards managing disk space. However, be cautious with the tool-cache and swap-storage options. If set to true, they might remove necessary tools or swap storage that could be required later in the workflow. Ensure that these settings are appropriate for your use case.

  2. Documentation: 📄 Consider adding a comment or documentation to explain why freeing disk space is necessary in this workflow. This will help future maintainers understand the context and rationale behind this step.

  3. Testing: 🧪 Before merging, test this change thoroughly to ensure that no essential tools are removed and that the workflow still functions as expected.


😼 Mergecat review of .tool-versions

@@ -1,6 +1,6 @@
 earthly 0.8.15
-golang 1.22.7
-golangci-lint 1.62.2
+golang 1.23.4
+golangci-lint 1.63.4
 helm 3.16.3
 helm-cr 1.6.1
 helm-ct 3.11.0

Feedback & Suggestions:

  1. Version Compatibility: Ensure that the updated versions of golang and golangci-lint are compatible with the rest of your toolchain and any existing codebase. Sometimes, newer versions introduce breaking changes or deprecate certain features. 🛠️

  2. Changelog Review: Review the changelogs for golang 1.23.4 and golangci-lint 1.63.4 to understand any new features, bug fixes, or potential issues that might affect your project. 📜

  3. Testing: After updating the versions, run your test suite to ensure that everything works as expected with the new versions. This can help catch any unforeseen issues early. ✅


😼 Mergecat review of pkg/server/server.go

@@ -107,7 +107,7 @@ func (s *Server) ensureWebhooks(ctx context.Context) error {
 
 		if wh == nil {
 			if err = vcsClient.CreateHook(ctx, repo, fullUrl, s.ctr.Config.WebhookSecret); err != nil {
-				log.Info().Err(err).Msgf("failed to create hook for %s:", repo)
+				log.Error().Err(err).Msgf("failed to create hook for %s:", repo)
 			}
 		}
 	}

Feedback & Suggestions:

  1. Log Level Change: The change from log.Info() to log.Error() is appropriate here since failing to create a webhook is indeed an error condition. This change improves the clarity of the log messages by correctly categorizing the severity of the event. 👍

  2. Security Consideration: Ensure that repo and any other sensitive information are sanitized or obfuscated if they contain sensitive data before logging. This helps prevent potential information leakage. 🔒

  3. Error Handling: Consider adding more context to the error message if possible. This can help in debugging by providing more information about the failure. For example, including the error message from err in the log could be beneficial. 🛠️


😼 Mergecat review of charts/kubechecks/templates/clusterrole.yaml

@@ -7,5 +7,5 @@ rules:
     resources: ['applications', 'appprojects', 'applicationsets', 'services']
     verbs: ['get', 'list', 'watch']
   - apiGroups: [''] # The core API group, which is indicated by an empty string
-    resources: ['secrets']
+    resources: ['secrets', 'configmaps']
     verbs: ['get', 'list', 'watch']

Feedback & Suggestions:

  1. Security Consideration: Adding configmaps to the resources with get, list, and watch permissions can expose sensitive information if any ConfigMaps contain such data. Ensure that this change is necessary and that the ConfigMaps do not contain sensitive information. Consider using more granular permissions if possible. 🔒

  2. Documentation Update: If this change is intentional, update any relevant documentation to reflect the new permissions. This helps maintain clarity for future developers or operators who might work with this configuration. 📚

  3. Review Usage: Double-check the usage of ConfigMaps in your application to ensure that the added permissions align with the application's needs. This can prevent over-permissioning, which is a common security risk. 🔍


😼 Mergecat review of Earthfile

@@ -171,7 +171,7 @@ golang-ci-lint:
     WORKDIR /src
     COPY . .
 
-    RUN golangci-lint --timeout 15m run --verbose
+    RUN golangci-lint --timeout 25m run --verbose --exclude-dirs mock
 
 staticcheck-golang:
     ARG --required STATICCHECK_VERSION

Feedback & Suggestions:

  1. Timeout Increase: The timeout for golangci-lint has been increased from 15 minutes to 25 minutes. While this may be necessary for larger codebases, it's important to ensure that this increase is justified. Longer timeouts can lead to longer CI/CD pipeline times, which might affect developer productivity. Consider optimizing the code or the linter configuration to reduce the time needed.

  2. Excluding Directories: The addition of --exclude-dirs mock is a good practice if the mock directory contains generated code or files that should not be linted. However, ensure that excluding this directory does not hide potential issues. It's a good idea to document why certain directories are excluded to maintain clarity for future developers.

  3. Documentation: If these changes are significant for your workflow, consider updating any relevant documentation or comments in the code to reflect why these changes were made. This can help other team members understand the rationale behind the configuration adjustments.


😼 Mergecat review of localdev/terraform/modules/vcs_files/mr5_files/apps/httpdump/overlays/a/kustomization.yaml

@@ -2,7 +2,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 
 resources:
-  - ../../base
+- ../../base
 
-patchesStrategicMerge:
-  - replica-patch.yaml
\ No newline at end of file
+patches:
+- path: replica-patch.yaml

Feedback & Suggestions:

  1. Patch Format Change: The change from patchesStrategicMerge to patches with a path key is a significant alteration. Ensure that this change aligns with the intended behavior, as patchesStrategicMerge and patches can have different implications in Kustomize. Double-check that the new format is supported by your Kustomize version and that it achieves the desired patching effect.

  2. Newline at End of File: The diff indicates there is no newline at the end of the file. It's a good practice to include a newline at the end of files to avoid potential issues with some tools and version control systems. Consider adding a newline to maintain consistency and avoid warnings.


😼 Mergecat review of cmd/controller.go

@@ -79,6 +79,11 @@ var ControllerCmd = &cobra.Command{
 			log.Fatal().Err(err).Msg("failed to process schema locations")
 		}
 
+		log.Info().Strs("locations", cfg.KyvernoPoliciesLocation).Msg("processing kyverno policies locations")
+		if err = processLocations(ctx, ctr, cfg.KyvernoPoliciesLocation); err != nil {
+			log.Fatal().Err(err).Msg("failed to process kyverno policies locations")
+		}
+
 		processors, err := getProcessors(ctr)
 		if err != nil {
 			log.Fatal().Err(err).Msg("failed to create processors")

Feedback & Suggestions:

  1. Error Handling Consistency: The error handling for processing Kyverno policies locations is consistent with the existing pattern, which is good. However, consider whether a log.Fatal is appropriate for this error. If processing Kyverno policies is optional, you might want to log the error and continue execution instead of terminating the program.

  2. Performance Consideration: If processLocations is a potentially time-consuming operation, consider running it in a separate goroutine if the order of execution is not critical. This can improve the startup time of your application.

  3. Configuration Validation: Ensure that cfg.KyvernoPoliciesLocation is validated before being used. If it can be empty or invalid, add a check to handle such cases gracefully.

  4. Logging Clarity: The log message for processing Kyverno policies is clear. Ensure that cfg.KyvernoPoliciesLocation is always populated with meaningful data to avoid logging empty or misleading information.


😼 Mergecat review of pkg/config/config.go

@@ -69,6 +69,10 @@ type ServerConfig struct {
 	// -- preupgrade
 	EnablePreupgrade     bool            `mapstructure:"enable-preupgrade"`
 	WorstPreupgradeState pkg.CommitState `mapstructure:"worst-preupgrade-state"`
+	// -- kyverno
+	EnableKyvernoChecks     bool            `mapstructure:"enable-kyverno-checks"`
+	KyvernoPoliciesLocation []string        `mapstructure:"kyverno-policies-location"`
+	WorstKyvernoState       pkg.CommitState `mapstructure:"worst-kyverno-state"`
 
 	// misc
 	AdditionalAppsNamespaces []string      `mapstructure:"additional-apps-namespaces"`

Feedback & Suggestions:

  1. Consistency in Naming: Ensure that the naming conventions for the new Kyverno fields are consistent with the existing fields. For example, consider whether KyvernoPoliciesLocation should be singular or plural based on how similar fields are named.

  2. Documentation: Add comments or documentation for the new Kyverno fields to explain their purpose and usage. This will help future developers understand the context and functionality of these fields.

  3. Validation: Consider adding validation logic for the new fields, especially if there are specific constraints or expected formats for KyvernoPoliciesLocation or WorstKyvernoState.

  4. Security Considerations: If any of the new fields involve sensitive data (e.g., paths to sensitive policy files), ensure that appropriate security measures are in place to protect this information.

  5. Testing: Ensure that there are tests in place to verify the correct behavior of the new Kyverno configuration options. This includes testing the parsing and handling of these fields.

By addressing these points, you can enhance the robustness and maintainability of the configuration code. 🛠️


😼 Mergecat review of cmd/processors.go

@@ -7,6 +7,7 @@ import (
 	"github.com/zapier/kubechecks/pkg/checks/diff"
 	"github.com/zapier/kubechecks/pkg/checks/hooks"
 	"github.com/zapier/kubechecks/pkg/checks/kubeconform"
+	"github.com/zapier/kubechecks/pkg/checks/kyverno"
 	"github.com/zapier/kubechecks/pkg/checks/preupgrade"
 	"github.com/zapier/kubechecks/pkg/checks/rego"
 	"github.com/zapier/kubechecks/pkg/container"
@@ -57,5 +58,13 @@ func getProcessors(ctr container.Container) ([]checks.ProcessorEntry, error) {
 		})
 	}
 
+	if ctr.Config.EnableKyvernoChecks {
+		procs = append(procs, checks.ProcessorEntry{
+			Name:       "running kyverno check",
+			Processor:  kyverno.Check,
+			WorstState: ctr.Config.WorstKyvernoState,
+		})
+	}
+
 	return procs, nil
 }

Feedback & Suggestions:

  1. Error Handling: Consider adding error handling for the kyverno.Check processor similar to how it's done for the rego.NewChecker. This will ensure that any issues with the Kyverno check are caught and handled gracefully. 🛡️

  2. Consistency: Ensure that the naming conventions for configuration flags and states are consistent across the codebase. For example, EnableKyvernoChecks and WorstKyvernoState should follow the same pattern as other configurations. This improves readability and maintainability. 📚

  3. Documentation: If EnableKyvernoChecks and WorstKyvernoState are new configuration options, ensure they are documented appropriately in the configuration files or documentation to inform users of their purpose and usage. 📝


😼 Mergecat review of cmd/root.go

@@ -119,6 +119,11 @@ func init() {
 		newStringOpts().
 			withDefault("kubechecks again"))
 	stringSliceFlag(flags, "additional-apps-namespaces", "Additional namespaces other than the ArgoCDNamespace to monitor for applications.")
+	boolFlag(flags, "enable-kyverno-checks", "Enable kyverno policy checks.")
+	stringFlag(flags, "kyverno-policies-location", "Sets kyverno policy locations to be used for every check request. This is a git url in either git or http(s) format.")
+	stringFlag(flags, "worst-kyverno-state", "The worst state that can be returned from the kyverno checks.",
+		newStringOpts().
+			withDefault("panic"))
 
 	panicIfError(viper.BindPFlags(flags))
 	setupLogOutput()

Feedback & Suggestions:

  1. Consistency in Naming: Ensure that the naming of flags is consistent with existing ones. For example, consider using kyverno-policy-location instead of kyverno-policies-location to match the singular form used in other flags like vcs-base-url.

  2. Security Consideration: When dealing with URLs, especially those that might be used to fetch policies, consider validating the URL format to prevent potential security issues. This can help avoid injection attacks or malformed URLs.

  3. Documentation: Ensure that the documentation for these new flags is updated accordingly. This includes any README files or user guides that explain how to use these flags.

  4. Default Values: The default value for worst-kyverno-state is set to "panic". Ensure that this is the desired behavior, as it might cause the application to terminate unexpectedly if not handled properly.

  5. Testing: Add tests to verify the behavior of these new flags, especially how they interact with the existing system. This will help ensure that the new functionality works as expected and does not introduce regressions.


😼 Mergecat review of localdev/kubechecks/values.yaml

@@ -24,17 +24,27 @@ configMap:
     # KUBECHECKS_SCHEMAS_LOCATION: https://github.com/zapier/kubecheck-schemas.git
     KUBECHECKS_TIDY_OUTDATED_COMMENTS_MODE: "delete"
     KUBECHECKS_ENABLE_CONFTEST: "false"
+    # KUBECHECKS_ENABLE_KYVERNO_CHECKS: "false"
+    # KUBECHECKS_KYVERNO_POLICIES_LOCATION: "https://githum.com/zapier/kyverno-policies.git?subdir=/checks&branch=master"
+    KUBECHECKS_ARGOCD_SEND_FULL_REPOSITORY: "true"
 
 
 deployment:
   annotations:
     reloader.stakater.com/auto: "true" 
   
   image:
-    pullPolicy: Never
+    pullPolicy: IfNotPresent
     name: "kubechecks"
     tag: ""
 
+  resources:
+    limits:
+      memory: 1Gi
+    requests:
+      memory: 256Mi
+      cpu: 200m
+
 secrets:
   create: true
   env:

Feedback & Suggestions:

  1. Resource Requests and Limits:

    • The addition of resource requests and limits is a good practice for ensuring that your application has the necessary resources while also preventing it from consuming too much. However, consider also setting a CPU limit to complement the memory limit for better resource management. 🧠
  2. Image Pull Policy:

    • Changing the pullPolicy from Never to IfNotPresent is generally a good idea for environments where the image might not always be available locally. However, ensure that this aligns with your deployment strategy, especially in development environments where you might want to use the latest local build. 🔄
  3. Kyverno Checks:

    • The commented-out lines for Kyverno checks and policies location should be reviewed for correctness. The URL for KUBECHECKS_KYVERNO_POLICIES_LOCATION contains a typo: "githum" should be "github". This should be corrected to avoid confusion when these lines are uncommented. 🐞
  4. Security Considerations:

    • Ensure that any sensitive information, such as API tokens, is not hardcoded in the configuration files. Consider using a more secure method for managing secrets, such as Kubernetes Secrets or a secret management tool. 🔐

😼 Mergecat review of docs/usage.md

@@ -48,6 +48,7 @@ The full list of supported environment variables is described below:
 |`KUBECHECKS_ENABLE_CONFTEST`|Set to true to enable conftest policy checking of manifests.|`false`|
 |`KUBECHECKS_ENABLE_HOOKS_RENDERER`|Render hooks.|`true`|
 |`KUBECHECKS_ENABLE_KUBECONFORM`|Enable kubeconform checks.|`true`|
+|`KUBECHECKS_ENABLE_KYVERNO_CHECKS`|Enable kyverno policy checks.|`false`|
 |`KUBECHECKS_ENABLE_PREUPGRADE`|Enable preupgrade checks.|`true`|
 |`KUBECHECKS_ENSURE_WEBHOOKS`|Ensure that webhooks are created in repositories referenced by argo.|`false`|
 |`KUBECHECKS_FALLBACK_K8S_VERSION`|Fallback target Kubernetes version for schema / upgrade checks.|`1.23.0`|
@@ -57,6 +58,7 @@ The full list of supported environment variables is described below:
 |`KUBECHECKS_KUBERNETES_CLUSTERID`|Kubernetes Cluster ID, must be specified if kubernetes-type is eks.||
 |`KUBECHECKS_KUBERNETES_CONFIG`|Path to your kubernetes config file, used to monitor applications.||
 |`KUBECHECKS_KUBERNETES_TYPE`|Kubernetes Type One of eks, or local.|`local`|
+|`KUBECHECKS_KYVERNO_POLICIES_LOCATION`|Sets kyverno policy locations to be used for every check request. This is a git url in either git or http(s) format.||
 |`KUBECHECKS_LABEL_FILTER`|(Optional) If set, The label that must be set on an MR (as "kubechecks:<value>") for kubechecks to process the merge request webhook.||
 |`KUBECHECKS_LOG_LEVEL`|Set the log output level. One of error, warn, info, debug, trace.|`info`|
 |`KUBECHECKS_MAX_CONCURRENCT_CHECKS`|Number of concurrent checks to run.|`32`|
@@ -85,4 +87,8 @@ The full list of supported environment variables is described below:
 |`KUBECHECKS_WORST_CONFTEST_STATE`|The worst state that can be returned from conftest.|`panic`|
 |`KUBECHECKS_WORST_HOOKS_STATE`|The worst state that can be returned from the hooks renderer.|`panic`|
 |`KUBECHECKS_WORST_KUBECONFORM_STATE`|The worst state that can be returned from kubeconform.|`panic`|
+|`KUBECHECKS_WORST_KYVERNO_STATE`|The worst state that can be returned from the kyverno checks.|`panic`|
 |`KUBECHECKS_WORST_PREUPGRADE_STATE`|The worst state that can be returned from preupgrade checks.|`panic`|
+
+
+See [integrations](./integrations) for more information on the tools integrated into `kubechecks` .
\ No newline at end of file

Feedback & Suggestions:

  1. Newline at End of File: Ensure there is a newline at the end of the file. This is a common convention in many programming environments and can prevent potential issues with certain tools or systems. 📝

  2. Consistency in Descriptions: The description for KUBECHECKS_KYVERNO_POLICIES_LOCATION mentions "This is a git url in either git or http(s) format." Consider rephrasing to match the style of other entries, e.g., "Can be a git URL in either git or HTTP(S) format." for consistency. 🔄

  3. Documentation Link: The addition of the link to integrations is a good idea. Ensure that the linked document exists and is up-to-date to provide users with the necessary information. 🔗


😼 Mergecat review of pkg/argo_client/manifests.go

@@ -119,7 +119,7 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
 	settingsCloser, settingsClient := a.GetSettingsClient()
 	defer settingsCloser.Close()
 
-	log.Info().Msg("get settings")
+	log.Debug().Msg("get settings")
 	argoSettings, err := settingsClient.Get(ctx, &settings.SettingsQuery{})
 	if err != nil {
 		getManifestsFailed.WithLabelValues(app.Name).Inc()
@@ -134,30 +134,30 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
 		repoTarget = pullRequest.HeadRef
 	}
 
-	log.Info().Msg("get repo")
+	log.Debug().Msg("get repo")
 	repo, err := getRepo(ctx, source.RepoURL, repoTarget)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get repo")
 	}
 
 	var packageDir string
 	if a.sendFullRepository {
-		log.Info().Msg("sending full repository")
+		log.Debug().Msg("sending full repository")
 		packageDir = repo.Directory
 	} else {
-		log.Info().Msg("packaging app")
+	log.Debug().Str("app", app.Name).Msg("packaging app")
 		packageDir, err = packageApp(ctx, source, refs, repo, getRepo)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to package application")
 		}
 	}
 
-	log.Info().Msg("compressing files")
+	log.Debug().Msg("compressing files")
 	f, filesWritten, checksum, err := tgzstream.CompressFiles(packageDir, []string{"*"}, []string{".git"})
 	if err != nil {
 		return nil, fmt.Errorf("failed to compress files: %w", err)
 	}
-	log.Info().Msgf("%d files compressed", filesWritten)
+	log.Debug().Msgf("%d files compressed", filesWritten)
 	//if filesWritten == 0 {
 	//	return nil, fmt.Errorf("no files to send")
 	//}
@@ -226,13 +226,13 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
 		RefSources:         refSources,
 	}
 
-	log.Info().Msg("generating manifest with files")
+	log.Debug().Msg("generating manifest with files")
 	stream, err := a.repoClient.GenerateManifestWithFiles(ctx)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get manifests with files")
 	}
 
-	log.Info().Msg("sending request")
+	log.Debug().Msg("sending request")
 	if err := stream.Send(&repoapiclient.ManifestRequestWithFiles{
 		Part: &repoapiclient.ManifestRequestWithFiles_Request{
 			Request: &q,
@@ -241,7 +241,7 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
 		return nil, errors.Wrap(err, "failed to send request")
 	}
 
-	log.Info().Msg("sending metadata")
+	log.Debug().Msg("sending metadata")
 	if err := stream.Send(&repoapiclient.ManifestRequestWithFiles{
 		Part: &repoapiclient.ManifestRequestWithFiles_Metadata{
 			Metadata: &repoapiclient.ManifestFileMetadata{
@@ -252,19 +252,19 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
 		return nil, errors.Wrap(err, "failed to send metadata")
 	}
 
-	log.Info().Msg("sending file")
+	log.Debug().Msg("sending file")
 	err = sendFile(ctx, stream, f)
 	if err != nil {
 		return nil, fmt.Errorf("failed to send manifest stream file: %w", err)
 	}
 
-	log.Info().Msg("receiving repsonse")
+	log.Debug().Msg("receiving response")
 	response, err := stream.CloseAndRecv()
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get response")
 	}
 
-	log.Info().Msg("done!")
+	log.Debug().Msg("done!")
 	return response.Manifests, nil
 }
 

Feedback & Suggestions:

  1. Log Level Change: The change from log.Info() to log.Debug() is appropriate for reducing verbosity in production environments. However, ensure that this change aligns with your logging strategy and that important information isn't lost in debug logs, which might not be enabled in all environments. Consider keeping critical logs at the Info level if they are essential for monitoring or troubleshooting.

  2. Consistency in Logging: The addition of log.Debug().Str("app", app.Name).Msg("packaging app") is a good practice for adding context to logs. Ensure that similar context is added to other log statements where applicable, especially if they are related to specific applications or processes.

  3. Performance Consideration: While changing log levels doesn't directly impact performance, excessive logging, even at the debug level, can affect performance. Ensure that debug logging is used judiciously and is disabled in performance-sensitive environments.

  4. Typo Correction: The typo in the log message "receiving repsonse" was corrected to "receiving response". This is a good catch, as clear and correct log messages are crucial for effective debugging and monitoring.

Overall, the changes improve the logging granularity, which can be beneficial for debugging while keeping production logs clean. 🐾


😼 Mergecat review of go.mod

@@ -1,38 +1,41 @@
 module github.com/zapier/kubechecks
 
-go 1.22.0
+go 1.22.8
 
-toolchain go1.22.7
+toolchain go1.23.4
 
 require (
 	github.com/argoproj/argo-cd/v2 v2.13.1
 	github.com/argoproj/gitops-engine v0.7.1-0.20240905010810-bd7681ae3f8b
 	github.com/aws/aws-sdk-go-v2 v1.32.6
-	github.com/aws/aws-sdk-go-v2/config v1.27.24
+	github.com/aws/aws-sdk-go-v2/config v1.27.33
 	github.com/aws/aws-sdk-go-v2/service/eks v1.46.0
 	github.com/aws/aws-sdk-go-v2/service/sts v1.33.2
 	github.com/aws/smithy-go v1.22.1
 	github.com/bradleyfalzon/ghinstallation/v2 v2.11.0
 	github.com/cenkalti/backoff/v4 v4.3.0
 	github.com/chainguard-dev/git-urls v1.0.2
 	github.com/creasty/defaults v1.7.0
-	github.com/ghodss/yaml v1.0.0
+	github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
+	github.com/go-git/go-billy/v5 v5.5.0
 	github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399
 	github.com/go-logr/zerologr v1.2.3
 	github.com/google/go-github/v62 v62.0.0
 	github.com/google/uuid v1.6.0
 	github.com/heptiolabs/healthcheck v0.0.0-20211123025425-613501dd5deb
 	github.com/imdario/mergo v0.3.16
 	github.com/jeremywohl/flatten v1.0.1
+	github.com/kyverno/kyverno v1.13.1
 	github.com/labstack/echo-contrib v0.17.1
 	github.com/labstack/echo/v4 v4.13.3
 	github.com/masterminds/semver v1.5.0
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/open-policy-agent/conftest v0.49.1
+	github.com/opentracing/opentracing-go v1.2.0
 	github.com/pkg/errors v0.9.1
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
-	github.com/prometheus/client_golang v1.20.3
+	github.com/prometheus/client_golang v1.20.4
 	github.com/rikatz/kubepug v1.4.0
 	github.com/rs/zerolog v1.33.0
 	github.com/sashabaranov/go-openai v1.36.0
@@ -52,7 +55,7 @@ require (
 	go.opentelemetry.io/otel/sdk v1.33.0
 	go.opentelemetry.io/otel/sdk/metric v1.33.0
 	go.opentelemetry.io/otel/trace v1.33.0
-	golang.org/x/exp v0.0.0-20240112132812-db7319d0e0e3
+	golang.org/x/exp v0.0.0-20240823005443-9b4947da3948
 	golang.org/x/net v0.33.0
 	golang.org/x/oauth2 v0.24.0
 	google.golang.org/grpc v1.67.1
@@ -67,137 +70,249 @@ require (
 )
 
 require (
-	cloud.google.com/go v0.112.1 // indirect
+	cloud.google.com/go v0.115.1 // indirect
+	cloud.google.com/go/auth v0.9.1 // indirect
+	cloud.google.com/go/auth/oauth2adapt v0.2.4 // indirect
 	cloud.google.com/go/compute/metadata v0.5.0 // indirect
-	cloud.google.com/go/iam v1.1.6 // indirect
-	cloud.google.com/go/storage v1.38.0 // indirect
-	cuelang.org/go v0.7.0 // indirect
+	cloud.google.com/go/iam v1.2.0 // indirect
+	cloud.google.com/go/kms v1.19.0 // indirect
+	cloud.google.com/go/longrunning v0.6.0 // indirect
+	cloud.google.com/go/storage v1.43.0 // indirect
+	cuelabs.dev/go/oci/ociregistry v0.0.0-20240807094312-a32ad29eed79 // indirect
+	cuelang.org/go v0.10.0 // indirect
 	dario.cat/mergo v1.0.1 // indirect
+	filippo.io/edwards25519 v1.1.0 // indirect
+	github.com/AliyunContainerService/ack-ram-tool/pkg/credentials/alibabacloudsdkgo/helper v0.2.0 // indirect
+	github.com/Azure/azure-sdk-for-go v68.0.0+incompatible // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.14.0 // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.1.0 // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.0.1 // indirect
 	github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect
+	github.com/Azure/go-autorest v14.2.0+incompatible // indirect
+	github.com/Azure/go-autorest/autorest v0.11.29 // indirect
+	github.com/Azure/go-autorest/autorest/adal v0.9.24 // indirect
+	github.com/Azure/go-autorest/autorest/azure/auth v0.5.13 // indirect
+	github.com/Azure/go-autorest/autorest/azure/cli v0.4.6 // indirect
+	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
+	github.com/Azure/go-autorest/logger v0.2.1 // indirect
+	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
+	github.com/Azure/go-ntlmssp v0.0.0-20221128193559-754e69321358 // indirect
+	github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 // indirect
 	github.com/BurntSushi/toml v1.3.2 // indirect
 	github.com/CycloneDX/cyclonedx-go v0.8.0 // indirect
+	github.com/IGLOU-EU/go-wildcard v1.0.3 // indirect
 	github.com/KeisukeYamashita/go-vcl v0.4.0 // indirect
 	github.com/MakeNowJust/heredoc v1.0.0 // indirect
 	github.com/Masterminds/goutils v1.1.1 // indirect
-	github.com/Masterminds/semver v1.5.0 // indirect
 	github.com/Masterminds/semver/v3 v3.3.0 // indirect
 	github.com/Masterminds/sprig/v3 v3.3.0 // indirect
 	github.com/Microsoft/go-winio v0.6.2 // indirect
+	github.com/NYTimes/gziphandler v1.1.1 // indirect
 	github.com/OneOfOne/xxhash v1.2.8 // indirect
 	github.com/ProtonMail/go-crypto v1.0.0 // indirect
+	github.com/ThalesIgnite/crypto11 v1.2.5 // indirect
 	github.com/agext/levenshtein v1.2.3 // indirect
 	github.com/agnivade/levenshtein v1.1.1 // indirect
+	github.com/alibabacloud-go/alibabacloud-gateway-spi v0.0.5 // indirect
+	github.com/alibabacloud-go/cr-20160607 v1.0.1 // indirect
+	github.com/alibabacloud-go/cr-20181201 v1.0.10 // indirect
+	github.com/alibabacloud-go/darabonba-openapi v0.2.1 // indirect
+	github.com/alibabacloud-go/debug v1.0.1 // indirect
+	github.com/alibabacloud-go/endpoint-util v1.1.1 // indirect
+	github.com/alibabacloud-go/openapi-util v0.1.1 // indirect
+	github.com/alibabacloud-go/tea v1.2.2 // indirect
+	github.com/alibabacloud-go/tea-utils v1.4.5 // indirect
+	github.com/alibabacloud-go/tea-utils/v2 v2.0.6 // indirect
+	github.com/alibabacloud-go/tea-xml v1.1.3 // indirect
+	github.com/aliyun/credentials-go v1.3.8 // indirect
 	github.com/anchore/go-struct-converter v0.0.0-20221118182256-c68fdcfa2092 // indirect
+	github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
 	github.com/apparentlymart/go-textseg/v13 v13.0.0 // indirect
+	github.com/aptible/supercronic v0.2.30 // indirect
+	github.com/aquilax/truncate v1.0.0 // indirect
 	github.com/argoproj/pkg v0.13.7-0.20230627120311-a4dd357b057e // indirect
+	github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect
 	github.com/aws/aws-sdk-go v1.55.5 // indirect
-	github.com/aws/aws-sdk-go-v2/credentials v1.17.24 // indirect
-	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.9 // indirect
+	github.com/aws/aws-sdk-go-v2/credentials v1.17.32 // indirect
+	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.13 // indirect
 	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25 // indirect
 	github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25 // indirect
-	github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
+	github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 // indirect
+	github.com/aws/aws-sdk-go-v2/service/ecr v1.33.0 // indirect
+	github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.25.6 // indirect
 	github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.1 // indirect
 	github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.6 // indirect
-	github.com/aws/aws-sdk-go-v2/service/sso v1.22.1 // indirect
-	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.2 // indirect
+	github.com/aws/aws-sdk-go-v2/service/kms v1.35.5 // indirect
+	github.com/aws/aws-sdk-go-v2/service/sso v1.22.7 // indirect
+	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.7 // indirect
+	github.com/awslabs/amazon-ecr-credential-helper/ecr-login v0.0.0-20240909191326-0ee4ec5d16bf // indirect
 	github.com/basgys/goxml2json v1.1.0 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
+	github.com/blang/semver v3.5.1+incompatible // indirect
 	github.com/blang/semver/v4 v4.0.0 // indirect
 	github.com/bmatcuk/doublestar/v4 v4.6.1 // indirect
 	github.com/bombsimon/logrusr/v2 v2.0.1 // indirect
-	github.com/bufbuild/protocompile v0.6.0 // indirect
+	github.com/bufbuild/protocompile v0.10.0 // indirect
+	github.com/buildkite/agent/v3 v3.78.0 // indirect
+	github.com/buildkite/go-pipeline v0.11.0 // indirect
+	github.com/buildkite/interpolate v0.1.3 // indirect
+	github.com/buildkite/roko v1.2.0 // indirect
+	github.com/cenkalti/backoff/v3 v3.2.2 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/chai2010/gettext-go v1.0.2 // indirect
-	github.com/cloudflare/circl v1.3.7 // indirect
+	github.com/chrismellard/docker-credential-acr-env v0.0.0-20230304212654-82a0ddb27589 // indirect
+	github.com/clbanning/mxj/v2 v2.7.0 // indirect
+	github.com/cloudflare/circl v1.4.0 // indirect
 	github.com/cockroachdb/apd/v3 v3.2.1 // indirect
+	github.com/common-nighthawk/go-figure v0.0.0-20210622060536-734e95fb86be // indirect
+	github.com/containerd/stargz-snapshotter/estargz v0.15.1 // indirect
 	github.com/containerd/typeurl/v2 v2.1.1 // indirect
 	github.com/coreos/go-oidc/v3 v3.11.0 // indirect
+	github.com/coreos/go-semver v0.3.1 // indirect
+	github.com/coreos/go-systemd/v22 v22.5.0 // indirect
 	github.com/cpuguy83/dockercfg v0.3.1 // indirect
+	github.com/cyberphone/json-canonicalization v0.0.0-20231217050601-ba74d44ecf5f // indirect
 	github.com/cyphar/filepath-securejoin v0.3.2 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
+	github.com/dgraph-io/ristretto v0.1.1 // indirect
 	github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
-	github.com/distribution/reference v0.5.0 // indirect
+	github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352 // indirect
+	github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 // indirect
+	github.com/dimchansky/utfbom v1.1.1 // indirect
+	github.com/distribution/reference v0.6.0 // indirect
+	github.com/djherbis/times v1.6.0 // indirect
 	github.com/dlclark/regexp2 v1.11.4 // indirect
+	github.com/docker/cli v27.2.0+incompatible // indirect
+	github.com/docker/distribution v2.8.3+incompatible // indirect
 	github.com/docker/docker v27.2.1+incompatible // indirect
-	github.com/docker/go-connections v0.4.0 // indirect
+	github.com/docker/docker-credential-helpers v0.8.2 // indirect
+	github.com/docker/go-connections v0.5.0 // indirect
 	github.com/docker/go-units v0.5.0 // indirect
-	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
+	github.com/dustin/go-humanize v1.0.1 // indirect
+	github.com/emicklei/go-restful/v3 v3.12.1 // indirect
+	github.com/emicklei/proto v1.13.2 // indirect
 	github.com/emirpasic/gods v1.18.1 // indirect
 	github.com/evanphx/json-patch v5.9.0

</details>

---

## Dependency Review
<details><summary>Click to read mergecats review!</summary>

No suggestions found
</details>

@MeNsaaH MeNsaaH closed this Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants