From 13fe1aa3957f77a1fe4368ea83e95efe54ce49a9 Mon Sep 17 00:00:00 2001
From: Wouter Dullaert <wouter.dullaert@exoscale.ch>
Date: Thu, 2 Nov 2023 17:59:50 +0100
Subject: [PATCH] fix(api): Do not cache server responses when refreshing API
 defs

Also ignores any CLI or env parameters passed in when querying
for API definitions: only profile specified params are used.

Fixes #216
---
 cli/api.go     | 19 +++++------
 cli/request.go | 87 ++++++++++++++++++++++++++------------------------
 2 files changed, 54 insertions(+), 52 deletions(-)

diff --git a/cli/api.go b/cli/api.go
index ab892f2..b3934d4 100644
--- a/cli/api.go
+++ b/cli/api.go
@@ -200,16 +200,13 @@ func Load(entrypoint string, root *cobra.Command) (API, error) {
 		return API{}, err
 	}
 
-	// For fetching specs, we apply a 24-hour cache time if no cache headers
-	// are set. So APIs can opt-in to caching if they want control, otherwise
-	// we try and do the right thing and not hit them too often. Localhost
-	// is never cached to make local development easier.
-	client := MinCachedTransport(24 * time.Hour).Client()
-	if viper.GetBool("rsh-no-cache") || req.URL.Hostname() == "localhost" {
-		client = &http.Client{Transport: InvalidateCachedTransport()}
-	}
-
-	httpResp, err := MakeRequest(req, WithClient(client))
+	// We already cache the parsed API specs, no need to cache the
+	// server response.
+	// We will almost never be in a situation where we don't want to use
+	// the parsed API cache, but do want to use a cached response from
+	// the server.
+	client := &http.Client{Transport: InvalidateCachedTransport()}
+	httpResp, err := MakeRequest(req, WithClient(client), IgnoreCLIParams())
 	if err != nil {
 		return API{}, err
 	}
@@ -249,7 +246,7 @@ func Load(entrypoint string, root *cobra.Command) (API, error) {
 			return API{}, err
 		}
 
-		resp, err := MakeRequest(req, WithClient(client))
+		resp, err := MakeRequest(req, WithClient(client), IgnoreCLIParams())
 		if err != nil {
 			return API{}, err
 		}
diff --git a/cli/request.go b/cli/request.go
index 32224a0..ca851a1 100644
--- a/cli/request.go
+++ b/cli/request.go
@@ -71,30 +71,40 @@ func fixAddress(addr string) string {
 	return addr
 }
 
-type requestOption struct {
-	client       *http.Client
-	disableLog   bool
-	ignoreStatus bool
+type requestConfig struct {
+	client          *http.Client
+	disableLog      bool
+	ignoreStatus    bool
+	ignoreCLIParams bool
 }
 
+type requestOption func(*requestConfig)
+
 // WithClient sets the client to use for the request.
 func WithClient(c *http.Client) requestOption {
-	return requestOption{
-		client: c,
+	return func(conf *requestConfig) {
+		conf.client = c
 	}
 }
 
 // WithoutLog disabled debug logging for the given request/response.
 func WithoutLog() requestOption {
-	return requestOption{
-		disableLog: true,
+	return func(conf *requestConfig) {
+		conf.disableLog = true
 	}
 }
 
 // IgnoreStatus ignores the response status code.
 func IgnoreStatus() requestOption {
-	return requestOption{
-		ignoreStatus: true,
+	return func(conf *requestConfig) {
+		conf.ignoreStatus = true
+	}
+}
+
+// IgnoreCLIParams only applies the profile, but ignores commandline and env params
+func IgnoreCLIParams() requestOption {
+	return func(conf *requestConfig) {
+		conf.ignoreCLIParams = true
 	}
 }
 
@@ -103,6 +113,11 @@ func IgnoreStatus() requestOption {
 // before sending it out on the wire. If verbose mode is enabled, it will
 // print out both the request and response.
 func MakeRequest(req *http.Request, options ...requestOption) (*http.Response, error) {
+	requestConf := &requestConfig{}
+	for _, opt := range options {
+		opt(requestConf)
+	}
+
 	name, config := findAPI(req.URL.String())
 
 	if config == nil {
@@ -134,25 +149,27 @@ func MakeRequest(req *http.Request, options ...requestOption) (*http.Response, e
 		}
 	}
 
-	// Allow env vars and commandline arguments to override config.
-	for _, h := range viper.GetStringSlice("rsh-header") {
-		parts := strings.SplitN(h, ":", 2)
-		value := ""
-		if len(parts) > 1 {
-			value = parts[1]
+	if !requestConf.ignoreCLIParams {
+		// Allow env vars and commandline arguments to override config.
+		for _, h := range viper.GetStringSlice("rsh-header") {
+			parts := strings.SplitN(h, ":", 2)
+			value := ""
+			if len(parts) > 1 {
+				value = parts[1]
+			}
+
+			req.Header.Add(parts[0], value)
 		}
 
-		req.Header.Add(parts[0], value)
-	}
+		for _, q := range viper.GetStringSlice("rsh-query") {
+			parts := strings.SplitN(q, "=", 2)
+			value := ""
+			if len(parts) > 1 {
+				value = parts[1]
+			}
 
-	for _, q := range viper.GetStringSlice("rsh-query") {
-		parts := strings.SplitN(q, "=", 2)
-		value := ""
-		if len(parts) > 1 {
-			value = parts[1]
+			query.Add(parts[0], value)
 		}
-
-		query.Add(parts[0], value)
 	}
 
 	// Save modified query string arguments.
@@ -243,28 +260,16 @@ func MakeRequest(req *http.Request, options ...requestOption) (*http.Response, e
 		client = &http.Client{Transport: InvalidateCachedTransport()}
 	}
 
-	log := true
-	setStatus := true
-	for _, option := range options {
-		if option.client != nil {
-			client = option.client
-		}
-
-		if option.disableLog {
-			log = false
-		}
-
-		if option.ignoreStatus {
-			setStatus = false
-		}
+	if requestConf.client != nil {
+		client = requestConf.client
 	}
 
-	resp, err := doRequestWithRetry(log, client, req)
+	resp, err := doRequestWithRetry(!requestConf.disableLog, client, req)
 	if err != nil {
 		return nil, err
 	}
 
-	if setStatus {
+	if !requestConf.ignoreStatus {
 		lastStatus = resp.StatusCode
 	}