This potentially affects someone who is using NewConfigFlags(). Previously, it would return a ConfigFlags with CacheDir already populated with the default cache directory. Now it will return an empty CacheDir. This could be a breaking change for someone if they have built a client that uses this to know where the cache directory will be located.

There really isn't any way for a caller to know what the cache directory is going to be otherwise.

I wonder if it would be better to just change how defaultCacheDir is determined and leave everything else as-is?

So instead of this:

var (
	defaultCacheDir = filepath.Join(homedir.HomeDir(), ".kube", "cache")
)

Can you set defaultCacheDir conditionally if the environment variable is set, and using the current default value otherwise? Maybe using an init() func or something to do that.

Then toDiscoveryClient() and NewConfigFlags() don't need to change at all and everything should work exactly the same as it has been, just with the environment variable option.

Thoughts on this?

That is really a good point @brianpursley and indeed it breaks the backward compatibility.

But for your suggestion, after setting KUBECACHEDIR in init function, there might be a risk to not using latest KUBECACHEDIR in case of updates?

Maybe we can leave defaultCacheDir as is and we can just use cache-dir if it is not equal to defaultCacheDir.

I mean something like that;

cacheDir := defaultCacheDir
if kcd := os.Getenv("KUBECACHEDIR"); kcd != "" {
cacheDir = kcd
}

// retrieve a user-provided value for the "cache-dir"
// override httpCacheDir and discoveryCacheDir if user-value is given.
// user-provided value has higher precedence than default
// and KUBECACHEDIR environment variable.
if f.CacheDir != nil && *f.CacheDir != "" && *f.CacheDir !=  defaultCacheDir {
cacheDir = *f.CacheDir
}

Only problem in here is user can not use defaultCacheDir by passing --cache-dir if KUBECACHEDIR is set but I think, this is too edge case?

I think it is OK to change, it just needs to be set in all the same places it is currently.

What about something like this?...

Remove the defaultCacheDir variable and add a function that can return the default cache dir, something like this:

func getDefaultCacheDir() string {
	if kcd := os.Getenv("KUBECACHEDIR"); kcd != "" {
		return kcd
	}
	return filepath.Join(homedir.HomeDir(), ".kube", "cache")
}

Then you can replace defaultCacheDir with getDefaultCacheDir() which will follow the rules of looking at the KUBECACHEDIR environment variable first, then constructing it from the homedir if the environment variable is not set.

@brianpursley thanks your the suggestion. I agree, this is better solution and changed this PR that way.