From a21abfe64a606e41a63638f434db192ae4872334 Mon Sep 17 00:00:00 2001
From: Stefan Bueringer <sbueringer@gmail.com>
Date: Fri, 7 Oct 2022 16:03:06 +0200
Subject: [PATCH] clusterctl: adjust Overrider interface so Path can return an
 error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stefan Büringer buringerst@vmware.com
---
 cmd/clusterctl/client/repository/overrides.go | 36 +++++++++----------
 .../src/developer/providers/v1.2-to-v1.3.md   |  1 +
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/cmd/clusterctl/client/repository/overrides.go b/cmd/clusterctl/client/repository/overrides.go
index 133a0eb62046..32c4a742ff5e 100644
--- a/cmd/clusterctl/client/repository/overrides.go
+++ b/cmd/clusterctl/client/repository/overrides.go
@@ -17,7 +17,6 @@ limitations under the License.
 package repository
 
 import (
-	"fmt"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -29,7 +28,6 @@ import (
 	"k8s.io/client-go/util/homedir"
 
 	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/config"
-	logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log"
 )
 
 const (
@@ -39,7 +37,7 @@ const (
 
 // Overrider provides behavior to determine the overrides layer.
 type Overrider interface {
-	Path() string
+	Path() (string, error)
 }
 
 // overrides implements the Overrider interface.
@@ -69,30 +67,28 @@ func newOverride(o *newOverrideInput) Overrider {
 
 // Path returns the fully formed path to the file within the specified
 // overrides config.
-func (o *overrides) Path() string {
+func (o *overrides) Path() (string, error) {
 	basepath := filepath.Join(homedir.HomeDir(), config.ConfigFolder, overrideFolder)
 	f, err := o.configVariablesClient.Get(overrideFolderKey)
 	if err == nil && strings.TrimSpace(f) != "" {
 		basepath = f
 
-		evaluatedBasePath, err := envsubst.Eval(basepath, os.Getenv)
+		basepath, err = envsubst.Eval(basepath, os.Getenv)
 		if err != nil {
-			logf.Log.Info(fmt.Sprintf("⚠️overridesFolder %q could not be evaluated: %v", basepath, err))
-		} else {
-			basepath = evaluatedBasePath
+			return "", errors.Wrapf(err, "unable to evaluate %s: %q", overrideFolderKey, basepath)
 		}
 
 		if runtime.GOOS == "windows" {
 			parsedBasePath, err := url.Parse(basepath)
 			if err != nil {
-				logf.Log.Info(fmt.Sprintf("⚠️overridesFolder %q could not be parsed: %v", basepath, err))
-			} else {
-				// in case of windows, we should take care of removing the additional / which is required by the URI standard
-				// for windows local paths. see https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/ for more details.
-				// Encoded file paths are not required in Windows 10 versions <1803 and are unsupported in Windows 10 >=1803
-				// https://support.microsoft.com/en-us/help/4467268/url-encoded-unc-paths-not-url-decoded-in-windows-10-version-1803-later
-				basepath = filepath.FromSlash(strings.TrimPrefix(parsedBasePath.Path, "/"))
+				return "", errors.Wrapf(err, "unable to parse %s: %q", overrideFolderKey, basepath)
 			}
+
+			// in case of windows, we should take care of removing the additional / which is required by the URI standard
+			// for windows local paths. see https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/ for more details.
+			// Encoded file paths are not required in Windows 10 versions <1803 and are unsupported in Windows 10 >=1803
+			// https://support.microsoft.com/en-us/help/4467268/url-encoded-unc-paths-not-url-decoded-in-windows-10-version-1803-later
+			basepath = filepath.FromSlash(strings.TrimPrefix(parsedBasePath.Path, "/"))
 		}
 	}
 
@@ -101,15 +97,19 @@ func (o *overrides) Path() string {
 		o.providerLabel,
 		o.version,
 		o.filePath,
-	)
+	), nil
 }
 
 // getLocalOverride return local override file from the config folder, if it exists.
 // This is required for development purposes, but it can be used also in production as a workaround for problems on the official repositories.
 func getLocalOverride(info *newOverrideInput) ([]byte, error) {
-	overridePath := newOverride(info).Path()
+	overridePath, err := newOverride(info).Path()
+	if err != nil {
+		return nil, err
+	}
+
 	// it the local override exists, use it
-	_, err := os.Stat(overridePath)
+	_, err = os.Stat(overridePath)
 	if err == nil {
 		content, err := os.ReadFile(overridePath) //nolint:gosec
 		if err != nil {
diff --git a/docs/book/src/developer/providers/v1.2-to-v1.3.md b/docs/book/src/developer/providers/v1.2-to-v1.3.md
index e4f15a984bc4..4d2eff57ff0f 100644
--- a/docs/book/src/developer/providers/v1.2-to-v1.3.md
+++ b/docs/book/src/developer/providers/v1.2-to-v1.3.md
@@ -40,6 +40,7 @@ in Cluster API are kept in sync with the versions used by `sigs.k8s.io/controlle
 - A new timeout `nodeVolumeDetachTimeout` has been introduced that defines how long the controller will spend on waiting for all volumes to be detached.
 The default value is 0, meaning that the volume can be detached without any time limitations.
 - A new annotation `machine.cluster.x-k8s.io/exclude-wait-for-node-volume-detach` has been introduced that allows explicitly skip the waiting for node volume detaching.
+- The `Path` func in the `sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository.Overrider` interface has been adjusted to also return an error.
 
 ### Other