refactor: cleanup cli apply functions (#11928)

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

cmd/cli/kubectl-kyverno/commands/apply/command.go

@@ -170,61 +170,58 @@ func Command() *cobra.Command {
 }
 
 func (c *ApplyCommandConfig) applyCommandHelper(out io.Writer) (*processor.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, error) {
-	rc, resources1, skipInvalidPolicies, responses1, err := c.checkArguments()
+	var skippedInvalidPolicies SkippedInvalidPolicies
+	err := c.checkArguments()
 	if err != nil {
-		return rc, resources1, skipInvalidPolicies, responses1, err
+		return nil, nil, skippedInvalidPolicies, nil, err
 	}
-	rc, resources1, skipInvalidPolicies, responses1, err, mutateLogPathIsDir := c.getMutateLogPathIsDir(skipInvalidPolicies)
+	mutateLogPathIsDir, err := c.getMutateLogPathIsDir()
 	if err != nil {
-		return rc, resources1, skipInvalidPolicies, responses1, err
+		return nil, nil, skippedInvalidPolicies, nil, err
 	}
-	rc, resources1, skipInvalidPolicies, responses1, err = c.cleanPreviousContent(mutateLogPathIsDir, skipInvalidPolicies)
+	if err := c.cleanPreviousContent(mutateLogPathIsDir); err != nil {
-	if err != nil {
+		return nil, nil, skippedInvalidPolicies, nil, err
-		return rc, resources1, skipInvalidPolicies, responses1, err
 	}
 	var userInfo *kyvernov2.RequestInfo
 	if c.UserInfoPath != "" {
 		info, err := userinfo.Load(nil, c.UserInfoPath, "")
 		if err != nil {
-			return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("failed to load request info (%w)", err)
+			return nil, nil, skippedInvalidPolicies, nil, fmt.Errorf("failed to load request info (%w)", err)
 		}
 		deprecations.CheckUserInfo(out, c.UserInfoPath, info)
 		userInfo = &info.RequestInfo
 	}
 	variables, err := variables.New(out, nil, "", c.ValuesFile, nil, c.Variables...)
 	if err != nil {
-		return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("failed to decode yaml (%w)", err)
+		return nil, nil, skippedInvalidPolicies, nil, fmt.Errorf("failed to decode yaml (%w)", err)
 	}
 	var store store.Store
-	rc, resources1, skipInvalidPolicies, responses1, policies, vaps, vapBindings, err := c.loadPolicies(skipInvalidPolicies)
+	policies, vaps, vapBindings, err := c.loadPolicies()
 	if err != nil {
-		return rc, resources1, skipInvalidPolicies, responses1, err
+		return nil, nil, skippedInvalidPolicies, nil, err
 	}
 	var targetResources []*unstructured.Unstructured
 	if len(c.TargetResourcePaths) > 0 {
 		targetResources, err = c.loadResources(out, c.TargetResourcePaths, policies, vaps, nil)
 		if err != nil {
-			return rc, resources1, skipInvalidPolicies, responses1, err
+			return nil, nil, skippedInvalidPolicies, nil, err
 		}
 	}
-
+	dClient, err := c.initStoreAndClusterClient(&store, targetResources...)
-	rc, resources1, skipInvalidPolicies, responses1, dClient, err := c.initStoreAndClusterClient(&store, skipInvalidPolicies, targetResources...)
 	if err != nil {
-		return rc, resources1, skipInvalidPolicies, responses1, err
+		return nil, nil, skippedInvalidPolicies, nil, err
 	}
-
 	resources, err := c.loadResources(out, c.ResourcePaths, policies, vaps, dClient)
 	if err != nil {
-		return rc, resources1, skipInvalidPolicies, responses1, err
+		return nil, nil, skippedInvalidPolicies, nil, err
 	}
-
 	var exceptions []*kyvernov2.PolicyException
 	if c.inlineExceptions {
 		exceptions = exception.SelectFrom(resources)
 	} else {
 		exceptions, err = exception.Load(c.Exception...)
 		if err != nil {
-			return rc, resources1, skipInvalidPolicies, responses1, fmt.Errorf("Error: failed to load exceptions (%s)", err)
+			return nil, nil, skippedInvalidPolicies, nil, fmt.Errorf("Error: failed to load exceptions (%s)", err)
 		}
 	}
 	if !c.Stdin && !c.PolicyReport && !c.GenerateExceptions {
@@ -239,38 +236,37 @@ func (c *ApplyCommandConfig) applyCommandHelper(out io.Writer) (*processor.Resul
 			fmt.Fprintf(out, "\nApplying %d policy rule(s) to %d resource(s)...\n", policyRulesCount, len(resources))
 		}
 	}
-
+	rc, resources1, responses1, err := c.applyPolicytoResource(
-	rc, resources1, responses1, err = c.applyPolicytoResource(
 		out,
 		&store,
 		variables,
 		policies,
 		resources,
 		exceptions,
-		&skipInvalidPolicies,
+		&skippedInvalidPolicies,
 		dClient,
 		userInfo,
 		mutateLogPathIsDir,
 	)
 	if err != nil {
-		return rc, resources1, skipInvalidPolicies, responses1, err
+		return rc, resources1, skippedInvalidPolicies, responses1, err
 	}
 	responses2, err := c.applyValidatingAdmissionPolicytoResource(vaps, vapBindings, resources1, variables.NamespaceSelectors(), rc, dClient)
 	if err != nil {
-		return rc, resources1, skipInvalidPolicies, responses1, err
+		return rc, resources1, skippedInvalidPolicies, responses1, err
 	}
 	var responses []engineapi.EngineResponse
 	responses = append(responses, responses1...)
 	responses = append(responses, responses2...)
-	return rc, resources1, skipInvalidPolicies, responses, nil
+	return rc, resources1, skippedInvalidPolicies, responses, nil
 }
 
-func (c *ApplyCommandConfig) getMutateLogPathIsDir(skipInvalidPolicies SkippedInvalidPolicies) (*processor.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, error, bool) {
+func (c *ApplyCommandConfig) getMutateLogPathIsDir() (bool, error) {
 	mutateLogPathIsDir, err := checkMutateLogPath(c.MutateLogPath)
 	if err != nil {
-		return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("failed to create file/folder (%w)", err), false
+		return false, fmt.Errorf("failed to create file/folder (%w)", err)
 	}
-	return nil, nil, skipInvalidPolicies, nil, err, mutateLogPathIsDir
+	return mutateLogPathIsDir, nil
 }
 
 func (c *ApplyCommandConfig) applyValidatingAdmissionPolicytoResource(
@@ -388,7 +384,12 @@ func (c *ApplyCommandConfig) loadResources(out io.Writer, paths []string, polici
 	return resources, nil
 }
 
-func (c *ApplyCommandConfig) loadPolicies(skipInvalidPolicies SkippedInvalidPolicies) (*processor.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, []kyvernov1.PolicyInterface, []admissionregistrationv1beta1.ValidatingAdmissionPolicy, []admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding, error) {
+func (c *ApplyCommandConfig) loadPolicies() (
+	[]kyvernov1.PolicyInterface,
+	[]admissionregistrationv1beta1.ValidatingAdmissionPolicy,
+	[]admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding,
+	error,
+) {
 	// load policies
 	var policies []kyvernov1.PolicyInterface
 	var vaps []admissionregistrationv1beta1.ValidatingAdmissionPolicy
@@ -399,12 +400,12 @@ func (c *ApplyCommandConfig) loadPolicies(skipInvalidPolicies SkippedInvalidPoli
 		if isGit {
 			gitSourceURL, err := url.Parse(path)
 			if err != nil {
-				return nil, nil, skipInvalidPolicies, nil, nil, nil, nil, fmt.Errorf("failed to load policies (%w)", err)
+				return nil, nil, nil, fmt.Errorf("failed to load policies (%w)", err)
 			}
 			pathElems := strings.Split(gitSourceURL.Path[1:], "/")
 			if len(pathElems) <= 1 {
 				err := fmt.Errorf("invalid URL path %s - expected https://<any_git_source_domain>/:owner/:repository/:branch (without --git-branch flag) OR https://<any_git_source_domain>/:owner/:repository/:directory (with --git-branch flag)", gitSourceURL.Path)
-				return nil, nil, skipInvalidPolicies, nil, nil, nil, nil, fmt.Errorf("failed to parse URL (%w)", err)
+				return nil, nil, nil, fmt.Errorf("failed to parse URL (%w)", err)
 			}
 			gitSourceURL.Path = strings.Join([]string{pathElems[0], pathElems[1]}, "/")
 			repoURL := gitSourceURL.String()
@@ -413,11 +414,11 @@ func (c *ApplyCommandConfig) loadPolicies(skipInvalidPolicies SkippedInvalidPoli
 			fs := memfs.New()
 			if _, err := gitutils.Clone(repoURL, fs, c.GitBranch); err != nil {
 				log.Log.V(3).Info(fmt.Sprintf("failed to clone repository  %v as it is not valid", repoURL), "error", err)
-				return nil, nil, skipInvalidPolicies, nil, nil, nil, nil, fmt.Errorf("failed to clone repository (%w)", err)
+				return nil, nil, nil, fmt.Errorf("failed to clone repository (%w)", err)
 			}
 			policyYamls, err := gitutils.ListYamls(fs, gitPathToYamls)
 			if err != nil {
-				return nil, nil, skipInvalidPolicies, nil, nil, nil, nil, fmt.Errorf("failed to list YAMLs in repository (%w)", err)
+				return nil, nil, nil, fmt.Errorf("failed to list YAMLs in repository (%w)", err)
 			}
 			for _, policyYaml := range policyYamls {
 				loaderResults, err := policy.Load(fs, "", policyYaml)
@@ -444,10 +445,13 @@ func (c *ApplyCommandConfig) loadPolicies(skipInvalidPolicies SkippedInvalidPoli
 			}
 		}
 	}
-	return nil, nil, skipInvalidPolicies, nil, policies, vaps, vapBindings, nil
+	return policies, vaps, vapBindings, nil
 }
 
-func (c *ApplyCommandConfig) initStoreAndClusterClient(store *store.Store, skipInvalidPolicies SkippedInvalidPolicies, targetResources ...*unstructured.Unstructured) (*processor.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, dclient.Interface, error) {
+func (c *ApplyCommandConfig) initStoreAndClusterClient(store *store.Store, targetResources ...*unstructured.Unstructured) (
+	dclient.Interface,
+	error,
+) {
 	store.SetLocal(true)
 	store.SetRegistryAccess(c.RegistryAccess)
 	if c.Cluster {
@@ -458,19 +462,19 @@ func (c *ApplyCommandConfig) initStoreAndClusterClient(store *store.Store, skipI
 	if c.Cluster {
 		restConfig, err := config.CreateClientConfigWithContext(c.KubeConfig, c.Context)
 		if err != nil {
-			return nil, nil, skipInvalidPolicies, nil, nil, err
+			return nil, err
 		}
 		kubeClient, err := kubernetes.NewForConfig(restConfig)
 		if err != nil {
-			return nil, nil, skipInvalidPolicies, nil, nil, err
+			return nil, err
 		}
 		dynamicClient, err := dynamic.NewForConfig(restConfig)
 		if err != nil {
-			return nil, nil, skipInvalidPolicies, nil, nil, err
+			return nil, err
 		}
 		dClient, err = dclient.NewClient(context.Background(), dynamicClient, kubeClient, 15*time.Minute)
 		if err != nil {
-			return nil, nil, skipInvalidPolicies, nil, nil, err
+			return nil, err
 		}
 	}
 	if len(targetResources) > 0 && !c.Cluster {
@@ -478,17 +482,16 @@ func (c *ApplyCommandConfig) initStoreAndClusterClient(store *store.Store, skipI
 		for _, t := range targetResources {
 			targets = append(targets, t)
 		}
-
 		dClient, err = dclient.NewFakeClient(runtime.NewScheme(), map[schema.GroupVersionResource]string{}, targets...)
 		dClient.SetDiscovery(dclient.NewFakeDiscoveryClient(nil))
 		if err != nil {
-			return nil, nil, skipInvalidPolicies, nil, nil, err
+			return nil, err
 		}
 	}
-	return nil, nil, skipInvalidPolicies, nil, dClient, err
+	return dClient, err
 }
 
-func (c *ApplyCommandConfig) cleanPreviousContent(mutateLogPathIsDir bool, skipInvalidPolicies SkippedInvalidPolicies) (*processor.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, error) {
+func (c *ApplyCommandConfig) cleanPreviousContent(mutateLogPathIsDir bool) error {
 	// empty the previous contents of the file just in case if the file already existed before with some content(so as to perform overwrites)
 	// the truncation of files for the case when mutateLogPath is dir, is handled under pkg/kyverno/apply/common.go
 	if !mutateLogPathIsDir && c.MutateLogPath != "" {
@@ -496,27 +499,26 @@ func (c *ApplyCommandConfig) cleanPreviousContent(mutateLogPathIsDir bool, skipI
 		// Necessary for us to include the file via variable as it is part of the CLI.
 		_, err := os.OpenFile(c.MutateLogPath, os.O_TRUNC|os.O_WRONLY, 0o600) // #nosec G304
 		if err != nil {
-			return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("failed to truncate the existing file at %s (%w)", c.MutateLogPath, err)
+			return fmt.Errorf("failed to truncate the existing file at %s (%w)", c.MutateLogPath, err)
 		}
 	}
-	return nil, nil, skipInvalidPolicies, nil, nil
+	return nil
 }
 
-func (c *ApplyCommandConfig) checkArguments() (*processor.ResultCounts, []*unstructured.Unstructured, SkippedInvalidPolicies, []engineapi.EngineResponse, error) {
+func (c *ApplyCommandConfig) checkArguments() error {
-	var skipInvalidPolicies SkippedInvalidPolicies
 	if c.ValuesFile != "" && c.Variables != nil {
-		return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("pass the values either using set flag or values_file flag")
+		return fmt.Errorf("pass the values either using set flag or values_file flag")
 	}
 	if len(c.PolicyPaths) == 0 {
-		return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("require policy")
+		return fmt.Errorf("require policy")
 	}
 	if (len(c.PolicyPaths) > 0 && c.PolicyPaths[0] == "-") && len(c.ResourcePaths) > 0 && c.ResourcePaths[0] == "-" {
-		return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("a stdin pipe can be used for either policies or resources, not both")
+		return fmt.Errorf("a stdin pipe can be used for either policies or resources, not both")
 	}
 	if len(c.ResourcePaths) == 0 && !c.Cluster {
-		return nil, nil, skipInvalidPolicies, nil, fmt.Errorf("resource file(s) or cluster required")
+		return fmt.Errorf("resource file(s) or cluster required")
 	}
-	return nil, nil, skipInvalidPolicies, nil, nil
+	return nil
 }
 
 type WarnExitCodeError struct {