Create new kubeconfig for SupportBundleClient

[XinShuYang]

Fixes: #6687

• Create a new kubeconfig for local support bundle requests to avoid errors caused
  by the absence of this file.
• For ResolveKubeconfig, detailed error messages are returned, differentiating
  between kubeconfig file errors and InClusterConfig errors.

[XinShuYang]

/test-all

[XinShuYang]

/test-windows-e2e

[XinShuYang]

/test-windows-e2e

[luolanzone]

I believe the incluster way should be tried only when the path is the default one:

Suggested change

	if _, err := os.Stat(path); os.IsNotExist(err) {
	if path == clientcmd.RecommendedHomeFile {
	config, inClusterErr := rest.InClusterConfig()
	if inClusterErr == nil {
	return config, nil
	}
	return nil, fmt.Errorf(
	"failed to resolve kubeconfig: neither a valid kubeconfig file was found at '%s', nor could InClusterConfig be used: %w",
	path, inClusterErr,
	)
	}
	return nil, fmt.Errorf("failed to resolve kubeconfig: kubeconfig file does not exist at path '%s'", path)
	}
	if _, err := os.Stat(path); path == clientcmd.RecommendedHomeFile && os.IsNotExist(err) {
	config, inClusterErr := rest.InClusterConfig()
	if inClusterErr == nil {
	return config, nil
	}
	return nil, fmt.Errorf(
	"failed to resolve kubeconfig: no valid kubeconfig file was found at the default path '%s', and InClusterConfig can't be used: %w",
	path, inClusterErr,
	)
	}

[XinShuYang]

I believe my original code has the same behavior and can handle an additional case where the file's absence at a non-default path results in a different error message.

[antoninbas]

so my understanding is that there is no change in behavior, just better error messages?

[XinShuYang]

so my understanding is that there is no change in behavior, just better error messages?

There is only one case where a non-existent non-default path can lead to an error message and stop further processing, which is not covered in the original code. Other changes indeed aim to improve the error messaging.

[antoninbas]

maybe we should use clientcmd.NewNonInteractiveDeferredLoadingClientConfig directly (like we do in other parts of the code). it is a bit weird to use BuildConfigFromFlags, which itself has a fallback to inClusterConfig (even though it should not be used in our case).

[XinShuYang]

Updated, thanks!

[antoninbas]

I feel like the function could be cleaned up further. Maybe something like:

withExplicitPath := path != ""
if !withExplicitPath {
        path = strings.TrimSpace(os.Getenv("KUBECONFIG"))
        if path == "" {
                path = clientcmd.RecommendedHomeFile // default path
        }
}
if _, err := os.Stat(path); err != nil {
        if !os.IsNotExist(err) {
                return nil, err // unexpected error
        }
        if withExplicitPath {
                // user-provided path is not valid, return error
                // ...
        }
        // no file found at default path, fallback to InClusterConfig
        // ...
}

[XinShuYang]

Yes, I feel this is better, updated.

[antoninbas]

so my understanding is that there is no change in behavior, just better error messages?

[antoninbas]

LGTM

[antoninbas]

/test-all

[antoninbas]

@luolanzone any further comments?